[PATCH i-g-t v3 0/7] Xe: Add tests for PXP

[PATCH i-g-t v3 0/7] Xe: Add tests for PXP

[Daniele Ceraolo Spurio]

Given that PXP introduces new interfaces and behaviors, we need to
update existing IGTs and add new ones to cover all the new scenarios.

Minor changes have been done to common functions to propagate the
PXP info and error returns.

v3: part of the v2 series has been split off and merged (see [1]) to
allow the kernel side to be merged without IGT failures. This is rebased
on top of that and addresses the review comments on v2 on the remaining
patches.

[1] https://patchwork.freedesktop.org/series/144158/
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>

Daniele Ceraolo Spurio (7):
  drm-uapi/xe: Sync with PXP uapi updates
  tests/intel/xe_vm: Update invalid flag subtest with valid PXP flag
  tests/intel/xe_query: Add test for PXP status query
  tests/intel/xe_pxp: Add PXP object and queue creation tests
  tests/intel/xe_pxp: Test PXP submissions
  tests/intel/xe_pxp: Termination tests
  tests/intel/xe_pxp: Test encrypted FBs

 include/drm-uapi/xe_drm.h |  123 +++-
 lib/intel_batchbuffer.c   |   45 +-
 lib/intel_batchbuffer.h   |    1 +
 lib/xe/xe_ioctl.c         |   26 +-
 lib/xe/xe_ioctl.h         |    4 +-
 tests/intel/xe_mmap.c     |    1 +
 tests/intel/xe_pxp.c      | 1173 +++++++++++++++++++++++++++++++++++++
 tests/intel/xe_query.c    |   61 ++
 tests/intel/xe_vm.c       |   23 +
 tests/meson.build         |    1 +
 10 files changed, 1435 insertions(+), 23 deletions(-)
 create mode 100644 tests/intel/xe_pxp.c

-- 
2.43.0

[PATCH i-g-t v3 1/7] drm-uapi/xe: Sync with PXP uapi updates

[Daniele Ceraolo Spurio]

