[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <SJ0PR11MB6744272E218E88BCB7F05956924D2@SJ0PR11MB6744.namprd11.prod.outlook.com>
Date: Wed, 23 Oct 2024 09:41:27 +0000
From: "Duan, Zhenzhong" <zhenzhong.duan@...el.com>
To: "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "dwmw2@...radead.org" <dwmw2@...radead.org>, "baolu.lu@...ux.intel.com"
<baolu.lu@...ux.intel.com>, "joro@...tes.org" <joro@...tes.org>,
"will@...nel.org" <will@...nel.org>, "robin.murphy@....com"
<robin.murphy@....com>, "Peng, Chao P" <chao.p.peng@...el.com>, "Kyung Min
Park" <kyung.min.park@...el.com>
Subject: RE: [PATCH v2 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes()
Just found I mixed the present bit processing with non-present entry in this patch,
I'll resend after fix, please ignore it. Sorry for the noise.
Thanks
Zhenzhong
>-----Original Message-----
>From: Duan, Zhenzhong <zhenzhong.duan@...el.com>
>Subject: [PATCH v2 1/2] iommu/vt-d: Fix checks in dmar_fault_dump_ptes()
>
>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 of an entry before dump its content.
>
>Fixes: 914ff7719e8a ("iommu/vt-d: Dump DMAR translation structure when DMA
>fault occurs")
>Signed-off-by: Zhenzhong Duan <zhenzhong.duan@...el.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
Powered by blists - more mailing lists