RE: [PATCH v1 6/6] iommufd: Implement query of host VTD IOMMU's capability
>-Original Message-
>From: Cédric Le Goater
>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
>> ---
>> 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_TYPE0
>> #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_ERRATA4
>>
>> #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
>> #include
>> +#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 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
---
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_TYPE0
#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_ERRATA4
#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
#include
+#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);
}
}