Align with kernel commit 41a97c4a1294 ("drm/xe/pxp/uapi: Add API to
mark a BO as using PXP")

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 include/drm-uapi/xe_drm.h | 123 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 121 insertions(+), 2 deletions(-)

diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
index 08e263b3b..f0a4e0a3a 100644
--- a/include/drm-uapi/xe_drm.h
+++ b/include/drm-uapi/xe_drm.h
@@ -629,6 +629,39 @@ struct drm_xe_query_uc_fw_version {
 	__u64 reserved;
 };
 
+/**
+ * struct drm_xe_query_pxp_status - query if PXP is ready
+ *
+ * If PXP is enabled and no fatal error has occurred, the status will be set to
+ * one of the following values:
+ * 0: PXP init still in progress
+ * 1: PXP init complete
+ *
+ * If PXP is not enabled or something has gone wrong, the query will be failed
+ * with one of the following error codes:
+ * -ENODEV: PXP not supported or disabled;
+ * -EIO: fatal error occurred during init, so PXP will never be enabled;
+ * -EINVAL: incorrect value provided as part of the query;
+ * -EFAULT: error copying the memory between kernel and userspace.
+ *
+ * The status can only be 0 in the first few seconds after driver load. If
+ * everything works as expected, the status will transition to init complete in
+ * less than 1 second, while in case of errors the driver might take longer to
+ * start returning an error code, but it should still take less than 10 seconds.
+ *
+ * The supported session type bitmask is based on the values in
+ * enum drm_xe_pxp_session_type. TYPE_NONE is always supported and therefore
+ * is not reported in the bitmask.
+ *
+ */
+struct drm_xe_query_pxp_status {
+	/** @status: current PXP status */
+	__u32 status;
+
+	/** @supported_session_types: bitmask of supported PXP session types */
+	__u32 supported_session_types;
+};
+
 /**
  * struct drm_xe_device_query - Input of &DRM_IOCTL_XE_DEVICE_QUERY - main
  * structure to query device information
@@ -648,6 +681,7 @@ struct drm_xe_query_uc_fw_version {
  *    attributes.
  *  - %DRM_XE_DEVICE_QUERY_GT_TOPOLOGY
  *  - %DRM_XE_DEVICE_QUERY_ENGINE_CYCLES
+ *  - %DRM_XE_DEVICE_QUERY_PXP_STATUS
  *
  * If size is set to 0, the driver fills it with the required size for
  * the requested type of data to query. If size is equal to the required
@@ -700,6 +734,7 @@ struct drm_xe_device_query {
 #define DRM_XE_DEVICE_QUERY_ENGINE_CYCLES	6
 #define DRM_XE_DEVICE_QUERY_UC_FW_VERSION	7
 #define DRM_XE_DEVICE_QUERY_OA_UNITS		8
+#define DRM_XE_DEVICE_QUERY_PXP_STATUS		9
 	/** @query: The type of data to query */
 	__u32 query;
 
@@ -743,8 +778,23 @@ struct drm_xe_device_query {
  *  - %DRM_XE_GEM_CPU_CACHING_WC - Allocate the pages as write-combined. This
  *    is uncached. Scanout surfaces should likely use this. All objects
  *    that can be placed in VRAM must use this.
+ *
+ * This ioctl supports setting the following properties via the
+ * %DRM_XE_GEM_CREATE_EXTENSION_SET_PROPERTY extension, which uses the
+ * generic @drm_xe_ext_set_property struct:
+ *
+ *  - %DRM_XE_GEM_CREATE_SET_PROPERTY_PXP_TYPE - set the type of PXP session
+ *    this object will be used with. Valid values are listed in enum
+ *    drm_xe_pxp_session_type. %DRM_XE_PXP_TYPE_NONE is the default behavior, so
+ *    there is no need to explicitly set that. Objects used with session of type
+ *    %DRM_XE_PXP_TYPE_HWDRM will be marked as invalid if a PXP invalidation
+ *    event occurs after their creation. Attempting to flip an invalid object
+ *    will cause a black frame to be displayed instead. Submissions with invalid
+ *    objects mapped in the VM will be rejected.
  */
 struct drm_xe_gem_create {
+#define DRM_XE_GEM_CREATE_EXTENSION_SET_PROPERTY	0
+#define   DRM_XE_GEM_CREATE_SET_PROPERTY_PXP_TYPE	0
 	/** @extensions: Pointer to the first extension struct, if any */
 	__u64 extensions;
 
@@ -811,6 +861,32 @@ struct drm_xe_gem_create {
 
 /**
  * struct drm_xe_gem_mmap_offset - Input of &DRM_IOCTL_XE_GEM_MMAP_OFFSET
+ *
+ * The @flags can be:
+ *  - %DRM_XE_MMAP_OFFSET_FLAG_PCI_BARRIER - For user to query special offset
+ *    for use in mmap ioctl. Writing to the returned mmap address will generate a
+ *    PCI memory barrier with low overhead (avoiding IOCTL call as well as writing
+ *    to VRAM which would also add overhead), acting like an MI_MEM_FENCE
+ *    instruction.
+ *
+ * Note: The mmap size can be at most 4K, due to HW limitations. As a result
+ * this interface is only supported on CPU architectures that support 4K page
+ * size. The mmap_offset ioctl will detect this and gracefully return an
+ * error, where userspace is expected to have a different fallback method for
+ * triggering a barrier.
+ *
+ * Roughly the usage would be as follows:
+ *
+ * .. code-block:: C
+ *
+ *     struct drm_xe_gem_mmap_offset mmo = {
+ *         .handle = 0, // must be set to 0
+ *         .flags = DRM_XE_MMAP_OFFSET_FLAG_PCI_BARRIER,
+ *     };
+ *
+ *     err = ioctl(fd, DRM_IOCTL_XE_GEM_MMAP_OFFSET, &mmo);
+ *     map = mmap(NULL, size, PROT_WRITE, MAP_SHARED, fd, mmo.offset);
+ *     map[i] = 0xdeadbeaf; // issue barrier
  */
 struct drm_xe_gem_mmap_offset {
 	/** @extensions: Pointer to the first extension struct, if any */
@@ -819,7 +895,8 @@ struct drm_xe_gem_mmap_offset {
 	/** @handle: Handle for the object being mapped. */
 	__u32 handle;
 
-	/** @flags: Must be zero */
+#define DRM_XE_MMAP_OFFSET_FLAG_PCI_BARRIER     (1 << 0)
+	/** @flags: Flags */
 	__u32 flags;
 
 	/** @offset: The fake offset to use for subsequent mmap call */
@@ -906,6 +983,9 @@ struct drm_xe_vm_destroy {
  *    will only be valid for DRM_XE_VM_BIND_OP_MAP operations, the BO
  *    handle MBZ, and the BO offset MBZ. This flag is intended to
  *    implement VK sparse bindings.
+ *  - %DRM_XE_VM_BIND_FLAG_CHECK_PXP - If the object is encrypted via PXP,
+ *    reject the binding if the encryption key is no longer valid. This
+ *    flag has no effect on BOs that are not marked as using PXP.
  */
 struct drm_xe_vm_bind_op {
 	/** @extensions: Pointer to the first extension struct, if any */
@@ -996,6 +1076,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_CHECK_PXP	(1 << 4)
 	/** @flags: Bind flags */
 	__u32 flags;
 
@@ -1087,6 +1168,24 @@ struct drm_xe_vm_bind {
 /**
  * struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE
  *
+ * This ioctl supports setting the following properties via the
+ * %DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY extension, which uses the
+ * generic @drm_xe_ext_set_property struct:
+ *
+ *  - %DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY - set the queue priority.
+ *    CAP_SYS_NICE is required to set a value above normal.
+ *  - %DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE - set the queue timeslice
+ *    duration in microseconds.
+ *  - %DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE - set the type of PXP session
+ *    this queue will be used with. Valid values are listed in enum
+ *    drm_xe_pxp_session_type. %DRM_XE_PXP_TYPE_NONE is the default behavior, so
+ *    there is no need to explicitly set that. When a queue of type
+ *    %DRM_XE_PXP_TYPE_HWDRM is created, the PXP default HWDRM session
+ *    (%XE_PXP_HWDRM_DEFAULT_SESSION) will be started, if isn't already running.
+ *    Given that going into a power-saving state kills PXP HWDRM sessions,
+ *    runtime PM will be blocked while queues of this type are alive.
+ *    All PXP queues will be killed if a PXP invalidation event occurs.
+ *
  * The example below shows how to use @drm_xe_exec_queue_create to create
  * a simple exec_queue (no parallel submission) of class
  * &DRM_XE_ENGINE_CLASS_RENDER.
@@ -1110,7 +1209,7 @@ struct drm_xe_exec_queue_create {
 #define DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY		0
 #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY		0
 #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE		1
-
+#define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE		2
 	/** @extensions: Pointer to the first extension struct, if any */
 	__u64 extensions;
 
@@ -1729,6 +1828,26 @@ struct drm_xe_oa_stream_info {
 	__u64 reserved[3];
 };
 
+/**
+ * enum drm_xe_pxp_session_type - Supported PXP session types.
+ *
+ * We currently only support HWDRM sessions, which are used for protected
+ * content that ends up being displayed, but the HW supports multiple types, so
+ * we might extend support in the future.
+ */
+enum drm_xe_pxp_session_type {
+	/** @DRM_XE_PXP_TYPE_NONE: PXP not used */
+	DRM_XE_PXP_TYPE_NONE = 0,
+	/**
+	 * @DRM_XE_PXP_TYPE_HWDRM: HWDRM sessions are used for content that ends
+	 * up on the display.
+	 */
+	DRM_XE_PXP_TYPE_HWDRM = 1,
+};
+
+/* ID of the protected content session managed by Xe when PXP is active */
+#define DRM_XE_PXP_HWDRM_DEFAULT_SESSION 0xf
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.43.0

[PATCH i-g-t v3 2/7] tests/intel/xe_vm: Update invalid flag subtest with valid PXP flag

[Daniele Ceraolo Spurio]

The invalid flag subtest also checks all the valid flags, so add the new
PXP flag to it.

v2: only test the new flag if the kernel supports it.
v3: rebase on the split of the update on the first invalid flag number
to its own patch.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #v2
---
 tests/intel/xe_vm.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/intel/xe_vm.c b/tests/intel/xe_vm.c
index 0730dd3d3..40e92d805 100644
--- a/tests/intel/xe_vm.c
+++ b/tests/intel/xe_vm.c
@@ -2173,6 +2173,22 @@ test_mmap_style_bind(int fd, struct drm_xe_engine_class_instance *eci,
 	xe_vm_destroy(fd, vm);
 }
 
+static bool pxp_interface_supported(int fd)
+{
+	struct drm_xe_device_query query = {
+		.extensions = 0,
+		.query = DRM_XE_DEVICE_QUERY_PXP_STATUS,
+		.size = 0,
+		.data = 0,
+	};
+	int ret = 0;
+
+	if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
+		ret = -errno;
+
+	return ret != -EINVAL;
+}
+
 /**
  * SUBTEST: bind-flag-invalid
  * Description:
@@ -2221,6 +2237,13 @@ static void bind_flag_invalid(int fd)
 	igt_assert(syncobj_wait(fd, &sync[0].handle, 1, INT64_MAX, 0, NULL));
 	syncobj_reset(fd, &sync[0].handle, 1);
 
+	if (pxp_interface_supported(fd)) {
+		bind.bind.flags = DRM_XE_VM_BIND_FLAG_CHECK_PXP;
+		igt_ioctl(fd, DRM_IOCTL_XE_VM_BIND, &bind);
+		igt_assert(syncobj_wait(fd, &sync[0].handle, 1, INT64_MAX, 0, NULL));
+		syncobj_reset(fd, &sync[0].handle, 1);
+	}
+
 	bind.bind.flags = DRM_XE_VM_BIND_FLAG_NULL;
 	bind.bind.obj = 0;
 	igt_ioctl(fd, DRM_IOCTL_XE_VM_BIND, &bind);
-- 
2.43.0

[PATCH i-g-t v3 3/7] tests/intel/xe_query: Add test for PXP status query

[Daniele Ceraolo Spurio]

Add a new test to exercise the PXP status query.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/intel/xe_query.c | 61 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/tests/intel/xe_query.c b/tests/intel/xe_query.c
index 1566680e7..0a2ff8d7f 100644
--- a/tests/intel/xe_query.c
+++ b/tests/intel/xe_query.c
@@ -1077,6 +1077,66 @@ static void test_query_oa_units(int fd)
 	}
 }
 
+/**
+ * SUBTEST: query-pxp-status
+ * Description: Display PXP supported types and current status
+ *
+ * SUBTEST: multigpu-query-pxp-status
+ * Description: Display fields for PXP unit query for all Xe devices
+ * Sub-category: MultiGPU
+ */
+static void test_query_pxp_status(int fd)
+{
+	struct drm_xe_query_pxp_status *qpxp;
+	struct drm_xe_device_query query = {
+		.extensions = 0,
+		.query = DRM_XE_DEVICE_QUERY_PXP_STATUS,
+		.size = 0,
+		.data = 0,
+	};
+	int ret;
+
+	/*
+	 * if we run this test on an older kernel that doesn't have the PXP
+	 * query, the ioctl will return -EINVAL.
+	 */
+	errno = 0;
+	ret = igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
+	igt_require(errno != EINVAL);
+	igt_assert_eq(ret, 0);
+
+	/* make sure the returned size is big enough */
+	igt_assert(query.size >= sizeof(*qpxp));
+
+	qpxp = malloc(query.size);
+	igt_assert(qpxp);
+
+	memset(qpxp, 0, query.size);
+
+	query.data = to_user_pointer(qpxp);
+	ret = igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
+	if (ret == -ENODEV) {
+		igt_info("PXP not supported\n");
+		return;
+	}
+
+	igt_assert_eq(ret, 0);
+	igt_assert_neq(qpxp->supported_session_types, 0);
+
+	switch (qpxp->status) {
+	case 0:
+		igt_info("PXP initialization still in progress\n");
+		break;
+	case 1:
+		igt_info("PXP initialization complete\n");
+		break;
+	default:
+		igt_assert_f(0, "unexpected PXP status %u\n", qpxp->status);
+	}
+
+	igt_info("PXP supported types mask 0x%x\n", qpxp->supported_session_types);
+}
+
 igt_main
 {
 	const struct {
@@ -1094,6 +1154,7 @@ igt_main
 		{ "query-uc-fw-version-guc", test_query_uc_fw_version_guc },
 		{ "query-uc-fw-version-huc", test_query_uc_fw_version_huc },
 		{ "query-oa-units", test_query_oa_units },
+		{ "query-pxp-status", test_query_pxp_status },
 		{ "query-invalid-cs-cycles", test_engine_cycles_invalid },
 		{ "query-invalid-query", test_query_invalid_query },
 		{ "query-invalid-size", test_query_invalid_size },
-- 
2.43.0

[PATCH i-g-t v3 4/7] tests/intel/xe_pxp: Add PXP object and queue creation tests

[Daniele Ceraolo Spurio]

PXP support introduces new SET_PROPERTY extensions to both BOs and
exec_queues to mark them as being used for PXP workloads, so we need to
test both correct and incorrect usage of those new interfaces.

Since this is the first usage of extensions for BO creation, the
common BO code has been update to support the extra parameter.

v2: fix memory lead, remove unneeded igt_require (Alan)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/xe/xe_ioctl.c     |  14 +--
 lib/xe/xe_ioctl.h     |   2 +-
 tests/intel/xe_mmap.c |   1 +
 tests/intel/xe_pxp.c  | 201 ++++++++++++++++++++++++++++++++++++++++++
 tests/meson.build     |   1 +
 5 files changed, 213 insertions(+), 6 deletions(-)
 create mode 100644 tests/intel/xe_pxp.c

diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
index 6d8388918..01ab7c758 100644
--- a/lib/xe/xe_ioctl.c
+++ b/lib/xe/xe_ioctl.c
@@ -264,7 +264,8 @@ static bool vram_selected(int fd, uint32_t selected_regions)
 }
 
 static uint32_t ___xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
-				uint32_t flags, uint16_t cpu_caching, uint32_t *handle)
+				uint32_t flags, uint16_t cpu_caching, void *ext,
+				uint32_t *handle)
 {
 	struct drm_xe_gem_create create = {
 		.vm_id = vm,
@@ -275,6 +276,9 @@ static uint32_t ___xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t pla
 	};
 	int err;
 
+	if (ext)
+		create.extensions = to_user_pointer(ext);
+
 	/*
 	 * In case vram_if_possible returned system_memory,
 	 * visible VRAM cannot be requested through flags
@@ -292,11 +296,11 @@ static uint32_t ___xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t pla
 }
 
 uint32_t __xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
-			uint32_t flags, uint32_t *handle)
+			uint32_t flags, void *ext, uint32_t *handle)
 {
 	uint16_t cpu_caching = __xe_default_cpu_caching(fd, placement, flags);
 
-	return ___xe_bo_create(fd, vm, size, placement, flags, cpu_caching, handle);
+	return ___xe_bo_create(fd, vm, size, placement, flags, cpu_caching, ext, handle);
 }
 
 uint32_t xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
@@ -304,7 +308,7 @@ uint32_t xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
 {
 	uint32_t handle;
 
-	igt_assert_eq(__xe_bo_create(fd, vm, size, placement, flags, &handle), 0);
+	igt_assert_eq(__xe_bo_create(fd, vm, size, placement, flags, NULL, &handle), 0);
 
 	return handle;
 }
@@ -312,7 +316,7 @@ uint32_t xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
 uint32_t __xe_bo_create_caching(int fd, uint32_t vm, uint64_t size, uint32_t placement,
 				uint32_t flags, uint16_t cpu_caching, uint32_t *handle)
 {
-	return ___xe_bo_create(fd, vm, size, placement, flags, cpu_caching, handle);
+	return ___xe_bo_create(fd, vm, size, placement, flags, cpu_caching, NULL, handle);
 }
 
 uint32_t xe_bo_create_caching(int fd, uint32_t vm, uint64_t size, uint32_t placement,
diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
index 18cc2b72b..c8a2d81c5 100644
--- a/lib/xe/xe_ioctl.h
+++ b/lib/xe/xe_ioctl.h
@@ -67,7 +67,7 @@ void xe_vm_unbind_all_async(int fd, uint32_t vm, uint32_t exec_queue,
 			    uint32_t num_syncs);
 void xe_vm_destroy(int fd, uint32_t vm);
 uint32_t __xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
-			uint32_t flags, uint32_t *handle);
+			uint32_t flags, void *ext, uint32_t *handle);
 uint32_t xe_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
 		      uint32_t flags);
 uint32_t __xe_bo_create_caching(int fd, uint32_t vm, uint64_t size, uint32_t placement,
diff --git a/tests/intel/xe_mmap.c b/tests/intel/xe_mmap.c
index 72dc16fc7..5fd641075 100644
--- a/tests/intel/xe_mmap.c
+++ b/tests/intel/xe_mmap.c
@@ -298,6 +298,7 @@ static void test_small_bar(int fd)
 	igt_assert_neq(__xe_bo_create(fd, 0, visible_size + page_size,
 				      vram_memory(fd, 0),
 				      DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
+				      NULL,
 				      &bo),
 		       0);
 
diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
new file mode 100644
index 000000000..234fa8782
--- /dev/null
+++ b/tests/intel/xe_pxp.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+#include "igt.h"
+#include "xe_drm.h"
+#include "xe/xe_ioctl.h"
+#include "xe/xe_query.h"
+
+IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
+/* Note: PXP = "Protected Xe Path" */
+
+/**
+ * TEST: Test PXP functionality
+ * Category: Content protection
+ * Mega feature: PXP
+ * Sub-category: PXP tests
+ * Functionality: Execution of protected content
+ * Test category: functionality test
+ */
+
+static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size,
+			   uint32_t session_type, uint32_t *handle)
+{
+	struct drm_xe_ext_set_property ext = {
+		.base.next_extension = 0,
+		.base.name = DRM_XE_GEM_CREATE_EXTENSION_SET_PROPERTY,
+		.property = DRM_XE_GEM_CREATE_SET_PROPERTY_PXP_TYPE,
+		.value = session_type,
+	};
+	int ret = 0;
+
+	if (__xe_bo_create(fd, vm, size, system_memory(fd), 0, &ext, handle)) {
+		ret = -errno;
+		errno = 0;
+	}
+
+	return ret;
+}
+
+static int __create_pxp_rcs_queue(int fd, uint32_t vm,
+				  uint32_t session_type,
+				  uint32_t *q)
+{
+	struct drm_xe_engine_class_instance inst = {
+		.engine_class = DRM_XE_ENGINE_CLASS_RENDER,
+	};
+	struct drm_xe_ext_set_property ext = { 0 };
+	uint64_t ext_ptr = to_user_pointer(&ext);
+
+	ext.base.next_extension = 0,
+	ext.base.name = DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY,
+	ext.property = DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE,
+	ext.value = session_type;
+
+	return __xe_exec_queue_create(fd, vm, 1, 1, &inst, ext_ptr, q);
+}
+
+static int query_pxp_status(int fd)
+{
+	struct drm_xe_query_pxp_status *pxp_query;
+	struct drm_xe_device_query query = {
+		.extensions = 0,
+		.query = DRM_XE_DEVICE_QUERY_PXP_STATUS,
+		.size = 0,
+		.data = 0,
+	};
+	int ret;
+
+	if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
+		return -errno;
+
+	pxp_query = malloc(query.size);
+	igt_assert(pxp_query);
+	memset(pxp_query, 0, query.size);
+
+	query.data = to_user_pointer(pxp_query);
+
+	if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
+		ret = -errno;
+	else
+		ret = pxp_query->status;
+
+	free(pxp_query);
+
+	return ret;
+}
+
+static bool is_pxp_hw_supported(int fd)
+{
+	int pxp_status;
+	int i = 0;
+
+	/* PXP init completes after driver init, so we might have to wait for it */
+	while (i++ < 50) {
+		pxp_status = query_pxp_status(fd);
+
+		/* -EINVAL means the PXP interface is not available */
+		igt_require(pxp_status != -EINVAL);
+
+		/* -ENODEV means PXP not supported or disabled */
+		if (pxp_status == -ENODEV)
+			return false;
+
+		/* status 1 means pxp is ready */
+		if (pxp_status == 1)
+			return true;
+
+		/*
+		 * 0 means init still in progress, any other remaining state
+		 * is an error
+		 */
+		igt_assert_eq(pxp_status, 0);
+
+		usleep(50*1000);
+	}
+
+	igt_assert_f(0, "PXP failed to initialize within the timeout\n");
+	return false;
+}
+
+/**
+ * SUBTEST: pxp-bo-alloc
+ * Description: Verify PXP bo allocation works as expected
+ */
+static void test_pxp_bo_alloc(int fd, bool pxp_supported)
+{
+	uint32_t bo;
+	int ret;
+
+	/* BO creation with DRM_XE_PXP_TYPE_NONE must always succeed */
+	ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_NONE, &bo);
+	igt_assert_eq(ret, 0);
+	gem_close(fd, bo);
+
+	/* BO creation with DRM_XE_PXP_TYPE_HWDRM must only succeed if PXP is supported */
+	ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_HWDRM, &bo);
+	igt_assert_eq(ret, pxp_supported ? 0 : -ENODEV);
+	if (!ret)
+		gem_close(fd, bo);
+
+	/* BO creation with an invalid type must always fail */
+	ret = __pxp_bo_create(fd, 0, 4096, 0xFF, &bo);
+	igt_assert_eq(ret, -EINVAL);
+}
+
+/**
+ * SUBTEST: pxp-queue-alloc
+ * Description: Verify PXP exec queue creation works as expected
+ */
+static void test_pxp_queue_creation(int fd, bool pxp_supported)
+{
+	uint32_t q;
+	uint32_t vm;
+	int ret;
+
+	vm = xe_vm_create(fd, 0, 0);
+
+	/* queue creation with DRM_XE_PXP_TYPE_NONE must always succeed */
+	ret = __create_pxp_rcs_queue(fd, vm, DRM_XE_PXP_TYPE_NONE, &q);
+	igt_assert_eq(ret, 0);
+	xe_exec_queue_destroy(fd, q);
+
+	/* queue creation with DRM_XE_PXP_TYPE_HWDRM must only succeed if PXP is supported */
+	ret = __create_pxp_rcs_queue(fd, vm, DRM_XE_PXP_TYPE_HWDRM, &q);
+	igt_assert_eq(ret, pxp_supported ? 0 : -ENODEV);
+	if (!ret)
+		xe_exec_queue_destroy(fd, q);
+
+	/* queue creation with an invalid type must always fail */
+	ret = __create_pxp_rcs_queue(fd, vm, 0xFF, &q);
+	igt_assert_eq(ret, -EINVAL);
+
+	xe_vm_destroy(fd, vm);
+}
+
+igt_main
+{
+	int xe_fd = -1;
+	bool pxp_supported = true;
+
+	igt_fixture {
+		xe_fd = drm_open_driver(DRIVER_XE);
+		igt_require(xe_has_engine_class(xe_fd, DRM_XE_ENGINE_CLASS_RENDER));
+		pxp_supported = is_pxp_hw_supported(xe_fd);
+	}
+
+	igt_subtest_group {
+		igt_describe("Verify PXP allocations work as expected");
+		igt_subtest("pxp-bo-alloc")
+		test_pxp_bo_alloc(xe_fd, pxp_supported);
+
+		igt_subtest("pxp-queue-alloc")
+		test_pxp_queue_creation(xe_fd, pxp_supported);
+	}
+
+	igt_fixture {
+		close(xe_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 33dffad31..2c9f115f5 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -311,6 +311,7 @@ intel_xe_progs = [
 	'xe_pm',
 	'xe_pm_residency',
 	'xe_prime_self_import',
+	'xe_pxp',
 	'xe_query',
 	'xe_render_copy',
 	'xe_vm',
-- 
2.43.0

[PATCH i-g-t v3 5/7] tests/intel/xe_pxp: Test PXP submissions

[Daniele Ceraolo Spurio]

The render supports PXP usage via rendercopy. We can use this to test
that a user is able to correctly encrypt their data. In particular, we
cover these 2 scenarios:

1) copy from clear to encrypted - we expect the dest buffer to not match
   the src one due to the encryption.

2) copy from encrypted to encrypted = we expect the 2 BOs to match since
   they hold the same data encrypted with the same key.

Note that the clear to clear copy is already covered by the
xe_render_copy test, while encrypted to clear is obviously not a
supported case.

Since the render_copy uses the intel_batchbuffer helpers, those helpers
have been updated to support vm bind of protected objects.

v2: move igt_require calls inside the subtest
v3: Better comments for rsvd1 overload, rename variables for clarity,
updated commit msg.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/intel_batchbuffer.c |  37 ++++--
 tests/intel/xe_pxp.c    | 275 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 304 insertions(+), 8 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 72bbbf8c6..ee499ee7f 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -1332,8 +1332,19 @@ void intel_bb_destroy(struct intel_bb *ibb)
 	free(ibb);
 }
 
+/*
+ * Since we re-use drm_i915_gem_exec_object2 to store details about the object
+ * for submission to Xe, we don't have dedicated fields for the info needed for
+ * binding the object, so we store it into rsvd1. The data is stores as follows:
+ *   Bits 63-12: Objects size in multiples of SZ_4K (i.e. size / SZ_4K)
+ *   Bits 9-11: Unused
+ *   Bit 8: Object protected
+ *   Bits 0-7: Pat index
+ */
 #define XE_OBJ_SIZE(rsvd1) ((rsvd1) & ~(SZ_4K-1))
-#define XE_OBJ_PAT_IDX(rsvd1) ((rsvd1) & (SZ_4K-1))
+#define XE_OBJ_PAT_IDX(rsvd1) ((rsvd1) & (0xFF))
+#define XE_OBJ_PXP_BIT (0x100)
+#define XE_OBJ_PXP(rsvd1) ((rsvd1) & (XE_OBJ_PXP_BIT))
 
 static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct intel_bb *ibb,
 						   uint32_t op, uint32_t flags,
@@ -1355,6 +1366,9 @@ static struct drm_xe_vm_bind_op *xe_alloc_bind_ops(struct intel_bb *ibb,
 
 		ops->op = op;
 		ops->flags = flags;
+
+		if (XE_OBJ_PXP(objects[i]->rsvd1))
+			ops->flags |= DRM_XE_VM_BIND_FLAG_CHECK_PXP;
 		ops->obj_offset = 0;
 		ops->addr = objects[i]->offset;
 		ops->range = XE_OBJ_SIZE(objects[i]->rsvd1);
@@ -1745,7 +1759,7 @@ static void __remove_from_objects(struct intel_bb *ibb,
 static struct drm_i915_gem_exec_object2 *
 __intel_bb_add_object(struct intel_bb *ibb, uint32_t handle, uint64_t size,
 		      uint64_t offset, uint64_t alignment, uint8_t pat_index,
-		      bool write)
+		      bool protected, bool write)
 {
 	struct drm_i915_gem_exec_object2 *object;
 
@@ -1822,17 +1836,24 @@ __intel_bb_add_object(struct intel_bb *ibb, uint32_t handle, uint64_t size,
 		object->alignment = alignment;
 		object->rsvd1 = size;
 		igt_assert(!XE_OBJ_PAT_IDX(object->rsvd1));
+		igt_assert(!XE_OBJ_PXP(object->rsvd1));
 
 		if (pat_index == DEFAULT_PAT_INDEX)
 			pat_index = intel_get_pat_idx_wb(ibb->fd);
 
 		/*
-		 * XXX: For now encode the pat_index in the first few bits of
-		 * rsvd1. intel_batchbuffer should really stop using the i915
-		 * drm_i915_gem_exec_object2 to encode VMA placement
-		 * information on xe...
+		 * XXX: For now encode the pat_index and whether the bo is
+		 * protected in the first few bits of rsvd1 (8 bits for the
+		 * pat and 1 bit for the flag, see comment above
+		 * xe_alloc_bind_ops for the full map).
+		 * intel_batchbuffer should really stop using the i915
+		 * drm_i915_gem_exec_object2 to encode VMA placement information
+		 * on xe...
 		 */
 		object->rsvd1 |= pat_index;
+
+		if (protected)
+			object->rsvd1 |= XE_OBJ_PXP_BIT;
 	}
 
 	return object;
@@ -1845,7 +1866,7 @@ intel_bb_add_object(struct intel_bb *ibb, uint32_t handle, uint64_t size,
 	struct drm_i915_gem_exec_object2 *obj = NULL;
 
 	obj = __intel_bb_add_object(ibb, handle, size, offset,
-				    alignment, DEFAULT_PAT_INDEX, write);
+				    alignment, DEFAULT_PAT_INDEX, false, write);
 	igt_assert(obj);
 
 	return obj;
@@ -1909,7 +1930,7 @@ __intel_bb_add_intel_buf(struct intel_bb *ibb, struct intel_buf *buf,
 
 	obj = __intel_bb_add_object(ibb, buf->handle, intel_buf_bo_size(buf),
 				    buf->addr.offset, alignment, buf->pat_index,
-				    write);
+				    buf->is_protected, write);
 	igt_assert(obj);
 	buf->addr.offset = obj->offset;
 
diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
index 234fa8782..2ab920d07 100644
--- a/tests/intel/xe_pxp.c
+++ b/tests/intel/xe_pxp.c
@@ -4,6 +4,10 @@
  */
 
 #include "igt.h"
+#include "intel_batchbuffer.h"
+#include "intel_bufops.h"
+#include "intel_mocs.h"
+#include "intel_pat.h"
 #include "xe_drm.h"
 #include "xe/xe_ioctl.h"
 #include "xe/xe_query.h"
@@ -39,6 +43,15 @@ static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size,
 	return ret;
 }
 
+static uint32_t pxp_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t type)
+{
+	uint32_t handle;
+
+	igt_assert_eq(__pxp_bo_create(fd, vm, size, type, &handle), 0);
+
+	return handle;
+}
+
 static int __create_pxp_rcs_queue(int fd, uint32_t vm,
 				  uint32_t session_type,
 				  uint32_t *q)
@@ -57,6 +70,17 @@ static int __create_pxp_rcs_queue(int fd, uint32_t vm,
 	return __xe_exec_queue_create(fd, vm, 1, 1, &inst, ext_ptr, q);
 }
 
+static uint32_t create_pxp_rcs_queue(int fd, uint32_t vm)
+{
+	uint32_t q;
+	int err;
+
+	err = __create_pxp_rcs_queue(fd, vm, DRM_XE_PXP_TYPE_HWDRM, &q);
+	igt_assert_eq(err, 0);
+
+	return q;
+}
+
 static int query_pxp_status(int fd)
 {
 	struct drm_xe_query_pxp_status *pxp_query;
@@ -175,13 +199,252 @@ static void test_pxp_queue_creation(int fd, bool pxp_supported)
 	xe_vm_destroy(fd, vm);
 }
 
+static void fill_bo_content(int fd, uint32_t bo, uint32_t size, uint8_t initcolor)
+{
+	uint32_t *ptr;
+
+	ptr = xe_bo_mmap_ext(fd, bo, size, PROT_READ|PROT_WRITE);
+
+	/* read and count all dword matches till size */
+	memset(ptr, initcolor, size);
+
+	igt_assert(munmap(ptr, size) == 0);
+}
+
+static void __check_bo_color(int fd, uint32_t bo, uint32_t size, uint32_t color, bool should_match)
+{
+	uint64_t comp;
+	uint64_t *ptr;
+	int i, num_matches = 0;
+
+	comp = color;
+	comp = comp | (comp << 32);
+
+	ptr =  xe_bo_mmap_ext(fd, bo, size, PROT_READ);
+
+	igt_assert_eq(size % sizeof(uint64_t), 0);
+
+	for (i = 0; i < (size / sizeof(uint64_t)); i++)
+		if (ptr[i] == comp)
+			++num_matches;
+
+	if (should_match)
+		igt_assert_eq(num_matches, (size / sizeof(uint64_t)));
+	else
+		igt_assert_eq(num_matches, 0);
+}
+
+static void check_bo_color(int fd, uint32_t bo, uint32_t size, uint8_t color, bool should_match)
+{
+	uint32_t comp;
+
+	/*
+	 * We memset the buffer using a u8 color value. However, this is too
+	 * small to ensure the encrypted data does not accidentally match it,
+	 * so we scale it up to a bigger size.
+	 */
+	comp = color;
+	comp = comp | (comp << 8) | (comp << 16) | (comp << 24);
+
+	return __check_bo_color(fd, bo, size, comp, should_match);
+}
+
+static uint32_t __bo_create_and_fill(int fd, uint32_t vm, bool protected,
+				     uint32_t size, uint8_t init_color)
+{
+	uint32_t bo;
+
+	if (protected)
+		bo = pxp_bo_create(fd, vm, size, DRM_XE_PXP_TYPE_HWDRM);
+	else
+		bo = xe_bo_create(fd, vm, size, system_memory(fd), 0);
+
+	fill_bo_content(fd, bo, size, init_color);
+
+	return bo;
+}
+
+static uint32_t pxp_bo_create_and_fill(int fd, uint32_t vm, uint32_t size,
+				       uint8_t init_color)
+{
+	return __bo_create_and_fill(fd, vm, true, size, init_color);
+}
+
+static uint32_t regular_bo_create_and_fill(int fd, uint32_t vm, uint32_t size,
+					   uint8_t init_color)
+{
+	return __bo_create_and_fill(fd, vm, false, size, init_color);
+}
+
+static struct intel_buf *buf_create(int fd, struct buf_ops *bops, uint32_t handle,
+				    int width, int height, int bpp, uint64_t size)
+{
+	igt_assert(handle);
+	igt_assert(size);
+	return intel_buf_create_full(bops, handle, width, height, bpp, 0,
+				     I915_TILING_NONE, 0, size, 0,
+				     system_memory(fd),
+				     DEFAULT_PAT_INDEX, DEFAULT_MOCS_INDEX);
+}
+
+/* Rendering tests surface attributes */
+#define TSTSURF_WIDTH		64
+#define TSTSURF_HEIGHT		64
+#define TSTSURF_BYTESPP		4
+#define TSTSURF_STRIDE		(TSTSURF_WIDTH * TSTSURF_BYTESPP)
+#define TSTSURF_SIZE		(TSTSURF_STRIDE * TSTSURF_HEIGHT)
+#define TSTSURF_INITCOLOR1  0xAA
+#define TSTSURF_FILLCOLOR1  0x55
+#define TSTSURF_INITCOLOR2  0x33
+
+static void pxp_rendercopy(int fd, uint32_t q, uint32_t vm, uint32_t copy_size,
+			   uint32_t srcbo, bool src_pxp, uint32_t dstbo, bool dst_pxp)
+{
+	igt_render_copyfunc_t render_copy;
+	struct intel_buf *srcbuf, *dstbuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
+
+	/*
+	 * we use the defined width and height below, which only works if the BO
+	 * size is TSTSURF_SIZE
+	 */
+	igt_assert_eq(copy_size, TSTSURF_SIZE);
+
+	render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
+	igt_assert(render_copy);
+
+	bops = buf_ops_create(fd);
+	igt_assert(bops);
+
+	ibb = intel_bb_create_with_context(fd, q, vm, NULL, 4096);
+	igt_assert(ibb);
+	intel_bb_set_pxp(ibb, true, DISPLAY_APPTYPE, DRM_XE_PXP_HWDRM_DEFAULT_SESSION);
+
+	dstbuf = buf_create(fd, bops, dstbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+			    TSTSURF_BYTESPP * 8, TSTSURF_SIZE);
+	intel_buf_set_pxp(dstbuf, dst_pxp);
+
+	srcbuf = buf_create(fd, bops, srcbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+			    TSTSURF_BYTESPP * 8, TSTSURF_SIZE);
+	intel_buf_set_pxp(srcbuf, src_pxp);
+
+	render_copy(ibb, srcbuf, 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT, dstbuf, 0, 0);
+	intel_bb_sync(ibb);
+
+	intel_buf_destroy(srcbuf);
+	intel_buf_destroy(dstbuf);
+	intel_bb_destroy(ibb);
+	buf_ops_destroy(bops);
+}
+
+/**
+ * SUBTEST: regular-src-to-pxp-dest-rendercopy
+ * Description: copy from a regular BO to a PXP one and verify the encryption
+ */
+static void test_render_regular_src_to_pxp_dest(int fd)
+{
+	uint32_t vm, srcbo, dstbo;
+	uint32_t q;
+
+	vm = xe_vm_create(fd, 0, 0);
+
+	/*
+	 * Perform a protected render operation but only label the dest as
+	 * protected. After rendering, the content should be encrypted.
+	 */
+	q = create_pxp_rcs_queue(fd, vm);
+
+	srcbo = regular_bo_create_and_fill(fd, vm, TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+	dstbo = pxp_bo_create_and_fill(fd, vm, TSTSURF_SIZE, TSTSURF_INITCOLOR1);
+
+	pxp_rendercopy(fd, q, vm, TSTSURF_SIZE, srcbo, false, dstbo, true);
+
+	check_bo_color(fd, dstbo, TSTSURF_SIZE, TSTSURF_FILLCOLOR1, false);
+
+	gem_close(fd, srcbo);
+	gem_close(fd, dstbo);
+	xe_exec_queue_destroy(fd, q);
+	xe_vm_destroy(fd, vm);
+}
+
+static int bocmp(int fd, uint32_t bo1, uint32_t bo2, uint32_t size)
+{
+	uint32_t *ptr1, *ptr2;
+	int ret;
+
+	ptr1 = xe_bo_mmap_ext(fd, bo1, size, PROT_READ);
+	ptr2 = xe_bo_mmap_ext(fd, bo2, size, PROT_READ);
+
+	ret = memcmp(ptr1, ptr2, size);
+
+	igt_assert_eq(munmap(ptr1, size), 0);
+	igt_assert_eq(munmap(ptr2, size), 0);
+
+	return ret;
+}
+
+/**
+ * SUBTEST: pxp-src-to-pxp-dest-rendercopy
+ * Description: copy between 2 PXP BOs and verify the encryption
+ */
+
+static void test_render_pxp_protsrc_to_protdest(int fd)
+{
+	uint32_t vm, srcbo, dstbo, dstbo2;
+	uint32_t q;
+
+	vm = xe_vm_create(fd, 0, 0);
+
+	q = create_pxp_rcs_queue(fd, vm);
+
+	/*
+	 * Copy from a regular src to a PXP dst to get a buffer with a
+	 * valid encryption.
+	 */
+	srcbo = regular_bo_create_and_fill(fd, vm, TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+	dstbo = pxp_bo_create_and_fill(fd, vm, TSTSURF_SIZE, TSTSURF_INITCOLOR1);
+
+	pxp_rendercopy(fd, q, vm, TSTSURF_SIZE, srcbo, false, dstbo, true);
+
+	check_bo_color(fd, dstbo, TSTSURF_SIZE, TSTSURF_FILLCOLOR1, false);
+
+	/*
+	 * Reuse prior dst as the new-src and create dst2 as the new-dest.
+	 * After the rendering, we should find no difference in content since
+	 * both new-src and new-dest are labelled as encrypted. HW should read
+	 * and decrypt new-src, perform the copy and re-encrypt with the same
+	 * key when going into new-dest
+	 */
+	dstbo2 = pxp_bo_create_and_fill(fd, vm, TSTSURF_SIZE, TSTSURF_INITCOLOR2);
+
+	pxp_rendercopy(fd, q, vm, TSTSURF_SIZE, dstbo, true, dstbo2, true);
+
+	igt_assert_eq(bocmp(fd, dstbo, dstbo2, TSTSURF_SIZE), 0);
+
+	gem_close(fd, srcbo);
+	gem_close(fd, dstbo);
+	gem_close(fd, dstbo2);
+	xe_exec_queue_destroy(fd, q);
+	xe_vm_destroy(fd, vm);
+}
+
+static void require_pxp_render(bool pxp_supported, uint32_t devid)
+{
+	igt_require_f(pxp_supported, "PXP not supported\n");
+	igt_require_f(igt_get_render_copyfunc(devid),
+		      "No rendercopy found for devid=%x\n", devid);
+}
+
 igt_main
 {
 	int xe_fd = -1;
 	bool pxp_supported = true;
+	uint32_t devid = 0;
 
 	igt_fixture {
 		xe_fd = drm_open_driver(DRIVER_XE);
+		devid = intel_get_drm_devid(xe_fd);
 		igt_require(xe_has_engine_class(xe_fd, DRM_XE_ENGINE_CLASS_RENDER));
 		pxp_supported = is_pxp_hw_supported(xe_fd);
 	}
@@ -195,6 +458,18 @@ igt_main
 		test_pxp_queue_creation(xe_fd, pxp_supported);
 	}
 
+	igt_subtest_group {
+		igt_describe("Verify protected render operations:");
+		igt_subtest("regular-src-to-pxp-dest-rendercopy") {
+			require_pxp_render(pxp_supported, devid);
+			test_render_regular_src_to_pxp_dest(xe_fd);
+		}
+		igt_subtest("pxp-src-to-pxp-dest-rendercopy") {
+			require_pxp_render(pxp_supported, devid);
+			test_render_pxp_protsrc_to_protdest(xe_fd);
+		}
+	}
+
 	igt_fixture {
 		close(xe_fd);
 	}
-- 
2.43.0

[PATCH i-g-t v3 6/7] tests/intel/xe_pxp: Termination tests

[Daniele Ceraolo Spurio]

There are several events that can cause the PXP key to be invalidated
and trigger a PXP termination (suspend, PXP termination irq). After a
termination, we expect the key to be different and the raw encrypted
data to change for the same source data.
Additionally, all PXP objects are invalidated during a termination and
can no longer be used in submission or kept mapped to VMs; we therefore
need to test both the execution and bind ioctls to make sure they work
as expected after a termination.

v2: move igt_require calls inside the subtest
v3: block rpm for tests trying a different type of termination, rework
invalid bind test to try a new vm as well (Alan)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/intel_batchbuffer.c |   8 +-
 lib/intel_batchbuffer.h |   1 +
 lib/xe/xe_ioctl.c       |  12 +-
 lib/xe/xe_ioctl.h       |   2 +
 tests/intel/xe_pxp.c    | 455 +++++++++++++++++++++++++++++++++++++++-
 5 files changed, 465 insertions(+), 13 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index ee499ee7f..68614ed6b 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -2428,9 +2428,9 @@ static void update_offsets(struct intel_bb *ibb,
 
 #define LINELEN 76
 
-static int
-__xe_bb_exec(struct intel_bb *ibb, uint64_t flags, bool sync)
+int __xe_bb_exec(struct intel_bb *ibb, uint64_t flags, bool sync)
 {
+	int ret = 0;
 	uint32_t engine = flags & (I915_EXEC_BSD_MASK | I915_EXEC_RING_MASK);
 	uint32_t engine_id;
 	struct drm_xe_sync syncs[2] = {
@@ -2507,12 +2507,12 @@ __xe_bb_exec(struct intel_bb *ibb, uint64_t flags, bool sync)
 	ibb->engine_syncobj = syncobj_create(ibb->fd, 0);
 	syncs[1].handle = ibb->engine_syncobj;
 
-	xe_exec_sync(ibb->fd, engine_id, ibb->batch_offset, syncs, 2);
+	ret = xe_exec_sync_failable(ibb->fd, engine_id, ibb->batch_offset, syncs, 2);
 
 	if (sync)
 		intel_bb_sync(ibb);
 
-	return 0;
+	return ret;
 }
 
 static int
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 178aaa9d8..6a7e8df4a 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -508,6 +508,7 @@ void intel_bb_dump_cache(struct intel_bb *ibb);
 
 void intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
 		   uint64_t flags, bool sync);
+int __xe_bb_exec(struct intel_bb *ibb, uint64_t flags, bool sync);
 
 uint64_t intel_bb_get_object_offset(struct intel_bb *ibb, uint32_t handle);
 bool intel_bb_object_offset_to_buf(struct intel_bb *ibb, struct intel_buf *buf);
diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
index 01ab7c758..cd7314eda 100644
--- a/lib/xe/xe_ioctl.c
+++ b/lib/xe/xe_ioctl.c
@@ -462,8 +462,8 @@ void xe_exec(int fd, struct drm_xe_exec *exec)
 	igt_assert_eq(__xe_exec(fd, exec), 0);
 }
 
-void xe_exec_sync(int fd, uint32_t exec_queue, uint64_t addr,
-		  struct drm_xe_sync *sync, uint32_t num_syncs)
+int xe_exec_sync_failable(int fd, uint32_t exec_queue, uint64_t addr,
+			  struct drm_xe_sync *sync, uint32_t num_syncs)
 {
 	struct drm_xe_exec exec = {
 		.exec_queue_id = exec_queue,
@@ -473,7 +473,13 @@ void xe_exec_sync(int fd, uint32_t exec_queue, uint64_t addr,
 		.num_batch_buffer = 1,
 	};
 
-	igt_assert_eq(__xe_exec(fd, &exec), 0);
+	return __xe_exec(fd, &exec);
+}
+
+void xe_exec_sync(int fd, uint32_t exec_queue, uint64_t addr,
+		  struct drm_xe_sync *sync, uint32_t num_syncs)
+{
+	igt_assert_eq(xe_exec_sync_failable(fd, exec_queue, addr, sync, num_syncs), 0);
 }
 
 void xe_exec_wait(int fd, uint32_t exec_queue, uint64_t addr)
diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
index c8a2d81c5..9bdf73b2b 100644
--- a/lib/xe/xe_ioctl.h
+++ b/lib/xe/xe_ioctl.h
@@ -91,6 +91,8 @@ int __xe_exec(int fd, struct drm_xe_exec *exec);
 void xe_exec(int fd, struct drm_xe_exec *exec);
 void xe_exec_sync(int fd, uint32_t exec_queue, uint64_t addr,
 		  struct drm_xe_sync *sync, uint32_t num_syncs);
+int xe_exec_sync_failable(int fd, uint32_t exec_queue, uint64_t addr,
+		  struct drm_xe_sync *sync, uint32_t num_syncs);
 void xe_exec_wait(int fd, uint32_t exec_queue, uint64_t addr);
 int __xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
 		     uint32_t exec_queue, int64_t *timeout);
diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
index 2ab920d07..db7112fd2 100644
--- a/tests/intel/xe_pxp.c
+++ b/tests/intel/xe_pxp.c
@@ -4,6 +4,7 @@
  */
 
 #include "igt.h"
+#include "igt_syncobj.h"
 #include "intel_batchbuffer.h"
 #include "intel_bufops.h"
 #include "intel_mocs.h"
@@ -81,6 +82,15 @@ static uint32_t create_pxp_rcs_queue(int fd, uint32_t vm)
 	return q;
 }
 
+static uint32_t create_regular_rcs_queue(int fd, uint32_t vm)
+{
+	struct drm_xe_engine_class_instance inst = {
+		.engine_class = DRM_XE_ENGINE_CLASS_RENDER,
+	};
+
+	return xe_exec_queue_create(fd, vm, &inst, 0);
+}
+
 static int query_pxp_status(int fd)
 {
 	struct drm_xe_query_pxp_status *pxp_query;
@@ -338,15 +348,25 @@ static void pxp_rendercopy(int fd, uint32_t q, uint32_t vm, uint32_t copy_size,
 	buf_ops_destroy(bops);
 }
 
-/**
- * SUBTEST: regular-src-to-pxp-dest-rendercopy
- * Description: copy from a regular BO to a PXP one and verify the encryption
- */
-static void test_render_regular_src_to_pxp_dest(int fd)
+static void copy_bo_cpu(int fd, uint32_t bo, uint32_t *dst, uint32_t size)
+{
+	uint32_t *src_ptr;
+
+	src_ptr = xe_bo_mmap_ext(fd, bo, size, PROT_READ);
+
+	memcpy(dst, src_ptr, size);
+
+	igt_assert_eq(munmap(src_ptr, size), 0);
+}
+
+static void __test_render_regular_src_to_pxp_dest(int fd, uint32_t *outpixels, int outsize)
 {
 	uint32_t vm, srcbo, dstbo;
 	uint32_t q;
 
+	if (outpixels)
+		igt_assert_lte(TSTSURF_SIZE, outsize);
+
 	vm = xe_vm_create(fd, 0, 0);
 
 	/*
@@ -362,12 +382,24 @@ static void test_render_regular_src_to_pxp_dest(int fd)
 
 	check_bo_color(fd, dstbo, TSTSURF_SIZE, TSTSURF_FILLCOLOR1, false);
 
+	if (outpixels)
+		copy_bo_cpu(fd, dstbo, outpixels, TSTSURF_SIZE);
+
 	gem_close(fd, srcbo);
 	gem_close(fd, dstbo);
 	xe_exec_queue_destroy(fd, q);
 	xe_vm_destroy(fd, vm);
 }
 
+/**
+ * SUBTEST: regular-src-to-pxp-dest-rendercopy
+ * Description: copy from a regular BO to a PXP one and verify the encryption
+ */
+static void test_render_regular_src_to_pxp_dest(int fd)
+{
+	__test_render_regular_src_to_pxp_dest(fd, NULL, 0);
+}
+
 static int bocmp(int fd, uint32_t bo1, uint32_t bo2, uint32_t size)
 {
 	uint32_t *ptr1, *ptr2;
@@ -429,13 +461,402 @@ static void test_render_pxp_protsrc_to_protdest(int fd)
 	xe_vm_destroy(fd, vm);
 }
 
-static void require_pxp_render(bool pxp_supported, uint32_t devid)
+#define PS_OP_TAG_LOW 0x1234fed0
+#define PS_OP_TAG_HI 0x5678cbaf
+static void emit_pipectrl(struct intel_bb *ibb, struct intel_buf *fenceb)
+{
+	uint32_t pipe_ctl_flags = 0;
+
+	intel_bb_out(ibb, GFX_OP_PIPE_CONTROL(2));
+	intel_bb_out(ibb, pipe_ctl_flags);
+
+	pipe_ctl_flags = (PIPE_CONTROL_FLUSH_ENABLE |
+			  PIPE_CONTROL_CS_STALL |
+			  PIPE_CONTROL_QW_WRITE);
+	intel_bb_out(ibb, GFX_OP_PIPE_CONTROL(6));
+	intel_bb_out(ibb, pipe_ctl_flags);
+
+	intel_bb_emit_reloc(ibb, fenceb->handle, 0, I915_GEM_DOMAIN_COMMAND, 0,
+			    fenceb->addr.offset);
+	intel_bb_out(ibb, PS_OP_TAG_LOW);
+	intel_bb_out(ibb, PS_OP_TAG_HI);
+	intel_bb_out(ibb, MI_NOOP);
+	intel_bb_out(ibb, MI_NOOP);
+}
+
+static void assert_pipectl_storedw_done(int fd, uint32_t bo)
+{
+	uint32_t *ptr;
+
+	ptr = xe_bo_mmap_ext(fd, bo, 4096, PROT_READ|PROT_WRITE);
+	igt_assert_eq(ptr[0], PS_OP_TAG_LOW);
+	igt_assert_eq(ptr[1], PS_OP_TAG_HI);
+
+	igt_assert(munmap(ptr, 4096) == 0);
+}
+
+static int submit_flush_store_dw(int fd, uint32_t q, bool q_is_pxp, uint32_t vm,
+				 uint32_t dst, bool dst_is_pxp)
+{
+	struct intel_buf *dstbuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
+	int ret = 0;
+
+	bops = buf_ops_create(fd);
+	igt_assert(bops);
+
+	ibb = intel_bb_create_with_context(fd, q, vm, NULL, 4096);
+	igt_assert(ibb);
+	intel_bb_set_pxp(ibb, q_is_pxp, DISPLAY_APPTYPE, DRM_XE_PXP_HWDRM_DEFAULT_SESSION);
+
+	dstbuf = buf_create(fd, bops, dst, 256, 4, 32, 4096);
+	intel_buf_set_pxp(dstbuf, dst_is_pxp);
+
+	intel_bb_ptr_set(ibb, 0);
+	intel_bb_add_intel_buf(ibb, dstbuf, true);
+	emit_pipectrl(ibb, dstbuf);
+	intel_bb_emit_bbe(ibb);
+	ret = __xe_bb_exec(ibb, 0, false);
+	if (ret == 0)
+		ret = intel_bb_sync(ibb);
+	if (ret == 0)
+		assert_pipectl_storedw_done(fd, dst);
+
+	intel_buf_destroy(dstbuf);
+	intel_bb_destroy(ibb);
+	buf_ops_destroy(bops);
+
+	return ret;
+}
+
+static void trigger_pxp_debugfs_forced_teardown(int xe_fd)
+{
+	char str[32];
+	int ret;
+	int fd;
+
+	fd = igt_debugfs_dir(xe_fd);
+	igt_assert(fd >= 0);
+	ret = igt_debugfs_simple_read(fd, "pxp/terminate", str, 32);
+	igt_assert_f(ret >= 0, "Can't open pxp termination debugfs\n");
+
+	/* give the kernel time to handle the termination */
+	sleep(1);
+}
+
+enum termination_type {
+	PXP_TERMINATION_IRQ,
+	PXP_TERMINATION_RPM,
+	PXP_TERMINATION_SUSPEND
+};
+
+static void trigger_termination(int fd, enum termination_type type)
+{
+	switch (type) {
+	case PXP_TERMINATION_IRQ:
+		trigger_pxp_debugfs_forced_teardown(fd);
+		break;
+	case PXP_TERMINATION_RPM:
+		igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+		break;
+	case PXP_TERMINATION_SUSPEND:
+		igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_DEVICES);
+		break;
+	}
+}
+
+/**
+ * SUBTEST: pxp-termination-key-update-post-termination-irq
+ * Description: Verify key is changed after a termination irq
+ */
+
+/**
+ * SUBTEST: pxp-termination-key-update-post-suspend
+ * Description: Verify key is changed after a suspend/resume cycle
+ */
+
+/**
+ * SUBTEST: pxp-termination-key-update-post-rpm
+ * Description: Verify key is changed after a runtime suspend/resume cycle
+ */
+
+static void test_pxp_teardown_keychange(int fd, enum termination_type type)
+{
+	uint32_t* encrypted_data_before;
+	uint32_t* encrypted_data_after;
+	int matched_after_keychange = 0, loop = 0;
+
+	encrypted_data_before = malloc(TSTSURF_SIZE);
+	encrypted_data_after = malloc(TSTSURF_SIZE);
+	igt_assert(encrypted_data_before && encrypted_data_after);
+
+	__test_render_regular_src_to_pxp_dest(fd, encrypted_data_before, TSTSURF_SIZE);
+
+	trigger_termination(fd, type);
+
+	__test_render_regular_src_to_pxp_dest(fd, encrypted_data_after, TSTSURF_SIZE);
+
+	while (loop < (TSTSURF_SIZE/TSTSURF_BYTESPP)) {
+		if (encrypted_data_before[loop] == encrypted_data_after[loop])
+			++matched_after_keychange;
+		++loop;
+	}
+	igt_assert_eq(matched_after_keychange, 0);
+
+	free(encrypted_data_before);
+	free(encrypted_data_after);
+}
+
+static void pxp_vm_bind_sync(int fd, uint32_t vm, uint32_t bo, uint64_t addr,
+			     uint64_t size, uint32_t op)
+{
+	struct drm_xe_sync sync = {
+		.type = DRM_XE_SYNC_TYPE_SYNCOBJ,
+		.flags = DRM_XE_SYNC_FLAG_SIGNAL,
+		.handle = syncobj_create(fd, 0),
+	};
+
+	__xe_vm_bind_assert(fd, vm, 0, bo, 0, addr, size, op,
+			    DRM_XE_VM_BIND_FLAG_CHECK_PXP, &sync, 1, 0, 0);
+
+	igt_assert(syncobj_wait(fd, &sync.handle, 1, INT64_MAX, 0, NULL));
+	syncobj_destroy(fd, sync.handle);
+}
+
+/**
+ * SUBTEST: pxp-stale-bo-bind-post-termination-irq
+ * Description: verify that VM bind on a stale BO (due to a termination irq) is rejected.
+ */
+
+/**
+ * SUBTEST: pxp-stale-bo-bind-post-suspend
+ * Description: verify that VM bind on a stale BO (due to a suspend/resume cycle)
+ *              is rejected.
+ */
+
+/**
+ * SUBTEST: pxp-stale-bo-bind-post-rpm
+ * Description: verify that VM bind on a stale BO (due to a runtime suspend/resume
+ *              cycle) is rejected.
+ */
+
+static void __test_pxp_stale_bo_bind(int fd, enum termination_type type, bool pxp)
+{
+	uint32_t vm, q;
+	uint32_t bo;
+	uint32_t flags = pxp ? DRM_XE_VM_BIND_FLAG_CHECK_PXP : 0;
+	int ret;
+
+	vm = xe_vm_create(fd, 0, 0);
+	q = create_pxp_rcs_queue(fd, vm); /* start PXP session */
+
+	bo = pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_HWDRM);
+
+	/* map the BO to the VM to make sure it works */
+	pxp_vm_bind_sync(fd, vm, bo, 0, 4096, DRM_XE_VM_BIND_OP_MAP);
+
+	xe_exec_queue_destroy(fd, q);
+	trigger_termination(fd, type);
+
+	/* map of a stale PXP BO must fail if (and only if) the CHECK_PXP flag is set */
+	ret = __xe_vm_bind(fd, vm, 0, bo, 0, 0, 4096, DRM_XE_VM_BIND_OP_MAP,
+			   flags, NULL, 0, 0, DEFAULT_PAT_INDEX, 0);
+	igt_assert_eq(ret, pxp ? -ENOEXEC : 0);
+
+	/* unmap must always work */
+	pxp_vm_bind_sync(fd, vm, bo, 0, 0, DRM_XE_VM_BIND_OP_UNMAP_ALL);
+
+	xe_vm_destroy(fd, vm);
+
+	/* mapping on a brand new vm should have the same behavior */
+	vm = xe_vm_create(fd, 0, 0);
+	ret = __xe_vm_bind(fd, vm, 0, bo, 0, 0, 4096, DRM_XE_VM_BIND_OP_MAP,
+			   flags, NULL, 0, 0, DEFAULT_PAT_INDEX, 0);
+	igt_assert_eq(ret, pxp ? -ENOEXEC : 0);
+
+	gem_close(fd, bo);
+	xe_vm_destroy(fd, vm);
+}
+
+static void test_pxp_stale_bo_bind(int fd, enum termination_type type)
+{
+	__test_pxp_stale_bo_bind(fd, type, true);
+}
+
+static uint32_t create_and_bind_simple_pxp_batch(int fd, uint32_t vm,
+						 uint32_t size, uint64_t addr)
+{
+	uint32_t bo;
+	uint32_t *map;
+	bo = pxp_bo_create(fd, vm, 4096, DRM_XE_PXP_TYPE_HWDRM);
+	pxp_vm_bind_sync(fd, vm, bo, addr, size, DRM_XE_VM_BIND_OP_MAP);
+
+	map = xe_bo_map(fd, bo, 4096);
+	*map = MI_BATCH_BUFFER_END;
+	munmap(map, 4096);
+
+	return bo;
+}
+
+/**
+ * SUBTEST: pxp-stale-bo-exec-post-termination-irq
+ * Description: verify that a submission using VM with a mapped stale BO (due to
+ *              a termination irq) is rejected.
+ */
+
+/**
+ * SUBTEST: pxp-stale-bo-exec-post-suspend
+ * Description: verify that a submission using VM with a mapped stale BO (due to
+ *              a suspend/resume cycle) is rejected.
+ */
+
+/**
+ * SUBTEST: pxp-stale-bo-exec-post-rpm
+ * Description: verify that a submission using VM with a mapped stale BO (due to
+ *              a runtime suspend/resume cycle) is rejected.
+ */
+
+static void __test_pxp_stale_bo_exec(int fd, enum termination_type type, bool pxp)
+{
+	uint32_t vm, q;
+	uint32_t bo;
+	int expected;
+
+	vm = xe_vm_create(fd, 0, 0);
+
+	q = create_pxp_rcs_queue(fd, vm); /* start a PXP session */
+	bo = create_and_bind_simple_pxp_batch(fd, vm, 4096, 0);
+
+	xe_exec_queue_destroy(fd, q);
+	trigger_termination(fd, type);
+
+	/* create a clean queue using the VM with the invalid object mapped in */
+	if (pxp) {
+		q = create_pxp_rcs_queue(fd, vm);
+		expected = -ENOEXEC;
+	} else {
+		q = create_regular_rcs_queue(fd, vm);
+		expected = 0;
+	}
+
+	igt_assert_eq(xe_exec_sync_failable(fd, q, 0, NULL, 0), expected);
+
+	/* now make sure we can unmap the stale BO and have a clean exec after */
+	if (pxp) {
+		pxp_vm_bind_sync(fd, vm, 0, 0, 4096, DRM_XE_VM_BIND_OP_UNMAP);
+		gem_close(fd, bo);
+
+		bo = create_and_bind_simple_pxp_batch(fd, vm, 4096, 0);
+		igt_assert_eq(xe_exec_sync_failable(fd, q, 0, NULL, 0), 0);
+	}
+
+	xe_exec_queue_destroy(fd, q);
+	gem_close(fd, bo);
+	xe_vm_destroy(fd, vm);
+}
+
+static void test_pxp_stale_bo_exec(int fd, enum termination_type type)
+{
+	__test_pxp_stale_bo_exec(fd, type, true);
+}
+
+/**
+ * SUBTEST: pxp-stale-queue-post-termination-irq
+ * Description: verify that submissions on a stale queue (due to a termination
+ *              irq) are cancelled
+ */
+
+/**
+ * SUBTEST: pxp-stale-queue-post-suspend
+ * Description: verify that submissions on a stale queue (due to a suspend/resume
+ *              cycle) are cancelled
+ */
+
+static void test_pxp_stale_queue_execution(int fd, enum termination_type type)
+{
+	uint32_t vm, q;
+	uint32_t dstbo;
+
+	vm = xe_vm_create(fd, 0, 0);
+	q = create_pxp_rcs_queue(fd, vm);
+
+	dstbo = regular_bo_create_and_fill(fd, vm, 4096, 0);
+
+	igt_assert_eq(submit_flush_store_dw(fd, q, true, vm, dstbo, false), 0);
+
+	trigger_termination(fd, type);
+
+	/* when we execute an invalid queue we expect the job to be canceled */
+	igt_assert_eq(submit_flush_store_dw(fd, q, true, vm, dstbo, false), -ECANCELED);
+
+	gem_close(fd, dstbo);
+	xe_exec_queue_destroy(fd, q);
+	xe_vm_destroy(fd, vm);
+}
+
+/**
+ * SUBTEST: pxp-optout
+ * Description: verify that submssions with stale objects/queues are not blocked
+ *              if the user does not opt-in to the PXP checks.
+ */
+static void test_pxp_optout(int fd)
+{
+	__test_pxp_stale_bo_exec(fd, PXP_TERMINATION_IRQ, false);
+	__test_pxp_stale_bo_bind(fd, PXP_TERMINATION_IRQ, false);
+}
+
+static void require_pxp(bool pxp_supported)
 {
 	igt_require_f(pxp_supported, "PXP not supported\n");
+}
+
+static void require_pxp_render(bool pxp_supported, uint32_t devid)
+{
+	require_pxp(pxp_supported);
 	igt_require_f(igt_get_render_copyfunc(devid),
 		      "No rendercopy found for devid=%x\n", devid);
 }
 
+static void termination_tests(int fd, bool pxp_supported, uint32_t devid,
+			      enum termination_type type, const char *tag)
+{
+	int fw_handle;
+
+	if (type != PXP_TERMINATION_RPM) {
+		/* avoid rpm entry for non-rpm tests */
+		fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
+		igt_require(fw_handle >= 0);
+	} else {
+		igt_setup_runtime_pm(fd);
+	}
+
+	igt_subtest_f("pxp-termination-key-update-post-%s", tag) {
+		require_pxp_render(pxp_supported, devid);
+		test_pxp_teardown_keychange(fd, type);
+	}
+	igt_subtest_f("pxp-stale-bo-bind-post-%s", tag) {
+		require_pxp(pxp_supported);
+		test_pxp_stale_bo_bind(fd, type);
+	}
+	igt_subtest_f("pxp-stale-bo-exec-post-%s", tag) {
+		require_pxp(pxp_supported);
+		test_pxp_stale_bo_exec(fd, type);
+	}
+
+	/* An active PXP queue holds an RPM ref, so we can't test RPM with it */
+	if (type != PXP_TERMINATION_RPM) {
+		igt_subtest_f("pxp-stale-queue-post-%s", tag) {
+			require_pxp(pxp_supported);
+			test_pxp_stale_queue_execution(fd, type);
+		}
+
+		close(fw_handle);
+	} else {
+		igt_restore_runtime_pm();
+	}
+}
+
 igt_main
 {
 	int xe_fd = -1;
@@ -470,6 +891,28 @@ igt_main
 		}
 	}
 
+	igt_subtest_group {
+		const struct mode {
+			enum termination_type type;
+			const char *tag;
+		} modes[] = {
+			{ PXP_TERMINATION_IRQ, "termination-irq" },
+			{ PXP_TERMINATION_SUSPEND, "suspend" },
+			{ PXP_TERMINATION_RPM, "rpm" },
+			{ 0, NULL }
+		};
+
+		igt_describe("Verify teardown management");
+
+		for (const struct mode *m = modes; m->tag; m++)
+			termination_tests(xe_fd, pxp_supported, devid, m->type, m->tag);
+
+		igt_subtest("pxp-optout") {
+			require_pxp(pxp_supported);
+			test_pxp_optout(xe_fd);
+		}
+	}
+
 	igt_fixture {
 		close(xe_fd);
 	}
-- 
2.43.0

[PATCH i-g-t v3 7/7] tests/intel/xe_pxp: Test encrypted FBs

[Daniele Ceraolo Spurio]

PXP allows a user to send an encrypted BO to the display HW without
having to decode it. The driver needs however to tell the HW that the
BO is encrypted, otherwise it won't be displayed correctly. Furthermore,
if PXP is terminated before the FB is displayed, we expect to see a black
screen instead of what's in the BO.

We can test both these flows by by displaying the expected image from a
non-encrypted FB and and making sure that the CRC match when we display
the encrypted FB.

v2: move igt_require calls inside the subtest
v3: move repeated code to an helper (Alan)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/intel/xe_pxp.c | 268 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 261 insertions(+), 7 deletions(-)

diff --git a/tests/intel/xe_pxp.c b/tests/intel/xe_pxp.c
index db7112fd2..2591e6f2d 100644
--- a/tests/intel/xe_pxp.c
+++ b/tests/intel/xe_pxp.c
@@ -25,8 +25,8 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
  * Test category: functionality test
  */
 
-static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size,
-			   uint32_t session_type, uint32_t *handle)
+static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t placement,
+			   uint32_t session_type, uint32_t flags, uint32_t *handle)
 {
 	struct drm_xe_ext_set_property ext = {
 		.base.next_extension = 0,
@@ -36,7 +36,7 @@ static int __pxp_bo_create(int fd, uint32_t vm, uint64_t size,
 	};
 	int ret = 0;
 
-	if (__xe_bo_create(fd, vm, size, system_memory(fd), 0, &ext, handle)) {
+	if (__xe_bo_create(fd, vm, size, placement, flags, &ext, handle)) {
 		ret = -errno;
 		errno = 0;
 	}
@@ -48,7 +48,19 @@ static uint32_t pxp_bo_create(int fd, uint32_t vm, uint64_t size, uint32_t type)
 {
 	uint32_t handle;
 
-	igt_assert_eq(__pxp_bo_create(fd, vm, size, type, &handle), 0);
+	igt_assert_eq(__pxp_bo_create(fd, vm, size, system_memory(fd), type, 0, &handle), 0);
+
+	return handle;
+}
+
+static uint32_t pxp_bo_create_display(int fd, uint32_t vm, uint64_t size, uint32_t type)
+{
+	uint32_t handle;
+
+	igt_assert_eq(__pxp_bo_create(fd, vm, size, vram_if_possible(fd, 0), type,
+				      DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM |
+				      DRM_XE_GEM_CREATE_FLAG_SCANOUT,
+				      &handle), 0);
 
 	return handle;
 }
@@ -164,18 +176,18 @@ static void test_pxp_bo_alloc(int fd, bool pxp_supported)
 	int ret;
 
 	/* BO creation with DRM_XE_PXP_TYPE_NONE must always succeed */
-	ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_NONE, &bo);
+	ret = __pxp_bo_create(fd, 0, 4096, system_memory(fd), DRM_XE_PXP_TYPE_NONE, 0, &bo);
 	igt_assert_eq(ret, 0);
 	gem_close(fd, bo);
 
 	/* BO creation with DRM_XE_PXP_TYPE_HWDRM must only succeed if PXP is supported */
-	ret = __pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_HWDRM, &bo);
+	ret = __pxp_bo_create(fd, 0, 4096, system_memory(fd), DRM_XE_PXP_TYPE_HWDRM, 0, &bo);
 	igt_assert_eq(ret, pxp_supported ? 0 : -ENODEV);
 	if (!ret)
 		gem_close(fd, bo);
 
 	/* BO creation with an invalid type must always fail */
-	ret = __pxp_bo_create(fd, 0, 4096, 0xFF, &bo);
+	ret = __pxp_bo_create(fd, 0, 4096, system_memory(fd), 0xFF, 0, &bo);
 	igt_assert_eq(ret, -EINVAL);
 }
 
@@ -806,6 +818,225 @@ static void test_pxp_optout(int fd)
 	__test_pxp_stale_bo_bind(fd, PXP_TERMINATION_IRQ, false);
 }
 
+static void setup_fb(int fd, igt_fb_t *pxp_fb, int width, int height, uint32_t size)
+{
+	/* create an FB using a PXP BO */
+	igt_init_fb(pxp_fb, fd, width, height,
+		    DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_NONE,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+
+	igt_calc_fb_size(pxp_fb);
+
+	pxp_fb->gem_handle = pxp_bo_create_display(fd, 0, size, DRM_XE_PXP_TYPE_HWDRM);
+
+	do_or_die(__kms_addfb(pxp_fb->fd, pxp_fb->gem_handle,
+			pxp_fb->width, pxp_fb->height,
+			pxp_fb->drm_format, pxp_fb->modifier,
+			pxp_fb->strides, pxp_fb->offsets, pxp_fb->num_planes, DRM_MODE_FB_MODIFIERS,
+			&pxp_fb->fb_id));
+}
+
+static void setup_protected_fb_from_ref(int fd, igt_fb_t *ref_fb, igt_fb_t *pxp_fb,
+					uint32_t q, uint32_t vm)
+{
+	struct intel_buf *srcbuf, *dstbuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
+	igt_render_copyfunc_t render_copy;
+
+	render_copy = igt_get_render_copyfunc(intel_get_drm_devid(fd));
+	igt_assert(render_copy);
+
+	bops = buf_ops_create(fd);
+	igt_assert(bops);
+
+	/* create an FB using a PXP BO */
+	setup_fb(fd, pxp_fb, ref_fb->width, ref_fb->height, ref_fb->size);
+
+	/* copy the contents of ref_fb into the pxp BO */
+	srcbuf = igt_fb_create_intel_buf(fd, bops, ref_fb, "ref_fb");
+	dstbuf = igt_fb_create_intel_buf(fd, bops, pxp_fb, "pxp_fb");
+	intel_buf_set_pxp(dstbuf, true);
+
+	ibb = intel_bb_create_with_context(fd, q, vm, NULL, 4096);
+	igt_assert(ibb);
+	intel_bb_set_pxp(ibb, true, DISPLAY_APPTYPE, DRM_XE_PXP_HWDRM_DEFAULT_SESSION);
+
+	render_copy(ibb, srcbuf, 0, 0, pxp_fb->width, pxp_fb->height, dstbuf, 0, 0);
+	intel_bb_sync(ibb);
+
+	/* make sure the contents of the BOs don't match */
+	igt_assert_neq(bocmp(fd, pxp_fb->gem_handle, ref_fb->gem_handle, pxp_fb->size), 0);
+
+	intel_bb_destroy(ibb);
+	intel_buf_destroy(srcbuf);
+	intel_buf_destroy(dstbuf);
+	buf_ops_destroy(bops);
+}
+
+static void commit_fb(igt_display_t *display, igt_plane_t *plane, igt_fb_t *fb, drmModeModeInfo *mode)
+{
+	igt_plane_set_fb(plane, fb);
+	igt_fb_set_size(fb, plane, mode->hdisplay, mode->vdisplay);
+	igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
+
+	igt_display_commit2(display, COMMIT_ATOMIC);
+}
+
+static void compare_crcs(int fd, igt_display_t *display, igt_fb_t *ref_fb, igt_fb_t *pxp_fb)
+{
+	igt_output_t *output;
+	drmModeModeInfo *mode;
+	igt_plane_t *plane;
+	igt_pipe_t *pipe;
+	igt_pipe_crc_t *pipe_crc;
+	igt_crc_t ref_crc, new_crc;
+
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+		pipe = &display->pipes[output->pending_pipe];
+		pipe_crc = igt_pipe_crc_new(fd, pipe->pipe,
+					    IGT_PIPE_CRC_SOURCE_AUTO);
+		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(igt_pipe_connector_valid(pipe->pipe, output));
+		igt_output_set_pipe(output, pipe->pipe);
+
+		commit_fb(display, plane, ref_fb, mode);
+		igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
+
+		commit_fb(display, plane, pxp_fb, mode);
+		igt_pipe_crc_collect_crc(pipe_crc, &new_crc);
+
+		igt_assert_crc_equal(&ref_crc, &new_crc);
+
+		/*
+		 * Testing with one pipe-output combination is sufficient.
+		 * So break the loop.
+		 */
+		break;
+	}
+}
+
+/**
+ * SUBTEST: display-pxp-fb
+ * Description: Test that an encrypted fb is displayed correctly by comparing
+ *              its CRCs with the ones generated by a non-encrypted FB
+ *              containing the same image
+ */
+
+static void test_display_pxp_fb(int fd, igt_display_t *display)
+{
+	igt_output_t *output;
+	drmModeModeInfo *mode;
+	igt_fb_t ref_fb, pxp_fb;
+	igt_plane_t *plane;
+	igt_pipe_t *pipe;
+	int width = 0, height = 0, i = 0;
+	uint32_t q;
+	uint32_t vm;
+
+	vm = xe_vm_create(fd, 0, 0);
+	q = create_pxp_rcs_queue(fd, vm); /* start the PXP session */
+
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+
+		width = max_t(int, width, mode->hdisplay);
+		height = max_t(int, height, mode->vdisplay);
+	}
+
+	igt_create_color_fb(fd, width, height, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+			    0, 1, 0, &ref_fb);
+
+	/* Do a modeset on all outputs */
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+		pipe = &display->pipes[i];
+		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(igt_pipe_connector_valid(i, output));
+		igt_output_set_pipe(output, i);
+
+		commit_fb(display, plane, &ref_fb, mode);
+
+		i++;
+	}
+
+	/* Create an encrypted FB with the same contents as ref_fb */
+	setup_protected_fb_from_ref(fd, &ref_fb, &pxp_fb, q, vm);
+
+	/* Flip both FBs and make sure the CRCs match */
+	compare_crcs(fd, display, &ref_fb, &pxp_fb);
+
+	igt_remove_fb(fd, &ref_fb);
+	igt_remove_fb(fd, &pxp_fb);
+	xe_exec_queue_destroy(fd, q);
+	xe_vm_destroy(fd, vm);
+}
+
+/**
+ * SUBTEST: display-black-pxp-fb
+ * Description: Test that an invalid encrypted fb is correctly converted to a
+ *              black screen by comparing its CRCs with the ones generated by a
+ *              non-encrypted FB filled with black
+ */
+
+static void test_display_black_pxp_fb(int fd, igt_display_t *display)
+{
+	igt_output_t *output;
+	drmModeModeInfo *mode;
+	igt_fb_t ref_fb, pxp_fb;
+	igt_plane_t *plane;
+	igt_pipe_t *pipe;
+	int width = 0, height = 0, i = 0;
+	uint32_t q;
+	uint32_t vm;
+
+	vm = xe_vm_create(fd, 0, 0);
+	q = create_pxp_rcs_queue(fd, vm); /* start the PXP session */
+
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+
+		width = max_t(int, width, mode->hdisplay);
+		height = max_t(int, height, mode->vdisplay);
+	}
+
+	/* create a black fb */
+	igt_create_color_fb(fd, width, height, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+			    0, 0, 0, &ref_fb);
+
+	/* Do a modeset on all outputs */
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+		pipe = &display->pipes[i];
+		plane = igt_pipe_get_plane_type(pipe, DRM_PLANE_TYPE_PRIMARY);
+		igt_require(igt_pipe_connector_valid(i, output));
+		igt_output_set_pipe(output, i);
+
+		igt_plane_set_fb(plane, &ref_fb);
+		igt_fb_set_size(&ref_fb, plane, mode->hdisplay, mode->vdisplay);
+		igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+		i++;
+	}
+
+	/* Create an fb filled with a non-black color */
+	setup_fb(fd, &pxp_fb, ref_fb.width, ref_fb.height, ref_fb.size);
+	fill_bo_content(fd, pxp_fb.gem_handle, pxp_fb.size, TSTSURF_INITCOLOR1);
+
+	/* invalidate the BO */
+	trigger_termination(fd, PXP_TERMINATION_IRQ);
+
+	/* Flip both FBs and make sure the CRCs match */
+	compare_crcs(fd, display, &ref_fb, &pxp_fb);
+
+	igt_remove_fb(fd, &ref_fb);
+	igt_remove_fb(fd, &pxp_fb);
+	xe_exec_queue_destroy(fd, q);
+	xe_vm_destroy(fd, vm);
+}
+
 static void require_pxp(bool pxp_supported)
 {
 	igt_require_f(pxp_supported, "PXP not supported\n");
@@ -818,6 +1049,12 @@ static void require_pxp_render(bool pxp_supported, uint32_t devid)
 		      "No rendercopy found for devid=%x\n", devid);
 }
 
