| Message ID | aeba05a5ec7a0657217933f165ee0ac7e86e1b1b.1715867907.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | automation: Add build and test for Linux stubdomain | expand |
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,
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.
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,
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:
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(+)