diff mbox series

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

Message ID 20240516101338.86763-2-Jiqian.Chen@amd.com (mailing list archive)
State Superseded
Headers show
Series Support device passthrough when dom0 is PVH on Xen | expand

Commit Message

Chen, Jiqian May 16, 2024, 10:13 a.m. UTC 
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@amd.com>
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 hw/xen/xen-host-pci-device.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Comments

Stewart Hildebrand Oct. 14, 2024, 7:34 p.m. UTC | #1 
+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@amd.com>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>

I think you can safely remove the RFC tag since the Xen bits have been
upstreamed.

> ---
>  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) {
Chen, Jiqian Oct. 15, 2024, 2:12 a.m. UTC | #2 
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@amd.com>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> 
> 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) {
>
diff mbox series

Patch

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"
 
 #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;
 
     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);
+    /* 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) {