+static void require_display(int xe_fd, igt_display_t *display)
+{
+	igt_require_pipe_crc(xe_fd);
+	igt_display_require(display, xe_fd);
+}
+
 static void termination_tests(int fd, bool pxp_supported, uint32_t devid,
 			      enum termination_type type, const char *tag)
 {
@@ -913,6 +1150,23 @@ igt_main
 		}
 	}
 
+	igt_subtest_group {
+		igt_display_t display;
+
+		igt_describe("Test the flip of PXP objects to display");
+		igt_subtest("display-pxp-fb") {
+			require_display(xe_fd, &display);
+			require_pxp_render(pxp_supported, devid);
+			test_display_pxp_fb(xe_fd, &display);
+		}
+
+		igt_subtest("display-black-pxp-fb") {
+			require_display(xe_fd, &display);
+			require_pxp_render(pxp_supported, devid);
+			test_display_black_pxp_fb(xe_fd, &display);
+		}
+	}
+
 	igt_fixture {
 		close(xe_fd);
 	}
-- 
2.43.0

✗ GitLab.Pipeline: warning for Xe: Add tests for PXP (rev3)

[Patchwork]

== Series Details ==

Series: Xe: Add tests for PXP (rev3)
URL   : https://patchwork.freedesktop.org/series/142450/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1363685 for the overview.

