[PATCH v2 0/2] libxl: Skip invalid IRQs

[PATCH v2 0/2] libxl: Skip invalid IRQs

[Jason Andryuk]

Allow assigning devices without a legacy IRQ.
v2 adds patch 2 to switch to unsigned int.

Jason Andryuk (2):
  tools/libxl: Skip invalid IRQs
  tools/libxl: Switch irq to unsigned int

 tools/libs/light/libxl_pci.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

-- 
2.49.0

[PATCH v2 1/2] tools/libxl: Skip invalid IRQs

[Jason Andryuk]

A PCI device's irq field is an 8-bit number.  A value of 0xff indicates
that the device IRQ is not connected.  Additionally, the Linux ACPI code
can convert these 0xff values to IRQ_NOTCONNECTED(0x80000000) because
"0x80000000 is guaranteed to be outside the available range of
interrupts and easy to distinguish from other possible incorrect
values."  When the hypercall to assign that IRQ fails, device
passthrough as a whole fails.

Add checking for a valid IRQ and skip the IRQ handling for PCI devices
outside that range.  This allows for passthrough of devices without
legacy IRQs.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v2:
Remove () from around fscanf()
---
 tools/libs/light/libxl_pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 1647fd6f47..6ddcdef6ad 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -26,6 +26,9 @@
 #define PCI_BDF_XSPATH         "%04x-%02x-%02x-%01x"
 #define PCI_PT_QDEV_ID         "pci-pt-%02x_%02x.%01x"
 
+/* PCI Interrupt Line is an 8-bit value, 0xff means disconnected. */
+#define PCI_IRQ_LINE_LIMIT     0xff
+
 static unsigned int pci_encode_bdf(libxl_device_pci *pci)
 {
     unsigned int value;
@@ -1495,7 +1498,7 @@ static void pci_add_dm_done(libxl__egc *egc,
             LOGED(ERROR, domainid, "Couldn't open %s", sysfs_path);
             goto out_no_irq;
         }
-        if ((fscanf(f, "%u", &irq) == 1) && irq) {
+        if (fscanf(f, "%u", &irq) == 1 && irq > 0 && irq < PCI_IRQ_LINE_LIMIT) {
             r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
             if (r < 0) {
                 LOGED(ERROR, domainid, "xc_physdev_map_pirq irq=%d (error=%d)",
@@ -2257,7 +2260,7 @@ skip_bar:
             goto skip_legacy_irq;
         }
 
-        if ((fscanf(f, "%u", &irq) == 1) && irq) {
+        if (fscanf(f, "%u", &irq) == 1 && irq > 0 && irq < PCI_IRQ_LINE_LIMIT) {
             rc = xc_physdev_unmap_pirq(ctx->xch, domid, irq);
             if (rc < 0) {
                 /*
-- 
2.49.0

[PATCH v2 2/2] tools/libxl: Switch irq to unsigned int

[Jason Andryuk]

The PCI device irq is read with fscanf(%u).  Switch the irq variable to
unsigned int to match.

Linux driver/pci/pci-sysfs.c:irq_show() uses %u to print the value.

However, unsigned int irq doesn't compile because of:
error: pointer targets in passing argument 4 of 'xc_physdev_map_pirq' differ in signedness [-Werror=pointer-sign]

Add int pirq to provide the desired type instead of re-using irq.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v2:
New
---
 tools/libs/light/libxl_pci.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 6ddcdef6ad..a8460fb3ec 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -1420,8 +1420,8 @@ static void pci_add_dm_done(libxl__egc *egc,
     char *sysfs_path;
     FILE *f;
     unsigned long long start, end, flags, size;
-    int irq, i;
-    int r;
+    unsigned int irq;
+    int i, r;
     uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
     uint32_t domainid = domid;
     bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid);
@@ -1499,7 +1499,9 @@ static void pci_add_dm_done(libxl__egc *egc,
             goto out_no_irq;
         }
         if (fscanf(f, "%u", &irq) == 1 && irq > 0 && irq < PCI_IRQ_LINE_LIMIT) {
-            r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
+            int pirq = irq;
+
+            r = xc_physdev_map_pirq(ctx->xch, domid, irq, &pirq);
             if (r < 0) {
                 LOGED(ERROR, domainid, "xc_physdev_map_pirq irq=%d (error=%d)",
                     irq, r);
@@ -1507,10 +1509,10 @@ static void pci_add_dm_done(libxl__egc *egc,
                 rc = ERROR_FAIL;
                 goto out;
             }
-            r = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
+            r = xc_domain_irq_permission(ctx->xch, domid, pirq, 1);
             if (r < 0) {
                 LOGED(ERROR, domainid,
-                    "xc_domain_irq_permission irq=%d (error=%d)", irq, r);
+                    "xc_domain_irq_permission irq=%d (error=%d)", pirq, r);
                 fclose(f);
                 rc = ERROR_FAIL;
                 goto out;
@@ -2182,8 +2184,8 @@ static void pci_remove_detached(libxl__egc *egc,
 {
     STATE_AO_GC(prs->aodev->ao);
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    unsigned int start = 0, end = 0, flags = 0, size = 0;
-    int  irq = 0, i, stubdomid = 0;
+    unsigned int start = 0, end = 0, flags = 0, size = 0, irq = 0;
+    int i, stubdomid = 0;
     const char *sysfs_path;
     FILE *f;
     uint32_t domainid = prs->domid;
-- 
2.49.0

Re: [PATCH v2 1/2] tools/libxl: Skip invalid IRQs

[Anthony PERARD]

On Fri, Apr 18, 2025 at 05:05:49PM -0400, Jason Andryuk wrote:
> A PCI device's irq field is an 8-bit number.  A value of 0xff indicates
> that the device IRQ is not connected.  Additionally, the Linux ACPI code
> can convert these 0xff values to IRQ_NOTCONNECTED(0x80000000) because
> "0x80000000 is guaranteed to be outside the available range of
> interrupts and easy to distinguish from other possible incorrect
> values."  When the hypercall to assign that IRQ fails, device
> passthrough as a whole fails.
> 
> Add checking for a valid IRQ and skip the IRQ handling for PCI devices
> outside that range.  This allows for passthrough of devices without
> legacy IRQs.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD

Re: [PATCH v2 2/2] tools/libxl: Switch irq to unsigned int

[Anthony PERARD]

On Fri, Apr 18, 2025 at 05:05:50PM -0400, Jason Andryuk wrote:
> The PCI device irq is read with fscanf(%u).  Switch the irq variable to
> unsigned int to match.
> 
> Linux driver/pci/pci-sysfs.c:irq_show() uses %u to print the value.
> 
> However, unsigned int irq doesn't compile because of:
> error: pointer targets in passing argument 4 of 'xc_physdev_map_pirq' differ in signedness [-Werror=pointer-sign]
> 
> Add int pirq to provide the desired type instead of re-using irq.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD