Changes down below:
On Mon, 2026-08-17 at 14:50 +0800, Junrui Luo via B4 Relay wrote:
> From: Junrui Luo <[email protected]>
>
> A new mapping takes over the page tables of the mappings it replaces.
> nouveau_uvmm_sm_prepare() only acquires page tables for the range no
> existing mapping covers, and the map path frees the replaced mappings
> without putting their references. That is only valid while all of
> them
> use the same page size, which select_page_shift() no longer
> guarantees.
>
> Rebinding a GART BO over a 2MiB VRAM BO therefore leaves the new
> mapping
> owning page tables built for a different page size, and it then maps
> at a
> size that was never referenced over that range. Since raw map does
> not
> allocate, nvkm_vmm_iter() can walk down to a NULL leaf and
> dereference
> it. The remainders of a split have the same problem: op_map_prepare()
> recomputes a page size with select_page_shift() while the remainder
> keeps
> the parent's page tables, so a parent that was itself downgraded can
> leave
> a remainder that re-aligns to a larger size. This happens on the
> unmap
> path too.
>
> Reject the bind, and make split remainders inherit the page size of
> the
> mapping they are split from.
>
> Fixes: c488a94e7e14 ("drm/nouveau/uvmm: Allow larger pages")
> Reported-by: Yuhao Jiang <[email protected]>
> Assisted-by: Claude:claude-opus-5
> Cc: [email protected]
> Signed-off-by: Junrui Luo <[email protected]>
> ---
> drivers/gpu/drm/nouveau/nouveau_uvmm.c | 33
> ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index f5e4756b4de4..6404c54d097c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -85,6 +85,8 @@ struct uvmm_map_args {
> u64 addr;
> u64 range;
> u8 kind;
> + /* Page size to give the new mapping, or 0 to derive it from
> the op. */
> + u8 page_shift;
> };
>
> static int
> @@ -655,7 +657,8 @@ op_map_prepare(struct nouveau_uvmm *uvmm,
>
> uvma->region = args->region;
> uvma->kind = args->kind;
> - uvma->page_shift = select_page_shift(uvmm, op);
> + uvma->page_shift = args->page_shift ? args->page_shift :
> + select_page_shift(uvmm, op);
This can just be:
args->page_shift ?: select_page_shift(uvmm, op);
>
> drm_gpuva_map(&uvmm->base, &uvma->va, op);
>
> @@ -684,8 +687,20 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm
> *uvmm,
> struct drm_gpuva_op *op;
> u64 vmm_get_start = args ? args->addr : 0;
> u64 vmm_get_end = args ? args->addr + args->range : 0;
> + u8 map_page_shift = 0;
> int ret;
>
> + /* A new mapping takes over the page tables of the mappings
> it replaces,
> + * so every one of them has to be using its page size. The
> new mapping
> + * is the last op drm_gpuvm_sm_map_ops_create() emits.
> + */
> + if (args) {
> + struct drm_gpuva_op *last = drm_gpuva_last_op(ops);
> +
> + if (last->op == DRM_GPUVA_OP_MAP)
> + map_page_shift = select_page_shift(uvmm,
> &last->map);
> + }
> +
> drm_gpuva_for_each_op(op, ops) {
> switch (op->op) {
> case DRM_GPUVA_OP_MAP: {
> @@ -713,11 +728,22 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm
> *uvmm,
> struct uvmm_map_args remap_args = {
> .kind = uvma_from_va(va)->kind,
> .region = uvma_from_va(va)->region,
> + /* The remainders of the split keep
> the page
> + * tables of the mapping they are
> split from,
> + * so they must keep its page size
> too.
> + */
> + .page_shift = uvma_from_va(va)-
> >page_shift,
> };
> u64 ustart = va->va.addr;
> u64 urange = va->va.range;
> u64 uend = ustart + urange;
>
> + if (map_page_shift &&
> + uvma_from_va(va)->page_shift !=
> map_page_shift) {
Let's just take this value from remap_args instead of doing
uvma_from_va(va), it makes it a bit easier for people to understand
what's happening here.
With those nitpicks fixed:
Reviewed-by: Lyude Paul <[email protected]>
> + ret = -EINVAL;
> + goto unwind;
> + }
> +
> op_unmap_prepare(r->unmap);
>
> if (r->prev) {
> @@ -756,6 +782,11 @@ nouveau_uvmm_sm_prepare(struct nouveau_uvmm
> *uvmm,
> u64 uend = ustart + urange;
> u8 page_shift = uvma_from_va(va)-
> >page_shift;
>
> + if (map_page_shift && page_shift !=
> map_page_shift) {
> + ret = -EINVAL;
> + goto unwind;
> + }
> +
> op_unmap_prepare(u);
>
> if (!args)
