Re: [PATCH 1/2] drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()
Hi Brajesh,
On Wed, 2026-09-02 at 15:25 +0530, Brajesh Gupta wrote:
> 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 0008
> [ 1179.295067] Mem abort info:
> [ 1179.297877] ESR = 0x9604
> [ 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 = 0x0004, ISS2 = 0x
> [ 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=9822a000
> [ 1179.343402] [0008] pgd=, p4d=
> [ 1179.350243] Internal error: Oops: 9604 [#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: 0005 (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 : 8000839ab8c0
> [ 1179.420153] x29: 8000839ab8c0 x28: 0001 x27:
> 8f386000
> [ 1179.427283] x26: 16d1df98 x25: 00247000 x24:
> 01e6
> [ 1179.434413] x23: 0002 x22: x21:
> 0247
> [ 1179.441540] x20: 0245 x19: 16d1df60 x18:
> 0002
> [ 1179.448668] x17: x16: x15:
> 0001
> [ 1179.455793] x14: 00060810 x13: 80007fff x12:
> 04190480
> [ 1179.462921] x11: 8000853f7000 x10: 8000811ae000 x9 :
> 041900b8
> [ 1179.470051] x8 : x7 : 990c4001 x6 :
> 0007
> [ 1179.477177] x5 : 16d1df60 x4 : x3 :
> 0a7d8000
> [ 1179.484306] x2 : 01ff x1 : x0 :
>
> [ 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 ]---
>
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
I'd say this should also have the stable tag (that could also be added while
applying the patch if you forget to do so).
> Signed-off-by: Brajesh Gupta
> ---
> 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
> +#include
> #include
> #include
> #include
> @@ -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",
nit: Could you break down this a bit e.g. by
Re: [PATCH 1/2] drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()
On Wed, 2026-09-02 at 15:25 +0530, Brajesh Gupta wrote:
Hi Brajesh,
> *** NOTE: This is an internal email from Imagination Technologies ***
>
>
>
>
> 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 0008
> [ 1179.295067] Mem abort info:
> [ 1179.297877] ESR = 0x9604
> [ 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 = 0x0004, ISS2 = 0x
> [ 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=9822a000
> [ 1179.343402] [0008] pgd=,
> p4d=
> [ 1179.350243] Internal error: Oops: 9604 [#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: 0005 (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 : 8000839ab8c0
> [ 1179.420153] x29: 8000839ab8c0 x28: 0001 x27:
> 8f386000
> [ 1179.427283] x26: 16d1df98 x25: 00247000 x24:
> 01e6
> [ 1179.434413] x23: 0002 x22: x21:
> 0247
> [ 1179.441540] x20: 0245 x19: 16d1df60 x18:
> 0002
> [ 1179.448668] x17: x16: x15:
> 0001
> [ 1179.455793] x14: 00060810 x13: 80007fff x12:
> 04190480
> [ 1179.462921] x11: 8000853f7000 x10: 8000811ae000 x9 :
> 041900b8
> [ 1179.470051] x8 : x7 : 990c4001 x6 :
> 0007
> [ 1179.477177] x5 : 16d1df60 x4 : x3 :
> 0a7d8000
> [ 1179.484306] x2 : 01ff x1 : x0 :
>
> [ 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 ]---
>
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Brajesh Gupta
> ---
> 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
> +#include
> #include
> #include
> #include
> @@ -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;
> }
>
> --
>
Re: [PATCH 1/2] drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl()
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 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/[email protected]?part=1
