On 13/08/2024 04:12, Duan, Zhenzhong 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.
>
>
>> -----Original Message-----
>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--d...@eviden.com>
>> Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID-
>> selective PASID-based iotlb invalidation
>>
>>
>>
>> On 08/08/2024 14:40, Duan, Zhenzhong 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 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote:
>>>> On 05/08/2024 08:27, Zhenzhong Duan 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.
>>>>>
>>>>>
>>>>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
>>>>> flush stage-2 iotlb entries with matching domain id and pasid.
>>>>>
>>>>> With scalable modern mode introduced, guest could send PASID-
>> selective
>>>>> PASID-based iotlb invalidation to flush both stage-1 and stage-2
>>>>> entries.
>>>>>
>>>>> By this chance, remove old IOTLB related definition.
>>>>>
>>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>>>> ---
>>>>> hw/i386/intel_iommu_internal.h | 14 +++---
>>>>> hw/i386/intel_iommu.c | 81
>>>>> ++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 90 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/hw/i386/intel_iommu_internal.h
>>>>> b/hw/i386/intel_iommu_internal.h
>>>>> index 8fa27c7f3b..19e4ed52ca 100644
>>>>> --- a/hw/i386/intel_iommu_internal.h
>>>>> +++ b/hw/i386/intel_iommu_internal.h
>>>>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
>>>>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
>>>>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
>>>>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
>>>>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
>>>>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
>>>>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) &
>>>>> VTD_PASID_ID_MASK)
>>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO
>> 0xfff00000000001c0ULL
>>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
>>>>>
>>>>> /* Mask for Device IOTLB Invalidate Descriptor */
>>>>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) &
>>>>> 0xfffffffffffff000ULL)
>>>>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
>>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>>>>> VTD_SL_TM)) : \
>>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>>>>
>>>>> +/* Masks for PIOTLB Invalidate Descriptor */
>>>>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
>>>>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
>>>>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
>>>>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) &
>>>>> VTD_DOMAIN_ID_MASK)
>>>>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
>>>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
>>>> Why did this value change since last post? The 'type' field should
>>>> always be zero in this desc
>>> Yes, type[6:4] are all zero for all existing invalidation type. But they
>>> are not real reserved bits.
>>>
>>> So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0.
>> Other masks consider these zeroes as reserved.
>> I think we should do the same.
>> For instance, context cache invalidation is : #define
>> VTD_INV_DESC_CC_RSVD 0xfffc00000000ffc0ULL
> Yes, I'll make a separate patch to fix it.
Oops, I just saw your patch, sorry for the misunderstanding!!
I think we should continue treating these bits as reserved because the
descriptor type detection code only checks the 4 LSB.
>
> Thanks
> Zhenzhong
