qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] vfio: Move realize after attach_dev
@ 2025-04-23  7:28 Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities Zhenzhong Duan
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan

Hi,

This series addresses Cédric's suggestion[1] and Donald's suggestion[2] to
move realize() call after attach_device().

This way avoid the need to introduce realize_late() to further complex the
interface in nesting series.

[1] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg01211.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg00898.html

Test:
net card passthrough and ping test
hotplug/unplug

Based on vfio-next(856f36c005).

Thanks
Zhenzhong

Changelog:
v2:
- drop the idea to save host iommu capabilities in VFIODevice.caps
- introduce a new function to create and realize hiod
- remove hiod_typename property

Zhenzhong Duan (5):
  vfio/iommufd: Make a separate call to get IOMMU capabilities
  vfio/iommufd: Move realize() after attachment
  vfio/container: Move realize() after attachment
  vfio: Cleanup host IOMMU device creation
  vfio: Remove hiod_typename property

 include/hw/vfio/vfio-container-base.h |  3 ---
 include/hw/vfio/vfio-device.h         |  3 ++-
 hw/vfio/container.c                   | 25 ++++++++++++--------
 hw/vfio/device.c                      | 33 ++++++++++++---------------
 hw/vfio/iommufd.c                     | 31 +++++++++++++------------
 5 files changed, 47 insertions(+), 48 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
@ 2025-04-23  7:28 ` Zhenzhong Duan
  2025-04-23  8:31   ` Joao Martins
  2025-04-23  7:28 ` [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment Zhenzhong Duan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan

Currently we depend on .realize() calling iommufd_backend_get_device_info()
to get IOMMU capabilities and check for dirty page tracking support.

By make a extra separate call, this dependency is removed. This happens
only during device attach, it's not a hot path.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 48db105422..2253778b3a 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -287,7 +287,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
 {
     ERRP_GUARD();
     IOMMUFDBackend *iommufd = vbasedev->iommufd;
-    uint32_t flags = 0;
+    uint32_t type, flags = 0;
+    uint64_t hw_caps;
     VFIOIOASHwpt *hwpt;
     uint32_t hwpt_id;
     int ret;
@@ -324,7 +325,12 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
      * vfio_migration_realize() may decide to use VF dirty tracking
      * instead.
      */
-    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
+    if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev->devid,
+                                         &type, NULL, 0, &hw_caps, errp)) {
+        return false;
+    }
+
+    if (hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
         flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
     }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities Zhenzhong Duan
@ 2025-04-23  7:28 ` Zhenzhong Duan
  2025-04-23  9:39   ` Joao Martins
  2025-04-23  7:28 ` [PATCH v2 3/5] vfio/container: " Zhenzhong Duan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan, Donald Dutile

Previously device attaching depends on realize() getting host IOMMU
capabilities to check dirty tracking support.

Now we have a separate call to ioctl(IOMMU_GET_HW_INFO) to get host
IOMMU capabilities and check that for dirty tracking support, there
is no dependency any more, move realize() call after attachment
succeed.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Suggested-by: Donald Dutile <ddutile@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 2253778b3a..f273dc8712 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -500,17 +500,6 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
 
     space = vfio_address_space_get(as);
 
-    /*
-     * The HostIOMMUDevice data from legacy backend is static and doesn't need
-     * any information from the (type1-iommu) backend to be initialized. In
-     * contrast however, the IOMMUFD HostIOMMUDevice data requires the iommufd
-     * FD to be connected and having a devid to be able to successfully call
-     * iommufd_backend_get_device_info().
-     */
-    if (!vfio_device_hiod_realize(vbasedev, errp)) {
-        goto err_alloc_ioas;
-    }
-
     /* try to attach to an existing container in this space */
     QLIST_FOREACH(bcontainer, &space->containers, next) {
         container = container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
@@ -585,6 +574,10 @@ found_container:
         goto err_listener_register;
     }
 
