[i-g-t v4 0/4] xe: Test scratch page under fault mode

[i-g-t v4 0/4] xe: Test scratch page under fault mode

[Oak Zeng]

This series intoduces a subtest for Xe to test scratch page under fault
mode.

It also includes some helper functions to make the test easier.

Today most of igt tests use one single function to perform multiple
subtests. flags are used to control the behavior of the test. While
this is a good way to write efficient tests, it also makes the test
difficult to maintain, especially when we add more and more subtests
into single test function.

This patch series introduces some helper functions with which we can
write more readable tests. This is illustrated in patch 4 of this series.

Note the intention of this patch series is not to replace the existing
igt tests, but to provide an alternative way to write simple tests.

Bommu Krishnaiah (1):
  lib/xe/xe_util: Introduce helper functions

Oak Zeng (3):
  lib/xe: Fix a comment error
  tests/intel/xe_vm: Exclude invalid_flags tests from LNL and BMG
  tests/intel/xe_exec_fault_mode: Test scratch page under fault mode

 lib/xe/xe_ioctl.c                |   2 +-
 lib/xe/xe_util.c                 | 228 +++++++++++++++++++++++++++++++
 lib/xe/xe_util.h                 |  39 ++++++
 tests/intel/xe_exec_fault_mode.c |  69 ++++++++++
 tests/intel/xe_vm.c              |  47 +++++--
 5 files changed, 369 insertions(+), 16 deletions(-)

-- 
2.26.3

[i-g-t v4 1/4] lib/xe: Fix a comment error

[Oak Zeng]

The timeout value of __xe_wait_ufence returns the
remaining time, not elapsed time. Fix it.

Signed-off-by: Oak Zeng <oak.zeng@intel.com>
---
 lib/xe/xe_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
index 6d8388918..028102aa2 100644
--- a/lib/xe/xe_ioctl.c
+++ b/lib/xe/xe_ioctl.c
@@ -496,7 +496,7 @@ void xe_exec_wait(int fd, uint32_t exec_queue, uint64_t addr)
  *
  * Function compares @value with memory pointed by @addr until they are equal.
  *
- * Returns (in @timeout), the elapsed time in nanoseconds if user fence was
+ * Returns (in @timeout), the remaining time in nanoseconds if user fence was
  * signalled. Returns 0 on success, -errno of ioctl on error.
  */
 int __xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
-- 
2.26.3

[i-g-t v4 2/4] lib/xe/xe_util: Introduce helper functions

[Oak Zeng]

From: Bommu Krishnaiah <krishnaiah.bommu@intel.com>

Introduce helper functions for buffer creation, binding,
destruction and command submission etc. With those helpers,
writing a xe igt test will be much easier, which will be
showed in a coming example.

v2: use to_user_pointer to cast a pointer (Kamil)
    s/insert_store/xe_insert_store (Kamil)
    s/cmdbuf_fill_func_t/xe_cmdbuf_fill_func_t (Kamil)
v3: refactor command buffer fill interface (Zbigniew)
v4: add more asserts, check function parameters, drop
    function xe_fill_cmdbuf, xe_close_cmdbuf etc (Zbigniew)

Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
Signed-off-by: Oak Zeng <oak.zeng@intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 lib/xe/xe_util.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/xe/xe_util.h |  39 ++++++++
 2 files changed, 267 insertions(+)

diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
index 06b378ce0..4b53c2c88 100644
--- a/lib/xe/xe_util.c
+++ b/lib/xe/xe_util.c
@@ -13,6 +13,234 @@
 #include "xe/xe_query.h"
 #include "xe/xe_util.h"
 
