On 09/12/2024 04:13, Jason Wang wrote:
> Caution: External email. Do not open attachments or click links, unless this
> email comes from a known sender and you know the content is safe.
>
>
> On Wed, Dec 4, 2024 at 2:14 PM CLEMENT MATHIEU--DRIF
> <clement.mathieu--d...@eviden.com> wrote:
>>
>>
>>
>> On 04/12/2024 04:34, Jason Wang wrote:
>>> Caution: External email. Do not open attachments or click links, unless
>>> this email comes from a known sender and you know the content is safe.
>>>
>>>
>>> On Mon, Nov 11, 2024 at 4:39 PM Zhenzhong Duan <zhenzhong.d...@intel.com>
>>> 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-flts".
>>>> 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().
>>>>
>>>> Suggested-by: Jason Wang <jasow...@redhat.com>
>>>> Signed-off-by: Yi Liu <yi.l....@intel.com>
>>>> Signed-off-by: Yi Sun <yi.y....@linux.intel.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@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 2c977aa7da..e8b211e8b0 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 b921793c3a..a7a81aebee 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;
>>>> }
>>>>
>>>> @@ -3827,6 +3828,7 @@ static Property vtd_properties[] = {
>>>> VTD_HOST_ADDRESS_WIDTH),
>>>> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>>>> FALSE),
>>>> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode,
>>>> FALSE),
>>>> + DEFINE_PROP_BOOL("x-flts", 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),
>>>> @@ -4558,7 +4560,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;
>>>> }
>>>>
>>>> @@ -4737,6 +4742,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-flts=on");
>>>
>>> This seems to be wired, should we say "scalable mode is needed for
>>> scalable modern mode"?
>>
>> Hi Jason,
>>
>> We agreed to use the following sentence: "x-flts is only available in
>> scalable mode"
>>
>> Does it look goot to you?
>
> Better but if we add more features to the scalable modern, we need to
> change the error message here.
Hi Jason
Maybe the weirdness comes from the fact that x-flts on the command line
is mapped to scalable_modern in the code?
Thanks
>cmd
>
> Thanks
>
>>
>> Thanks
>> cmd
>>
>>>
>>>> + return false;
>>>> + }
>>>> +
>>>> if (!s->scalable_modern && s->aw_bits != VTD_HOST_AW_39BIT &&
>>>> s->aw_bits != VTD_HOST_AW_48BIT) {
>>>> error_setg(errp, "%s mode: supported values for aw-bits are:
>>>> %d, %d",
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> Thanks
>>>
>
