All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes
@ 2021-01-05 23:57 David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 1/5] xen: Fix event channel callback via INTX/GSI David Woodhouse
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

Fix various breakages with INTX/GSI event channel interrupt delivery,
and add a command line option to test it so that it hopefully doesn't
get so broken again.

Karim attempted to rip this out in 2017 but put it back because it's still
necessary with older versions of Xen. This fixes it properly, and makes it
easier to test. cf. https://lkml.org/lkml/2017/4/10/266

v2:
 • Add no_vector_callback to Documentation/admin-guide/kernel-parameters.txt
 • Further fixes to conditional smp_ops initialisation in xen_hvm_smp_init()

David Woodhouse (5):
      xen: Fix event channel callback via INTX/GSI
      xen: Set platform PCI device INTX affinity to CPU0
      x86/xen: Add a no_vector_callback option to test PCI INTX delivery
      x86/xen: Don't register Xen IPIs when they aren't going to be used
      x86/xen: Fix xen_hvm_smp_init() when vector callback not available

 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/arm/xen/enlighten.c                        |  2 +-
 arch/x86/xen/enlighten_hvm.c                    | 15 ++++++++++++---
 arch/x86/xen/smp_hvm.c                          | 27 +++++++++++++++++----------
 drivers/xen/events/events_base.c                | 10 ----------
 drivers/xen/platform-pci.c                      |  8 +++++++-
 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 +-
 10 files changed, 97 insertions(+), 48 deletions(-)





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/5] xen: Fix event channel callback via INTX/GSI
  2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
@ 2021-01-05 23:57 ` David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 2/5] xen: Set platform PCI device INTX affinity to CPU0 David Woodhouse
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

From: David Woodhouse <dwmw@amazon.co.uk>

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>
---
 arch/arm/xen/enlighten.c          |  2 +-
 drivers/xen/events/events_base.c  | 10 -----
 drivers/xen/platform-pci.c        |  1 -
 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 +-
 7 files changed, 57 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/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index a8030332a191..e850f79351cb 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -2060,16 +2060,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..9db557b76511 100644
--- 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 2a93b7c9c159..dc1537335414 100644
--- 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 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 44634d970a5c..b1b5b6fe9b52 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -683,29 +683,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)
  */
@@ -818,11 +852,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 00c7235ae93e..2c43b0ef1e4d 100644
--- 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) {		\
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/5] xen: Set platform PCI device INTX affinity to CPU0
  2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 1/5] xen: Fix event channel callback via INTX/GSI David Woodhouse
@ 2021-01-05 23:57 ` David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery David Woodhouse
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

From: David Woodhouse <dwmw@amazon.co.uk>

With INTX or GSI delivery, Xen uses the event channel structures of CPU0.

If the interrupt gets handled by Linux on a different CPU, then no events
are seen as pending. Rather than introducing locking to allow other CPUs
to process CPU0's events, just ensure that the PCI interrupts happens
only on CPU0.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 drivers/xen/platform-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 9db557b76511..18f0ed8b1f93 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) {
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery
  2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 1/5] xen: Fix event channel callback via INTX/GSI David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 2/5] xen: Set platform PCI device INTX affinity to CPU0 David Woodhouse