+#define UFENCE_LENGTH		sizeof(((struct drm_xe_sync *)0)->timeline_value)
+
+/**
+ * xe_cmdbuf_exec_ufence_gpuva:
+ * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
+ *
+ * Returns the GPU virtual address of the execution user fence located at the
+ * end of the command buffer.
+ */
+static uint64_t xe_cmdbuf_exec_ufence_gpuva(struct xe_cmdbuf *cmdbuf)
+{
+	return cmdbuf->buf.gpu_addr + cmdbuf->buf.size - UFENCE_LENGTH;
+}
+
+/**
+ * xe_cmdbuf_exec_ufence_cpuva:
+ * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
+ *
+ * Returns the CPU virtual address of the execution user fence located at the
+ * end of the command buffer.
+ */
+static void *xe_cmdbuf_exec_ufence_cpuva(struct xe_cmdbuf *cmdbuf)
+{
+	return (char *)cmdbuf->buf.cpu_addr + cmdbuf->buf.size - UFENCE_LENGTH;
+}
+
+
+/**
+ * __xe_submit_cmd:
+ * @cmdbuf Pointer to the command buffer structure
+ *
+ * Submits a command buffer to the GPU, waits for its completion, and verifies
+ * the user fence value
+ *
+ * Return: The result of waiting for the user fence value
+ */
+int64_t __xe_submit_cmd(struct xe_cmdbuf *cmdbuf)
+{
+	int64_t timeout = NSEC_PER_SEC;
+	int ret;
+
+	struct drm_xe_sync sync[1] = {
+		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
+			.flags = DRM_XE_SYNC_FLAG_SIGNAL,
+			.timeline_value = USER_FENCE_VALUE,
+			.addr = xe_cmdbuf_exec_ufence_gpuva(cmdbuf),},
+	};
+	struct drm_xe_exec exec = {
+		.num_batch_buffer = 1,
+		.num_syncs = 1,
+		.syncs = to_user_pointer(sync),
+		.exec_queue_id = cmdbuf->exec_queue,
+		.address = cmdbuf->buf.gpu_addr,
+	};
+
+	igt_assert(cmdbuf);
+	ret = __xe_exec(cmdbuf->buf.fd, &exec);
+	if (ret)
+		return ret;
+
+	ret = __xe_wait_ufence(cmdbuf->buf.fd,
+			       (uint64_t *)xe_cmdbuf_exec_ufence_cpuva(cmdbuf),
+			       USER_FENCE_VALUE, cmdbuf->exec_queue, &timeout);
+	/* Reset the fence so the exec ufence can be reused */
+	memset((char *)xe_cmdbuf_exec_ufence_cpuva(cmdbuf), 0, UFENCE_LENGTH);
+
+	return ret;
+}
+
+/**
+ * xe_submit_cmd:
+ * @cmdbuf Pointer to the command buffer structure
+ *
+ * Wrapper function to submit a command buffer and assert its successful
+ * execution.
+ */
+void xe_submit_cmd(struct xe_cmdbuf *cmdbuf)
+{
+	int64_t ret;
+
+	igt_assert(cmdbuf);
+	ret = __xe_submit_cmd(cmdbuf);
+	igt_assert_eq(ret, 0);
+}
+
+/**
+ *xe_create_buffer:
+ * @buffer Pointer to the xe_buf structure containing buffer details.
+ *
+ * Creates a buffer, maps it to both CPU and GPU address spaces.
+ */
+int xe_create_buffer(struct xe_buf *buffer)
+{
+	struct drm_xe_sync sync[1] = {
+		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
+		  .flags = DRM_XE_SYNC_FLAG_SIGNAL,
+		  .timeline_value = USER_FENCE_VALUE },
+	};
+
+	if (buffer->fd < 0)
+		return -EINVAL;
+
+	if (buffer->size == 0)
+		return -EINVAL;
+
+	if (!(buffer->placement & all_memory_regions(buffer->fd)))
+		return -EINVAL;
+
+	buffer->bind_queue = xe_bind_exec_queue_create(buffer->fd,
+						       buffer->vm, 0);
+	buffer->bind_ufence = aligned_alloc(xe_get_default_alignment(buffer->fd),
+					    PAGE_ALIGN_UFENCE);
+	sync->addr = to_user_pointer(buffer->bind_ufence);
+
+	/* create and bind the buffer->bo */
+	buffer->bo = xe_bo_create(buffer->fd, 0, buffer->size,
+				  buffer->placement, buffer->flag);
+	buffer->cpu_addr = xe_bo_map(buffer->fd, buffer->bo, buffer->size);
+	xe_vm_bind_async(buffer->fd, buffer->vm, buffer->bind_queue,
+			 buffer->bo, 0, buffer->gpu_addr,
+			 buffer->size, sync, 1);
+
+	xe_wait_ufence(buffer->fd, buffer->bind_ufence,
+		       USER_FENCE_VALUE, buffer->bind_queue, NSEC_PER_SEC);
+	memset(buffer->bind_ufence, 0, PAGE_ALIGN_UFENCE);
+
+	return 0;
+}
+
+/**
+ * xe_destroy_buffer:
+ * @buffer Pointer to the xe_buf structure containing buffer details
+ *
+ * Destroys a buffer created by xe_create_buffer and releases associated
+ * resources.
+ */
+void xe_destroy_buffer(struct xe_buf *buffer)
+{
+	struct drm_xe_sync sync[1] = {
+		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
+		  .flags = DRM_XE_SYNC_FLAG_SIGNAL,
+		  .timeline_value = USER_FENCE_VALUE },
+	};
+
+	igt_assert(buffer);
+	sync->addr = to_user_pointer(buffer->bind_ufence);
+
+	xe_vm_unbind_async(buffer->fd, buffer->vm, buffer->bind_queue,
+			   0, buffer->gpu_addr, buffer->size, sync, 1);
+	xe_wait_ufence(buffer->fd, buffer->bind_ufence,
+		       USER_FENCE_VALUE, buffer->bind_queue, NSEC_PER_SEC);
+	memset(buffer->bind_ufence, 0, PAGE_ALIGN_UFENCE);
+
+	munmap(buffer->cpu_addr, buffer->size);
+	gem_close(buffer->fd, buffer->bo);
+
+	free(buffer->bind_ufence);
+	xe_exec_queue_destroy(buffer->fd, buffer->bind_queue);
+}
+
+/**
+ * xe_cmdbuf_insert_store:
+ * @cmdbuf: command buffer where commands will be inserted.
+ * @dst_va Destination virtual address to store the value.
+ * @val Value to be stored.
+ *
+ * Inserts a MI_STORE_DWORD_IMM_GEN4 command into a command  buffer, which stores
+ * an immediate value to a given destination virtual address.
+ */
+void xe_cmdbuf_insert_store(struct xe_cmdbuf *cmdbuf,
+			    uint64_t dst_va, uint32_t val)
+{
+	uint32_t *batch = cmdbuf->buf.cpu_addr;
+
+	/* Leaves at least one dword for MI_BATCH_BUFFER_END */
+	igt_assert(cmdbuf->write_index + 4 <=
+		   cmdbuf->cmd_size/sizeof(uint32_t) - 1);
+
+	batch[cmdbuf->write_index++] = MI_STORE_DWORD_IMM_GEN4;
+	batch[cmdbuf->write_index++] = dst_va;
+	batch[cmdbuf->write_index++] = dst_va >> 32;
+	batch[cmdbuf->write_index++] = val;
+}
+
+void xe_cmdbuf_insert_bbe(struct xe_cmdbuf *cmdbuf)
+{
+	uint32_t *batch = cmdbuf->buf.cpu_addr;
+
+	igt_assert(cmdbuf->write_index <= cmdbuf->cmd_size/sizeof(uint32_t) - 1);
+	batch[cmdbuf->write_index++] = MI_BATCH_BUFFER_END;
+}
+
+/**
+ * xe_create_cmdbuf:
+ * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
+ * @eci Pointer to the engine class instance for execution.
+ *
+ * Creates a command buffer, fills it with commands using the provided fill
+ * function, and sets up the execution queue for submission.
+ */
+void xe_create_cmdbuf(struct xe_cmdbuf *cmdbuf,
+		       struct drm_xe_engine_class_instance *eci)
+{
+	struct xe_buf *buf = &cmdbuf->buf;
+	/*
+	 * make some room for a exec_ufence, which will be used to sync the
+	 * submission of this command....
+	 */
+	buf->size = xe_bb_size(buf->fd,
+			       cmdbuf->cmd_size + PAGE_ALIGN_UFENCE);
+	xe_create_buffer(buf);
+	cmdbuf->exec_queue = xe_exec_queue_create(buf->fd, buf->vm, eci, 0);
+	cmdbuf->write_index = 0;
+}
+
+/**
+ * xe_destroy_cmdbuf:
+ * @cmdbuf Pointer to the xe_buf structure representing the command buffer.
+ *
+ * Destroys a command buffer created by xe_create_cmdbuf and releases
+ * associated resources.
+ */
+void xe_destroy_cmdbuf(struct xe_cmdbuf *cmdbuf)
+{
+	xe_exec_queue_destroy(cmdbuf->buf.fd, cmdbuf->exec_queue);
+	xe_destroy_buffer(&cmdbuf->buf);
+}
+
 static bool __region_belongs_to_regions_type(struct drm_xe_mem_region *region,
 					     uint32_t *mem_regions_type,
 					     int num_regions)
diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
index 06ebd3c2a..76e9d5eff 100644
--- a/lib/xe/xe_util.h
+++ b/lib/xe/xe_util.h
@@ -14,6 +14,45 @@
 
 #include "xe_query.h"
 
+#define USER_FENCE_VALUE        0xdeadbeefdeadbeefull
+#define PAGE_ALIGN_UFENCE	4096
+
+struct xe_buf {
+	void *cpu_addr;
+	uint64_t gpu_addr;
+	/*the user fence used to vm bind this buffer*/
+	uint64_t *bind_ufence;
+	uint64_t size;
+	uint32_t flag;
+	uint32_t vm;
+	uint32_t bo;
+	uint32_t placement;
+	uint32_t bind_queue;
+	int fd;
+	bool is_userptr;
+};
+
+struct xe_cmdbuf {
+	struct xe_buf buf;
+	/* command size in bytes, not including exec_ufence */
+	uint64_t cmd_size;
+	uint32_t exec_queue;
+	/* Dword index to writ to command buffer */
+	uint32_t write_index;
+};
+
+int xe_create_buffer(struct xe_buf *buffer);
+void xe_destroy_buffer(struct xe_buf *buffer);
+
+void xe_create_cmdbuf(struct xe_cmdbuf *cmdbuf,
+		      struct drm_xe_engine_class_instance *eci);
+void xe_cmdbuf_insert_store(struct xe_cmdbuf *cmdbuf, uint64_t dst_va,
+			    uint32_t val);
+void xe_cmdbuf_insert_bbe(struct xe_cmdbuf *cmdbuf);
+void xe_submit_cmd(struct xe_cmdbuf *cmdbuf);
+int64_t __xe_submit_cmd(struct xe_cmdbuf *cmdbuf);
+void xe_destroy_cmdbuf(struct xe_cmdbuf *cmdbuf);
+
 #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
 	(xe_region_class(fd, region) == DRM_XE_MEM_REGION_CLASS_SYSMEM)
 #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
