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

From: Duan, Zhenzhong
Date: Wed Oct 23 2024 - 01:17:08 EST




>-----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