automation: Add build and test for Linux stubdomain

[PATCH 07/12] libxl: Allow stubdomain to control interupts of PCI device

Posted by Marek Marczykowski-Górecki 6 months, 1 week ago

Especially allow it to control MSI/MSI-X enabling bits. This part only
writes a flag to a sysfs, the actual implementation is on the kernel
side.

This requires Linux >= 5.10 in dom0 (or relevant patch backported).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libs/light/libxl_pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 96cb4da0794e..6f357b70b815 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -1513,6 +1513,14 @@ static void pci_add_dm_done(libxl__egc *egc,
             rc = ERROR_FAIL;
             goto out;
         }
+    } else if (libxl_is_stubdom(ctx, domid, NULL)) {
+        /* Allow acces to MSI enable flag in PCI config space for the stubdom */
+        if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/allow_interrupt_control",
+                             pci) < 0 ) {
+            LOGD(ERROR, domainid, "Setting allow_interrupt_control for device");
+            rc = ERROR_FAIL;
+            goto out;
+        }
     }
 
 out_no_irq:
-- 
git-series 0.9.1

Re: [PATCH 07/12] libxl: Allow stubdomain to control interupts of PCI device

Posted by Anthony PERARD 4 months ago

On Thu, May 16, 2024 at 03:58:28PM +0200, Marek Marczykowski-Górecki wrote:
> Especially allow it to control MSI/MSI-X enabling bits. This part only
> writes a flag to a sysfs, the actual implementation is on the kernel
> side.
> 
> This requires Linux >= 5.10 in dom0 (or relevant patch backported).

Does it not work before 5.10? Because the
Documentation/ABI/testing/sysfs-driver-pciback in linux tree say that
allow_interrupt_control is in 5.6.

> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index 96cb4da0794e..6f357b70b815 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -1513,6 +1513,14 @@ static void pci_add_dm_done(libxl__egc *egc,
>              rc = ERROR_FAIL;
>              goto out;
>          }
> +    } else if (libxl_is_stubdom(ctx, domid, NULL)) {
> +        /* Allow acces to MSI enable flag in PCI config space for the stubdom */

s/acces/access/

> +        if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/allow_interrupt_control",
> +                             pci) < 0 ) {
> +            LOGD(ERROR, domainid, "Setting allow_interrupt_control for device");
> +            rc = ERROR_FAIL;
> +            goto out;

Is it possible to make this non-fatal for cases where the kernel is
older than the introduction of the new setting? Or does pci passthrough
doesn't work at all with a stubdom before the change in the kernel?

If making this new setting conditional is an option, you could
potentially improve the error code returned by sysfs_write_bdf() to
distinguish between an open() failure and write() failure, to avoid
checking the existance of the path ahead of the call. But maybe that
pointless because it doesn't appear possible to distinguish between
permission denied and not found.

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH 07/12] libxl: Allow stubdomain to control interupts of PCI device

Posted by Marek Marczykowski-Górecki 4 months ago

On Thu, Jul 25, 2024 at 02:06:04PM +0000, Anthony PERARD wrote:
> On Thu, May 16, 2024 at 03:58:28PM +0200, Marek Marczykowski-Górecki wrote:
> > Especially allow it to control MSI/MSI-X enabling bits. This part only
> > writes a flag to a sysfs, the actual implementation is on the kernel
> > side.
> >
> > This requires Linux >= 5.10 in dom0 (or relevant patch backported).
> 
> Does it not work before 5.10? Because the
> Documentation/ABI/testing/sysfs-driver-pciback in linux tree say that
> allow_interrupt_control is in 5.6.

For MSI-X to work at least with Linux it needs a fixup
2c269f42d0f382743ab230308b836ffe5ae9b2ae, which was backported to
5.10.201, but not further. 

> > diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> > index 96cb4da0794e..6f357b70b815 100644
> > --- a/tools/libs/light/libxl_pci.c
> > +++ b/tools/libs/light/libxl_pci.c
> > @@ -1513,6 +1513,14 @@ static void pci_add_dm_done(libxl__egc *egc,
> >              rc = ERROR_FAIL;
> >              goto out;
> >          }
> > +    } else if (libxl_is_stubdom(ctx, domid, NULL)) {
> > +        /* Allow acces to MSI enable flag in PCI config space for the stubdom */
> 
> s/acces/access/
> 
> > +        if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/allow_interrupt_control",
> > +                             pci) < 0 ) {
> > +            LOGD(ERROR, domainid, "Setting allow_interrupt_control for device");
> > +            rc = ERROR_FAIL;
> > +            goto out;
> 
> Is it possible to make this non-fatal for cases where the kernel is
> older than the introduction of the new setting? Or does pci passthrough
> doesn't work at all with a stubdom before the change in the kernel?

MSI/MSI-X will not work. And if QEMU wouldn't hide MSI/MSI-X (upstream
one doesn't), Linux won't fallback to INTx, so the device won't work at
all.

> If making this new setting conditional is an option, you could
> potentially improve the error code returned by sysfs_write_bdf() to
> distinguish between an open() failure and write() failure, to avoid
> checking the existance of the path ahead of the call. But maybe that
> pointless because it doesn't appear possible to distinguish between
> permission denied and not found.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Re: [PATCH 07/12] libxl: Allow stubdomain to control interupts of PCI device

Posted by Anthony PERARD 3 months, 4 weeks ago

On Thu, Jul 25, 2024 at 04:16:37PM +0200, Marek Marczykowski-Górecki wrote:
> On Thu, Jul 25, 2024 at 02:06:04PM +0000, Anthony PERARD wrote:
> > On Thu, May 16, 2024 at 03:58:28PM +0200, Marek Marczykowski-Górecki wrote:
> > > Especially allow it to control MSI/MSI-X enabling bits. This part only
> > > writes a flag to a sysfs, the actual implementation is on the kernel
> > > side.
> > >
> > > This requires Linux >= 5.10 in dom0 (or relevant patch backported).
> > 
> > Does it not work before 5.10? Because the
> > Documentation/ABI/testing/sysfs-driver-pciback in linux tree say that
> > allow_interrupt_control is in 5.6.
> 
> For MSI-X to work at least with Linux it needs a fixup
> 2c269f42d0f382743ab230308b836ffe5ae9b2ae, which was backported to
> 5.10.201, but not further. 
> 
> > > diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> > > index 96cb4da0794e..6f357b70b815 100644
> > > --- a/tools/libs/light/libxl_pci.c
> > > +++ b/tools/libs/light/libxl_pci.c
> > > @@ -1513,6 +1513,14 @@ static void pci_add_dm_done(libxl__egc *egc,
> > >              rc = ERROR_FAIL;
> > >              goto out;
> > >          }
> > > +    } else if (libxl_is_stubdom(ctx, domid, NULL)) {
> > > +        /* Allow acces to MSI enable flag in PCI config space for the stubdom */
> > 
> > s/acces/access/
> > 
> > > +        if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/allow_interrupt_control",
> > > +                             pci) < 0 ) {
> > > +            LOGD(ERROR, domainid, "Setting allow_interrupt_control for device");
> > > +            rc = ERROR_FAIL;
> > > +            goto out;
> > 
> > Is it possible to make this non-fatal for cases where the kernel is
> > older than the introduction of the new setting? Or does pci passthrough
> > doesn't work at all with a stubdom before the change in the kernel?
> 
> MSI/MSI-X will not work. And if QEMU wouldn't hide MSI/MSI-X (upstream
> one doesn't), Linux won't fallback to INTx, so the device won't work at
> all.

Ok

I guess this patch is fine then:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech