Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
related to scalable mode translation, thus there are multiple combinations.
This vIOMMU implementation wants to simplify it with a new property "x-fls".
When enabled in scalable mode, first stage translation also known as scalable
modern mode is supported. When enabled in legacy mode, throw out error.
With scalable modern mode exposed to user, also accurate the pasid entry
check in vtd_pe_type_check().
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/i386/intel_iommu_internal.h | 2 ++
hw/i386/intel_iommu.c | 28 +++++++++++++++++++---------
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 2702edd27f..f13576d334 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -195,6 +195,7 @@
#define VTD_ECAP_PASID (1ULL << 40)
#define VTD_ECAP_SMTS (1ULL << 43)
#define VTD_ECAP_SLTS (1ULL << 46)
+#define VTD_ECAP_FLTS (1ULL << 47)
/* CAP_REG */
/* (offset >> 4) << 24 */
@@ -211,6 +212,7 @@
#define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
#define VTD_CAP_DRAIN_WRITE (1ULL << 54)
#define VTD_CAP_DRAIN_READ (1ULL << 55)
+#define VTD_CAP_FS1GP (1ULL << 56)
#define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ | VTD_CAP_DRAIN_WRITE)
#define VTD_CAP_CM (1ULL << 7)
#define VTD_PASID_ID_SHIFT 20
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 068a08f522..14578655e1 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -803,16 +803,18 @@ static inline bool vtd_is_fl_level_supported(IntelIOMMUState *s, uint32_t level)
}
/* Return true if check passed, otherwise false */
-static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
- VTDPASIDEntry *pe)
+static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
{
switch (VTD_PE_GET_TYPE(pe)) {
- case VTD_SM_PASID_ENTRY_SLT:
- return true;
- case VTD_SM_PASID_ENTRY_PT:
- return x86_iommu->pt_supported;
case VTD_SM_PASID_ENTRY_FLT:
+ return !!(s->ecap & VTD_ECAP_FLTS);
+ case VTD_SM_PASID_ENTRY_SLT:
+ return !!(s->ecap & VTD_ECAP_SLTS);
case VTD_SM_PASID_ENTRY_NESTED:
+ /* Not support NESTED page table type yet */
+ return false;
+ case VTD_SM_PASID_ENTRY_PT:
+ return !!(s->ecap & VTD_ECAP_PT);
default:
/* Unknown type */
return false;
@@ -861,7 +863,6 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
uint8_t pgtt;
uint32_t index;
dma_addr_t entry_size;
- X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
index = VTD_PASID_TABLE_INDEX(pasid);
entry_size = VTD_PASID_ENTRY_SIZE;
@@ -875,7 +876,7 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
}
/* Do translation type check */
- if (!vtd_pe_type_check(x86_iommu, pe)) {
+ if (!vtd_pe_type_check(s, pe)) {
return -VTD_FR_PASID_TABLE_ENTRY_INV;
}
@@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
VTD_HOST_AW_AUTO),
DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
+ DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
@@ -4509,7 +4511,10 @@ static void vtd_cap_init(IntelIOMMUState *s)
}
/* TODO: read cap/ecap from host to decide which cap to be exposed. */
- if (s->scalable_mode) {
+ if (s->scalable_modern) {
+ s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
+ s->cap |= VTD_CAP_FS1GP;
+ } else if (s->scalable_mode) {
s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
}
@@ -4683,6 +4688,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
}
}
+ if (!s->scalable_mode && s->scalable_modern) {
+ error_setg(errp, "Legacy mode: not support x-fls=on");
+ return false;
+ }
+
if (s->aw_bits == VTD_HOST_AW_AUTO) {
if (s->scalable_modern) {
s->aw_bits = VTD_HOST_AW_48BIT;
--
2.34.1On 2024/9/30 17:26, Zhenzhong Duan wrote:
> Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
> related to scalable mode translation, thus there are multiple combinations.
>
> This vIOMMU implementation wants to simplify it with a new property "x-fls".
> When enabled in scalable mode, first stage translation also known as scalable
> modern mode is supported. When enabled in legacy mode, throw out error.
>
> With scalable modern mode exposed to user, also accurate the pasid entry
> check in vtd_pe_type_check().
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Maybe a Suggested-by tag can help to understand where this idea comes. :)
> ---
> hw/i386/intel_iommu_internal.h | 2 ++
> hw/i386/intel_iommu.c | 28 +++++++++++++++++++---------
> 2 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 2702edd27f..f13576d334 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -195,6 +195,7 @@
> #define VTD_ECAP_PASID (1ULL << 40)
> #define VTD_ECAP_SMTS (1ULL << 43)
> #define VTD_ECAP_SLTS (1ULL << 46)
> +#define VTD_ECAP_FLTS (1ULL << 47)
>
> /* CAP_REG */
> /* (offset >> 4) << 24 */
> @@ -211,6 +212,7 @@
> #define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
> #define VTD_CAP_DRAIN_WRITE (1ULL << 54)
> #define VTD_CAP_DRAIN_READ (1ULL << 55)
> +#define VTD_CAP_FS1GP (1ULL << 56)
> #define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ | VTD_CAP_DRAIN_WRITE)
> #define VTD_CAP_CM (1ULL << 7)
> #define VTD_PASID_ID_SHIFT 20
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 068a08f522..14578655e1 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -803,16 +803,18 @@ static inline bool vtd_is_fl_level_supported(IntelIOMMUState *s, uint32_t level)
> }
>
> /* Return true if check passed, otherwise false */
> -static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
> - VTDPASIDEntry *pe)
> +static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
> {
> switch (VTD_PE_GET_TYPE(pe)) {
> - case VTD_SM_PASID_ENTRY_SLT:
> - return true;
> - case VTD_SM_PASID_ENTRY_PT:
> - return x86_iommu->pt_supported;
> case VTD_SM_PASID_ENTRY_FLT:
> + return !!(s->ecap & VTD_ECAP_FLTS);
> + case VTD_SM_PASID_ENTRY_SLT:
> + return !!(s->ecap & VTD_ECAP_SLTS);
> case VTD_SM_PASID_ENTRY_NESTED:
> + /* Not support NESTED page table type yet */
> + return false;
> + case VTD_SM_PASID_ENTRY_PT:
> + return !!(s->ecap & VTD_ECAP_PT);
> default:
> /* Unknown type */
> return false;
> @@ -861,7 +863,6 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
> uint8_t pgtt;
> uint32_t index;
> dma_addr_t entry_size;
> - X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>
> index = VTD_PASID_TABLE_INDEX(pasid);
> entry_size = VTD_PASID_ENTRY_SIZE;
> @@ -875,7 +876,7 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
> }
>
> /* Do translation type check */
> - if (!vtd_pe_type_check(x86_iommu, pe)) {
> + if (!vtd_pe_type_check(s, pe)) {
> return -VTD_FR_PASID_TABLE_ENTRY_INV;
> }
>
> @@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
> VTD_HOST_AW_AUTO),
> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
a question: is there any requirement on the layout of this array? Should
new fields added in the end?
> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> @@ -4509,7 +4511,10 @@ static void vtd_cap_init(IntelIOMMUState *s)
> }
>
> /* TODO: read cap/ecap from host to decide which cap to be exposed. */
> - if (s->scalable_mode) {
> + if (s->scalable_modern) {
> + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
> + s->cap |= VTD_CAP_FS1GP;
> + } else if (s->scalable_mode) {
> s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> }
>
> @@ -4683,6 +4688,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
> }
> }
>
> + if (!s->scalable_mode && s->scalable_modern) {
> + error_setg(errp, "Legacy mode: not support x-fls=on");
> + return false;
> + }
> +
> if (s->aw_bits == VTD_HOST_AW_AUTO) {
> if (s->scalable_modern) {
> s->aw_bits = VTD_HOST_AW_48BIT;
--
Regards,
Yi Liu
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Sent: Monday, November 4, 2024 12:25 PM
>Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>scalable modern mode
>
>On 2024/9/30 17:26, Zhenzhong Duan wrote:
>> Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
>> related to scalable mode translation, thus there are multiple combinations.
>>
>> This vIOMMU implementation wants to simplify it with a new property "x-fls".
>> When enabled in scalable mode, first stage translation also known as scalable
>> modern mode is supported. When enabled in legacy mode, throw out error.
>>
>> With scalable modern mode exposed to user, also accurate the pasid entry
>> check in vtd_pe_type_check().
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>Maybe a Suggested-by tag can help to understand where this idea comes. :)
Will add:
Suggested-by: Jason Wang <jasowang@redhat.com>
>
>> ---
>> hw/i386/intel_iommu_internal.h | 2 ++
>> hw/i386/intel_iommu.c | 28 +++++++++++++++++++---------
>> 2 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index 2702edd27f..f13576d334 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -195,6 +195,7 @@
>> #define VTD_ECAP_PASID (1ULL << 40)
>> #define VTD_ECAP_SMTS (1ULL << 43)
>> #define VTD_ECAP_SLTS (1ULL << 46)
>> +#define VTD_ECAP_FLTS (1ULL << 47)
>>
>> /* CAP_REG */
>> /* (offset >> 4) << 24 */
>> @@ -211,6 +212,7 @@
>> #define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
>> #define VTD_CAP_DRAIN_WRITE (1ULL << 54)
>> #define VTD_CAP_DRAIN_READ (1ULL << 55)
>> +#define VTD_CAP_FS1GP (1ULL << 56)
>> #define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ |
>VTD_CAP_DRAIN_WRITE)
>> #define VTD_CAP_CM (1ULL << 7)
>> #define VTD_PASID_ID_SHIFT 20
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 068a08f522..14578655e1 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -803,16 +803,18 @@ static inline bool
>vtd_is_fl_level_supported(IntelIOMMUState *s, uint32_t level)
>> }
>>
>> /* Return true if check passed, otherwise false */
>> -static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
>> - VTDPASIDEntry *pe)
>> +static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>> {
>> switch (VTD_PE_GET_TYPE(pe)) {
>> - case VTD_SM_PASID_ENTRY_SLT:
>> - return true;
>> - case VTD_SM_PASID_ENTRY_PT:
>> - return x86_iommu->pt_supported;
>> case VTD_SM_PASID_ENTRY_FLT:
>> + return !!(s->ecap & VTD_ECAP_FLTS);
>> + case VTD_SM_PASID_ENTRY_SLT:
>> + return !!(s->ecap & VTD_ECAP_SLTS);
>> case VTD_SM_PASID_ENTRY_NESTED:
>> + /* Not support NESTED page table type yet */
>> + return false;
>> + case VTD_SM_PASID_ENTRY_PT:
>> + return !!(s->ecap & VTD_ECAP_PT);
>> default:
>> /* Unknown type */
>> return false;
>> @@ -861,7 +863,6 @@ static int
>vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>> uint8_t pgtt;
>> uint32_t index;
>> dma_addr_t entry_size;
>> - X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>>
>> index = VTD_PASID_TABLE_INDEX(pasid);
>> entry_size = VTD_PASID_ENTRY_SIZE;
>> @@ -875,7 +876,7 @@ static int
>vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>> }
>>
>> /* Do translation type check */
>> - if (!vtd_pe_type_check(x86_iommu, pe)) {
>> + if (!vtd_pe_type_check(s, pe)) {
>> return -VTD_FR_PASID_TABLE_ENTRY_INV;
>> }
>>
>> @@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
>> VTD_HOST_AW_AUTO),
>> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>FALSE),
>> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode,
>FALSE),
>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control,
>false),
>
>a question: is there any requirement on the layout of this array? Should
>new fields added in the end?
Looked over the history, seems we didn't have an explicit rule in vtd_properties.
I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of scalable mode.
Let me know if you have preference to add in the end.
Thanks
Zhenzhong
>
>> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>> @@ -4509,7 +4511,10 @@ static void vtd_cap_init(IntelIOMMUState *s)
>> }
>>
>> /* TODO: read cap/ecap from host to decide which cap to be exposed. */
>> - if (s->scalable_mode) {
>> + if (s->scalable_modern) {
>> + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
>> + s->cap |= VTD_CAP_FS1GP;
>> + } else if (s->scalable_mode) {
>> s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
>> }
>>
>> @@ -4683,6 +4688,11 @@ static bool vtd_decide_config(IntelIOMMUState *s,
>Error **errp)
>> }
>> }
>>
>> + if (!s->scalable_mode && s->scalable_modern) {
>> + error_setg(errp, "Legacy mode: not support x-fls=on");
>> + return false;
>> + }
>> +
>> if (s->aw_bits == VTD_HOST_AW_AUTO) {
>> if (s->scalable_modern) {
>> s->aw_bits = VTD_HOST_AW_48BIT;
>
>--
>Regards,
>Yi Liu
On 2024/11/4 14:25, Duan, Zhenzhong wrote:
>
>
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Monday, November 4, 2024 12:25 PM
>> Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>> scalable modern mode
>>
>> On 2024/9/30 17:26, Zhenzhong Duan wrote:
>>> Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
>>> related to scalable mode translation, thus there are multiple combinations.
>>>
>>> This vIOMMU implementation wants to simplify it with a new property "x-fls".
>>> When enabled in scalable mode, first stage translation also known as scalable
>>> modern mode is supported. When enabled in legacy mode, throw out error.
>>>
>>> With scalable modern mode exposed to user, also accurate the pasid entry
>>> check in vtd_pe_type_check().
>>>
>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>
>> Maybe a Suggested-by tag can help to understand where this idea comes. :)
>
> Will add:
> Suggested-by: Jason Wang <jasowang@redhat.com>
>
>>
>>> ---
>>> hw/i386/intel_iommu_internal.h | 2 ++
>>> hw/i386/intel_iommu.c | 28 +++++++++++++++++++---------
>>> 2 files changed, 21 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>>> index 2702edd27f..f13576d334 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -195,6 +195,7 @@
>>> #define VTD_ECAP_PASID (1ULL << 40)
>>> #define VTD_ECAP_SMTS (1ULL << 43)
>>> #define VTD_ECAP_SLTS (1ULL << 46)
>>> +#define VTD_ECAP_FLTS (1ULL << 47)
>>>
>>> /* CAP_REG */
>>> /* (offset >> 4) << 24 */
>>> @@ -211,6 +212,7 @@
>>> #define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
>>> #define VTD_CAP_DRAIN_WRITE (1ULL << 54)
>>> #define VTD_CAP_DRAIN_READ (1ULL << 55)
>>> +#define VTD_CAP_FS1GP (1ULL << 56)
>>> #define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ |
>> VTD_CAP_DRAIN_WRITE)
>>> #define VTD_CAP_CM (1ULL << 7)
>>> #define VTD_PASID_ID_SHIFT 20
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 068a08f522..14578655e1 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -803,16 +803,18 @@ static inline bool
>> vtd_is_fl_level_supported(IntelIOMMUState *s, uint32_t level)
>>> }
>>>
>>> /* Return true if check passed, otherwise false */
>>> -static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
>>> - VTDPASIDEntry *pe)
>>> +static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>> {
>>> switch (VTD_PE_GET_TYPE(pe)) {
>>> - case VTD_SM_PASID_ENTRY_SLT:
>>> - return true;
>>> - case VTD_SM_PASID_ENTRY_PT:
>>> - return x86_iommu->pt_supported;
>>> case VTD_SM_PASID_ENTRY_FLT:
>>> + return !!(s->ecap & VTD_ECAP_FLTS);
>>> + case VTD_SM_PASID_ENTRY_SLT:
>>> + return !!(s->ecap & VTD_ECAP_SLTS);
>>> case VTD_SM_PASID_ENTRY_NESTED:
>>> + /* Not support NESTED page table type yet */
>>> + return false;
>>> + case VTD_SM_PASID_ENTRY_PT:
>>> + return !!(s->ecap & VTD_ECAP_PT);
>>> default:
>>> /* Unknown type */
>>> return false;
>>> @@ -861,7 +863,6 @@ static int
>> vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>>> uint8_t pgtt;
>>> uint32_t index;
>>> dma_addr_t entry_size;
>>> - X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>>>
>>> index = VTD_PASID_TABLE_INDEX(pasid);
>>> entry_size = VTD_PASID_ENTRY_SIZE;
>>> @@ -875,7 +876,7 @@ static int
>> vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>>> }
>>>
>>> /* Do translation type check */
>>> - if (!vtd_pe_type_check(x86_iommu, pe)) {
>>> + if (!vtd_pe_type_check(s, pe)) {
>>> return -VTD_FR_PASID_TABLE_ENTRY_INV;
>>> }
>>>
>>> @@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
>>> VTD_HOST_AW_AUTO),
>>> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>> FALSE),
>>> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode,
>> FALSE),
>>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
>>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control,
>> false),
>>
>> a question: is there any requirement on the layout of this array? Should
>> new fields added in the end?
>
> Looked over the history, seems we didn't have an explicit rule in vtd_properties.
> I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of scalable mode.
> Let me know if you have preference to add in the end.
I don't have a preference for now as long as it does not break any
functionality. BTW. Will x-flt or x-flts better?
>
>>
>>> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>>> @@ -4509,7 +4511,10 @@ static void vtd_cap_init(IntelIOMMUState *s)
>>> }
>>>
>>> /* TODO: read cap/ecap from host to decide which cap to be exposed. */
>>> - if (s->scalable_mode) {
>>> + if (s->scalable_modern) {
>>> + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_FLTS;
>>> + s->cap |= VTD_CAP_FS1GP;
>>> + } else if (s->scalable_mode) {
>>> s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
>>> }
>>>
>>> @@ -4683,6 +4688,11 @@ static bool vtd_decide_config(IntelIOMMUState *s,
>> Error **errp)
>>> }
>>> }
>>>
>>> + if (!s->scalable_mode && s->scalable_modern) {
>>> + error_setg(errp, "Legacy mode: not support x-fls=on");
>>> + return false;
>>> + }
>>> +
>>> if (s->aw_bits == VTD_HOST_AW_AUTO) {
>>> if (s->scalable_modern) {
>>> s->aw_bits = VTD_HOST_AW_48BIT;
>>
>> --
>> Regards,
>> Yi Liu
--
Regards,
Yi Liu
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Sent: Monday, November 4, 2024 3:23 PM
>Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>scalable modern mode
>
>On 2024/11/4 14:25, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>> Sent: Monday, November 4, 2024 12:25 PM
>>> Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>>> scalable modern mode
>>>
>>> On 2024/9/30 17:26, Zhenzhong Duan wrote:
>>>> Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities
>>>> related to scalable mode translation, thus there are multiple combinations.
>>>>
>>>> This vIOMMU implementation wants to simplify it with a new property "x-fls".
>>>> When enabled in scalable mode, first stage translation also known as
>scalable
>>>> modern mode is supported. When enabled in legacy mode, throw out error.
>>>>
>>>> With scalable modern mode exposed to user, also accurate the pasid entry
>>>> check in vtd_pe_type_check().
>>>>
>>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>
>>> Maybe a Suggested-by tag can help to understand where this idea comes. :)
>>
>> Will add:
>> Suggested-by: Jason Wang <jasowang@redhat.com>
>>
>>>
>>>> ---
>>>> hw/i386/intel_iommu_internal.h | 2 ++
>>>> hw/i386/intel_iommu.c | 28 +++++++++++++++++++---------
>>>> 2 files changed, 21 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>>>> index 2702edd27f..f13576d334 100644
>>>> --- a/hw/i386/intel_iommu_internal.h
>>>> +++ b/hw/i386/intel_iommu_internal.h
>>>> @@ -195,6 +195,7 @@
>>>> #define VTD_ECAP_PASID (1ULL << 40)
>>>> #define VTD_ECAP_SMTS (1ULL << 43)
>>>> #define VTD_ECAP_SLTS (1ULL << 46)
>>>> +#define VTD_ECAP_FLTS (1ULL << 47)
>>>>
>>>> /* CAP_REG */
>>>> /* (offset >> 4) << 24 */
>>>> @@ -211,6 +212,7 @@
>>>> #define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
>>>> #define VTD_CAP_DRAIN_WRITE (1ULL << 54)
>>>> #define VTD_CAP_DRAIN_READ (1ULL << 55)
>>>> +#define VTD_CAP_FS1GP (1ULL << 56)
>>>> #define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ |
>>> VTD_CAP_DRAIN_WRITE)
>>>> #define VTD_CAP_CM (1ULL << 7)
>>>> #define VTD_PASID_ID_SHIFT 20
>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>>> index 068a08f522..14578655e1 100644
>>>> --- a/hw/i386/intel_iommu.c
>>>> +++ b/hw/i386/intel_iommu.c
>>>> @@ -803,16 +803,18 @@ static inline bool
>>> vtd_is_fl_level_supported(IntelIOMMUState *s, uint32_t level)
>>>> }
>>>>
>>>> /* Return true if check passed, otherwise false */
>>>> -static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu,
>>>> - VTDPASIDEntry *pe)
>>>> +static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry
>*pe)
>>>> {
>>>> switch (VTD_PE_GET_TYPE(pe)) {
>>>> - case VTD_SM_PASID_ENTRY_SLT:
>>>> - return true;
>>>> - case VTD_SM_PASID_ENTRY_PT:
>>>> - return x86_iommu->pt_supported;
>>>> case VTD_SM_PASID_ENTRY_FLT:
>>>> + return !!(s->ecap & VTD_ECAP_FLTS);
>>>> + case VTD_SM_PASID_ENTRY_SLT:
>>>> + return !!(s->ecap & VTD_ECAP_SLTS);
>>>> case VTD_SM_PASID_ENTRY_NESTED:
>>>> + /* Not support NESTED page table type yet */
>>>> + return false;
>>>> + case VTD_SM_PASID_ENTRY_PT:
>>>> + return !!(s->ecap & VTD_ECAP_PT);
>>>> default:
>>>> /* Unknown type */
>>>> return false;
>>>> @@ -861,7 +863,6 @@ static int
>>> vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>>>> uint8_t pgtt;
>>>> uint32_t index;
>>>> dma_addr_t entry_size;
>>>> - X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>>>>
>>>> index = VTD_PASID_TABLE_INDEX(pasid);
>>>> entry_size = VTD_PASID_ENTRY_SIZE;
>>>> @@ -875,7 +876,7 @@ static int
>>> vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s,
>>>> }
>>>>
>>>> /* Do translation type check */
>>>> - if (!vtd_pe_type_check(x86_iommu, pe)) {
>>>> + if (!vtd_pe_type_check(s, pe)) {
>>>> return -VTD_FR_PASID_TABLE_ENTRY_INV;
>>>> }
>>>>
>>>> @@ -3779,6 +3780,7 @@ static Property vtd_properties[] = {
>>>> VTD_HOST_AW_AUTO),
>>>> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>>> FALSE),
>>>> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState,
>scalable_mode,
>>> FALSE),
>>>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
>>>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control,
>>> false),
>>>
>>> a question: is there any requirement on the layout of this array? Should
>>> new fields added in the end?
>>
>> Looked over the history, seems we didn't have an explicit rule in vtd_properties.
>> I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of scalable
>mode.
>> Let me know if you have preference to add in the end.
>
>I don't have a preference for now as long as it does not break any
>functionality. BTW. Will x-flt or x-flts better?
So first level support(fls) vs. first level translation(flt) or first level translation support(flts),
looks same for me, but I can change to x-flt or x-flts if you prefer.
Thanks
Zhenzhong
On 2024/11/5 11:11, Duan, Zhenzhong wrote:
>>>>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern, FALSE),
>>>>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control,
>>>> false),
>>>>
>>>> a question: is there any requirement on the layout of this array? Should
>>>> new fields added in the end?
>>>
>>> Looked over the history, seems we didn't have an explicit rule in vtd_properties.
>>> I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of scalable
>> mode.
>>> Let me know if you have preference to add in the end.
>>
>> I don't have a preference for now as long as it does not break any
>> functionality. BTW. Will x-flt or x-flts better?
>
> So first level support(fls) vs. first level translation(flt) or first level translation support(flts),
> looks same for me, but I can change to x-flt or x-flts if you prefer.
x-flts looks better as it suits more how spec tells it (FSTS in the eap
register). :)
--
Regards,
Yi Liu
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Sent: Tuesday, November 5, 2024 1:56 PM
>Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>scalable modern mode
>
>On 2024/11/5 11:11, Duan, Zhenzhong wrote:
>
>>>>>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern,
>FALSE),
>>>>>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState,
>snoop_control,
>>>>> false),
>>>>>
>>>>> a question: is there any requirement on the layout of this array? Should
>>>>> new fields added in the end?
>>>>
>>>> Looked over the history, seems we didn't have an explicit rule in
>vtd_properties.
>>>> I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of
>scalable
>>> mode.
>>>> Let me know if you have preference to add in the end.
>>>
>>> I don't have a preference for now as long as it does not break any
>>> functionality. BTW. Will x-flt or x-flts better?
>>
>> So first level support(fls) vs. first level translation(flt) or first level translation
>support(flts),
>> looks same for me, but I can change to x-flt or x-flts if you prefer.
>
>x-flts looks better as it suits more how spec tells it (FSTS in the eap
>register). :)
Got it, just double confirm you prefer x-flts, not x-fsts?
Thanks
Zhenzhong
On 2024/11/5 14:03, Duan, Zhenzhong wrote:
>
>
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Tuesday, November 5, 2024 1:56 PM
>> Subject: Re: [PATCH v4 15/17] intel_iommu: Introduce a property x-fls for
>> scalable modern mode
>>
>> On 2024/11/5 11:11, Duan, Zhenzhong wrote:
>>
>>>>>>> + DEFINE_PROP_BOOL("x-fls", IntelIOMMUState, scalable_modern,
>> FALSE),
>>>>>>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState,
>> snoop_control,
>>>>>> false),
>>>>>>
>>>>>> a question: is there any requirement on the layout of this array? Should
>>>>>> new fields added in the end?
>>>>>
>>>>> Looked over the history, seems we didn't have an explicit rule in
>> vtd_properties.
>>>>> I put "x-fls" just under "x-scalable-mode" as stage-1 is a sub-feature of
>> scalable
>>>> mode.
>>>>> Let me know if you have preference to add in the end.
>>>>
>>>> I don't have a preference for now as long as it does not break any
>>>> functionality. BTW. Will x-flt or x-flts better?
>>>
>>> So first level support(fls) vs. first level translation(flt) or first level translation
>> support(flts),
>>> looks same for me, but I can change to x-flt or x-flts if you prefer.
>>
>> x-flts looks better as it suits more how spec tells it (FSTS in the eap
>> register). :)
>
> Got it, just double confirm you prefer x-flts, not x-fsts?
x-flts as most of the code use flt instead of fst.
--
Regards,
Yi Liu
© 2016 - 2024 Red Hat, Inc.