Re: [RFC QEMU PATCH v7 1/1] xen/pci: get gsi for passthrough devices

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2024/10/15 03:34, Stewart Hildebrand wrote:
> +Edgar
> 
> On 5/16/24 06:13, 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, qemu wants to use
>> gsi to map pirq, xen_pt_realize->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.
>>
>> Get gsi by using new function supported by Xen tools.
>>
>> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> 
> I think you can safely remove the RFC tag since the Xen bits have been
> upstreamed.
Thank you! I will send a new version later this week and modify the patch 
according to your comments.

> 
>> ---
>>  hw/xen/xen-host-pci-device.c | 19 +++++++++++++++----
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
>> index 8c6e9a1716a2..2fe6a60434ba 100644
>> --- a/hw/xen/xen-host-pci-device.c
>> +++ b/hw/xen/xen-host-pci-device.c
>> @@ -10,6 +10,7 @@
>>  #include "qapi/error.h"
>>  #include "qemu/cutils.h"
>>  #include "xen-host-pci-device.h"
>> +#include "hw/xen/xen_native.h"
> 
> The inclusion order unfortunately seems to be delicate.
> "hw/xen/xen_native.h" should be before all the other xen
> includes, but after "qemu/osdep.h".
> 
>>  
>>  #define XEN_HOST_PCI_MAX_EXT_CAP \
>>      ((PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) / (PCI_CAP_SIZEOF + 
>> 4))
>> @@ -329,12 +330,17 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice 
>> *d, uint32_t cap)
>>      return -1;
>>  }
>>  
>> +#define PCI_SBDF(seg, bus, dev, func) \
>> +            ((((uint32_t)(seg)) << 16) | \
>> +            (PCI_BUILD_BDF(bus, PCI_DEVFN(dev, func))))
>> +
>>  void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>>                               uint8_t bus, uint8_t dev, uint8_t func,
>>                               Error **errp)
>>  {
>>      ERRP_GUARD();
>>      unsigned int v;
>> +    uint32_t sdbf;
> 
> Typo: s/sdbf/sbdf/
> 
>>  
>>      d->config_fd = -1;
>>      d->domain = domain;
>> @@ -364,11 +370,16 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, 
>> uint16_t domain,
>>      }
>>      d->device_id = v;
>>  
>> -    xen_host_pci_get_dec_value(d, "irq", &v, errp);
>> -    if (*errp) {
>> -        goto error;
>> +    sdbf = PCI_SBDF(domain, bus, dev, func);
>> +    d->irq = xc_physdev_gsi_from_dev(xen_xc, sdbf);
> 
> This was renamed to xc_pcidev_get_gsi.
> 
> This also needs some sort of Xen interface version guard for backward
> compatibility since it's a new call introduced in Xen 4.20.
> 
>> +    /* fail to get gsi, fallback to irq */
>> +    if (d->irq == -1) {
>> +        xen_host_pci_get_dec_value(d, "irq", &v, errp);
>> +        if (*errp) {
>> +            goto error;
>> +        }
>> +        d->irq = v;
>>      }
>> -    d->irq = v;
>>  
>>      xen_host_pci_get_hex_value(d, "class", &v, errp);
>>      if (*errp) {
> 

-- 
Best regards,
Jiqian Chen.