Re: [PATCH v2 3/5] drm/xe/vram: revamp CPU VRAM mapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The commit intentionally leaves the global VRAM mapping (`xe->mem.vram->mapping`) uninitialized (NULL), but the display panic handler (`xe_panic_page_set_pixel()`) still attempts to use it. This will cause an invalid memory access during a kernel panic, resulting in a nested fault and preventing the panic screen from being drawn.
--

commit c5769f31bf8cec4dae25246aba506ccb4445b29b
Author: Matthew Auld <matthew.auld@intel.com>

    drm/xe/vram: revamp CPU VRAM mapping
    
    Previously, we called devm_ioremap_wc() for the entire PCI LMEM BAR (which
    can be significantly larger than usable memory, e.g., mapping 16G for a
    10G card), and then simply assigned subsets of this global mapping to each
    tile. By moving the devm_ioremap_wc() call into vram_region_init() and
    mapping on a per-tile basis, we restrict the virtual address space to exactly
    the usable_size of each tile. As a consequence the global vram->mapping is
    now NULL.

> diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
> index 56cff1e44530b..04d831b101bd1 100644
> --- a/drivers/gpu/drm/xe/xe_vram.c
> +++ b/drivers/gpu/drm/xe/xe_vram.c
[ ... ]
> @@ -257,8 +254,15 @@ static int vram_region_init(struct xe_device *xe, struct xe_vram_region *vram,
>  		return -ENODEV;
>  	}
>  
> +	if (vram != xe->mem.vram) {
> +		struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +
> +		vram->mapping = devm_ioremap_wc(&pdev->dev, vram->io_start, vram->io_size);
> +		if (!vram->mapping)
> +			return -ENOMEM;
> +	}
> +
>  	vram->dpa_base = lmem_bar->dpa_base + offset;
> -	vram->mapping = lmem_bar->mapping + offset;
>  	vram->usable_size = usable_size;

[Severity: High]
Does this change break the drm_panic display handler by leaving the global
xe->mem.vram->mapping as NULL?

Looking at drivers/gpu/drm/xe/display/xe_panic.c, xe_panic_page_set_pixel()
appears to explicitly use the global vram->mapping to compute the IOMEM
address for drawing pixels when the framebuffer is in VRAM:

		if (xe_bo_is_vram(bo)) {
			/* Display is always mapped on root tile */
			struct xe_vram_region *vram = xe_bo_device(bo)->mem.vram;

			if (panic->page < 0 || new_page < panic->page) {
				xe_res_first(bo->ttm.resource, new_page * PAGE_SIZE,
					     bo->ttm.base.size - new_page * PAGE_SIZE, &panic->res);
			} else {
				xe_res_next(&panic->res, PAGE_SIZE * (new_page - panic->page));
			}
			iosys_map_set_vaddr_iomem(&panic->vmap,
						  vram->mapping + panic->res.start);

If vram->mapping is NULL, this could cause an invalid memory access when
dereferenced by iosys_map_wr() during a kernel panic, resulting in a nested
fault and preventing the panic screen from being displayed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902103931.826063-7-matthew.auld@intel.com?part=3