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

[Matthew Auld <matthew.auld@intel.com>]

On 02/09/2026 11:49, sashiko-bot@kernel.org wrote:
> 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.

Indeed. I think we can just go via the root tile instead.

>