| Message ID | 20240930092631.2997543-16-zhenzhong.duan@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | intel_iommu: Enable stage-1 translation for emulated device | expand |
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. :) > --- > 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;
>-----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
>-----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). :)
>-----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.
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;