RE: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk()

[Duan, Zhenzhong]

>-----Original Message-----
>From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
>Sent: Wednesday, October 23, 2024 12:50 PM
>Subject: Re: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk()
>
>On 2024/10/22 17:50, Zhenzhong Duan wrote:
>> It's not accurate to dump super page as non-present page,
>> meanwhile bit7 in first level page table entry is PAT bit,
>
>Can you please explain how the 'bit7 in first level page table entry' is
>relevant to the changes made in this patch?

When iterate to level 1, it is leaf page table entry, bit 7 is PAT bit instead of PS bit.
dma_pte_superpage(pte) may return true, then " PTE not present at level 1" may
be printed out.

>
>Also, please make full use of the maximum length of the commit message
>line.

Sure.

Thanks
Zhenzhong

>
>> also pointer pte is never NULL in pgtable_walk() context.
>>
>> Fixes: 914ff7719e8a ("iommu/vt-d: Dump DMAR translation structure when
>DMA fault occurs")
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
>> ---
>>   drivers/iommu/intel/iommu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 8288b0ee7a61..fec5cc1147f3 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -707,14 +707,14 @@ static void pgtable_walk(struct intel_iommu *iommu,
>unsigned long pfn,
>>   	while (1) {
>>   		offset = pfn_level_offset(pfn, level);
>>   		pte = &parent[offset];
>> -		if (!pte || (dma_pte_superpage(pte) || !dma_pte_present(pte))) {
>> +		if (!dma_pte_present(pte)) {
>>   			pr_info("PTE not present at level %d\n", level);
>>   			break;
>>   		}
>>
>>   		pr_info("pte level: %d, pte value: 0x%016llx\n", level, pte->val);
>>
>> -		if (level == 1)
>> +		if (level == 1 || dma_pte_superpage(pte))
>>   			break;
>>
>>   		parent = phys_to_virt(dma_pte_addr(pte));
>
>Thanks,
>baolu