[PATCH] xen: Fix event channel callback via INTX/GSI

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

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, in the xs_reset_watches() call
called from xs_init().

This mostly doesn't matter much in practice because for Xen versions
below 4.0 we avoid 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.

But those working on Xen and Xen-compatible hypervisor implementations
do want to test that INTX/GSI delivery works correctly, as there are
still guest kernels in active use which don't use vector delivery yet.
So it's useful to have it actually working.

To that end, 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 is no longer
needed.

Add a 'no_vector_callback' command line argument to test it.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
 arch/arm/xen/enlighten.c          |  2 +-
 arch/x86/xen/enlighten_hvm.c      | 11 ++++-
 drivers/xen/events/events_base.c  | 10 -----
 drivers/xen/platform-pci.c        |  7 ++++
 drivers/xen/xenbus/xenbus.h       |  1 +
 drivers/xen/xenbus/xenbus_comms.c |  8 ----
 drivers/xen/xenbus/xenbus_probe.c | 68 ++++++++++++++++++++++++-------
 include/xen/xenbus.h              |  2 +-
 8 files changed, 74 insertions(+), 35 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 60e901cd0de6..5a957a9a0984 100644
--- 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/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 9e87ab010c82..a1c07e0c888e 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -188,6 +188,8 @@ static int xen_cpu_dead_hvm(unsigned int cpu)
        return 0;
 }
 
+static bool no_vector_callback __initdata;
+
 static void __init xen_hvm_guest_init(void)
 {
        if (xen_pv_domain())
@@ -207,7 +209,7 @@ static void __init xen_hvm_guest_init(void)
 
        xen_panic_handler_init();
 
-       if (xen_feature(XENFEAT_hvm_callback_vector))
+       if (!no_vector_callback && xen_feature(XENFEAT_hvm_callback_vector))
                xen_have_vector_callback = 1;
 
        xen_hvm_smp_init();
@@ -233,6 +235,13 @@ static __init int xen_parse_nopv(char *arg)
 }
 early_param("xen_nopv", xen_parse_nopv);
 
+static __init int xen_parse_no_vector_callback(char *arg)
+{
+       no_vector_callback = true;
+       return 0;
+}
+early_param("no_vector_callback", xen_parse_no_vector_callback);
+
 bool __init xen_hvm_need_lapic(void)
 {
        if (xen_pv_domain())
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 6038c4c35db5..bbebe248b726 100644
--- 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 dd911e1ff782..5c3015a90a73 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -132,6 +132,13 @@ static int platform_pci_probe(struct pci_dev *pdev,
                        dev_warn(&pdev->dev, "request_irq failed err=%d\n", 
ret);
                        goto out;
                }
+               /*
+                * It doesn't strictly *have* to run on CPU0 but it sure
+                * as hell better process the event channel ports delivered
+                * to CPU0.
+                */
+               irq_set_affinity(pdev->irq, cpumask_of(0));
+
                callback_via = get_callback_via(pdev);
                ret = xen_set_callback_via(callback_via);
                if (ret) {
diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h
index 5f5b8a7d5b80..05bbda51103f 100644
--- a/drivers/xen/xenbus/xenbus.h
+++ b/drivers/xen/xenbus/xenbus.h
@@ -113,6 +113,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 eb5151fc8efa..e5fda0256feb 100644
--- 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 38725d97d909..876f381b100a 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -682,29 +682,63 @@ 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)
 {
-       if (!xen_domain())
-               return -ENODEV;
-
-       if (xen_initial_domain() || xen_hvm_domain())
-               return 0;
+       /*
+        * 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, which is
+        * the (XEN_PVPVM && !xen_have_vector_callback) case.
+        */
+       if (xen_store_domain_type == XS_PV ||
+           (xen_store_domain_type == XS_HVM &&
+            (!IS_ENABLED(CONFIG_XEN_PVHVM) || xen_have_vector_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 && xen_store_domain_type == XS_HVM &&
+           IS_ENABLED(CONFIG_XEN_PVHVM) && !xen_have_vector_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)
  */
@@ -817,11 +851,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 5a8315e6d8a6..61202c83d560 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -183,7 +183,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) {         \
-- 
2.26.2

Attachment: smime.p7s
Description: S/MIME cryptographic signature