[PATCH] drm/xe/pf: Clear all LMTT pages on alloc

[Michal Wajdeczko]

On 02.07.2025 10:02, Piotr Piórkowski wrote:
> Michal Wajdeczko <michal.wajdeczko at intel.com> wrote on śro [2025-lip-02 00:00:52 +0200]:
>> Our LMEM buffer objects are not cleared by default on alloc
>> and during VF provisioning we only setup LMTT PTEs for the
>> actually provisioned LMEM range. But beyond that valid range
>> we might leave some stale data that could either point to some
>> other VFs allocations or even to the PF pages.
>>
>> Explicitly clear all new LMTT page to avoid the risk that a
>> malicious VF would try to exploit that gap.
>>
>> While around add asserts to catch any undesired PTE overwrites
>> and low-level debug traces to track LMTT PT life-cycle.
>>
>> Fixes: b1d204058218 ("drm/xe/pf: Introduce Local Memory Translation Table")
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Cc: Lukasz Laguna <lukasz.laguna at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_lmtt.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_lmtt.c b/drivers/gpu/drm/xe/xe_lmtt.c
>> index b56437a816e4..381f576036d0 100644
>> --- a/drivers/gpu/drm/xe/xe_lmtt.c
>> +++ b/drivers/gpu/drm/xe/xe_lmtt.c
>> @@ -78,6 +78,9 @@ static struct xe_lmtt_pt *lmtt_pt_alloc(struct xe_lmtt *lmtt, unsigned int level
>>  	}
>>  
>>  	lmtt_assert(lmtt, xe_bo_is_vram(bo));
>> +	lmtt_debug(lmtt, "level=%u addr=%#llx\n", level, (u64)xe_bo_main_addr(bo, XE_PAGE_SIZE));
>> +
>> +	xe_map_memset(lmtt_to_xe(lmtt), &bo->vmap, 0, 0, xe_bo_size(bo));
> 
> We have a dedicated macro LMTT_PTE_INVALID
> Maybe it's worth using it here.

this would require to introduce  {xe,iosys}_map_memset64() helpers

but at this point we only care about BO being zeroed, not filled with
any specific value (like LMTT_PTE_INVALID which accidentally is also 0)

> 
> I wonder why we care about cleaning LMTT only during allocation and
> not in lmtt_drop_pages and lmtt_destroy_pt.

because at lmtt_drop_pages() we explicitly remove all references to
existing PTs by resetting root entry at the PD level, so HW can't walk
those pages anymore

> To be precise, in lmtt_drop_pages we do lmtt_write_pte(lmtt, pd, LMTT_PTE_INVALID, vfid),
> but from what I understand, this is missing at the lmtt_destroy_pt level.

the designed flow is:
 - call xe_lmtt_prepare_pages() to build PT tree with clear leaf PTs
 - call xe_lmtt_populate_pages() to populate leaf PT with PTEs
 - call xe_lmtt_populate_pages() to populate leaf PT with PTEs
 - ...
 - call xe_lmtt_drop_pages() to remove whole PT tree

since currently there is no way to selectively unmap any address range
or change the back-end BOs, thus there is no point in doing selective
clear of any PTE, except at the top level PD, which we do

> 
> Thanks,
> Piotr
>>  
>>  	pt->level = level;
>>  	pt->bo = bo;
>> @@ -91,6 +94,9 @@ static struct xe_lmtt_pt *lmtt_pt_alloc(struct xe_lmtt *lmtt, unsigned int level
>>  
>>  static void lmtt_pt_free(struct xe_lmtt_pt *pt)
>>  {
>> +	lmtt_debug(&pt->bo->tile->sriov.pf.lmtt, "level=%u addr=%llx\n",
>> +		   pt->level, (u64)xe_bo_main_addr(pt->bo, XE_PAGE_SIZE));
>> +
>>  	xe_bo_unpin_map_no_vm(pt->bo);
>>  	kfree(pt);
>>  }
>> @@ -226,9 +232,14 @@ static void lmtt_write_pte(struct xe_lmtt *lmtt, struct xe_lmtt_pt *pt,
>>  
>>  	switch (lmtt->ops->lmtt_pte_size(level)) {
>>  	case sizeof(u32):
>> +		lmtt_assert(lmtt, !overflows_type(pte, u32));
>> +		lmtt_assert(lmtt, !pte || !iosys_map_rd(&pt->bo->vmap, idx * sizeof(u32), u32));
>> +
>>  		xe_map_wr(lmtt_to_xe(lmtt), &pt->bo->vmap, idx * sizeof(u32), u32, pte);
>>  		break;
>>  	case sizeof(u64):
>> +		lmtt_assert(lmtt, !pte || !iosys_map_rd(&pt->bo->vmap, idx * sizeof(u64), u64));
>> +
>>  		xe_map_wr(lmtt_to_xe(lmtt), &pt->bo->vmap, idx * sizeof(u64), u64, pte);
>>  		break;
>>  	default:
>> -- 
>> 2.47.1
>>
>