Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID-selective PASID-based iotlb invalidation

[CLEMENT MATHIEU--DRIF]

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.duan@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
>
> Thanks
>
> Zhenzhong
>