[PATCH v1 0/6] VFIO and IOMMU prerequisite stuff for IOMMU nesting support

[PATCH v1 0/6] VFIO and IOMMU prerequisite stuff for IOMMU nesting support

[Zhenzhong Duan]

Hi,

The first 6 patches of [1] are all VFIO or IOMMUFD related additions.
Split them out per Cédric and seek for quick acceptance.

I didn't copy changelog from [1] as it's a mix of the whole nesting series.
Compared to rfcv3 in [1], changed to save raw data in VendorCaps, so we can
keep all vendor structure decoding inside the backend and VFIO wouldn't need
to care about types nor what's inside the data.


Test done:
- VFIO devices hotplug/unplug
- build test on Windows

[1] https://lists.gnu.org/archive/html/qemu-devel/2025-05/msg05002.html

Thanks
Zhenzhong

Zhenzhong Duan (6):
  backends/iommufd: Add a helper to invalidate user-managed HWPT
  vfio/iommufd: Add properties and handlers to
    TYPE_HOST_IOMMU_DEVICE_IOMMUFD
  vfio/iommufd: Initialize iommufd specific members in
    HostIOMMUDeviceIOMMUFD
  vfio/iommufd: Implement [at|de]tach_hwpt handlers
  vfio/iommufd: Save vendor specific device info
  iommufd: Implement query of host VTD IOMMU's capability

 hw/i386/intel_iommu_internal.h     |  1 +
 include/system/host_iommu_device.h | 18 ++++++
 include/system/iommufd.h           | 54 +++++++++++++++++
 backends/iommufd.c                 | 94 +++++++++++++++++++++++++++++-
 hw/vfio/iommufd.c                  | 36 ++++++++++--
 backends/trace-events              |  1 +
 6 files changed, 197 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v1 1/6] backends/iommufd: Add a helper to invalidate user-managed HWPT

[Zhenzhong Duan]

This helper passes cache invalidation request from guest to invalidate
stage-1 page table cache in host hardware.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/system/iommufd.h |  4 ++++
 backends/iommufd.c       | 33 +++++++++++++++++++++++++++++++++
 backends/trace-events    |  1 +
 3 files changed, 38 insertions(+)

diff --git a/include/system/iommufd.h b/include/system/iommufd.h
index cbab75bfbf..5399519626 100644
--- a/include/system/iommufd.h
+++ b/include/system/iommufd.h
@@ -61,6 +61,10 @@ bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
                                       uint64_t iova, ram_addr_t size,
                                       uint64_t page_size, uint64_t *data,
                                       Error **errp);
+bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
+                                      uint32_t data_type, uint32_t entry_len,
+                                      uint32_t *entry_num, void *data_ptr,
+                                      Error **errp);
 
 #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
 #endif
diff --git a/backends/iommufd.c b/backends/iommufd.c
index b73f75cd0b..c8788a6438 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -311,6 +311,39 @@ bool iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
     return true;
 }
 
+bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
+                                      uint32_t data_type, uint32_t entry_len,
+                                      uint32_t *entry_num, void *data_ptr,
+                                      Error **errp)
+{
+    int ret, fd = be->fd;
+    uint32_t total_entries = *entry_num;
+    struct iommu_hwpt_invalidate cache = {
+        .size = sizeof(cache),
+        .hwpt_id = id,
+        .data_type = data_type,
+        .entry_len = entry_len,
+        .entry_num = total_entries,
+        .data_uptr = (uintptr_t)data_ptr,
+    };
+
+    ret = ioctl(fd, IOMMU_HWPT_INVALIDATE, &cache);
+    trace_iommufd_backend_invalidate_cache(fd, id, data_type, entry_len,
+                                           total_entries, cache.entry_num,
+                                           (uintptr_t)data_ptr,
+                                           ret ? errno : 0);
+    if (ret) {
+        *entry_num = cache.entry_num;
+        error_setg_errno(errp, errno, "IOMMU_HWPT_INVALIDATE failed:"
+                         " totally %d entries, processed %d entries",
+                         total_entries, cache.entry_num);
+    } else {
+        g_assert(total_entries == cache.entry_num);
+    }
+
+    return !ret;
+}
+
 static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
 {
     HostIOMMUDeviceCaps *caps = &hiod->caps;
diff --git a/backends/trace-events b/backends/trace-events
index 40811a3162..7278214ea5 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -18,3 +18,4 @@ iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id, uint32_
 iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d id=%d (%d)"
 iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int ret) " iommufd=%d hwpt=%u enable=%d (%d)"
 iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%u iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)"
+iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num, uint64_t data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
-- 
2.34.1

[PATCH v1 2/6] vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD

[Zhenzhong Duan]

Enhance HostIOMMUDeviceIOMMUFD object with 3 new members, specific
to the iommufd BE + 2 new class functions.

IOMMUFD BE includes IOMMUFD handle, devid and hwpt_id. IOMMUFD handle
and devid are used to allocate/free ioas and hwpt. hwpt_id is used to
re-attach IOMMUFD backed device to its default VFIO sub-system created
hwpt, i.e., when vIOMMU is disabled by guest. These properties will be
initialized after attachment.

2 new class functions are [at|de]tach_hwpt(). They are used to
attach/detach hwpt. VFIO and VDPA can have different implementions,
so implementation will be in sub-class instead of HostIOMMUDeviceIOMMUFD,
e.g., in HostIOMMUDeviceIOMMUFDVFIO.

Add two wrappers host_iommu_device_iommufd_[at|de]tach_hwpt to
wrap the two functions.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/system/iommufd.h | 50 ++++++++++++++++++++++++++++++++++++++++
 backends/iommufd.c       | 22 ++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/include/system/iommufd.h b/include/system/iommufd.h
index 5399519626..a704575662 100644
--- a/include/system/iommufd.h
+++ b/include/system/iommufd.h
@@ -67,4 +67,54 @@ bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
                                       Error **errp);
 
 #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