-- 
2.26.3

Re: [i-g-t v4 2/4] lib/xe/xe_util: Introduce helper functions

[Zbigniew Kempczyński]

On Thu, Feb 20, 2025 at 07:42:00PM -0500, Oak Zeng wrote:
> From: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> 
> Introduce helper functions for buffer creation, binding,
> destruction and command submission etc. With those helpers,
> writing a xe igt test will be much easier, which will be
> showed in a coming example.
> 
> v2: use to_user_pointer to cast a pointer (Kamil)
>     s/insert_store/xe_insert_store (Kamil)
>     s/cmdbuf_fill_func_t/xe_cmdbuf_fill_func_t (Kamil)
> v3: refactor command buffer fill interface (Zbigniew)
> v4: add more asserts, check function parameters, drop
>     function xe_fill_cmdbuf, xe_close_cmdbuf etc (Zbigniew)
> 
> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  lib/xe/xe_util.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/xe/xe_util.h |  39 ++++++++
>  2 files changed, 267 insertions(+)
> 
> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> index 06b378ce0..4b53c2c88 100644
> --- a/lib/xe/xe_util.c
> +++ b/lib/xe/xe_util.c
> @@ -13,6 +13,234 @@
>  #include "xe/xe_query.h"
>  #include "xe/xe_util.h"
>  
> +#define UFENCE_LENGTH		sizeof(((struct drm_xe_sync *)0)->timeline_value)
> +
> +/**
> + * xe_cmdbuf_exec_ufence_gpuva:
> + * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
> + *
> + * Returns the GPU virtual address of the execution user fence located at the
> + * end of the command buffer.
> + */
> +static uint64_t xe_cmdbuf_exec_ufence_gpuva(struct xe_cmdbuf *cmdbuf)
> +{
> +	return cmdbuf->buf.gpu_addr + cmdbuf->buf.size - UFENCE_LENGTH;
> +}
> +
> +/**
> + * xe_cmdbuf_exec_ufence_cpuva:
> + * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
> + *
> + * Returns the CPU virtual address of the execution user fence located at the
> + * end of the command buffer.
> + */
> +static void *xe_cmdbuf_exec_ufence_cpuva(struct xe_cmdbuf *cmdbuf)
> +{
> +	return (char *)cmdbuf->buf.cpu_addr + cmdbuf->buf.size - UFENCE_LENGTH;
> +}
> +
> +
> +/**
> + * __xe_submit_cmd:
> + * @cmdbuf Pointer to the command buffer structure
> + *
> + * Submits a command buffer to the GPU, waits for its completion, and verifies
> + * the user fence value
> + *
> + * Return: The result of waiting for the user fence value
> + */
> +int64_t __xe_submit_cmd(struct xe_cmdbuf *cmdbuf)
> +{
> +	int64_t timeout = NSEC_PER_SEC;
> +	int ret;
> +
> +	struct drm_xe_sync sync[1] = {
> +		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
> +			.flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +			.timeline_value = USER_FENCE_VALUE,
> +			.addr = xe_cmdbuf_exec_ufence_gpuva(cmdbuf),},
> +	};
> +	struct drm_xe_exec exec = {
> +		.num_batch_buffer = 1,
> +		.num_syncs = 1,
> +		.syncs = to_user_pointer(sync),
> +		.exec_queue_id = cmdbuf->exec_queue,
> +		.address = cmdbuf->buf.gpu_addr,
> +	};
> +
> +	igt_assert(cmdbuf);

That's cosmetic but I would add empty line after the assert. I then
quickly find where're checks and were rest of the code.

