On 2023/12/11 23:48, Roger Pau Monné wrote:
> On Mon, Dec 11, 2023 at 12:40:09AM +0800, 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.hu...@amd.com>
>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>> ---
>> tools/libs/light/libxl_pci.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
>> index 96cb4da079..9e75f0c263 100644
>> --- a/tools/libs/light/libxl_pci.c
>> +++ b/tools/libs/light/libxl_pci.c
>> @@ -1416,7 +1416,7 @@ 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 gsi, i;
>> int r;
>> uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
>> uint32_t domainid = domid;
>> @@ -1439,7 +1439,7 @@ static void pci_add_dm_done(libxl__egc *egc,
>> pci->bus, pci->dev, pci->func);
>> f = fopen(sysfs_path, "r");
>> start = end = flags = size = 0;
>> - irq = 0;
>> + gsi = 0;
>
> unsigned int (so it matches the fscanf format), and initialized at
> definition.
As what you said below, I need to use irq if there is no gsi sysfs. So, I think
it is not necessary to change the name of this local variable.
>
>>
>> if (f == NULL) {
>> LOGED(ERROR, domainid, "Couldn't open %s", sysfs_path);
>> @@ -1478,26 +1478,26 @@ 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);
>
> You need to keep the fallback mechanism of reading the irq node, or
> else xl would stop working on any kernel that doesn't expose this
> sysfs node, you would break passthrough on all current Linux versions.
Yes, you are right. If there is no gsi sysfs, will still use irq, in next
version. Thanks.
>
> Thanks, Roger.
--
Best regards,
Jiqian Chen.
