Re: [RFC XEN PATCH v12 6/7] tools: Add new function to get gsi from dev

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jul 08, 2024 at 07:41:23PM +0800, Jiqian Chen wrote:
> When passthrough a device to domU, QEMU and xl tools use its gsi
> number to do pirq mapping, see QEMU code
> xen_pt_realize->xc_physdev_map_pirq, and xl code
> pci_add_dm_done->xc_physdev_map_pirq, but the gsi number is got
> from file /sys/bus/pci/devices/<sbdf>/irq, that is wrong, because
> irq is not equal with gsi, they are in different spaces, so pirq
> mapping fails.
> 
> And in current codes, there is no method to get gsi for userspace.
> For above purpose, add new function to get gsi, and the
> corresponding ioctl is implemented on linux kernel side.
> 
> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> Signed-off-by: Chen Jiqian <Jiqian.Chen@xxxxxxx>
> ---
> RFC: it needs to wait for the corresponding third patch on linux kernel side 
> to be merged.
> https://lore.kernel.org/xen-devel/20240607075109.126277-4-Jiqian.Chen@xxxxxxx/
> This patch must be merged after the patch on linux kernel side
> 
> CC: Anthony PERARD <anthony@xxxxxxxxxxxxxx>
> Remaining comment @Anthony PERARD:
> Do I need to make " opening of /dev/xen/privcmd " as a single function, then 
> use it in this
> patch and other libraries?
> ---
>  tools/include/xen-sys/Linux/privcmd.h |  7 ++++++
>  tools/include/xenctrl.h               |  2 ++
>  tools/libs/ctrl/xc_physdev.c          | 35 +++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/tools/include/xen-sys/Linux/privcmd.h 
> b/tools/include/xen-sys/Linux/privcmd.h
> index bc60e8fd55eb..4cf719102116 100644
> --- a/tools/include/xen-sys/Linux/privcmd.h
> +++ b/tools/include/xen-sys/Linux/privcmd.h
> @@ -95,6 +95,11 @@ typedef struct privcmd_mmap_resource {
>       __u64 addr;
>  } privcmd_mmap_resource_t;
>  
> +typedef struct privcmd_gsi_from_pcidev {
> +     __u32 sbdf;
> +     __u32 gsi;
> +} privcmd_gsi_from_pcidev_t;
> +
>  /*
>   * @cmd: IOCTL_PRIVCMD_HYPERCALL
>   * @arg: &privcmd_hypercall_t
> @@ -114,6 +119,8 @@ typedef struct privcmd_mmap_resource {
>       _IOC(_IOC_NONE, 'P', 6, sizeof(domid_t))
>  #define IOCTL_PRIVCMD_MMAP_RESOURCE                          \
>       _IOC(_IOC_NONE, 'P', 7, sizeof(privcmd_mmap_resource_t))
> +#define IOCTL_PRIVCMD_GSI_FROM_PCIDEV                                \
> +     _IOC(_IOC_NONE, 'P', 10, sizeof(privcmd_gsi_from_pcidev_t))
>  #define IOCTL_PRIVCMD_UNIMPLEMENTED                          \
>       _IOC(_IOC_NONE, 'P', 0xFF, 0)
>  
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 9ceca0cffc2f..3720e22b399a 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -1641,6 +1641,8 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
>                            uint32_t domid,
>                            int pirq);
>  
> +int xc_physdev_gsi_from_pcidev(xc_interface *xch, uint32_t sbdf);
> +
>  /*
>   *  LOGGING AND ERROR REPORTING
>   */
> diff --git a/tools/libs/ctrl/xc_physdev.c b/tools/libs/ctrl/xc_physdev.c
> index e9fcd755fa62..54edb0f3c0dc 100644
> --- a/tools/libs/ctrl/xc_physdev.c
> +++ b/tools/libs/ctrl/xc_physdev.c
> @@ -111,3 +111,38 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
>      return rc;
>  }
>  
> +int xc_physdev_gsi_from_pcidev(xc_interface *xch, uint32_t sbdf)

FWIW, I'm not sure it's fine to use the xc_physdev prefix here, as
this is not a PHYSDEVOP hypercall.

As Anthony suggested, it would be better placed in xc_linux.c, and
possibly named xc_pcidev_get_gsi() or similar, to avoid polluting the
xc_physdev namespace.

Thanks, Roger.