> +	ret = __xe_exec(cmdbuf->buf.fd, &exec);
> +	if (ret)
> +		return ret;
> +
> +	ret = __xe_wait_ufence(cmdbuf->buf.fd,
> +			       (uint64_t *)xe_cmdbuf_exec_ufence_cpuva(cmdbuf),
> +			       USER_FENCE_VALUE, cmdbuf->exec_queue, &timeout);
> +	/* Reset the fence so the exec ufence can be reused */
> +	memset((char *)xe_cmdbuf_exec_ufence_cpuva(cmdbuf), 0, UFENCE_LENGTH);
> +
> +	return ret;
> +}
> +
> +/**
> + * xe_submit_cmd:
> + * @cmdbuf Pointer to the command buffer structure
> + *
> + * Wrapper function to submit a command buffer and assert its successful
> + * execution.
> + */
> +void xe_submit_cmd(struct xe_cmdbuf *cmdbuf)
> +{
> +	int64_t ret;
> +
> +	igt_assert(cmdbuf);
> +	ret = __xe_submit_cmd(cmdbuf);
> +	igt_assert_eq(ret, 0);

As ret is not returned, I would shorten this to:

igt_assert(cmdbuf);
igt_assert_eq(__xe_submit_cmd(cmdbuf), 0);

> +}
> +
> +/**
> + *xe_create_buffer:
> + * @buffer Pointer to the xe_buf structure containing buffer details.
> + *
> + * Creates a buffer, maps it to both CPU and GPU address spaces.
> + */
> +int xe_create_buffer(struct xe_buf *buffer)
> +{
> +	struct drm_xe_sync sync[1] = {
> +		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
> +		  .flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +		  .timeline_value = USER_FENCE_VALUE },
> +	};

You forgot to assert on null buffer.