build:tests-debian-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013675):
   #1 [killpg+0x40]
   #2 [gsignal+0x10b]
   #3 [abort+0x121]
   #4 [<unknown>+0x121]
   #5 [__assert_fail+0x42]
   #6 [internal_assert+0xe5]
   #7 [igt_skip+0x169]
   #8 [__igt_skip_check+0x1b5]
   #9 [termination_tests+0x41e]
   #10 [__igt_unique____real_main1097+0x2f6]
   #11 [main+0x27]
   #12 [__libc_start_main+0xeb]
   #13 [_start+0x2a]
  ninja: build stopped: subcommand failed.
  section_end:1739415407:step_script
  section_start:1739415407:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415407:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-debian-meson-arm64 has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013678):
  [2/406] Linking target tests/xe_pat.
  [3/406] Linking target tests/xe_oa.
  [4/406] Linking target tests/xe_pm_residency.
  [5/406] Linking target tests/xe_pm.
  [6/406] Compiling C object 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o'.
  FAILED: tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o 
  /usr/bin/aarch64-linux-gnu-gcc -Itests/59830eb@@xe_pxp@exe -Itests -I../tests -I../include -I../include/drm-uapi -I../include/drm-uapi-experimental -I../include/linux-uapi -Ilib -I../lib -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/aarch64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/aarch64-linux-gnu -I/usr/include/valgrind -I/usr/include/alsa -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-conversion -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -D_LARGEFILE64_SOURCE=1 -pthread  -MD -MQ 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -MF 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o.d' -o 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -c ../tests/intel/xe_pxp.c
  ../tests/intel/xe_pxp.c: In function ‘termination_tests’:
  ../tests/intel/xe_pxp.c:1065:53: error: ‘O_RDONLY’ undeclared (first use in this function); did you mean ‘STA_RONLY’?
     fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
                                                       ^~~~~~~~
                                                       STA_RONLY
  ../tests/intel/xe_pxp.c:1065:53: note: each undeclared identifier is reported only once for each function it appears in
  ninja: build stopped: subcommand failed.
  section_end:1739415363:step_script
  section_start:1739415363:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415364:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-debian-meson-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013677):
  ninja: Entering directory `build'
  [1/746] Generating version.h with a custom command.
  [2/406] Linking target tests/xe_pat.
  [3/406] Linking target tests/xe_oa.
  [4/406] Compiling C object 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o'.
  FAILED: tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o 
  /usr/bin/arm-linux-gnueabihf-gcc -Itests/59830eb@@xe_pxp@exe -Itests -I../tests -I../include -I../include/drm-uapi -I../include/drm-uapi-experimental -I../include/linux-uapi -Ilib -I../lib -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabihf/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/arm-linux-gnueabihf -I/usr/include/valgrind -I/usr/include/alsa -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-conversion -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -D_LARGEFILE64_SOURCE=1 -pthread  -MD -MQ 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -MF 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o.d' -o 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -c ../tests/intel/xe_pxp.c
  ../tests/intel/xe_pxp.c: In function ‘termination_tests’:
  ../tests/intel/xe_pxp.c:1065:53: error: ‘O_RDONLY’ undeclared (first use in this function); did you mean ‘STA_RONLY’?
     fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
                                                       ^~~~~~~~
                                                       STA_RONLY
  ../tests/intel/xe_pxp.c:1065:53: note: each undeclared identifier is reported only once for each function it appears in
  ninja: build stopped: subcommand failed.
  section_end:1739415400:step_script
  section_start:1739415400:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415401:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-debian-meson-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013679):
  ninja: Entering directory `build'
  [1/743] Generating version.h with a custom command.
  [2/401] Linking target tests/xe_oa.
  [3/401] Linking target tests/xe_pm.
  [4/401] Compiling C object 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o'.
  FAILED: tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o 
  /usr/bin/mips-linux-gnu-gcc -Itests/59830eb@@xe_pxp@exe -Itests -I../tests -I../include -I../include/drm-uapi -I../include/drm-uapi-experimental -I../include/linux-uapi -Ilib -I../lib -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/mips-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/mips-linux-gnu -I/usr/include/valgrind -I/usr/include/alsa -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-conversion -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -D_LARGEFILE64_SOURCE=1 -pthread  -MD -MQ 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -MF 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o.d' -o 'tests/59830eb@@xe_pxp@exe/intel_xe_pxp.c.o' -c ../tests/intel/xe_pxp.c
  ../tests/intel/xe_pxp.c: In function ‘termination_tests’:
  ../tests/intel/xe_pxp.c:1065:53: error: ‘O_RDONLY’ undeclared (first use in this function); did you mean ‘STA_RONLY’?
     fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
                                                       ^~~~~~~~
                                                       STA_RONLY
  ../tests/intel/xe_pxp.c:1065:53: note: each undeclared identifier is reported only once for each function it appears in
  ninja: build stopped: subcommand failed.
  section_end:1739415396:step_script
  section_start:1739415396:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415397:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013670):
   #1 [killpg+0x40]
   #2 [gsignal+0x145]
   #3 [abort+0x12b]
   #4 [__assert_fail_base.cold+0xf]
   #5 [__assert_fail+0x46]
   #6 [internal_assert+0xe5]
   #7 [igt_skip+0x16a]
   #8 [__igt_skip_check+0x1bb]
   #9 [termination_tests+0x3c8]
   #10 [__igt_unique____real_main1097+0x246]
   #11 [main+0x23]
   #12 [__libc_start_main+0xf3]
   #13 [_start+0x2e]
  ninja: build stopped: subcommand failed.
  section_end:1739415376:step_script
  section_start:1739415376:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415377:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013674):
   #0 [killpg+0x40]
   #1 [gsignal+0x145]
   #2 [abort+0x12b]
   #3 [__assert_fail_base.cold+0xf]
   #4 [__assert_fail+0x46]
   #5 [internal_assert+0xe0]
   #6 [igt_skip+0x150]
   #7 [__igt_skip_check+0x127]
   #8 [termination_tests+0x6ee]
   #9 [__igt_unique____real_main1097+0x9c4]
   #10 [main+0x29]
   #11 [__libc_start_main+0xf3]
   #12 [_start+0x2e]
  ninja: build stopped: subcommand failed.
  section_end:1739415424:step_script
  section_start:1739415424:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415425:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libdrm-nouveau has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013673):
   #1 [killpg+0x40]
   #2 [gsignal+0x145]
   #3 [abort+0x12b]
   #4 [__assert_fail_base.cold+0xf]
   #5 [__assert_fail+0x46]
   #6 [internal_assert+0xe5]
   #7 [igt_skip+0x16a]
   #8 [__igt_skip_check+0x1bb]
   #9 [termination_tests+0x3c8]
   #10 [__igt_unique____real_main1097+0x246]
   #11 [main+0x23]
   #12 [__libc_start_main+0xf3]
   #13 [_start+0x2e]
  ninja: build stopped: subcommand failed.
  section_end:1739415409:step_script
  section_start:1739415409:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415410:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libunwind has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013671):
  ninja: build stopped: subcommand failed.
  ninja: Entering directory `build'
  [1/1104] Generating version.h with a custom command.
  [2/446] Linking target tests/xe_oa.
  [3/446] Generating xe_oa.testlist with a meson_exe.py custom command.
  [4/446] Generating xe_pxp.testlist with a meson_exe.py custom command.
  FAILED: tests/xe_pxp.testlist 
  /usr/bin/meson --internal exe --capture tests/xe_pxp.testlist -- /builds/gfx-ci/igt-ci-tags/build/tests/xe_pxp --show-testlist
  skipping is allowed only in fixtures, subtests or igt_simple_main
  please refer to lib/igt_core documentation
  xe_pxp: ../lib/igt_core.c:450: internal_assert: Assertion `0' failed.
  Received signal SIGABRT.
  Stack trace: 
  ninja: build stopped: subcommand failed.
  section_end:1739415363:step_script
  section_start:1739415363:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415364:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-oldest-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/71013672):
   #1 [killpg+0x40]
   #2 [gsignal+0x145]
   #3 [abort+0x12b]
   #4 [__assert_fail_base.cold+0xf]
   #5 [__assert_fail+0x46]
   #6 [internal_assert+0xe5]
   #7 [igt_skip+0x16a]
   #8 [__igt_skip_check+0x1bb]
   #9 [termination_tests+0x3c8]
   #10 [__igt_unique____real_main1097+0x246]
   #11 [main+0x23]
   #12 [__libc_start_main+0xf3]
   #13 [_start+0x2e]
  ninja: build stopped: subcommand failed.
  section_end:1739415360:step_script
  section_start:1739415360:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1739415360:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1363685

