Re: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()

["Cédric Le Goater" <clg@redhat.com>]

Zhenzhong,

On 8/22/25 08:40, Zhenzhong Duan wrote:
> Introduce helper vfio_pci_from_vfio_device() to transform from VFIODevice
> to VFIOPCIDevice, also to hide low level VFIO_DEVICE_TYPE_PCI type check.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> Link: https://lore.kernel.org/qemu-devel/20250801023533.1458644-1-zhenzhong.duan@intel.com
> [ clg: Added documentation ]
> Signed-off-by: Cédric Le Goater <clg@redhat.com>

Would you like me to merge these VFIO changes upfront ?

They do not seem to conflict with the series I have queued for 10.2 so far.

C.



> ---
>   hw/vfio/pci.h       | 12 ++++++++++++
>   hw/vfio/container.c |  4 ++--
>   hw/vfio/device.c    |  2 +-
>   hw/vfio/iommufd.c   |  4 ++--
>   hw/vfio/listener.c  |  4 ++--
>   hw/vfio/pci.c       |  9 +++++++++
>   6 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 810a842f4a..beb8fb9ee7 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -221,6 +221,18 @@ void vfio_pci_write_config(PCIDevice *pdev,
>   uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
>   void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
>   
> +/**
> + * vfio_pci_from_vfio_device: Transform from VFIODevice to
> + * VFIOPCIDevice
> + *
> + * This function checks if the given @vbasedev is a VFIO PCI device.
> + * If it is, it returns the containing VFIOPCIDevice.
> + *
> + * @vbasedev: The VFIODevice to transform
> + *
> + * Return: The VFIOPCIDevice on success, NULL on failure.
> + */
> +VFIOPCIDevice *vfio_pci_from_vfio_device(VFIODevice *vbasedev);
>   void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
>   bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
>   bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 3e13feaa74..134ddccc52 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1087,7 +1087,7 @@ static int vfio_legacy_pci_hot_reset(VFIODevice *vbasedev, bool single)
>           /* Prep dependent devices for reset and clear our marker. */
>           QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
>               if (!vbasedev_iter->dev->realized ||
> -                vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
> +                !vfio_pci_from_vfio_device(vbasedev_iter)) {
>                   continue;
>               }
>               tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
> @@ -1172,7 +1172,7 @@ out:
>   
>           QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
>               if (!vbasedev_iter->dev->realized ||
> -                vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
> +                !vfio_pci_from_vfio_device(vbasedev_iter)) {
>                   continue;
>               }
>               tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 52a1996dc4..08f12ac31f 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -129,7 +129,7 @@ static inline const char *action_to_str(int action)
>   
>   static const char *index_to_str(VFIODevice *vbasedev, int index)
>   {
> -    if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
> +    if (!vfio_pci_from_vfio_device(vbasedev)) {
>           return NULL;
>       }
>   
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 48c590b6a9..8c27222f75 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -737,8 +737,8 @@ iommufd_cdev_dep_get_realized_vpdev(struct vfio_pci_dependent_device *dep_dev,
>       }
>   
>       vbasedev_tmp = iommufd_cdev_pci_find_by_devid(dep_dev->devid);
> -    if (!vbasedev_tmp || !vbasedev_tmp->dev->realized ||
> -        vbasedev_tmp->type != VFIO_DEVICE_TYPE_PCI) {
> +    if (!vfio_pci_from_vfio_device(vbasedev_tmp) ||
> +        !vbasedev_tmp->dev->realized) {
>           return NULL;
>       }
>   
> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
> index f498e23a93..903dfd8bf2 100644
> --- a/hw/vfio/listener.c
> +++ b/hw/vfio/listener.c
> @@ -450,7 +450,7 @@ static void vfio_device_error_append(VFIODevice *vbasedev, Error **errp)
>        * MMIO region mapping failures are not fatal but in this case PCI
>        * peer-to-peer transactions are broken.
>        */
> -    if (vbasedev && vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
> +    if (vfio_pci_from_vfio_device(vbasedev)) {
>           error_append_hint(errp, "%s: PCI peer-to-peer transactions "
>                             "on BARs are not supported.\n", vbasedev->name);
>       }
> @@ -751,7 +751,7 @@ static bool vfio_section_is_vfio_pci(MemoryRegionSection *section,
>       owner = memory_region_owner(section->mr);
>   
>       QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
> -        if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
> +        if (!vfio_pci_from_vfio_device(vbasedev)) {
>               continue;
>           }
>           pcidev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 07257d0fa0..3fe5b03eb1 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2833,6 +2833,15 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
>       return ret;
>   }
>   
> +/* Transform from VFIODevice to VFIOPCIDevice. Return NULL if fails. */
> +VFIOPCIDevice *vfio_pci_from_vfio_device(VFIODevice *vbasedev)
> +{
> +    if (vbasedev && vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
> +        return container_of(vbasedev, VFIOPCIDevice, vbasedev);
> +    }
> +    return NULL;
> +}
> +
>   void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
>   {
>       PCIDevice *pdev = &vdev->pdev;