[PATCH 3/4] drm/xe: Allow freeing of a managed bo

[John Harrison]

On 7/25/2025 1:25 AM, Michal Wajdeczko wrote:
> On 7/25/2025 2:21 AM, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> If a bo is created via xe_managed_bo_create_pin_map() then it cannot be
>> freed by the driver using xe_bo_unpin_map_no_vm(), or indeed any other
>> existing function. The DRM layer will still have a pointer stashed
>> away for later freeing, causing a invalid memory access on driver
>> unload. So add a helper for releasing the DRM action as well.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_bo.c | 5 +++++
>>   drivers/gpu/drm/xe/xe_bo.h | 1 +
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
>> index ffca1cea5585..472cadcfc4dd 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.c
>> +++ b/drivers/gpu/drm/xe/xe_bo.c
>> @@ -2222,6 +2222,11 @@ struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile
>>   	return bo;
>>   }
>>   
> please add kernel-doc for this new public function
None of the xe_bo functions have kernel-doc :(. Someone needs to 
document the whole lot and I am definitely not the right person to do 
that. I don't really know what the doc for this function should say, to 
be quite honest. E.g. why is it _no_vm? This code was suggested by 
someone from the memory side (I forget who, it was a while ago) and it 
fixed my problem so here it is.

>
>> +void xe_managed_bo_unpin_map_no_vm(struct xe_device *xe, struct xe_bo *bo)
> do we really need xe? we can get xe from xe_bo_device(bo) if needed
I don't know. Maybe? Some way of getting to the drm.dev object is needed.

>
>> +{
>> +	devm_release_action(xe->drm.dev, __xe_bo_unpin_map_no_vm, bo);
>> +}do we want to use this function right away in xe_managed_bo_reinit_in_vram() ?
Is it worth changing? The point is to provide a way to do this from 
outside the bo code. Internal code can do what it wants directly. And 
abstracting a single line of code into a function call wrapper around a 
single line of code seems more like obfuscation than simplification.

John.

>> +
>>   struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
>>   					     const void *data, size_t size, u32 flags)
>>   {
>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
>> index 8cce413b5235..72f4348d0d52 100644
>> --- a/drivers/gpu/drm/xe/xe_bo.h
>> +++ b/drivers/gpu/drm/xe/xe_bo.h
>> @@ -122,6 +122,7 @@ struct xe_bo *xe_bo_create_pin_map_at_aligned(struct xe_device *xe,
>>   					      u64 alignment);
>>   struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
>>   					   size_t size, u32 flags);
>> +void xe_managed_bo_unpin_map_no_vm(struct xe_device *xe, struct xe_bo *bo);
>>   struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
>>   					     const void *data, size_t size, u32 flags);
>>   int xe_managed_bo_reinit_in_vram(struct xe_device *xe, struct xe_tile *tile, struct xe_bo **src);