+    if (!vfio_device_hiod_realize(vbasedev, errp)) {
+        goto err_hiod_realize;
+    }
+
     /*
      * TODO: examine RAM_BLOCK_DISCARD stuff, should we do group level
      * for discarding incompatibility check as well?
@@ -606,6 +599,8 @@ found_container:
                                    vbasedev->num_regions, vbasedev->flags);
     return true;
 
+err_hiod_realize:
+    vfio_cpr_unregister_container(bcontainer);
 err_listener_register:
     iommufd_cdev_ram_block_discard_disable(false);
 err_discard_disable:
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/5] vfio/container: Move realize() after attachment
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment Zhenzhong Duan
@ 2025-04-23  7:28 ` Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 4/5] vfio: Cleanup host IOMMU device creation Zhenzhong Duan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan, Donald Dutile

To match the change for IOMMUFD backend, move realize() after attachment
for legacy backend too.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Suggested-by: Donald Dutile <ddutile@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/container.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 23a3373470..1bb8e2de6c 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -883,10 +883,6 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
 
     trace_vfio_device_attach(vbasedev->name, groupid);
 
-    if (!vfio_device_hiod_realize(vbasedev, errp)) {
-        return false;
-    }
-
     group = vfio_group_get(groupid, as, errp);
     if (!group) {
         return false;
@@ -895,13 +891,15 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
         if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) {
             error_setg(errp, "device is already attached");
-            vfio_group_put(group);
-            return false;
+            goto group_put_exit;
         }
     }
     if (!vfio_device_get(group, name, vbasedev, errp)) {
-        vfio_group_put(group);
-        return false;
+        goto group_put_exit;
+    }
+
+    if (!vfio_device_hiod_realize(vbasedev, errp)) {
+        goto device_put_exit;
     }
 
     bcontainer = &group->container->bcontainer;
@@ -910,6 +908,12 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
     QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
 
     return true;
+
+device_put_exit:
+    vfio_device_put(vbasedev);
+group_put_exit:
+    vfio_group_put(group);
+    return false;
 }
 
 static void vfio_legacy_detach_device(VFIODevice *vbasedev)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/5] vfio: Cleanup host IOMMU device creation
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
                   ` (2 preceding siblings ...)
  2025-04-23  7:28 ` [PATCH v2 3/5] vfio/container: " Zhenzhong Duan
@ 2025-04-23  7:28 ` Zhenzhong Duan
  2025-04-23  7:28 ` [PATCH v2 5/5] vfio: Remove hiod_typename property Zhenzhong Duan
  2025-04-23 13:21 ` [PATCH v2 0/5] vfio: Move realize after attach_dev Cédric Le Goater
  5 siblings, 0 replies; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan

realize() is now moved after attachment, do the same for hiod creation.
Introduce a new function vfio_device_hiod_create_and_realize() to do
them all in one go.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-device.h |  3 ++-
 hw/vfio/container.c           |  5 ++++-
 hw/vfio/device.c              | 33 ++++++++++++++-------------------
 hw/vfio/iommufd.c             |  4 +++-
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 66797b4c92..65fa67e65a 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -123,7 +123,8 @@ bool vfio_device_irq_set_signaling(VFIODevice *vbasedev, int index, int subindex
 
 void vfio_device_reset_handler(void *opaque);
 bool vfio_device_is_mdev(VFIODevice *vbasedev);
-bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp);
+bool vfio_device_hiod_create_and_realize(VFIODevice *vbasedev,
+                                         const char *typename, Error **errp);
 bool vfio_device_attach(char *name, VFIODevice *vbasedev,
                         AddressSpace *as, Error **errp);
 void vfio_device_detach(VFIODevice *vbasedev);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 1bb8e2de6c..2de20692d1 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -898,7 +898,9 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
         goto group_put_exit;
     }
 
-    if (!vfio_device_hiod_realize(vbasedev, errp)) {
+    if (!vfio_device_hiod_create_and_realize(vbasedev,
+                                             TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO,
+                                             errp)) {
         goto device_put_exit;
     }
 
@@ -924,6 +926,7 @@ static void vfio_legacy_detach_device(VFIODevice *vbasedev)
     QLIST_REMOVE(vbasedev, container_next);
     vbasedev->bcontainer = NULL;
     trace_vfio_device_detach(vbasedev->name, group->groupid);
+    object_unref(vbasedev->hiod);
     vfio_device_put(vbasedev);
     vfio_group_put(group);
 }
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 4de6948cf4..d625a7c4db 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -347,15 +347,24 @@ bool vfio_device_is_mdev(VFIODevice *vbasedev)
     return subsys && (strcmp(subsys, "/sys/bus/mdev") == 0);
 }
 
-bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error **errp)
+bool vfio_device_hiod_create_and_realize(VFIODevice *vbasedev,
+                                         const char *typename, Error **errp)
 {
-    HostIOMMUDevice *hiod = vbasedev->hiod;
+    HostIOMMUDevice *hiod;
 
-    if (!hiod) {
+    if (vbasedev->mdev) {
         return true;
     }
 
-    return HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp);
+    hiod = HOST_IOMMU_DEVICE(object_new(typename));
+
+    if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp)) {
+        object_unref(hiod);
+        return false;
+    }
+
+    vbasedev->hiod = hiod;
+    return true;
 }
 
 VFIODevice *vfio_get_vfio_device(Object *obj)
@@ -372,7 +381,6 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
 {
     const VFIOIOMMUClass *ops =
         VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
-    HostIOMMUDevice *hiod = NULL;
 
     if (vbasedev->iommufd) {
         ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
@@ -380,19 +388,7 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
 
     assert(ops);
 
-
-    if (!vbasedev->mdev) {
-        hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
-        vbasedev->hiod = hiod;
-    }
-
-    if (!ops->attach_device(name, vbasedev, as, errp)) {
-        object_unref(hiod);
-        vbasedev->hiod = NULL;
-        return false;
-    }
-
-    return true;
+    return ops->attach_device(name, vbasedev, as, errp);
 }
 
 void vfio_device_detach(VFIODevice *vbasedev)
@@ -400,6 +396,5 @@ void vfio_device_detach(VFIODevice *vbasedev)
     if (!vbasedev->bcontainer) {
         return;
     }
-    object_unref(vbasedev->hiod);
     VFIO_IOMMU_GET_CLASS(vbasedev->bcontainer)->detach_device(vbasedev);
 }
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index f273dc8712..8a010a51ea 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -574,7 +574,8 @@ found_container:
         goto err_listener_register;
     }
 
-    if (!vfio_device_hiod_realize(vbasedev, errp)) {
+    if (!vfio_device_hiod_create_and_realize(vbasedev,
+                     TYPE_HOST_IOMMU_DEVICE_IOMMUFD_VFIO, errp)) {
         goto err_hiod_realize;
     }
 
@@ -630,6 +631,7 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
         iommufd_cdev_ram_block_discard_disable(false);
     }
 
+    object_unref(vbasedev->hiod);
     vfio_cpr_unregister_container(bcontainer);
     iommufd_cdev_detach_container(vbasedev, container);
     iommufd_cdev_container_destroy(container);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/5] vfio: Remove hiod_typename property
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
                   ` (3 preceding siblings ...)
  2025-04-23  7:28 ` [PATCH v2 4/5] vfio: Cleanup host IOMMU device creation Zhenzhong Duan
@ 2025-04-23  7:28 ` Zhenzhong Duan
  2025-04-23 13:21 ` [PATCH v2 0/5] vfio: Move realize after attach_dev Cédric Le Goater
  5 siblings, 0 replies; 10+ messages in thread
From: Zhenzhong Duan @ 2025-04-23  7:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, joao.m.martins,
	yi.l.liu, chao.p.peng, Zhenzhong Duan

Because we handle host IOMMU device creation in each container backend,
we know which type name to use, so hiod_typename property is useless
now, just remove it.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-container-base.h | 3 ---
 hw/vfio/container.c                   | 2 --
 hw/vfio/iommufd.c                     | 2 --
 3 files changed, 7 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index a441932be7..d9f3468f6b 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -115,9 +115,6 @@ OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
 struct VFIOIOMMUClass {
     ObjectClass parent_class;
 
-    /* Properties */
-    const char *hiod_typename;
-
     /* basic feature */
     bool (*setup)(VFIOContainerBase *bcontainer, Error **errp);
     int (*dma_map)(const VFIOContainerBase *bcontainer,
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 2de20692d1..a28f0668db 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -1103,8 +1103,6 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
 
-    vioc->hiod_typename = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO;
-
     vioc->setup = vfio_legacy_setup;
     vioc->dma_map = vfio_legacy_dma_map;
     vioc->dma_unmap = vfio_legacy_dma_unmap;
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 8a010a51ea..f24054a6a5 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -795,8 +795,6 @@ static void vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
 
-    vioc->hiod_typename = TYPE_HOST_IOMMU_DEVICE_IOMMUFD_VFIO;
-
     vioc->dma_map = iommufd_cdev_map;
     vioc->dma_unmap = iommufd_cdev_unmap;
     vioc->attach_device = iommufd_cdev_attach;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities
  2025-04-23  7:28 ` [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities Zhenzhong Duan
@ 2025-04-23  8:31   ` Joao Martins
  0 siblings, 0 replies; 10+ messages in thread
From: Joao Martins @ 2025-04-23  8:31 UTC (permalink / raw)
  To: Zhenzhong Duan, qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, yi.l.liu, chao.p.peng

On 23/04/2025 08:28, Zhenzhong Duan wrote:
> Currently we depend on .realize() calling iommufd_backend_get_device_info()
> to get IOMMU capabilities and check for dirty page tracking support.
> 
> By make a extra separate call, this dependency is removed. This happens
> only during device attach, it's not a hot path.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

I think this looks right now as it's placed in the auto domains path where mdevs
aren't involved:

	Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  hw/vfio/iommufd.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 48db105422..2253778b3a 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -287,7 +287,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>  {
>      ERRP_GUARD();
>      IOMMUFDBackend *iommufd = vbasedev->iommufd;
> -    uint32_t flags = 0;
> +    uint32_t type, flags = 0;
> +    uint64_t hw_caps;
>      VFIOIOASHwpt *hwpt;
>      uint32_t hwpt_id;
>      int ret;
> @@ -324,7 +325,12 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>       * vfio_migration_realize() may decide to use VF dirty tracking
>       * instead.
>       */
> -    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
> +    if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev->devid,
> +                                         &type, NULL, 0, &hw_caps, errp)) {
> +        return false;
> +    }
> +
> +    if (hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
>          flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>      }
>  



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment
  2025-04-23  7:28 ` [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment Zhenzhong Duan
@ 2025-04-23  9:39   ` Joao Martins
  0 siblings, 0 replies; 10+ messages in thread
