* [PATCH 0/2] vtd: Minor cleanup @ 2024-10-22 9:50 Zhenzhong Duan 2024-10-22 9:50 ` [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() Zhenzhong Duan 2024-10-22 9:50 ` [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() Zhenzhong Duan 0 siblings, 2 replies; 10+ messages in thread From: Zhenzhong Duan @ 2024-10-22 9:50 UTC (permalink / raw) To: iommu, linux-kernel Cc: dwmw2, baolu.lu, joro, will, robin.murphy, chao.p.peng, Zhenzhong Duan Hi, This cleanup the vtd dump code to dump entry when it's present. See patch log for details. Thanks Zhenzhong Zhenzhong Duan (2): iommu/vt-d: Fix checks in dmar_fault_dump_ptes() iommu/vt-d: Fix checks in pgtable_walk() drivers/iommu/intel/iommu.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() 2024-10-22 9:50 [PATCH 0/2] vtd: Minor cleanup Zhenzhong Duan @ 2024-10-22 9:50 ` Zhenzhong Duan 2024-10-23 3:12 ` Baolu Lu 2024-10-22 9:50 ` [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() Zhenzhong Duan 1 sibling, 1 reply; 10+ messages in thread From: Zhenzhong Duan @ 2024-10-22 9:50 UTC (permalink / raw) To: iommu, linux-kernel Cc: dwmw2, baolu.lu, joro, will, robin.murphy, chao.p.peng, Zhenzhong Duan In dmar_fault_dump_ptes(), return value of phys_to_virt() is used for checking if an entry is present. It's never NULL on x86 platform at least. This makes some zero entries are dumped like below: [ 442.240357] DMAR: pasid dir entry: 0x000000012c83e001 [ 442.246661] DMAR: pasid table entry[0]: 0x0000000000000000 [ 442.253429] DMAR: pasid table entry[1]: 0x0000000000000000 [ 442.260203] DMAR: pasid table entry[2]: 0x0000000000000000 [ 442.266969] DMAR: pasid table entry[3]: 0x0000000000000000 [ 442.273733] DMAR: pasid table entry[4]: 0x0000000000000000 [ 442.280479] DMAR: pasid table entry[5]: 0x0000000000000000 [ 442.287234] DMAR: pasid table entry[6]: 0x0000000000000000 [ 442.293989] DMAR: pasid table entry[7]: 0x0000000000000000 [ 442.300742] DMAR: PTE not present at level 2 Fix it by checking present bit in all entries. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- drivers/iommu/intel/iommu.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index a564eeaf2375..8288b0ee7a61 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -733,12 +733,17 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id, u8 devfn = source_id & 0xff; u8 bus = source_id >> 8; struct dma_pte *pgtable; + u64 entry; pr_info("Dump %s table entries for IOVA 0x%llx\n", iommu->name, addr); /* root entry dump */ rt_entry = &iommu->root_entry[bus]; - if (!rt_entry) { + entry = rt_entry->lo; + if (sm_supported(iommu) && devfn >= 0x80) + entry = rt_entry->hi; + + if (!(entry & 1)) { pr_info("root table entry is not present\n"); return; } @@ -766,28 +771,32 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id, goto pgtable_walk; } + /* For request-without-pasid, get the pasid from context entry */ + if (pasid == IOMMU_PASID_INVALID) + pasid = IOMMU_NO_PASID; + /* get the pointer to pasid directory entry */ dir = phys_to_virt(ctx_entry->lo & VTD_PAGE_MASK); - if (!dir) { + dir_index = pasid >> PASID_PDE_SHIFT; + pde = &dir[dir_index]; + + if (!pasid_pde_is_present(pde)) { pr_info("pasid directory entry is not present\n"); return; } - /* For request-without-pasid, get the pasid from context entry */ - if (intel_iommu_sm && pasid == IOMMU_PASID_INVALID) - pasid = IOMMU_NO_PASID; - dir_index = pasid >> PASID_PDE_SHIFT; - pde = &dir[dir_index]; pr_info("pasid dir entry: 0x%016llx\n", pde->val); /* get the pointer to the pasid table entry */ - entries = get_pasid_table_from_pde(pde); - if (!entries) { + entries = phys_to_virt(READ_ONCE(pde->val) & PDE_PFN_MASK); + index = pasid & PASID_PTE_MASK; + pte = &entries[index]; + + if (!pasid_pte_is_present(pte)) { pr_info("pasid table entry is not present\n"); return; } - index = pasid & PASID_PTE_MASK; - pte = &entries[index]; + for (i = 0; i < ARRAY_SIZE(pte->val); i++) pr_info("pasid table entry[%d]: 0x%016llx\n", i, pte->val[i]); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() 2024-10-22 9:50 ` [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() Zhenzhong Duan @ 2024-10-23 3:12 ` Baolu Lu 2024-10-23 5:09 ` Duan, Zhenzhong 0 siblings, 1 reply; 10+ messages in thread From: Baolu Lu @ 2024-10-23 3:12 UTC (permalink / raw) To: Zhenzhong Duan, iommu, linux-kernel Cc: baolu.lu, dwmw2, joro, will, robin.murphy, chao.p.peng On 2024/10/22 17:50, Zhenzhong Duan wrote: > In dmar_fault_dump_ptes(), return value of phys_to_virt() is used for > checking if an entry is present. It's never NULL on x86 platform at least. > This makes some zero entries are dumped like below: > > [ 442.240357] DMAR: pasid dir entry: 0x000000012c83e001 > [ 442.246661] DMAR: pasid table entry[0]: 0x0000000000000000 > [ 442.253429] DMAR: pasid table entry[1]: 0x0000000000000000 > [ 442.260203] DMAR: pasid table entry[2]: 0x0000000000000000 > [ 442.266969] DMAR: pasid table entry[3]: 0x0000000000000000 > [ 442.273733] DMAR: pasid table entry[4]: 0x0000000000000000 > [ 442.280479] DMAR: pasid table entry[5]: 0x0000000000000000 > [ 442.287234] DMAR: pasid table entry[6]: 0x0000000000000000 > [ 442.293989] DMAR: pasid table entry[7]: 0x0000000000000000 > [ 442.300742] DMAR: PTE not present at level 2 > > Fix it by checking present bit in all entries. > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> It seems that we still need a Fixes tag here. > --- > drivers/iommu/intel/iommu.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index a564eeaf2375..8288b0ee7a61 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -733,12 +733,17 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id, > u8 devfn = source_id & 0xff; > u8 bus = source_id >> 8; > struct dma_pte *pgtable; > + u64 entry; > > pr_info("Dump %s table entries for IOVA 0x%llx\n", iommu->name, addr); > > /* root entry dump */ > rt_entry = &iommu->root_entry[bus]; > - if (!rt_entry) { > + entry = rt_entry->lo; > + if (sm_supported(iommu) && devfn >= 0x80) > + entry = rt_entry->hi; > + > + if (!(entry & 1)) { > pr_info("root table entry is not present\n"); > return; > } > @@ -766,28 +771,32 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id, > goto pgtable_walk; > } > > + /* For request-without-pasid, get the pasid from context entry */ > + if (pasid == IOMMU_PASID_INVALID) > + pasid = IOMMU_NO_PASID; > + > /* get the pointer to pasid directory entry */ > dir = phys_to_virt(ctx_entry->lo & VTD_PAGE_MASK); Is above code correct in the scalable mode? > - if (!dir) { > + dir_index = pasid >> PASID_PDE_SHIFT; > + pde = &dir[dir_index]; > + > + if (!pasid_pde_is_present(pde)) { > pr_info("pasid directory entry is not present\n"); > return; > } > - /* For request-without-pasid, get the pasid from context entry */ > - if (intel_iommu_sm && pasid == IOMMU_PASID_INVALID) > - pasid = IOMMU_NO_PASID; > > - dir_index = pasid >> PASID_PDE_SHIFT; > - pde = &dir[dir_index]; > pr_info("pasid dir entry: 0x%016llx\n", pde->val); > > /* get the pointer to the pasid table entry */ > - entries = get_pasid_table_from_pde(pde); > - if (!entries) { > + entries = phys_to_virt(READ_ONCE(pde->val) & PDE_PFN_MASK); > + index = pasid & PASID_PTE_MASK; > + pte = &entries[index]; > + > + if (!pasid_pte_is_present(pte)) { > pr_info("pasid table entry is not present\n"); > return; > } > - index = pasid & PASID_PTE_MASK; > - pte = &entries[index]; > + > for (i = 0; i < ARRAY_SIZE(pte->val); i++) > pr_info("pasid table entry[%d]: 0x%016llx\n", i, pte->val[i]); > Thanks, baolu ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() 2024-10-23 3:12 ` Baolu Lu @ 2024-10-23 5:09 ` Duan, Zhenzhong 2024-10-23 7:37 ` Baolu Lu 0 siblings, 1 reply; 10+ messages in thread From: Duan, Zhenzhong @ 2024-10-23 5:09 UTC (permalink / raw) To: Baolu Lu, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Cc: dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, Peng, Chao P >-----Original Message----- >From: Baolu Lu <baolu.lu@linux.intel.com> >Subject: Re: [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() > >On 2024/10/22 17:50, Zhenzhong Duan wrote: >> In dmar_fault_dump_ptes(), return value of phys_to_virt() is used for >> checking if an entry is present. It's never NULL on x86 platform at least. >> This makes some zero entries are dumped like below: >> >> [ 442.240357] DMAR: pasid dir entry: 0x000000012c83e001 >> [ 442.246661] DMAR: pasid table entry[0]: 0x0000000000000000 >> [ 442.253429] DMAR: pasid table entry[1]: 0x0000000000000000 >> [ 442.260203] DMAR: pasid table entry[2]: 0x0000000000000000 >> [ 442.266969] DMAR: pasid table entry[3]: 0x0000000000000000 >> [ 442.273733] DMAR: pasid table entry[4]: 0x0000000000000000 >> [ 442.280479] DMAR: pasid table entry[5]: 0x0000000000000000 >> [ 442.287234] DMAR: pasid table entry[6]: 0x0000000000000000 >> [ 442.293989] DMAR: pasid table entry[7]: 0x0000000000000000 >> [ 442.300742] DMAR: PTE not present at level 2 >> >> Fix it by checking present bit in all entries. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > >It seems that we still need a Fixes tag here. OK, will add. > >> --- >> drivers/iommu/intel/iommu.c | 31 ++++++++++++++++++++----------- >> 1 file changed, 20 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index a564eeaf2375..8288b0ee7a61 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -733,12 +733,17 @@ void dmar_fault_dump_ptes(struct intel_iommu >*iommu, u16 source_id, >> u8 devfn = source_id & 0xff; >> u8 bus = source_id >> 8; >> struct dma_pte *pgtable; >> + u64 entry; >> >> pr_info("Dump %s table entries for IOVA 0x%llx\n", iommu->name, addr); >> >> /* root entry dump */ >> rt_entry = &iommu->root_entry[bus]; >> - if (!rt_entry) { >> + entry = rt_entry->lo; >> + if (sm_supported(iommu) && devfn >= 0x80) >> + entry = rt_entry->hi; >> + >> + if (!(entry & 1)) { >> pr_info("root table entry is not present\n"); >> return; >> } >> @@ -766,28 +771,32 @@ void dmar_fault_dump_ptes(struct intel_iommu >*iommu, u16 source_id, >> goto pgtable_walk; >> } >> >> + /* For request-without-pasid, get the pasid from context entry */ >> + if (pasid == IOMMU_PASID_INVALID) >> + pasid = IOMMU_NO_PASID; >> + >> /* get the pointer to pasid directory entry */ >> dir = phys_to_virt(ctx_entry->lo & VTD_PAGE_MASK); > >Is above code correct in the scalable mode? I didn't find issue, could you show some light? Thanks Zhenzhong > >> - if (!dir) { >> + dir_index = pasid >> PASID_PDE_SHIFT; >> + pde = &dir[dir_index]; >> + >> + if (!pasid_pde_is_present(pde)) { >> pr_info("pasid directory entry is not present\n"); >> return; >> } >> - /* For request-without-pasid, get the pasid from context entry */ >> - if (intel_iommu_sm && pasid == IOMMU_PASID_INVALID) >> - pasid = IOMMU_NO_PASID; >> >> - dir_index = pasid >> PASID_PDE_SHIFT; >> - pde = &dir[dir_index]; >> pr_info("pasid dir entry: 0x%016llx\n", pde->val); >> >> /* get the pointer to the pasid table entry */ >> - entries = get_pasid_table_from_pde(pde); >> - if (!entries) { >> + entries = phys_to_virt(READ_ONCE(pde->val) & PDE_PFN_MASK); >> + index = pasid & PASID_PTE_MASK; >> + pte = &entries[index]; >> + >> + if (!pasid_pte_is_present(pte)) { >> pr_info("pasid table entry is not present\n"); >> return; >> } >> - index = pasid & PASID_PTE_MASK; >> - pte = &entries[index]; >> + >> for (i = 0; i < ARRAY_SIZE(pte->val); i++) >> pr_info("pasid table entry[%d]: 0x%016llx\n", i, pte->val[i]); >> > >Thanks, >baolu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() 2024-10-23 5:09 ` Duan, Zhenzhong @ 2024-10-23 7:37 ` Baolu Lu 0 siblings, 0 replies; 10+ messages in thread From: Baolu Lu @ 2024-10-23 7:37 UTC (permalink / raw) To: Duan, Zhenzhong, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Cc: baolu.lu, dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, Peng, Chao P On 2024/10/23 13:09, Duan, Zhenzhong wrote: >>> --- >>> drivers/iommu/intel/iommu.c | 31 ++++++++++++++++++++----------- >>> 1 file changed, 20 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>> index a564eeaf2375..8288b0ee7a61 100644 >>> --- a/drivers/iommu/intel/iommu.c >>> +++ b/drivers/iommu/intel/iommu.c >>> @@ -733,12 +733,17 @@ void dmar_fault_dump_ptes(struct intel_iommu >> *iommu, u16 source_id, >>> u8 devfn = source_id & 0xff; >>> u8 bus = source_id >> 8; >>> struct dma_pte *pgtable; >>> + u64 entry; >>> >>> pr_info("Dump %s table entries for IOVA 0x%llx\n", iommu->name, addr); >>> >>> /* root entry dump */ >>> rt_entry = &iommu->root_entry[bus]; >>> - if (!rt_entry) { >>> + entry = rt_entry->lo; >>> + if (sm_supported(iommu) && devfn >= 0x80) >>> + entry = rt_entry->hi; >>> + >>> + if (!(entry & 1)) { >>> pr_info("root table entry is not present\n"); >>> return; >>> } >>> @@ -766,28 +771,32 @@ void dmar_fault_dump_ptes(struct intel_iommu >> *iommu, u16 source_id, >>> goto pgtable_walk; >>> } >>> >>> + /* For request-without-pasid, get the pasid from context entry */ >>> + if (pasid == IOMMU_PASID_INVALID) >>> + pasid = IOMMU_NO_PASID; >>> + >>> /* get the pointer to pasid directory entry */ >>> dir = phys_to_virt(ctx_entry->lo & VTD_PAGE_MASK); >> Is above code correct in the scalable mode? > I didn't find issue, could you show some light? It's fine. I just mixed it with the root entry. Thanks, baolu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() 2024-10-22 9:50 [PATCH 0/2] vtd: Minor cleanup Zhenzhong Duan 2024-10-22 9:50 ` [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() Zhenzhong Duan @ 2024-10-22 9:50 ` Zhenzhong Duan 2024-10-23 4:49 ` Baolu Lu 1 sibling, 1 reply; 10+ messages in thread From: Zhenzhong Duan @ 2024-10-22 9:50 UTC (permalink / raw) To: iommu, linux-kernel Cc: dwmw2, baolu.lu, joro, will, robin.murphy, chao.p.peng, Zhenzhong Duan, Kyung Min Park It's not accurate to dump super page as non-present page, meanwhile bit7 in first level page table entry is PAT bit, 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@intel.com> --- 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)); -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() 2024-10-22 9:50 ` [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() Zhenzhong Duan @ 2024-10-23 4:49 ` Baolu Lu 2024-10-23 5:16 ` Duan, Zhenzhong 0 siblings, 1 reply; 10+ messages in thread From: Baolu Lu @ 2024-10-23 4:49 UTC (permalink / raw) To: Zhenzhong Duan, iommu, linux-kernel Cc: baolu.lu, dwmw2, joro, will, robin.murphy, chao.p.peng, Kyung Min Park 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? Also, please make full use of the maximum length of the commit message line. > 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@intel.com> > --- > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() 2024-10-23 4:49 ` Baolu Lu @ 2024-10-23 5:16 ` Duan, Zhenzhong 2024-10-23 7:41 ` Baolu Lu 0 siblings, 1 reply; 10+ messages in thread From: Duan, Zhenzhong @ 2024-10-23 5:16 UTC (permalink / raw) To: Baolu Lu, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Cc: dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, Peng, Chao P, Kyung Min Park >-----Original Message----- >From: Baolu Lu <baolu.lu@linux.intel.com> >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@intel.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() 2024-10-23 5:16 ` Duan, Zhenzhong @ 2024-10-23 7:41 ` Baolu Lu 2024-10-23 7:48 ` Duan, Zhenzhong 0 siblings, 1 reply; 10+ messages in thread From: Baolu Lu @ 2024-10-23 7:41 UTC (permalink / raw) To: Duan, Zhenzhong, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Cc: baolu.lu, dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, Peng, Chao P, Kyung Min Park On 2024/10/23 13:16, Duan, Zhenzhong wrote: >> -----Original Message----- >> From: Baolu Lu<baolu.lu@linux.intel.com> >> 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. I see. Thank you! If you have a new version, can you please make it a bit clearer? My understanding is that dma_pte_superpage(pte) should not check against the leaf page table entries, right? Thanks, baolu ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() 2024-10-23 7:41 ` Baolu Lu @ 2024-10-23 7:48 ` Duan, Zhenzhong 0 siblings, 0 replies; 10+ messages in thread From: Duan, Zhenzhong @ 2024-10-23 7:48 UTC (permalink / raw) To: Baolu Lu, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Cc: dwmw2@infradead.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, Peng, Chao P, Kyung Min Park >-----Original Message----- >From: Baolu Lu <baolu.lu@linux.intel.com> >Subject: Re: [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() > >On 2024/10/23 13:16, Duan, Zhenzhong wrote: >>> -----Original Message----- >>> From: Baolu Lu<baolu.lu@linux.intel.com> >>> 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. > >I see. Thank you! > >If you have a new version, can you please make it a bit clearer? My >understanding is that dma_pte_superpage(pte) should not check against >the leaf page table entries, right? Right, will do. Thanks Zhenzhong ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-10-23 7:48 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-22 9:50 [PATCH 0/2] vtd: Minor cleanup Zhenzhong Duan 2024-10-22 9:50 ` [PATCH 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes() Zhenzhong Duan 2024-10-23 3:12 ` Baolu Lu 2024-10-23 5:09 ` Duan, Zhenzhong 2024-10-23 7:37 ` Baolu Lu 2024-10-22 9:50 ` [PATCH 2/2] iommu/vt-d: Fix checks in pgtable_walk() Zhenzhong Duan 2024-10-23 4:49 ` Baolu Lu 2024-10-23 5:16 ` Duan, Zhenzhong 2024-10-23 7:41 ` Baolu Lu 2024-10-23 7:48 ` Duan, Zhenzhong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox