On Thu, Dec 22, 2016 at 10:13:44AM +0000, Roger Pau Monné wrote:
> On Wed, Dec 21, 2016 at 11:30:36PM +0000, Colin Percival wrote:
> > This commit breaks the Xen boot:
> > > panic: NULL pcpu device_t
> > 
> > > cpuid = 0
> > 
> > > KDB: stack backtrace:
> > 
> > > db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 
> > > 0xffffffff82233a20
> > 
> > > vpanic() at vpanic+0x186/frame 0xffffffff82233aa0
> > 
> > > kassert_panic() at kassert_panic+0x126/frame 0xffffffff82233b10
> > 
> > > xen_setup_cpus() at xen_setup_cpus+0x5b/frame 0xffffffff82233b50
> > 
> > > mi_startup() at mi_startup+0x118/frame 0xffffffff82233b70
> > 
> > > btext() at btext+0x2c
> > 
> > 
> > I'm assuming this means xenpvcpu_attach isn't running early enough; does
> > anyone have time to track this down and fix it?
> 
> xen_intr_alloc_and_bind_ipi requires a non-NULL device_t, and with the changes
> to the AP startup code now xen_cpu_ipi_init gets called before the dummy pcpu
> devices are attached. I will try to find some time this afternoon/tomorrow in
> order to rework the ipi bind function so that it no longer requires a device_t
> (it seems kind of pointless anyway, since IPIs by definition always belong to
> the CPU package).

I've been able to successfully boot FreeBSD in Dom0 mode using the following
patch, I think it should also work for a PVH/HVM DomU, could someone please
give it a spin?

Thanks.

---8<---
diff --git a/sys/dev/xen/evtchn/evtchn_dev.c b/sys/dev/xen/evtchn/evtchn_dev.c
index 5d54d6d..df81251 100644
--- a/sys/dev/xen/evtchn/evtchn_dev.c
+++ b/sys/dev/xen/evtchn/evtchn_dev.c
@@ -373,9 +373,9 @@ evtchn_bind_user_port(struct per_user_data *u, struct 
user_evtchn *evtchn)
        mtx_lock(&u->bind_mutex);
        RB_INSERT(evtchn_tree, &u->evtchns, evtchn);
        mtx_unlock(&u->bind_mutex);
