linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging
@ 2024-08-27 18:17 Rob Clark
  2024-08-27 18:17 ` [PATCH v9 1/4] iommu/io-pgtable-arm: Make pgtable walker more generic Rob Clark
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Rob Clark @ 2024-08-27 18:17 UTC (permalink / raw)
  To: iommu
  Cc: Will Deacon, Robin Murphy, Rob Clark, Boris Brezillon,
	open list:DRM DRIVER for Qualcomm Adreno GPUs,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, Jason Gunthorpe,
	Joao Martins, Kevin Tian, Konrad Dybcio,
	moderated list:ARM SMMU DRIVERS,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, open list,
	open list:SUSPEND TO RAM, Marijn Suijten, Rafael J. Wysocki,
	Sean Paul, Steven Price

From: Rob Clark <robdclark@chromium.org>

This series extends io-pgtable-arm with a method to retrieve the page
table entries traversed in the process of address translation, and then
beefs up drm/msm gpu devcore dump to include this (and additional info)
in the devcore dump.

This is a respin of https://patchwork.freedesktop.org/series/94968/
(minus a patch that was already merged)

v2: Fix an armv7/32b build error in the last patch
v3: Incorperate Will Deacon's suggestion to make the interface
    callback based.
v4: Actually wire up the callback
v5: Drop the callback approach
v6: Make walk-data struct pgtable specific and rename
    io_pgtable_walk_data to arm_lpae_io_pgtable_walk_data
v7: Re-use the pgtable walker added for arm_lpae_read_and_clear_dirty()
v8: Pass pte pointer to callback so it can modify the actual pte
v9: Fix selftests_running case

Rob Clark (4):
  iommu/io-pgtable-arm: Make pgtable walker more generic
  iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys
  iommu/io-pgtable-arm: Add way to debug pgtable walk
  drm/msm: Extend gpu devcore dumps with pgtbl info

 drivers/gpu/drm/msm/adreno/adreno_gpu.c |  10 ++
 drivers/gpu/drm/msm/msm_gpu.c           |   9 ++
 drivers/gpu/drm/msm/msm_gpu.h           |   8 ++
 drivers/gpu/drm/msm/msm_iommu.c         |  22 ++++
 drivers/gpu/drm/msm/msm_mmu.h           |   3 +-
 drivers/iommu/io-pgtable-arm.c          | 149 +++++++++++++++---------
 include/linux/io-pgtable.h              |  15 +++
 7 files changed, 160 insertions(+), 56 deletions(-)

-- 
2.46.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v9 1/4] iommu/io-pgtable-arm: Make pgtable walker more generic
  2024-08-27 18:17 [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Rob Clark
@ 2024-08-27 18:17 ` Rob Clark
  2024-08-27 18:17 ` [PATCH v9 2/4] iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys Rob Clark
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2024-08-27 18:17 UTC (permalink / raw)
  To: iommu
  Cc: Will Deacon, Robin Murphy, Rob Clark, Joerg Roedel,
	moderated list:ARM SMMU DRIVERS, open list

From: Rob Clark <robdclark@chromium.org>

We can re-use this basic pgtable walk logic in a few places.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/iommu/io-pgtable-arm.c | 59 +++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f5d9fd1f45bf..0132f1eff688 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -747,33 +747,31 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
 }
 
 struct io_pgtable_walk_data {
-	struct iommu_dirty_bitmap	*dirty;
+	void				*data;
+	int (*visit)(struct io_pgtable_walk_data *walk_data, int lvl,
+		     arm_lpae_iopte *ptep, size_t size);
 	unsigned long			flags;
 	u64				addr;
 	const u64			end;
 };
 
-static int __arm_lpae_iopte_walk_dirty(struct arm_lpae_io_pgtable *data,
-				       struct io_pgtable_walk_data *walk_data,
-				       arm_lpae_iopte *ptep,
-				       int lvl);
+static int __arm_lpae_iopte_walk(struct arm_lpae_io_pgtable *data,
+				 struct io_pgtable_walk_data *walk_data,
+				 arm_lpae_iopte *ptep,
+				 int lvl);
 
-static int io_pgtable_visit_dirty(struct arm_lpae_io_pgtable *data,
-				  struct io_pgtable_walk_data *walk_data,
-				  arm_lpae_iopte *ptep, int lvl)
+static int io_pgtable_visit(struct arm_lpae_io_pgtable *data,
+			    struct io_pgtable_walk_data *walk_data,
+			    arm_lpae_iopte *ptep, int lvl)
 {
 	struct io_pgtable *iop = &data->iop;
 	arm_lpae_iopte pte = READ_ONCE(*ptep);
 
 	if (iopte_leaf(pte, lvl, iop->fmt)) {
 		size_t size = ARM_LPAE_BLOCK_SIZE(lvl, data);
-
-		if (iopte_writeable_dirty(pte)) {
-			iommu_dirty_bitmap_record(walk_data->dirty,
-						  walk_data->addr, size);
-			if (!(walk_data->flags & IOMMU_DIRTY_NO_CLEAR))
-				iopte_set_writeable_clean(ptep);
-		}
+		int ret = walk_data->visit(walk_data, lvl, ptep, size);
+		if (ret)
+			return ret;
 		walk_data->addr += size;
 		return 0;
 	}
@@ -782,13 +780,13 @@ static int io_pgtable_visit_dirty(struct arm_lpae_io_pgtable *data,
 		return -EINVAL;
 
 	ptep = iopte_deref(pte, data);
-	return __arm_lpae_iopte_walk_dirty(data, walk_data, ptep, lvl + 1);
+	return __arm_lpae_iopte_walk(data, walk_data, ptep, lvl + 1);
 }
 
-static int __arm_lpae_iopte_walk_dirty(struct arm_lpae_io_pgtable *data,
-				       struct io_pgtable_walk_data *walk_data,
-				       arm_lpae_iopte *ptep,
-				       int lvl)
+static int __arm_lpae_iopte_walk(struct arm_lpae_io_pgtable *data,
+				 struct io_pgtable_walk_data *walk_data,
+				 arm_lpae_iopte *ptep,
+				 int lvl)
 {
 	u32 idx;
 	int max_entries, ret;
@@ -803,7 +801,7 @@ static int __arm_lpae_iopte_walk_dirty(struct arm_lpae_io_pgtable *data,
 
 	for (idx = ARM_LPAE_LVL_IDX(walk_data->addr, lvl, data);
 	     (idx < max_entries) && (walk_data->addr < walk_data->end); ++idx) {
-		ret = io_pgtable_visit_dirty(data, walk_data, ptep + idx, lvl);
+		ret = io_pgtable_visit(data, walk_data, ptep + idx, lvl);
 		if (ret)
 			return ret;
 	}
@@ -811,6 +809,20 @@ static int __arm_lpae_iopte_walk_dirty(struct arm_lpae_io_pgtable *data,
 	return 0;
 }
 
+static int visit_dirty(struct io_pgtable_walk_data *walk_data, int lvl,
+		       arm_lpae_iopte *ptep, size_t size)
+{
+	struct iommu_dirty_bitmap *dirty = walk_data->data;
+
+	if (iopte_writeable_dirty(*ptep)) {
+		iommu_dirty_bitmap_record(dirty, walk_data->addr, size);
+		if (!(walk_data->flags & IOMMU_DIRTY_NO_CLEAR))
+			iopte_set_writeable_clean(ptep);
+	}
+
+	return 0;
+}
+
 static int arm_lpae_read_and_clear_dirty(struct io_pgtable_ops *ops,
 					 unsigned long iova, size_t size,
 					 unsigned long flags,
@@ -819,7 +831,8 @@ static int arm_lpae_read_and_clear_dirty(struct io_pgtable_ops *ops,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	struct io_pgtable_walk_data walk_data = {
-		.dirty = dirty,
+		.data = dirty,
+		.visit = visit_dirty,
 		.flags = flags,
 		.addr = iova,
 		.end = iova + size,
@@ -834,7 +847,7 @@ static int arm_lpae_read_and_clear_dirty(struct io_pgtable_ops *ops,
 	if (data->iop.fmt != ARM_64_LPAE_S1)
 		return -EINVAL;
 
-	return __arm_lpae_iopte_walk_dirty(data, &walk_data, ptep, lvl);
+	return __arm_lpae_iopte_walk(data, &walk_data, ptep, lvl);
 }
 
 static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
-- 
2.46.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v9 2/4] iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys
  2024-08-27 18:17 [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Rob Clark
  2024-08-27 18:17 ` [PATCH v9 1/4] iommu/io-pgtable-arm: Make pgtable walker more generic Rob Clark