Re: [PATCH i-g-t v3 1/7] drm-uapi/xe: Sync with PXP uapi updates

[Teres Alexis, Alan Previn]

not sure if an RB is required for UAPI syncing since its already in kernel now - but here it is:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

On Wed, 2025-02-12 at 16:38 -0800, Daniele Ceraolo Spurio wrote:
> Align with kernel commit 41a97c4a1294 ("drm/xe/pxp/uapi: Add API to
> mark a BO as using PXP")
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  include/drm-uapi/xe_drm.h | 123 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 121 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
alan:snip

Re: [PATCH i-g-t v3 5/7] tests/intel/xe_pxp: Test PXP submissions

[Teres Alexis, Alan Previn]

looks good.

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

On Wed, 2025-02-12 at 16:38 -0800, Ceraolo Spurio, Daniele wrote:
> The render supports PXP usage via rendercopy. We can use this to test
> that a user is able to correctly encrypt their data. In particular, we
> cover these 2 scenarios:
> 
> 1) copy from clear to encrypted - we expect the dest buffer to not match
>    the src one due to the encryption.
> 
> 2) copy from encrypted to encrypted = we expect the 2 BOs to match since
>    they hold the same data encrypted with the same key.
> 
> Note that the clear to clear copy is already covered by the
> xe_render_copy test, while encrypted to clear is obviously not a
> supported case.
> 
> Since the render_copy uses the intel_batchbuffer helpers, those helpers
> have been updated to support vm bind of protected objects.
> 
> v2: move igt_require calls inside the subtest
> v3: Better comments for rsvd1 overload, rename variables for clarity,
> updated commit msg.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  lib/intel_batchbuffer.c |  37 ++++--