> +
> +	if (buffer->fd < 0)
> +		return -EINVAL;
> +
> +	if (buffer->size == 0)
> +		return -EINVAL;
> +
> +	if (!(buffer->placement & all_memory_regions(buffer->fd)))
> +		return -EINVAL;
> +
> +	buffer->bind_queue = xe_bind_exec_queue_create(buffer->fd,
> +						       buffer->vm, 0);
> +	buffer->bind_ufence = aligned_alloc(xe_get_default_alignment(buffer->fd),
> +					    PAGE_ALIGN_UFENCE);
> +	sync->addr = to_user_pointer(buffer->bind_ufence);
> +
> +	/* create and bind the buffer->bo */
> +	buffer->bo = xe_bo_create(buffer->fd, 0, buffer->size,
> +				  buffer->placement, buffer->flag);
> +	buffer->cpu_addr = xe_bo_map(buffer->fd, buffer->bo, buffer->size);
> +	xe_vm_bind_async(buffer->fd, buffer->vm, buffer->bind_queue,
> +			 buffer->bo, 0, buffer->gpu_addr,
> +			 buffer->size, sync, 1);
> +
> +	xe_wait_ufence(buffer->fd, buffer->bind_ufence,
> +		       USER_FENCE_VALUE, buffer->bind_queue, NSEC_PER_SEC);
> +	memset(buffer->bind_ufence, 0, PAGE_ALIGN_UFENCE);
> +
> +	return 0;
> +}
> +
> +/**
> + * xe_destroy_buffer:
> + * @buffer Pointer to the xe_buf structure containing buffer details
> + *
> + * Destroys a buffer created by xe_create_buffer and releases associated
> + * resources.
> + */
> +void xe_destroy_buffer(struct xe_buf *buffer)
> +{
> +	struct drm_xe_sync sync[1] = {
> +		{ .type = DRM_XE_SYNC_TYPE_USER_FENCE,
> +		  .flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +		  .timeline_value = USER_FENCE_VALUE },
> +	};
> +
> +	igt_assert(buffer);

Please add blank line here.

> +	sync->addr = to_user_pointer(buffer->bind_ufence);
> +
> +	xe_vm_unbind_async(buffer->fd, buffer->vm, buffer->bind_queue,
> +			   0, buffer->gpu_addr, buffer->size, sync, 1);
> +	xe_wait_ufence(buffer->fd, buffer->bind_ufence,
> +		       USER_FENCE_VALUE, buffer->bind_queue, NSEC_PER_SEC);
> +	memset(buffer->bind_ufence, 0, PAGE_ALIGN_UFENCE);
> +
> +	munmap(buffer->cpu_addr, buffer->size);
> +	gem_close(buffer->fd, buffer->bo);
> +
> +	free(buffer->bind_ufence);
> +	xe_exec_queue_destroy(buffer->fd, buffer->bind_queue);
> +}
> +
> +/**
> + * xe_cmdbuf_insert_store:
> + * @cmdbuf: command buffer where commands will be inserted.
> + * @dst_va Destination virtual address to store the value.
> + * @val Value to be stored.
> + *
> + * Inserts a MI_STORE_DWORD_IMM_GEN4 command into a command  buffer, which stores
> + * an immediate value to a given destination virtual address.
> + */
> +void xe_cmdbuf_insert_store(struct xe_cmdbuf *cmdbuf,
> +			    uint64_t dst_va, uint32_t val)
> +{
> +	uint32_t *batch = cmdbuf->buf.cpu_addr;
> +
> +	/* Leaves at least one dword for MI_BATCH_BUFFER_END */
> +	igt_assert(cmdbuf->write_index + 4 <=
> +		   cmdbuf->cmd_size/sizeof(uint32_t) - 1);
> +
> +	batch[cmdbuf->write_index++] = MI_STORE_DWORD_IMM_GEN4;
> +	batch[cmdbuf->write_index++] = dst_va;
> +	batch[cmdbuf->write_index++] = dst_va >> 32;
> +	batch[cmdbuf->write_index++] = val;
> +}
> +
> +void xe_cmdbuf_insert_bbe(struct xe_cmdbuf *cmdbuf)
> +{
> +	uint32_t *batch = cmdbuf->buf.cpu_addr;
> +
> +	igt_assert(cmdbuf->write_index <= cmdbuf->cmd_size/sizeof(uint32_t) - 1);
> +	batch[cmdbuf->write_index++] = MI_BATCH_BUFFER_END;
> +}
> +
> +/**
> + * xe_create_cmdbuf:
> + * @cmdbuf Pointer to the xe_cmdbuf structure representing the command buffer.
> + * @eci Pointer to the engine class instance for execution.
> + *
> + * Creates a command buffer, fills it with commands using the provided fill
> + * function, and sets up the execution queue for submission.
> + */
> +void xe_create_cmdbuf(struct xe_cmdbuf *cmdbuf,
> +		       struct drm_xe_engine_class_instance *eci)
> +{
> +	struct xe_buf *buf = &cmdbuf->buf;

I would assert on cmdbuf == null before dereference.

> +	/*
> +	 * make some room for a exec_ufence, which will be used to sync the
> +	 * submission of this command....
> +	 */
> +	buf->size = xe_bb_size(buf->fd,
> +			       cmdbuf->cmd_size + PAGE_ALIGN_UFENCE);
> +	xe_create_buffer(buf);
> +	cmdbuf->exec_queue = xe_exec_queue_create(buf->fd, buf->vm, eci, 0);
> +	cmdbuf->write_index = 0;
> +}
> +
> +/**
> + * xe_destroy_cmdbuf:
> + * @cmdbuf Pointer to the xe_buf structure representing the command buffer.
> + *
> + * Destroys a command buffer created by xe_create_cmdbuf and releases
> + * associated resources.
> + */
> +void xe_destroy_cmdbuf(struct xe_cmdbuf *cmdbuf)
> +{

Same here.

> +	xe_exec_queue_destroy(cmdbuf->buf.fd, cmdbuf->exec_queue);
> +	xe_destroy_buffer(&cmdbuf->buf);
> +}
> +
>  static bool __region_belongs_to_regions_type(struct drm_xe_mem_region *region,
>  					     uint32_t *mem_regions_type,
>  					     int num_regions)
> diff --git a/lib/xe/xe_util.h b/lib/xe/xe_util.h
> index 06ebd3c2a..76e9d5eff 100644
> --- a/lib/xe/xe_util.h
> +++ b/lib/xe/xe_util.h
> @@ -14,6 +14,45 @@
>  
>  #include "xe_query.h"
>  
> +#define USER_FENCE_VALUE        0xdeadbeefdeadbeefull
> +#define PAGE_ALIGN_UFENCE	4096
> +
> +struct xe_buf {
> +	void *cpu_addr;
> +	uint64_t gpu_addr;
> +	/*the user fence used to vm bind this buffer*/
> +	uint64_t *bind_ufence;
> +	uint64_t size;
> +	uint32_t flag;
> +	uint32_t vm;
> +	uint32_t bo;
> +	uint32_t placement;
> +	uint32_t bind_queue;
> +	int fd;
> +	bool is_userptr;
> +};
> +
> +struct xe_cmdbuf {
> +	struct xe_buf buf;
> +	/* command size in bytes, not including exec_ufence */
> +	uint64_t cmd_size;
> +	uint32_t exec_queue;
> +	/* Dword index to writ to command buffer */
> +	uint32_t write_index;
> +};
> +
> +int xe_create_buffer(struct xe_buf *buffer);
> +void xe_destroy_buffer(struct xe_buf *buffer);
> +
> +void xe_create_cmdbuf(struct xe_cmdbuf *cmdbuf,
> +		      struct drm_xe_engine_class_instance *eci);
> +void xe_cmdbuf_insert_store(struct xe_cmdbuf *cmdbuf, uint64_t dst_va,
> +			    uint32_t val);
> +void xe_cmdbuf_insert_bbe(struct xe_cmdbuf *cmdbuf);
> +void xe_submit_cmd(struct xe_cmdbuf *cmdbuf);
> +int64_t __xe_submit_cmd(struct xe_cmdbuf *cmdbuf);
> +void xe_destroy_cmdbuf(struct xe_cmdbuf *cmdbuf);

