On Thu, 2026-10-01 at 22:39 +0900, Gyeyoung Baek wrote:
Hi Gyeyoung,
> Commit 0a8224058a58 ("drm/imagination: Fix page count for page table for
> map() interface") passed the device address to
> pvr_mmu_op_context_create(), but the preallocation is still sized from
> device_addr + sgt_offset with an exclusive end. The offset into the
> object's pages has no place in a device-virtual range, and the exclusive
> end allocates one table too many when the range ends on a table
> boundary.
>
> Count the tables from device_addr and the inclusive end of the range.
>
> Fixes: 0a8224058a58 ("drm/imagination: Fix page count for page table for
> map() interface")
> Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
> Signed-off-by: Gyeyoung Baek <[email protected]>
> ---
> drivers/gpu/drm/imagination/pvr_mmu.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c
> b/drivers/gpu/drm/imagination/pvr_mmu.c
> index 62eae7fcd5a..67e73d7d5d0 100644
> --- a/drivers/gpu/drm/imagination/pvr_mmu.c
> +++ b/drivers/gpu/drm/imagination/pvr_mmu.c
> @@ -2337,7 +2337,7 @@ void pvr_mmu_op_context_destroy(struct
> pvr_mmu_op_context *op_ctx)
> * @ctx: MMU context associated with owning VM context.
> * @sgt: Scatter gather table containing pages pinned for use by this
> context.
> * @device_addr: Virtual device address at the start of the requested
> mapping.
> - * @sgt_offset: Start offset of the requested device-virtual memory mapping.
> + * @sgt_offset: Offset into @sgt of 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.
> *
> @@ -2350,7 +2350,6 @@ struct pvr_mmu_op_context *
> pvr_mmu_op_context_create(struct pvr_mmu_context *ctx, struct sg_table *sgt,
> 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);
> @@ -2365,18 +2364,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 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.
> + * 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(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(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;
> + 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;
Shouldn't we use 'device_addr + sgt_offset' instead of just 'device_addr' as
start address for l0/l1 count calculation? Only account for requested mapping
instead of whole memory.
Thanks,
Brajesh
>
> /*
> * Alloc and push page table entries until we have enough of
>
