xen: Fix event channel callback via INTX/GSI

For a while, event channel notification via the PCI platform device
has been broken, because we attempt to communicate with xenstore before
we even have notifications working, with the xs_reset_watches() call
in xs_init().

We tend to get away with this on Xen versions below 4.0 because we avoid
calling xs_reset_watches() anyway, because xenstore might not cope with
reading a non-existent key. And newer Xen *does* have the vector
callback support, so we rarely fall back to INTX/GSI delivery.

To fix it, clean up a bit of the mess of xs_init() and xenbus_probe()
startup. Call xs_init() directly from xenbus_init() only in the !XS_HVM
case, deferring it to be called from xenbus_probe() in the XS_HVM case
instead.

Then fix up the invocation of xenbus_probe() to happen either from its
device_initcall if the callback is available early enough, or when the
callback is finally set up. This means that the hack of calling
xenbus_probe() from a workqueue after the first interrupt, or directly
from the PCI platform device setup, is no longer needed.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Link: https://lore.kernel.org/r/20210113132606.422794-2-dwmw2@infradead.org
Signed-off-by: Juergen Gross <jgross@suse.com>

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 60e901cd0de6a31af593f4100c21ee2e3edc3def..5a957a9a09843c1031b7a9a3a46d5a454c417867 100644 (file)
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -371,7 +371,7 @@ static int __init xen_guest_init(void)
        }
        gnttab_init();
        if (!xen_initial_domain())
-               xenbus_probe(NULL);
+               xenbus_probe();
 
        /*
         * Making sure board specific code will not set up ops for

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 6038c4c35db5ad55259ecb08208e17d52759682c..bbebe248b72647d631ab9838ae6f43ad3363ba2a 100644 (file)
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2010,16 +2010,6 @@ static struct irq_chip xen_percpu_chip __read_mostly = {
        .irq_ack                = ack_dynirq,
 };
 
-int xen_set_callback_via(uint64_t via)
-{
-       struct xen_hvm_param a;
-       a.domid = DOMID_SELF;
-       a.index = HVM_PARAM_CALLBACK_IRQ;
-       a.value = via;
-       return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
-}
-EXPORT_SYMBOL_GPL(xen_set_callback_via);
-
 #ifdef CONFIG_XEN_PVHVM
 /* Vector callbacks are better than PCI interrupts to receive event
  * channel notifications because we can receive vector callbacks on any

diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index dd911e1ff782cc4d54556d1d034fa9ed15c0b7bd..9db557b76511b235a17416fc7b881cf3ff496f90 100644 (file)
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -149,7 +149,6 @@ static int platform_pci_probe(struct pci_dev *pdev,
        ret = gnttab_init();
        if (ret)
                goto grant_out;
-       xenbus_probe(NULL);
        return 0;
 grant_out:
        gnttab_free_auto_xlat_frames();

diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h
index 2a93b7c9c159929557f845c549b7f7676efd3a11..dc153733541441ed624066829d88b407effe429a 100644 (file)
--- a/drivers/xen/xenbus/xenbus.h
+++ b/drivers/xen/xenbus/xenbus.h
@@ -115,6 +115,7 @@ int xenbus_probe_node(struct xen_bus_type *bus,
                      const char *type,
                      const char *nodename);
 int xenbus_probe_devices(struct xen_bus_type *bus);
+void xenbus_probe(void);
 
 void xenbus_dev_changed(const char *node, struct xen_bus_type *bus);
 

diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c
index eb5151fc8efab02899ce764b99a8d6f2955e9ca7..e5fda0256feb3d476aeee5f2d1e28ac7bd29f226 100644 (file)
--- a/drivers/xen/xenbus/xenbus_comms.c
+++ b/drivers/xen/xenbus/xenbus_comms.c
@@ -57,16 +57,8 @@ DEFINE_MUTEX(xs_response_mutex);
 static int xenbus_irq;
 static struct task_struct *xenbus_task;
 
-static DECLARE_WORK(probe_work, xenbus_probe);
-
-
 static irqreturn_t wake_waiting(int irq, void *unused)
 {
-       if (unlikely(xenstored_ready == 0)) {
-               xenstored_ready = 1;
-               schedule_work(&probe_work);
-       }
-
        wake_up(&xb_waitq);
        return IRQ_HANDLED;
 }

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 44634d970a5ca1f3bb4b0a1278e245121cd69135..c8f0282bb64975b34f44459c0565156293b0ea64 100644 (file)
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -683,29 +683,76 @@ void unregister_xenstore_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(unregister_xenstore_notifier);
 
-void xenbus_probe(struct work_struct *unused)
+void xenbus_probe(void)
 {
        xenstored_ready = 1;
 
+       /*
+        * In the HVM case, xenbus_init() deferred its call to
+        * xs_init() in case callbacks were not operational yet.
+        * So do it now.
+        */
+       if (xen_store_domain_type == XS_HVM)
+               xs_init();
+
        /* Notify others that xenstore is up */
        blocking_notifier_call_chain(&xenstore_chain, 0, NULL);
 }
