[PATCH 0/2] Fixes for map() path

[PATCH 0/2] Fixes for map() path

[Brajesh Gupta]

Two changes in map() path fix a kernel crash and a map failure.

Patch 0 fixes error path handling which triggers a kernel crash.

Patch 1 fixes prealloc page table memory requirement for a mapping
which led to map failure due to running out of page table space.

Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
Brajesh Gupta (2):
      drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()
      drm/imagination: Fix page count for page table for map() interface

 drivers/gpu/drm/imagination/pvr_mmu.c | 19 ++++++++++++-------
 drivers/gpu/drm/imagination/pvr_mmu.h |  2 +-
 drivers/gpu/drm/imagination/pvr_vm.c  |  4 ++--
 3 files changed, 15 insertions(+), 10 deletions(-)
---
base-commit: bd4f284df04d76fd65e57141cb1e6e7a49e4c3cb
change-id: 20260902-mmu_fix-3923aca60f60

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>

[PATCH 1/2] drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()

[Brajesh Gupta]

Map failure from pvr_mmu_map_sgl() interface was not returned correctly
to pvr_mmu_map() interface. This resulted in pvr_mmu_map() interface to
continue instead of returning an error to caller.
Fix it by returning a proper error code from pvr_mmu_map_sgl() interface.

Call stack for crash:
[ 1179.286237] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
[ 1179.295067] Mem abort info:
[ 1179.297877]   ESR = 0x0000000096000004
[ 1179.301656]   EC = 0x25: DABT (current EL), IL = 32 bits
[ 1179.306987]   SET = 0, FnV = 0
[ 1179.310048]   EA = 0, S1PTW = 0
[ 1179.313198]   FSC = 0x04: level 0 translation fault
[ 1179.318096] Data abort info:
[ 1179.320993]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 1179.326483]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 1179.331546]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 1179.336895] user pgtable: 4k pages, 48-bit VAs, pgdp=000000009822a000
[ 1179.343402] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
[ 1179.350243] Internal error: Oops: 0000000096000004 [#2]  SMP
[ 1179.355908] Modules linked in: powervr gpu_sched drm_shmem_helper drm_gpuvm drm_exec xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils dwc3_am62 at24 sa2ul sha512 libsha512 sha256 authenc sch_fq_codel fuse dm_mod ipv6
[ 1179.378992] CPU: 1 UID: 1000 PID: 680 Comm: deqp-vk Tainted: G      D             6.17.0 #1 PREEMPT
[ 1179.388120] Tainted: [D]=DIE
[ 1179.390994] Hardware name: Texas Instruments AM625 SK (DT)
[ 1179.396467] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 1179.403415] pc : pvr_mmu_op_context_unmap_curr_page+0x6c/0x134 [powervr]
[ 1179.410140] lr : pvr_mmu_op_context_unmap_curr_page+0x58/0x134 [powervr]
[ 1179.416848] sp : ffff8000839ab8c0
[ 1179.420153] x29: ffff8000839ab8c0 x28: 0000000000000001 x27: 000000008f386000
[ 1179.427283] x26: ffff000016d1df98 x25: 0000000000247000 x24: 00000000000001e6
[ 1179.434413] x23: 0000000000000002 x22: 000000000000ffff x21: 0000000000000247
[ 1179.441540] x20: 0000000000000245 x19: ffff000016d1df60 x18: 0000000000000002
[ 1179.448668] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000001
[ 1179.455793] x14: 0000000000060810 x13: ffff80007fffffff x12: ffff000004190480
[ 1179.462921] x11: ffff8000853f7000 x10: ffff8000811ae000 x9 : ffff0000041900b8
[ 1179.470051] x8 : 0000000000000000 x7 : 00000000990c4001 x6 : 0000000000000007
[ 1179.477177] x5 : ffff000016d1df60 x4 : 0000000000000000 x3 : ffff00000a7d8000
[ 1179.484306] x2 : 00000000000001ff x1 : 0000000000000000 x0 : 0000000000000000
[ 1179.491433] Call trace:
[ 1179.493872]  pvr_mmu_op_context_unmap_curr_page+0x6c/0x134 [powervr] (P)
[ 1179.500582]  pvr_mmu_map+0x31c/0x388 [powervr]
[ 1179.505027]  pvr_vm_gpuva_map+0x40/0x88 [powervr]
[ 1179.509732]  __drm_gpuvm_sm_map+0x250/0x44c [drm_gpuvm]
[ 1179.514952]  drm_gpuvm_sm_map+0x48/0x5c [drm_gpuvm]
[ 1179.519822]  pvr_vm_bind_op_exec+0x64/0x70 [powervr]
[ 1179.524785]  pvr_vm_map+0x1f8/0x2a8 [powervr]
[ 1179.529142]  pvr_ioctl_vm_map+0x12c/0x188 [powervr]
[ 1179.534018]  drm_ioctl_kernel+0xb8/0x128
[ 1179.537941]  drm_ioctl+0x21c/0x4ec
[ 1179.541337]  __arm64_sys_ioctl+0xac/0x108
[ 1179.545344]  invoke_syscall+0x44/0x100
[ 1179.549091]  el0_svc_common.constprop.0+0x40/0xe0
[ 1179.553790]  do_el0_svc+0x1c/0x28
[ 1179.557106]  el0_svc+0x34/0xf0
[ 1179.560159]  el0t_64_sync_handler+0xd0/0xe4
[ 1179.564334]  el0t_64_sync+0x198/0x19c
[ 1179.567996] Code: 54000300 35000360 f9402261 79409a62 (f9400421)
[ 1179.574081] ---[ end trace 0000000000000000 ]---

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_mmu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
index 3cac482e1034..175f0ba4d993 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.c
+++ b/drivers/gpu/drm/imagination/pvr_mmu.c
@@ -12,6 +12,7 @@
 #include "pvr_rogue_mmu_defs.h"
 
 #include <drm/drm_drv.h>