-       error = xen_intr_add_handler(evtchn_dev, evtchn_filter,
-           evtchn_interrupt, evtchn, INTR_TYPE_MISC | INTR_MPSAFE,
-           evtchn->handle);
+       error = xen_intr_add_handler(device_get_nameunit(evtchn_dev),
+           evtchn_filter, evtchn_interrupt, evtchn,
+           INTR_TYPE_MISC | INTR_MPSAFE, evtchn->handle);
        if (error != 0) {
                xen_intr_unbind(&evtchn->handle);
                mtx_lock(&u->bind_mutex);
diff --git a/sys/x86/xen/xen_apic.c b/sys/x86/xen/xen_apic.c
index 45c3c18..a7fa255 100644
--- a/sys/x86/xen/xen_apic.c
+++ b/sys/x86/xen/xen_apic.c
@@ -500,12 +500,9 @@ xen_cpu_ipi_init(int cpu)
 {
        xen_intr_handle_t *ipi_handle;
        const struct xen_ipi_handler *ipi;
-       device_t dev;
        int idx, rc;
 
        ipi_handle = DPCPU_ID_GET(cpu, ipi_handle);
-       dev = pcpu_find(cpu)->pc_device;
-       KASSERT((dev != NULL), ("NULL pcpu device_t"));
 
        for (ipi = xen_ipis, idx = 0; idx < nitems(xen_ipis); ipi++, idx++) {
 
@@ -514,7 +511,7 @@ xen_cpu_ipi_init(int cpu)
                        continue;
                }
 
-               rc = xen_intr_alloc_and_bind_ipi(dev, cpu, ipi->filter,
+               rc = xen_intr_alloc_and_bind_ipi(cpu, ipi->filter,
                    INTR_TYPE_TTY, &ipi_handle[idx]);
                if (rc != 0)
                        panic("Unable to allocate a XEN IPI port");
diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c
index 9c78306..3f6d190 100644
--- a/sys/x86/xen/xen_intr.c
+++ b/sys/x86/xen/xen_intr.c
@@ -392,7 +392,7 @@ xen_intr_release_isrc(struct xenisrc *isrc)
  */
 static int
 xen_intr_bind_isrc(struct xenisrc **isrcp, evtchn_port_t local_port,
-    enum evtchn_type type, device_t intr_owner, driver_filter_t filter,
+    enum evtchn_type type, const char *intr_owner, driver_filter_t filter,
     driver_intr_t handler, void *arg, enum intr_type flags,
     xen_intr_handle_t *port_handlep)
 {
@@ -401,8 +401,8 @@ xen_intr_bind_isrc(struct xenisrc **isrcp, evtchn_port_t 
local_port,
 
        *isrcp = NULL;
        if (port_handlep == NULL) {
-               device_printf(intr_owner,
-                             "xen_intr_bind_isrc: Bad event handle\n");
+               printf("%s: xen_intr_bind_isrc: Bad event handle\n",
+                   intr_owner);
                return (EINVAL);
        }
 
@@ -1175,8 +1175,9 @@ xen_intr_bind_local_port(device_t dev, evtchn_port_t 
local_port,
        struct xenisrc *isrc;
        int error;
 
-       error = xen_intr_bind_isrc(&isrc, local_port, EVTCHN_TYPE_PORT, dev,
-                   filter, handler, arg, flags, port_handlep);
+       error = xen_intr_bind_isrc(&isrc, local_port, EVTCHN_TYPE_PORT,
+           device_get_nameunit(dev), filter, handler, arg, flags,
+           port_handlep);
        if (error != 0)
                return (error);
 
@@ -1210,8 +1211,8 @@ xen_intr_alloc_and_bind_local_port(device_t dev, u_int 
remote_domain,
        }
 
        error = xen_intr_bind_isrc(&isrc, alloc_unbound.port, EVTCHN_TYPE_PORT,
-                                dev, filter, handler, arg, flags,
-                                port_handlep);
+           device_get_nameunit(dev), filter, handler, arg, flags,
+           port_handlep);
        if (error != 0) {
                evtchn_close_t close = { .port = alloc_unbound.port };
                if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close))
@@ -1245,8 +1246,8 @@ xen_intr_bind_remote_port(device_t dev, u_int 
remote_domain,
        }
 
        error = xen_intr_bind_isrc(&isrc, bind_interdomain.local_port,
-                                EVTCHN_TYPE_PORT, dev, filter, handler,
-                                arg, flags, port_handlep);
+           EVTCHN_TYPE_PORT, device_get_nameunit(dev), filter, handler, arg,
+           flags, port_handlep);
        if (error) {
                evtchn_close_t close = { .port = bind_interdomain.local_port };
                if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close))
@@ -1285,8 +1286,9 @@ xen_intr_bind_virq(device_t dev, u_int virq, u_int cpu,
                return (-error);
        }
 
-       error = xen_intr_bind_isrc(&isrc, bind_virq.port, EVTCHN_TYPE_VIRQ, dev,
-                                filter, handler, arg, flags, port_handlep);
+       error = xen_intr_bind_isrc(&isrc, bind_virq.port, EVTCHN_TYPE_VIRQ,
+           device_get_nameunit(dev), filter, handler, arg, flags,
+           port_handlep);
 
 #ifdef SMP
        if (error == 0)
@@ -1325,14 +1327,15 @@ xen_intr_bind_virq(device_t dev, u_int virq, u_int cpu,
 }
 
 int
-xen_intr_alloc_and_bind_ipi(device_t dev, u_int cpu,
-    driver_filter_t filter, enum intr_type flags,
-    xen_intr_handle_t *port_handlep)
+xen_intr_alloc_and_bind_ipi(u_int cpu, driver_filter_t filter,
+    enum intr_type flags, xen_intr_handle_t *port_handlep)
 {
 #ifdef SMP
        int vcpu_id = pcpu_find(cpu)->pc_vcpu_id;
        struct xenisrc *isrc;
        struct evtchn_bind_ipi bind_ipi = { .vcpu = vcpu_id };
+       /* Same size as the one used by intr_handler->ih_name. */
+       char name[MAXCOMLEN + 1];
        int error;
 
        /* Ensure the target CPU is ready to handle evtchn interrupts. */
@@ -1348,12 +1351,10 @@ xen_intr_alloc_and_bind_ipi(device_t dev, u_int cpu,
                return (-error);
        }
 
-       error = xen_intr_bind_isrc(&isrc, bind_ipi.port, EVTCHN_TYPE_IPI,
-                                  dev, filter, NULL, NULL, flags,
-                                  port_handlep);
-       if (error == 0)
-               error = intr_event_bind(isrc->xi_intsrc.is_event, cpu);
+       snprintf(name, sizeof(name), "cpu%u", cpu);
 
+       error = xen_intr_bind_isrc(&isrc, bind_ipi.port, EVTCHN_TYPE_IPI,
+           name, filter, NULL, NULL, flags, port_handlep);
        if (error != 0) {
                evtchn_close_t close = { .port = bind_ipi.port };
 
@@ -1549,7 +1550,7 @@ xen_intr_port(xen_intr_handle_t handle)
 }
 
 int
-xen_intr_add_handler(device_t dev, driver_filter_t filter,
+xen_intr_add_handler(const char *name, driver_filter_t filter,
     driver_intr_t handler, void *arg, enum intr_type flags,
     xen_intr_handle_t handle)
 {
@@ -1560,12 +1561,12 @@ xen_intr_add_handler(device_t dev, driver_filter_t 
filter,
        if (isrc == NULL || isrc->xi_cookie != NULL)
                return (EINVAL);
 
-       error = intr_add_handler(device_get_nameunit(dev), isrc->xi_vector,
-           filter, handler, arg, flags|INTR_EXCL, &isrc->xi_cookie);
+       error = intr_add_handler(name, isrc->xi_vector,filter, handler, arg,
+           flags|INTR_EXCL, &isrc->xi_cookie);
        if (error != 0) {
-               device_printf(dev,
-                   "xen_intr_add_handler: intr_add_handler failed: %d\n",
-                   error);
+               printf(
+                   "%s: xen_intr_add_handler: intr_add_handler failed: %d\n",
+                   name, error);
        }
 
        return (error);
diff --git a/sys/xen/xen_intr.h b/sys/xen/xen_intr.h
index fc419d6..eb3436d 100644
--- a/sys/xen/xen_intr.h
+++ b/sys/xen/xen_intr.h
@@ -143,7 +143,6 @@ int xen_intr_bind_virq(device_t dev, u_int virq, u_int cpu,
  * interupts and, if successful, associate the port with the specified
  * interrupt handler.
  *
- * \param dev       The device making this bind request.
  * \param cpu       The cpu receiving the IPI.
  * \param filter    The interrupt filter servicing this IPI.
  * \param irqflags  Interrupt handler flags.  See sys/bus.h.
@@ -152,7 +151,7 @@ int xen_intr_bind_virq(device_t dev, u_int virq, u_int cpu,
  *
  * \returns  0 on success, otherwise an errno.
  */
-int xen_intr_alloc_and_bind_ipi(device_t dev, u_int cpu,
+int xen_intr_alloc_and_bind_ipi(u_int cpu,
        driver_filter_t filter, enum intr_type irqflags,
        xen_intr_handle_t *handlep);
 
@@ -259,7 +258,7 @@ int xen_release_msi(int vector);
  *
  * \returns  0 on success, otherwise an errno.
  */
-int xen_intr_add_handler(device_t dev, driver_filter_t filter,
+int xen_intr_add_handler(const char *name, driver_filter_t filter,
        driver_intr_t handler, void *arg, enum intr_type flags,
        xen_intr_handle_t handle);
 

_______________________________________________
freebsd-xen@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-xen
To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"

Reply via email to