Re: [PATCH v3] vfio: Introduce helper vfio_pci_from_vfio_device()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 1/8/25 04:35, 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>
> ---
> v3: add one line comment to the helper
> v2: move helper to hw/vfio/pci.[hc]
>      rename with vfio_pci_ prefix
> 
>   hw/vfio/pci.h       | 1 +
>   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, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 81465a8214..53842cb149 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -219,6 +219,7 @@ 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);
>   

[*]

> +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);

Quoting https://lore.kernel.org/qemu-devel/87bjpl25e6.fsf@draig.linaro.org/:

   Generally APIs to the rest of QEMU should be documented in the
   headers. Comments on individual functions or internal details are
   fine to live in the C files.

> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 4fa692c1a3..85761544ba 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2824,6 +2824,15 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
>       return ret;
>   }
>   
> +/* Transform from VFIODevice to VFIOPCIDevice. Return NULL if fails. */

So this comment preferably goes in [*]. Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +VFIOPCIDevice *vfio_pci_from_vfio_device(VFIODevice *vbasedev)
> +{
> +    if (vbasedev && vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
> +        return container_of(vbasedev, VFIOPCIDevice, vbasedev);
> +    }
> +    return NULL;
> +}