Xen (HVM) and NMI

[Roger Pau Monné]

On Fri, Nov 08, 2019 at 10:19:01AM +0200, Andriy Gapon wrote:
> On 08/11/2019 10:03, Andriy Gapon wrote:
> > On 07/11/2019 20:08, Andriy Gapon wrote:
> >> For CPUs that do get interrupted I see stack traces like:
> >> cpustop_handler+0x28 ipi_nmi_handler+0x44 xen_cpustophard_handler+0x9
> >> intr_event_handle+0x8b intr_execute_handlers+0x58 xen_intr_handle_upcall+0x15a
> >> xen_intr_upcall_u+0x96 ...
> >> So, it looks like the NMI is delivered by the same mechanism as normal
> >> interrupts.  If a processor has interrupts disabled then the NMI would not get
> >> delivered?

Yes, sorry, I certainly didn't code this correctly regarding NMI
handling.

> >> Is there anything we could do to improve this?
> > 
> > I found this in Linux code:
> >     HYPERVISOR_vcpu_op(VCPUOP_send_nmi, xen_vcpu_nr(cpu), NULL);
> > It's in xen_send_IPI_one().
> > I wonder if that's that or if there is more to this than meets the eye.

Yes, something along this lines should work, we could even use the
native NMI signaling using the local APIC, but the hypercall shortcut
should be faster in most cases.

> I also found this in an old post.
> Ian Campbell wrote:
> > You need to register a callback with CALLBACKOP_register
> > CALLBACKTYPE_nmi. You also need to write the code in entry.S to receive
> > that callback. IIRC you also need to arrange that returning from an NMI
> > is always done with HYPERVISOR_iret and not optimised to a direct iret
> > as it can be otherwise. This is to allow the hypervisor to implement NMI
> > masking correctly.

That's AFAIK for PV guests which use a completely different mechanism
in order to receive interrupts. None of this is needed for FreeBSD
because there's no classic PV support.

Can you try the patch below?

Roger.
---8<---
diff --git a/sys/x86/xen/xen_apic.c b/sys/x86/xen/xen_apic.c
index 7d254ef3f734..7a6964d60cdb 100644
--- a/sys/x86/xen/xen_apic.c
+++ b/sys/x86/xen/xen_apic.c
@@ -72,7 +72,6 @@ static driver_filter_t xen_invlcache;
 static driver_filter_t xen_ipi_bitmap_handler;
 static driver_filter_t xen_cpustop_handler;
 static driver_filter_t xen_cpususpend_handler;
-static driver_filter_t xen_cpustophard_handler;
 #endif
 
 /*---------------------------------- Macros ----------------------------------*/
@@ -96,7 +95,6 @@ static struct xen_ipi_handler xen_ipis[] =
 	[IPI_TO_IDX(IPI_BITMAP_VECTOR)] = { xen_ipi_bitmap_handler,	"b"   },
 	[IPI_TO_IDX(IPI_STOP)]		= { xen_cpustop_handler,	"st"  },
 	[IPI_TO_IDX(IPI_SUSPEND)]	= { xen_cpususpend_handler,	"sp"  },
-	[IPI_TO_IDX(IPI_STOP_HARD)]	= { xen_cpustophard_handler,	"sth" },
 };
 #endif
 
@@ -259,12 +257,52 @@ xen_pv_lapic_ipi_raw(register_t icrlo, u_int dest)
 	XEN_APIC_UNSUPPORTED;
 }
 
+#define PCPU_ID_GET(id, field) (pcpu_find(id)->pc_##field)
+static void
+send_nmi(int dest)
+{
+	unsigned int cpu;
+
+	/*
+	 * NMIs are not routed over event channels, and instead delivered as on
+	 * native using the exception vector (#2). Triggering them can be done
+	 * using the local APIC, or an hypercall as a shortcut like it's done
+	 * below.
+	 */
+	switch(dest) {
+	case APIC_IPI_DEST_SELF:
+		HYPERVISOR_vcpu_op(VCPUOP_send_nmi, PCPU_GET(vcpu_id), NULL);
+		break;
+	case APIC_IPI_DEST_ALL:
+		CPU_FOREACH(cpu)
+			HYPERVISOR_vcpu_op(VCPUOP_send_nmi,
+			    PCPU_ID_GET(cpu, vcpu_id), NULL);
+		break;
+	case APIC_IPI_DEST_OTHERS:
+		CPU_FOREACH(cpu)
+			if (cpu != PCPU_GET(cpuid))
+				HYPERVISOR_vcpu_op(VCPUOP_send_nmi,
+				    PCPU_ID_GET(cpu, vcpu_id), NULL);
+		break;
+	default:
+		HYPERVISOR_vcpu_op(VCPUOP_send_nmi, PCPU_ID_GET(dest, vcpu_id),
+		    NULL);
+		break;
+	}
+}
+#undef PCPU_ID_GET
+
 static void
 xen_pv_lapic_ipi_vectored(u_int vector, int dest)
 {
 	xen_intr_handle_t *ipi_handle;
 	int ipi_idx, to_cpu, self;
 
+	if (vector >= IPI_NMI_FIRST) {
+		send_nmi(dest);
+		return;
+	}
+
 	ipi_idx = IPI_TO_IDX(vector);
 	if (ipi_idx >= nitems(xen_ipis))
 		panic("IPI out of range");
@@ -522,14 +560,6 @@ xen_cpususpend_handler(void *arg)
 	return (FILTER_HANDLED);
 }
 
-static int
-xen_cpustophard_handler(void *arg)
-{
-
-	ipi_nmi_handler();
-	return (FILTER_HANDLED);
-}
-
 /*----------------------------- XEN PV IPI setup -----------------------------*/
 /*
  * Those functions are provided outside of the Xen PV APIC implementation