>-----Original Message----- >From: Philippe Mathieu-Daudé <phi...@linaro.org> >Subject: Re: [PATCH v3] vfio: Introduce helper vfio_pci_from_vfio_device() > >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 <c...@redhat.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@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....@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.
I see, thanks > >> 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, FYI, Cedric helped to make the suggested change, so I'll not send v4. https://github.com/legoater/qemu/commit/d0ec15af9a5c0a8a9c1a7a71903eb4e00cae71b1 BRs, Zhenzhong > >Reviewed-by: Philippe Mathieu-Daudé <phi...@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; >> +}