+OBJECT_DECLARE_TYPE(HostIOMMUDeviceIOMMUFD, HostIOMMUDeviceIOMMUFDClass,
+                    HOST_IOMMU_DEVICE_IOMMUFD)
+
+/* Overload of the host IOMMU device for the iommufd backend */
+struct HostIOMMUDeviceIOMMUFD {
+    HostIOMMUDevice parent_obj;
+
+    IOMMUFDBackend *iommufd;
+    uint32_t devid;
+    uint32_t hwpt_id;
+};
+
+struct HostIOMMUDeviceIOMMUFDClass {
+    HostIOMMUDeviceClass parent_class;
+
+    /**
+     * @attach_hwpt: attach host IOMMU device to IOMMUFD hardware page table.
+     * VFIO and VDPA device can have different implementation.
+     *
+     * Mandatory callback.
+     *
+     * @idev: host IOMMU device backed by IOMMUFD backend.
+     *
+     * @hwpt_id: ID of IOMMUFD hardware page table.
+     *
+     * @errp: pass an Error out when attachment fails.
+     *
+     * Returns: true on success, false on failure.
+     */
+    bool (*attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, uint32_t hwpt_id,
+                        Error **errp);
+    /**
+     * @detach_hwpt: detach host IOMMU device from IOMMUFD hardware page table.
+     * VFIO and VDPA device can have different implementation.
+     *
+     * Mandatory callback.
+     *
+     * @idev: host IOMMU device backed by IOMMUFD backend.
+     *
+     * @errp: pass an Error out when attachment fails.
+     *
+     * Returns: true on success, false on failure.
+     */
+    bool (*detach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, Error **errp);
+};
+
+bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           uint32_t hwpt_id, Error **errp);
+bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           Error **errp);
 #endif
diff --git a/backends/iommufd.c b/backends/iommufd.c
index c8788a6438..b114fb08e7 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -344,6 +344,26 @@ bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
     return !ret;
 }
 
+bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           uint32_t hwpt_id, Error **errp)
+{
+    HostIOMMUDeviceIOMMUFDClass *idevc =
+        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
+
+    g_assert(idevc->attach_hwpt);
+    return idevc->attach_hwpt(idev, hwpt_id, errp);
+}
+
+bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           Error **errp)
+{
+    HostIOMMUDeviceIOMMUFDClass *idevc =
+        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
+
+    g_assert(idevc->detach_hwpt);
+    return idevc->detach_hwpt(idev, errp);
+}
+
 static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
 {
     HostIOMMUDeviceCaps *caps = &hiod->caps;
@@ -382,6 +402,8 @@ static const TypeInfo types[] = {
     }, {
         .name = TYPE_HOST_IOMMU_DEVICE_IOMMUFD,
         .parent = TYPE_HOST_IOMMU_DEVICE,
+        .instance_size = sizeof(HostIOMMUDeviceIOMMUFD),
+        .class_size = sizeof(HostIOMMUDeviceIOMMUFDClass),
         .class_init = hiod_iommufd_class_init,
         .abstract = true,
     }
-- 
2.34.1

[PATCH v1 3/6] vfio/iommufd: Initialize iommufd specific members in HostIOMMUDeviceIOMMUFD

[Zhenzhong Duan]

There are three iommufd specific members in HostIOMMUDeviceIOMMUFD
that need to be initialized after attachment, they will all be used
by vIOMMU.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index af1c7ab10a..5fde2b633a 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -814,6 +814,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
                                       Error **errp)
 {
     VFIODevice *vdev = opaque;
+    HostIOMMUDeviceIOMMUFD *idev;
     HostIOMMUDeviceCaps *caps = &hiod->caps;
     enum iommu_hw_info_type type;
     union {
@@ -833,6 +834,11 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     caps->type = type;
     caps->hw_caps = hw_caps;
 
+    idev = HOST_IOMMU_DEVICE_IOMMUFD(hiod);
+    idev->iommufd = vdev->iommufd;
+    idev->devid = vdev->devid;
+    idev->hwpt_id = vdev->hwpt->hwpt_id;
+
     return true;
 }
 
-- 
2.34.1

[PATCH v1 4/6] vfio/iommufd: Implement [at|de]tach_hwpt handlers

[Zhenzhong Duan]

Implement [at|de]tach_hwpt handlers in VFIO subsystem. vIOMMU
utilizes them to attach to or detach from hwpt on host side.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/iommufd.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 5fde2b633a..d661737c17 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -810,6 +810,24 @@ static void vfio_iommu_iommufd_class_init(ObjectClass *klass, const void *data)
     vioc->query_dirty_bitmap = iommufd_query_dirty_bitmap;
 };
 