-EXPORT_SYMBOL_GPL(xenbus_probe);
 
-static int __init xenbus_probe_initcall(void)
+/*
+ * Returns true when XenStore init must be deferred in order to
+ * allow the PCI platform device to be initialised, before we
+ * can actually have event channel interrupts working.
+ */
+static bool xs_hvm_defer_init_for_callback(void)
 {
-       if (!xen_domain())
-               return -ENODEV;
+#ifdef CONFIG_XEN_PVHVM
+       return xen_store_domain_type == XS_HVM &&
+               !xen_have_vector_callback;
+#else
+       return false;
+#endif
+}
 
-       if (xen_initial_domain() || xen_hvm_domain())
-               return 0;
+static int __init xenbus_probe_initcall(void)
+{
+       /*
+        * Probe XenBus here in the XS_PV case, and also XS_HVM unless we
+        * need to wait for the platform PCI device to come up.
+        */
+       if (xen_store_domain_type == XS_PV ||
+           (xen_store_domain_type == XS_HVM &&
+            !xs_hvm_defer_init_for_callback()))
+               xenbus_probe();
 
-       xenbus_probe(NULL);
        return 0;
 }
-
 device_initcall(xenbus_probe_initcall);
 
+int xen_set_callback_via(uint64_t via)
+{
+       struct xen_hvm_param a;
+       int ret;
+
+       a.domid = DOMID_SELF;
+       a.index = HVM_PARAM_CALLBACK_IRQ;
+       a.value = via;
+
+       ret = HYPERVISOR_hvm_op(HVMOP_set_param, &a);
+       if (ret)
+               return ret;
+
+       /*
+        * If xenbus_probe_initcall() deferred the xenbus_probe()
+        * due to the callback not functioning yet, we can do it now.
+        */
+       if (!xenstored_ready && xs_hvm_defer_init_for_callback())
+               xenbus_probe();
+
+       return ret;
+}
+EXPORT_SYMBOL_GPL(xen_set_callback_via);
+
 /* Set up event channel for xenstored which is run as a local process
  * (this is normally used only in dom0)
  */
@@ -818,11 +865,17 @@ static int __init xenbus_init(void)
                break;
        }
 
-       /* Initialize the interface to xenstore. */
-       err = xs_init();
-       if (err) {
-               pr_warn("Error initializing xenstore comms: %i\n", err);
-               goto out_error;
+       /*
+        * HVM domains may not have a functional callback yet. In that
+        * case let xs_init() be called from xenbus_probe(), which will
+        * get invoked at an appropriate time.
+        */
+       if (xen_store_domain_type != XS_HVM) {
+               err = xs_init();
+               if (err) {
+                       pr_warn("Error initializing xenstore comms: %i\n", err);
+                       goto out_error;
+               }
        }
 
        if ((xen_store_domain_type != XS_LOCAL) &&

diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 00c7235ae93e7daae3ce106f76fc5dfce1cd1bdd..2c43b0ef1e4d504a652d0d574d5b6e41fa107091 100644 (file)
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -192,7 +192,7 @@ void xs_suspend_cancel(void);
 
 struct work_struct;
 
-void xenbus_probe(struct work_struct *);
+void xenbus_probe(void);
 
 #define XENBUS_IS_ERR_READ(str) ({                     \
        if (!IS_ERR(str) && strlen(str) == 0) {         \