From: Joao Martins @ 2025-04-23  9:39 UTC (permalink / raw)
  To: Zhenzhong Duan, qemu-devel
  Cc: alex.williamson, clg, eric.auger, nicolinc, yi.l.liu, chao.p.peng,
	Donald Dutile

On 23/04/2025 08:28, Zhenzhong Duan wrote:
> Previously device attaching depends on realize() getting host IOMMU
> capabilities to check dirty tracking support.
> 
> Now we have a separate call to ioctl(IOMMU_GET_HW_INFO) to get host
> IOMMU capabilities and check that for dirty tracking support, there
> is no dependency any more, move realize() call after attachment
> succeed.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Suggested-by: Donald Dutile <ddutile@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  hw/vfio/iommufd.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 2253778b3a..f273dc8712 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -500,17 +500,6 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>  
>      space = vfio_address_space_get(as);
>  
> -    /*
> -     * The HostIOMMUDevice data from legacy backend is static and doesn't need
> -     * any information from the (type1-iommu) backend to be initialized. In
> -     * contrast however, the IOMMUFD HostIOMMUDevice data requires the iommufd
> -     * FD to be connected and having a devid to be able to successfully call
> -     * iommufd_backend_get_device_info().
> -     */
> -    if (!vfio_device_hiod_realize(vbasedev, errp)) {
> -        goto err_alloc_ioas;
> -    }
> -
>      /* try to attach to an existing container in this space */
>      QLIST_FOREACH(bcontainer, &space->containers, next) {
>          container = container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
> @@ -585,6 +574,10 @@ found_container:
>          goto err_listener_register;
>      }
>  
> +    if (!vfio_device_hiod_realize(vbasedev, errp)) {
> +        goto err_hiod_realize;
> +    }
> +
>      /*
>       * TODO: examine RAM_BLOCK_DISCARD stuff, should we do group level
>       * for discarding incompatibility check as well?
> @@ -606,6 +599,8 @@ found_container:
>                                     vbasedev->num_regions, vbasedev->flags);
>      return true;
>  
> +err_hiod_realize:
> +    vfio_cpr_unregister_container(bcontainer);
>  err_listener_register:
>      iommufd_cdev_ram_block_discard_disable(false);
>  err_discard_disable:



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/5] vfio: Move realize after attach_dev
  2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
                   ` (4 preceding siblings ...)
  2025-04-23  7:28 ` [PATCH v2 5/5] vfio: Remove hiod_typename property Zhenzhong Duan
@ 2025-04-23 13:21 ` Cédric Le Goater
  2025-04-23 13:35   ` Cédric Le Goater
  5 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2025-04-23 13:21 UTC (permalink / raw)
  To: Zhenzhong Duan, qemu-devel
  Cc: alex.williamson, eric.auger, nicolinc, joao.m.martins, yi.l.liu,
	chao.p.peng

On 4/23/25 09:28, Zhenzhong Duan wrote:
> Hi,
> 
> This series addresses Cédric's suggestion[1] and Donald's suggestion[2] to
> move realize() call after attach_device().
> 
> This way avoid the need to introduce realize_late() to further complex the
> interface in nesting series.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg01211.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg00898.html
> 
> Test:
> net card passthrough and ping test
> hotplug/unplug
> 
> Based on vfio-next(856f36c005).
> 
> Thanks
> Zhenzhong
> 
> Changelog:
> v2:
> - drop the idea to save host iommu capabilities in VFIODevice.caps
> - introduce a new function to create and realize hiod
> - remove hiod_typename property
> 
> Zhenzhong Duan (5):
>    vfio/iommufd: Make a separate call to get IOMMU capabilities
>    vfio/iommufd: Move realize() after attachment
>    vfio/container: Move realize() after attachment
>    vfio: Cleanup host IOMMU device creation
>    vfio: Remove hiod_typename property
> 
>   include/hw/vfio/vfio-container-base.h |  3 ---
>   include/hw/vfio/vfio-device.h         |  3 ++-
>   hw/vfio/container.c                   | 25 ++++++++++++--------
>   hw/vfio/device.c                      | 33 ++++++++++++---------------
>   hw/vfio/iommufd.c                     | 31 +++++++++++++------------
>   5 files changed, 47 insertions(+), 48 deletions(-)
> 

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/5] vfio: Move realize after attach_dev
  2025-04-23 13:21 ` [PATCH v2 0/5] vfio: Move realize after attach_dev Cédric Le Goater
@ 2025-04-23 13:35   ` Cédric Le Goater
  0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2025-04-23 13:35 UTC (permalink / raw)
  To: Zhenzhong Duan, qemu-devel
  Cc: alex.williamson, eric.auger, nicolinc, joao.m.martins, yi.l.liu,
	chao.p.peng

On 4/23/25 15:21, Cédric Le Goater wrote:
> On 4/23/25 09:28, Zhenzhong Duan wrote:
>> Hi,
>>
>> This series addresses Cédric's suggestion[1] and Donald's suggestion[2] to
>> move realize() call after attach_device().
>>
>> This way avoid the need to introduce realize_late() to further complex the
>> interface in nesting series.
>>
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg01211.html
>> [2] https://lists.gnu.org/archive/html/qemu-devel/2025-04/msg00898.html
>>
>> Test:
>> net card passthrough and ping test
>> hotplug/unplug
>>
>> Based on vfio-next(856f36c005).
>>
>> Thanks
>> Zhenzhong
>>
>> Changelog:
>> v2:
>> - drop the idea to save host iommu capabilities in VFIODevice.caps
>> - introduce a new function to create and realize hiod
>> - remove hiod_typename property
>>
>> Zhenzhong Duan (5):
>>    vfio/iommufd: Make a separate call to get IOMMU capabilities
>>    vfio/iommufd: Move realize() after attachment
>>    vfio/container: Move realize() after attachment
>>    vfio: Cleanup host IOMMU device creation
>>    vfio: Remove hiod_typename property
>>
>>   include/hw/vfio/vfio-container-base.h |  3 ---
>>   include/hw/vfio/vfio-device.h         |  3 ++-
>>   hw/vfio/container.c                   | 25 ++++++++++++--------
>>   hw/vfio/device.c                      | 33 ++++++++++++---------------
>>   hw/vfio/iommufd.c                     | 31 +++++++++++++------------
>>   5 files changed, 47 insertions(+), 48 deletions(-)
>>
> 
> Reviewed-by: Cédric Le Goater <clg@redhat.com>

Applied to vfio-next.

Thanks,

C.




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-23 13:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23  7:28 [PATCH v2 0/5] vfio: Move realize after attach_dev Zhenzhong Duan
2025-04-23  7:28 ` [PATCH v2 1/5] vfio/iommufd: Make a separate call to get IOMMU capabilities Zhenzhong Duan
2025-04-23  8:31   ` Joao Martins
2025-04-23  7:28 ` [PATCH v2 2/5] vfio/iommufd: Move realize() after attachment Zhenzhong Duan
2025-04-23  9:39   ` Joao Martins
2025-04-23  7:28 ` [PATCH v2 3/5] vfio/container: " Zhenzhong Duan
2025-04-23  7:28 ` [PATCH v2 4/5] vfio: Cleanup host IOMMU device creation Zhenzhong Duan
2025-04-23  7:28 ` [PATCH v2 5/5] vfio: Remove hiod_typename property Zhenzhong Duan
2025-04-23 13:21 ` [PATCH v2 0/5] vfio: Move realize after attach_dev Cédric Le Goater
2025-04-23 13:35   ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).