@ 2021-01-05 23:57 ` David Woodhouse
  2021-01-06 14:46   ` boris.ostrovsky
  2021-01-05 23:57 ` [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used David Woodhouse
  2021-01-05 23:57 ` [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available David Woodhouse
  4 siblings, 1 reply; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

From: David Woodhouse <dwmw@amazon.co.uk>

It's useful to be able to test non-vector event channel delivery, to make
sure Linux will work properly on older Xen which doesn't have it.

It's also useful for those working on Xen and Xen-compatible hypervisors,
because there are guest kernels still in active use which use PCI INTX
even when vector delivery is available.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/x86/xen/enlighten_hvm.c                    | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c722ec19cd00..35b10982d363 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3389,6 +3389,10 @@
 
 	nosync		[HW,M68K] Disables sync negotiation for all devices.
 
+	no_vector_callback
+			[KNL,X86,XEN] Disable the vector callback for Xen
+			event channel interrupts.
+
 	nowatchdog	[KNL] Disable both lockup detectors, i.e.
 			soft-lockup and NMI watchdog (hard-lockup).
 
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())
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used
  2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
                   ` (2 preceding siblings ...)
  2021-01-05 23:57 ` [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery David Woodhouse
@ 2021-01-05 23:57 ` David Woodhouse
  2021-01-06 14:47   ` boris.ostrovsky
  2021-01-05 23:57 ` [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available David Woodhouse
  4 siblings, 1 reply; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

From: David Woodhouse <dwmw@amazon.co.uk>

In the case where xen_have_vector_callback is false, we still register
the IPI vectors in xen_smp_intr_init() for the secondary CPUs even
though they aren't going to be used. Stop doing that.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/xen/enlighten_hvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index a1c07e0c888e..7a6ef517e81a 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -164,10 +164,10 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
 	else
 		per_cpu(xen_vcpu_id, cpu) = cpu;
 	rc = xen_vcpu_setup(cpu);
-	if (rc)
+	if (rc || !xen_have_vector_callback)
 		return rc;
 
-	if (xen_have_vector_callback && xen_feature(XENFEAT_hvm_safe_pvclock))
+	if (xen_feature(XENFEAT_hvm_safe_pvclock))
 		xen_setup_timer(cpu);
 
 	rc = xen_smp_intr_init(cpu);
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available
  2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
                   ` (3 preceding siblings ...)
  2021-01-05 23:57 ` [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used David Woodhouse
@ 2021-01-05 23:57 ` David Woodhouse
  2021-01-06 14:48   ` boris.ostrovsky
  4 siblings, 1 reply; 10+ messages in thread
From: David Woodhouse @ 2021-01-05 23:57 UTC (permalink / raw)
  To: x86
  Cc: Stefano Stabellini, Boris Ostrovsky, Juergen Gross, Paul Durrant,
	jgrall, karahmed, xen-devel

From: David Woodhouse <dwmw@amazon.co.uk>

Only the IPI-related functions in the smp_ops should be conditional
on the vector callback being available. The rest should still happen:

 • xen_hvm_smp_prepare_boot_cpu()

   This function does two things, both of which should still happen if
   there is no vector callback support.

   The call to xen_vcpu_setup() for vCPU0 should still happen as it just
   sets up the vcpu_info for CPU0. That does happen for the secondary
   vCPUs too, from xen_cpu_up_prepare_hvm().

   The second thing it does is call xen_init_spinlocks(), which perhaps
   counter-intuitively should *also* still be happening in the case
   without vector callbacks, so that it can clear its local xen_pvspin
   flag and disable the virt_spin_lock_key accordingly.

   Checking xen_have_vector_callback in xen_init_spinlocks() itself
   would affect PV guests, so set the global nopvspin flag in
   xen_hvm_smp_init() instead, when vector callbacks aren't available.

 • xen_hvm_smp_prepare_cpus()

   This does some IPI-related setup by calling xen_smp_intr_init() and
   xen_init_lock_cpu(), which can be made conditional. And it sets the
   xen_vcpu_id to XEN_VCPU_ID_INVALID for all possible CPUS, which does
   need to happen.

 • xen_smp_cpus_done()

   This offlines any vCPUs which doesn't fit in the global shared_info
   page, if separate vcpu_info placement isn't available. That part also
   needs to happen regardless of vector callback support.

 • xen_hvm_cpu_die()

   This doesn't actually do anything other than commin_cpu_die() right
   right now in the !vector_callback case; all three teardown functions
   it calls should be no-ops. But to guard against future regressions
   it's useful to call it anyway, and for it to explicitly check for
   xen_have_vector_callback before calling those additional functions.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/xen/smp_hvm.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index f5e7db4f82ab..056430a1080b 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -33,9 +33,11 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus)
 	int cpu;
 
 	native_smp_prepare_cpus(max_cpus);
-	WARN_ON(xen_smp_intr_init(0));
 
-	xen_init_lock_cpu(0);
+	if (xen_have_vector_callback) {
+		WARN_ON(xen_smp_intr_init(0));
+		xen_init_lock_cpu(0);
+	}
 
 	for_each_possible_cpu(cpu) {
 		if (cpu == 0)
@@ -50,9 +52,11 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus)
 static void xen_hvm_cpu_die(unsigned int cpu)
 {
 	if (common_cpu_die(cpu) == 0) {
-		xen_smp_intr_free(cpu);
-		xen_uninit_lock_cpu(cpu);
-		xen_teardown_timer(cpu);
+		if (xen_have_vector_callback) {
+			xen_smp_intr_free(cpu);
+			xen_uninit_lock_cpu(cpu);
+			xen_teardown_timer(cpu);
+		}
 	}
 }
 #else
@@ -64,14 +68,17 @@ static void xen_hvm_cpu_die(unsigned int cpu)
 
 void __init xen_hvm_smp_init(void)
 {
-	if (!xen_have_vector_callback)
+	smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
+	smp_ops.smp_prepare_cpus = xen_hvm_smp_prepare_cpus;
+	smp_ops.smp_cpus_done = xen_smp_cpus_done;
+	smp_ops.cpu_die = xen_hvm_cpu_die;
+
+	if (!xen_have_vector_callback) {
+		nopvspin = true;
 		return;
+	}
 
-	smp_ops.smp_prepare_cpus = xen_hvm_smp_prepare_cpus;
 	smp_ops.smp_send_reschedule = xen_smp_send_reschedule;
-	smp_ops.cpu_die = xen_hvm_cpu_die;
 	smp_ops.send_call_func_ipi = xen_smp_send_call_function_ipi;
 	smp_ops.send_call_func_single_ipi = xen_smp_send_call_function_single_ipi;
-	smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
-	smp_ops.smp_cpus_done = xen_smp_cpus_done;
 }
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery
  2021-01-05 23:57 ` [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery David Woodhouse
@ 2021-01-06 14:46   ` boris.ostrovsky
  2021-01-06 15:44     ` David Woodhouse
  0 siblings, 1 reply; 10+ messages in thread
From: boris.ostrovsky @ 2021-01-06 14:46 UTC (permalink / raw)
  To: David Woodhouse, x86
  Cc: Stefano Stabellini, Juergen Gross, Paul Durrant, jgrall,
	karahmed, xen-devel


On 1/5/21 6:57 PM, David Woodhouse wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c722ec19cd00..35b10982d363 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3389,6 +3389,10 @@
>  
>  	nosync		[HW,M68K] Disables sync negotiation for all devices.
>  
> +	no_vector_callback
> +			[KNL,X86,XEN] Disable the vector callback for Xen
> +			event channel interrupts.


Sorry, I should have noticed this before --- I think this needs to be prefixed with "xen_". I know it's pretty long but otherwise this will be the first Xen-only option without such prefix (or without "xen." module). We already have a bunch of long names for Xen so it won't necessarily stick out.


-boris



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used
  2021-01-05 23:57 ` [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used David Woodhouse
@ 2021-01-06 14:47   ` boris.ostrovsky
  0 siblings, 0 replies; 10+ messages in thread
From: boris.ostrovsky @ 2021-01-06 14:47 UTC (permalink / raw)
  To: David Woodhouse, x86
  Cc: Stefano Stabellini, Juergen Gross, Paul Durrant, jgrall,
	karahmed, xen-devel


On 1/5/21 6:57 PM, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> In the case where xen_have_vector_callback is false, we still register
> the IPI vectors in xen_smp_intr_init() for the secondary CPUs even
> though they aren't going to be used. Stop doing that.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>



Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available
  2021-01-05 23:57 ` [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available David Woodhouse
@ 2021-01-06 14:48   ` boris.ostrovsky
  0 siblings, 0 replies; 10+ messages in thread
From: boris.ostrovsky @ 2021-01-06 14:48 UTC (permalink / raw)
  To: David Woodhouse, x86
  Cc: Stefano Stabellini, Juergen Gross, Paul Durrant, jgrall,
	karahmed, xen-devel


On 1/5/21 6:57 PM, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Only the IPI-related functions in the smp_ops should be conditional
> on the vector callback being available. The rest should still happen:
>
>  • xen_hvm_smp_prepare_boot_cpu()
>
>    This function does two things, both of which should still happen if
>    there is no vector callback support.
>
>    The call to xen_vcpu_setup() for vCPU0 should still happen as it just
>    sets up the vcpu_info for CPU0. That does happen for the secondary
>    vCPUs too, from xen_cpu_up_prepare_hvm().
>
>    The second thing it does is call xen_init_spinlocks(), which perhaps
>    counter-intuitively should *also* still be happening in the case
>    without vector callbacks, so that it can clear its local xen_pvspin
>    flag and disable the virt_spin_lock_key accordingly.
>
>    Checking xen_have_vector_callback in xen_init_spinlocks() itself
>    would affect PV guests, so set the global nopvspin flag in
>    xen_hvm_smp_init() instead, when vector callbacks aren't available.
>
>  • xen_hvm_smp_prepare_cpus()
>
>    This does some IPI-related setup by calling xen_smp_intr_init() and
>    xen_init_lock_cpu(), which can be made conditional. And it sets the
>    xen_vcpu_id to XEN_VCPU_ID_INVALID for all possible CPUS, which does
>    need to happen.
>
>  • xen_smp_cpus_done()
>
>    This offlines any vCPUs which doesn't fit in the global shared_info
>    page, if separate vcpu_info placement isn't available. That part also
>    needs to happen regardless of vector callback support.
>
>  • xen_hvm_cpu_die()
>
>    This doesn't actually do anything other than commin_cpu_die() right
>    right now in the !vector_callback case; all three teardown functions
>    it calls should be no-ops. But to guard against future regressions
>    it's useful to call it anyway, and for it to explicitly check for
>    xen_have_vector_callback before calling those additional functions.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery
  2021-01-06 14:46   ` boris.ostrovsky
@ 2021-01-06 15:44     ` David Woodhouse
  0 siblings, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2021-01-06 15:44 UTC (permalink / raw)
  To: boris.ostrovsky, x86
  Cc: Stefano Stabellini, Juergen Gross, Paul Durrant, jgrall,
	karahmed, xen-devel

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

On Wed, 2021-01-06 at 09:46 -0500, boris.ostrovsky@oracle.com wrote:
> Sorry, I should have noticed this before --- I think this needs to be
> prefixed with "xen_". I know it's pretty long but otherwise this will
> be the first Xen-only option without such prefix (or without "xen."
> module). We already have a bunch of long names for Xen so it won't
> necessarily stick out.

Done and reposted; thanks.


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-06 15:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 23:57 [PATCH v2 0/5] Xen INTX/GSI event channel delivery fixes David Woodhouse
2021-01-05 23:57 ` [PATCH v2 1/5] xen: Fix event channel callback via INTX/GSI David Woodhouse
2021-01-05 23:57 ` [PATCH v2 2/5] xen: Set platform PCI device INTX affinity to CPU0 David Woodhouse
2021-01-05 23:57 ` [PATCH v2 3/5] x86/xen: Add a no_vector_callback option to test PCI INTX delivery David Woodhouse
2021-01-06 14:46   ` boris.ostrovsky
2021-01-06 15:44     ` David Woodhouse
2021-01-05 23:57 ` [PATCH v2 4/5] x86/xen: Don't register Xen IPIs when they aren't going to be used David Woodhouse
2021-01-06 14:47   ` boris.ostrovsky
2021-01-05 23:57 ` [PATCH v2 5/5] x86/xen: Fix xen_hvm_smp_init() when vector callback not available David Woodhouse
2021-01-06 14:48   ` boris.ostrovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.