[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/events: Fix Global and Domain VIRQ tracking


  • To: Juergen Gross <jgross@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Chris Wright <chrisw@xxxxxxxxxxxx>, "Jeremy Fitzhardinge" <jeremy@xxxxxxxxxxxxx>
  • From: Jason Andryuk <jason.andryuk@xxxxxxx>
  • Date: Wed, 13 Aug 2025 11:03:28 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=suse.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fbXIz+dpf07LB8Lps4BsCGr7J8+XyURVjsYYM66Yg1Y=; b=WayCJ5Vyv1Gc4z/M1yktRmM5RRMEQse2enL59vlTX0XP2VjPrNT0gAwVoYp1uYXd24/oyp9viVYg7uuBq330wBbv1pRglUcpIuhiOSaCaeDdxVZkzcIH18oebMafVyt96cLZNMC+aKmkx032lTCJSWm53zoN/wkbFABkRsI1NG0Aw6JZwUiEMxZIwU+J3+pJnIUl90NJFR9brEF34oeP1aWkHQzJ6JEQQw7K0sD74PH4dOctqWlpSmT3ncXDtYE94XcEBlz5YdgFQQ8Ob0ccBtE91bonxHQovZH35d544PQrI5/nHCB1dMshUUM0T39p9qYm0LW71bYl3sz9730TgQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=qcPt3LjFUgL5sInauuz86Coww4uSJ64ZeGfmP7j/wGAXupyDxREFppM9z6QC0KUcpc4/HsBXvP9o8qJ9H9mNdDW3QwT4ljC6JzyJXzdmNq18wQKfK809zJ/OtOKGcBJB9rilp6LK4irym8/FobbDuEsG4mQM2pmOwH+2bsqKfAURkk08dsK64JFD7qnbj7wkYathBEpzg+N04GnS/VIEWWhzZ8THmiM1VdT2lTs3DGb9osHXk0xaiVGWNi7Ac9YGdoZPKoeTYsvyRcNLAJvadLSzf/CF0/xXLCQs9G8JfOKY08O2TfsOReKpYmsqBSDFlASuetugD+PtQoyWO6+yfA==
  • Cc: <stable@xxxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 13 Aug 2025 15:03:54 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2025-08-12 15:00, Jason Andryuk wrote:
VIRQs come in 3 flavors, per-VPU, per-domain, and global.  The existing
tracking of VIRQs is handled by per-cpu variables virq_to_irq.

The issue is that bind_virq_to_irq() sets the per_cpu virq_to_irq at
registration time - typically CPU 0.  Later, the interrupt can migrate,
and info->cpu is updated.  When calling unbind_from_irq(), the per-cpu
virq_to_irq is cleared for a different cpu.  If bind_virq_to_irq() is
called again with CPU 0, the stale irq is returned.

Change the virq_to_irq tracking to use CPU 0 for per-domain and global
VIRQs.  As there can be at most one of each, there is no need for
per-vcpu tracking.  Also, per-domain and global VIRQs need to be
registered on CPU 0 and can later move, so this matches the expectation.

Fixes: e46cdb66c8fc ("xen: event channels")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
---
Fixes is the introduction of the virq_to_irq per-cpu array.

This was found with the out-of-tree argo driver during suspend/resume.
On suspend, the per-domain VIRQ_ARGO is unbound.  On resume, the driver
attempts to bind VIRQ_ARGO.  The stale irq is returned, but the
WARN_ON(info == NULL || info->type != IRQT_VIRQ) in bind_virq_to_irq()
triggers for NULL info.  The bind fails and execution continues with the
driver trying to clean up by unbinding.  This eventually faults over the
NULL info.
---
  drivers/xen/events/events_base.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 41309d38f78c..a27e4d7f061e 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -159,7 +159,19 @@ static DEFINE_MUTEX(irq_mapping_update_lock);
static LIST_HEAD(xen_irq_list_head); -/* IRQ <-> VIRQ mapping. */
+static bool is_per_vcpu_virq(int virq) {
+       switch (virq) {
+       case VIRQ_TIMER:
+       case VIRQ_DEBUG:
+       case VIRQ_XENOPROF:
+       case VIRQ_XENPMU:
+               return true;
+       default:
+               return false;
+       }
+}
+
+/* IRQ <-> VIRQ mapping.  Global/Domain virqs are tracked in cpu 0.  */
  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = 
-1};
/* IRQ <-> IPI mapping */
@@ -974,6 +986,9 @@ static void __unbind_from_irq(struct irq_info *info, 
unsigned int irq)
switch (info->type) {
                case IRQT_VIRQ:
+                       if (!is_per_vcpu_virq(virq_from_irq(info)))
+                               cpu = 0;
+
                        per_cpu(virq_to_irq, cpu)[virq_from_irq(info)] = -1;
                        break;
                case IRQT_IPI:

Thinking about it a little more, bind_virq_to_irq() should ensure cpu == 0 for per-domain and global VIRQs to ensure the property holds. Also virq_to_irq accesses should go through wrappers to ensure all accesses are handled consistently.

I'll send a v2.

Regards,
Jason



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.