[PATCH 0/2] tools: Fix PVH dom0 passthrough with legacy irq

[PATCH 0/2] tools: Fix PVH dom0 passthrough with legacy irq

[Jason Andryuk]

A PCI device may not have a legacy IRQ assigned.  This series allows
passthrough of such a device to a guest.

It relies on a Linux change to xen-pciback to also handle missing legacy
IRQs:
https://lore.kernel.org/xen-devel/20250226200134.29759-1-jason.andryuk@amd.com/T/#u

Jason Andryuk (2):
  tools/ctrl: Silence missing GSI in xc_pcidev_get_gsi()
  tools/libxl: Skip missing PCI GSIs

 tools/libs/ctrl/xc_linux.c   |  3 ++-
 tools/libs/light/libxl_x86.c | 10 ++++++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/2] tools/ctrl: Silence missing GSI in xc_pcidev_get_gsi()

[Jason Andryuk]

It is valid for a PCI device to not have a legacy IRQ.  In that case, do
not print an error to keep the lgs clean.

This relies on pciback being updated to return -ENOENT for a missing
GSI.

Fixes: b93e5981d258 ("tools: Add new function to get gsi from dev")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 tools/libs/ctrl/xc_linux.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libs/ctrl/xc_linux.c b/tools/libs/ctrl/xc_linux.c
index 92591e49a1..c18f09392f 100644
--- a/tools/libs/ctrl/xc_linux.c
+++ b/tools/libs/ctrl/xc_linux.c
@@ -78,7 +78,8 @@ int xc_pcidev_get_gsi(xc_interface *xch, uint32_t sbdf)
                 IOCTL_PRIVCMD_PCIDEV_GET_GSI, &dev_gsi);
 
     if (ret < 0) {
-        PERROR("Failed to get gsi from dev");
+        if (errno != ENOENT)
+            PERROR("Failed to get gsi from dev");
     } else {
         ret = dev_gsi.gsi;
     }
-- 
2.34.1

Re: [PATCH 1/2] tools/ctrl: Silence missing GSI in xc_pcidev_get_gsi()

[Roger Pau Monné]

On Wed, Feb 26, 2025 at 03:10:21PM -0500, Jason Andryuk wrote:
> It is valid for a PCI device to not have a legacy IRQ.  In that case, do
> not print an error to keep the lgs clean.
                                 ^ logs?
