mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series
@ 2026-09-27  8:25 Gyeyoung Baek
  2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Gyeyoung Baek @ 2026-09-27  8:25 UTC (permalink / raw)
  To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: imagination, dri-devel, linux-kernel, Gyeyoung Baek

These three patches fix pre-existing bugs in this driver that a VM_BIND
series depends on. They address what Sashiko, an automated review tool,
flagged while reviewing that series (see Link below).

tests/imagination/pvr_vm_map.c, sent separately to igt-dev (see Link
below) and not yet merged, reproduces all three bugs and shows they are
fixed by these patches.

Link: https://lore.kernel.org/all/20260817-pvr-vm-bind-v1-0-0a0f21be7d38@gmail.com/ # VM_BIND series Sashiko reviewed
Link: https://lore.kernel.org/all/20260926152341.783867-1-gye976@gmail.com/ # IGT reproducer (not yet merged)
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
Gyeyoung Baek (3):
      drm/imagination: Fix reference and vm_bo handling in remap()
      drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error
      drm/imagination: Size page table preallocation by device address

 drivers/gpu/drm/imagination/pvr_mmu.c | 29 +++++++++++++++--------------
 drivers/gpu/drm/imagination/pvr_mmu.h |  3 ++-
 drivers/gpu/drm/imagination/pvr_vm.c  | 13 +++++++------
 3 files changed, 24 insertions(+), 21 deletions(-)
---
base-commit: 98c7fc4219b93f953ee85c07fb6c8256046aba73
change-id: 20260927-pvr-fixes-a-40876d8bbb25

Best regards,
--  
Gyeyoung Baek <gye976@gmail.com>


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

