[PATCH i-g-t v3 03/10] tests/intel/xe_svm: Add SVM basic tests using malloc and mmap

[Bommu, Krishnaiah]

> -----Original Message-----
> From: Zeng, Oak <oak.zeng at intel.com>
> Sent: Friday, May 17, 2024 8:10 PM
> To: Bommu, Krishnaiah <krishnaiah.bommu at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Subject: RE: [PATCH i-g-t v3 03/10] tests/intel/xe_svm: Add SVM basic tests
> using malloc and mmap
> 
> 
> 
> > -----Original Message-----
> > From: Bommu, Krishnaiah <krishnaiah.bommu at intel.com>
> > Sent: Friday, May 17, 2024 7:47 AM
> > To: igt-dev at lists.freedesktop.org
> > Cc: Bommu, Krishnaiah <krishnaiah.bommu at intel.com>; Zeng, Oak
> > <oak.zeng at intel.com>; Ghimiray, Himal Prasad
> > <himal.prasad.ghimiray at intel.com>
> > Subject: [PATCH i-g-t v3 03/10] tests/intel/xe_svm: Add SVM basic
> > tests using malloc and mmap
> >
> > Adds subtests to verify the basic functionality of Shared Virtual
> > Memory (SVM) in the xe driver. The tests ensure that memory allocated
> > with malloc or mmap can be correctly used for GPU command submission
> > and data verification.
> >
> > Subtests:
> > - svm-basic-malloc: Verifies SVM functionality using memory allocated
> > with malloc.
> > - svm-basic-mmap: Verifies SVM functionality using memory allocated
> > with mmap.
> >
> > The verification is done by writing a specific value to the allocated
> > memory via GPU and checking if the value is correctly stored.
> >
> > Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
> > Cc: Oak Zeng <oak.zeng at intel.com>
> > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> > ---
> >  include/drm-uapi/xe_drm.h |  1 +
> >  tests/intel/xe_svm.c      | 52 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> > index 0b709b374..69c8792bb 100644
> > --- a/include/drm-uapi/xe_drm.h
> > +++ b/include/drm-uapi/xe_drm.h
> > @@ -973,6 +973,7 @@ struct drm_xe_vm_bind_op {
> >  #define DRM_XE_VM_BIND_FLAG_IMMEDIATE	(1 << 1)
> >  #define DRM_XE_VM_BIND_FLAG_NULL	(1 << 2)
> >  #define DRM_XE_VM_BIND_FLAG_DUMPABLE	(1 << 3)
> > +#define DRM_XE_VM_BIND_FLAG_SYSTEM_ALLOCATOR	(1 << 4)
> 
> You introduced this flag but didn't use it.
> 
> Use it to perform the whole address space vm bind in th igt fixture.
> 
__xe_vm_bind_assert(fd, vm, 0, 0, 0, 0, 0x1ull << 47, DRM_XE_VM_BIND_OP_MAP,
                                DRM_XE_VM_BIND_FLAG_SYSTEM_ALLOCATOR, 0, 0, 0, 0);
I added this in previous patch, I forget to add this in this patch, I will add in igt fixture

Regards,
Krishna.
> 
> >  	/** @flags: Bind flags */
> >  	__u32 flags;
> >
> > diff --git a/tests/intel/xe_svm.c b/tests/intel/xe_svm.c index
> > 6302af95b..741961529 100644
> > --- a/tests/intel/xe_svm.c
> > +++ b/tests/intel/xe_svm.c
> > @@ -21,6 +21,12 @@
> >   *
> >   * SUBTEST: xe-basic
> >   * Description: Basic test to verify store dword functionality using
> > helper functions
> > + *
> > + * SUBTEST: svm-basic-malloc
> > + * Description: Verify SVM basic functionality using malloc.
> > + *
> > + * SUBTEST: svm-basic-mmap
> > + * Description: Verify SVM basic functionality using mmap.
> >   */
> >
> >  #include <fcntl.h>
> > @@ -75,6 +81,44 @@ static void xe_basic(int fd, uint32_t vm, struct
> > drm_xe_engine_class_instance *e
> >  	xe_destroy_buffer(&dst_buf);
> >  }
> >
> > +/**
> > + * @brief Tests SVM functionality using malloc or mmap.
> > + *
> > + * This function tests the ability to use malloc or mmap allocated
> > +memory
> > + * for direct GPU command submission and data verification.
> > + */
> > +static void svm_basic(int fd, uint32_t vm, struct
> > +drm_xe_engine_class_instance
> > *eci, bool test_mmap)
> > +{
> > +	uint64_t gpu_va = 0x1a0000;
> > +	size_t bo_size = xe_bb_size(fd, PAGE_ALIGN_UFENCE);
> > +	uint32_t *dst;
> > +
> > +	struct xe_buffer cmd_buf = {
> > +		.fd = fd,
> > +		.gpu_addr = (void *)(uintptr_t)gpu_va,
> > +		.vm = vm,
> > +		.size = bo_size,
> > +		.placement = vram_if_possible(fd, eci->gt_id),
> > +		.flag = DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
> > +	};
> > +
> > +	if (test_mmap)
> > +		dst = mmap(NULL, 4, PROT_READ|PROT_WRITE,
> > MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
> > +	else
> > +		dst = aligned_alloc(xe_get_default_alignment(fd), 4);
> 
> 
> Hmm, I currently assume system allocator should be able handle a allocation of
> 4 bytes, even without an alignment. If not, it is a driver bug. Let's remove the
> alignment for now.
> > +
> > +	xe_create_cmdbuf(&cmd_buf, insert_store, (uint64_t)dst, 0xc0ffee,
> > +eci);
> 
> Maybe change the dst_va parameter of xe_create_cmdbuf to void *, and do a
> cast inside xe_create_cmd_buf to cast the dst_va to uinit64_t. this way we don't
> need to cast every time we call xe_create_cmdbuf.
> 
> Oak
> 
> > +	xe_submit_cmd(&cmd_buf);
> > +
> > +	igt_assert_eq(*dst, 0xc0ffee);
> > +
> > +	xe_destroy_cmdbuf(&cmd_buf);
> > +	if (test_mmap)
> > +		munmap(dst, 4);
> > +	else
> > +		free(dst);
> > +}
> > +
> >  igt_main
> >  {
> >  	int fd;
> > @@ -91,6 +135,14 @@ igt_main
> >  		xe_for_each_engine(fd, hwe)
> >  			xe_basic(fd, vm, hwe);
> >
> > +	igt_subtest_f("svm-basic-malloc")
> > +		xe_for_each_engine(fd, hwe)
> > +			svm_basic(fd, vm, hwe, false);
> > +
> > +	igt_subtest_f("svm-basic-mmap")
> > +		xe_for_each_engine(fd, hwe)
> > +			svm_basic(fd, vm, hwe, true);
> > +
> >  	igt_fixture {
> >  		xe_vm_destroy(fd, vm);
> >  		drm_close_driver(fd);
> > --
> > 2.25.1