@ 2024-08-27 18:17 ` Rob Clark
  2024-08-27 18:17 ` [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk Rob Clark
  2024-10-28 11:53 ` [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Mostafa Saleh
  3 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2024-08-27 18:17 UTC (permalink / raw)
  To: iommu
  Cc: Will Deacon, Robin Murphy, Rob Clark, Joerg Roedel,
	moderated list:ARM SMMU DRIVERS, open list

From: Rob Clark <robdclark@chromium.org>

Re-use the generic pgtable walk path.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/iommu/io-pgtable-arm.c | 75 +++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 0132f1eff688..efed8e519111 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -710,42 +710,6 @@ static size_t arm_lpae_unmap_pages(struct io_pgtable_ops *ops, unsigned long iov
 				data->start_level, ptep);
 }
 
-static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
-					 unsigned long iova)
-{
-	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
-	arm_lpae_iopte pte, *ptep = data->pgd;
-	int lvl = data->start_level;
-
-	do {
-		/* Valid IOPTE pointer? */
-		if (!ptep)
-			return 0;
-
-		/* Grab the IOPTE we're interested in */
-		ptep += ARM_LPAE_LVL_IDX(iova, lvl, data);
-		pte = READ_ONCE(*ptep);
-
-		/* Valid entry? */
-		if (!pte)
-			return 0;
-
-		/* Leaf entry? */
-		if (iopte_leaf(pte, lvl, data->iop.fmt))
-			goto found_translation;
-
-		/* Take it to the next level */
-		ptep = iopte_deref(pte, data);
-	} while (++lvl < ARM_LPAE_MAX_LEVELS);
-
-	/* Ran out of page tables to walk */
-	return 0;
-
-found_translation:
-	iova &= (ARM_LPAE_BLOCK_SIZE(lvl, data) - 1);
-	return iopte_to_paddr(pte, data) | iova;
-}
-
 struct io_pgtable_walk_data {
 	void				*data;
 	int (*visit)(struct io_pgtable_walk_data *walk_data, int lvl,
@@ -760,6 +724,41 @@ static int __arm_lpae_iopte_walk(struct arm_lpae_io_pgtable *data,
 				 arm_lpae_iopte *ptep,
 				 int lvl);
 
+struct iova_to_phys_data {
+	arm_lpae_iopte pte;
+	int lvl;
+};
+
+static int visit_iova_to_phys(struct io_pgtable_walk_data *walk_data, int lvl,
+			      arm_lpae_iopte *ptep, size_t size)
+{
+	struct iova_to_phys_data *data = walk_data->data;
+	data->pte = *ptep;
+	data->lvl = lvl;
+	return 0;
+}
+
+static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
+					 unsigned long iova)
+{
+	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	struct iova_to_phys_data d;
+	struct io_pgtable_walk_data walk_data = {
+		.data = &d,
+		.visit = visit_iova_to_phys,
+		.addr = iova,
+		.end = iova + 1,
+	};
+	int ret;
+
+	ret = __arm_lpae_iopte_walk(data, &walk_data, data->pgd, data->start_level);
+	if (ret)
+		return 0;
+
+	iova &= (ARM_LPAE_BLOCK_SIZE(d.lvl, data) - 1);
+	return iopte_to_paddr(d.pte, data) | iova;
+}
+
 static int io_pgtable_visit(struct arm_lpae_io_pgtable *data,
 			    struct io_pgtable_walk_data *walk_data,
 			    arm_lpae_iopte *ptep, int lvl)
@@ -776,8 +775,10 @@ static int io_pgtable_visit(struct arm_lpae_io_pgtable *data,
 		return 0;
 	}
 
-	if (WARN_ON(!iopte_table(pte, lvl)))
+	if (!iopte_table(pte, lvl)) {
+		WARN_ON(!selftest_running);
 		return -EINVAL;
+	}
 
 	ptep = iopte_deref(pte, data);
 	return __arm_lpae_iopte_walk(data, walk_data, ptep, lvl + 1);
-- 
2.46.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk
  2024-08-27 18:17 [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Rob Clark
  2024-08-27 18:17 ` [PATCH v9 1/4] iommu/io-pgtable-arm: Make pgtable walker more generic Rob Clark
  2024-08-27 18:17 ` [PATCH v9 2/4] iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys Rob Clark
@ 2024-08-27 18:17 ` Rob Clark
  2024-08-30 13:20   ` Will Deacon
  2024-10-28 11:53 ` [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Mostafa Saleh
  3 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2024-08-27 18:17 UTC (permalink / raw)
  To: iommu
  Cc: Will Deacon, Robin Murphy, Rob Clark, Joerg Roedel,
	Jason Gunthorpe, Joao Martins, Kevin Tian, Steven Price, Lu Baolu,
	Boris Brezillon, moderated list:ARM SMMU DRIVERS, open list

From: Rob Clark <robdclark@chromium.org>

Add an io-pgtable method to walk the pgtable returning the raw PTEs that
would be traversed for a given iova access.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++++++++
 include/linux/io-pgtable.h     | 15 +++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index efed8e519111..11af49119f37 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -759,6 +759,30 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
 	return iopte_to_paddr(d.pte, data) | iova;
 }
 
+static int visit_pgtable_walk(struct io_pgtable_walk_data *walk_data, int lvl,
+			      arm_lpae_iopte *ptep, size_t size)
+{
+	struct arm_lpae_io_pgtable_walk_data *data = walk_data->data;
+	data->ptes[data->level++] = *ptep;
+	return 0;
+}
+
+static int arm_lpae_pgtable_walk(struct io_pgtable_ops *ops, unsigned long iova,
+				 void *wd)
+{
+	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	struct io_pgtable_walk_data walk_data = {
+		.data = wd,
+		.visit = visit_pgtable_walk,
+		.addr = iova,
+		.end = iova + 1,
+	};
+
+	((struct arm_lpae_io_pgtable_walk_data *)wd)->level = 0;
+
+	return __arm_lpae_iopte_walk(data, &walk_data, data->pgd, data->start_level);
+}
+
 static int io_pgtable_visit(struct arm_lpae_io_pgtable *data,
 			    struct io_pgtable_walk_data *walk_data,
 			    arm_lpae_iopte *ptep, int lvl)
@@ -930,6 +954,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
 		.unmap_pages	= arm_lpae_unmap_pages,
 		.iova_to_phys	= arm_lpae_iova_to_phys,
 		.read_and_clear_dirty = arm_lpae_read_and_clear_dirty,
+		.pgtable_walk	= arm_lpae_pgtable_walk,
 	};
 
 	return data;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index f9a81761bfce..76eabd890e6a 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -174,12 +174,26 @@ struct io_pgtable_cfg {
 	};
 };
 