* [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap()
  2026-09-27  8:25 [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series Gyeyoung Baek
@ 2026-09-27  8:25 ` Gyeyoung Baek
  2026-09-28  9:10   ` Brajesh Gupta
  2026-09-27  8:25 ` [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error Gyeyoung Baek
  2026-09-27  8:25 ` [PATCH 3/3] drm/imagination: Size page table preallocation by device address Gyeyoung Baek
  2 siblings, 1 reply; 8+ messages in thread
From: Gyeyoung Baek @ 2026-09-27  8:25 UTC (permalink / raw)
  To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: imagination, dri-devel, linux-kernel, Gyeyoung Baek

When a map overlaps part of an existing mapping, pvr_vm_gpuva_remap()
splits the mapping into prev/next parts covering what the request did
not take, instead of creating something new. It gets two things wrong.

- A GEM reference is taken for each part but it's never needed and never
  dropped, so it leaks one reference per split (remap-next-in-2m in
  tests/imagination/pvr_vm_map.c):

    CRITICAL: 33558528 bytes of shmem still held after close

- The parts still belong to the original object being split, but
  pvr_vm_gpuva_remap() links them to ctx->gpuvm_bo, the new object's
  vm_bo:

    prev_va --obj--> BO_A          BO_B <--obj-- vm_bo (ctx->gpuvm_bo)
            \___________link___________/  (mismatch: BO_A != BO_B)

  drm_gpuva_link() catches the mismatch:

    WARNING: drivers/gpu/drm/drm_gpuvm.c:2108 at drm_gpuva_link+0x2ec/0x310
      drm_WARN_ON(obj != vm_bo->obj)
    Call trace:
     drm_gpuva_link
     pvr_vm_gpuva_remap
     __drm_gpuvm_sm_map
     pvr_vm_map
     pvr_ioctl_vm_map

  Link them to op->remap.unmap->va->vm_bo instead.

The locking of the GPUVA lists of the other objects touched by a split is
not addressed here; it is handled by switching the GPUVM to immediate
mode.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/gpu/drm/imagination/pvr_vm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index ceb78694cd9..c5ae0b79fe6 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -418,6 +418,8 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
 static int
 pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
 {
+	/* The split parts belong to the object of the mapping being split. */
+	struct drm_gpuvm_bo *vm_bo = op->remap.unmap->va->vm_bo;
 	struct pvr_vm_bind_op *ctx = op_ctx;
 	u64 va_start = 0, va_range = 0;
 	int err;
@@ -433,14 +435,12 @@ pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
 	drm_gpuva_remap(&ctx->prev_va->base, &ctx->next_va->base, &op->remap);
 
 	if (op->remap.prev) {
-		pvr_gem_object_get(gem_to_pvr_gem(ctx->prev_va->base.gem.obj));
-		drm_gpuva_link(&ctx->prev_va->base, ctx->gpuvm_bo);
+		drm_gpuva_link(&ctx->prev_va->base, vm_bo);
 		ctx->prev_va = NULL;
 	}
 
 	if (op->remap.next) {
-		pvr_gem_object_get(gem_to_pvr_gem(ctx->next_va->base.gem.obj));
-		drm_gpuva_link(&ctx->next_va->base, ctx->gpuvm_bo);
+		drm_gpuva_link(&ctx->next_va->base, vm_bo);
 		ctx->next_va = NULL;
 	}
 

-- 
2.43.0


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

* [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error
  2026-09-27  8:25 [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series Gyeyoung Baek
  2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
@ 2026-09-27  8:25 ` Gyeyoung Baek
  2026-09-28  9:01   ` Brajesh Gupta
  2026-09-27  8:25 ` [PATCH 3/3] drm/imagination: Size page table preallocation by device address Gyeyoung Baek
  2 siblings, 1 reply; 8+ messages in thread
From: Gyeyoung Baek @ 2026-09-27  8:25 UTC (permalink / raw)
  To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: imagination, dri-devel, linux-kernel, Gyeyoung Baek

pvr_mmu_op_context_unmap_curr_page() is called here to clean up after an
error. Its own success overwrites that error, so success is returned.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/gpu/drm/imagination/pvr_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
index 3cac482e103..3e494bfa936 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.c
+++ b/drivers/gpu/drm/imagination/pvr_mmu.c
@@ -2553,7 +2553,7 @@ pvr_mmu_map_sgl(struct pvr_mmu_op_context *op_ctx, struct scatterlist *sgl,
 
 err_destroy_pages:
 	memcpy(&op_ctx->curr_page, &ptr_copy, sizeof(op_ctx->curr_page));
-	err = pvr_mmu_op_context_unmap_curr_page(op_ctx, page);
+	pvr_mmu_op_context_unmap_curr_page(op_ctx, page);
 
 	return err;
 }

-- 
2.43.0


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

* [PATCH 3/3] drm/imagination: Size page table preallocation by device address
  2026-09-27  8:25 [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series Gyeyoung Baek
  2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
  2026-09-27  8:25 ` [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error Gyeyoung Baek
@ 2026-09-27  8:25 ` Gyeyoung Baek
  2026-09-28  9:03   ` Brajesh Gupta
  2 siblings, 1 reply; 8+ messages in thread
From: Gyeyoung Baek @ 2026-09-27  8:25 UTC (permalink / raw)
  To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: imagination, dri-devel, linux-kernel, Gyeyoung Baek

pvr_mmu_op_context_create() preallocates the page tables a mapping
needs. How many is set by the device-virtual address being mapped, not
by the offset into the object's pages, which it used instead.

Size the preallocation from the device address instead.

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
 drivers/gpu/drm/imagination/pvr_mmu.c | 27 ++++++++++++++-------------
 drivers/gpu/drm/imagination/pvr_mmu.h |  3 ++-
 drivers/gpu/drm/imagination/pvr_vm.c  |  5 +++--
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
index 3e494bfa936..dd5f6d32864 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.c
+++ b/drivers/gpu/drm/imagination/pvr_mmu.c
@@ -2335,7 +2335,8 @@ void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx)
  * pvr_mmu_op_context_create() - Create an MMU op context.
  * @ctx: MMU context associated with owning VM context.
  * @sgt: Scatter gather table containing pages pinned for use by this context.
- * @sgt_offset: Start offset of the requested device-virtual memory mapping.
+ * @sgt_offset: Offset into @sgt of the start of the requested mapping.
+ * @device_addr: Device-virtual address at the start of the requested mapping.
  * @size: Size in bytes of the requested device-virtual memory mapping. For an
  * unmapping, this should be zero so that no page tables are allocated.
  *
@@ -2346,7 +2347,7 @@ void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx)
  */
 struct pvr_mmu_op_context *
 pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
-			  u64 sgt_offset, u64 size)
+			  u64 sgt_offset, u64 device_addr, u64 size)
 {
 	int err;
 
@@ -2362,18 +2363,18 @@ pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
 
 	if (size) {
 		/*
-		 * The number of page table objects we need to prealloc is
-		 * indicated by the mapping size, start offset and the sizes
-		 * of the areas mapped per PT or PD. The range calculation is
-		 * identical to that for the index into a table for a device
-		 * address, so we reuse those functions here.
+		 * The page tables needed are set by the device-virtual range
+		 * being mapped: one level 1 table per 1GiB region and one
+		 * level 0 table per 2MiB region the range touches. Tables that
+		 * already exist are not consumed, so this is an upper bound.
 		 */
-		const u32 l1_start_idx = pvr_page_table_l2_idx(sgt_offset);
-		const u32 l1_end_idx = pvr_page_table_l2_idx(sgt_offset + size);
-		const u32 l1_count = l1_end_idx - l1_start_idx + 1;
-		const u32 l0_start_idx = pvr_page_table_l1_idx(sgt_offset);
-		const u32 l0_end_idx = pvr_page_table_l1_idx(sgt_offset + size);
-		const u32 l0_count = l0_end_idx - l0_start_idx + 1;
+		const u64 last_addr = device_addr + size - 1;
+		const u64 l1_count =
+			(last_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) -
+			(device_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) + 1;
+		const u64 l0_count =
+			(last_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) -
+			(device_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) + 1;
 
 		/*
 		 * Alloc and push page table entries until we have enough of
diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
index a8ecd460168..7088f80805e 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.h
+++ b/drivers/gpu/drm/imagination/pvr_mmu.h
@@ -99,7 +99,8 @@ dma_addr_t pvr_mmu_get_root_table_dma_addr(struct pvr_mmu_context *ctx);
 void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx);
 struct pvr_mmu_op_context *
 pvr_mmu_op_context_create(struct pvr_mmu_context *ctx,
-			  struct sg_table *sgt, u64 sgt_offset, u64 size);
+			  struct sg_table *sgt, u64 sgt_offset,
+			  u64 device_addr, u64 size);
 
 int pvr_mmu_map(struct pvr_mmu_op_context *op_ctx, u64 size, u64 flags,
 		u64 device_addr);
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index c5ae0b79fe6..7bb58ca14e4 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -276,7 +276,8 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
 		goto err_bind_op_fini;
 
 	bind_op->mmu_op_ctx =
-		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, offset, size);
+		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, offset,
+					  device_addr, size);
 	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
 	if (err) {
 		bind_op->mmu_op_ctx = NULL;
@@ -318,7 +319,7 @@ pvr_vm_bind_op_unmap_init(struct pvr_vm_bind_op *bind_op,
 	}
 
 	bind_op->mmu_op_ctx =
-		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, 0, 0);
+		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, 0, 0, 0);
 	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
 	if (err) {
 		bind_op->mmu_op_ctx = NULL;

-- 
2.43.0


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

* Re: [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error
  2026-09-27  8:25 ` [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error Gyeyoung Baek
@ 2026-09-28  9:01   ` Brajesh Gupta
  2026-09-28  9:19     ` Gyeyoung Baek
  0 siblings, 1 reply; 8+ messages in thread
From: Brajesh Gupta @ 2026-09-28  9:01 UTC (permalink / raw)
  To: Luigi Santivetti, tzimmermann, simona, airlied, Alessio Belle,
	maarten.lankhorst, mripard, gye976
  Cc: dri-devel, imagination, linux-kernel

On Sun, 2026-09-27 at 17:25 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> pvr_mmu_op_context_unmap_curr_page() is called here to clean up after an
> error. Its own success overwrites that error, so success is returned.
> 
This issue is recently fixed under
https://patchwork.freedesktop.org/patch/755273/. Please have a look.

Thanks,
Brajesh

> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
>  drivers/gpu/drm/imagination/pvr_mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 3cac482e103..3e494bfa936 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2553,7 +2553,7 @@ pvr_mmu_map_sgl(struct pvr_mmu_op_context *op_ctx, struct scatterlist *sgl,
>  
>  err_destroy_pages:
>  	memcpy(&op_ctx->curr_page, &ptr_copy, sizeof(op_ctx->curr_page));
> -	err = pvr_mmu_op_context_unmap_curr_page(op_ctx, page);
> +	pvr_mmu_op_context_unmap_curr_page(op_ctx, page);
>  
>  	return err;
>  }
> 


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

* Re: [PATCH 3/3] drm/imagination: Size page table preallocation by device address
  2026-09-27  8:25 ` [PATCH 3/3] drm/imagination: Size page table preallocation by device address Gyeyoung Baek
@ 2026-09-28  9:03   ` Brajesh Gupta
  0 siblings, 0 replies; 8+ messages in thread
From: Brajesh Gupta @ 2026-09-28  9:03 UTC (permalink / raw)
  To: Luigi Santivetti, tzimmermann, simona, airlied, Alessio Belle,
	maarten.lankhorst, mripard, gye976
  Cc: dri-devel, imagination, linux-kernel

On Sun, 2026-09-27 at 17:25 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> pvr_mmu_op_context_create() preallocates the page tables a mapping
> needs. How many is set by the device-virtual address being mapped, not
> by the offset into the object's pages, which it used instead.
> 
> Size the preallocation from the device address instead.
> 
This issue is recently fixed under
https://patchwork.freedesktop.org/patch/755274/. Please have a look.

Thanks,
Brajesh
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
>  drivers/gpu/drm/imagination/pvr_mmu.c | 27 ++++++++++++++-------------
>  drivers/gpu/drm/imagination/pvr_mmu.h |  3 ++-
>  drivers/gpu/drm/imagination/pvr_vm.c  |  5 +++--
>  3 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 3e494bfa936..dd5f6d32864 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2335,7 +2335,8 @@ void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx)
>   * pvr_mmu_op_context_create() - Create an MMU op context.
>   * @ctx: MMU context associated with owning VM context.
>   * @sgt: Scatter gather table containing pages pinned for use by this context.
> - * @sgt_offset: Start offset of the requested device-virtual memory mapping.
> + * @sgt_offset: Offset into @sgt of the start of the requested mapping.
> + * @device_addr: Device-virtual address at the start of the requested mapping.
>   * @size: Size in bytes of the requested device-virtual memory mapping. For an
>   * unmapping, this should be zero so that no page tables are allocated.
>   *
> @@ -2346,7 +2347,7 @@ void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx)
>   */
>  struct pvr_mmu_op_context *
>  pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
> -			  u64 sgt_offset, u64 size)
> +			  u64 sgt_offset, u64 device_addr, u64 size)
>  {
>  	int err;
>  
> @@ -2362,18 +2363,18 @@ pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
>  
>  	if (size) {
>  		/*
> -		 * The number of page table objects we need to prealloc is
> -		 * indicated by the mapping size, start offset and the sizes
> -		 * of the areas mapped per PT or PD. The range calculation is
> -		 * identical to that for the index into a table for a device
> -		 * address, so we reuse those functions here.
> +		 * The page tables needed are set by the device-virtual range
> +		 * being mapped: one level 1 table per 1GiB region and one
> +		 * level 0 table per 2MiB region the range touches. Tables that
> +		 * already exist are not consumed, so this is an upper bound.
>  		 */
> -		const u32 l1_start_idx = pvr_page_table_l2_idx(sgt_offset);
> -		const u32 l1_end_idx = pvr_page_table_l2_idx(sgt_offset + size);
> -		const u32 l1_count = l1_end_idx - l1_start_idx + 1;
> -		const u32 l0_start_idx = pvr_page_table_l1_idx(sgt_offset);
> -		const u32 l0_end_idx = pvr_page_table_l1_idx(sgt_offset + size);
> -		const u32 l0_count = l0_end_idx - l0_start_idx + 1;
> +		const u64 last_addr = device_addr + size - 1;
> +		const u64 l1_count =
> +			(last_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) -
> +			(device_addr >> ROGUE_MMUCTRL_VADDR_PC_INDEX_SHIFT) + 1;
> +		const u64 l0_count =
> +			(last_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) -
> +			(device_addr >> ROGUE_MMUCTRL_VADDR_PD_INDEX_SHIFT) + 1;
>  
>  		/*
>  		 * Alloc and push page table entries until we have enough of
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
> index a8ecd460168..7088f80805e 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.h
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.h
> @@ -99,7 +99,8 @@ dma_addr_t pvr_mmu_get_root_table_dma_addr(struct pvr_mmu_context *ctx);
>  void pvr_mmu_op_context_destroy(struct pvr_mmu_op_context *op_ctx);
>  struct pvr_mmu_op_context *
>  pvr_mmu_op_context_create(struct pvr_mmu_context *ctx,
> -			  struct sg_table *sgt, u64 sgt_offset, u64 size);
> +			  struct sg_table *sgt, u64 sgt_offset,
> +			  u64 device_addr, u64 size);
>  
>  int pvr_mmu_map(struct pvr_mmu_op_context *op_ctx, u64 size, u64 flags,
>  		u64 device_addr);
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index c5ae0b79fe6..7bb58ca14e4 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -276,7 +276,8 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
>  		goto err_bind_op_fini;
>  
>  	bind_op->mmu_op_ctx =
> -		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, offset, size);
> +		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, sgt, offset,
> +					  device_addr, size);
>  	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
>  	if (err) {
>  		bind_op->mmu_op_ctx = NULL;
> @@ -318,7 +319,7 @@ pvr_vm_bind_op_unmap_init(struct pvr_vm_bind_op *bind_op,
>  	}
>  
>  	bind_op->mmu_op_ctx =
> -		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, 0, 0);
> +		pvr_mmu_op_context_create(vm_ctx->mmu_ctx, NULL, 0, 0, 0);
>  	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
>  	if (err) {
>  		bind_op->mmu_op_ctx = NULL;
> 


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

* Re: [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap()
  2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
@ 2026-09-28  9:10   ` Brajesh Gupta
  0 siblings, 0 replies; 8+ messages in thread
From: Brajesh Gupta @ 2026-09-28  9:10 UTC (permalink / raw)
  To: Luigi Santivetti, tzimmermann, simona, airlied, Alessio Belle,
	maarten.lankhorst, mripard, gye976
  Cc: dri-devel, imagination, linux-kernel

On Sun, 2026-09-27 at 17:25 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> When a map overlaps part of an existing mapping, pvr_vm_gpuva_remap()
> splits the mapping into prev/next parts covering what the request did
> not take, instead of creating something new. It gets two things wrong.
> 
> - A GEM reference is taken for each part but it's never needed and never
>   dropped, so it leaks one reference per split (remap-next-in-2m in
>   tests/imagination/pvr_vm_map.c):
> 
>     CRITICAL: 33558528 bytes of shmem still held after close
> 
> - The parts still belong to the original object being split, but
>   pvr_vm_gpuva_remap() links them to ctx->gpuvm_bo, the new object's
>   vm_bo:
> 
>     prev_va --obj--> BO_A          BO_B <--obj-- vm_bo (ctx->gpuvm_bo)
>             \___________link___________/  (mismatch: BO_A != BO_B)
> 
>   drm_gpuva_link() catches the mismatch:
> 
>     WARNING: drivers/gpu/drm/drm_gpuvm.c:2108 at drm_gpuva_link+0x2ec/0x310
>       drm_WARN_ON(obj != vm_bo->obj)
>     Call trace:
>      drm_gpuva_link
>      pvr_vm_gpuva_remap
>      __drm_gpuvm_sm_map
>      pvr_vm_map
>      pvr_ioctl_vm_map
> 
>   Link them to op->remap.unmap->va->vm_bo instead.
> 
> The locking of the GPUVA lists of the other objects touched by a split is
> not addressed here; it is handled by switching the GPUVM to immediate
> mode.
> 
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>

Reviewed-by: Brajesh Gupta <brajesh.gupta@imgtec.com>

Thanks,
Brajesh
> ---
>  drivers/gpu/drm/imagination/pvr_vm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index ceb78694cd9..c5ae0b79fe6 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -418,6 +418,8 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
>  static int
>  pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
>  {
> +	/* The split parts belong to the object of the mapping being split. */
> +	struct drm_gpuvm_bo *vm_bo = op->remap.unmap->va->vm_bo;
>  	struct pvr_vm_bind_op *ctx = op_ctx;
>  	u64 va_start = 0, va_range = 0;
>  	int err;
> @@ -433,14 +435,12 @@ pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
>  	drm_gpuva_remap(&ctx->prev_va->base, &ctx->next_va->base, &op->remap);
>  
>  	if (op->remap.prev) {
> -		pvr_gem_object_get(gem_to_pvr_gem(ctx->prev_va->base.gem.obj));
> -		drm_gpuva_link(&ctx->prev_va->base, ctx->gpuvm_bo);
> +		drm_gpuva_link(&ctx->prev_va->base, vm_bo);
>  		ctx->prev_va = NULL;
>  	}
>  
>  	if (op->remap.next) {
> -		pvr_gem_object_get(gem_to_pvr_gem(ctx->next_va->base.gem.obj));
> -		drm_gpuva_link(&ctx->next_va->base, ctx->gpuvm_bo);
> +		drm_gpuva_link(&ctx->next_va->base, vm_bo);
>  		ctx->next_va = NULL;
>  	}
>  
> 


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

* Re: [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error
  2026-09-28  9:01   ` Brajesh Gupta
@ 2026-09-28  9:19     ` Gyeyoung Baek
  0 siblings, 0 replies; 8+ messages in thread
From: Gyeyoung Baek @ 2026-09-28  9:19 UTC (permalink / raw)
  To: Brajesh Gupta
  Cc: Luigi Santivetti, tzimmermann, simona, airlied, Alessio Belle,
	maarten.lankhorst, mripard, dri-devel, imagination, linux-kernel

Hi, Thanks for pointing that out, I should have rechecked the tree before
sending v2.
Sorry for the noise!


> This issue is recently fixed under
> https://patchwork.freedesktop.org/patch/755273/. Please have a look.
>

--
Thanks,
Gyeyoung

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

end of thread, other threads:[~2026-09-28  9:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-27  8:25 [PATCH 0/3] drm/imagination: Fix pre-existing bugs flagged by Sashiko's review of a VM_BIND series Gyeyoung Baek
2026-09-27  8:25 ` [PATCH 1/3] drm/imagination: Fix reference and vm_bo handling in remap() Gyeyoung Baek
2026-09-28  9:10   ` Brajesh Gupta
2026-09-27  8:25 ` [PATCH 2/3] drm/imagination: Fix pvr_mmu_map_sgl() overwriting its own error Gyeyoung Baek
2026-09-28  9:01   ` Brajesh Gupta
2026-09-28  9:19     ` Gyeyoung Baek
2026-09-27  8:25 ` [PATCH 3/3] drm/imagination: Size page table preallocation by device address Gyeyoung Baek
2026-09-28  9:03   ` Brajesh Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®