Re: [PATCH i-g-t 3/4] tests/intel/xe_madvise: Add atomic-global subtest

["Gupta, Varun" <varun.gupta@intel.com>]

On 29-Apr-26 6:41 PM, Sharma, Nishit wrote:
>
> On 4/23/2026 12:08 PM, Varun Gupta wrote:
>> Validate that madvise ATOMIC_GLOBAL permits both CPU and GPU atomic
>> access on SVM memory.  The test sets ATOMIC_GLOBAL on heap-allocated
>> memory, performs 100 CPU atomic increments while data resides in SMEM,
>> then executes GPU MI_ATOMIC_INC which triggers the page-fault handler
>> to migrate data to VRAM.  The final counter value must equal 101
>> (CPU + GPU increments).
>>
>> Signed-off-by: Varun Gupta <varun.gupta@intel.com>
>> ---
>>   tests/intel/xe_madvise.c | 75 ++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 75 insertions(+)
>>
>> diff --git a/tests/intel/xe_madvise.c b/tests/intel/xe_madvise.c
>> index 1f4fb16b9..c251186d3 100644
>> --- a/tests/intel/xe_madvise.c
>> +++ b/tests/intel/xe_madvise.c
>> @@ -865,6 +865,77 @@ static void test_atomic_device(int fd, struct 
>> drm_xe_engine_class_instance *eci)
>>       xe_vm_destroy(fd, vm);
>>   }
>>   +/**
>> + * SUBTEST: atomic-global
>> + * Description: madvise atomic global supports both CPU and GPU 
>> atomic operations,
>> + *        test does CPU atomic increments on SMEM then GPU 
>> MI_ATOMIC_INC
>> + *        which triggers fault-driven migration to VRAM
>> + * Test category: functionality test
>> + */
>> +static void test_atomic_global(int fd, struct 
>> drm_xe_engine_class_instance *eci)
>> +{
>> +    struct drm_xe_sync sync[1] = {
>> +        { .type = DRM_XE_SYNC_TYPE_USER_FENCE,
>> +          .flags = DRM_XE_SYNC_FLAG_SIGNAL,
>> +          .timeline_value = USER_FENCE_VALUE },
>> +    };
>> +    struct drm_xe_exec exec = {
>> +        .num_batch_buffer = 1,
>> +        .num_syncs = 1,
>> +        .syncs = to_user_pointer(sync),
>> +    };
>> +    struct atomic_data *data;
>> +    uint32_t vm, exec_queue;
>> +    uint64_t addr;
>> +    size_t bo_size;
>> +    int va_bits, i;
>> +    int n_cpu_ops = 100;
>> +
>> +    va_bits = xe_va_bits(fd);
>> +    vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_LR_MODE |
>> +              DRM_XE_VM_CREATE_FLAG_FAULT_MODE, 0);
>> +
>> +    bo_size = xe_bb_size(fd, sizeof(*data));
>> +    data = aligned_alloc(bo_size, bo_size);
>> +    igt_assert(data);
>> +    memset(data, 0, bo_size);
>> +
>> +    addr = to_user_pointer(data);
>> +
>> +    sync[0].addr = to_user_pointer(&data->vm_sync);
>> +    __xe_vm_bind_assert(fd, vm, 0, 0, 0, 0, 0x1ull << va_bits,
>> +                DRM_XE_VM_BIND_OP_MAP,
>> +                DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR,
>> +                sync, 1, 0, 0);
> Same comment as in [PATCH i-g-t 2/4]
>> +    xe_wait_ufence(fd, &data->vm_sync, USER_FENCE_VALUE, 0, FIVE_SEC);
>> +    data->vm_sync = 0;
>> +
>> +    xe_vm_madvise(fd, vm, addr, bo_size, 0,
>> +              DRM_XE_MEM_RANGE_ATTR_ATOMIC, DRM_XE_ATOMIC_GLOBAL, 0, 
>> 0);
>> +
>> +    for (i = 0; i < n_cpu_ops; i++)
>> +        __atomic_fetch_add(&data->data, 1, __ATOMIC_SEQ_CST);
>
> This is built-in GCC atomic operation. Same will work in ARM compiler? 
> Earlier we have experienced IGT pipeline failure in upstream because of
>
> typecasting not supported in ARM compiler.


Yes, this should work on arm as well. The operand is plain uint32_t with 
literal 1, no casts or type mismatches.
Tested compilation locally.

>
>> +
>> +    igt_assert_eq(data->data, n_cpu_ops);
>> +
>> +    atomic_build_batch(data, addr);
>> +
>> +    exec_queue = xe_exec_queue_create(fd, vm, eci, 0);
>> +    exec.exec_queue_id = exec_queue;
>> +    exec.address = addr + ((char *)&data->batch - (char *)data);
>> +
>> +    sync[0].addr = to_user_pointer(&data->exec_sync);
>> +    xe_exec(fd, &exec);
>> +    xe_wait_ufence(fd, &data->exec_sync, USER_FENCE_VALUE,
>> +               exec_queue, FIVE_SEC);
>> +
>> +    igt_assert_eq(data->data, n_cpu_ops + 1);
>> +
>> +    xe_exec_queue_destroy(fd, exec_queue);
>> +    free(data);
>> +    xe_vm_destroy(fd, vm);
>> +}
>> +
>>   int igt_main()
>>   {
>>       struct drm_xe_engine_class_instance *hwe;
>> @@ -932,6 +1003,10 @@ int igt_main()
>>           igt_subtest("atomic-device")
>>               xe_for_each_engine(fd, hwe)
>>                   test_atomic_device(fd, hwe);
>> +
>> +        igt_subtest("atomic-global")
>> +            xe_for_each_engine(fd, hwe)
>> +                test_atomic_global(fd, hwe);
>>       }
>>         igt_fixture() {
> Already SVM tests available for atomic-device operations. What's the 
> value add in these added tests? Or can't we modify existing IGTs? 


The existing atomic tests in system_allocator validate madvise 
atomic+preffered_loc together. This one tests atomic mode in isolation.
Modifying existing ones would be a big overhead and would change the purpose

THanks