>-----Original Message-----
>From: Liu, Yi L <yi.l....@intel.com>
>Subject: Re: [PATCH v2 07/17] intel_iommu: Check if the input address is
>canonical
>
>On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--d...@eviden.com>
>>
>> First stage translation must fail if the address to translate is
>> not canonical.
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--d...@eviden.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> hw/i386/intel_iommu_internal.h | 2 ++
>> hw/i386/intel_iommu.c | 21 +++++++++++++++++++++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 51e9b1fc43..668583aeca 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -320,6 +320,8 @@ typedef enum VTDFaultReason {
>> VTD_FR_PASID_ENTRY_P = 0x59,
>> VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b, /*Invalid PASID table entry
>*/
>>
>> + VTD_FR_FS_NON_CANONICAL = 0x80, /* SNG.1 : Address for FS not
>canonical.*/
>> +
>> /* Output address in the interrupt address range for scalable mode */
>> VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>> VTD_FR_MAX, /* Guard */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 0bcbd5b777..6121cca4cd 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1821,6 +1821,7 @@ static const bool vtd_qualified_faults[] = {
>> [VTD_FR_PASID_ENTRY_P] = true,
>> [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>> [VTD_FR_SM_INTERRUPT_ADDR] = true,
>> + [VTD_FR_FS_NON_CANONICAL] = true,
>> [VTD_FR_MAX] = false,
>> };
>>
>> @@ -1924,6 +1925,20 @@ static inline bool vtd_flpte_present(uint64_t
>flpte)
>> return !!(flpte & VTD_FL_P);
>> }
>>
>> +/* Return true if IOVA is canonical, otherwise false. */
>> +static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t
>iova,
>> + VTDContextEntry *ce, uint32_t pasid)
>> +{
>> + uint64_t iova_limit = vtd_iova_limit(s, ce, s->aw_bits, pasid);
>> + uint64_t upper_bits_mask = ~(iova_limit - 1);
>> + uint64_t upper_bits = iova & upper_bits_mask;
>> + bool msb = ((iova & (iova_limit >> 1)) != 0);
>> + return !(
>> + (!msb && (upper_bits != 0)) ||
>> + (msb && (upper_bits != upper_bits_mask))
>> + );
>> +}
>> +
>
>will the below be clearer?
>
> if (msb)
> return upper_bits == upper_bits_mask;
> else
> return !upper_bits;
Yes, clearer, will do.
Thanks
Zhenzhong