+/**
+ * struct arm_lpae_io_pgtable_walk_data - information from a pgtable walk
+ *
+ * @ptes:     The recorded PTE values from the walk
+ * @level:    The level of the last PTE
+ *
+ * @level also specifies the last valid index in @ptes
+ */
+struct arm_lpae_io_pgtable_walk_data {
+	u64 ptes[4];
+	int level;
+};
+
 /**
  * struct io_pgtable_ops - Page table manipulation API for IOMMU drivers.
  *
  * @map_pages:    Map a physically contiguous range of pages of the same size.
  * @unmap_pages:  Unmap a range of virtually contiguous pages of the same size.
  * @iova_to_phys: Translate iova to physical address.
+ * @pgtable_walk: (optional) Perform a page table walk for a given iova.
  *
  * These functions map directly onto the iommu_ops member functions with
  * the same names.
@@ -193,6 +207,7 @@ struct io_pgtable_ops {
 			      struct iommu_iotlb_gather *gather);
 	phys_addr_t (*iova_to_phys)(struct io_pgtable_ops *ops,
 				    unsigned long iova);
+	int (*pgtable_walk)(struct io_pgtable_ops *ops, unsigned long iova, void *wd);
 	int (*read_and_clear_dirty)(struct io_pgtable_ops *ops,
 				    unsigned long iova, size_t size,
 				    unsigned long flags,
-- 
2.46.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk
  2024-08-27 18:17 ` [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk Rob Clark
@ 2024-08-30 13:20   ` Will Deacon
  2024-08-30 14:32     ` Rob Clark
  0 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2024-08-30 13:20 UTC (permalink / raw)
  To: Rob Clark
  Cc: iommu, Robin Murphy, Rob Clark, Joerg Roedel, Jason Gunthorpe,
	Joao Martins, Kevin Tian, Steven Price, Lu Baolu, Boris Brezillon,
	moderated list:ARM SMMU DRIVERS, open list

On Tue, Aug 27, 2024 at 11:17:11AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Add an io-pgtable method to walk the pgtable returning the raw PTEs that
> would be traversed for a given iova access.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++++++++
>  include/linux/io-pgtable.h     | 15 +++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index efed8e519111..11af49119f37 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -759,6 +759,30 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
>  	return iopte_to_paddr(d.pte, data) | iova;
>  }
>  
> +static int visit_pgtable_walk(struct io_pgtable_walk_data *walk_data, int lvl,
> +			      arm_lpae_iopte *ptep, size_t size)
> +{
> +	struct arm_lpae_io_pgtable_walk_data *data = walk_data->data;
> +	data->ptes[data->level++] = *ptep;
> +	return 0;
> +}

I'm struggling to see how this works given that the ->visit() callback
is only called for leaf entries in the page-table. What am I missing?

Will


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk
  2024-08-30 13:20   ` Will Deacon
@ 2024-08-30 14:32     ` Rob Clark
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2024-08-30 14:32 UTC (permalink / raw)
  To: Will Deacon
  Cc: iommu, Robin Murphy, Rob Clark, Joerg Roedel, Jason Gunthorpe,
	Joao Martins, Kevin Tian, Steven Price, Lu Baolu, Boris Brezillon,
	moderated list:ARM SMMU DRIVERS, open list

On Fri, Aug 30, 2024 at 6:20 AM Will Deacon <will@kernel.org> wrote:
>
> On Tue, Aug 27, 2024 at 11:17:11AM -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Add an io-pgtable method to walk the pgtable returning the raw PTEs that
> > would be traversed for a given iova access.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++++++++
> >  include/linux/io-pgtable.h     | 15 +++++++++++++++
> >  2 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index efed8e519111..11af49119f37 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -759,6 +759,30 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
> >       return iopte_to_paddr(d.pte, data) | iova;
> >  }
> >
> > +static int visit_pgtable_walk(struct io_pgtable_walk_data *walk_data, int lvl,
> > +                           arm_lpae_iopte *ptep, size_t size)
> > +{
> > +     struct arm_lpae_io_pgtable_walk_data *data = walk_data->data;
> > +     data->ptes[data->level++] = *ptep;
> > +     return 0;
> > +}
>
> I'm struggling to see how this works given that the ->visit() callback
> is only called for leaf entries in the page-table. What am I missing?

Yeah, we should just go back to my v6 before I tried to re-use the
arm_lpae_read_and_clear_dirty() walker.. addressing review comments is
just making it worse since then

BR,
-R


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging
  2024-08-27 18:17 [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Rob Clark
                   ` (2 preceding siblings ...)
  2024-08-27 18:17 ` [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk Rob Clark
@ 2024-10-28 11:53 ` Mostafa Saleh
  2024-10-28 21:33   ` Rob Clark
  3 siblings, 1 reply; 8+ messages in thread
From: Mostafa Saleh @ 2024-10-28 11:53 UTC (permalink / raw)
  To: Rob Clark
  Cc: iommu, Will Deacon, Robin Murphy, Rob Clark, Boris Brezillon,
	open list:DRM DRIVER for Qualcomm Adreno GPUs,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, Jason Gunthorpe,
	Joao Martins, Kevin Tian, Konrad Dybcio,
	moderated list:ARM SMMU DRIVERS,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, open list,
	open list:SUSPEND TO RAM, Marijn Suijten, Rafael J. Wysocki,
	Sean Paul, Steven Price

Hi Rob,

On Tue, Aug 27, 2024 at 11:17:08AM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> This series extends io-pgtable-arm with a method to retrieve the page
> table entries traversed in the process of address translation, and then
> beefs up drm/msm gpu devcore dump to include this (and additional info)
> in the devcore dump.
> 
> This is a respin of https://patchwork.freedesktop.org/series/94968/
> (minus a patch that was already merged)
> 
> v2: Fix an armv7/32b build error in the last patch
> v3: Incorperate Will Deacon's suggestion to make the interface
>     callback based.
> v4: Actually wire up the callback
> v5: Drop the callback approach
> v6: Make walk-data struct pgtable specific and rename
>     io_pgtable_walk_data to arm_lpae_io_pgtable_walk_data
> v7: Re-use the pgtable walker added for arm_lpae_read_and_clear_dirty()
> v8: Pass pte pointer to callback so it can modify the actual pte
> v9: Fix selftests_running case
> 
> Rob Clark (4):
>   iommu/io-pgtable-arm: Make pgtable walker more generic
>   iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys
>   iommu/io-pgtable-arm: Add way to debug pgtable walk
>   drm/msm: Extend gpu devcore dumps with pgtbl info

Do you have plans to post another version of this series, as I am
working on some patches, that would use some of the common page walk
logic, so it would be more convenient to have them upstream.
Otherwise, I can have your series as a dependency.

Thanks,
Mostafa


> 
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  10 ++
>  drivers/gpu/drm/msm/msm_gpu.c           |   9 ++
>  drivers/gpu/drm/msm/msm_gpu.h           |   8 ++
>  drivers/gpu/drm/msm/msm_iommu.c         |  22 ++++
>  drivers/gpu/drm/msm/msm_mmu.h           |   3 +-
>  drivers/iommu/io-pgtable-arm.c          | 149 +++++++++++++++---------
>  include/linux/io-pgtable.h              |  15 +++
>  7 files changed, 160 insertions(+), 56 deletions(-)
> 
> -- 
> 2.46.0
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging
  2024-10-28 11:53 ` [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Mostafa Saleh
@ 2024-10-28 21:33   ` Rob Clark
  0 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2024-10-28 21:33 UTC (permalink / raw)
  To: Mostafa Saleh
  Cc: Rob Clark, iommu, Will Deacon, Robin Murphy, Boris Brezillon,
	open list:DRM DRIVER for Qualcomm Adreno GPUs,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, Jason Gunthorpe,
	Joao Martins, Kevin Tian, Konrad Dybcio,
	moderated list:ARM SMMU DRIVERS,
	open list:DRM DRIVER for Qualcomm Adreno GPUs, open list,
	open list:SUSPEND TO RAM, Marijn Suijten, Rafael J. Wysocki,
	Sean Paul, Steven Price

On Mon, Oct 28, 2024 at 4:53 AM Mostafa Saleh <smostafa@google.com> wrote:
>
> Hi Rob,
>
> On Tue, Aug 27, 2024 at 11:17:08AM -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > This series extends io-pgtable-arm with a method to retrieve the page
> > table entries traversed in the process of address translation, and then
> > beefs up drm/msm gpu devcore dump to include this (and additional info)
> > in the devcore dump.
> >
> > This is a respin of https://patchwork.freedesktop.org/series/94968/
> > (minus a patch that was already merged)
> >
> > v2: Fix an armv7/32b build error in the last patch
> > v3: Incorperate Will Deacon's suggestion to make the interface
> >     callback based.
> > v4: Actually wire up the callback
> > v5: Drop the callback approach
> > v6: Make walk-data struct pgtable specific and rename
> >     io_pgtable_walk_data to arm_lpae_io_pgtable_walk_data
> > v7: Re-use the pgtable walker added for arm_lpae_read_and_clear_dirty()
> > v8: Pass pte pointer to callback so it can modify the actual pte
> > v9: Fix selftests_running case
> >
> > Rob Clark (4):
> >   iommu/io-pgtable-arm: Make pgtable walker more generic
> >   iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys
> >   iommu/io-pgtable-arm: Add way to debug pgtable walk
> >   drm/msm: Extend gpu devcore dumps with pgtbl info
>
> Do you have plans to post another version of this series, as I am
> working on some patches, that would use some of the common page walk
> logic, so it would be more convenient to have them upstream.
> Otherwise, I can have your series as a dependency.

Sorry, this had dropped off my radar, I'll post an updated iteration
in a couple minutes.

BR,
-R

> Thanks,
> Mostafa
>
>
> >
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.c |  10 ++
> >  drivers/gpu/drm/msm/msm_gpu.c           |   9 ++
> >  drivers/gpu/drm/msm/msm_gpu.h           |   8 ++
> >  drivers/gpu/drm/msm/msm_iommu.c         |  22 ++++
> >  drivers/gpu/drm/msm/msm_mmu.h           |   3 +-
> >  drivers/iommu/io-pgtable-arm.c          | 149 +++++++++++++++---------
> >  include/linux/io-pgtable.h              |  15 +++
> >  7 files changed, 160 insertions(+), 56 deletions(-)
> >
> > --
> > 2.46.0
> >


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-10-28 21:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-27 18:17 [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Rob Clark
2024-08-27 18:17 ` [PATCH v9 1/4] iommu/io-pgtable-arm: Make pgtable walker more generic Rob Clark
2024-08-27 18:17 ` [PATCH v9 2/4] iommu/io-pgtable-arm: Re-use the pgtable walk for iova_to_phys Rob Clark
2024-08-27 18:17 ` [PATCH v9 3/4] iommu/io-pgtable-arm: Add way to debug pgtable walk Rob Clark
2024-08-30 13:20   ` Will Deacon
2024-08-30 14:32     ` Rob Clark
2024-10-28 11:53 ` [PATCH v9 0/4] io-pgtable-arm + drm/msm: Extend iova fault debugging Mostafa Saleh
2024-10-28 21:33   ` Rob Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).