alan:snip

Re: [PATCH i-g-t v3 6/7] tests/intel/xe_pxp: Termination tests

[Teres Alexis, Alan Previn]

Thanks for the tweaks / refactors from last review - all looks good now:

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>


On Wed, 2025-02-12 at 16:39 -0800, Ceraolo Spurio, Daniele wrote:
> There are several events that can cause the PXP key to be invalidated
> and trigger a PXP termination (suspend, PXP termination irq). After a
> termination, we expect the key to be different and the raw encrypted
> data to change for the same source data.
> Additionally, all PXP objects are invalidated during a termination and
> can no longer be used in submission or kept mapped to VMs; we therefore
> need to test both the execution and bind ioctls to make sure they work
> as expected after a termination.
> 
> v2: move igt_require calls inside the subtest
> v3: block rpm for tests trying a different type of termination, rework
> invalid bind test to try a new vm as well (Alan)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> 
alan:snip

> +static void pxp_vm_bind_sync(int fd, uint32_t vm, uint32_t bo, uint64_t addr,
> +                            uint64_t size, uint32_t op)
> +{
> +       struct drm_xe_sync sync = {
> +               .type = DRM_XE_SYNC_TYPE_SYNCOBJ,
> +               .flags = DRM_XE_SYNC_FLAG_SIGNAL,
> +               .handle = syncobj_create(fd, 0),
> +       };
> +
> +       __xe_vm_bind_assert(fd, vm, 0, bo, 0, addr, size, op,
> +                           DRM_XE_VM_BIND_FLAG_CHECK_PXP, &sync, 1, 0, 0);
> +
> +       igt_assert(syncobj_wait(fd, &sync.handle, 1, INT64_MAX, 0, NULL));
> +       syncobj_destroy(fd, sync.handle);
> +}
> +
> +/**
> + * SUBTEST: pxp-stale-bo-bind-post-termination-irq
> + * Description: verify that VM bind on a stale BO (due to a termination irq) is rejected.
> + */
> +
> +/**
> + * SUBTEST: pxp-stale-bo-bind-post-suspend
> + * Description: verify that VM bind on a stale BO (due to a suspend/resume cycle)
> + *              is rejected.
> + */
> +
> +/**
> + * SUBTEST: pxp-stale-bo-bind-post-rpm
> + * Description: verify that VM bind on a stale BO (due to a runtime suspend/resume
> + *              cycle) is rejected.
> + */
> +
> +static void __test_pxp_stale_bo_bind(int fd, enum termination_type type, bool pxp)
> +{
> +       uint32_t vm, q;
> +       uint32_t bo;
> +       uint32_t flags = pxp ? DRM_XE_VM_BIND_FLAG_CHECK_PXP : 0;
> +       int ret;
> +
> +       vm = xe_vm_create(fd, 0, 0);
> +       q = create_pxp_rcs_queue(fd, vm); /* start PXP session */
> +
> +       bo = pxp_bo_create(fd, 0, 4096, DRM_XE_PXP_TYPE_HWDRM);
> +
> +       /* map the BO to the VM to make sure it works */
> +       pxp_vm_bind_sync(fd, vm, bo, 0, 4096, DRM_XE_VM_BIND_OP_MAP);
> +
> +       xe_exec_queue_destroy(fd, q);
> +       trigger_termination(fd, type);
> +
> +       /* map of a stale PXP BO must fail if (and only if) the CHECK_PXP flag is set */
> +       ret = __xe_vm_bind(fd, vm, 0, bo, 0, 0, 4096, DRM_XE_VM_BIND_OP_MAP,
> +                          flags, NULL, 0, 0, DEFAULT_PAT_INDEX, 0);
> +       igt_assert_eq(ret, pxp ? -ENOEXEC : 0);
> +
> +       /* unmap must always work */
> +       pxp_vm_bind_sync(fd, vm, bo, 0, 0, DRM_XE_VM_BIND_OP_UNMAP_ALL);
> +
> +       xe_vm_destroy(fd, vm);
> 
alan: The rework in this function means we are more targetted on
addressing the different aspects of the expected UAPI behaviors.
Nice! - thanks.
> +
> +       /* mapping on a brand new vm should have the same behavior */
> +       vm = xe_vm_create(fd, 0, 0);
> +       ret = __xe_vm_bind(fd, vm, 0, bo, 0, 0, 4096, DRM_XE_VM_BIND_OP_MAP,
> +                          flags, NULL, 0, 0, DEFAULT_PAT_INDEX, 0);
> +       igt_assert_eq(ret, pxp ? -ENOEXEC : 0);
> +
> +       gem_close(fd, bo);
> +       xe_vm_destroy(fd, vm);
> +}
> +
> 
alan:snip
>  
> +static void termination_tests(int fd, bool pxp_supported, uint32_t devid,
> +                             enum termination_type type, const char *tag)
> +{
> +       int fw_handle;
> +
> +       if (type != PXP_TERMINATION_RPM) {
> +               /* avoid rpm entry for non-rpm tests */
> +               fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
> +               igt_require(fw_handle >= 0);
> +       } else {
> +               igt_setup_runtime_pm(fd);
> +       }
alan: This is a better approach here - now we can guarantee control
over it being a runtime-rpm triggered teardown vs other trigger types.
Thanks for this change.
> +
> +       igt_subtest_f("pxp-termination-key-update-post-%s", tag) {
> +               require_pxp_render(pxp_supported, devid);
> +               test_pxp_teardown_keychange(fd, type);
> +       }
> +       igt_subtest_f("pxp-stale-bo-bind-post-%s", tag) {
> +               require_pxp(pxp_supported);
> +               test_pxp_stale_bo_bind(fd, type);
> +       }
> +       igt_subtest_f("pxp-stale-bo-exec-post-%s", tag) {
> +               require_pxp(pxp_supported);
> +               test_pxp_stale_bo_exec(fd, type);
> +       }
> +
> +       /* An active PXP queue holds an RPM ref, so we can't test RPM with it */
> +       if (type != PXP_TERMINATION_RPM) {
> +               igt_subtest_f("pxp-stale-queue-post-%s", tag) {
> +                       require_pxp(pxp_supported);
> +                       test_pxp_stale_queue_execution(fd, type);
> +               }
> +
> +               close(fw_handle);
> +       } else {
> +               igt_restore_runtime_pm();
> +       }
> +}
> +
alan:snip

