| Message ID | 20240105070920.350113-6-Jiqian.Chen@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support device passthrough when dom0 is PVH on Xen | expand |
On 05.01.2024 08:09, Jiqian Chen wrote: > --- a/tools/libs/light/libxl_pci.c > +++ b/tools/libs/light/libxl_pci.c > @@ -1479,8 +1479,14 @@ static void pci_add_dm_done(libxl__egc *egc, > fclose(f); > if (!pci_supp_legacy_irq()) > goto out_no_irq; > - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, > pci->bus, pci->dev, pci->func); > + > + if (access(sysfs_path, F_OK) != 0) { I suppose you want to also check errno, and fall back only when you got back (I guess) ENOENT. Jan > + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > + pci->bus, pci->dev, pci->func); > + } > + > f = fopen(sysfs_path, "r"); > if (f == NULL) { > LOGED(ERROR, domainid, "Couldn't open %s", sysfs_path); > @@ -2231,9 +2237,14 @@ skip_bar: > if (!pci_supp_legacy_irq()) > goto skip_legacy_irq; > > - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, > pci->bus, pci->dev, pci->func); > > + if (access(sysfs_path, F_OK) != 0) { > + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > + pci->bus, pci->dev, pci->func); > + } > + > f = fopen(sysfs_path, "r"); > if (f == NULL) { > LOGED(ERROR, domid, "Couldn't open %s", sysfs_path);
On 2024/1/5 15:36, Jan Beulich wrote: > On 05.01.2024 08:09, Jiqian Chen wrote: >> --- a/tools/libs/light/libxl_pci.c >> +++ b/tools/libs/light/libxl_pci.c >> @@ -1479,8 +1479,14 @@ static void pci_add_dm_done(libxl__egc *egc, >> fclose(f); >> if (!pci_supp_legacy_irq()) >> goto out_no_irq; >> - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, >> pci->bus, pci->dev, pci->func); >> + >> + if (access(sysfs_path, F_OK) != 0) { > > I suppose you want to also check errno, and fall back only when you got > back (I guess) ENOENT. Ok, will check errno in next version, if it is ENOENT, then use irq, if it is not ENOENT, then log error and go to out. > > Jan > >> + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> + pci->bus, pci->dev, pci->func); >> + } >> + >> f = fopen(sysfs_path, "r"); >> if (f == NULL) { >> LOGED(ERROR, domainid, "Couldn't open %s", sysfs_path); >> @@ -2231,9 +2237,14 @@ skip_bar: >> if (!pci_supp_legacy_irq()) >> goto skip_legacy_irq; >> >> - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, >> pci->bus, pci->dev, pci->func); >> >> + if (access(sysfs_path, F_OK) != 0) { >> + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> + pci->bus, pci->dev, pci->func); >> + } >> + >> f = fopen(sysfs_path, "r"); >> if (f == NULL) { >> LOGED(ERROR, domid, "Couldn't open %s", sysfs_path); >
On Fri, 5 Jan 2024, Jiqian Chen wrote: > In PVH dom0, it uses the linux local interrupt mechanism, > when it allocs irq for a gsi, it is dynamic, and follow > the principle of applying first, distributing first. And > the irq number is alloced from small to large, but the > applying gsi number is not, may gsi 38 comes before gsi > 28, that causes the irq number is not equal with the gsi > number. And when passthrough a device, xl wants to use > gsi to map pirq, see pci_add_dm_done->xc_physdev_map_pirq, > but the gsi number is got from file > /sys/bus/pci/devices/<sbdf>/irq in current code, so it > will fail when mapping. > > So, use real gsi number read from gsi sysfs. > > Co-developed-by: Huang Rui <ray.huang@amd.com> > Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
On 2024/1/6 09:11, Stefano Stabellini wrote: > On Fri, 5 Jan 2024, Jiqian Chen wrote: >> In PVH dom0, it uses the linux local interrupt mechanism, >> when it allocs irq for a gsi, it is dynamic, and follow >> the principle of applying first, distributing first. And >> the irq number is alloced from small to large, but the >> applying gsi number is not, may gsi 38 comes before gsi >> 28, that causes the irq number is not equal with the gsi >> number. And when passthrough a device, xl wants to use >> gsi to map pirq, see pci_add_dm_done->xc_physdev_map_pirq, >> but the gsi number is got from file >> /sys/bus/pci/devices/<sbdf>/irq in current code, so it >> will fail when mapping. >> >> So, use real gsi number read from gsi sysfs. >> >> Co-developed-by: Huang Rui <ray.huang@amd.com> >> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com> > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Thank you very much! I will add this in next version. And this implementation depends on the third patch on kernel side, if the maintainer doesn't like to add a gsi sysfs, this implementation may be changed in the future, if so, I will let you know. >
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c index e1341d1e9850..ab51dc099725 100644 --- a/tools/libs/light/libxl_pci.c +++ b/tools/libs/light/libxl_pci.c @@ -1479,8 +1479,14 @@ static void pci_add_dm_done(libxl__egc *egc, fclose(f); if (!pci_supp_legacy_irq()) goto out_no_irq; - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, pci->bus, pci->dev, pci->func); + + if (access(sysfs_path, F_OK) != 0) { + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, + pci->bus, pci->dev, pci->func); + } + f = fopen(sysfs_path, "r"); if (f == NULL) { LOGED(ERROR, domainid, "Couldn't open %s", sysfs_path); @@ -2231,9 +2237,14 @@ skip_bar: if (!pci_supp_legacy_irq()) goto skip_legacy_irq; - sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/gsi", pci->domain, pci->bus, pci->dev, pci->func); + if (access(sysfs_path, F_OK) != 0) { + sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, + pci->bus, pci->dev, pci->func); + } + f = fopen(sysfs_path, "r"); if (f == NULL) { LOGED(ERROR, domid, "Couldn't open %s", sysfs_path);
In PVH dom0, it uses the linux local interrupt mechanism, when it allocs irq for a gsi, it is dynamic, and follow the principle of applying first, distributing first. And the irq number is alloced from small to large, but the applying gsi number is not, may gsi 38 comes before gsi 28, that causes the irq number is not equal with the gsi number. And when passthrough a device, xl wants to use gsi to map pirq, see pci_add_dm_done->xc_physdev_map_pirq, but the gsi number is got from file /sys/bus/pci/devices/<sbdf>/irq in current code, so it will fail when mapping. So, use real gsi number read from gsi sysfs. Co-developed-by: Huang Rui <ray.huang@amd.com> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com> --- tools/libs/light/libxl_pci.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)