Re: [PATCH v2 5/7] drm/xe/vram: Allow splitting a region into pools

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe/vram: kernel pool bounds break fixed-address BIOS takeover
--

commit 822201337f9b8f0df54e932ff7b27d938393e680
Author: Piotr Piórkowski <piotr.piorkowski@intel.com>
drm/xe/vram: Allow splitting a region into pools

This commit adds xe_vram_pool_create() to carve out dedicated pools
from the low addresses of a VRAM region, intended for kernel allocations.

> diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
> index ec95b53bc53b1..1df4f2514afc0 100644
> --- a/drivers/gpu/drm/xe/xe_vram.c
> +++ b/drivers/gpu/drm/xe/xe_vram.c
[ ... ]
> @@ -358,19 +371,88 @@ static int vram_region_init(struct xe_device *xe, struct xe_vram_region *vram,
> +/**
> + * xe_vram_pool_create - Add a pool to a VRAM region
> + * @vram: the VRAM region
> + * @purpose: the purpose served by the new pool
> + * @size: the size of the new pool
> + *
> + * Add a pool at the bottom of @vram and shrink the default pool accordingly.
> + * Existing non-default pools stay at the bottom in creation order. All pools
> + * share the region, the TTM manager and the placement, while the VRAM manager
> + * backs each of them with a buddy allocator of its own.
> + *
> + * The bottom of a region is also where its CPU visible window starts, so on
> + * a small BAR device the new pool takes CPU visible memory away from the
> + * default pool.
> + *
> + * Creating a pool does not make allocations use it. BO placement resolves
> + * pools through the tile, so the caller has to publish the new pool there,
> + * e.g. in &xe_tile.mem.kernel_vram for a kernel-only pool.
> + *
> + * Return: the new pool, or NULL if the pool cannot be created, in particular
> + * once the TTM manager of @vram is initialized.
> + */
> +struct xe_vram_pool *xe_vram_pool_create(struct xe_vram_region *vram,
> +					 enum xe_vram_pool_purpose purpose, u64 size)

[Severity: Medium]
Will this kernel pool partitioning break the initial display plane takeover?

Currently, initial_plane_bo() allocates the pre-existing BIOS framebuffer
at a fixed physical address using ttm_bo_type_kernel:

drivers/gpu/drm/xe/display/xe_initial_plane.c:initial_plane_bo() {
    ...
    bo = xe_bo_create_pin_map_at_novm(xe, tile0, size, phys_base,
                                      ttm_bo_type_kernel, flags, 0, false);
    ...
}

When __xe_bo_fixed_placement() processes this, it intersects the fixed
placement address with the kernel pool bounds using xe_vram_pool_place():

drivers/gpu/drm/xe/xe_bo.c:__xe_bo_fixed_placement() {
    ...
    xe_vram_pool_place(bo_vram_pool(xe, flags, vram_flag, type), place);
    if (place->lpfn && place->fpfn >= place->lpfn)
        return -EINVAL;
    ...
}

Since the kernel pool is a small carve-out at the bottom of VRAM, if the BIOS
framebuffer resides outside this pool, clamping its bounds will result in
place->fpfn >= place->lpfn, failing the allocation with -EINVAL.

How should fixed-address allocations outside the kernel pool be handled when
they use ttm_bo_type_kernel?

> +{
> +	struct xe_device *xe = vram->xe;
> +	struct xe_vram_pool *pool;
> +	u64 offset;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005093021.3741599-1-piotr.piorkowski@intel.com?part=5