+static bool
+host_iommu_device_iommufd_vfio_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           uint32_t hwpt_id, Error **errp)
+{
+    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
+
+    return !iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
+}
+
+static bool
+host_iommu_device_iommufd_vfio_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
+                                           Error **errp)
+{
+    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
+
+    return iommufd_cdev_detach_ioas_hwpt(vbasedev, errp);
+}
+
 static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
                                       Error **errp)
 {
@@ -864,10 +882,14 @@ hiod_iommufd_vfio_get_page_size_mask(HostIOMMUDevice *hiod)
 static void hiod_iommufd_vfio_class_init(ObjectClass *oc, const void *data)
 {
     HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_CLASS(oc);
+    HostIOMMUDeviceIOMMUFDClass *idevc = HOST_IOMMU_DEVICE_IOMMUFD_CLASS(oc);
 
     hiodc->realize = hiod_iommufd_vfio_realize;
     hiodc->get_iova_ranges = hiod_iommufd_vfio_get_iova_ranges;
     hiodc->get_page_size_mask = hiod_iommufd_vfio_get_page_size_mask;
+
+    idevc->attach_hwpt = host_iommu_device_iommufd_vfio_attach_hwpt;
+    idevc->detach_hwpt = host_iommu_device_iommufd_vfio_detach_hwpt;
 };
 
 static const TypeInfo types[] = {
-- 
2.34.1

[PATCH v1 5/6] vfio/iommufd: Save vendor specific device info

[Zhenzhong Duan]

Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
specific. Save them as raw data in a union supporting different vendors,
then get_cap() can provide a general interface for querying capability
and hide different vendor data format under it.

Because IOMMU_GET_HW_INFO is only supported in linux, so declare those
capability related structures with CONFIG_LINUX.

Suggested-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/system/host_iommu_device.h | 11 +++++++++++
 hw/vfio/iommufd.c                  |  8 +++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 809cced4ba..10fccc10be 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -14,6 +14,13 @@
 
 #include "qom/object.h"
 #include "qapi/error.h"
+#ifdef CONFIG_LINUX
+#include "linux/iommufd.h"
+
+typedef union VendorCaps {
+    struct iommu_hw_info_vtd vtd;
+    struct iommu_hw_info_arm_smmuv3 smmuv3;
+} VendorCaps;
 
 /**
  * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities.
@@ -26,7 +33,9 @@
 typedef struct HostIOMMUDeviceCaps {
     uint32_t type;
     uint64_t hw_caps;
+    VendorCaps vendor_caps;
 } HostIOMMUDeviceCaps;
+#endif
 
 #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
 OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE)
@@ -38,7 +47,9 @@ struct HostIOMMUDevice {
     void *agent; /* pointer to agent device, ie. VFIO or VDPA device */
     PCIBus *aliased_bus;
     int aliased_devfn;
+#ifdef CONFIG_LINUX
     HostIOMMUDeviceCaps caps;
+#endif
 };
 
 /**
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index d661737c17..fbf47cab09 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -834,16 +834,14 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     VFIODevice *vdev = opaque;
     HostIOMMUDeviceIOMMUFD *idev;
     HostIOMMUDeviceCaps *caps = &hiod->caps;
+    VendorCaps *vendor_caps = &caps->vendor_caps;
     enum iommu_hw_info_type type;
-    union {
-        struct iommu_hw_info_vtd vtd;
-    } data;
     uint64_t hw_caps;
 
     hiod->agent = opaque;
 
-    if (!iommufd_backend_get_device_info(vdev->iommufd, vdev->devid,
-                                         &type, &data, sizeof(data),
+    if (!iommufd_backend_get_device_info(vdev->iommufd, vdev->devid, &type,
+                                         vendor_caps, sizeof(*vendor_caps),
                                          &hw_caps, errp)) {
         return false;
     }
-- 
2.34.1

[PATCH v1 6/6] iommufd: Implement query of host VTD IOMMU's capability

[Zhenzhong Duan]

Implement query of HOST_IOMMU_DEVICE_CAP_[NESTING|FS1GP|ERRATA] for IOMMUFD
backed host VTD IOMMU device.

Query on these capabilities is not supported for legacy backend because there
is no plan to support nesting with legacy backend backed host device.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu_internal.h     |  1 +
 include/system/host_iommu_device.h |  7 ++++++
 backends/iommufd.c                 | 39 ++++++++++++++++++++++++++++--
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index e8b211e8b0..2cda744786 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -191,6 +191,7 @@
 #define VTD_ECAP_PT                 (1ULL << 6)
 #define VTD_ECAP_SC                 (1ULL << 7)
 #define VTD_ECAP_MHMV               (15ULL << 20)
+#define VTD_ECAP_NEST               (1ULL << 26)
 #define VTD_ECAP_SRS                (1ULL << 31)
 #define VTD_ECAP_PASID              (1ULL << 40)
 #define VTD_ECAP_SMTS               (1ULL << 43)
diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 10fccc10be..c2770cb469 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -29,6 +29,10 @@ typedef union VendorCaps {
  *
  * @hw_caps: host platform IOMMU capabilities (e.g. on IOMMUFD this represents
  *           the @out_capabilities value returned from IOMMU_GET_HW_INFO ioctl)
+ *
+ * @vendor_caps: host platform IOMMU vendor specific capabilities (e.g. on
+ *               IOMMUFD this represents raw vendor data from data_uptr
+ *               buffer returned from IOMMU_GET_HW_INFO ioctl)
  */
 typedef struct HostIOMMUDeviceCaps {
     uint32_t type;
@@ -116,6 +120,9 @@ struct HostIOMMUDeviceClass {
  */
 #define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE        0
 #define HOST_IOMMU_DEVICE_CAP_AW_BITS           1
+#define HOST_IOMMU_DEVICE_CAP_NESTING           2
+#define HOST_IOMMU_DEVICE_CAP_FS1GP             3
+#define HOST_IOMMU_DEVICE_CAP_ERRATA            4
 
 #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
 #endif
diff --git a/backends/iommufd.c b/backends/iommufd.c
index b114fb08e7..63209659f3 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -21,6 +21,7 @@
 #include "hw/vfio/vfio-device.h"
 #include <sys/ioctl.h>
 #include <linux/iommufd.h>
+#include "hw/i386/intel_iommu_internal.h"
 
 static void iommufd_backend_init(Object *obj)
 {
@@ -364,6 +365,41 @@ bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
     return idevc->detach_hwpt(idev, errp);
 }
 
+static int hiod_iommufd_get_vtd_cap(HostIOMMUDevice *hiod, int cap,
+                                    Error **errp)
+{
+    struct iommu_hw_info_vtd *caps = &hiod->caps.vendor_caps.vtd;
+
+    switch (cap) {
+    case HOST_IOMMU_DEVICE_CAP_NESTING:
+        return !!(caps->ecap_reg & VTD_ECAP_NEST);
+    case HOST_IOMMU_DEVICE_CAP_FS1GP:
+        return !!(caps->cap_reg & VTD_CAP_FS1GP);
+    case HOST_IOMMU_DEVICE_CAP_ERRATA:
+        return caps->flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17;
+    default:
+        error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
+        return -EINVAL;
+    }
+}
+
+static int hiod_iommufd_get_vendor_cap(HostIOMMUDevice *hiod, int cap,
+                                       Error **errp)
+{
+    enum iommu_hw_info_type type = hiod->caps.type;
+
+    switch (type) {
+    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
+        return hiod_iommufd_get_vtd_cap(hiod, cap, errp);
+    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
+    case IOMMU_HW_INFO_TYPE_NONE:
+        break;
+    }
+
+    error_setg(errp, "%s: unsupported capability type %x", hiod->name, type);
+    return -EINVAL;
+}
+
 static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
 {
     HostIOMMUDeviceCaps *caps = &hiod->caps;
@@ -374,8 +410,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
     case HOST_IOMMU_DEVICE_CAP_AW_BITS:
         return vfio_device_get_aw_bits(hiod->agent);
     default:
-        error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
-        return -EINVAL;
+        return hiod_iommufd_get_vendor_cap(hiod, cap, errp);
     }
 }
 
-- 
2.34.1

Re: [PATCH v1 1/6] backends/iommufd: Add a helper to invalidate user-managed HWPT

[Cédric Le Goater]

Hello Zhenzhong,

On 5/28/25 08:04, Zhenzhong Duan wrote:
> This helper passes cache invalidation request from guest to invalidate
> stage-1 page table cache in host hardware.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/system/iommufd.h |  4 ++++
>   backends/iommufd.c       | 33 +++++++++++++++++++++++++++++++++
>   backends/trace-events    |  1 +
>   3 files changed, 38 insertions(+)
> 
> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
> index cbab75bfbf..5399519626 100644
> --- a/include/system/iommufd.h
> +++ b/include/system/iommufd.h
> @@ -61,6 +61,10 @@ bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
>                                         uint64_t iova, ram_addr_t size,
>                                         uint64_t page_size, uint64_t *data,
>                                         Error **errp);
> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
> +                                      uint32_t data_type, uint32_t entry_len,
> +                                      uint32_t *entry_num, void *data_ptr,
> +                                      Error **errp);
>   
>   #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
>   #endif
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index b73f75cd0b..c8788a6438 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -311,6 +311,39 @@ bool iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
>       return true;
>   }
>   
> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
> +                                      uint32_t data_type, uint32_t entry_len,
> +                                      uint32_t *entry_num, void *data_ptr,
> +                                      Error **errp)
> +{
> +    int ret, fd = be->fd;
> +    uint32_t total_entries = *entry_num;
> +    struct iommu_hwpt_invalidate cache = {
> +        .size = sizeof(cache),
> +        .hwpt_id = id,
> +        .data_type = data_type,
> +        .entry_len = entry_len,
> +        .entry_num = total_entries,
> +        .data_uptr = (uintptr_t)data_ptr,

Minor, other helpers use a 'data' variable name.

> +    };
> +
> +    ret = ioctl(fd, IOMMU_HWPT_INVALIDATE, &cache);
> +    trace_iommufd_backend_invalidate_cache(fd, id, data_type, entry_len,
> +                                           total_entries, cache.entry_num,
> +                                           (uintptr_t)data_ptr,
> +                                           ret ? errno : 0);
> +    if (ret) {
> +        *entry_num = cache.entry_num;
> +        error_setg_errno(errp, errno, "IOMMU_HWPT_INVALIDATE failed:"
> +                         " totally %d entries, processed %d entries",
> +                         total_entries, cache.entry_num);
> +    } else {
> +        g_assert(total_entries == cache.entry_num);

Killing the VMM because a kernel device ioctl failed is brute force.
Can't we update the 'Error *' parameter instead to report that the
invalidation is partial or something went wrong ?

What kind of errors are we trying to catch ?

Looking at the kernel iommufd_hwpt_invalidate() routine and
intel_nested_cache_invalidate_user(), it doesn't seem possible to
return a different number of cache entries. Are you anticipating
other implementations (sMMU) ?

Thanks,

C.

Re: [PATCH v1 2/6] vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD

[Cédric Le Goater]

On 5/28/25 08:04, Zhenzhong Duan wrote:
> Enhance HostIOMMUDeviceIOMMUFD object with 3 new members, specific
> to the iommufd BE + 2 new class functions.
> 
> IOMMUFD BE includes IOMMUFD handle, devid and hwpt_id. IOMMUFD handle
> and devid are used to allocate/free ioas and hwpt. hwpt_id is used to
> re-attach IOMMUFD backed device to its default VFIO sub-system created
> hwpt, i.e., when vIOMMU is disabled by guest. These properties will be
> initialized after attachment.
> 
> 2 new class functions are [at|de]tach_hwpt(). They are used to
> attach/detach hwpt. VFIO and VDPA can have different implementions,
> so implementation will be in sub-class instead of HostIOMMUDeviceIOMMUFD,
> e.g., in HostIOMMUDeviceIOMMUFDVFIO.
> 
> Add two wrappers host_iommu_device_iommufd_[at|de]tach_hwpt to
> wrap the two functions.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/system/iommufd.h | 50 ++++++++++++++++++++++++++++++++++++++++
>   backends/iommufd.c       | 22 ++++++++++++++++++
>   2 files changed, 72 insertions(+)
> 
> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
> index 5399519626..a704575662 100644
> --- a/include/system/iommufd.h
> +++ b/include/system/iommufd.h
> @@ -67,4 +67,54 @@ bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>                                         Error **errp);
>   
>   #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
> +OBJECT_DECLARE_TYPE(HostIOMMUDeviceIOMMUFD, HostIOMMUDeviceIOMMUFDClass,
> +                    HOST_IOMMU_DEVICE_IOMMUFD)
> +
> +/* Overload of the host IOMMU device for the iommufd backend */
> +struct HostIOMMUDeviceIOMMUFD {
> +    HostIOMMUDevice parent_obj;
> +
> +    IOMMUFDBackend *iommufd;
> +    uint32_t devid;
> +    uint32_t hwpt_id;
> +};

the addition of new attributes would be better placed in the next patch.

Anyhow,



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

Thanks,

C.



> +struct HostIOMMUDeviceIOMMUFDClass {
> +    HostIOMMUDeviceClass parent_class;
> +
> +    /**
> +     * @attach_hwpt: attach host IOMMU device to IOMMUFD hardware page table.
> +     * VFIO and VDPA device can have different implementation.
> +     *
> +     * Mandatory callback.
> +     *
> +     * @idev: host IOMMU device backed by IOMMUFD backend.
> +     *
> +     * @hwpt_id: ID of IOMMUFD hardware page table.
> +     *
> +     * @errp: pass an Error out when attachment fails.
> +     *
> +     * Returns: true on success, false on failure.
> +     */
> +    bool (*attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, uint32_t hwpt_id,
> +                        Error **errp);
> +    /**
> +     * @detach_hwpt: detach host IOMMU device from IOMMUFD hardware page table.
> +     * VFIO and VDPA device can have different implementation.
> +     *
> +     * Mandatory callback.
> +     *
> +     * @idev: host IOMMU device backed by IOMMUFD backend.
> +     *
> +     * @errp: pass an Error out when attachment fails.
> +     *
> +     * Returns: true on success, false on failure.
> +     */
> +    bool (*detach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, Error **errp);
> +};
> +
> +bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           uint32_t hwpt_id, Error **errp);
> +bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           Error **errp);
>   #endif
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index c8788a6438..b114fb08e7 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -344,6 +344,26 @@ bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>       return !ret;
>   }
>   
> +bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           uint32_t hwpt_id, Error **errp)
> +{
> +    HostIOMMUDeviceIOMMUFDClass *idevc =
> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
> +
> +    g_assert(idevc->attach_hwpt);
> +    return idevc->attach_hwpt(idev, hwpt_id, errp);
> +}
> +
> +bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           Error **errp)
> +{
> +    HostIOMMUDeviceIOMMUFDClass *idevc =
> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
> +
> +    g_assert(idevc->detach_hwpt);
> +    return idevc->detach_hwpt(idev, errp);
> +}
> +
>   static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>   {
>       HostIOMMUDeviceCaps *caps = &hiod->caps;
> @@ -382,6 +402,8 @@ static const TypeInfo types[] = {
>       }, {
>           .name = TYPE_HOST_IOMMU_DEVICE_IOMMUFD,
>           .parent = TYPE_HOST_IOMMU_DEVICE,
> +        .instance_size = sizeof(HostIOMMUDeviceIOMMUFD),
> +        .class_size = sizeof(HostIOMMUDeviceIOMMUFDClass),
>           .class_init = hiod_iommufd_class_init,
>           .abstract = true,
>       }

Re: [PATCH v1 3/6] vfio/iommufd: Initialize iommufd specific members in HostIOMMUDeviceIOMMUFD

[Cédric Le Goater]

On 5/28/25 08:04, Zhenzhong Duan wrote:
> There are three iommufd specific members in HostIOMMUDeviceIOMMUFD
> that need to be initialized after attachment, they will all be used
> by vIOMMU.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   hw/vfio/iommufd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index af1c7ab10a..5fde2b633a 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -814,6 +814,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>                                         Error **errp)
>   {
>       VFIODevice *vdev = opaque;
> +    HostIOMMUDeviceIOMMUFD *idev;
>       HostIOMMUDeviceCaps *caps = &hiod->caps;
>       enum iommu_hw_info_type type;
>       union {
> @@ -833,6 +834,11 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>       caps->type = type;
>       caps->hw_caps = hw_caps;
>   
> +    idev = HOST_IOMMU_DEVICE_IOMMUFD(hiod);
> +    idev->iommufd = vdev->iommufd;
> +    idev->devid = vdev->devid;
> +    idev->hwpt_id = vdev->hwpt->hwpt_id;
> +
>       return true;
>   }
>   

Re: [PATCH v1 4/6] vfio/iommufd: Implement [at|de]tach_hwpt handlers

[Cédric Le Goater]

On 5/28/25 08:04, Zhenzhong Duan wrote:
> Implement [at|de]tach_hwpt handlers in VFIO subsystem. vIOMMU
> utilizes them to attach to or detach from hwpt on host side.
> 
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


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

Thanks,

C.


> ---
>   hw/vfio/iommufd.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 5fde2b633a..d661737c17 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -810,6 +810,24 @@ static void vfio_iommu_iommufd_class_init(ObjectClass *klass, const void *data)
>       vioc->query_dirty_bitmap = iommufd_query_dirty_bitmap;
>   };
>   
> +static bool
> +host_iommu_device_iommufd_vfio_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           uint32_t hwpt_id, Error **errp)
> +{
> +    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
> +
> +    return !iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
> +}
> +
> +static bool
> +host_iommu_device_iommufd_vfio_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           Error **errp)
> +{
> +    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
> +
> +    return iommufd_cdev_detach_ioas_hwpt(vbasedev, errp);
> +}
> +
>   static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>                                         Error **errp)
>   {
> @@ -864,10 +882,14 @@ hiod_iommufd_vfio_get_page_size_mask(HostIOMMUDevice *hiod)
>   static void hiod_iommufd_vfio_class_init(ObjectClass *oc, const void *data)
>   {
>       HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_CLASS(oc);
> +    HostIOMMUDeviceIOMMUFDClass *idevc = HOST_IOMMU_DEVICE_IOMMUFD_CLASS(oc);
>   
>       hiodc->realize = hiod_iommufd_vfio_realize;
>       hiodc->get_iova_ranges = hiod_iommufd_vfio_get_iova_ranges;
>       hiodc->get_page_size_mask = hiod_iommufd_vfio_get_page_size_mask;
> +
> +    idevc->attach_hwpt = host_iommu_device_iommufd_vfio_attach_hwpt;
> +    idevc->detach_hwpt = host_iommu_device_iommufd_vfio_detach_hwpt;
>   };
>   
>   static const TypeInfo types[] = {

Re: [PATCH v1 6/6] iommufd: Implement query of host VTD IOMMU's capability

[Cédric Le Goater]

On 5/28/25 08:04, Zhenzhong Duan wrote:
> Implement query of HOST_IOMMU_DEVICE_CAP_[NESTING|FS1GP|ERRATA] for IOMMUFD
> backed host VTD IOMMU device.
> 
> Query on these capabilities is not supported for legacy backend because there
> is no plan to support nesting with legacy backend backed host device.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu_internal.h     |  1 +
>   include/system/host_iommu_device.h |  7 ++++++
>   backends/iommufd.c                 | 39 ++++++++++++++++++++++++++++--
>   3 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index e8b211e8b0..2cda744786 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -191,6 +191,7 @@
>   #define VTD_ECAP_PT                 (1ULL << 6)
>   #define VTD_ECAP_SC                 (1ULL << 7)
>   #define VTD_ECAP_MHMV               (15ULL << 20)
> +#define VTD_ECAP_NEST               (1ULL << 26)
>   #define VTD_ECAP_SRS                (1ULL << 31)
>   #define VTD_ECAP_PASID              (1ULL << 40)
>   #define VTD_ECAP_SMTS               (1ULL << 43)
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 10fccc10be..c2770cb469 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -29,6 +29,10 @@ typedef union VendorCaps {
>    *
>    * @hw_caps: host platform IOMMU capabilities (e.g. on IOMMUFD this represents
>    *           the @out_capabilities value returned from IOMMU_GET_HW_INFO ioctl)
> + *
> + * @vendor_caps: host platform IOMMU vendor specific capabilities (e.g. on
> + *               IOMMUFD this represents raw vendor data from data_uptr
> + *               buffer returned from IOMMU_GET_HW_INFO ioctl)
>    */
>   typedef struct HostIOMMUDeviceCaps {
>       uint32_t type;
> @@ -116,6 +120,9 @@ struct HostIOMMUDeviceClass {
>    */
>   #define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE        0
>   #define HOST_IOMMU_DEVICE_CAP_AW_BITS           1
> +#define HOST_IOMMU_DEVICE_CAP_NESTING           2
> +#define HOST_IOMMU_DEVICE_CAP_FS1GP             3
> +#define HOST_IOMMU_DEVICE_CAP_ERRATA            4
>   
>   #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
>   #endif
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index b114fb08e7..63209659f3 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -21,6 +21,7 @@
>   #include "hw/vfio/vfio-device.h"
>   #include <sys/ioctl.h>
>   #include <linux/iommufd.h>
> +#include "hw/i386/intel_iommu_internal.h"
>   
>   static void iommufd_backend_init(Object *obj)
>   {
> @@ -364,6 +365,41 @@ bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>       return idevc->detach_hwpt(idev, errp);
>   }
>   
> +static int hiod_iommufd_get_vtd_cap(HostIOMMUDevice *hiod, int cap,
> +                                    Error **errp)
> +{
> +    struct iommu_hw_info_vtd *caps = &hiod->caps.vendor_caps.vtd;
> +
> +    switch (cap) {
> +    case HOST_IOMMU_DEVICE_CAP_NESTING:
> +        return !!(caps->ecap_reg & VTD_ECAP_NEST);
> +    case HOST_IOMMU_DEVICE_CAP_FS1GP:
> +        return !!(caps->cap_reg & VTD_CAP_FS1GP);
> +    case HOST_IOMMU_DEVICE_CAP_ERRATA:
> +        return caps->flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17;
> +    default:
> +        error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> +        return -EINVAL;
> +    }
> +}


This is intel specific. Why not handle these capabilities directly from
vtd_check_hiod() under hw/i386/intel_iommu.c ?


Thanks,

C.


> +static int hiod_iommufd_get_vendor_cap(HostIOMMUDevice *hiod, int cap,
> +                                       Error **errp)
> +{
> +    enum iommu_hw_info_type type = hiod->caps.type;
> +
> +    switch (type) {
> +    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
> +        return hiod_iommufd_get_vtd_cap(hiod, cap, errp);
> +    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
> +    case IOMMU_HW_INFO_TYPE_NONE:
> +        break;
> +    }
> +
> +    error_setg(errp, "%s: unsupported capability type %x", hiod->name, type);
> +    return -EINVAL;
> +}
> +
>   static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>   {
>       HostIOMMUDeviceCaps *caps = &hiod->caps;
> @@ -374,8 +410,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>       case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>           return vfio_device_get_aw_bits(hiod->agent);
>       default:
> -        error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> -        return -EINVAL;
> +        return hiod_iommufd_get_vendor_cap(hiod, cap, errp);
>       }
>   }
>   

RE: [PATCH v1 1/6] backends/iommufd: Add a helper to invalidate user-managed HWPT

[Duan, Zhenzhong]

Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v1 1/6] backends/iommufd: Add a helper to invalidate user-
>managed HWPT
>
>Hello Zhenzhong,
>
>On 5/28/25 08:04, Zhenzhong Duan wrote:
>> This helper passes cache invalidation request from guest to invalidate
>> stage-1 page table cache in host hardware.
>>
>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/system/iommufd.h |  4 ++++
>>   backends/iommufd.c       | 33 +++++++++++++++++++++++++++++++++
>>   backends/trace-events    |  1 +
>>   3 files changed, 38 insertions(+)
>>
>> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>> index cbab75bfbf..5399519626 100644
>> --- a/include/system/iommufd.h
>> +++ b/include/system/iommufd.h
>> @@ -61,6 +61,10 @@ bool
>iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
>>                                         uint64_t iova, ram_addr_t size,
>>                                         uint64_t page_size, uint64_t *data,
>>                                         Error **errp);
>> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>> +                                      uint32_t data_type, uint32_t entry_len,
>> +                                      uint32_t *entry_num, void *data_ptr,
>> +                                      Error **errp);
>>
>>   #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD
>TYPE_HOST_IOMMU_DEVICE "-iommufd"
>>   #endif
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index b73f75cd0b..c8788a6438 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -311,6 +311,39 @@ bool
>iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
>>       return true;
>>   }
>>
>> +bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>> +                                      uint32_t data_type, uint32_t entry_len,
>> +                                      uint32_t *entry_num, void *data_ptr,
>> +                                      Error **errp)
>> +{
>> +    int ret, fd = be->fd;
>> +    uint32_t total_entries = *entry_num;
>> +    struct iommu_hwpt_invalidate cache = {
>> +        .size = sizeof(cache),
>> +        .hwpt_id = id,
>> +        .data_type = data_type,
>> +        .entry_len = entry_len,
>> +        .entry_num = total_entries,
>> +        .data_uptr = (uintptr_t)data_ptr,
>
>Minor, other helpers use a 'data' variable name.

Will do.

>
>> +    };
>> +
>> +    ret = ioctl(fd, IOMMU_HWPT_INVALIDATE, &cache);
>> +    trace_iommufd_backend_invalidate_cache(fd, id, data_type, entry_len,
>> +                                           total_entries, cache.entry_num,
>> +                                           (uintptr_t)data_ptr,
>> +                                           ret ? errno : 0);
>> +    if (ret) {
>> +        *entry_num = cache.entry_num;
>> +        error_setg_errno(errp, errno, "IOMMU_HWPT_INVALIDATE failed:"
>> +                         " totally %d entries, processed %d entries",
>> +                         total_entries, cache.entry_num);
>> +    } else {
>> +        g_assert(total_entries == cache.entry_num);
>
>Killing the VMM because a kernel device ioctl failed is brute force.
>Can't we update the 'Error *' parameter instead to report that the
>invalidation is partial or something went wrong ?

Will do, like below:

--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -339,7 +339,10 @@ bool iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
                          " totally %d entries, processed %d entries",
                          total_entries, cache.entry_num);
     } else {
-        g_assert(total_entries == cache.entry_num);
+        error_setg_errno(errp, -EFAULT, "IOMMU_HWPT_INVALIDATE succeed with unprocessed entries:"
+                         " totally %d entries, processed %d entries",
+                         total_entries, cache.entry_num);
+        ret = -EFAULT;
     }

     return !ret;

