Re: [PATCH i-g-t v7 01/10] lib/xe: Add instance parameter to xe_vm_madvise and introduce lr_sync helpers

["Sharma, Nishit" <nishit.sharma@intel.com>]

On 11/17/2025 6:04 PM, Hellstrom, Thomas wrote:
> On Thu, 2025-11-13 at 16:33 +0000, nishit.sharma@intel.com wrote:
>> From: Nishit Sharma <nishit.sharma@intel.com>
>>
>> Add an 'instance' parameter to xe_vm_madvise and __xe_vm_madvise to
>> support per-instance memory advice operations.Implement
>> xe_vm_bind_lr_sync
>> and xe_vm_unbind_lr_sync helpers for synchronous VM bind/unbind using
>> user
>> fences.
>> These changes improve memory advice and binding operations for multi-
>> GPU
>> and multi-instance scenarios in IGT tests.
> s memory advice/memory advise/ ?
Git it. Will edit the description.
>
> Also the lr_sync part is unrelated and should be split out to a
> separate patch.

Sure will create separate patch for lr_sync part. Also the 
xe_exec_system_allocator() changes available in Patch [2/10] should be 
merged along madvise changes within

Patch [1/10]?


>
> Thanks,
> Thomas
>
>
>
>> Signed-off-by: Nishit Sharma <nishit.sharma@intel.com>
>> ---
>>   include/drm-uapi/xe_drm.h |  4 +--
>>   lib/xe/xe_ioctl.c         | 53 +++++++++++++++++++++++++++++++++++--
>> --
>>   lib/xe/xe_ioctl.h         | 11 +++++---
>>   3 files changed, 58 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
>> index 89ab54935..3472efa58 100644
>> --- a/include/drm-uapi/xe_drm.h
>> +++ b/include/drm-uapi/xe_drm.h
>> @@ -2060,8 +2060,8 @@ struct drm_xe_madvise {
>>   			/** @preferred_mem_loc.migration_policy:
>> Page migration policy */
>>   			__u16 migration_policy;
>>   
>> -			/** @preferred_mem_loc.pad : MBZ */
>> -			__u16 pad;
>> +			/** @preferred_mem_loc.region_instance:
>> Region instance */
>> +			__u16 region_instance;
>>   
>>   			/** @preferred_mem_loc.reserved : Reserved
>> */
>>   			__u64 reserved;
>> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
>> index 39c4667a1..06ce8a339 100644
>> --- a/lib/xe/xe_ioctl.c
>> +++ b/lib/xe/xe_ioctl.c
>> @@ -687,7 +687,8 @@ int64_t xe_wait_ufence(int fd, uint64_t *addr,
>> uint64_t value,
>>   }
>>   
>>   int __xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range,
>> -		    uint64_t ext, uint32_t type, uint32_t op_val,
>> uint16_t policy)
>> +		    uint64_t ext, uint32_t type, uint32_t op_val,
>> uint16_t policy,
>> +		    uint16_t instance)
>>   {
>>   	struct drm_xe_madvise madvise = {
>>   		.type = type,
>> @@ -704,6 +705,7 @@ int __xe_vm_madvise(int fd, uint32_t vm, uint64_t
>> addr, uint64_t range,
>>   	case DRM_XE_MEM_RANGE_ATTR_PREFERRED_LOC:
>>   		madvise.preferred_mem_loc.devmem_fd = op_val;
>>   		madvise.preferred_mem_loc.migration_policy = policy;
>> +		madvise.preferred_mem_loc.region_instance =
>> instance;
>>   		igt_debug("madvise.preferred_mem_loc.devmem_fd =
>> %d\n",
>>   			  madvise.preferred_mem_loc.devmem_fd);
>>   		break;
>> @@ -731,14 +733,55 @@ int __xe_vm_madvise(int fd, uint32_t vm,
>> uint64_t addr, uint64_t range,
>>    * @type: type of attribute
>>    * @op_val: fd/atomic value/pat index, depending upon type of
>> operation
>>    * @policy: Page migration policy
>> + * @instance: vram instance
>>    *
>>    * Function initializes different members of struct drm_xe_madvise
>> and calls
>>    * MADVISE IOCTL .
>>    *
>> - * Asserts in case of error returned by DRM_IOCTL_XE_MADVISE.
>> + * Returns error number in failure and 0 if pass.
>>    */
>> -void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range,
>> -		   uint64_t ext, uint32_t type, uint32_t op_val,
>> uint16_t policy)
>> +int xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range,
>> +		   uint64_t ext, uint32_t type, uint32_t op_val,
>> uint16_t policy,
>> +		   uint16_t instance)
>>   {
>> -	igt_assert_eq(__xe_vm_madvise(fd, vm, addr, range, ext,
>> type, op_val, policy), 0);
>> +	return __xe_vm_madvise(fd, vm, addr, range, ext, type,
>> op_val, policy, instance);
>> +}
>> +
>> +#define        BIND_SYNC_VAL   0x686868
>> +void xe_vm_bind_lr_sync(int fd, uint32_t vm, uint32_t bo, uint64_t
>> offset,
>> +			uint64_t addr, uint64_t size, uint32_t
>> flags)
>> +{
>> +	volatile uint64_t *sync_addr = malloc(sizeof(*sync_addr));
>> +	struct drm_xe_sync sync = {
>> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
>> +		.type = DRM_XE_SYNC_TYPE_USER_FENCE,
>> +		.addr = to_user_pointer((uint64_t *)sync_addr),
>> +		.timeline_value = BIND_SYNC_VAL,
>> +	};
>> +
>> +	igt_assert(!!sync_addr);
>> +	xe_vm_bind_async_flags(fd, vm, 0, bo, 0, addr, size, &sync,
>> 1, flags);
>> +	if (*sync_addr != BIND_SYNC_VAL)
>> +		xe_wait_ufence(fd, (uint64_t *)sync_addr,
>> BIND_SYNC_VAL, 0, NSEC_PER_SEC * 10);
>> +	/* Only free if the wait succeeds */
>> +	free((void *)sync_addr);
>> +}
>> +
>> +void xe_vm_unbind_lr_sync(int fd, uint32_t vm, uint64_t offset,
>> +			  uint64_t addr, uint64_t size)
>> +{
>> +	volatile uint64_t *sync_addr = malloc(sizeof(*sync_addr));
>> +	struct drm_xe_sync sync = {
>> +		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
>> +		.type = DRM_XE_SYNC_TYPE_USER_FENCE,
>> +		.addr = to_user_pointer((uint64_t *)sync_addr),
>> +		.timeline_value = BIND_SYNC_VAL,
>> +	};
>> +
>> +	igt_assert(!!sync_addr);
>> +	*sync_addr = 0;
>> +	xe_vm_unbind_async(fd, vm, 0, 0, addr, size, &sync, 1);
>> +	if (*sync_addr != BIND_SYNC_VAL)
>> +		xe_wait_ufence(fd, (uint64_t *)sync_addr,
>> BIND_SYNC_VAL, 0, NSEC_PER_SEC * 10);
>> +	free((void *)sync_addr);
>>   }
>> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
>> index ae8a23a54..1ae38029d 100644
>> --- a/lib/xe/xe_ioctl.h
>> +++ b/lib/xe/xe_ioctl.h
>> @@ -100,13 +100,18 @@ int __xe_wait_ufence(int fd, uint64_t *addr,
>> uint64_t value,
>>   int64_t xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
>>   		       uint32_t exec_queue, int64_t timeout);
>>   int __xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range, uint64_t ext,
>> -		    uint32_t type, uint32_t op_val, uint16_t
>> policy);
>> -void xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range, uint64_t ext,
>> -		   uint32_t type, uint32_t op_val, uint16_t policy);
>> +		    uint32_t type, uint32_t op_val, uint16_t policy,
>> uint16_t instance);
>> +int xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
>> range, uint64_t ext,
>> +		  uint32_t type, uint32_t op_val, uint16_t policy,
>> uint16_t instance);
>>   int xe_vm_number_vmas_in_range(int fd, struct
>> drm_xe_vm_query_mem_range_attr *vmas_attr);
>>   int xe_vm_vma_attrs(int fd, struct drm_xe_vm_query_mem_range_attr
>> *vmas_attr,
>>   		    struct drm_xe_mem_range_attr *mem_attr);
>>   struct drm_xe_mem_range_attr
>>   *xe_vm_get_mem_attr_values_in_range(int fd, uint32_t vm, uint64_t
>> start,
>>   				    uint64_t range, uint32_t
>> *num_ranges);
>> +void xe_vm_bind_lr_sync(int fd, uint32_t vm, uint32_t bo,
>> +			uint64_t offset, uint64_t addr,
>> +			uint64_t size, uint32_t flags);
>> +void xe_vm_unbind_lr_sync(int fd, uint32_t vm, uint64_t offset,
>> +			  uint64_t addr, uint64_t size);
>>   #endif /* XE_IOCTL_H */