[Intel-xe] [PATCH 3/3] drm/xe/bo: sync kernel fences for KMD buffers

[Matthew Auld]

On 25/10/2023 22:00, Matthew Brost wrote:
> On Wed, Oct 25, 2023 at 06:39:41PM +0100, Matthew Auld wrote:
>> With things like pipelined evictions, VRAM pages can be marked as free
>> and yet still have some active kernel fences, with the idea that the
>> next caller to allocate the memory will respect them. However it looks
>> like we are missing synchronisation for KMD internal buffers, like
>> page-tables, lrc etc. For userspace objects we should already have the
>> required serialisation for CPU access via the fault handler, and
>> likewise for GPU access when vm_binding them.
>>
>> To fix this serialise against any kernel fences for all KMD objects at
>> creation. This seems to resolve some severe corruption seen during
>> evictions.
>>
>> Closes: ?
>> Testcase: igt at xe-evict-ccs
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
>> Cc: Matthew Brost <matthew.brost at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_bo.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index 61789c0e88fb..26a103aa5d48 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -1272,6 +1272,16 @@ struct xe_bo *__xe_bo_create_locked(struct xe_device *xe, struct xe_bo *bo,
>>   	else
>>   		ttm_bo_move_to_lru_tail_unlocked(&bo->ttm);
>>   
>> +	/*
>> +	 * TTM can give us VRAM that still has active fences i.e GPU might still
>> +	 * be accessing it. To keep things simple just sync any kernel fences
>> +	 * here, if the buffer is KMD internal. For normal userspace objects we
>> +	 * should already have the required pipelining or sync waiting.
>> +	 */
>> +	if (type == ttm_bo_type_kernel)
>> +		dma_resv_wait_timeout(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
>> +				      false, MAX_SCHEDULE_TIMEOUT);
>> +
> 
> So this is the case where we create a kernel BO within a VM (e.g. memory
> for page tables) and the memory we received is also within the same VM
> (bo->ttm.base.resv == VM resv object)? The memory we received could
> still be being used thus we need to wait for the memory to be idle? Is
> that right? Also how did you arrive at the DMA_RESV_USAGE_KERNEL slot to
> determine idle?

Yeah, if the BO dma-resv is the same as the vm, then waiting on the vm 
dma-resv would also work, or using the KERNEL fences inside as 
dependencies if we want to pipeline it.

In TTM when evicting VRAM it can be done async. When TTM finds 
appropriate BO(s) to evict we don't need to sync wait on any fences 
(like for userspace job etc). In our xe_bo_move() callback when handed 
some object to evict, we just keep the pipeline going, and schedule the 
move from VRAM -> TT, using the dma-resv fences as input dependencies 
for the copy job (if any). At the end of xe_bo_move() we call 
ttm_bo_move_accel_cleanup(), passing in the final fence for the copy 
job. TTM then adds that fence as USAGE_KERNEL into the dma-resv of the 
object being evicted. The other important step is attaching that same 
fence onto the actual VRAM resource manager, as per 
ttm_bo_move_pipeline_evict(). With the fence attached to the VRAM 
manager the buddy allocator blocks are all freed, even though the memory 
ranges they represent are potentially still being accessed by the GPU. 
The next users that come along allocating memory from the VRAM manager, 
will get the copy job fence that was attached added to their dma-resv 
with USAGE_KERNEL. See ttm_bo_mem_space()/ttm_bo_mem_force_space() -> 
ttm_bo_add_move_fence().

So with all that waiting for DMA_RESV_USAGE_KERNEL here for KMD internal 
stuff should be enough.

> 
> Matt
> 
>>   	return bo;
>>   }
>>   
>> -- 
>> 2.41.0
>>