Re: [PATCH i-g-t v3 6/7] tests/intel/xe_pxp: Termination tests

[Kamil Konieczny]

Hi Daniele,
On 2025-02-12 at 16:39:00 -0800, Daniele Ceraolo Spurio wrote:
> There are several events that can cause the PXP key to be invalidated
> and trigger a PXP termination (suspend, PXP termination irq). After a
> termination, we expect the key to be different and the raw encrypted
> data to change for the same source data.
> Additionally, all PXP objects are invalidated during a termination and
> can no longer be used in submission or kept mapped to VMs; we therefore
> need to test both the execution and bind ioctls to make sure they work
> as expected after a termination.
> 
> v2: move igt_require calls inside the subtest
> v3: block rpm for tests trying a different type of termination, rework
> invalid bind test to try a new vm as well (Alan)

I have few nits, one is from our CI fail:

../tests/intel/xe_pxp.c:1065:53: error: ‘O_RDONLY’ undeclared (first use in this function); did you mean ‘STA_RONLY’?
fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
                                                  ^~~~~~~~
						 STA_RONLY
../tests/intel/xe_pxp.c:1065:53: note: each undeclared identifier is reported only once for each function it appears in

Please include a proper lib header in a first patch which uses O_RDONLY.

Rest nits below.

> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  lib/intel_batchbuffer.c |   8 +-
>  lib/intel_batchbuffer.h |   1 +
>  lib/xe/xe_ioctl.c       |  12 +-
>  lib/xe/xe_ioctl.h       |   2 +
>  tests/intel/xe_pxp.c    | 455 +++++++++++++++++++++++++++++++++++++++-
>  5 files changed, 465 insertions(+), 13 deletions(-)
> 
[...]
>  
> +static void termination_tests(int fd, bool pxp_supported, uint32_t devid,
> +			      enum termination_type type, const char *tag)
> +{
> +	int fw_handle;
> +
> +	if (type != PXP_TERMINATION_RPM) {
> +		/* avoid rpm entry for non-rpm tests */
> +		fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);

File or dir opens should happen in igt_fixture

> +		igt_require(fw_handle >= 0);

This should be in each subtest which needs this.

> +	} else {
> +		igt_setup_runtime_pm(fd);

This should also be in fixture, consider creating xe_pxp_pm test.

> +	}
> +
> +	igt_subtest_f("pxp-termination-key-update-post-%s", tag) {
> +		require_pxp_render(pxp_supported, devid);
> +		test_pxp_teardown_keychange(fd, type);
> +	}
> +	igt_subtest_f("pxp-stale-bo-bind-post-%s", tag) {
> +		require_pxp(pxp_supported);
> +		test_pxp_stale_bo_bind(fd, type);
> +	}
> +	igt_subtest_f("pxp-stale-bo-exec-post-%s", tag) {
> +		require_pxp(pxp_supported);
> +		test_pxp_stale_bo_exec(fd, type);
> +	}
> +
> +	/* An active PXP queue holds an RPM ref, so we can't test RPM with it */
> +	if (type != PXP_TERMINATION_RPM) {
> +		igt_subtest_f("pxp-stale-queue-post-%s", tag) {
> +			require_pxp(pxp_supported);
> +			test_pxp_stale_queue_execution(fd, type);
> +		}
> +
> +		close(fw_handle);
> +	} else {
> +		igt_restore_runtime_pm();
> +	}