> 
> This relies on pciback being updated to return -ENOENT for a missing
> GSI.
> 
> Fixes: b93e5981d258 ("tools: Add new function to get gsi from dev")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
>  tools/libs/ctrl/xc_linux.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libs/ctrl/xc_linux.c b/tools/libs/ctrl/xc_linux.c
> index 92591e49a1..c18f09392f 100644
> --- a/tools/libs/ctrl/xc_linux.c
> +++ b/tools/libs/ctrl/xc_linux.c
> @@ -78,7 +78,8 @@ int xc_pcidev_get_gsi(xc_interface *xch, uint32_t sbdf)
>                  IOCTL_PRIVCMD_PCIDEV_GET_GSI, &dev_gsi);
>  
>      if (ret < 0) {
> -        PERROR("Failed to get gsi from dev");
> +        if (errno != ENOENT)
> +            PERROR("Failed to get gsi from dev");

Nit: isn't the style of xc_pcidev_get_gsi() wrong?  From what I see in
this same file and all other files in libs/ctrl it should use the
hypervisor coding style.

Thanks, Roger.

Re: [PATCH 1/2] tools/ctrl: Silence missing GSI in xc_pcidev_get_gsi()

[Roger Pau Monné]

On Wed, Feb 26, 2025 at 03:10:21PM -0500, Jason Andryuk wrote:
> It is valid for a PCI device to not have a legacy IRQ.  In that case, do
> not print an error to keep the lgs clean.
> 
> This relies on pciback being updated to return -ENOENT for a missing
> GSI.
> 
> Fixes: b93e5981d258 ("tools: Add new function to get gsi from dev")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
>  tools/libs/ctrl/xc_linux.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libs/ctrl/xc_linux.c b/tools/libs/ctrl/xc_linux.c
> index 92591e49a1..c18f09392f 100644
> --- a/tools/libs/ctrl/xc_linux.c
> +++ b/tools/libs/ctrl/xc_linux.c
> @@ -78,7 +78,8 @@ int xc_pcidev_get_gsi(xc_interface *xch, uint32_t sbdf)
>                  IOCTL_PRIVCMD_PCIDEV_GET_GSI, &dev_gsi);
>  
>      if (ret < 0) {
> -        PERROR("Failed to get gsi from dev");
> +        if (errno != ENOENT)
> +            PERROR("Failed to get gsi from dev");

While here, could you maybe print the S:B:D:F as part of the error
message? (seeing as it's a function parameter).

Thanks, Roger.

[PATCH 2/2] tools/libxl: Skip missing PCI GSIs

[Jason Andryuk]

A PCI device may not have a legacy IRQ.  In that case, we don't need to
do anything, so don't fail in libxl__arch_hvm_map_gsi() and
libxl__arch_hvm_unmap_gsi().

Requires an updated pciback to return -ENOENT.

Fixes: f97f885c7198 ("tools: Add new function to do PIRQ (un)map on PVH dom0")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 tools/libs/light/libxl_x86.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index a3164a3077..63208362af 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -901,7 +901,10 @@ int libxl__arch_hvm_map_gsi(libxl__gc *gc, uint32_t sbdf, uint32_t domid)
     int pirq = -1, gsi, r;
 
     gsi = xc_pcidev_get_gsi(CTX->xch, sbdf);
-    if (gsi < 0) {
+    if (gsi == -1 && errno == ENOENT) {
+        LOGD(DEBUG, domid, "xc_pcidev_get_gsi no gsi");
+        return 0;
+    } else if (gsi < 0) {
         return ERROR_FAIL;
     }
 
@@ -925,7 +928,10 @@ int libxl__arch_hvm_unmap_gsi(libxl__gc *gc, uint32_t sbdf, uint32_t domid)
     int pirq = -1, gsi, r;
 
     gsi = xc_pcidev_get_gsi(CTX->xch, sbdf);
-    if (gsi < 0) {
+    if (gsi == -1 && errno == ENOENT) {
+        LOGD(DEBUG, domid, "xc_pcidev_get_gsi no gsi");
+        return 0;
+    } else if (gsi < 0) {
         return ERROR_FAIL;
     }
 
-- 
2.34.1

Re: [PATCH 2/2] tools/libxl: Skip missing PCI GSIs

[Jan Beulich]

On 26.02.2025 21:10, Jason Andryuk wrote:
> --- a/tools/libs/light/libxl_x86.c
> +++ b/tools/libs/light/libxl_x86.c
> @@ -901,7 +901,10 @@ int libxl__arch_hvm_map_gsi(libxl__gc *gc, uint32_t sbdf, uint32_t domid)
>      int pirq = -1, gsi, r;
>  
>      gsi = xc_pcidev_get_gsi(CTX->xch, sbdf);
> -    if (gsi < 0) {
> +    if (gsi == -1 && errno == ENOENT) {
> +        LOGD(DEBUG, domid, "xc_pcidev_get_gsi no gsi");
> +        return 0;
> +    } else if (gsi < 0) {
>          return ERROR_FAIL;
>      }
>  
> @@ -925,7 +928,10 @@ int libxl__arch_hvm_unmap_gsi(libxl__gc *gc, uint32_t sbdf, uint32_t domid)
>      int pirq = -1, gsi, r;
>  
>      gsi = xc_pcidev_get_gsi(CTX->xch, sbdf);
> -    if (gsi < 0) {
> +    if (gsi == -1 && errno == ENOENT) {
> +        LOGD(DEBUG, domid, "xc_pcidev_get_gsi no gsi");
> +        return 0;
> +    } else if (gsi < 0) {
>          return ERROR_FAIL;
>      }
>  

Why the special-casing of the value -1?

Jan

Re: [PATCH 2/2] tools/libxl: Skip missing PCI GSIs

[Jason Andryuk]

On 2025-02-27 03:25, Jan Beulich wrote:
> On 26.02.2025 21:10, Jason Andryuk wrote:
>> --- a/tools/libs/light/libxl_x86.c
>> +++ b/tools/libs/light/libxl_x86.c

>> @@ -925,7 +928,10 @@ int libxl__arch_hvm_unmap_gsi(libxl__gc *gc, uint32_t sbdf, uint32_t domid)
>>       int pirq = -1, gsi, r;
>>   
>>       gsi = xc_pcidev_get_gsi(CTX->xch, sbdf);
>> -    if (gsi < 0) {
>> +    if (gsi == -1 && errno == ENOENT) {
>> +        LOGD(DEBUG, domid, "xc_pcidev_get_gsi no gsi");
>> +        return 0;
>> +    } else if (gsi < 0) {
>>           return ERROR_FAIL;
>>       }
>>   
> 
> Why the special-casing of the value -1?

No good reason.  I'll restore it to < 0.  I originally thought 
xc_pcidev_get_gsi() was returning -errno in gsi.  That was not the case.

Regards,
Jason