* [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®