Last thing regarding api, I think clearer would be if you would use
object name prefixing before operation in function names, I mean:

1. for xe_buffer - xe_buffer_(create|destroy)
2. for xe_cmdbuf - xe_cmdbuf_(create|destroy|insert|insert_bbe|submit)

I've checked - noone defined xe_buffer yet, so you may simply rename
xe_buf -> xe_buffer.

> +
>  #define XE_IS_SYSMEM_MEMORY_REGION(fd, region) \
>  	(xe_region_class(fd, region) == DRM_XE_MEM_REGION_CLASS_SYSMEM)
>  #define XE_IS_VRAM_MEMORY_REGION(fd, region) \
> -- 
> 2.26.3
>

Generally code looks good to me, just reply/address my nits
and I can give you my r-b.

--
Zbigniew

[i-g-t v4 3/4] tests/intel/xe_vm: Exclude invalid_flags tests from LNL and BMG

[Oak Zeng]

Due to a fix of out of bound prefetch issue, we now allow scratch
page coexist with fault mode on LNL and BMG, thus exclude those
tests on such HW.

Signed-off-by: Oak Zeng <oak.zeng@intel.com>
---
 tests/intel/xe_vm.c | 47 ++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/tests/intel/xe_vm.c b/tests/intel/xe_vm.c
index 0730dd3d3..834e0afd7 100644
--- a/tests/intel/xe_vm.c
+++ b/tests/intel/xe_vm.c
@@ -2349,6 +2349,7 @@ igt_main
 	struct drm_xe_engine_class_instance *hwe, *hwe_non_copy = NULL;
 	uint64_t bind_size;
 	int fd;
+	uint16_t dev_id;
 	const struct section {
 		const char *name;
 		int bo_n_pages;
@@ -2441,23 +2442,28 @@ igt_main
 		{ NULL },
 	};
 
-        const struct vm_create_section {
-                const char *name;
-                __u32 flags;
-        } xe_vm_create_invalid_flags[] = {
-                { "xe_vm_create_fault", DRM_XE_VM_CREATE_FLAG_FAULT_MODE },
-                { "xe_vm_create_scratch_fault",
-                        DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE |
-                        DRM_XE_VM_CREATE_FLAG_FAULT_MODE },
-                { "xe_vm_create_scratch_fault_lr",
-                        ~(DRM_XE_VM_CREATE_FLAG_LR_MODE |
-                        DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE |
-                        DRM_XE_VM_CREATE_FLAG_FAULT_MODE) },
-                { }
-        };
+	const struct vm_create_section {
+		const char *name;
+		__u32 flags;
+	} xe_vm_create_invalid_flags1[] = {
+		{ "xe_vm_create_fault", DRM_XE_VM_CREATE_FLAG_FAULT_MODE },
+		{ }
+	};
+
+	const struct vm_create_section xe_vm_create_invalid_flags2[] = {
+		{ "xe_vm_create_scratch_fault",
+			DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE |
+			DRM_XE_VM_CREATE_FLAG_FAULT_MODE },
+		{ "xe_vm_create_scratch_fault_lr",
+			~(DRM_XE_VM_CREATE_FLAG_LR_MODE |
+			DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE |
+			DRM_XE_VM_CREATE_FLAG_FAULT_MODE) },
+		{ }
+	};
 
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_XE);
+		dev_id = intel_get_drm_devid(fd);
 
 		xe_for_each_engine(fd, hwe)
 			if (hwe->engine_class != DRM_XE_ENGINE_CLASS_COPY) {
@@ -2715,11 +2721,22 @@ igt_main
 		}
 	}
 