+#include <drm/drm_print.h>
 #include <linux/atomic.h>
 #include <linux/bitops.h>
 #include <linux/dma-mapping.h>
@@ -2553,7 +2554,9 @@ 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);
+	if (pvr_mmu_op_context_unmap_curr_page(op_ctx, page))
+		drm_err(from_pvr_device(op_ctx->mmu_ctx->pvr_dev), "%s : Failure in unmapping pages\n",
+			__func__);
 
 	return err;
 }

-- 
2.43.0

[PATCH 2/2] drm/imagination: Fix page count for page table for map() interface

[Brajesh Gupta]

GPU virtual start address wasn't included in page count for page table
calculation for mapping an BO object in map() interface. It resulted in
map failure later due to not enough pages at L0/L1 level.
Update pvr_mmu_op_context_create() interface to pass device address as well
to allow correct calculation for page table memory.

Miscalculation of page table pages for mapping a BO starting at a device
address 0x8001b45000 of size 0x8ca000:
               old       new
L0 count       5          6
L1 count       1          1

Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_mmu.c | 14 ++++++++------
 drivers/gpu/drm/imagination/pvr_mmu.h |  2 +-
 drivers/gpu/drm/imagination/pvr_vm.c  |  4 ++--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
index 175f0ba4d993..52d8fbc00384 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.c
+++ b/drivers/gpu/drm/imagination/pvr_mmu.c
@@ -2336,6 +2336,7 @@ 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.
+ * @device_add: Virtual device address at the start of the requested mapping.
  * @sgt_offset: Start offset of the requested device-virtual memory 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.
