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

Zhenzhong Duan posted 21 patches 2 months, 3 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>
[PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Zhenzhong Duan 2 months, 3 weeks ago
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>
---
 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;
-- 
2.47.1


Re: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Cédric Le Goater 2 months, 2 weeks ago
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;


RE: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Duan, Zhenzhong 2 months, 2 weeks ago
Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v5 04/21] vfio: Introduce helper
>vfio_pci_from_vfio_device()
>
>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.

Yes, I think it's fine to pick this patch upfront.

Thanks
Zhenzhong
Re: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Eric Auger 2 months, 2 weeks ago

On 8/22/25 8:40 AM, 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>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  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;


Re: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Yi Liu 2 months, 2 weeks ago
On 2025/8/22 14: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>
> ---
>   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(-)

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

Re: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Nicolin Chen via 2 months, 3 weeks ago
On Fri, Aug 22, 2025 at 02:40:42AM -0400, 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>

I think we should drop the link? The link points to the v3 that
is not the officially accepted one now, as this PATCH-04 would
be? IOW, the commit should probably have a link to this patch
instead.

Also, in general, your "Signed-off-by" should be the last line,
when you submit a patch.

With that,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

> 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

Nit: this could fit into one line.
RE: [PATCH v5 04/21] vfio: Introduce helper vfio_pci_from_vfio_device()
Posted by Duan, Zhenzhong 2 months, 3 weeks ago

>-----Original Message-----
>From: Nicolin Chen <nicolinc@nvidia.com>
>Subject: Re: [PATCH v5 04/21] vfio: Introduce helper
>vfio_pci_from_vfio_device()
>
>On Fri, Aug 22, 2025 at 02:40:42AM -0400, 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>
>
>I think we should drop the link? The link points to the v3 that
>is not the officially accepted one now, as this PATCH-04 would
>be? IOW, the commit should probably have a link to this patch
>instead.

OK, will remove them.

>
>Also, in general, your "Signed-off-by" should be the last line,
>when you submit a patch.

Will do.

>
>With that,
>
>Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
>
>> 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
>
>Nit: this could fit into one line.

Sure, will do.

Thanks
Zhenzhong