>
>What kind of errors are we trying to catch ?

I'm taking it as a kernel bug when ret = 0 and total_entries != cache.entry_num

>
>Looking at the kernel iommufd_hwpt_invalidate() routine and
>intel_nested_cache_invalidate_user(), it doesn't seem possible to
>return a different number of cache entries. Are you anticipating
>other implementations (sMMU) ?

Yes, same for sMMU's arm_vsmmu_cache_invalidate() and selftest's
mock_viommu_cache_invalidate() and mock_domain_cache_invalidate_user().

I'm not sure if this should apply to all types of IOMMUs, uAPI doc doesn't talk about it.

@Liu, Yi L, @nicolinc@nvidia.com, @Jason Gunthorpe, should I treat ret = 0 and total_entries != cache.entry_num as a kernel bug or not?

Thanks
Zhenzhong

RE: [PATCH v1 2/6] vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v1 2/6] vfio/iommufd: Add properties and handlers to
>TYPE_HOST_IOMMU_DEVICE_IOMMUFD
>
>On 5/28/25 08:04, Zhenzhong Duan wrote:
>> Enhance HostIOMMUDeviceIOMMUFD object with 3 new members, specific
>> to the iommufd BE + 2 new class functions.
>>
>> IOMMUFD BE includes IOMMUFD handle, devid and hwpt_id. IOMMUFD handle
>> and devid are used to allocate/free ioas and hwpt. hwpt_id is used to
>> re-attach IOMMUFD backed device to its default VFIO sub-system created
>> hwpt, i.e., when vIOMMU is disabled by guest. These properties will be
>> initialized after attachment.
>>
>> 2 new class functions are [at|de]tach_hwpt(). They are used to
>> attach/detach hwpt. VFIO and VDPA can have different implementions,
>> so implementation will be in sub-class instead of HostIOMMUDeviceIOMMUFD,
>> e.g., in HostIOMMUDeviceIOMMUFDVFIO.
>>
>> Add two wrappers host_iommu_device_iommufd_[at|de]tach_hwpt to
>> wrap the two functions.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/system/iommufd.h | 50
>++++++++++++++++++++++++++++++++++++++++
>>   backends/iommufd.c       | 22 ++++++++++++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>> index 5399519626..a704575662 100644
>> --- a/include/system/iommufd.h
>> +++ b/include/system/iommufd.h
>> @@ -67,4 +67,54 @@ bool
>iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>>                                         Error **errp);
>>
>>   #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD
>TYPE_HOST_IOMMU_DEVICE "-iommufd"
>> +OBJECT_DECLARE_TYPE(HostIOMMUDeviceIOMMUFD,
>HostIOMMUDeviceIOMMUFDClass,
>> +                    HOST_IOMMU_DEVICE_IOMMUFD)
>> +
>> +/* Overload of the host IOMMU device for the iommufd backend */
>> +struct HostIOMMUDeviceIOMMUFD {
>> +    HostIOMMUDevice parent_obj;
>> +
>> +    IOMMUFDBackend *iommufd;
>> +    uint32_t devid;
>> +    uint32_t hwpt_id;
>> +};
>
>the addition of new attributes would be better placed in the next patch.

