Date: Fri, 8 Nov 2019 15:00:49 +0100 From: Roger Pau =?iso-8859-1?Q?Monn=E9?= <roger.pau@citrix.com> To: Andriy Gapon <avg@FreeBSD.org> Cc: <freebsd-xen@FreeBSD.org> Subject: Re: Xen (HVM) and NMI Message-ID: <20191108140049.GW14005@Air-de-Roger> In-Reply-To: <760f4fe2-8a5b-87a7-f84f-f250bb4eb31c@FreeBSD.org> References: <e543673e-d55f-af0d-5aed-b00ad75de7cc@FreeBSD.org> <62c12d1e-658c-93fc-fad8-3d97d45b4dd1@FreeBSD.org> <a95f4a92-dc5f-35e2-9f81-b26c2b3d0369@FreeBSD.org> <20191108095754.GV14005@Air-de-Roger> <760f4fe2-8a5b-87a7-f84f-f250bb4eb31c@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 08, 2019 at 03:15:40PM +0200, Andriy Gapon wrote:
> On 08/11/2019 11:57, Roger Pau Monné wrote:
> > On Fri, Nov 08, 2019 at 10:19:01AM +0200, Andriy Gapon wrote:
> >>> 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.
>
> Perhaps HYPERVISOR_iret is still needed even for us?
No, that's just for classic PV.
> This is said with zero knowledge of the topic.
>
> > Can you try the patch below?
>
> I tried it and, unfortunately, it was not a success. And I don't have much
> diagnostic.
My bad, that's what happens when you send untested patches, the
previous patch was missing an apic_cpuid and was hitting a page-fault
in the kernel. Patch below should work fine.
Roger.
---8<---
diff --git a/sys/x86/xen/xen_apic.c b/sys/x86/xen/xen_apic.c
index 7d254ef3f734..8bf2158dbec2 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(apic_cpuid(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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20191108140049.GW14005>