>-----Original Message-----
>From: Liu, Yi L <yi.l....@intel.com>
>Sent: Friday, October 25, 2024 5:51 PM
>Subject: Re: [PATCH v4] intel_iommu: Introduce property "stale-tm" to control
>Transient Mapping (TM) field
>
>On 2024/10/23 15:57, Zhenzhong Duan wrote:
>> VT-d spec removed Transient Mapping (TM) field from second-level page-tables
>> and treat the field as Reserved(0) since revision 3.2.
>>
>> Changing the field as reserved(0) will break backward compatibility, so
>> introduce a property "stale-tm" to allow user to control the setting.
>>
>> Use hw_compat_9_1 to handle the compatibility for machines before 9.2 which
>
>is hw_compat_9_1 a typo? Looks to be pc_compat_9_1. :)
Good catch! Yes, will fix.
Thanks
Zhenzhong
>
>Otherwise I think it is good.
>
>Reviewed-by: Yi Liu <yi.l....@intel.com>
>
>> allow guest to set the field. Starting from 9.2, this field is reserved(0)
>> by default to match spec. Of course, user can force it on command line.
>>
>> This doesn't impact function of vIOMMU as there was no logic to emulate
>> Transient Mapping.
>>
>> Suggested-by: Yi Liu <yi.l....@intel.com>
>> Suggested-by: Jason Wang <jasow...@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> v4: s/x-stale-tm/stale-tm (Jason)
>> v3: still need to check x86_iommu->dt_supported
>> v2: introcude "x-stale-tm" to handle migration compatibility (Jason)
>>
>> hw/i386/intel_iommu_internal.h | 12 ++++++------
>> include/hw/i386/intel_iommu.h | 3 +++
>> hw/i386/intel_iommu.c | 7 ++++---
>> hw/i386/pc.c | 1 +
>> 4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index 13d5d129ae..2f9bc0147d 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -412,8 +412,8 @@ typedef union VTDInvDesc VTDInvDesc;
>> /* Rsvd field masks for spte */
>> #define VTD_SPTE_SNP 0x800ULL
>>
>> -#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, dt_supported) \
>> - dt_supported ? \
>> +#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, stale_tm) \
>> + stale_tm ? \
>> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) :
>\
>> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>> #define VTD_SPTE_PAGE_L2_RSVD_MASK(aw) \
>> @@ -423,12 +423,12 @@ typedef union VTDInvDesc VTDInvDesc;
>> #define VTD_SPTE_PAGE_L4_RSVD_MASK(aw) \
>> (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>
>> -#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, dt_supported) \
>> - dt_supported ? \
>> +#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, stale_tm) \
>> + stale_tm ? \
>> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>VTD_SL_TM)) : \
>> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>> -#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, dt_supported) \
>> - dt_supported ? \
>> +#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, stale_tm) \
>> + stale_tm ? \
>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>VTD_SL_TM)) : \
>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>
>> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
>> index 1eb05c29fc..d372cd396b 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -306,6 +306,9 @@ struct IntelIOMMUState {
>> bool dma_translation; /* Whether DMA translation supported */
>> bool pasid; /* Whether to support PASID */
>>
>> + /* Transient Mapping, Reserved(0) since VTD spec revision 3.2 */
>> + bool stale_tm;
>> +
>> /*
>> * Protects IOMMU states in general. Currently it protects the
>> * per-IOMMU IOTLB cache, and context entry cache in VTDAddressSpace.
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 08fe218935..8612d0917b 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3372,6 +3372,7 @@ static Property vtd_properties[] = {
>> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false),
>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>> DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, dma_translation,
>true),
>> + DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> @@ -4138,15 +4139,15 @@ static void vtd_init(IntelIOMMUState *s)
>> */
>> vtd_spte_rsvd[0] = ~0ULL;
>> vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits,
>> - x86_iommu->dt_supported);
>> + x86_iommu->dt_supported &&
>> s->stale_tm);
>> vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits);
>> vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits);
>> vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits);
>>
>> vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits,
>> -
>> x86_iommu->dt_supported);
>> + x86_iommu->dt_supported &&
>> s->stale_tm);
>> vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits,
>> -
>> x86_iommu->dt_supported);
>> + x86_iommu->dt_supported &&
>> s->stale_tm);
>>
>> if (s->scalable_mode || s->snoop_control) {
>> vtd_spte_rsvd[1] &= ~VTD_SPTE_SNP;
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 2047633e4c..830614d930 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -82,6 +82,7 @@
>> GlobalProperty pc_compat_9_1[] = {
>> { "ICH9-LPC", "x-smi-swsmi-timer", "off" },
>> { "ICH9-LPC", "x-smi-periodic-timer", "off" },
>> + { TYPE_INTEL_IOMMU_DEVICE, "stale-tm", "on" },
>> };
>> const size_t pc_compat_9_1_len = G_N_ELEMENTS(pc_compat_9_1);
>>
>
>--
>Regards,
>Yi Liu