@@ -2347,8 +2348,9 @@ 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 device_addr, u64 sgt_offset, u64 size)
 {
+	u64 start_addr = device_addr + sgt_offset;
 	int err;
 
 	struct pvr_mmu_op_context *op_ctx = kzalloc_obj(*op_ctx);
@@ -2364,16 +2366,16 @@ 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
+		 * indicated by the mapping size, start address 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.
 		 */
-		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_start_idx = pvr_page_table_l2_idx(start_addr);
+		const u32 l1_end_idx = pvr_page_table_l2_idx(start_addr + 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_start_idx = pvr_page_table_l1_idx(start_addr);
+		const u32 l0_end_idx = pvr_page_table_l1_idx(start_addr + size);
 		const u32 l0_count = l0_end_idx - l0_start_idx + 1;
 
 		/*
diff --git a/drivers/gpu/drm/imagination/pvr_mmu.h b/drivers/gpu/drm/imagination/pvr_mmu.h
index a8ecd460168d..2c02d61ba0a2 100644
--- a/drivers/gpu/drm/imagination/pvr_mmu.h
+++ b/drivers/gpu/drm/imagination/pvr_mmu.h
@@ -99,7 +99,7 @@ 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 device_addr, u64 sgt_offset, 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 396d349fb6ce..867a4a44958a 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -276,7 +276,7 @@ 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, device_addr, offset, size);
 	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
 	if (err) {
 		bind_op->mmu_op_ctx = NULL;
@@ -318,7 +318,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, device_addr, 0, 0);
 	err = PTR_ERR_OR_ZERO(bind_op->mmu_op_ctx);
 	if (err) {
 		bind_op->mmu_op_ctx = NULL;

-- 
2.43.0

Re: [PATCH 2/2] drm/imagination: Fix page count for page table for map() interface

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incorrect calculation of the virtual address range for page table preallocation due to mistakenly adding the physical buffer offset (`sgt_offset`) to the virtual device address (`device_addr`).

Pre-existing issues:
- [High] Massive integer underflow in page table count calculation when a mapping crosses a page table boundary leads to an infinite allocation loop and kernel memory exhaustion (OOM).
--

commit e3abd749cd49c287313d0450da2a3a8485c9e53f
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>

    drm/imagination: Fix page count for page table for map() interface
    
    GPU virtual start address wasn't included in page count for page table
    calculation for mapping an BO object in map() interface. It resulted in
    map failure later due to not enough pages at L0/L1 level.
    Update pvr_mmu_op_context_create() interface to pass device address as well
    to allow correct calculation for page table memory.

> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 175f0ba4d993f..52d8fbc003841 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2347,8 +2348,9 @@ 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 device_addr, u64 sgt_offset, u64 size)
>  {
> +	u64 start_addr = device_addr + sgt_offset;

[Severity: High]
Does this code incorrectly mix the virtual device address (device_addr) and
the physical buffer offset (sgt_offset) to compute the starting address?

If sgt_offset represents the offset within the backing buffer, adding it to
the virtual address skews the calculation for the required page tables. This
could cause valid userspace mapping requests to randomly fail with -ENOMEM
if the shifted range covers fewer page table boundaries than the actual
mapping, causing the driver to allocate too few tables.

[ ... ]

> @@ -2364,16 +2366,16 @@ 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
> +		 * indicated by the mapping size, start address 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.
>  		 */
> -		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_start_idx = pvr_page_table_l2_idx(start_addr);
> +		const u32 l1_end_idx = pvr_page_table_l2_idx(start_addr + 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_start_idx = pvr_page_table_l1_idx(start_addr);
> +		const u32 l0_end_idx = pvr_page_table_l1_idx(start_addr + size);
>  		const u32 l0_count = l0_end_idx - l0_start_idx + 1;

[Severity: High]
This is a pre-existing issue, but can this subtraction cause a massive
integer underflow when a mapping crosses a page table boundary?

Because pvr_page_table_l1_idx() and pvr_page_table_l2_idx() return local
indices bounded within their parent tables (e.g., 0-511), if the address range
crosses a boundary (1GB for L1 tables or 2MB for L0 tables), l0_end_idx could
be smaller than l0_start_idx (e.g., 0 and 511).

The subtraction l0_end_idx - l0_start_idx + 1 would evaluate to an unsigned
32-bit underflow.

When this happens, the subsequent loop for (int i = 0; i < l0_count; i++)
in pvr_mmu_op_context_create() iterates billions of times, exhausting kernel
memory via kzalloc and triggering the OOM killer. This is directly reachable
by unprivileged userspace via the GPUVM map IOCTL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-mmu_fix-v1-0-c55a23fb7cab@imgtec.com?part=2

Re: [PATCH 1/2] drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Latent NULL pointer dereference in `pvr_mmu_op_context_unmap_curr_page()` when unmapping missing page tables, due to `-ENXIO` being improperly swallowed.
--

commit 9512cb5ae9264ab1bc6ea5bf3b2a374fe88ffc0e
Author: Brajesh Gupta <brajesh.gupta@imgtec.com>

drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()

This commit ensures that map failures from pvr_mmu_map_sgl() are correctly
returned to the caller, preventing the pvr_mmu_map() interface from continuing
incorrectly. It also adds logging for page unmapping failures during cleanup.

> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 3cac482e10347..175f0ba4d993f 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c

[ ... ]

> @@ -2553,7 +2554,9 @@ 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);
> +	if (pvr_mmu_op_context_unmap_curr_page(op_ctx, page))

[Severity: High]
This is a pre-existing issue, but could this unmap call trigger a NULL pointer
dereference under certain conditions?

If an unmap operation spans an L1 boundary into an unmapped area where the
L0 table is missing, pvr_mmu_op_context_next_page() clears
op_ctx->curr_page.l0_table to NULL and calls pvr_mmu_op_context_load_tables().

In pvr_mmu_op_context_load_tables(), the -ENXIO error indicating a missing L0
table is swallowed:

	if (err == -ENXIO)
		err = 0;

Because it returns 0 instead of -ENXIO, the loop inside
pvr_mmu_op_context_unmap_curr_page() misses its guard:

	err = pvr_mmu_op_context_next_page(op_ctx, false);
	if (err == -ENXIO)
		continue;
	else if (err)
		return err;

	pvr_page_destroy(op_ctx);

This causes it to unconditionally proceed to pvr_page_destroy(), which then
dereferences the NULL l0_table when checking if the entry is valid.

Since this patch modifies the error handling around unmapping pages, is there
a way to address the swallowed -ENXIO so that sparse unmap requests or
rollbacks that cross into a missing L0 table skip properly instead of crashing?

> +		drm_err(from_pvr_device(op_ctx->mmu_ctx->pvr_dev), "%s : Failure in unmapping pages\n",
> +			__func__);
>  
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-mmu_fix-v1-0-c55a23fb7cab@imgtec.com?part=1