When vIOMMU is configured x-flts=on in scalable mode, stage-1 page table
is passed to host to construct nested page table. We need to check
compatibility of some critical IOMMU capabilities between vIOMMU and
host IOMMU to ensure guest stage-1 page table could be used by host.

For instance, vIOMMU supports stage-1 1GB huge page mapping, but host
does not, then this IOMMUFD backed device should be failed.

Declare an enum type host_iommu_device_iommu_hw_info_type aliased to
iommu_hw_info_type which come from iommufd header file. This can avoid
build failure on windows which doesn't support iommufd.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/system/host_iommu_device.h | 13 ++++++++++++
 hw/i386/intel_iommu.c              | 34 ++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 250600fc1d..aa3885d7ee 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -133,5 +133,18 @@ struct HostIOMMUDeviceClass {
 #define HOST_IOMMU_DEVICE_CAP_FS1GP             3
 #define HOST_IOMMU_DEVICE_CAP_ERRATA            4
 
+/**
+ * enum host_iommu_device_iommu_hw_info_type - IOMMU Hardware Info Types
+ * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE: Used by the drivers that do not
+ *                                             report hardware info
+ * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type
+ *
+ * This is alias to enum iommu_hw_info_type but for general purpose.
+ */
+enum host_iommu_device_iommu_hw_info_type {
+    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE,
+    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD,
+};
+
 #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
 #endif
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 7709f55be5..9de60e607d 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -39,6 +39,7 @@
 #include "kvm/kvm_i386.h"
 #include "migration/vmstate.h"
 #include "trace.h"
+#include "system/iommufd.h"
 
 /* context entry operations */
 #define VTD_CE_GET_RID2PASID(ce) \
@@ -4346,6 +4347,39 @@ static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
         return true;
     }
 
+    /* Remaining checks are all stage-1 translation specific */
+    if (!object_dynamic_cast(OBJECT(hiod), TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
+        error_setg(errp, "Need IOMMUFD backend when x-flts=on");
+        return false;
+    }
+
+    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE, errp);
+    if (ret < 0) {
+        return false;
+    }
+    if (ret != HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD) {
+        error_setg(errp, "Incompatible host platform IOMMU type %d", ret);
+        return false;
+    }
+
+    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_NESTING, errp);
+    if (ret < 0) {
+        return false;
+    }
+    if (ret != 1) {
+        error_setg(errp, "Host IOMMU doesn't support nested translation");
+        return false;
+    }
+
+    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_FS1GP, errp);
+    if (ret < 0) {
+        return false;
+    }
+    if (s->fs1gp && ret != 1) {
+        error_setg(errp, "Stage-1 1GB huge page is unsupported by host IOMMU");
+        return false;
+    }
+
     error_setg(errp, "host device is uncompatible with stage-1 translation");
     return false;
 }
-- 
2.34.1

Hi Zhenzhong,


On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
> When vIOMMU is configured x-flts=on in scalable mode, stage-1 page table
> is passed to host to construct nested page table. We need to check
> compatibility of some critical IOMMU capabilities between vIOMMU and
> host IOMMU to ensure guest stage-1 page table could be used by host.
>
> For instance, vIOMMU supports stage-1 1GB huge page mapping, but host
> does not, then this IOMMUFD backed device should be failed.
is this 1GB huge page mapping a requiring for SIOV?
>
> Declare an enum type host_iommu_device_iommu_hw_info_type aliased to
> iommu_hw_info_type which come from iommufd header file. This can avoid
s/come/comes
> build failure on windows which doesn't support iommufd.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/system/host_iommu_device.h | 13 ++++++++++++
>  hw/i386/intel_iommu.c              | 34 ++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
>
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 250600fc1d..aa3885d7ee 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -133,5 +133,18 @@ struct HostIOMMUDeviceClass {
>  #define HOST_IOMMU_DEVICE_CAP_FS1GP             3
>  #define HOST_IOMMU_DEVICE_CAP_ERRATA            4
>  
> +/**
> + * enum host_iommu_device_iommu_hw_info_type - IOMMU Hardware Info Types
> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE: Used by the drivers that do not
> + *                                             report hardware info
> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type
> + *
> + * This is alias to enum iommu_hw_info_type but for general purpose.
> + */
> +enum host_iommu_device_iommu_hw_info_type {
> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE,
> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD,
> +};
> +
>  #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
>  #endif
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 7709f55be5..9de60e607d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -39,6 +39,7 @@
>  #include "kvm/kvm_i386.h"
>  #include "migration/vmstate.h"
>  #include "trace.h"
> +#include "system/iommufd.h"
>  
>  /* context entry operations */
>  #define VTD_CE_GET_RID2PASID(ce) \
> @@ -4346,6 +4347,39 @@ static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
>          return true;
>      }
>  
> +    /* Remaining checks are all stage-1 translation specific */
> +    if (!object_dynamic_cast(OBJECT(hiod), TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
> +        error_setg(errp, "Need IOMMUFD backend when x-flts=on");
> +        return false;
> +    }
> +
> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE, errp);
> +    if (ret < 0) {
> +        return false;
Can't you simply rely on the check below?
> +    }
> +    if (ret != HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD) {
> +        error_setg(errp, "Incompatible host platform IOMMU type %d", ret);
> +        return false;
> +    }
> +
> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_NESTING, errp);
> +    if (ret < 0) {
> +        return false;
> +    }
same heere
> +    if (ret != 1) {
> +        error_setg(errp, "Host IOMMU doesn't support nested translation");
> +        return false;
> +    }
> +
> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_FS1GP, errp);
> +    if (ret < 0) {
> +        return false;
> +    }
> +    if (s->fs1gp && ret != 1) {
looking in the vtd spec I don't find FS1GP. Is it the same as FL1GP?
Maybe I am not looking the correct spec though. Why do you need to check
both ret and fs1gp
Even why do you need a member to store the cap? Looks FL1GP can only
take 0 or 1 value?
> +        error_setg(errp, "Stage-1 1GB huge page is unsupported by host IOMMU");
> +        return false;
> +    }
> +
>      error_setg(errp, "host device is uncompatible with stage-1 translation");
>      return false;
>  }
Eric

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv2 11/20] intel_iommu: Check for compatibility with
>IOMMUFD backed device when x-flts=on
>
>Hi Zhenzhong,
>
>
>On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
>> When vIOMMU is configured x-flts=on in scalable mode, stage-1 page table
>> is passed to host to construct nested page table. We need to check
>> compatibility of some critical IOMMU capabilities between vIOMMU and
>> host IOMMU to ensure guest stage-1 page table could be used by host.
>>
>> For instance, vIOMMU supports stage-1 1GB huge page mapping, but host
>> does not, then this IOMMUFD backed device should be failed.
>is this 1GB huge page mapping a requiring for SIOV?