Same here, both close() and ...restore...() should be done in
igt_fixture { ... }

To test if all is ok call your test with --show-testlist
without sudo:

./xe_pxp --show-testlist

Regards,
Kamil

> +}
> +
>  igt_main
>  {
>  	int xe_fd = -1;
> @@ -470,6 +891,28 @@ igt_main
>  		}
>  	}
>  
> +	igt_subtest_group {
> +		const struct mode {
> +			enum termination_type type;
> +			const char *tag;
> +		} modes[] = {
> +			{ PXP_TERMINATION_IRQ, "termination-irq" },
> +			{ PXP_TERMINATION_SUSPEND, "suspend" },
> +			{ PXP_TERMINATION_RPM, "rpm" },
> +			{ 0, NULL }
> +		};
> +
> +		igt_describe("Verify teardown management");
> +
> +		for (const struct mode *m = modes; m->tag; m++)
> +			termination_tests(xe_fd, pxp_supported, devid, m->type, m->tag);
> +
> +		igt_subtest("pxp-optout") {
> +			require_pxp(pxp_supported);
> +			test_pxp_optout(xe_fd);
> +		}
> +	}
> +
>  	igt_fixture {
>  		close(xe_fd);
>  	}
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t v3 6/7] tests/intel/xe_pxp: Termination tests

[Daniele Ceraolo Spurio]

On 2/14/2025 12:07 PM, Kamil Konieczny wrote:
> Hi Daniele,
> On 2025-02-12 at 16:39:00 -0800, Daniele Ceraolo Spurio wrote:
>> There are several events that can cause the PXP key to be invalidated
>> and trigger a PXP termination (suspend, PXP termination irq). After a
>> termination, we expect the key to be different and the raw encrypted
>> data to change for the same source data.
>> Additionally, all PXP objects are invalidated during a termination and
>> can no longer be used in submission or kept mapped to VMs; we therefore
>> need to test both the execution and bind ioctls to make sure they work
>> as expected after a termination.
>>
>> v2: move igt_require calls inside the subtest
>> v3: block rpm for tests trying a different type of termination, rework
>> invalid bind test to try a new vm as well (Alan)
> I have few nits, one is from our CI fail:
>
> ../tests/intel/xe_pxp.c:1065:53: error: ‘O_RDONLY’ undeclared (first use in this function); did you mean ‘STA_RONLY’?
> fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
>                                                    ^~~~~~~~
> 						 STA_RONLY
> ../tests/intel/xe_pxp.c:1065:53: note: each undeclared identifier is reported only once for each function it appears in
>
> Please include a proper lib header in a first patch which uses O_RDONLY.

This builds fine for me locally, I guess I'm getting O_RDONLY from some 
system include. I'll fix it.

>
> Rest nits below.
>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
>> ---
>>   lib/intel_batchbuffer.c |   8 +-
>>   lib/intel_batchbuffer.h |   1 +
>>   lib/xe/xe_ioctl.c       |  12 +-
>>   lib/xe/xe_ioctl.h       |   2 +
>>   tests/intel/xe_pxp.c    | 455 +++++++++++++++++++++++++++++++++++++++-
>>   5 files changed, 465 insertions(+), 13 deletions(-)
>>
> [...]
>>   
>> +static void termination_tests(int fd, bool pxp_supported, uint32_t devid,
>> +			      enum termination_type type, const char *tag)
>> +{
>> +	int fw_handle;
>> +
>> +	if (type != PXP_TERMINATION_RPM) {
>> +		/* avoid rpm entry for non-rpm tests */
>> +		fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
> File or dir opens should happen in igt_fixture
>
>> +		igt_require(fw_handle >= 0);
> This should be in each subtest which needs this.
>
>> +	} else {
>> +		igt_setup_runtime_pm(fd);
> This should also be in fixture, consider creating xe_pxp_pm test.
>
>> +	}
>> +
>> +	igt_subtest_f("pxp-termination-key-update-post-%s", tag) {
>> +		require_pxp_render(pxp_supported, devid);
>> +		test_pxp_teardown_keychange(fd, type);
>> +	}
>> +	igt_subtest_f("pxp-stale-bo-bind-post-%s", tag) {
>> +		require_pxp(pxp_supported);
>> +		test_pxp_stale_bo_bind(fd, type);
>> +	}
>> +	igt_subtest_f("pxp-stale-bo-exec-post-%s", tag) {
>> +		require_pxp(pxp_supported);
>> +		test_pxp_stale_bo_exec(fd, type);
>> +	}
>> +
>> +	/* An active PXP queue holds an RPM ref, so we can't test RPM with it */
>> +	if (type != PXP_TERMINATION_RPM) {
>> +		igt_subtest_f("pxp-stale-queue-post-%s", tag) {
>> +			require_pxp(pxp_supported);
>> +			test_pxp_stale_queue_execution(fd, type);
>> +		}
>> +
>> +		close(fw_handle);
>> +	} else {
>> +		igt_restore_runtime_pm();
>> +	}
> Same here, both close() and ...restore...() should be done in
> igt_fixture { ... }
>
> To test if all is ok call your test with --show-testlist
> without sudo:
>
> ./xe_pxp --show-testlist

is it ok if I move everything inside igt_subtest, without using 
igt_fixture? it makes things much simpler in code and I'm getting no 
errors from --show-testlist when doing so.

Thanks,
Daniele

>
> Regards,
> Kamil
>
>> +}
>> +
>>   igt_main
>>   {
>>   	int xe_fd = -1;
>> @@ -470,6 +891,28 @@ igt_main
>>   		}
>>   	}
>>   
>> +	igt_subtest_group {
>> +		const struct mode {
>> +			enum termination_type type;
>> +			const char *tag;
>> +		} modes[] = {
>> +			{ PXP_TERMINATION_IRQ, "termination-irq" },
>> +			{ PXP_TERMINATION_SUSPEND, "suspend" },
>> +			{ PXP_TERMINATION_RPM, "rpm" },
>> +			{ 0, NULL }
>> +		};
>> +
>> +		igt_describe("Verify teardown management");
>> +
>> +		for (const struct mode *m = modes; m->tag; m++)
>> +			termination_tests(xe_fd, pxp_supported, devid, m->type, m->tag);
>> +
>> +		igt_subtest("pxp-optout") {
>> +			require_pxp(pxp_supported);
>> +			test_pxp_optout(xe_fd);
>> +		}
>> +	}
>> +
>>   	igt_fixture {
>>   		close(xe_fd);
>>   	}
>> -- 
>> 2.43.0
>>