OK, will merge this patch with the next patch.

Thanks
Zhenzhong

>
>Anyhow,
>
>
>
>Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
>Thanks,
>
>C.
>
>
>
>> +struct HostIOMMUDeviceIOMMUFDClass {
>> +    HostIOMMUDeviceClass parent_class;
>> +
>> +    /**
>> +     * @attach_hwpt: attach host IOMMU device to IOMMUFD hardware page
>table.
>> +     * VFIO and VDPA device can have different implementation.
>> +     *
>> +     * Mandatory callback.
>> +     *
>> +     * @idev: host IOMMU device backed by IOMMUFD backend.
>> +     *
>> +     * @hwpt_id: ID of IOMMUFD hardware page table.
>> +     *
>> +     * @errp: pass an Error out when attachment fails.
>> +     *
>> +     * Returns: true on success, false on failure.
>> +     */
>> +    bool (*attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, uint32_t hwpt_id,
>> +                        Error **errp);
>> +    /**
>> +     * @detach_hwpt: detach host IOMMU device from IOMMUFD hardware
>page table.
>> +     * VFIO and VDPA device can have different implementation.
>> +     *
>> +     * Mandatory callback.
>> +     *
>> +     * @idev: host IOMMU device backed by IOMMUFD backend.
>> +     *
>> +     * @errp: pass an Error out when attachment fails.
>> +     *
>> +     * Returns: true on success, false on failure.
>> +     */
>> +    bool (*detach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, Error **errp);
>> +};
>> +
>> +bool
>host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>> +                                           uint32_t hwpt_id, Error **errp);
>> +bool
>host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>> +                                           Error **errp);
>>   #endif
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index c8788a6438..b114fb08e7 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -344,6 +344,26 @@ bool
>iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t id,
>>       return !ret;
>>   }
>>
>> +bool
>host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>> +                                           uint32_t hwpt_id, Error **errp)
>> +{
>> +    HostIOMMUDeviceIOMMUFDClass *idevc =
>> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
>> +
>> +    g_assert(idevc->attach_hwpt);
>> +    return idevc->attach_hwpt(idev, hwpt_id, errp);
>> +}
>> +
>> +bool
>host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>> +                                           Error **errp)
>> +{
>> +    HostIOMMUDeviceIOMMUFDClass *idevc =
>> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
>> +
>> +    g_assert(idevc->detach_hwpt);
>> +    return idevc->detach_hwpt(idev, errp);
>> +}
>> +
>>   static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error
>**errp)
>>   {
>>       HostIOMMUDeviceCaps *caps = &hiod->caps;
>> @@ -382,6 +402,8 @@ static const TypeInfo types[] = {
>>       }, {
>>           .name = TYPE_HOST_IOMMU_DEVICE_IOMMUFD,
>>           .parent = TYPE_HOST_IOMMU_DEVICE,
>> +        .instance_size = sizeof(HostIOMMUDeviceIOMMUFD),
>> +        .class_size = sizeof(HostIOMMUDeviceIOMMUFDClass),
>>           .class_init = hiod_iommufd_class_init,
>>           .abstract = true,
>>       }

RE: [PATCH v1 6/6] iommufd: Implement query of host VTD IOMMU's capability

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v1 6/6] iommufd: Implement query of host VTD IOMMU's
>capability
>
>On 5/28/25 08:04, Zhenzhong Duan wrote:
>> Implement query of HOST_IOMMU_DEVICE_CAP_[NESTING|FS1GP|ERRATA]
>for IOMMUFD
>> backed host VTD IOMMU device.
>>
>> Query on these capabilities is not supported for legacy backend because there
>> is no plan to support nesting with legacy backend backed host device.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu_internal.h     |  1 +
>>   include/system/host_iommu_device.h |  7 ++++++
>>   backends/iommufd.c                 | 39 ++++++++++++++++++++++++++++--
>>   3 files changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index e8b211e8b0..2cda744786 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -191,6 +191,7 @@
>>   #define VTD_ECAP_PT                 (1ULL << 6)
>>   #define VTD_ECAP_SC                 (1ULL << 7)
>>   #define VTD_ECAP_MHMV               (15ULL << 20)
>> +#define VTD_ECAP_NEST               (1ULL << 26)
>>   #define VTD_ECAP_SRS                (1ULL << 31)
>>   #define VTD_ECAP_PASID              (1ULL << 40)
>>   #define VTD_ECAP_SMTS               (1ULL << 43)
>> diff --git a/include/system/host_iommu_device.h
>b/include/system/host_iommu_device.h
>> index 10fccc10be..c2770cb469 100644
>> --- a/include/system/host_iommu_device.h
>> +++ b/include/system/host_iommu_device.h
>> @@ -29,6 +29,10 @@ typedef union VendorCaps {
>>    *
>>    * @hw_caps: host platform IOMMU capabilities (e.g. on IOMMUFD this
>represents
>>    *           the @out_capabilities value returned from IOMMU_GET_HW_INFO
>ioctl)
>> + *
>> + * @vendor_caps: host platform IOMMU vendor specific capabilities (e.g. on
>> + *               IOMMUFD this represents raw vendor data from data_uptr
>> + *               buffer returned from IOMMU_GET_HW_INFO ioctl)
>>    */
>>   typedef struct HostIOMMUDeviceCaps {
>>       uint32_t type;
>> @@ -116,6 +120,9 @@ struct HostIOMMUDeviceClass {
>>    */
>>   #define HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE        0
>>   #define HOST_IOMMU_DEVICE_CAP_AW_BITS           1
>> +#define HOST_IOMMU_DEVICE_CAP_NESTING           2
>> +#define HOST_IOMMU_DEVICE_CAP_FS1GP             3
>> +#define HOST_IOMMU_DEVICE_CAP_ERRATA            4
>>
>>   #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
>>   #endif
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index b114fb08e7..63209659f3 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -21,6 +21,7 @@
>>   #include "hw/vfio/vfio-device.h"
>>   #include <sys/ioctl.h>
>>   #include <linux/iommufd.h>
>> +#include "hw/i386/intel_iommu_internal.h"
>>
>>   static void iommufd_backend_init(Object *obj)
>>   {
>> @@ -364,6 +365,41 @@ bool
>host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>>       return idevc->detach_hwpt(idev, errp);
>>   }
>>
>> +static int hiod_iommufd_get_vtd_cap(HostIOMMUDevice *hiod, int cap,
>> +                                    Error **errp)
>> +{
>> +    struct iommu_hw_info_vtd *caps = &hiod->caps.vendor_caps.vtd;
>> +
>> +    switch (cap) {
>> +    case HOST_IOMMU_DEVICE_CAP_NESTING:
>> +        return !!(caps->ecap_reg & VTD_ECAP_NEST);
>> +    case HOST_IOMMU_DEVICE_CAP_FS1GP:
>> +        return !!(caps->cap_reg & VTD_CAP_FS1GP);
>> +    case HOST_IOMMU_DEVICE_CAP_ERRATA:
>> +        return caps->flags & IOMMU_HW_INFO_VTD_ERRATA_772415_SPR17;
>> +    default:
>> +        error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>> +        return -EINVAL;
>> +    }
>> +}
>
>
>This is intel specific. Why not handle these capabilities directly from
>vtd_check_hiod() under hw/i386/intel_iommu.c ?

Do you mean checking hiod->caps.vendor_caps.vtd.ecap/cap_reg directly
in intel_iommu.c and drop vendor cap checking by .get_cap()? That's fine
if we don't have plan to support virtio-iommu querying vendor caps in a
standard way.

Thanks
Zhenzhong

Re: [PATCH v1 1/6] backends/iommufd: Add a helper to invalidate user-managed HWPT

[Nicolin Chen]

On Thu, May 29, 2025 at 06:46:20AM +0000, Duan, Zhenzhong wrote:
> >Looking at the kernel iommufd_hwpt_invalidate() routine and
> >intel_nested_cache_invalidate_user(), it doesn't seem possible to
> >return a different number of cache entries. Are you anticipating
> >other implementations (sMMU) ?
> 
> Yes, same for sMMU's arm_vsmmu_cache_invalidate() and selftest's
> mock_viommu_cache_invalidate() and mock_domain_cache_invalidate_user().
> 
> I'm not sure if this should apply to all types of IOMMUs, uAPI doc doesn't talk about it.

It should. The uAPI defines that at entry_num:
 * struct iommu_hwpt_invalidate - ioctl(IOMMU_HWPT_INVALIDATE)
 ...
 * @entry_num: Input the number of cache invalidation requests in the array.
 *             Output the number of requests successfully handled by kernel.

This applies to either ret != 0 case too.

> @Liu, Yi L, @nicolinc@nvidia.com, @Jason Gunthorpe, should I treat ret = 0
> and total_entries != cache.entry_num as a kernel bug or not?

Selftest has that coverage, so it would be a kernel bug that will
unlikely occur. That being said, it doesn't hurt to do that IMHO.

Thanks
Nicolin