No, but if guest has configured that support, but host doesn't support it, VFIO
device should fail the plug.

>>
>> Declare an enum type host_iommu_device_iommu_hw_info_type aliased to
>> iommu_hw_info_type which come from iommufd header file. This can avoid
>s/come/comes

Will do.

>> build failure on windows which doesn't support iommufd.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  include/system/host_iommu_device.h | 13 ++++++++++++
>>  hw/i386/intel_iommu.c              | 34 ++++++++++++++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/include/system/host_iommu_device.h
>b/include/system/host_iommu_device.h
>> index 250600fc1d..aa3885d7ee 100644
>> --- a/include/system/host_iommu_device.h
>> +++ b/include/system/host_iommu_device.h
>> @@ -133,5 +133,18 @@ struct HostIOMMUDeviceClass {
>>  #define HOST_IOMMU_DEVICE_CAP_FS1GP             3
>>  #define HOST_IOMMU_DEVICE_CAP_ERRATA            4
>>
>> +/**
>> + * enum host_iommu_device_iommu_hw_info_type - IOMMU Hardware Info
>Types
>> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE: Used by the
>drivers that do not
>> + *                                             report hardware info
>> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d
>iommu info type
>> + *
>> + * This is alias to enum iommu_hw_info_type but for general purpose.
>> + */
>> +enum host_iommu_device_iommu_hw_info_type {
>> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE,
>> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD,
>> +};
>> +
>>  #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
>>  #endif
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 7709f55be5..9de60e607d 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -39,6 +39,7 @@
>>  #include "kvm/kvm_i386.h"
>>  #include "migration/vmstate.h"
>>  #include "trace.h"
>> +#include "system/iommufd.h"
>>
>>  /* context entry operations */
>>  #define VTD_CE_GET_RID2PASID(ce) \
>> @@ -4346,6 +4347,39 @@ static bool vtd_check_hiod(IntelIOMMUState *s,
>HostIOMMUDevice *hiod,
>>          return true;
>>      }
>>
>> +    /* Remaining checks are all stage-1 translation specific */
>> +    if (!object_dynamic_cast(OBJECT(hiod),
>TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>> +        error_setg(errp, "Need IOMMUFD backend when x-flts=on");
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE,
>errp);
>> +    if (ret < 0) {
>> +        return false;
>Can't you simply rely on the check below?

I think not, below code will overwrite errp.

>> +    }
>> +    if (ret != HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD) {
>> +        error_setg(errp, "Incompatible host platform IOMMU type %d", ret);
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_NESTING, errp);
>> +    if (ret < 0) {
>> +        return false;
>> +    }
>same heere
>> +    if (ret != 1) {
>> +        error_setg(errp, "Host IOMMU doesn't support nested translation");
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_FS1GP, errp);
>> +    if (ret < 0) {
>> +        return false;
>> +    }
>> +    if (s->fs1gp && ret != 1) {
>looking in the vtd spec I don't find FS1GP. Is it the same as FL1GP?
Yes.

>Maybe I am not looking the correct spec though. Why do you need to check
>both ret and fs1gp

Ret < 0 means error happen, e.g., vIOMMU checks an unrecognized cap.
0 or 1 means no error and unsupported vs. supported for FS1GP.

>Even why do you need a member to store the cap? Looks FL1GP can only
>take 0 or 1 value?

You means s->fs1gp? That's user configuration for vIOMMU.
We need to check user's config of FS1GP with host's FS1GP to ensure compatibility.

Yes, Fs1GP takes only 0 or 1, aw_bits can have other values.

Thanks
Zhenzhong

>> +        error_setg(errp, "Stage-1 1GB huge page is unsupported by host IOMMU");
>> +        return false;
>> +    }
>> +
>>      error_setg(errp, "host device is uncompatible with stage-1 translation");
>>      return false;
>>  }
>Eric

On 2/21/25 1:49 PM, Eric Auger wrote:
> Hi Zhenzhong,
> 
> 
> On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
>> When vIOMMU is configured x-flts=on in scalable mode, stage-1 page table
>> is passed to host to construct nested page table. We need to check
>> compatibility of some critical IOMMU capabilities between vIOMMU and
>> host IOMMU to ensure guest stage-1 page table could be used by host.
>>
>> For instance, vIOMMU supports stage-1 1GB huge page mapping, but host
>> does not, then this IOMMUFD backed device should be failed.
> is this 1GB huge page mapping a requiring for SIOV?
>>
>> Declare an enum type host_iommu_device_iommu_hw_info_type aliased to
>> iommu_hw_info_type which come from iommufd header file. This can avoid
> s/come/comes
>> build failure on windows which doesn't support iommufd.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  include/system/host_iommu_device.h | 13 ++++++++++++
>>  hw/i386/intel_iommu.c              | 34 ++++++++++++++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
>> index 250600fc1d..aa3885d7ee 100644
>> --- a/include/system/host_iommu_device.h
>> +++ b/include/system/host_iommu_device.h
>> @@ -133,5 +133,18 @@ struct HostIOMMUDeviceClass {
>>  #define HOST_IOMMU_DEVICE_CAP_FS1GP             3
>>  #define HOST_IOMMU_DEVICE_CAP_ERRATA            4
>>  
>> +/**
>> + * enum host_iommu_device_iommu_hw_info_type - IOMMU Hardware Info Types
>> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE: Used by the drivers that do not
>> + *                                             report hardware info
>> + * @HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type
>> + *
>> + * This is alias to enum iommu_hw_info_type but for general purpose.
>> + */
>> +enum host_iommu_device_iommu_hw_info_type {
>> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_NONE,
>> +    HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD,
>> +};
>> +
>>  #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX       64
>>  #endif
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 7709f55be5..9de60e607d 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -39,6 +39,7 @@
>>  #include "kvm/kvm_i386.h"
>>  #include "migration/vmstate.h"
>>  #include "trace.h"
>> +#include "system/iommufd.h"
>>  
>>  /* context entry operations */
>>  #define VTD_CE_GET_RID2PASID(ce) \
>> @@ -4346,6 +4347,39 @@ static bool vtd_check_hiod(IntelIOMMUState *s, HostIOMMUDevice *hiod,
>>          return true;
>>      }
>>  
>> +    /* Remaining checks are all stage-1 translation specific */
>> +    if (!object_dynamic_cast(OBJECT(hiod), TYPE_HOST_IOMMU_DEVICE_IOMMUFD)) {
>> +        error_setg(errp, "Need IOMMUFD backend when x-flts=on");
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE, errp);
>> +    if (ret < 0) {
>> +        return false;
> Can't you simply rely on the check below?
>> +    }
>> +    if (ret != HOST_IOMMU_DEVICE_IOMMU_HW_INFO_TYPE_INTEL_VTD) {
>> +        error_setg(errp, "Incompatible host platform IOMMU type %d", ret);
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_NESTING, errp);
>> +    if (ret < 0) {
>> +        return false;
>> +    }
> same heere
>> +    if (ret != 1) {
>> +        error_setg(errp, "Host IOMMU doesn't support nested translation");
>> +        return false;
>> +    }
>> +
>> +    ret = hiodc->get_cap(hiod, HOST_IOMMU_DEVICE_CAP_FS1GP, errp);
>> +    if (ret < 0) {
>> +        return false;
>> +    }
>> +    if (s->fs1gp && ret != 1) {
> looking in the vtd spec I don't find FS1GP. Is it the same as FL1GP?
I am now looking at spec rev from june 22 and it seems it has been
renamed. So please ignore this comment

Eric
> Maybe I am not looking the correct spec though. Why do you need to check
> both ret and fs1gp
> Even why do you need a member to store the cap? Looks FL1GP can only
> take 0 or 1 value?
>> +        error_setg(errp, "Stage-1 1GB huge page is unsupported by host IOMMU");
>> +        return false;
>> +    }
>> +
>>      error_setg(errp, "host device is uncompatible with stage-1 translation");
>>      return false;
>>  }
> Eric