-	for (const struct vm_create_section *s = xe_vm_create_invalid_flags; s->name; s++) {
+	for (const struct vm_create_section *s = xe_vm_create_invalid_flags1;
+	     s->name; s++) {
 		igt_subtest_f("invalid-flag-%s", s->name)
 			invalid_flag(fd, s->flags);
 	}
 
+	for (const struct vm_create_section *s = xe_vm_create_invalid_flags2;
+	     s->name; s++) {
+		igt_subtest_f("invalid-flag-%s", s->name) {
+			igt_skip_on_f(IS_LUNARLAKE(dev_id) ||
+				      IS_BATTLEMAGE(dev_id),
+				      "Skip test on this platform\n");
+			invalid_flag(fd, s->flags);
+		}
+	}
+
 	igt_subtest("invalid-extensions")
 		invalid_extensions(fd);
 
-- 
2.26.3

[i-g-t v4 4/4] tests/intel/xe_exec_fault_mode: Test scratch page under fault mode

[Oak Zeng]

On certain HW (such as lunarlake and battlemage), driver now allows
scratch page be enabled under fault mode. Test this functionality

v1: Move the test to separate function.
    Write to scratch page before rebind (Matt)

Signed-off-by: Oak Zeng <oak.zeng@intel.com>
---
 tests/intel/xe_exec_fault_mode.c | 69 ++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
index ae40e099b..7e1bdca01 100644
--- a/tests/intel/xe_exec_fault_mode.c
+++ b/tests/intel/xe_exec_fault_mode.c
@@ -17,6 +17,7 @@
 #include "igt.h"
 #include "lib/igt_syncobj.h"
 #include "lib/intel_reg.h"
+#include "lib/xe/xe_util.h"
 #include "xe_drm.h"
 
 #include "xe/xe_ioctl.h"
@@ -36,6 +37,65 @@
 #define INVALID_VA	(0x1 << 8)
 #define ENABLE_SCRATCH  (0x1 << 9)
 
+
+/**
+ * SUBTEST: scratch-fault
+ * Description: Test scratch page functionality
+ * Test category: functionality test
+ */
+static void test_scratch(int fd, struct drm_xe_engine_class_instance *eci)
+{
+	size_t bb_size = xe_bb_size(fd, PAGE_ALIGN_UFENCE);
+	uint32_t vm;
+
+	struct xe_buf dst_buf = {
+		.fd = fd,
+		.size = bb_size,
+		.gpu_addr = 0x1a0000,
+		.placement = vram_if_possible(fd, eci->gt_id),
+		.flag = DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
+	};
+
+	struct xe_cmdbuf cmdbuf = {
+		.buf = {
+			.fd = fd,
+			.gpu_addr = 0x10a0000,
+			.placement = vram_if_possible(fd, eci->gt_id),
+			.flag = DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
+		},
+		.cmd_size = bb_size,
+	};
+
+	vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_LR_MODE |
+			  DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE |
+			  DRM_XE_VM_CREATE_FLAG_FAULT_MODE, 0);
+
+	dst_buf.vm = vm;
+	cmdbuf.buf.vm = vm;
+	/* Submit a command to write 0x1a0000
+	 * Since 0x1a0000 is mapped scratch page, cmd execution should still
+	 * be successful. Write is either be dropped by HW (NULL PTE case) or
+	 * written to scratch page.
+	 */
+	xe_create_cmdbuf(&cmdbuf, eci);
+	xe_fill_cmdbuf(&cmdbuf, xe_cmdbuf_insert_store,
+		       dst_buf.gpu_addr, 0xc0ffee);
+	xe_close_cmdbuf(&cmdbuf);
+	xe_submit_cmd(&cmdbuf);
+
+	/* Create a buffer object, vm_bind it to 0x1a0000, then re-submit
+	 * the command buffer. This should write to the buffer object.
+	 * Check the buffer object to see if the write was successful.
+	 */
+	xe_create_buffer(&dst_buf);
+	xe_submit_cmd(&cmdbuf);
+	igt_assert_eq(*(uint64_t *)dst_buf.cpu_addr, 0xc0ffee);
+
+	xe_destroy_cmdbuf(&cmdbuf);
+	xe_destroy_buffer(&dst_buf);
+	xe_vm_destroy(fd, vm);
+}
+
 /**
  * SUBTEST: invalid-va
  * Description: Access invalid va and check for EIO through user fence.
@@ -458,6 +518,7 @@ igt_main
 		{ NULL },
 	};
 	int fd;
+	uint32_t dev_id;
 
 	igt_fixture {
 		struct timespec tv = {};
@@ -466,6 +527,7 @@ igt_main
 		int timeout = igt_run_in_simulation() ? 20 : 2;
 
 		fd = drm_open_driver(DRIVER_XE);
+		dev_id = intel_get_drm_devid(fd);
 		do {
 			if (ret)
 				usleep(5000);
@@ -508,6 +570,13 @@ igt_main
 		xe_for_each_engine(fd, hwe)
 			test_exec(fd, hwe, 1, 1, ENABLE_SCRATCH | INVALID_VA);
 
+	igt_describe("Check if scratch page works under fault mode");
+	igt_subtest("scratch-fault") {
+		igt_skip_on(!IS_LUNARLAKE(dev_id) && !IS_BATTLEMAGE(dev_id));
+		xe_for_each_engine(fd, hwe)
+			test_scratch(fd, hwe);
+	}
+
 	igt_fixture {
 		drm_close_driver(fd);
 	}
-- 
2.26.3