Re: [PATCH 1/4] drm/xe: Add uapi for dumpable bos

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

On 2024-01-12 18:19, Souza, Jose wrote:
> On Fri, 2024-01-12 at 13:41 +0100, Maarten Lankhorst wrote:
>> Add the flag XE_VM_BIND_FLAG_DUMPABLE to notify devcoredump that this
>> mapping should be dumped.
>>
>> This is not hooked up, but the uapi should be ready before merging.
>>
>> It's likely easier to dump the contents of the bo's at devcoredump
>> readout time, so it's better if the bos will stay unmodified after
>> a hang. The NEEDS_CPU_MAPPING flag is removed as requirement.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_vm.c | 8 +++-----
>>   include/uapi/drm/xe_drm.h  | 1 +
>>   2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index a7e7a0b24099..0a3b9c0ad772 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -2687,12 +2687,13 @@ static int vm_bind_ioctl_ops_execute(struct xe_vm *vm,
>>   #ifdef TEST_VM_ASYNC_OPS_ERROR
>>   #define SUPPORTED_FLAGS	\
>>   	(FORCE_ASYNC_OP_ERROR | DRM_XE_VM_BIND_FLAG_READONLY | \
>> -	 DRM_XE_VM_BIND_FLAG_IMMEDIATE | DRM_XE_VM_BIND_FLAG_NULL | 0xffff)
>> +	 DRM_XE_VM_BIND_FLAG_IMMEDIATE | DRM_XE_VM_BIND_FLAG_NULL | \
>> +	 DRM_XE_VM_BIND_FLAG_DUMPABLE | 0xffff)
>>   #else
>>   #define SUPPORTED_FLAGS	\
>>   	(DRM_XE_VM_BIND_FLAG_READONLY | \
>>   	 DRM_XE_VM_BIND_FLAG_IMMEDIATE | DRM_XE_VM_BIND_FLAG_NULL | \
>> -	 0xffff)
>> +	 DRM_XE_VM_BIND_FLAG_DUMPABLE | 0xffff)
>>   #endif
>>   #define XE_64K_PAGE_MASK 0xffffull
>>   #define ALL_DRM_XE_SYNCS_FLAGS (DRM_XE_SYNCS_FLAG_WAIT_FOR_OP)
>> @@ -2919,9 +2920,6 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>>   		u16 pat_index = bind_ops[i].pat_index;
>>   		u16 coh_mode;
>>   
>> -		if (!obj)
>> -			continue;
>> -
> 
> Why are you dropping this ^
> Without this block DRM_XE_VM_BIND_FLAG_NULL binds fails.
I have no idea why this is there, that also explains why my userptr 
stuff failed. Had to readd the check, but didn't notice I removed it in 
the first place. :)

Cheers,
~Maarten