[igt-dev] [PATCH i-g-t 00/16] Introduce PXP Test

[igt-dev] [PATCH i-g-t 00/16] Introduce PXP Test

[Alan Previn]

This series adds gem_pxp tests for the new PXP subsystem currently
being reviewed at https://patchwork.freedesktop.org/series/86798/ .
This series currently includes 4 groups of tests addressing the
features and restrictions described by Daniele's series :
   1. test i915 interfaces for allocation of protected bo's
      and contexts and enforcement of UAPI rule disallowing the
      modification of parameters after it's been created.
   2. verify PXP subsystem protected sessions generate encrypted
      content on protected output buffers and decrypt protected
      inputs buffers.
   3. verify i915 PXP auto-teardown succeeds on suspend-resume
      cycles and gem-exec of stale protected assets fail. Ensure
      protected-contexts adhere to stricter invalidation
      enforcement upon teardown event.
   4. Ensure that display plane decryption works as expected with
      protected buffers with and without HDCP.

NOTE: Although this series is on a third revision, the feature
has not been merged into the kernel yet. One final revision may
be necesssary depending on whether additional UAPI changes are
made on the kernel series. 

Changes from prior rev1 to now:
   v3:
      - Addressed all rev2 review comments.
      - In line with one of the rev2 comments, a thorough fixup
        of all line-breaks in function calls was made for a more
        consistent styling.
      - Rebased on igt upstream repo and updated to latest kernel
        UAPI that added GEM_CREATE_EXT.
   v2: 
      - Addressed all rev1 review comments except these:
           1.Chris Wilson : "...have the caller do 1-3 once for its protected
             context. Call it something like intel_bb_enable_pxp(),
             intel_bb_set_pxp if it should be reversible.".
             -  This couldn't be implemented because [1] HW needs different
             instruction sequences for enabling/disabling PXP depending
             on the engine class and [2] the pair of "pxp-enable" and "pxp-
             disable" instructions need to be contained within the same batch
             that is dispatched to the hardware. That said, implementing
             internal intel_batchbuffer funtionality for this would conflict
             with how rendercopy_gen9 uses batch buffer memory by repositioing
             the pointer and consuming unused portions of the batch buffer as
             3d state offsets that batchbuffer has no visibility.
         
      - Added these additional subtests:
           1. verify that buffer sharing works across testing pxp context.
           2. verify teardown bans contexts via DRM_IOCTL_I915_GET_RESET_STAT.
           3. verify display plane decryption of protected buffers.

Alan Previn (15):
  Sync i915_drm.h UAPI from kernel
  Add PXP UAPI support in i915_drm.h
  Add basic PXP testing of buffer and context alloc
  Perform a regular 3d copy as a control checkpoint
  Add PXP attribute support in batchbuffer and buffer_ops libs
  Add MI_SET_APPID instruction definition
  Enable protected session cmd in gen12_render_copyfunc
  Add subtest to copy raw source to protected dest
  Add test where both src and dest are protected
  Verify PXP teardown occured through suspend-resume
  Verify execbuf fails with stale PXP context after teardown
  Verify execbuf fails with stale PXP buffer after teardown
  Verify execbuf ok with stale prot-buff and regular context
  Ensure RESET_STAT reports invalidated protected context
  Verify protected surfaces are dma buffer sharable

Karthik B S (1):
  tests/i915_pxp: CRC validation for display tests.

 include/drm-uapi/i915_drm.h |  448 +++++++++++-
 lib/intel_batchbuffer.c     |   23 +-
 lib/intel_batchbuffer.h     |   31 +
 lib/intel_bufops.h          |   15 +
 lib/intel_reg.h             |    9 +
 lib/rendercopy_gen9.c       |   72 +-
 tests/i915/gem_pxp.c        | 1308 +++++++++++++++++++++++++++++++++++
 tests/meson.build           |    1 +
 8 files changed, 1864 insertions(+), 43 deletions(-)
 create mode 100644 tests/i915/gem_pxp.c

-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 01/16] Sync i915_drm.h UAPI from kernel

[Alan Previn]

Sync i915_drm.h UAPI from drm-tip kernel baselined on following tag:
"drm-tip: 2021y-05m-12d-18h-04m-49s UTC integration manifest"

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 include/drm-uapi/i915_drm.h | 396 ++++++++++++++++++++++++++++++++----
 1 file changed, 361 insertions(+), 35 deletions(-)

diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index bf9ea471..d9fbf218 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -62,8 +62,8 @@ extern "C" {
 #define I915_ERROR_UEVENT		"ERROR"
 #define I915_RESET_UEVENT		"RESET"
 
-/*
- * i915_user_extension: Base class for defining a chain of extensions
+/**
+ * struct i915_user_extension - Base class for defining a chain of extensions
  *
  * Many interfaces need to grow over time. In most cases we can simply
  * extend the struct and have userspace pass in more data. Another option,
@@ -76,12 +76,58 @@ extern "C" {
  * increasing complexity, and for large parts of that interface to be
  * entirely optional. The downside is more pointer chasing; chasing across
  * the boundary with pointers encapsulated inside u64.
+ *
+ * Example chaining:
+ *
+ * .. code-block:: C
+ *
+ *	struct i915_user_extension ext3 {
+ *		.next_extension = 0, // end
+ *		.name = ...,
+ *	};
+ *	struct i915_user_extension ext2 {
+ *		.next_extension = (uintptr_t)&ext3,
+ *		.name = ...,
+ *	};
+ *	struct i915_user_extension ext1 {
+ *		.next_extension = (uintptr_t)&ext2,
+ *		.name = ...,
+ *	};
+ *
+ * Typically the struct i915_user_extension would be embedded in some uAPI
+ * struct, and in this case we would feed it the head of the chain(i.e ext1),
+ * which would then apply all of the above extensions.
+ *
  */
 struct i915_user_extension {
+	/**
+	 * @next_extension:
+	 *
+	 * Pointer to the next struct i915_user_extension, or zero if the end.
+	 */
 	__u64 next_extension;
+	/**
+	 * @name: Name of the extension.
+	 *
+	 * Note that the name here is just some integer.
+	 *
+	 * Also note that the name space for this is not global for the whole
+	 * driver, but rather its scope/meaning is limited to the specific piece
+	 * of uAPI which has embedded the struct i915_user_extension.
+	 */
 	__u32 name;
-	__u32 flags; /* All undefined bits must be zero. */
-	__u32 rsvd[4]; /* Reserved for future use; must be zero. */
+	/**
+	 * @flags: MBZ
+	 *
+	 * All undefined bits must be zero.
+	 */
+	__u32 flags;
+	/**
+	 * @rsvd: MBZ
+	 *
+	 * Reserved for future use; must be zero.
+	 */
+	__u32 rsvd[4];
 };
 
 /*
@@ -360,6 +406,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_QUERY			0x39
 #define DRM_I915_GEM_VM_CREATE		0x3a
 #define DRM_I915_GEM_VM_DESTROY		0x3b
+#define DRM_I915_GEM_CREATE_EXT		0x3c
 /* Must be kept compact -- no holes */
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
@@ -392,6 +439,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_ENTERVT	DRM_IO(DRM_COMMAND_BASE + DRM_I915_GEM_ENTERVT)
 #define DRM_IOCTL_I915_GEM_LEAVEVT	DRM_IO(DRM_COMMAND_BASE + DRM_I915_GEM_LEAVEVT)
 #define DRM_IOCTL_I915_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_CREATE, struct drm_i915_gem_create)
+#define DRM_IOCTL_I915_GEM_CREATE_EXT	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_CREATE_EXT, struct drm_i915_gem_create_ext)
 #define DRM_IOCTL_I915_GEM_PREAD	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PREAD, struct drm_i915_gem_pread)
 #define DRM_IOCTL_I915_GEM_PWRITE	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
 #define DRM_IOCTL_I915_GEM_MMAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
@@ -943,6 +991,7 @@ struct drm_i915_gem_exec_object {
 	__u64 offset;
 };
 
+/* DRM_IOCTL_I915_GEM_EXECBUFFER was removed in Linux 5.13 */
 struct drm_i915_gem_execbuffer {
 	/**
 	 * List of buffers to be validated with their relocations to be
@@ -1053,12 +1102,12 @@ struct drm_i915_gem_exec_fence {
 	__u32 flags;
 };
 
-/**
+/*
  * See drm_i915_gem_execbuffer_ext_timeline_fences.
  */
 #define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
 
-/**
+/*
  * This structure describes an array of drm_syncobj and associated points for
  * timeline variants of drm_syncobj. It is invalid to append this structure to
  * the execbuf if I915_EXEC_FENCE_ARRAY is set.
@@ -1699,7 +1748,7 @@ struct drm_i915_gem_context_param {
 	__u64 value;
 };
 
-/**
+/*
  * Context SSEU programming
  *
  * It may be necessary for either functional or performance reason to configure
@@ -2066,7 +2115,7 @@ struct drm_i915_perf_open_param {
 	__u64 properties_ptr;
 };
 
-/**
+/*
  * Enable data capture for a stream that was either opened in a disabled state
  * via I915_PERF_FLAG_DISABLED or was later disabled via
  * I915_PERF_IOCTL_DISABLE.
@@ -2080,7 +2129,7 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_ENABLE	_IO('i', 0x0)
 
-/**
+/*
  * Disable data capture for a stream.
  *
  * It is an error to try and read a stream that is disabled.
@@ -2089,11 +2138,11 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_DISABLE	_IO('i', 0x1)
 
-/**
+/*
  * Change metrics_set captured by a stream.
  *
  * If the stream is bound to a specific context, the configuration change
- * will performed __inline__ with that context such that it takes effect before
+ * will performed inline with that context such that it takes effect before
  * the next execbuf submission.
  *
  * Returns the previously bound metrics set id, or a negative error code.
@@ -2102,7 +2151,7 @@ struct drm_i915_perf_open_param {
  */
 #define I915_PERF_IOCTL_CONFIG	_IO('i', 0x2)
 
-/**
+/*
  * Common to all i915 perf records
  */
 struct drm_i915_perf_record_header {
@@ -2150,7 +2199,7 @@ enum drm_i915_perf_record_type {
 	DRM_I915_PERF_RECORD_MAX /* non-ABI */
 };
 
-/**
+/*
  * Structure to upload perf dynamic configuration into the kernel.
  */
 struct drm_i915_perf_oa_config {
@@ -2171,53 +2220,95 @@ struct drm_i915_perf_oa_config {
 	__u64 flex_regs_ptr;
 };
 
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
 struct drm_i915_query_item {
+	/** @query_id: The id for this query */
 	__u64 query_id;
 #define DRM_I915_QUERY_TOPOLOGY_INFO    1
 #define DRM_I915_QUERY_ENGINE_INFO	2
 #define DRM_I915_QUERY_PERF_CONFIG      3
+#define DRM_I915_QUERY_MEMORY_REGIONS   4
 /* Must be kept compact -- no holes and well documented */
 
-	/*
+	/**
+	 * @length:
+	 *
 	 * When set to zero by userspace, this is filled with the size of the
-	 * data to be written at the data_ptr pointer. The kernel sets this
+	 * data to be written at the @data_ptr pointer. The kernel sets this
 	 * value to a negative value to signal an error on a particular query
 	 * item.
 	 */
 	__s32 length;
 
-	/*
+	/**
+	 * @flags:
+	 *
 	 * When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
 	 *
 	 * When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-	 * following :
-	 *         - DRM_I915_QUERY_PERF_CONFIG_LIST
-	 *         - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-	 *         - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+	 * following:
+	 *
+	 *	- DRM_I915_QUERY_PERF_CONFIG_LIST
+	 *      - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+	 *      - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
 	 */
 	__u32 flags;
 #define DRM_I915_QUERY_PERF_CONFIG_LIST          1
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
 #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID   3
 
-	/*
-	 * Data will be written at the location pointed by data_ptr when the
-	 * value of length matches the length of the data to be written by the
+	/**
+	 * @data_ptr:
+	 *
+	 * Data will be written at the location pointed by @data_ptr when the
+	 * value of @length matches the length of the data to be written by the
 	 * kernel.
 	 */
 	__u64 data_ptr;
 };
 
+/**
+ * struct drm_i915_query - Supply an array of struct drm_i915_query_item for the
+ * kernel to fill out.
+ *
+ * Note that this is generally a two step process for each struct
+ * drm_i915_query_item in the array:
+ *
+ * 1. Call the DRM_IOCTL_I915_QUERY, giving it our array of struct
+ *    drm_i915_query_item, with &drm_i915_query_item.length set to zero. The
+ *    kernel will then fill in the size, in bytes, which tells userspace how
+ *    memory it needs to allocate for the blob(say for an array of properties).
+ *
+ * 2. Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ *    &drm_i915_query_item.data_ptr equal to our newly allocated blob. Note that
+ *    the &drm_i915_query_item.length should still be the same as what the
+ *    kernel previously set. At this point the kernel can fill in the blob.
+ *
+ * Note that for some query items it can make sense for userspace to just pass
+ * in a buffer/blob equal to or larger than the required size. In this case only
+ * a single ioctl call is needed. For some smaller query items this can work
+ * quite well.
+ *
+ */
 struct drm_i915_query {
+	/** @num_items: The number of elements in the @items_ptr array */
 	__u32 num_items;
 
-	/*
-	 * Unused for now. Must be cleared to zero.
+	/**
+	 * @flags: Unused for now. Must be cleared to zero.
 	 */
 	__u32 flags;
 
-	/*
-	 * This points to an array of num_items drm_i915_query_item structures.
+	/**
+	 * @items_ptr:
+	 *
+	 * Pointer to an array of struct drm_i915_query_item. The number of
+	 * array elements is @num_items.
 	 */
 	__u64 items_ptr;
 };
@@ -2291,21 +2382,21 @@ struct drm_i915_query_topology_info {
  * Describes one engine and it's capabilities as known to the driver.
  */
 struct drm_i915_engine_info {
-	/** Engine class and instance. */
+	/** @engine: Engine class and instance. */
 	struct i915_engine_class_instance engine;
 
-	/** Reserved field. */
+	/** @rsvd0: Reserved field. */
 	__u32 rsvd0;
 
-	/** Engine flags. */
+	/** @flags: Engine flags. */
 	__u64 flags;
 
-	/** Capabilities of this engine. */
+	/** @capabilities: Capabilities of this engine. */
 	__u64 capabilities;
 #define I915_VIDEO_CLASS_CAPABILITY_HEVC		(1 << 0)
 #define I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC	(1 << 1)
 
-	/** Reserved fields. */
+	/** @rsvd1: Reserved fields. */
 	__u64 rsvd1[4];
 };
 
@@ -2316,13 +2407,13 @@ struct drm_i915_engine_info {
  * an array of struct drm_i915_engine_info structures.
  */
 struct drm_i915_query_engine_info {
-	/** Number of struct drm_i915_engine_info structs following. */
+	/** @num_engines: Number of struct drm_i915_engine_info structs following. */
 	__u32 num_engines;
 
-	/** MBZ */
+	/** @rsvd: MBZ */
 	__u32 rsvd[3];
 
-	/** Marker for drm_i915_engine_info structures. */
+	/** @engines: Marker for drm_i915_engine_info structures. */
 	struct drm_i915_engine_info engines[];
 };
 
@@ -2376,6 +2467,241 @@ struct drm_i915_query_perf_config {
 	__u8 data[];
 };
 
+/**
+ * enum drm_i915_gem_memory_class - Supported memory classes
+ */
+enum drm_i915_gem_memory_class {
+	/** @I915_MEMORY_CLASS_SYSTEM: System memory */
+	I915_MEMORY_CLASS_SYSTEM = 0,
+	/** @I915_MEMORY_CLASS_DEVICE: Device local-memory */
+	I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance - Identify particular memory region
+ */
+struct drm_i915_gem_memory_class_instance {
+	/** @memory_class: See enum drm_i915_gem_memory_class */
+	__u16 memory_class;
+
+	/** @memory_instance: Which instance */
+	__u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info - Describes one region as known to the
+ * driver.
+ *
+ * Note that we reserve some stuff here for potential future work. As an example
+ * we might want expose the capabilities for a given region, which could include
+ * things like if the region is CPU mappable/accessible, what are the supported
+ * mapping types etc.
+ *
+ * Note that to extend struct drm_i915_memory_region_info and struct
+ * drm_i915_query_memory_regions in the future the plan is to do the following:
+ *
+ * .. code-block:: C
+ *
+ *	struct drm_i915_memory_region_info {
+ *		struct drm_i915_gem_memory_class_instance region;
+ *		union {
+ *			__u32 rsvd0;
+ *			__u32 new_thing1;
+ *		};
+ *		...
+ *		union {
+ *			__u64 rsvd1[8];
+ *			struct {
+ *				__u64 new_thing2;
+ *				__u64 new_thing3;
+ *				...
+ *			};
+ *		};
+ *	};
+ *
+ * With this things should remain source compatible between versions for
+ * userspace, even as we add new fields.
+ *
+ * Note this is using both struct drm_i915_query_item and struct drm_i915_query.
+ * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS
+ * at &drm_i915_query_item.query_id.
+ */
+struct drm_i915_memory_region_info {
+	/** @region: The class:instance pair encoding */
+	struct drm_i915_gem_memory_class_instance region;
+
+	/** @rsvd0: MBZ */
+	__u32 rsvd0;
+
+	/** @probed_size: Memory probed by the driver (-1 = unknown) */
+	__u64 probed_size;
+
+	/** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
+	__u64 unallocated_size;
+
+	/** @rsvd1: MBZ */
+	__u64 rsvd1[8];
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * The region info query enumerates all regions known to the driver by filling
+ * in an array of struct drm_i915_memory_region_info structures.
+ *
+ * Example for getting the list of supported regions:
+ *
+ * .. code-block:: C
+ *
+ *	struct drm_i915_query_memory_regions *info;
+ *	struct drm_i915_query_item item = {
+ *		.query_id = DRM_I915_QUERY_MEMORY_REGIONS;
+ *	};
+ *	struct drm_i915_query query = {
+ *		.num_items = 1,
+ *		.items_ptr = (uintptr_t)&item,
+ *	};
+ *	int err, i;
+ *
+ *	// First query the size of the blob we need, this needs to be large
+ *	// enough to hold our array of regions. The kernel will fill out the
+ *	// item.length for us, which is the number of bytes we need.
+ *	err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ *	if (err) ...
+ *
+ *	info = calloc(1, item.length);
+ *	// Now that we allocated the required number of bytes, we call the ioctl
+ *	// again, this time with the data_ptr pointing to our newly allocated
+ *	// blob, which the kernel can then populate with the all the region info.
+ *	item.data_ptr = (uintptr_t)&info,
+ *
+ *	err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ *	if (err) ...
+ *
+ *	// We can now access each region in the array
+ *	for (i = 0; i < info->num_regions; i++) {
+ *		struct drm_i915_memory_region_info mr = info->regions[i];
+ *		u16 class = mr.region.class;
+ *		u16 instance = mr.region.instance;
+ *
+ *		....
+ *	}
+ *
+ *	free(info);
+ */
+struct drm_i915_query_memory_regions {
+	/** @num_regions: Number of supported regions */
+	__u32 num_regions;
+
+	/** @rsvd: MBZ */
+	__u32 rsvd[3];
+
+	/** @regions: Info about each supported region */
+	struct drm_i915_memory_region_info regions[];
+};
+
+/**
+ * struct drm_i915_gem_create_ext - Existing gem_create behaviour, with added
+ * extension support using struct i915_user_extension.
+ *
+ * Note that in the future we want to have our buffer flags here, at least for
+ * the stuff that is immutable. Previously we would have two ioctls, one to
+ * create the object with gem_create, and another to apply various parameters,
+ * however this creates some ambiguity for the params which are considered
+ * immutable. Also in general we're phasing out the various SET/GET ioctls.
+ */
+struct drm_i915_gem_create_ext {
+	/**
+	 * @size: Requested size for the object.
+	 *
+	 * The (page-aligned) allocated size for the object will be returned.
+	 *
+	 * Note that for some devices we have might have further minimum
+	 * page-size restrictions(larger than 4K), like for device local-memory.
+	 * However in general the final size here should always reflect any
+	 * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
+	 * extension to place the object in device local-memory.
+	 */
+	__u64 size;
+	/**
+	 * @handle: Returned handle for the object.
+	 *
+	 * Object handles are nonzero.
+	 */
+	__u32 handle;
+	/** @flags: MBZ */
+	__u32 flags;
+	/**
+	 * @extensions: The chain of extensions to apply to this object.
+	 *
+	 * This will be useful in the future when we need to support several
+	 * different extensions, and we need to apply more than one when
+	 * creating the object. See struct i915_user_extension.
+	 *
+	 * If we don't supply any extensions then we get the same old gem_create
+	 * behaviour.
+	 *
+	 * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
+	 * struct drm_i915_gem_create_ext_memory_regions.
+	 */
+#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
+	__u64 extensions;
+};
+
+/**
+ * struct drm_i915_gem_create_ext_memory_regions - The
+ * I915_GEM_CREATE_EXT_MEMORY_REGIONS extension.
+ *
+ * Set the object with the desired set of placements/regions in priority
+ * order. Each entry must be unique and supported by the device.
+ *
+ * This is provided as an array of struct drm_i915_gem_memory_class_instance, or
+ * an equivalent layout of class:instance pair encodings. See struct
+ * drm_i915_query_memory_regions and DRM_I915_QUERY_MEMORY_REGIONS for how to
+ * query the supported regions for a device.
+ *
+ * As an example, on discrete devices, if we wish to set the placement as
+ * device local-memory we can do something like:
+ *
+ * .. code-block:: C
+ *
+ *	struct drm_i915_gem_memory_class_instance region_lmem = {
+ *              .memory_class = I915_MEMORY_CLASS_DEVICE,
+ *              .memory_instance = 0,
+ *      };
+ *      struct drm_i915_gem_create_ext_memory_regions regions = {
+ *              .base = { .name = I915_GEM_CREATE_EXT_MEMORY_REGIONS },
+ *              .regions = (uintptr_t)&region_lmem,
+ *              .num_regions = 1,
+ *      };
+ *      struct drm_i915_gem_create_ext create_ext = {
+ *              .size = 16 * PAGE_SIZE,
+ *              .extensions = (uintptr_t)&regions,
+ *      };
+ *
+ *      int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
+ *      if (err) ...
+ *
+ * At which point we get the object handle in &drm_i915_gem_create_ext.handle,
+ * along with the final object size in &drm_i915_gem_create_ext.size, which
+ * should account for any rounding up, if required.
+ */
+struct drm_i915_gem_create_ext_memory_regions {
+	/** @base: Extension link. See struct i915_user_extension. */
+	struct i915_user_extension base;
+
+	/** @pad: MBZ */
+	__u32 pad;
+	/** @num_regions: Number of elements in the @regions array. */
+	__u32 num_regions;
+	/**
+	 * @regions: The regions/placements array.
+	 *
+	 * An array of struct drm_i915_gem_memory_class_instance.
+	 */
+	__u64 regions;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 02/16] Add PXP UAPI support in i915_drm.h

[Alan Previn]

At the time of this commit, PXP hasnt been merged into
upstream kernel so this has no kernel tag reference yet

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 include/drm-uapi/i915_drm.h | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index d9fbf218..e67e3e5c 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -1743,6 +1743,26 @@ struct drm_i915_gem_context_param {
  * Default is 16 KiB.
  */
 #define I915_CONTEXT_PARAM_RINGSIZE	0xc
+
+/*
+ * I915_CONTEXT_PARAM_PROTECTED_CONTENT:
+ *
+ * Mark that the context makes use of protected content, which will result
+ * in the context being invalidated when the protected content session is.
+ * This flag can only be set at context creation time and, when set to true,
+ * must be preceded by an explicit setting of I915_CONTEXT_PARAM_RECOVERABLE
+ * to false. This flag can't be set to true in conjunction with setting the
+ * I915_CONTEXT_PARAM_BANNABLE flag to false.
+ *
+ * Given the numerous restriction on this flag, there are several unique
+ * failure cases:
+ *
+ * -ENODEV: feature not available
+ * -EEXIST: trying to modify an existing context
+ * -EPERM: trying to mark a recoverable or not bannable context as protected
+ * -EACCES: submitting an invalidated context for execution
+ */
+#define I915_CONTEXT_PARAM_PROTECTED_CONTENT    0xd
 /* Must be kept compact -- no holes and well documented */
 
 	__u64 value;
@@ -1973,6 +1993,12 @@ struct drm_i915_reg_read {
 struct drm_i915_reset_stats {
 	__u32 ctx_id;
 	__u32 flags;
+	/*
+	 * contexts marked as using protected content are invalidated when the
+	 * protected content session dies. Submission of invalidated contexts
+	 * is rejected with -EACCES.
+	 */
+#define I915_CONTEXT_INVALIDATED 0x1
 
 	/* All resets since boot/module reload, for all contexts */
 	__u32 reset_count;
@@ -2645,6 +2671,7 @@ struct drm_i915_gem_create_ext {
 	 * struct drm_i915_gem_create_ext_memory_regions.
 	 */
 #define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
+#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
 	__u64 extensions;
 };
 
@@ -2702,6 +2729,31 @@ struct drm_i915_gem_create_ext_memory_regions {
 	__u64 regions;
 };
 
+/*
+ * I915_OBJECT_PARAM_PROTECTED_CONTENT:
+ *
+ * If set to true, buffer contents is expected to be protected by PXP
+ * encryption and requires decryption for scan out and processing. This is
+ * only possible on platforms that have PXP enabled, on all other scenarios
+ * setting this flag will cause the ioctl to fail and return -ENODEV.
+ *
+ * The buffer contents are considered invalid after a PXP session teardown.
+ * It is recommended to use protected buffers only with contexts created
+ * using the I915_CONTEXT_PARAM_PROTECTED_CONTENT flag, as that will enable
+ * extra checks at submission time on the validity of the objects involved,
+ * which can lead to the following errors:
+ *
+ * -ENODEV: PXP session not currently active
+ * -EIO: buffer has become invalid after a teardown event
+ */
+struct drm_i915_gem_create_ext_protected_content {
+	struct i915_user_extension base;
+	__u32 flags;
+};
+
+/* ID of the protected content session managed by i915 when PXP is active */
+#define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Alan Previn]

Test PXP capability support as well as the allocation of protected
buffers and protected contexts.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 366 +++++++++++++++++++++++++++++++++++++++++++
 tests/meson.build    |   1 +
 2 files changed, 367 insertions(+)
 create mode 100644 tests/i915/gem_pxp.c

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
new file mode 100644
index 00000000..6340daae
--- /dev/null
+++ b/tests/i915/gem_pxp.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include <sys/ioctl.h>
+
+#include "igt.h"
+#include "i915/gem.h"
+
+IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
+/* Note: PXP = "Protected Xe Path" */
+
+/* test-configs for protected buffer creation*/
+#define BO_ALLOC_NO_HW_SUPPORT 1
+#define BO_ALLOC_PROTECT_ON 2
+#define BO_ALLOC_PROTECT_OFF 3
+
+/* test-configs for protected context creation*/
+#define CTX_ALLOC_NO_HW_SUPPORT 1
+#define CTX_ALLOC_RECOVER_OFF_PROTECT_OFF 2
+#define CTX_ALLOC_RECOVER_OFF_PROTECT_ON 3
+#define CTX_ALLOC_RECOVER_ON_PROTECT_OFF 4
+#define CTX_ALLOC_RECOVER_ON_PROTECT_ON 5
+#define CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON 6
+#define CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF 7
+#define CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID 8
+#define CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID 9
+
+static bool is_pxp_hw_supported(int i915)
+{
+	uint32_t devid = intel_get_drm_devid(i915);
+	const struct intel_device_info *devinfo = intel_get_device_info(devid);
+
+	if (!devinfo) {
+		igt_info("No device info found - assume no PXP support.\n");
+		return false;
+	}
+	if (devinfo->is_tigerlake || devinfo->is_rocketlake ||
+		devinfo->is_alderlake_s) {
+		return true;
+	}
+
+	return false;
+}
+
+static uint32_t create_protected_bo(int i915, uint32_t size, bool protected_is_true,
+				    int expected_return)
+{
+	int ret;
+
+	struct drm_i915_gem_create_ext_protected_content protected_ext = {
+		.base = { .name = I915_GEM_CREATE_EXT_PROTECTED_CONTENT },
+		.flags = 0,
+	};
+
+	struct drm_i915_gem_create_ext create_ext = {
+		.size = size,
+		.extensions = 0,
+	};
+
+	if (protected_is_true)
+		create_ext.extensions = (uintptr_t)&protected_ext;
+
+	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
+	igt_assert_eq(ret, expected_return);
+
+	return create_ext.handle;
+}
+
+static void test_create_protected_buffer(int i915, uint32_t test_cfg)
+{
+	uint32_t bo;
+
+	switch (test_cfg) {
+	case BO_ALLOC_NO_HW_SUPPORT:
+		bo = create_protected_bo(i915, 4096, false, 0);
+		gem_close(i915, bo);
+		bo = create_protected_bo(i915, 4096, true, -ENODEV);
+		igt_assert_eq(bo, 0);
+		break;
+
+	case BO_ALLOC_PROTECT_OFF:
+		bo = create_protected_bo(i915, 4096, false, 0);
+		gem_close(i915, bo);
+		break;
+
+	case BO_ALLOC_PROTECT_ON:
+		bo = create_protected_bo(i915, 4096, true, 0);
+		gem_close(i915, bo);
+		break;
+
+	default:
+		igt_info("Skipping unknown buffer test_cfg = %d\n", test_cfg);
+		break;
+	}
+}
+
+static int create_ext_ioctl(int i915, struct drm_i915_gem_context_create_ext *arg)
+{
+	int err;
+
+	err = 0;
+	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
+		err = -errno;
+		igt_assume(err);
+	}
+
+	errno = 0;
+	return err;
+}
+
+static uint32_t create_protected_ctx(int i915, bool with_protected_param, bool protected_is_true,
+				     bool with_recoverable_param, bool recoverable_is_true,
+				     int expected_return)
+{
+	int ret;
+
+	struct drm_i915_gem_context_create_ext_setparam p_prot = {
+		.base = {
+			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+			.next_extension = 0,
+		},
+		.param = {
+			.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
+			.value = 0,
+		}
+	};
+	struct drm_i915_gem_context_create_ext_setparam p_norecover = {
+		.base = {
+			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
+			.next_extension = 0,
+		},
+		.param = {
+			.param = I915_CONTEXT_PARAM_RECOVERABLE,
+			.value = 0,
+		}
+	};
+	struct drm_i915_gem_context_create_ext create = {
+		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
+		.extensions = 0,
+	};
+
+	p_prot.param.value = protected_is_true;
+	p_norecover.param.value = recoverable_is_true;
+
+	if (with_protected_param && with_recoverable_param) {
+		create.extensions = to_user_pointer(&(p_norecover.base));
+		p_norecover.base.next_extension = to_user_pointer(&(p_prot.base));
+	} else if (!with_protected_param && with_recoverable_param) {
+		create.extensions = to_user_pointer(&(p_norecover.base));
+		p_norecover.base.next_extension = 0;
+	} else if (with_protected_param && !with_recoverable_param) {
+		create.extensions = to_user_pointer(&(p_prot.base));
+		p_prot.base.next_extension = 0;
+	} else if (!with_protected_param && !with_recoverable_param) {
+		create.flags = 0;
+	}
+
+	ret = create_ext_ioctl(i915, &create);
+	igt_assert_eq(ret, expected_return);
+
+	return create.ctx_id;
+}
+
+#define CHANGE_PARAM_PROTECTED 0x0001
+#define CHANGE_PARAM_RECOVERY 0x0002
+static void modify_ctx_protected_param(int i915, uint32_t ctx_id, uint32_t change_param_mask,
+				       bool param_value, int expected_return)
+{
+	int ret;
+
+	struct drm_i915_gem_context_param ctx_param = {
+		.ctx_id = ctx_id,
+		.param = 0,
+		.value = 0,
+	};
+
+	if (change_param_mask == CHANGE_PARAM_PROTECTED) {
+		ctx_param.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT;
+		ctx_param.value = (int)param_value;
+	} else if (change_param_mask == CHANGE_PARAM_RECOVERY) {
+		ctx_param.param = I915_CONTEXT_PARAM_RECOVERABLE;
+		ctx_param.value = (int)param_value;
+	} else {
+		return;
+	}
+
+	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &ctx_param);
+
+	igt_assert_eq(ret, expected_return);
+}
+
+static void assert_ctx_protected_param(int i915, uint32_t ctx_id, bool is_protected)
+{
+	int ret;
+
+	struct drm_i915_gem_context_param ctx_param = {
+		.ctx_id = ctx_id,
+		.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
+	};
+
+	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
+	igt_assert_eq(ret, 0);
+	igt_assert_eq(ctx_param.value, (int)is_protected);
+}
+
+static void assert_ctx_recovery_param(int i915, uint32_t ctx_id, bool is_recoverable)
+{
+	int ret;
+
+	struct drm_i915_gem_context_param ctx_param = {
+		.ctx_id = ctx_id,
+		.param = I915_CONTEXT_PARAM_RECOVERABLE,
+	};
+
+	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
+	igt_assert_eq(ret, 0);
+	igt_assert_eq(ctx_param.value, (int)is_recoverable);
+}
+
+static void test_create_protected_context(int i915, uint32_t test_cfg)
+{
+	uint32_t ctx;
+
+	switch (test_cfg) {
+	case CTX_ALLOC_NO_HW_SUPPORT:
+		ctx = create_protected_ctx(i915, true, true, true, false, -ENODEV);
+		igt_assert_eq(ctx, 0);
+	case CTX_ALLOC_RECOVER_OFF_PROTECT_OFF:
+		ctx = create_protected_ctx(i915, true, false, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, false);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_ALLOC_RECOVER_OFF_PROTECT_ON:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_ALLOC_RECOVER_ON_PROTECT_OFF:
+		ctx = create_protected_ctx(i915, true, false, true, true, 0);
+		assert_ctx_protected_param(i915, ctx, false);
+		assert_ctx_recovery_param(i915, ctx, true);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_ALLOC_RECOVER_ON_PROTECT_ON:
+		ctx = create_protected_ctx(i915, true, true, true, true, -EPERM);
+		igt_assert_eq(ctx, 0);
+		ctx = create_protected_ctx(i915, true, true, false, false, -EPERM);
+		igt_assert_eq(ctx, 0);
+		break;
+
+	case CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
+		assert_ctx_protected_param(i915, ctx, true);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	case CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID:
+		ctx = create_protected_ctx(i915, false, false, false, false, 0);
+		assert_ctx_protected_param(i915, ctx, false);
+		assert_ctx_recovery_param(i915, ctx, true);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, false, 0);
+		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, true, -EPERM);
+		assert_ctx_protected_param(i915, ctx, false);
+		assert_ctx_recovery_param(i915, ctx, false);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	default:
+		igt_info("Skipping unknown context test_cfg = %d\n", test_cfg);
+		break;
+	}
+}
+
+igt_main
+{
+	int i915 = -1;
+	bool pxp_supported = false;
+
+	igt_fixture
+	{
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require(i915);
+		igt_require_gem(i915);
+		pxp_supported = is_pxp_hw_supported(i915);
+	}
+
+	igt_subtest_group {
+		igt_fixture {
+			igt_require((pxp_supported == 0));
+		}
+
+		igt_describe("Verify protected buffer on unsupported hw:");
+		igt_subtest("hw-rejects-pxp-buffer")
+			test_create_protected_buffer(i915, BO_ALLOC_NO_HW_SUPPORT);
+		igt_describe("Verify protected context on unsupported hw:");
+		igt_subtest("hw-rejects-pxp-context")
+			test_create_protected_context(i915, CTX_ALLOC_NO_HW_SUPPORT);
+	}
+
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(pxp_supported);
+		}
+
+		igt_describe("Verify protected buffer on supported hw:");
+		igt_subtest("create-regular-buffer")
+			test_create_protected_buffer(i915, BO_ALLOC_PROTECT_OFF);
+		igt_subtest("create-protected-buffer")
+			test_create_protected_buffer(i915, BO_ALLOC_PROTECT_ON);
+
+		igt_describe("Verify protected context on supported hw:");
+		igt_subtest("create-regular-context-1")
+			test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_OFF);
+		igt_subtest("create-regular-context-2")
+			test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_OFF);
+		igt_subtest("fail-invalid-protected-context")
+			test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_ON);
+		igt_subtest("create-valid-protected-context")
+			test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_ON);
+
+		igt_describe("Verify protected context integrity:");
+		igt_subtest("reject-modify-context-protection-on")
+			test_create_protected_context(i915, CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID);
+		igt_subtest("reject-modify-context-protection-off-1")
+			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON);
+		igt_subtest("reject-modify-context-protection-off-2")
+			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF);
+		igt_subtest("reject-modify-context-protection-off-3")
+			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID);
+	}
+
+	igt_fixture {
+		close(i915);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 19cc4ebe..e0bff4d1 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -187,6 +187,7 @@ i915_progs = [
 	'gem_pread_after_blit',
 	'gem_pwrite',
 	'gem_pwrite_snooped',
+	'gem_pxp',
 	'gem_read_read_speed',
 	'gem_readwrite',
 	'gem_reg_read',
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 04/16] Perform a regular 3d copy as a control checkpoint

[Alan Previn]

As a control checkpoint, allocate buffers to be used
as texture and render target in the 3d engine for a
regular src to dest copy blit and verify buffer pixels.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 162 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 6340daae..94655c27 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -27,6 +27,11 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 #define CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID 8
 #define CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID 9
 
+/* test-configs for protected rendering operations */
+#define COPY3D_BASELINE_SRC_TO_DST 1
+#define COPY3D_PROTECTED_SRC_TO_PROTDST 2
+#define COPY3D_PROTECTED_PROTSRC_TO_PROTDST 3
+
 static bool is_pxp_hw_supported(int i915)
 {
 	uint32_t devid = intel_get_drm_devid(i915);
@@ -165,6 +170,7 @@ static uint32_t create_protected_ctx(int i915, bool with_protected_param, bool p
 
 #define CHANGE_PARAM_PROTECTED 0x0001
 #define CHANGE_PARAM_RECOVERY 0x0002
+
 static void modify_ctx_protected_param(int i915, uint32_t ctx_id, uint32_t change_param_mask,
 				       bool param_value, int expected_return)
 {
@@ -302,10 +308,153 @@ static void test_create_protected_context(int i915, uint32_t test_cfg)
 	}
 }
 
+static void fill_bo_content(int i915, uint32_t bo, uint32_t size, uint32_t initcolor)
+{
+	uint32_t *ptr, *ptrtmp;
+	int loop = 0;
+
+	ptr = gem_mmap__device_coherent(i915, bo, 0, size, PROT_WRITE);
+	ptrtmp = ptr;
+
+	/* read and count all dword matches till size */
+	while (loop++ < (size/4)) {
+		*ptrtmp = initcolor;
+		++ptrtmp;
+	}
+
+	igt_assert(gem_munmap(ptr, size) == 0);
+}
+
+#define COMPARE_COLOR_READIBLE     1
+#define COMPARE_COLOR_UNREADIBLE   2
+#define COMPARE_N_PIXELS_VERBOSELY 0
+
+static void assert_bo_content_check(int i915, uint32_t bo, int compare_op,
+				    uint32_t size, uint32_t color)
+{
+	uint32_t *ptr, *ptrtmp;
+	int loop = 0, num_matches = 0;
+	uint32_t value;
+	bool op_readible = (compare_op == COMPARE_COLOR_READIBLE);
+
+	ptr = gem_mmap__device_coherent(i915, bo, 0, size, PROT_READ);
+	ptrtmp = ptr;
+
+	if (COMPARE_N_PIXELS_VERBOSELY) {
+		igt_info("--------->>>\n");
+		while (loop < COMPARE_N_PIXELS_VERBOSELY && loop < (size/4)) {
+			value = *ptrtmp;
+			igt_info("Color read = 0x%08x ", value);
+			igt_info("expected %c= 0x%08x)\n", op_readible?'=':'!', color);
+			++ptrtmp;
+			++loop;
+		}
+		igt_info("<<<---------\n");
+		ptrtmp = ptr;
+		loop = 0;
+	}
+
+	/* count all pixels for matches */
+	while (loop++ < (size/4)) {
+		value = *ptrtmp;
+		if (value == color)
+			++num_matches;
+		++ptrtmp;
+	}
+
+	if (op_readible)
+		igt_assert_eq(num_matches, (size/4));
+	else
+		igt_assert_eq(num_matches, 0);
+
+	igt_assert(gem_munmap(ptr, size) == 0);
+}
+
+static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size,
+					 uint32_t init_color)
+{
+	uint32_t bo;
+
+	bo = create_protected_bo(i915, size, protected, 0);
+	igt_assert(bo);
+	fill_bo_content(i915, bo, size, init_color);
+	assert_bo_content_check(i915, bo, COMPARE_COLOR_READIBLE, size, init_color);
+
+	return bo;
+}
+
+/*
+ * Rendering tests surface attributes, keep it simple:
+ * page aligned width==stride, thus, and size
+ */
+#define TSTSURF_WIDTH       1024
+#define TSTSURF_HEIGHT      128
+#define TSTSURF_BYTESPP     4
+#define TSTSURF_STRIDE      (TSTSURF_WIDTH*TSTSURF_BYTESPP)
+#define TSTSURF_SIZE        (TSTSURF_STRIDE*TSTSURF_HEIGHT)
+#define TSTSURF_FILLCOLOR1  0xfaceface
+#define TSTSURF_INITCOLOR1  0x12341234
+
+static void test_render_protected_buffer(int i915, uint32_t test_cfg)
+{
+	uint32_t ctx, srcbo, dstbo;
+	struct intel_buf *srcbuf, *dstbuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
+	uint32_t devid;
+
+	devid = intel_get_drm_devid(i915);
+	igt_assert(devid);
+
+	bops = buf_ops_create(i915);
+	igt_assert(bops);
+
+	switch (test_cfg) {
+	case COPY3D_BASELINE_SRC_TO_DST:
+		/* Perform a regular 3d copy as a control checkpoint */
+		ctx = create_protected_ctx(i915, false, false, false, false, 0);
+		assert_ctx_protected_param(i915, ctx, false);
+
+		dstbo = alloc_and_fill_dest_buff(i915, false, TSTSURF_SIZE, TSTSURF_INITCOLOR1);
+		dstbuf = intel_buf_create_using_handle(bops, dstbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+						       TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+
+		srcbo = alloc_and_fill_dest_buff(i915, false, TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+		srcbuf = intel_buf_create_using_handle(bops, srcbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+						       TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+
+		ibb = intel_bb_create_with_context(i915, ctx, 4096);
+		igt_assert(ibb);
+
+		gen12_render_copyfunc(ibb, srcbuf, 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT, dstbuf, 0, 0);
+		gem_sync(i915, dstbo);
+
+		assert_bo_content_check(i915, dstbo, COMPARE_COLOR_READIBLE,
+					TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+
+		intel_bb_destroy(ibb);
+		intel_buf_destroy(srcbuf);
+		gem_close(i915, srcbo);
+		intel_buf_destroy(dstbuf);
+		gem_close(i915, dstbo);
+		gem_context_destroy(i915, ctx);
+		break;
+
+	default:
+		igt_info("Skipping unknown render test_cfg = %d\n", test_cfg);
+		break;
+	}
+
+	buf_ops_destroy(bops);
+}
+
+
 igt_main
 {
 	int i915 = -1;
 	bool pxp_supported = false;
+	igt_render_copyfunc_t rendercopy = NULL;
+	uint32_t devid = 0;
 
 	igt_fixture
 	{
@@ -359,6 +508,19 @@ igt_main
 		igt_subtest("reject-modify-context-protection-off-3")
 			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID);
 	}
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(pxp_supported);
+			devid = intel_get_drm_devid(i915);
+			igt_assert(devid);
+			rendercopy = igt_get_render_copyfunc(devid);
+			igt_require(rendercopy);
+		}
+
+		igt_describe("Verify protected render operations:");
+		igt_subtest("regular-baseline-src-copy-readible")
+			test_render_protected_buffer(i915, COPY3D_BASELINE_SRC_TO_DST);
+	}
 
 	igt_fixture {
 		close(i915);
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 05/16] Add PXP attribute support in batchbuffer and buffer_ops libs

[Alan Previn]

Eventually when we get to testing PXP rendering capability,
we shall reuse lib's rendercopy feature. Rendercopy libraries
shall retrieve information about PXP-session-enablement and
which buffers are protected from these new flags.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/intel_batchbuffer.c | 21 +++++++++++++++++++++
 lib/intel_batchbuffer.h | 28 ++++++++++++++++++++++++++++
 lib/intel_bufops.h      | 15 +++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 0b2c5b21..23957109 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -2357,6 +2357,27 @@ uint64_t intel_bb_offset_reloc_to_object(struct intel_bb *ibb,
 				  delta, offset, presumed_offset);
 }
 
+/*
+ * @intel_bb_set_pxp:
+ * @ibb: pointer to intel_bb
+ * @new_state: enable or disable pxp session
+ * @apptype: pxp session input identifies what type of session to enable
+ * @appid: pxp session input provides which appid to use
+ *
+ * This function merely stores the pxp state and session information to
+ * be retrieved and programmed later by supporting libraries such as
+ * gen12_render_copy that must program the HW within the same dispatch
+ */
+void intel_bb_set_pxp(struct intel_bb *ibb, bool new_state,
+		      uint32_t apptype, uint32_t appid)
+{
+	igt_assert(ibb);
+
+	ibb->pxp.enabled = new_state;
+	ibb->pxp.apptype = new_state ? apptype : 0;
+	ibb->pxp.appid   = new_state ? appid : 0;
+}
+
 static void intel_bb_dump_execbuf(struct intel_bb *ibb,
 				  struct drm_i915_gem_execbuffer2 *execbuf)
 {
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 6f148713..389da7b2 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -438,6 +438,11 @@ typedef void (*igt_media_spinfunc_t)(int i915,
 
 igt_media_spinfunc_t igt_get_media_spinfunc(int devid);
 
+struct igt_pxp {
+	bool     enabled;
+	uint32_t apptype;
+	uint32_t appid;
+};
 
 /*
  * Batchbuffer without libdrm dependency
@@ -464,6 +469,7 @@ struct intel_bb {
 	bool supports_48b_address;
 	bool uses_full_ppgtt;
 
+	struct igt_pxp pxp;
 	uint32_t ctx;
 	uint32_t vm_id;
 
@@ -578,6 +584,27 @@ static inline void intel_bb_out(struct intel_bb *ibb, uint32_t dword)
 	igt_assert(intel_bb_offset(ibb) <= ibb->size);
 }
 
+void intel_bb_set_pxp(struct intel_bb *ibb, bool new_state,
+		      uint32_t apptype, uint32_t appid);
+
+static inline bool intel_bb_pxp_enabled(struct intel_bb *ibb)
+{
+	igt_assert(ibb);
+	return ibb->pxp.enabled;
+}
+
+static inline uint32_t intel_bb_pxp_apptype(struct intel_bb *ibb)
+{
+	igt_assert(ibb);
+	return ibb->pxp.apptype;
+}
+
+static inline uint32_t intel_bb_pxp_appid(struct intel_bb *ibb)
+{
+	igt_assert(ibb);
+	return ibb->pxp.appid;
+}
+
 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, bool write);
@@ -691,3 +718,4 @@ typedef void (*igt_huc_copyfunc_t)(int fd,
 
 igt_huc_copyfunc_t	igt_get_huc_copyfunc(int devid);
 #endif
+
diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
index 9e57d53e..bffeb1b0 100644
--- a/lib/intel_bufops.h
+++ b/lib/intel_bufops.h
@@ -50,6 +50,9 @@ struct intel_buf {
 	uint32_t *ptr;
 	bool cpu_write;
 
+	/* Content Protection*/
+	bool is_protected;
+
 	/* For debugging purposes */
 	char name[INTEL_BUF_NAME_MAXSIZE + 1];
 };
@@ -155,6 +158,18 @@ struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
 							 int stride);
 void intel_buf_destroy(struct intel_buf *buf);
 
+static inline void intel_buf_set_pxp(struct intel_buf *buf, bool new_pxp_state)
+{
+	igt_assert(buf);
+	buf->is_protected = new_pxp_state;
+}
+
+static inline bool intel_buf_pxp(const struct intel_buf *buf)
+{
+	igt_assert(buf);
+	return buf->is_protected;
+}
+
 void *intel_buf_cpu_map(struct intel_buf *buf, bool write);
 void *intel_buf_device_map(struct intel_buf *buf, bool write);
 void intel_buf_unmap(struct intel_buf *buf);
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 06/16] Add MI_SET_APPID instruction definition

[Alan Previn]

Add MI_SET_APPID instruction and param definitions

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/intel_reg.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index ac1fc6cb..46b5da20 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -2546,6 +2546,15 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define CTXT_PALETTE_SAVE_DISABLE	(1<<3)
 #define CTXT_PALETTE_RESTORE_DISABLE	(1<<2)
 
+#define MI_SET_APPID                    (0x0E << 23)
+#define APPID_CTXSAVE_INHIBIT           (1 << 8)
+#define APPID_CTXREST_INHIBIT           (1 << 9)
+#define DISPLAY_APPTYPE                 (0)
+#define TRANSCODE_APPTYPE               (1)
+#define APPTYPE(n)                      (n << 7)
+#define APPID(n)                        (n & 0x7f)
+
+
 /* Dword 0 */
 #define MI_VERTEX_BUFFER		(0x17<<23)
 #define MI_VERTEX_BUFFER_IDX(x)		(x<<20)
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 07/16] Enable protected session cmd in gen12_render_copyfunc

[Alan Previn]

1. In _gen9_render_op, check if the incoming batchbuffer
   is marked with pxp enabled. If so, insert MI_SET_APPID
   along with PIPE_CONTROL instructions at the start and
   end of the rendering operation in the command buffer.

2. The two PIPE_CONTROLs will enable protected memory
   at the start of the batch and disabling protected
   memory at the end of it. These PIPE_CONTROLs require a
   Post-Sync operation with a write to memory for hardware
   to accept.

3. In order to satisfy #2, _gen9_render_op uses unused
   regions of the ibb buffer for the PIPE_CONTROL PostSync
   write to memory (no different from how other 3d states
   are being referenced).

4. _gen9_render_op shall check the incoming surface
   buffers for "is_protected" flag and if its set, it
   will mark the SURFACE_STATE's MOCS field accordingly.

NOTE: _gen9_render_op needs to program the HW to enable
the PXP session as part of the rendering batch buffer
because the HW requires that enabling/disabling protected
memory access must be programmed in pairs within the same
"dispatch of rendering commands" to HW.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/rendercopy_gen9.c | 72 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
index eecf73d3..bfcf311b 100644
--- a/lib/rendercopy_gen9.c
+++ b/lib/rendercopy_gen9.c
@@ -19,7 +19,8 @@
 #include "intel_bufops.h"
 #include "intel_batchbuffer.h"
 #include "intel_io.h"
-#include "rendercopy.h"
+#include "igt.h"
+#include "i915/gem.h"
 #include "gen9_render.h"
 #include "intel_reg.h"
 #include "igt_aux.h"
@@ -152,6 +153,8 @@ gen8_bind_buf(struct intel_bb *ibb, const struct intel_buf *buf, int is_dst) {
 		ss->ss0.tiled_mode = 3;
 
 	ss->ss1.memory_object_control = I915_MOCS_PTE << 1;
+	if (intel_buf_pxp(buf))
+		ss->ss1.memory_object_control |= 1;
 
 	if (buf->tiling == I915_TILING_Yf)
 		ss->ss5.trmode = 1;
@@ -873,6 +876,53 @@ static void gen8_emit_primitive(struct intel_bb *ibb, uint32_t offset)
 	intel_bb_out(ibb, 0);	/* index buffer offset, ignored */
 }
 
+#define GFX_OP_PIPE_CONTROL    ((3 << 29) | (3 << 27) | (2 << 24))
+#define PIPE_CONTROL_CS_STALL	            (1 << 20)
+#define PIPE_CONTROL_RENDER_TARGET_FLUSH    (1 << 12)
+#define PIPE_CONTROL_FLUSH_ENABLE           (1 << 7)
+#define PIPE_CONTROL_DATA_CACHE_INVALIDATE  (1 << 5)
+#define PIPE_CONTROL_PROTECTEDPATH_DISABLE  (1 << 27)
+#define PIPE_CONTROL_PROTECTEDPATH_ENABLE   (1 << 22)
+#define PIPE_CONTROL_POST_SYNC_OP           (1 << 14)
+#define PIPE_CONTROL_POST_SYNC_OP_STORE_DW_IDX (1 << 21)
+#define PS_OP_TAG_START                     0x1234fed0
+#define PS_OP_TAG_END                       0x5678cbaf
+static void gen12_emit_pxp_state(struct intel_bb *ibb, bool enable,
+		 uint32_t pxp_write_op_offset)
+{
+	uint32_t pipe_ctl_flags;
+	uint32_t set_app_id, ps_op_id;
+
+	if (enable) {
+		pipe_ctl_flags = PIPE_CONTROL_FLUSH_ENABLE;
+		intel_bb_out(ibb, GFX_OP_PIPE_CONTROL);
+		intel_bb_out(ibb, pipe_ctl_flags);
+
+		set_app_id =  MI_SET_APPID |
+			      APPTYPE(intel_bb_pxp_apptype(ibb)) |
+			      APPID(intel_bb_pxp_appid(ibb));
+		intel_bb_out(ibb, set_app_id);
+
+		pipe_ctl_flags = PIPE_CONTROL_PROTECTEDPATH_ENABLE;
+		ps_op_id = PS_OP_TAG_START;
+	} else {
+		pipe_ctl_flags = PIPE_CONTROL_PROTECTEDPATH_DISABLE;
+		ps_op_id = PS_OP_TAG_END;
+	}
+
+	pipe_ctl_flags |= (PIPE_CONTROL_CS_STALL |
+			   PIPE_CONTROL_RENDER_TARGET_FLUSH |
+			   PIPE_CONTROL_DATA_CACHE_INVALIDATE |
+			   PIPE_CONTROL_POST_SYNC_OP);
+	intel_bb_out(ibb, GFX_OP_PIPE_CONTROL | 4);
+	intel_bb_out(ibb, pipe_ctl_flags);
+	intel_bb_emit_reloc(ibb, ibb->handle, 0, I915_GEM_DOMAIN_COMMAND,
+			    (enable ? pxp_write_op_offset : (pxp_write_op_offset+8)),
+			    ibb->batch_offset);
+	intel_bb_out(ibb, ps_op_id);
+	intel_bb_out(ibb, ps_op_id);
+}
+
 /* The general rule is if it's named gen6 it is directly copied from
  * gen6_render_copyfunc.
  *
@@ -922,6 +972,7 @@ void _gen9_render_op(struct intel_bb *ibb,
 	uint32_t vertex_buffer;
 	uint32_t aux_pgtable_state;
 	bool fast_clear = !src;
+	uint32_t pxp_scratch_offset;
 
 	if (!fast_clear)
 		igt_assert(src->bpp == dst->bpp);
@@ -950,8 +1001,12 @@ void _gen9_render_op(struct intel_bb *ibb,
 	aux_pgtable_state = gen12_create_aux_pgtable_state(ibb, aux_pgtable_buf);
 
 	/* TODO: there is other state which isn't setup */
+	pxp_scratch_offset = intel_bb_offset(ibb);
 	intel_bb_ptr_set(ibb, 0);
 
+	if (intel_bb_pxp_enabled(ibb))
+		gen12_emit_pxp_state(ibb, true, pxp_scratch_offset);
+
 	/* Start emitting the commands. The order roughly follows the mesa blorp
 	 * order */
 	intel_bb_out(ibb, G4X_PIPELINE_SELECT | PIPELINE_SELECT_3D |
@@ -963,13 +1018,12 @@ void _gen9_render_op(struct intel_bb *ibb,
 		for (int i = 0; i < 4; i++) {
 			intel_bb_out(ibb, MI_STORE_DWORD_IMM);
 			intel_bb_emit_reloc(ibb, dst->handle,
-					    I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
-                                            dst->cc.offset + i*sizeof(float),
-					    dst->addr.offset);
+					I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
+					dst->cc.offset + i*sizeof(float),
+					dst->addr.offset);
 			intel_bb_out(ibb, *(uint32_t*)&clear_color[i]);
-               }
-       }
-
+		}
+	}
 
 	gen8_emit_sip(ibb);
 
@@ -1023,10 +1077,14 @@ void _gen9_render_op(struct intel_bb *ibb,
 	gen8_emit_vf_topology(ibb);
 	gen8_emit_primitive(ibb, vertex_buffer);
 
+	if (intel_bb_pxp_enabled(ibb))
+		gen12_emit_pxp_state(ibb, false, pxp_scratch_offset);
+
 	intel_bb_emit_bbe(ibb);
 	intel_bb_exec(ibb, intel_bb_offset(ibb),
 		      I915_EXEC_RENDER | I915_EXEC_NO_RELOC, false);
 	dump_batch(ibb);
+
 	intel_bb_reset(ibb, false);
 }
 
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 08/16] Add subtest to copy raw source to protected dest

[Alan Previn]

Add subtest to 3d-copy raw source buffer (with
known readible content) to a destination buffer
marked as protected with a protected session using
default session keys. The destination buffer is
verified to be different from the source (when
read via CPU) because its encrypted.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 94655c27..cd839527 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -393,7 +393,9 @@ static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size
 #define TSTSURF_STRIDE      (TSTSURF_WIDTH*TSTSURF_BYTESPP)
 #define TSTSURF_SIZE        (TSTSURF_STRIDE*TSTSURF_HEIGHT)
 #define TSTSURF_FILLCOLOR1  0xfaceface
+#define TSTSURF_FILLCOLOR2  0xdeaddead
 #define TSTSURF_INITCOLOR1  0x12341234
+#define TSTSURF_INITCOLOR2  0x56785678
 
 static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 {
@@ -440,6 +442,42 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		gem_context_destroy(i915, ctx);
 		break;
 
+	case COPY3D_PROTECTED_SRC_TO_PROTDST:
+		/* Perform a protected render operation but only label
+		 * the dest as protected. After rendering, the content
+		 * should be encrypted
+		 */
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+
+		dstbo = alloc_and_fill_dest_buff(i915, true, TSTSURF_SIZE, TSTSURF_INITCOLOR2);
+		dstbuf = intel_buf_create_using_handle(bops, dstbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+							TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+		intel_buf_set_pxp(dstbuf, true);
+
+		srcbo = alloc_and_fill_dest_buff(i915, false, TSTSURF_SIZE, TSTSURF_FILLCOLOR2);
+		srcbuf = intel_buf_create_using_handle(bops, srcbo, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+							TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+
+		ibb = intel_bb_create_with_context(i915, ctx, 4096);
+		igt_assert(ibb);
+
+		intel_bb_set_pxp(ibb, true, DISPLAY_APPTYPE, I915_PROTECTED_CONTENT_DEFAULT_SESSION);
+
+		gen12_render_copyfunc(ibb, srcbuf, 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT, dstbuf, 0, 0);
+		gem_sync(i915, dstbo);
+
+		assert_bo_content_check(i915, dstbo, COMPARE_COLOR_UNREADIBLE,
+					TSTSURF_SIZE, TSTSURF_FILLCOLOR2);
+
+		intel_bb_destroy(ibb);
+		intel_buf_destroy(srcbuf);
+		gem_close(i915, srcbo);
+		intel_buf_destroy(dstbuf);
+		gem_close(i915, dstbo);
+		gem_context_destroy(i915, ctx);
+		break;
+
 	default:
 		igt_info("Skipping unknown render test_cfg = %d\n", test_cfg);
 		break;
@@ -520,6 +558,8 @@ igt_main
 		igt_describe("Verify protected render operations:");
 		igt_subtest("regular-baseline-src-copy-readible")
 			test_render_protected_buffer(i915, COPY3D_BASELINE_SRC_TO_DST);
+		igt_subtest("protected-raw-src-copy-not-readible")
+			test_render_protected_buffer(i915, COPY3D_PROTECTED_SRC_TO_PROTDST);
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 09/16] Add test where both src and dest are protected

[Alan Previn]

When both the source and destination surfaces are
protected, the destination pixel result of the 3d
copy operation would be the same as the source. By
appending this test case to the end of the prior
test (raw-src to protected-dest) and reusing the
previous' test destination as the current source,
we can prove that new-source was decrypted properly
as we would see the difference in results: repeating
the same render operation but with a src buffer
that is protected in this case yields matching
(but still encrypted) output rendered pixels.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 110 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 97 insertions(+), 13 deletions(-)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index cd839527..22a2d09a 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -327,29 +327,51 @@ static void fill_bo_content(int i915, uint32_t bo, uint32_t size, uint32_t initc
 
 #define COMPARE_COLOR_READIBLE     1
 #define COMPARE_COLOR_UNREADIBLE   2
+#define COMPARE_BUFFER_READIBLE    3
+#define COMPARE_BUFFER_UNREADIBLE  4
+#define COPY_BUFFER                5
 #define COMPARE_N_PIXELS_VERBOSELY 0
 
-static void assert_bo_content_check(int i915, uint32_t bo, int compare_op,
-				    uint32_t size, uint32_t color)
+static void assert_bo_content_check(int i915, uint32_t bo, int compare_op, uint32_t size,
+				    uint32_t color, uint32_t *auxptr, int auxsize)
 {
-	uint32_t *ptr, *ptrtmp;
+	uint32_t *ptr, *ptrtmp, *auxtmp;
 	int loop = 0, num_matches = 0;
 	uint32_t value;
-	bool op_readible = (compare_op == COMPARE_COLOR_READIBLE);
+	bool op_readible = ((compare_op == COMPARE_COLOR_READIBLE) ||
+		 (compare_op == COMPARE_BUFFER_READIBLE));
+	bool chk_buff = ((compare_op == COMPARE_BUFFER_READIBLE) ||
+		 (compare_op == COMPARE_BUFFER_UNREADIBLE));
+	bool copy_buff = (compare_op == COPY_BUFFER);
 
 	ptr = gem_mmap__device_coherent(i915, bo, 0, size, PROT_READ);
 	ptrtmp = ptr;
 
+	if (chk_buff || copy_buff) {
+		if (auxsize < size)
+			auxptr = NULL;
+		igt_assert(auxptr);
+		auxtmp = auxptr;
+	}
+
 	if (COMPARE_N_PIXELS_VERBOSELY) {
 		igt_info("--------->>>\n");
 		while (loop < COMPARE_N_PIXELS_VERBOSELY && loop < (size/4)) {
 			value = *ptrtmp;
-			igt_info("Color read = 0x%08x ", value);
-			igt_info("expected %c= 0x%08x)\n", op_readible?'=':'!', color);
+			if (chk_buff)
+				color = *auxtmp;
+			if (copy_buff)
+				igt_info("Color copy = 0x%08x\n", value);
+			else {
+				igt_info("Color read = 0x%08x ", value);
+				igt_info("expected %c= 0x%08x)\n", op_readible?'=':'!', color);
+			}
+			++auxtmp;
 			++ptrtmp;
 			++loop;
 		}
 		igt_info("<<<---------\n");
+		auxtmp = auxptr;
 		ptrtmp = ptr;
 		loop = 0;
 	}
@@ -357,8 +379,25 @@ static void assert_bo_content_check(int i915, uint32_t bo, int compare_op,
 	/* count all pixels for matches */
 	while (loop++ < (size/4)) {
 		value = *ptrtmp;
-		if (value == color)
-			++num_matches;
+		switch (compare_op) {
+		case COMPARE_COLOR_READIBLE:
+		case COMPARE_COLOR_UNREADIBLE:
+			if (value == color)
+				++num_matches;
+			break;
+		case COMPARE_BUFFER_READIBLE:
+		case COMPARE_BUFFER_UNREADIBLE:
+			if (value == (*auxtmp))
+				++num_matches;
+			++auxtmp;
+			break;
+		case COPY_BUFFER:
+			*auxtmp = value;
+			++auxtmp;
+			break;
+		default:
+			break;
+		}
 		++ptrtmp;
 	}
 
@@ -378,7 +417,8 @@ static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size
 	bo = create_protected_bo(i915, size, protected, 0);
 	igt_assert(bo);
 	fill_bo_content(i915, bo, size, init_color);
-	assert_bo_content_check(i915, bo, COMPARE_COLOR_READIBLE, size, init_color);
+	assert_bo_content_check(i915, bo, COMPARE_COLOR_READIBLE,
+				size, init_color, NULL, 0);
 
 	return bo;
 }
@@ -396,14 +436,16 @@ static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size
 #define TSTSURF_FILLCOLOR2  0xdeaddead
 #define TSTSURF_INITCOLOR1  0x12341234
 #define TSTSURF_INITCOLOR2  0x56785678
+#define TSTSURF_INITCOLOR3  0xabcdabcd
 
 static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 {
-	uint32_t ctx, srcbo, dstbo;
-	struct intel_buf *srcbuf, *dstbuf;
+	uint32_t ctx, srcbo, dstbo, dstbo2;
+	struct intel_buf *srcbuf, *dstbuf, *dstbuf2;
 	struct buf_ops *bops;
 	struct intel_bb *ibb;
 	uint32_t devid;
+	uint32_t encrypted[TSTSURF_SIZE/TSTSURF_BYTESPP];
 
 	devid = intel_get_drm_devid(i915);
 	igt_assert(devid);
@@ -432,7 +474,7 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		gem_sync(i915, dstbo);
 
 		assert_bo_content_check(i915, dstbo, COMPARE_COLOR_READIBLE,
-					TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+					TSTSURF_SIZE, TSTSURF_FILLCOLOR1, NULL, 0);
 
 		intel_bb_destroy(ibb);
 		intel_buf_destroy(srcbuf);
@@ -443,6 +485,7 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		break;
 
 	case COPY3D_PROTECTED_SRC_TO_PROTDST:
+	case COPY3D_PROTECTED_PROTSRC_TO_PROTDST:
 		/* Perform a protected render operation but only label
 		 * the dest as protected. After rendering, the content
 		 * should be encrypted
@@ -468,13 +511,52 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		gem_sync(i915, dstbo);
 
 		assert_bo_content_check(i915, dstbo, COMPARE_COLOR_UNREADIBLE,
-					TSTSURF_SIZE, TSTSURF_FILLCOLOR2);
+					TSTSURF_SIZE, TSTSURF_FILLCOLOR2, NULL, 0);
+
+		if (test_cfg == COPY3D_PROTECTED_SRC_TO_PROTDST) {
+			intel_bb_destroy(ibb);
+			intel_buf_destroy(srcbuf);
+			gem_close(i915, srcbo);
+			intel_buf_destroy(dstbuf);
+			gem_close(i915, dstbo);
+			gem_context_destroy(i915, ctx);
+			break;
+		}
+
+		/* For COPY3D_PROTECTED_PROTSRC_TO_PROTDST, reuse
+		 * prior dst as the new-src and create a new dst2 as
+		 * the new-dest. Take a copy of encrypted content
+		 * from new-src for comparison after render operation.
+		 * 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 render and re-encrypt when
+		 * going into new-dest
+		 */
+		assert_bo_content_check(i915, dstbo, COPY_BUFFER,
+			TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
+
+		dstbo2 = alloc_and_fill_dest_buff(i915, false, TSTSURF_SIZE, TSTSURF_INITCOLOR3);
+		dstbuf2 = intel_buf_create_using_handle(bops, dstbo2, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+							TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+		intel_buf_set_pxp(dstbuf2, true);
+
+		intel_buf_set_pxp(dstbuf, true);/*this time, src is protected*/
+
+		intel_bb_set_pxp(ibb, true, DISPLAY_APPTYPE, I915_PROTECTED_CONTENT_DEFAULT_SESSION);
+
+		gen12_render_copyfunc(ibb, dstbuf, 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT, dstbuf2, 0, 0);
+		gem_sync(i915, dstbo2);
+
+		assert_bo_content_check(i915, dstbo2, COMPARE_BUFFER_READIBLE, TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
 
 		intel_bb_destroy(ibb);
 		intel_buf_destroy(srcbuf);
 		gem_close(i915, srcbo);
 		intel_buf_destroy(dstbuf);
 		gem_close(i915, dstbo);
+		intel_buf_destroy(dstbuf2);
+		gem_close(i915, dstbo2);
 		gem_context_destroy(i915, ctx);
 		break;
 
@@ -560,6 +642,8 @@ igt_main
 			test_render_protected_buffer(i915, COPY3D_BASELINE_SRC_TO_DST);
 		igt_subtest("protected-raw-src-copy-not-readible")
 			test_render_protected_buffer(i915, COPY3D_PROTECTED_SRC_TO_PROTDST);
+		igt_subtest("protected-encrypted-src-copy-not-readible")
+			test_render_protected_buffer(i915, COPY3D_PROTECTED_PROTSRC_TO_PROTDST);
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 10/16] Verify PXP teardown occured through suspend-resume

[Alan Previn]

During a suspend-resume cycle, the hardware and driver shall
ensure the PXP session and keys are torn down and re-
established. Verify that key change did occur by repeating
the 3d rendercopy operation before and after a suspend cycle
and ensuring that the encrypted output is different.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 96 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 4 deletions(-)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 22a2d09a..37c38016 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -6,6 +6,8 @@
 #include <sys/ioctl.h>
 
 #include "igt.h"
+#include "igt_device.h"
+#include "igt_sysfs.h"
 #include "i915/gem.h"
 
 IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
@@ -31,6 +33,16 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 #define COPY3D_BASELINE_SRC_TO_DST 1
 #define COPY3D_PROTECTED_SRC_TO_PROTDST 2
 #define COPY3D_PROTECTED_PROTSRC_TO_PROTDST 3
+#define COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED 4
+
+/* test-configs for power-management triggered protected session teardown */
+#define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
+
+/* Struct and defintions for power management. */
+struct powermgt_data {
+	int debugfsdir;
+	bool has_runtime_pm;
+};
 
 static bool is_pxp_hw_supported(int i915)
 {
@@ -438,7 +450,7 @@ static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size
 #define TSTSURF_INITCOLOR2  0x56785678
 #define TSTSURF_INITCOLOR3  0xabcdabcd
 
-static void test_render_protected_buffer(int i915, uint32_t test_cfg)
+static void __test_render_protected_buffer(int i915, uint32_t test_cfg, uint32_t *outpixels, int outsize)
 {
 	uint32_t ctx, srcbo, dstbo, dstbo2;
 	struct intel_buf *srcbuf, *dstbuf, *dstbuf2;
@@ -486,6 +498,7 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 
 	case COPY3D_PROTECTED_SRC_TO_PROTDST:
 	case COPY3D_PROTECTED_PROTSRC_TO_PROTDST:
+	case COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED:
 		/* Perform a protected render operation but only label
 		 * the dest as protected. After rendering, the content
 		 * should be encrypted
@@ -513,7 +526,12 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		assert_bo_content_check(i915, dstbo, COMPARE_COLOR_UNREADIBLE,
 					TSTSURF_SIZE, TSTSURF_FILLCOLOR2, NULL, 0);
 
-		if (test_cfg == COPY3D_PROTECTED_SRC_TO_PROTDST) {
+		if (test_cfg == COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED)
+			assert_bo_content_check(i915, dstbo, COPY_BUFFER,
+						TSTSURF_SIZE, 0, outpixels, outsize);
+
+		if ((test_cfg == COPY3D_PROTECTED_SRC_TO_PROTDST) ||
+			(test_cfg == COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED)) {
 			intel_bb_destroy(ibb);
 			intel_buf_destroy(srcbuf);
 			gem_close(i915, srcbo);
@@ -534,7 +552,7 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		 * going into new-dest
 		 */
 		assert_bo_content_check(i915, dstbo, COPY_BUFFER,
-			TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
+					TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
 
 		dstbo2 = alloc_and_fill_dest_buff(i915, false, TSTSURF_SIZE, TSTSURF_INITCOLOR3);
 		dstbuf2 = intel_buf_create_using_handle(bops, dstbo2, TSTSURF_WIDTH, TSTSURF_HEIGHT,
@@ -548,7 +566,8 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 		gen12_render_copyfunc(ibb, dstbuf, 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT, dstbuf2, 0, 0);
 		gem_sync(i915, dstbo2);
 
-		assert_bo_content_check(i915, dstbo2, COMPARE_BUFFER_READIBLE, TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
+		assert_bo_content_check(i915, dstbo2, COMPARE_BUFFER_READIBLE,
+					TSTSURF_SIZE, 0, encrypted, TSTSURF_SIZE);
 
 		intel_bb_destroy(ibb);
 		intel_buf_destroy(srcbuf);
@@ -568,11 +587,66 @@ static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 	buf_ops_destroy(bops);
 }
 
+static void test_render_protected_buffer(int i915, uint32_t test_cfg)
+{
+	__test_render_protected_buffer(i915, test_cfg, NULL, 0);
+}
+
+static void init_powermgt_resources(int i915, struct powermgt_data *pm)
+{
+	pm->debugfsdir = igt_debugfs_dir(i915);
+	igt_require(pm->debugfsdir != -1);
+	pm->has_runtime_pm = igt_setup_runtime_pm(i915);
+	igt_require(pm->has_runtime_pm);
+
+}
+
+static void trigger_powermgt_suspend_cycle(int i915,
+	struct powermgt_data *pm)
+{
+	igt_pm_enable_sata_link_power_management();
+
+
+	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_DEVICES);
+	igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED);
+}
+
+static void test_protected_session_teardown(int i915, uint32_t test_cfg,
+		struct powermgt_data *pm)
+{
+	uint32_t encrypted_pixels_b4[TSTSURF_SIZE/TSTSURF_BYTESPP];
+	uint32_t encrypted_pixels_aft[TSTSURF_SIZE/TSTSURF_BYTESPP];
+	int matched_after_keychange = 0, loop = 0;
+
+	switch (test_cfg) {
+	case SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE:
+		__test_render_protected_buffer(i915, COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED,
+					       encrypted_pixels_b4, TSTSURF_SIZE);
+
+		trigger_powermgt_suspend_cycle(i915, pm);
+
+		__test_render_protected_buffer(i915, COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED,
+					       encrypted_pixels_aft, TSTSURF_SIZE);
+
+		while (loop < (TSTSURF_SIZE/TSTSURF_BYTESPP)) {
+			if (encrypted_pixels_b4[loop] == encrypted_pixels_aft[loop])
+				++matched_after_keychange;
+			++loop;
+		}
+		igt_assert_eq(matched_after_keychange, 0);
+		break;
+
+	default:
+		igt_info("Skipping unknown power-mgt test_cfg = %d\n", test_cfg);
+		break;
+	}
+}
 
 igt_main
 {
 	int i915 = -1;
 	bool pxp_supported = false;
+	struct powermgt_data pm = {0};
 	igt_render_copyfunc_t rendercopy = NULL;
 	uint32_t devid = 0;
 
@@ -645,6 +719,20 @@ igt_main
 		igt_subtest("protected-encrypted-src-copy-not-readible")
 			test_render_protected_buffer(i915, COPY3D_PROTECTED_PROTSRC_TO_PROTDST);
 	}
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(pxp_supported);
+			devid = intel_get_drm_devid(i915);
+			igt_assert(devid);
+			rendercopy = igt_get_render_copyfunc(devid);
+			igt_require(rendercopy);
+			init_powermgt_resources(i915, &pm);
+		}
+		igt_describe("Verify suspend-resume teardown management:");
+		igt_subtest("verify-pxp-key-change-after-suspend-resume") {
+			test_protected_session_teardown(i915, SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE, &pm);
+		}
+	}
 
 	igt_fixture {
 		close(i915);
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 11/16] Verify execbuf fails with stale PXP context after teardown

[Alan Previn]

Add a subtest to verify that reusing a stale protected context
in a gem_execbuff after a teardown (triggered by suspend-resume
cycle) shall fail with -EIO error.

NOTE: The end-to-end architecture requirement includes that
any break in the links of the PXP sessions needs to trigger a
full teardown and the application needs to be made aware of that
allowing it to re-establish the end-to-end pipeline of buffers,
contexts and renders again if it chooses to. This stricter
behavior targets only contexts created with PXP enabled.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 lib/intel_batchbuffer.c |   2 +-
 lib/intel_batchbuffer.h |   3 ++
 tests/i915/gem_pxp.c    | 110 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 23957109..e16ab056 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -2536,7 +2536,7 @@ static void update_offsets(struct intel_bb *ibb,
  * Note: In this step execobj for bb is allocated and inserted to the objects
  * array.
 */
-static int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
+int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
 			   uint64_t flags, bool sync)
 {
 	struct drm_i915_gem_execbuffer2 execbuf;
diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
index 389da7b2..b16ae00f 100644
--- a/lib/intel_batchbuffer.h
+++ b/lib/intel_batchbuffer.h
@@ -664,6 +664,9 @@ uint64_t intel_bb_offset_reloc_to_object(struct intel_bb *ibb,
 					 uint32_t offset,
 					 uint64_t presumed_offset);
 
+int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
+			uint64_t flags, bool sync);
+
 void intel_bb_dump_cache(struct intel_bb *ibb);
 
 void intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 37c38016..1952e230 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -37,6 +37,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 
 /* test-configs for power-management triggered protected session teardown */
 #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
+#define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
 
 /* Struct and defintions for power management. */
 struct powermgt_data {
@@ -611,12 +612,86 @@ static void trigger_powermgt_suspend_cycle(int i915,
 	igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED);
 }
 
+#define GFX_OP_PIPE_CONTROL    ((3 << 29) | (3 << 27) | (2 << 24))
+#define PIPE_CONTROL_CS_STALL	            (1 << 20)
+#define PIPE_CONTROL_RENDER_TARGET_FLUSH    (1 << 12)
+#define PIPE_CONTROL_FLUSH_ENABLE           (1 << 7)
+#define PIPE_CONTROL_DATA_CACHE_INVALIDATE  (1 << 5)
+#define PIPE_CONTROL_PROTECTEDPATH_DISABLE  (1 << 27)
+#define PIPE_CONTROL_PROTECTEDPATH_ENABLE   (1 << 22)
+#define PIPE_CONTROL_POST_SYNC_OP           (1 << 14)
+#define PIPE_CONTROL_POST_SYNC_OP_STORE_DW_IDX (1 << 21)
+#define PS_OP_TAG_BEFORE                    0x1234fed0
+#define PS_OP_TAG_AFTER                     0x5678cbaf
+
+static void emit_pipectrl(struct intel_bb *ibb, struct intel_buf *fenceb, bool before)
+{
+	uint32_t pipe_ctl_flags = 0;
+	uint32_t ps_op_id;
+
+	intel_bb_out(ibb, GFX_OP_PIPE_CONTROL);
+	intel_bb_out(ibb, pipe_ctl_flags);
+
+	if (before)
+		ps_op_id = PS_OP_TAG_BEFORE;
+	else
+		ps_op_id = PS_OP_TAG_AFTER;
+
+	pipe_ctl_flags = (PIPE_CONTROL_FLUSH_ENABLE |
+			  PIPE_CONTROL_CS_STALL |
+			  PIPE_CONTROL_POST_SYNC_OP);
+	intel_bb_out(ibb, GFX_OP_PIPE_CONTROL | 4);
+	intel_bb_out(ibb, pipe_ctl_flags);
+	intel_bb_emit_reloc(ibb, fenceb->handle, 0, I915_GEM_DOMAIN_COMMAND, (before?0:8), fenceb->addr.offset);
+	intel_bb_out(ibb, ps_op_id);
+	intel_bb_out(ibb, ps_op_id);
+	intel_bb_out(ibb, MI_NOOP);
+	intel_bb_out(ibb, MI_NOOP);
+}
+
+static void assert_pipectl_storedw_done(int i915, uint32_t bo)
+{
+	uint32_t *ptr;
+	uint32_t success_mask = 0x0;
+
+	ptr = gem_mmap__device_coherent(i915, bo, 0, 4096, PROT_READ);
+
+	if (ptr[0] == PS_OP_TAG_BEFORE && ptr[1] == PS_OP_TAG_BEFORE)
+		success_mask |= 0x1;
+
+	igt_assert_eq(success_mask, 0x1);
+	igt_assert(gem_munmap(ptr, 4096) == 0);
+}
+
+static void gem_execbuf_flush_store_dw(int i915, struct intel_bb *ibb,
+	uint32_t ctx, struct intel_buf *fence, int expected_gemexec_response)
+{
+	int execret;
+
+	intel_bb_ptr_set(ibb, 0);
+	intel_bb_add_intel_buf(ibb, fence, true);
+	emit_pipectrl(ibb, fence, true);
+	intel_bb_emit_bbe(ibb);
+	execret = __intel_bb_exec(ibb, intel_bb_offset(ibb),
+				  I915_EXEC_RENDER | I915_EXEC_NO_RELOC, false);
+
+	igt_assert_eq(execret, expected_gemexec_response);
+	if (expected_gemexec_response == 0) {
+		gem_sync(ibb->i915, fence->handle);
+		assert_pipectl_storedw_done(i915, fence->handle);
+	}
+}
+
 static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		struct powermgt_data *pm)
 {
 	uint32_t encrypted_pixels_b4[TSTSURF_SIZE/TSTSURF_BYTESPP];
 	uint32_t encrypted_pixels_aft[TSTSURF_SIZE/TSTSURF_BYTESPP];
 	int matched_after_keychange = 0, loop = 0;
+	uint32_t ctx, fencebo;
+	struct intel_buf *fencebuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
 
 	switch (test_cfg) {
 	case SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE:
@@ -636,6 +711,38 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		igt_assert_eq(matched_after_keychange, 0);
 		break;
 
+	case SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+
+		/* use normal buffers for testing for invalidation
+		 * of protected contexts to ensure kernel is catching
+		 * the invalidated context (not buffer)
+		 */
+		fencebo = alloc_and_fill_dest_buff(i915, false, 4096, 0);
+
+		ibb = intel_bb_create_with_context(i915, ctx, 4096);
+		igt_assert(ibb);
+
+		bops = buf_ops_create(i915);
+		igt_assert(bops);
+
+		fencebuf = intel_buf_create_using_handle(bops, fencebo, 256, 4,
+							 32, 0, I915_TILING_NONE, 0);
+		intel_bb_add_intel_buf(ibb, fencebuf, true);
+
+		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, 0);
+		trigger_powermgt_suspend_cycle(i915, pm);
+
+		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, -EACCES);
+
+		intel_bb_destroy(ibb);
+		intel_buf_destroy(fencebuf);
+		gem_close(i915, fencebo);
+		gem_context_destroy(i915, ctx);
+		buf_ops_destroy(bops);
+		break;
+
 	default:
 		igt_info("Skipping unknown power-mgt test_cfg = %d\n", test_cfg);
 		break;
@@ -732,6 +839,9 @@ igt_main
 		igt_subtest("verify-pxp-key-change-after-suspend-resume") {
 			test_protected_session_teardown(i915, SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE, &pm);
 		}
+		igt_subtest("reject-old-prot-context-execution-after-suspend-resume") {
+			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC, &pm);
+		}
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 12/16] Verify execbuf fails with stale PXP buffer after teardown

[Alan Previn]

Add a subtest to verify that reusing a stale protected buffer
in a gem_execbuff (with a protected context) after a teardown
(triggered by suspend-resume cycle) shall fail with -EIO error.

NOTE: The end-to-end architecture requirement includes that
any break in the links of the PXP sessions needs to trigger a
full teardown and the application needs to be made aware of that
allowing it to re-establish the end-to-end pipeline of buffers,
contexts and renders again if it chooses to. This stricter
behavior targets only contexts created with PXP enabled.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 1952e230..605b6524 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -38,6 +38,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 /* test-configs for power-management triggered protected session teardown */
 #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
 #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
+#define SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC 3
 
 /* Struct and defintions for power management. */
 struct powermgt_data {
@@ -688,10 +689,10 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 	uint32_t encrypted_pixels_b4[TSTSURF_SIZE/TSTSURF_BYTESPP];
 	uint32_t encrypted_pixels_aft[TSTSURF_SIZE/TSTSURF_BYTESPP];
 	int matched_after_keychange = 0, loop = 0;
-	uint32_t ctx, fencebo;
+	uint32_t ctx, ctx2, fencebo;
 	struct intel_buf *fencebuf;
 	struct buf_ops *bops;
-	struct intel_bb *ibb;
+	struct intel_bb *ibb, *ibb2;
 
 	switch (test_cfg) {
 	case SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE:
@@ -712,14 +713,19 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		break;
 
 	case SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC:
+	case SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC:
 		ctx = create_protected_ctx(i915, true, true, true, false, 0);
 		assert_ctx_protected_param(i915, ctx, true);
 
-		/* use normal buffers for testing for invalidation
-		 * of protected contexts to ensure kernel is catching
-		 * the invalidated context (not buffer)
-		 */
-		fencebo = alloc_and_fill_dest_buff(i915, false, 4096, 0);
+		if (test_cfg == SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC) {
+			/* use normal buffers for testing for invalidation
+			 * of protected contexts to ensure kernel is catching
+			 * the invalidated context
+			 */
+			fencebo = alloc_and_fill_dest_buff(i915, false, 4096, 0);
+		} else {
+			fencebo = alloc_and_fill_dest_buff(i915, true, 4096, 0);
+		}
 
 		ibb = intel_bb_create_with_context(i915, ctx, 4096);
 		igt_assert(ibb);
@@ -734,12 +740,31 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, 0);
 		trigger_powermgt_suspend_cycle(i915, pm);
 
-		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, -EACCES);
+		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC) {
+			/* after teardown, alloc new assets for everything
+			 * except the bo to ensure the kernel is catching
+			 * the invalidated bo
+			 */
+			ctx2 = create_protected_ctx(i915, true, true, true, false, 0);
+			assert_ctx_protected_param(i915, ctx2, true);
+			ibb2 = intel_bb_create_with_context(i915, ctx2, 4096);
+			igt_assert(ibb2);
+
+			intel_bb_detach_intel_buf(ibb, fencebuf);
+			intel_bb_add_intel_buf(ibb2, fencebuf, true);
+			gem_execbuf_flush_store_dw(i915, ibb2, ctx2, fencebuf, -ENOEXEC);
+		} else {
+			gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, -EACCES);
+		}
 
 		intel_bb_destroy(ibb);
 		intel_buf_destroy(fencebuf);
 		gem_close(i915, fencebo);
 		gem_context_destroy(i915, ctx);
+		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC) {
+			intel_bb_destroy(ibb2);
+			gem_context_destroy(i915, ctx2);
+		}
 		buf_ops_destroy(bops);
 		break;
 
@@ -842,6 +867,9 @@ igt_main
 		igt_subtest("reject-old-prot-context-execution-after-suspend-resume") {
 			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC, &pm);
 		}
+		igt_subtest("reject-old-prot-buffer-execution-after-suspend-resume") {
+			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC, &pm);
+		}
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 13/16] Verify execbuf ok with stale prot-buff and regular context

[Alan Previn]

Add a subtest to verify that reusing a stale protected
buffer in a gem_execbuff call, but using a regular (not-
protcted) context will succeed despite after a teardown
(triggered by suspend-resume cycle).

This ensures that user space applications that choose
not to opt-in for strict PXP teardown awareness (by
using a regular context) wont suffer gem_execbuff
failures if a protected buffer was among the assets
used in any of its rendering operations.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index 605b6524..ef93c4fe 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -39,6 +39,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
 #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
 #define SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC 3
+#define SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK 4
 
 /* Struct and defintions for power management. */
 struct powermgt_data {
@@ -714,8 +715,15 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 
 	case SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC:
 	case SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC:
-		ctx = create_protected_ctx(i915, true, true, true, false, 0);
-		assert_ctx_protected_param(i915, ctx, true);
+	case SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK:
+		if (test_cfg == SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK) {
+			ctx = create_protected_ctx(i915, false, false, false, false, 0);
+			assert_ctx_protected_param(i915, ctx, false);
+
+		} else {
+			ctx = create_protected_ctx(i915, true, true, true, false, 0);
+			assert_ctx_protected_param(i915, ctx, true);
+		}
 
 		if (test_cfg == SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC) {
 			/* use normal buffers for testing for invalidation
@@ -754,7 +762,10 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 			intel_bb_add_intel_buf(ibb2, fencebuf, true);
 			gem_execbuf_flush_store_dw(i915, ibb2, ctx2, fencebuf, -ENOEXEC);
 		} else {
-			gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, -EACCES);
+			if (test_cfg == SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC)
+				gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, -EACCES);
+			else if (test_cfg == SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK)
+				gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, 0);
 		}
 
 		intel_bb_destroy(ibb);
@@ -870,6 +881,9 @@ igt_main
 		igt_subtest("reject-old-prot-buffer-execution-after-suspend-resume") {
 			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC, &pm);
 		}
+		igt_subtest("allow-regular-ctx-old-prot-buff-execution-after-suspend-resume") {
+			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK, &pm);
+		}
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 14/16] Ensure RESET_STAT reports invalidated protected context

[Alan Previn]

When protected contexts are created but get invalidated
due to PXP session teardown (such as after a suspend-resume
cycle), RESET_STAT ioctl for said context will report it
as I915_CONTEXT_INVALIDATED.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index ef93c4fe..def452cc 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -40,6 +40,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
 #define SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC 3
 #define SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK 4
+#define SESSION_PMSUSPEND_STALEPROTCTX_IN_RESETSTAT 5
 
 /* Struct and defintions for power management. */
 struct powermgt_data {
@@ -684,6 +685,25 @@ static void gem_execbuf_flush_store_dw(int i915, struct intel_bb *ibb,
 	}
 }
 
+static void check_context_invalid_state(int i915,
+		uint32_t ctx, bool is_expected_invalid)
+{
+	struct drm_i915_reset_stats rs;
+	bool ctx_is_invalid = false;
+	int ret;
+
+	memset(&rs, 0, sizeof(rs));
+	rs.ctx_id = ctx;
+
+	ret = drmIoctl(i915, DRM_IOCTL_I915_GET_RESET_STATS, &rs);
+	igt_assert(ret == 0);
+
+	if (rs.flags & I915_CONTEXT_INVALIDATED)
+		ctx_is_invalid = true;
+	igt_assert_eq(is_expected_invalid, ctx_is_invalid);
+
+}
+
 static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		struct powermgt_data *pm)
 {
@@ -779,6 +799,27 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 		buf_ops_destroy(bops);
 		break;
 
+	case SESSION_PMSUSPEND_STALEPROTCTX_IN_RESETSTAT:
+		ctx = create_protected_ctx(i915, true, true, true, false, 0);
+		assert_ctx_protected_param(i915, ctx, true);
+
+		ibb = intel_bb_create_with_context(i915, ctx, 4096);
+		igt_assert(ibb);
+
+		bops = buf_ops_create(i915);
+		igt_assert(bops);
+
+		fencebuf = intel_buf_create_using_handle(bops, fencebo, 256, 4,
+							 32, 0, I915_TILING_NONE, 0);
+		intel_bb_add_intel_buf(ibb, fencebuf, true);
+
+		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf, 0);
+		check_context_invalid_state(i915, ctx, false);
+
+		trigger_powermgt_suspend_cycle(i915, pm);
+		check_context_invalid_state(i915, ctx, true);
+		break;
+
 	default:
 		igt_info("Skipping unknown power-mgt test_cfg = %d\n", test_cfg);
 		break;
@@ -884,6 +925,9 @@ igt_main
 		igt_subtest("allow-regular-ctx-old-prot-buff-execution-after-suspend-resume") {
 			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALECTX_STALEPROTBO_EXEC_OK, &pm);
 		}
+		igt_subtest("old-prot-context-reports-reset-stat-after-suspend-resume") {
+			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTCTX_IN_RESETSTAT, &pm);
+		}
 	}
 
 	igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 15/16] Verify protected surfaces are dma buffer sharable

[Alan Previn]

Verify we can export a protected surface from
protected context A into protected context B (with
different client driver handles) and protected
rendering is successful even after prior file
handle is closed (i.e. pxp specific refcounting
works).

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 tests/i915/gem_pxp.c | 111 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index def452cc..bed3b31e 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -4,6 +4,7 @@
  */
 
 #include <sys/ioctl.h>
+#include <fcntl.h>
 
 #include "igt.h"
 #include "igt_device.h"
@@ -35,6 +36,9 @@ IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated
 #define COPY3D_PROTECTED_PROTSRC_TO_PROTDST 3
 #define COPY3D_PROTECTED_SRC_TO_PROTDST_SAMPLED 4
 
+/* test-configs for protected rendering with dma-buffer sharing */
+#define BUFSHARED_PROTDST_DUAL_CTX_REFCOUNT 1
+
 /* test-configs for power-management triggered protected session teardown */
 #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
 #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
@@ -591,6 +595,111 @@ static void __test_render_protected_buffer(int i915, uint32_t test_cfg, uint32_t
 	buf_ops_destroy(bops);
 }
 
+static int export_handle(int fd, uint32_t handle, int *outfd)
+{
+	struct drm_prime_handle args;
+	int ret;
+
+	args.handle = handle;
+	args.flags = DRM_CLOEXEC;
+	args.fd = -1;
+
+	ret = drmIoctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
+	if (ret)
+		ret = errno;
+	*outfd = args.fd;
+
+	return ret;
+}
+
+static void test_sharing_protected_asssets(uint32_t test_cfg)
+{
+	uint32_t ctx[2], sbo[2], dbo[2];
+	struct intel_buf *sbuf[2], *dbuf[2];
+	struct buf_ops *bops[2];
+	struct intel_bb *ibb[2];
+	int fd[2], dmabuf_fd = 0, ret, n, num_matches = 0;
+	uint32_t encrypted[2][TSTSURF_SIZE/TSTSURF_BYTESPP];
+
+	switch (test_cfg) {
+	case BUFSHARED_PROTDST_DUAL_CTX_REFCOUNT:
+		/* First, create the client driver handles and
+		 * protected dest buffer (is exported via dma-buff
+		 * from first handle and imported to the second).
+		 */
+		for (n = 0; n < 2; ++n) {
+			fd[n] = drm_open_driver(DRIVER_INTEL);
+			igt_require(fd[n]);
+			if (n == 0) {
+				dbo[0] = alloc_and_fill_dest_buff(fd[0], true, TSTSURF_SIZE, TSTSURF_INITCOLOR1);
+			} else {
+				ret = export_handle(fd[0], dbo[0], &dmabuf_fd);
+				igt_assert(ret == 0);
+				dbo[1] = prime_fd_to_handle(fd[1], dmabuf_fd);
+				igt_assert(dbo[1]);
+			}
+		}
+		/* Repeat twice: Create a full set of assets to perform
+		 * a protected 3D session but using the same dest buffer
+		 * from above.
+		 */
+		for (n = 0; n < 2; ++n) {
+			ctx[n] = create_protected_ctx(fd[n],
+				true, true, true, false, 0);
+			assert_ctx_protected_param(fd[n],
+				ctx[n], true);
+			bops[n] = buf_ops_create(fd[n]);
+			if (n == 1)
+				fill_bo_content(fd[1], dbo[1], TSTSURF_SIZE, TSTSURF_INITCOLOR2);
+
+			dbuf[n] = intel_buf_create_using_handle(bops[n], dbo[n], TSTSURF_WIDTH, TSTSURF_HEIGHT,
+								TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+			intel_buf_set_pxp(dbuf[n], true);
+
+			sbo[n] = alloc_and_fill_dest_buff(fd[n], false, TSTSURF_SIZE, TSTSURF_FILLCOLOR1);
+			sbuf[n] = intel_buf_create_using_handle(bops[n], sbo[n], TSTSURF_WIDTH, TSTSURF_HEIGHT,
+								TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
+
+			ibb[n] = intel_bb_create_with_context(fd[n], ctx[n], 4096);
+			intel_bb_set_pxp(ibb[n], true, DISPLAY_APPTYPE, I915_PROTECTED_CONTENT_DEFAULT_SESSION);
+
+			gen12_render_copyfunc(ibb[n], sbuf[n], 0, 0, TSTSURF_WIDTH, TSTSURF_HEIGHT,
+					      dbuf[n], 0, 0);
+			gem_sync(fd[n], dbo[n]);
+
+			assert_bo_content_check(fd[n], dbo[n], COMPARE_COLOR_UNREADIBLE,
+						TSTSURF_SIZE, TSTSURF_FILLCOLOR1, NULL, 0);
+			assert_bo_content_check(fd[n], dbo[n], COPY_BUFFER,
+						TSTSURF_SIZE, 0, encrypted[n], TSTSURF_SIZE);
+
+			/* free up all assets except the dest buffer to
+			 * verify dma buff refcounting is performed on
+			 * the protected dest buffer on the 2nd loop with
+			 * successful reuse in another protected render.
+			 */
+			intel_bb_destroy(ibb[n]);
+			intel_buf_destroy(sbuf[n]);
+			intel_buf_destroy(dbuf[n]);
+			gem_close(fd[n], sbo[n]);
+			gem_close(fd[n], dbo[n]);
+			gem_context_destroy(fd[n], ctx[n]);
+			close(fd[n]);
+		}
+		/* Verify that encrypted output across loops were equal */
+		for (n = 0; n < (TSTSURF_SIZE/4); ++n)
+			if (encrypted[0][n] == encrypted[1][n])
+				++num_matches;
+		igt_assert(num_matches == (TSTSURF_SIZE/4));
+
+		break;
+
+	default:
+		igt_info("Skipping unknown asset test_cfg = %d\n", test_cfg);
+		break;
+	}
+
+}
+
 static void test_render_protected_buffer(int i915, uint32_t test_cfg)
 {
 	__test_render_protected_buffer(i915, test_cfg, NULL, 0);
@@ -902,6 +1011,8 @@ igt_main
 			test_render_protected_buffer(i915, COPY3D_PROTECTED_SRC_TO_PROTDST);
 		igt_subtest("protected-encrypted-src-copy-not-readible")
 			test_render_protected_buffer(i915, COPY3D_PROTECTED_PROTSRC_TO_PROTDST);
+		igt_subtest("dmabuf-shared-protected-dst-is-context-refcounted")
+			test_sharing_protected_asssets(BUFSHARED_PROTDST_DUAL_CTX_REFCOUNT);
 	}
 	igt_subtest_group {
 		igt_fixture {
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 16/16] tests/i915_pxp: CRC validation for display tests.

[Alan Previn]

From: Karthik B S <karthik.b.s@intel.com>

Added subtests to validate pxp using CRC validation.

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 tests/i915/gem_pxp.c | 263 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 262 insertions(+), 1 deletion(-)

diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
index bed3b31e..b691954e 100644
--- a/tests/i915/gem_pxp.c
+++ b/tests/i915/gem_pxp.c
@@ -10,6 +10,7 @@
 #include "igt_device.h"
 #include "igt_sysfs.h"
 #include "i915/gem.h"
+#include <fcntl.h>
 
 IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
 /* Note: PXP = "Protected Xe Path" */
@@ -457,6 +458,7 @@ static uint32_t alloc_and_fill_dest_buff(int i915, bool protected, uint32_t size
 #define TSTSURF_INITCOLOR1  0x12341234
 #define TSTSURF_INITCOLOR2  0x56785678
 #define TSTSURF_INITCOLOR3  0xabcdabcd
+#define TSTSURF_GREENCOLOR  0xFF00FF00
 
 static void __test_render_protected_buffer(int i915, uint32_t test_cfg, uint32_t *outpixels, int outsize)
 {
@@ -651,7 +653,6 @@ static void test_sharing_protected_asssets(uint32_t test_cfg)
 			bops[n] = buf_ops_create(fd[n]);
 			if (n == 1)
 				fill_bo_content(fd[1], dbo[1], TSTSURF_SIZE, TSTSURF_INITCOLOR2);
-
 			dbuf[n] = intel_buf_create_using_handle(bops[n], dbo[n], TSTSURF_WIDTH, TSTSURF_HEIGHT,
 								TSTSURF_BYTESPP*8, 0, I915_TILING_NONE, 0);
 			intel_buf_set_pxp(dbuf[n], true);
@@ -935,6 +936,249 @@ static void test_protected_session_teardown(int i915, uint32_t test_cfg,
 	}
 }
 
+static void setup_protected_fb(int i915, int width, int height, igt_fb_t *fb, uint32_t ctx)
+{
+	int err;
+	uint32_t srcbo;
+	struct intel_buf *srcbuf, *dstbuf;
+	struct buf_ops *bops;
+	struct intel_bb *ibb;
+	uint32_t devid;
+	igt_render_copyfunc_t rendercopy;
+
+	devid = intel_get_drm_devid(i915);
+	igt_assert(devid);
+
+	rendercopy = igt_get_render_copyfunc(devid);
+	igt_assert(rendercopy);
+
+	bops = buf_ops_create(i915);
+	igt_assert(bops);
+
+	igt_init_fb(fb, i915, width, height, DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+
+	igt_calc_fb_size(i915, width, height, DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb->size, &fb->strides[0]);
+
+	fb->gem_handle = create_protected_bo(i915, fb->size, true, 0);
+	err = __gem_set_tiling(i915, fb->gem_handle, igt_fb_mod_to_tiling(fb->modifier), fb->strides[0]);
+	igt_assert(err == 0 || err == -EOPNOTSUPP);
+
+	do_or_die(__kms_addfb(fb->fd, fb->gem_handle, fb->width, fb->height, fb->drm_format,
+			      fb->modifier, fb->strides, fb->offsets, fb->num_planes,
+			      LOCAL_DRM_MODE_FB_MODIFIERS, &fb->fb_id));
+
+	dstbuf = intel_buf_create_using_handle(bops, fb->gem_handle, fb->width, fb->height,
+					       fb->plane_bpp[0], 0, igt_fb_mod_to_tiling(fb->modifier), 0);
+	dstbuf->is_protected = true;
+
+	srcbo = alloc_and_fill_dest_buff(i915, false, fb->size, TSTSURF_GREENCOLOR);
+	srcbuf = intel_buf_create_using_handle(bops, srcbo, fb->width, fb->height,
+					       fb->plane_bpp[0], 0, igt_fb_mod_to_tiling(fb->modifier), 0);
+
+	ibb = intel_bb_create_with_context(i915, ctx, 4096);
+	igt_assert(ibb);
+
+	ibb->pxp.enabled = true;
+	ibb->pxp.apptype = DISPLAY_APPTYPE;
+	ibb->pxp.appid = I915_PROTECTED_CONTENT_DEFAULT_SESSION;
+
+	gen12_render_copyfunc(ibb, srcbuf, 0, 0, fb->width, fb->height, dstbuf, 0, 0);
+
+	gem_sync(i915, fb->gem_handle);
+	assert_bo_content_check(i915, fb->gem_handle, COMPARE_COLOR_UNREADIBLE, fb->size, TSTSURF_GREENCOLOR, NULL, 0);
+
+	intel_bb_destroy(ibb);
+	intel_buf_destroy(srcbuf);
+	gem_close(i915, srcbo);
+}
+
+static void __debugfs_read(int fd, const char *param, char *buf, int len)
+{
+	len = igt_debugfs_simple_read(fd, param, buf, len);
+	if (len < 0)
+		igt_assert_eq(len, -ENODEV);
+}
+
+#define debugfs_read(fd, p, arr) __debugfs_read(fd, p, arr, sizeof(arr))
+#define MAX_SINK_HDCP_CAP_BUF_LEN	5000
+
+#define CP_UNDESIRED				0
+#define CP_DESIRED				1
+#define CP_ENABLED				2
+
+#define KERNEL_AUTH_TIME_ALLOWED_MSEC		(3 *  6 * 1000)
+#define KERNEL_DISABLE_TIME_ALLOWED_MSEC	(1 * 1000)
+
+static bool
+wait_for_prop_value(igt_output_t *output, uint64_t expected,
+		    uint32_t timeout_mSec)
+{
+	uint64_t val;
+	int i;
+
+	for (i = 0; i < timeout_mSec; i++) {
+		val = igt_output_get_prop(output, IGT_CONNECTOR_CONTENT_PROTECTION);
+		if (val == expected)
+			return true;
+		usleep(1000);
+	}
+
+	igt_info("prop_value mismatch %" PRId64 " != %" PRId64 "\n", val, expected);
+
+	return false;
+}
+
+static bool sink_hdcp_capable(int i915, igt_output_t *output)
+{
+	char buf[MAX_SINK_HDCP_CAP_BUF_LEN];
+	int fd;
+
+	fd = igt_debugfs_connector_dir(i915, output->name, O_RDONLY);
+	if (fd < 0)
+		return false;
+
+	if (is_i915_device(i915))
+		debugfs_read(fd, "i915_hdcp_sink_capability", buf);
+	else
+		debugfs_read(fd, "hdcp_sink_capability", buf);
+
+	close(fd);
+
+	igt_debug("Sink capability: %s\n", buf);
+
+	return strstr(buf, "HDCP1.4");
+}
+
+static bool output_hdcp_capable(int i915, igt_output_t *output, bool content_type)
+{
+	if (!output->props[IGT_CONNECTOR_CONTENT_PROTECTION])
+		return false;
+	if (!output->props[IGT_CONNECTOR_HDCP_CONTENT_TYPE] && content_type)
+		return false;
+	if (!sink_hdcp_capable(i915, output))
+		return false;
+
+	return true;
+}
+
+static void test_display_protected_crc(int i915, igt_display_t *display, bool hdcp)
+{
+	igt_output_t *output;
+	drmModeModeInfo *mode;
+	igt_fb_t ref_fb, protected_fb;
+	igt_plane_t *plane;
+	igt_pipe_t *pipe;
+	igt_pipe_crc_t *pipe_crc;
+	igt_crc_t ref_crc, new_crc;
+	int width = 0, height = 0, i = 0, ret, count, valid_outputs = 0;
+	uint32_t ctx;
+	igt_output_t *hdcp_output[IGT_MAX_PIPES];
+
+	ctx = create_protected_ctx(i915, true, true, true, false, 0);
+
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+
+		width = max(width, mode->hdisplay);
+		height = max(height, mode->vdisplay);
+
+		if (!output_hdcp_capable(i915, output, 0))
+			continue;
+
+		hdcp_output[valid_outputs++] = output;
+	}
+
+	if (hdcp)
+		igt_require_f(valid_outputs > 0, "No HDCP capable connector found\n");
+
+	igt_create_color_fb(i915, width, height, DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, 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);
+
+		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++;
+	}
+
+	if (hdcp) {
+		/* Enable HDCP on all the valid connectors */
+		for (count = 0; count < valid_outputs; count++) {
+			igt_output_set_prop_value(hdcp_output[count], IGT_CONNECTOR_CONTENT_PROTECTION, CP_DESIRED);
+			if (output->props[IGT_CONNECTOR_HDCP_CONTENT_TYPE])
+				igt_output_set_prop_value(hdcp_output[count], IGT_CONNECTOR_HDCP_CONTENT_TYPE, 0);
+		}
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+
+		/*Verify that HDCP is enabled on all valid connectors */
+		for (count = 0; count < valid_outputs; count++) {
+			ret = wait_for_prop_value(hdcp_output[count], CP_ENABLED, KERNEL_AUTH_TIME_ALLOWED_MSEC);
+			igt_assert_f(ret, "Content Protection not enabled on %s\n", hdcp_output[count]->name);
+		}
+	}
+
+	setup_protected_fb(i915, width, height, &protected_fb, ctx);
+
+	for_each_connected_output(display, output) {
+		mode = igt_output_get_mode(output);
+		pipe = &display->pipes[output->pending_pipe];
+		pipe_crc = igt_pipe_crc_new(i915, pipe->pipe, INTEL_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);
+
+		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);
+		igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
+
+		igt_plane_set_fb(plane, &protected_fb);
+		igt_fb_set_size(&protected_fb, plane, mode->hdisplay, mode->vdisplay);
+		igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+		igt_pipe_crc_collect_crc(pipe_crc, &new_crc);
+		igt_assert_crc_equal(&ref_crc, &new_crc);
+
+		if (hdcp) {
+			/* Disable HDCP and collect CRC */
+			igt_output_set_prop_value(hdcp_output[0], IGT_CONNECTOR_CONTENT_PROTECTION, CP_UNDESIRED);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+			ret = wait_for_prop_value(hdcp_output[0], CP_UNDESIRED, KERNEL_DISABLE_TIME_ALLOWED_MSEC);
+			igt_assert_f(ret, "Content Protection not cleared\n");
+
+			igt_plane_set_fb(plane, &protected_fb);
+			igt_fb_set_size(&protected_fb, plane, mode->hdisplay, mode->vdisplay);
+			igt_plane_set_size(plane, mode->hdisplay, mode->vdisplay);
+
+			igt_display_commit2(display, COMMIT_ATOMIC);
+			igt_pipe_crc_collect_crc(pipe_crc, &new_crc);
+			igt_assert_f(!igt_check_crc_equal(&ref_crc, &new_crc), "CRC's are expected to be different after HDCP is disabled\n");
+		}
+		/*
+		 * Testing with one pipe-output combination is sufficient.
+		 * So break the loop.
+		 */
+		break;
+	}
+
+	gem_context_destroy(i915, ctx);
+	igt_remove_fb(i915, &ref_fb);
+	igt_remove_fb(i915, &protected_fb);
+}
+
 igt_main
 {
 	int i915 = -1;
@@ -942,6 +1186,7 @@ igt_main
 	struct powermgt_data pm = {0};
 	igt_render_copyfunc_t rendercopy = NULL;
 	uint32_t devid = 0;
+	igt_display_t display;
 
 	igt_fixture
 	{
@@ -1040,7 +1285,23 @@ igt_main
 			test_protected_session_teardown(i915, SESSION_PMSUSPEND_STALEPROTCTX_IN_RESETSTAT, &pm);
 		}
 	}
+	igt_subtest_group {
+		igt_fixture {
+			igt_require(pxp_supported);
+			devid = intel_get_drm_devid(i915);
+			igt_assert(devid);
+			rendercopy = igt_get_render_copyfunc(devid);
+			igt_require(rendercopy);
 
+			igt_require_pipe_crc(i915);
+			igt_display_require(&display, i915);
+		}
+		igt_describe("Test the display CRC");
+		igt_subtest("display-protected-crc-without-hdcp")
+			test_display_protected_crc(i915, &display, 0);
+		igt_subtest("display-protected-crc-with-hdcp")
+			test_display_protected_crc(i915, &display, 1);
+	}
 	igt_fixture {
 		close(i915);
 	}
-- 
2.25.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for Introduce PXP Test (rev3)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 14835 bytes --]

== Series Details ==

Series: Introduce PXP Test (rev3)
URL   : https://patchwork.freedesktop.org/series/87570/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10081 -> IGTPW_5812
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_5812 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_5812, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_5812:

### IGT changes ###

#### Warnings ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       [FAIL][1] ([i915#1602] / [i915#2029]) -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bdw-5557u/igt@runner@aborted.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in IGTPW_5812 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@nop-compute0:
    - fi-ilk-650:         NOTRUN -> [SKIP][3] ([fdo#109271]) +30 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@amdgpu/amd_cs_nop@nop-compute0.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-bdw-5557u:       NOTRUN -> [WARN][4] ([i915#2283])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_fence@basic-await@rcs0:
    - fi-elk-e7500:       [PASS][5] -> [FAIL][6] ([i915#3457])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-elk-e7500/igt@gem_exec_fence@basic-await@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-elk-e7500/igt@gem_exec_fence@basic-await@rcs0.html

  * igt@gem_exec_fence@basic-await@vcs0:
    - fi-bsw-n3050:       [PASS][7] -> [FAIL][8] ([i915#3457]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-n3050/igt@gem_exec_fence@basic-await@vcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-n3050/igt@gem_exec_fence@basic-await@vcs0.html

  * igt@gem_exec_fence@nb-await@bcs0:
    - fi-bsw-nick:        [PASS][9] -> [FAIL][10] ([i915#3457])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-nick/igt@gem_exec_fence@nb-await@bcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-nick/igt@gem_exec_fence@nb-await@bcs0.html

  * igt@gem_exec_fence@nb-await@vecs0:
    - fi-bsw-kefka:       [PASS][11] -> [FAIL][12] ([i915#3457])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-kefka/igt@gem_exec_fence@nb-await@vecs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-kefka/igt@gem_exec_fence@nb-await@vecs0.html

  * igt@i915_module_load@reload:
    - fi-ilk-650:         NOTRUN -> [DMESG-FAIL][13] ([i915#3457]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@execlists:
    - fi-bdw-5557u:       NOTRUN -> [DMESG-FAIL][14] ([i915#3462])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][15] -> [INCOMPLETE][16] ([i915#2782])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@mman:
    - fi-bdw-5557u:       NOTRUN -> [DMESG-WARN][17] ([i915#3457]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@i915_selftest@live@mman.html

  * igt@kms_busy@basic@modeset:
    - fi-ilk-650:         NOTRUN -> [INCOMPLETE][18] ([i915#3457])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@kms_busy@basic@modeset.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][19] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-ilk-650:         NOTRUN -> [SKIP][20] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [PASS][21] -> [DMESG-WARN][22] ([i915#2868])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-ilk-650:         NOTRUN -> [FAIL][23] ([i915#3457]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-tgl-u2:          [PASS][24] -> [FAIL][25] ([i915#2416])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-tgl-u2/igt@kms_frontbuffer_tracking@basic.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-tgl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-elk-e7500:       [PASS][26] -> [FAIL][27] ([i915#53]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-elk-e7500/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-elk-e7500/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-bwr-2160:        [PASS][28] -> [FAIL][29] ([i915#53])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bwr-2160/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bwr-2160/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_psr@cursor_plane_move:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][30] ([fdo#109271]) +9 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bdw-5557u/igt@kms_psr@cursor_plane_move.html

  
#### Possible fixes ####

  * igt@gem_exec_fence@basic-await@vcs0:
    - fi-bxt-dsi:         [FAIL][31] ([i915#3457]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bxt-dsi/igt@gem_exec_fence@basic-await@vcs0.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bxt-dsi/igt@gem_exec_fence@basic-await@vcs0.html
    - fi-bsw-kefka:       [FAIL][33] ([i915#3457]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-kefka/igt@gem_exec_fence@basic-await@vcs0.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-kefka/igt@gem_exec_fence@basic-await@vcs0.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-tgl-u2:          [FAIL][35] ([i915#1888]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_wait@busy@all:
    - fi-bsw-nick:        [FAIL][37] ([i915#3177] / [i915#3457]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-nick/igt@gem_wait@busy@all.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-nick/igt@gem_wait@busy@all.html
    - fi-bsw-kefka:       [FAIL][39] ([i915#3177] / [i915#3457]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-kefka/igt@gem_wait@busy@all.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-kefka/igt@gem_wait@busy@all.html
    - fi-pnv-d510:        [FAIL][41] ([i915#3457]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-pnv-d510/igt@gem_wait@busy@all.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-pnv-d510/igt@gem_wait@busy@all.html

  * igt@gem_wait@wait@all:
    - fi-bsw-nick:        [FAIL][43] ([i915#3457]) -> [PASS][44] +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-nick/igt@gem_wait@wait@all.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-nick/igt@gem_wait@wait@all.html

  * igt@kms_busy@basic@flip:
    - fi-ilk-650:         [INCOMPLETE][45] ([i915#3457]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-ilk-650/igt@kms_busy@basic@flip.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@kms_busy@basic@flip.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-elk-e7500:       [FAIL][47] ([i915#53]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-elk-e7500/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html

  
#### Warnings ####

  * igt@gem_exec_gttfill@basic:
    - fi-ilk-650:         [FAIL][49] ([i915#3472]) -> [FAIL][50] ([i915#3457] / [i915#3472])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-ilk-650/igt@gem_exec_gttfill@basic.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-ilk-650/igt@gem_exec_gttfill@basic.html

  * igt@i915_module_load@reload:
    - fi-bsw-kefka:       [DMESG-WARN][51] ([i915#1982] / [i915#3457]) -> [DMESG-FAIL][52] ([i915#1982] / [i915#3457])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-kefka/igt@i915_module_load@reload.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-kefka/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-nick:        [DMESG-FAIL][53] -> [INCOMPLETE][54] ([i915#2782] / [i915#2940])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-bsw-nick/igt@i915_selftest@live@execlists.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-bsw-nick/igt@i915_selftest@live@execlists.html

  * igt@runner@aborted:
    - fi-cfl-8700k:       [FAIL][55] ([i915#3363]) -> [FAIL][56] ([i915#2426] / [i915#3363])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-cfl-8700k/igt@runner@aborted.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-cfl-8700k/igt@runner@aborted.html
    - fi-kbl-r:           [FAIL][57] ([i915#1436] / [i915#2426] / [i915#3363]) -> [FAIL][58] ([i915#1436] / [i915#3363])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-kbl-r/igt@runner@aborted.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-kbl-r/igt@runner@aborted.html
    - fi-kbl-soraka:      [FAIL][59] ([i915#1436] / [i915#3363]) -> [FAIL][60] ([i915#1436] / [i915#2426] / [i915#3363])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10081/fi-kbl-soraka/igt@runner@aborted.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/fi-kbl-soraka/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2029]: https://gitlab.freedesktop.org/drm/intel/issues/2029
  [i915#2283]: https://gitlab.freedesktop.org/drm/intel/issues/2283
  [i915#2416]: https://gitlab.freedesktop.org/drm/intel/issues/2416
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
  [i915#2868]: https://gitlab.freedesktop.org/drm/intel/issues/2868
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#3177]: https://gitlab.freedesktop.org/drm/intel/issues/3177
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3363]: https://gitlab.freedesktop.org/drm/intel/issues/3363
  [i915#3457]: https://gitlab.freedesktop.org/drm/intel/issues/3457
  [i915#3462]: https://gitlab.freedesktop.org/drm/intel/issues/3462
  [i915#3472]: https://gitlab.freedesktop.org/drm/intel/issues/3472
  [i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53


Participating hosts (36 -> 33)
------------------------------

  Missing    (3): fi-rkl-11500t fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6083 -> IGTPW_5812

  CI-20190529: 20190529
  CI_DRM_10081: dc2124d77a63b582650bad3429c60d8163819ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5812: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/index.html
  IGT_6083: d28aee5c5f528aa6c352c3339f20aaed4d698ffa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_pxp@allow-regular-ctx-old-prot-buff-execution-after-suspend-resume
+igt@gem_pxp@create-protected-buffer
+igt@gem_pxp@create-regular-buffer
+igt@gem_pxp@create-regular-context-1
+igt@gem_pxp@create-regular-context-2
+igt@gem_pxp@create-valid-protected-context
+igt@gem_pxp@display-protected-crc-without-hdcp
+igt@gem_pxp@display-protected-crc-with-hdcp
+igt@gem_pxp@dmabuf-shared-protected-dst-is-context-refcounted
+igt@gem_pxp@fail-invalid-protected-context
+igt@gem_pxp@hw-rejects-pxp-buffer
+igt@gem_pxp@hw-rejects-pxp-context
+igt@gem_pxp@old-prot-context-reports-reset-stat-after-suspend-resume
+igt@gem_pxp@protected-encrypted-src-copy-not-readible
+igt@gem_pxp@protected-raw-src-copy-not-readible
+igt@gem_pxp@regular-baseline-src-copy-readible
+igt@gem_pxp@reject-modify-context-protection-off-1
+igt@gem_pxp@reject-modify-context-protection-off-2
+igt@gem_pxp@reject-modify-context-protection-off-3
+igt@gem_pxp@reject-modify-context-protection-on
+igt@gem_pxp@reject-old-prot-buffer-execution-after-suspend-resume
+igt@gem_pxp@reject-old-prot-context-execution-after-suspend-resume
+igt@gem_pxp@verify-pxp-key-change-after-suspend-resume

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5812/index.html

[-- Attachment #1.2: Type: text/html, Size: 18804 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Michal Wajdeczko]

On 14.05.2021 08:49, Alan Previn wrote:
> Test PXP capability support as well as the allocation of protected
> buffers and protected contexts.
> 
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  tests/i915/gem_pxp.c | 366 +++++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build    |   1 +
>  2 files changed, 367 insertions(+)
>  create mode 100644 tests/i915/gem_pxp.c
> 
> diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> new file mode 100644
> index 00000000..6340daae
> --- /dev/null
> +++ b/tests/i915/gem_pxp.c
> @@ -0,0 +1,366 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include <sys/ioctl.h>
> +
> +#include "igt.h"
> +#include "i915/gem.h"
> +
> +IGT_TEST_DESCRIPTION("Test PXP that manages protected content through arbitrated HW-PXP-session");
> +/* Note: PXP = "Protected Xe Path" */
> +
> +/* test-configs for protected buffer creation*/
> +#define BO_ALLOC_NO_HW_SUPPORT 1
> +#define BO_ALLOC_PROTECT_ON 2
> +#define BO_ALLOC_PROTECT_OFF 3
> +
> +/* test-configs for protected context creation*/
> +#define CTX_ALLOC_NO_HW_SUPPORT 1
> +#define CTX_ALLOC_RECOVER_OFF_PROTECT_OFF 2
> +#define CTX_ALLOC_RECOVER_OFF_PROTECT_ON 3
> +#define CTX_ALLOC_RECOVER_ON_PROTECT_OFF 4
> +#define CTX_ALLOC_RECOVER_ON_PROTECT_ON 5
> +#define CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON 6
> +#define CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF 7
> +#define CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID 8
> +#define CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID 9

why not use enum instead?

> +
> +static bool is_pxp_hw_supported(int i915)
> +{
> +	uint32_t devid = intel_get_drm_devid(i915);
> +	const struct intel_device_info *devinfo = intel_get_device_info(devid);

can intel_get_device_info really return NULL ?

> +
> +	if (!devinfo) {
> +		igt_info("No device info found - assume no PXP support.\n");
> +		return false;
> +	}
> +	if (devinfo->is_tigerlake || devinfo->is_rocketlake ||
> +		devinfo->is_alderlake_s) {
> +		return true;

shouldn't IS_TIGERLAKE and similar be used instead ?

> +	}
> +
> +	return false;
> +}
> +
> +static uint32_t create_protected_bo(int i915, uint32_t size, bool protected_is_true,
> +				    int expected_return)
> +{
> +	int ret;
> +
> +	struct drm_i915_gem_create_ext_protected_content protected_ext = {
> +		.base = { .name = I915_GEM_CREATE_EXT_PROTECTED_CONTENT },
> +		.flags = 0,
> +	};
> +
> +	struct drm_i915_gem_create_ext create_ext = {
> +		.size = size,
> +		.extensions = 0,
> +	};
> +
> +	if (protected_is_true)
> +		create_ext.extensions = (uintptr_t)&protected_ext;

hmm, if call to "create_protected" can be used to create "unprotected"
bo, then maybe function shall be named as "create_bo_ext" ?

> +
> +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
> +	igt_assert_eq(ret, expected_return);

hmm, usually such asserts are done at caller's level
so maybe this function shall have signature like:

  int create_protected_bo(int i915, uint32_t size, uint32_t *bo_out)

> +
> +	return create_ext.handle;
> +}
> +
> +static void test_create_protected_buffer(int i915, uint32_t test_cfg)
> +{
> +	uint32_t bo;
> +
> +	switch (test_cfg) {
> +	case BO_ALLOC_NO_HW_SUPPORT:
> +		bo = create_protected_bo(i915, 4096, false, 0);

igt_assert(bo) ?

> +		gem_close(i915, bo);
> +		bo = create_protected_bo(i915, 4096, true, -ENODEV);
> +		igt_assert_eq(bo, 0);
> +		break;
> +
> +	case BO_ALLOC_PROTECT_OFF:
> +		bo = create_protected_bo(i915, 4096, false, 0);
> +		gem_close(i915, bo);
> +		break;
> +
> +	case BO_ALLOC_PROTECT_ON:
> +		bo = create_protected_bo(i915, 4096, true, 0);
> +		gem_close(i915, bo);
> +		break;
> +
> +	default:
> +		igt_info("Skipping unknown buffer test_cfg = %d\n", test_cfg);
> +		break;
> +	}
> +}
> +
> +static int create_ext_ioctl(int i915, struct drm_i915_gem_context_create_ext *arg)

I guess you should rather export this function from lib, not copy it

> +{
> +	int err;
> +
> +	err = 0;
> +	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> +		err = -errno;
> +		igt_assume(err);
> +	}
> +
> +	errno = 0;
> +	return err;
> +}
> +
> +static uint32_t create_protected_ctx(int i915, bool with_protected_param, bool protected_is_true,
> +				     bool with_recoverable_param, bool recoverable_is_true,
> +				     int expected_return)
> +{
> +	int ret;
> +
> +	struct drm_i915_gem_context_create_ext_setparam p_prot = {
> +		.base = {
> +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> +			.next_extension = 0,
> +		},
> +		.param = {
> +			.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> +			.value = 0,
> +		}
> +	};
> +	struct drm_i915_gem_context_create_ext_setparam p_norecover = {
> +		.base = {
> +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> +			.next_extension = 0,
> +		},
> +		.param = {
> +			.param = I915_CONTEXT_PARAM_RECOVERABLE,
> +			.value = 0,
> +		}
> +	};
> +	struct drm_i915_gem_context_create_ext create = {
> +		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> +		.extensions = 0,
> +	};
> +
> +	p_prot.param.value = protected_is_true;
> +	p_norecover.param.value = recoverable_is_true;
> +
> +	if (with_protected_param && with_recoverable_param) {
> +		create.extensions = to_user_pointer(&(p_norecover.base));
> +		p_norecover.base.next_extension = to_user_pointer(&(p_prot.base));
> +	} else if (!with_protected_param && with_recoverable_param) {
> +		create.extensions = to_user_pointer(&(p_norecover.base));
> +		p_norecover.base.next_extension = 0;
> +	} else if (with_protected_param && !with_recoverable_param) {
> +		create.extensions = to_user_pointer(&(p_prot.base));
> +		p_prot.base.next_extension = 0;
> +	} else if (!with_protected_param && !with_recoverable_param) {
> +		create.flags = 0;
> +	}
> +
> +	ret = create_ext_ioctl(i915, &create);
> +	igt_assert_eq(ret, expected_return);
> +
> +	return create.ctx_id;
> +}
> +
> +#define CHANGE_PARAM_PROTECTED 0x0001
> +#define CHANGE_PARAM_RECOVERY 0x0002
> +static void modify_ctx_protected_param(int i915, uint32_t ctx_id, uint32_t change_param_mask,
> +				       bool param_value, int expected_return)
> +{
> +	int ret;
> +
> +	struct drm_i915_gem_context_param ctx_param = {
> +		.ctx_id = ctx_id,
> +		.param = 0,
> +		.value = 0,
> +	};
> +
> +	if (change_param_mask == CHANGE_PARAM_PROTECTED) {
> +		ctx_param.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT;
> +		ctx_param.value = (int)param_value;
> +	} else if (change_param_mask == CHANGE_PARAM_RECOVERY) {
> +		ctx_param.param = I915_CONTEXT_PARAM_RECOVERABLE;
> +		ctx_param.value = (int)param_value;
> +	} else {
> +		return;
> +	}
> +
> +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM, &ctx_param);
> +
> +	igt_assert_eq(ret, expected_return);
> +}
> +
> +static void assert_ctx_protected_param(int i915, uint32_t ctx_id, bool is_protected)
> +{
> +	int ret;
> +
> +	struct drm_i915_gem_context_param ctx_param = {
> +		.ctx_id = ctx_id,
> +		.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> +	};
> +
> +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
> +	igt_assert_eq(ret, 0);
> +	igt_assert_eq(ctx_param.value, (int)is_protected);
> +}
> +
> +static void assert_ctx_recovery_param(int i915, uint32_t ctx_id, bool is_recoverable)
> +{
> +	int ret;
> +
> +	struct drm_i915_gem_context_param ctx_param = {
> +		.ctx_id = ctx_id,
> +		.param = I915_CONTEXT_PARAM_RECOVERABLE,
> +	};
> +
> +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM, &ctx_param);
> +	igt_assert_eq(ret, 0);
> +	igt_assert_eq(ctx_param.value, (int)is_recoverable);
> +}
> +
> +static void test_create_protected_context(int i915, uint32_t test_cfg)
> +{
> +	uint32_t ctx;
> +
> +	switch (test_cfg) {
> +	case CTX_ALLOC_NO_HW_SUPPORT:
> +		ctx = create_protected_ctx(i915, true, true, true, false, -ENODEV);
> +		igt_assert_eq(ctx, 0);
> +	case CTX_ALLOC_RECOVER_OFF_PROTECT_OFF:
> +		ctx = create_protected_ctx(i915, true, false, true, false, 0);
> +		assert_ctx_protected_param(i915, ctx, false);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_ALLOC_RECOVER_OFF_PROTECT_ON:
> +		ctx = create_protected_ctx(i915, true, true, true, false, 0);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_ALLOC_RECOVER_ON_PROTECT_OFF:
> +		ctx = create_protected_ctx(i915, true, false, true, true, 0);
> +		assert_ctx_protected_param(i915, ctx, false);
> +		assert_ctx_recovery_param(i915, ctx, true);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_ALLOC_RECOVER_ON_PROTECT_ON:
> +		ctx = create_protected_ctx(i915, true, true, true, true, -EPERM);
> +		igt_assert_eq(ctx, 0);
> +		ctx = create_protected_ctx(i915, true, true, false, false, -EPERM);
> +		igt_assert_eq(ctx, 0);
> +		break;
> +
> +	case CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON:
> +		ctx = create_protected_ctx(i915, true, true, true, false, 0);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF:
> +		ctx = create_protected_ctx(i915, true, true, true, false, 0);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID:
> +		ctx = create_protected_ctx(i915, true, true, true, false, 0);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, true, -EPERM);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, false, -EPERM);
> +		assert_ctx_protected_param(i915, ctx, true);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	case CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID:
> +		ctx = create_protected_ctx(i915, false, false, false, false, 0);
> +		assert_ctx_protected_param(i915, ctx, false);
> +		assert_ctx_recovery_param(i915, ctx, true);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_RECOVERY, false, 0);
> +		modify_ctx_protected_param(i915, ctx, CHANGE_PARAM_PROTECTED, true, -EPERM);
> +		assert_ctx_protected_param(i915, ctx, false);
> +		assert_ctx_recovery_param(i915, ctx, false);
> +		gem_context_destroy(i915, ctx);
> +		break;
> +
> +	default:
> +		igt_info("Skipping unknown context test_cfg = %d\n", test_cfg);
> +		break;

as there is nothing common between all these test configs, why bother
with yet another dispatcher function with this giant switch statement?

note that igt_main is dispatching tests anyway...

IMHO better to use set of dedicated functions:

	test_ctx_no_hw_support()
	test_ctx_alloc_recover_off_protect_of()
	...


> +	}
> +}
> +
> +igt_main
> +{
> +	int i915 = -1;
> +	bool pxp_supported = false;
> +
> +	igt_fixture
> +	{
> +		i915 = drm_open_driver(DRIVER_INTEL);
> +		igt_require(i915);
> +		igt_require_gem(i915);
> +		pxp_supported = is_pxp_hw_supported(i915);
> +	}
> +
> +	igt_subtest_group {
> +		igt_fixture {
> +			igt_require((pxp_supported == 0));
> +		}
> +
> +		igt_describe("Verify protected buffer on unsupported hw:");
> +		igt_subtest("hw-rejects-pxp-buffer")
> +			test_create_protected_buffer(i915, BO_ALLOC_NO_HW_SUPPORT);
> +		igt_describe("Verify protected context on unsupported hw:");
> +		igt_subtest("hw-rejects-pxp-context")
> +			test_create_protected_context(i915, CTX_ALLOC_NO_HW_SUPPORT);
> +	}
> +
> +	igt_subtest_group {
> +		igt_fixture {
> +			igt_require(pxp_supported);
> +		}
> +
> +		igt_describe("Verify protected buffer on supported hw:");
> +		igt_subtest("create-regular-buffer")
> +			test_create_protected_buffer(i915, BO_ALLOC_PROTECT_OFF);
> +		igt_subtest("create-protected-buffer")
> +			test_create_protected_buffer(i915, BO_ALLOC_PROTECT_ON);
> +
> +		igt_describe("Verify protected context on supported hw:");
> +		igt_subtest("create-regular-context-1")
> +			test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_OFF);
> +		igt_subtest("create-regular-context-2")
> +			test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_OFF);
> +		igt_subtest("fail-invalid-protected-context")
> +			test_create_protected_context(i915, CTX_ALLOC_RECOVER_ON_PROTECT_ON);
> +		igt_subtest("create-valid-protected-context")
> +			test_create_protected_context(i915, CTX_ALLOC_RECOVER_OFF_PROTECT_ON);
> +
> +		igt_describe("Verify protected context integrity:");
> +		igt_subtest("reject-modify-context-protection-on")
> +			test_create_protected_context(i915, CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID);
> +		igt_subtest("reject-modify-context-protection-off-1")
> +			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON);
> +		igt_subtest("reject-modify-context-protection-off-2")
> +			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF);
> +		igt_subtest("reject-modify-context-protection-off-3")
> +			test_create_protected_context(i915, CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID);
> +	}
> +
> +	igt_fixture {
> +		close(i915);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 19cc4ebe..e0bff4d1 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -187,6 +187,7 @@ i915_progs = [
>  	'gem_pread_after_blit',
>  	'gem_pwrite',
>  	'gem_pwrite_snooped',
> +	'gem_pxp',
>  	'gem_read_read_speed',
>  	'gem_readwrite',
>  	'gem_reg_read',
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Teres Alexis, Alan Previn]

On Fri, 2021-05-14 at 11:00 +0200, Michal Wajdeczko wrote:
> 
> On 14.05.2021 08:49, Alan Previn wrote:
> > Test PXP capability support as well as the allocation of protected
> > buffers and protected contexts.
> > 
> > Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> > ---
> >  tests/i915/gem_pxp.c | 366
> > +++++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build    |   1 +
> >  2 files changed, 367 insertions(+)
> >  create mode 100644 tests/i915/gem_pxp.c
> > 
> > diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> > new file mode 100644
> > index 00000000..6340daae
> > --- /dev/null
> > +++ b/tests/i915/gem_pxp.c
> > @@ -0,0 +1,366 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2021 Intel Corporation
> > + */
> > +
> > +#include <sys/ioctl.h>
> > +
> > +#include "igt.h"
> > +#include "i915/gem.h"
> > +
> > +IGT_TEST_DESCRIPTION("Test PXP that manages protected content
> > through arbitrated HW-PXP-session");
> > +/* Note: PXP = "Protected Xe Path" */
> > +
> > +/* test-configs for protected buffer creation*/
> > +#define BO_ALLOC_NO_HW_SUPPORT 1
> > +#define BO_ALLOC_PROTECT_ON 2
> > +#define BO_ALLOC_PROTECT_OFF 3
> > +
> > +/* test-configs for protected context creation*/
> > +#define CTX_ALLOC_NO_HW_SUPPORT 1
> > +#define CTX_ALLOC_RECOVER_OFF_PROTECT_OFF 2
> > +#define CTX_ALLOC_RECOVER_OFF_PROTECT_ON 3
> > +#define CTX_ALLOC_RECOVER_ON_PROTECT_OFF 4
> > +#define CTX_ALLOC_RECOVER_ON_PROTECT_ON 5
> > +#define CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON 6
> > +#define CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF 7
> > +#define CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID 8
> > +#define CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID 9
> 
> why not use enum instead?
> 
> > +
> > +static bool is_pxp_hw_supported(int i915)
> > +{
> > +	uint32_t devid = intel_get_drm_devid(i915);
> > +	const struct intel_device_info *devinfo =
> > intel_get_device_info(devid);
> 
> can intel_get_device_info really return NULL ?
> 
> > +
> > +	if (!devinfo) {
> > +		igt_info("No device info found - assume no PXP
> > support.\n");
> > +		return false;
> > +	}
> > +	if (devinfo->is_tigerlake || devinfo->is_rocketlake ||
> > +		devinfo->is_alderlake_s) {
> > +		return true;
> 
> shouldn't IS_TIGERLAKE and similar be used instead ?
> 
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static uint32_t create_protected_bo(int i915, uint32_t size, bool
> > protected_is_true,
> > +				    int expected_return)
> > +{
> > +	int ret;
> > +
> > +	struct drm_i915_gem_create_ext_protected_content protected_ext
> > = {
> > +		.base = { .name = I915_GEM_CREATE_EXT_PROTECTED_CONTENT
> > },
> > +		.flags = 0,
> > +	};
> > +
> > +	struct drm_i915_gem_create_ext create_ext = {
> > +		.size = size,
> > +		.extensions = 0,
> > +	};
> > +
> > +	if (protected_is_true)
> > +		create_ext.extensions = (uintptr_t)&protected_ext;
> 
> hmm, if call to "create_protected" can be used to create
> "unprotected"
> bo, then maybe function shall be named as "create_bo_ext" ?
> 
> > +
> > +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
> > +	igt_assert_eq(ret, expected_return);
> 
> hmm, usually such asserts are done at caller's level
> so maybe this function shall have signature like:
> 
>   int create_protected_bo(int i915, uint32_t size, uint32_t *bo_out)
> 
> > +
> > +	return create_ext.handle;
> > +}
> > +
> > +static void test_create_protected_buffer(int i915, uint32_t
> > test_cfg)
> > +{
> > +	uint32_t bo;
> > +
> > +	switch (test_cfg) {
> > +	case BO_ALLOC_NO_HW_SUPPORT:
> > +		bo = create_protected_bo(i915, 4096, false, 0);
> 
> igt_assert(bo) ?
> 
> > +		gem_close(i915, bo);
> > +		bo = create_protected_bo(i915, 4096, true, -ENODEV);
> > +		igt_assert_eq(bo, 0);
> > +		break;
> > +
> > +	case BO_ALLOC_PROTECT_OFF:
> > +		bo = create_protected_bo(i915, 4096, false, 0);
> > +		gem_close(i915, bo);
> > +		break;
> > +
> > +	case BO_ALLOC_PROTECT_ON:
> > +		bo = create_protected_bo(i915, 4096, true, 0);
> > +		gem_close(i915, bo);
> > +		break;
> > +
> > +	default:
> > +		igt_info("Skipping unknown buffer test_cfg = %d\n",
> > test_cfg);
> > +		break;
> > +	}
> > +}
> > +
> > +static int create_ext_ioctl(int i915, struct
> > drm_i915_gem_context_create_ext *arg)
> 
> I guess you should rather export this function from lib, not copy it
> 
> > +{
> > +	int err;
> > +
> > +	err = 0;
> > +	if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT,
> > arg)) {
> > +		err = -errno;
> > +		igt_assume(err);
> > +	}
> > +
> > +	errno = 0;
> > +	return err;
> > +}
> > +
> > +static uint32_t create_protected_ctx(int i915, bool
> > with_protected_param, bool protected_is_true,
> > +				     bool with_recoverable_param, bool
> > recoverable_is_true,
> > +				     int expected_return)
> > +{
> > +	int ret;
> > +
> > +	struct drm_i915_gem_context_create_ext_setparam p_prot = {
> > +		.base = {
> > +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> > +			.next_extension = 0,
> > +		},
> > +		.param = {
> > +			.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> > +			.value = 0,
> > +		}
> > +	};
> > +	struct drm_i915_gem_context_create_ext_setparam p_norecover = {
> > +		.base = {
> > +			.name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> > +			.next_extension = 0,
> > +		},
> > +		.param = {
> > +			.param = I915_CONTEXT_PARAM_RECOVERABLE,
> > +			.value = 0,
> > +		}
> > +	};
> > +	struct drm_i915_gem_context_create_ext create = {
> > +		.flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > +		.extensions = 0,
> > +	};
> > +
> > +	p_prot.param.value = protected_is_true;
> > +	p_norecover.param.value = recoverable_is_true;
> > +
> > +	if (with_protected_param && with_recoverable_param) {
> > +		create.extensions =
> > to_user_pointer(&(p_norecover.base));
> > +		p_norecover.base.next_extension =
> > to_user_pointer(&(p_prot.base));
> > +	} else if (!with_protected_param && with_recoverable_param) {
> > +		create.extensions =
> > to_user_pointer(&(p_norecover.base));
> > +		p_norecover.base.next_extension = 0;
> > +	} else if (with_protected_param && !with_recoverable_param) {
> > +		create.extensions = to_user_pointer(&(p_prot.base));
> > +		p_prot.base.next_extension = 0;
> > +	} else if (!with_protected_param && !with_recoverable_param) {
> > +		create.flags = 0;
> > +	}
> > +
> > +	ret = create_ext_ioctl(i915, &create);
> > +	igt_assert_eq(ret, expected_return);
> > +
> > +	return create.ctx_id;
> > +}
> > +
> > +#define CHANGE_PARAM_PROTECTED 0x0001
> > +#define CHANGE_PARAM_RECOVERY 0x0002
> > +static void modify_ctx_protected_param(int i915, uint32_t ctx_id,
> > uint32_t change_param_mask,
> > +				       bool param_value, int
> > expected_return)
> > +{
> > +	int ret;
> > +
> > +	struct drm_i915_gem_context_param ctx_param = {
> > +		.ctx_id = ctx_id,
> > +		.param = 0,
> > +		.value = 0,
> > +	};
> > +
> > +	if (change_param_mask == CHANGE_PARAM_PROTECTED) {
> > +		ctx_param.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT;
> > +		ctx_param.value = (int)param_value;
> > +	} else if (change_param_mask == CHANGE_PARAM_RECOVERY) {
> > +		ctx_param.param = I915_CONTEXT_PARAM_RECOVERABLE;
> > +		ctx_param.value = (int)param_value;
> > +	} else {
> > +		return;
> > +	}
> > +
> > +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM,
> > &ctx_param);
> > +
> > +	igt_assert_eq(ret, expected_return);
> > +}
> > +
> > +static void assert_ctx_protected_param(int i915, uint32_t ctx_id,
> > bool is_protected)
> > +{
> > +	int ret;
> > +
> > +	struct drm_i915_gem_context_param ctx_param = {
> > +		.ctx_id = ctx_id,
> > +		.param = I915_CONTEXT_PARAM_PROTECTED_CONTENT,
> > +	};
> > +
> > +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM,
> > &ctx_param);
> > +	igt_assert_eq(ret, 0);
> > +	igt_assert_eq(ctx_param.value, (int)is_protected);
> > +}
> > +
> > +static void assert_ctx_recovery_param(int i915, uint32_t ctx_id,
> > bool is_recoverable)
> > +{
> > +	int ret;
> > +
> > +	struct drm_i915_gem_context_param ctx_param = {
> > +		.ctx_id = ctx_id,
> > +		.param = I915_CONTEXT_PARAM_RECOVERABLE,
> > +	};
> > +
> > +	ret = ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM,
> > &ctx_param);
> > +	igt_assert_eq(ret, 0);
> > +	igt_assert_eq(ctx_param.value, (int)is_recoverable);
> > +}
> > +
> > +static void test_create_protected_context(int i915, uint32_t
> > test_cfg)
> > +{
> > +	uint32_t ctx;
> > +
> > +	switch (test_cfg) {
> > +	case CTX_ALLOC_NO_HW_SUPPORT:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > false, -ENODEV);
> > +		igt_assert_eq(ctx, 0);
> > +	case CTX_ALLOC_RECOVER_OFF_PROTECT_OFF:
> > +		ctx = create_protected_ctx(i915, true, false, true,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, false);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_ALLOC_RECOVER_OFF_PROTECT_ON:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_ALLOC_RECOVER_ON_PROTECT_OFF:
> > +		ctx = create_protected_ctx(i915, true, false, true,
> > true, 0);
> > +		assert_ctx_protected_param(i915, ctx, false);
> > +		assert_ctx_recovery_param(i915, ctx, true);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_ALLOC_RECOVER_ON_PROTECT_ON:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > true, -EPERM);
> > +		igt_assert_eq(ctx, 0);
> > +		ctx = create_protected_ctx(i915, true, true, false,
> > false, -EPERM);
> > +		igt_assert_eq(ctx, 0);
> > +		break;
> > +
> > +	case CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_RECOVERY, true, -EPERM);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_PROTECTED, false, -EPERM);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID:
> > +		ctx = create_protected_ctx(i915, true, true, true,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_RECOVERY, true, -EPERM);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_PROTECTED, false, -EPERM);
> > +		assert_ctx_protected_param(i915, ctx, true);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	case CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID:
> > +		ctx = create_protected_ctx(i915, false, false, false,
> > false, 0);
> > +		assert_ctx_protected_param(i915, ctx, false);
> > +		assert_ctx_recovery_param(i915, ctx, true);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_RECOVERY, false, 0);
> > +		modify_ctx_protected_param(i915, ctx,
> > CHANGE_PARAM_PROTECTED, true, -EPERM);
> > +		assert_ctx_protected_param(i915, ctx, false);
> > +		assert_ctx_recovery_param(i915, ctx, false);
> > +		gem_context_destroy(i915, ctx);
> > +		break;
> > +
> > +	default:
> > +		igt_info("Skipping unknown context test_cfg = %d\n",
> > test_cfg);
> > +		break;
> 
> as there is nothing common between all these test configs, why bother
> with yet another dispatcher function with this giant switch
> statement?
> 
> note that igt_main is dispatching tests anyway...
> 
> IMHO better to use set of dedicated functions:
> 
> 	test_ctx_no_hw_support()
> 	test_ctx_alloc_recover_off_protect_of()
> 	...
> 
> 
> > +	}
> > +}
> > +
> > +igt_main
> > +{
> > +	int i915 = -1;
> > +	bool pxp_supported = false;
> > +
> > +	igt_fixture
> > +	{
> > +		i915 = drm_open_driver(DRIVER_INTEL);
> > +		igt_require(i915);
> > +		igt_require_gem(i915);
> > +		pxp_supported = is_pxp_hw_supported(i915);
> > +	}
> > +
> > +	igt_subtest_group {
> > +		igt_fixture {
> > +			igt_require((pxp_supported == 0));
> > +		}
> > +
> > +		igt_describe("Verify protected buffer on unsupported
> > hw:");
> > +		igt_subtest("hw-rejects-pxp-buffer")
> > +			test_create_protected_buffer(i915,
> > BO_ALLOC_NO_HW_SUPPORT);
> > +		igt_describe("Verify protected context on unsupported
> > hw:");
> > +		igt_subtest("hw-rejects-pxp-context")
> > +			test_create_protected_context(i915,
> > CTX_ALLOC_NO_HW_SUPPORT);
> > +	}
> > +
> > +	igt_subtest_group {
> > +		igt_fixture {
> > +			igt_require(pxp_supported);
> > +		}
> > +
> > +		igt_describe("Verify protected buffer on supported
> > hw:");
> > +		igt_subtest("create-regular-buffer")
> > +			test_create_protected_buffer(i915,
> > BO_ALLOC_PROTECT_OFF);
> > +		igt_subtest("create-protected-buffer")
> > +			test_create_protected_buffer(i915,
> > BO_ALLOC_PROTECT_ON);
> > +
> > +		igt_describe("Verify protected context on supported
> > hw:");
> > +		igt_subtest("create-regular-context-1")
> > +			test_create_protected_context(i915,
> > CTX_ALLOC_RECOVER_OFF_PROTECT_OFF);
> > +		igt_subtest("create-regular-context-2")
> > +			test_create_protected_context(i915,
> > CTX_ALLOC_RECOVER_ON_PROTECT_OFF);
> > +		igt_subtest("fail-invalid-protected-context")
> > +			test_create_protected_context(i915,
> > CTX_ALLOC_RECOVER_ON_PROTECT_ON);
> > +		igt_subtest("create-valid-protected-context")
> > +			test_create_protected_context(i915,
> > CTX_ALLOC_RECOVER_OFF_PROTECT_ON);
> > +
> > +		igt_describe("Verify protected context integrity:");
> > +		igt_subtest("reject-modify-context-protection-on")
> > +			test_create_protected_context(i915,
> > CTX_MODIFY_UNPROTECTED_BOTHPARAMS_TO_VALID);
> > +		igt_subtest("reject-modify-context-protection-off-1")
> > +			test_create_protected_context(i915,
> > CTX_MODIFY_PROTECTED_RECOVER_OFF_TO_ON);
> > +		igt_subtest("reject-modify-context-protection-off-2")
> > +			test_create_protected_context(i915,
> > CTX_MODIFY_PROTECTED_PROTECT_ON_TO_OFF);
> > +		igt_subtest("reject-modify-context-protection-off-3")
> > +			test_create_protected_context(i915,
> > CTX_MODIFY_PROTECTED_BOTHPARAMS_TO_INVALID);
> > +	}
> > +
> > +	igt_fixture {
> > +		close(i915);
> > +	}
> > +}
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 19cc4ebe..e0bff4d1 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -187,6 +187,7 @@ i915_progs = [
> >  	'gem_pread_after_blit',
> >  	'gem_pwrite',
> >  	'gem_pwrite_snooped',
> > +	'gem_pxp',
> >  	'gem_read_read_speed',
> >  	'gem_readwrite',
> >  	'gem_reg_read',
> > 


Okay - i will fix them all and push a new rev.
...alan
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 12/16] Verify execbuf fails with stale PXP buffer after teardown

[Teres Alexis, Alan Previn]

On Thu, 2021-05-13 at 23:49 -0700, Alan Previn wrote:
> Add a subtest to verify that reusing a stale protected buffer
> in a gem_execbuff (with a protected context) after a teardown
> (triggered by suspend-resume cycle) shall fail with -EIO error.
> 
> NOTE: The end-to-end architecture requirement includes that
> any break in the links of the PXP sessions needs to trigger a
> full teardown and the application needs to be made aware of that
> allowing it to re-establish the end-to-end pipeline of buffers,
> contexts and renders again if it chooses to. This stricter
> behavior targets only contexts created with PXP enabled.
> 
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  tests/i915/gem_pxp.c | 44 ++++++++++++++++++++++++++++++++++++----
> ----
>  1 file changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> index 1952e230..605b6524 100644
> --- a/tests/i915/gem_pxp.c
> +++ b/tests/i915/gem_pxp.c
> @@ -38,6 +38,7 @@ IGT_TEST_DESCRIPTION("Test PXP that manages
> protected content through arbitrated
>  /* test-configs for power-management triggered protected session
> teardown */
>  #define SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE 1
>  #define SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC 2
> +#define SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC 3
>  
>  /* Struct and defintions for power management. */
>  struct powermgt_data {
> @@ -688,10 +689,10 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  	uint32_t encrypted_pixels_b4[TSTSURF_SIZE/TSTSURF_BYTESPP];
>  	uint32_t encrypted_pixels_aft[TSTSURF_SIZE/TSTSURF_BYTESPP];
>  	int matched_after_keychange = 0, loop = 0;
> -	uint32_t ctx, fencebo;
> +	uint32_t ctx, ctx2, fencebo;
>  	struct intel_buf *fencebuf;
>  	struct buf_ops *bops;
> -	struct intel_bb *ibb;
> +	struct intel_bb *ibb, *ibb2;
>  
>  	switch (test_cfg) {
>  	case SESSION_PMSUSPEND_TEARDOWN_KEY_CHANGE:
> @@ -712,14 +713,19 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  		break;
>  
>  	case SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC:
> +	case SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC:
>  		ctx = create_protected_ctx(i915, true, true, true,
> false, 0);
>  		assert_ctx_protected_param(i915, ctx, true);
>  
> -		/* use normal buffers for testing for invalidation
> -		 * of protected contexts to ensure kernel is catching
> -		 * the invalidated context (not buffer)
> -		 */
> -		fencebo = alloc_and_fill_dest_buff(i915, false, 4096,
> 0);
> +		if (test_cfg ==
> SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC) {
> +			/* use normal buffers for testing for
> invalidation
> +			 * of protected contexts to ensure kernel is
> catching
> +			 * the invalidated context
> +			 */
> +			fencebo = alloc_and_fill_dest_buff(i915, false,
> 4096, 0);
> +		} else {
> +			fencebo = alloc_and_fill_dest_buff(i915, true,
> 4096, 0);
> +		}
>  
>  		ibb = intel_bb_create_with_context(i915, ctx, 4096);
>  		igt_assert(ibb);
> @@ -734,12 +740,31 @@ static void test_protected_session_teardown(int
> i915, uint32_t test_cfg,
>  		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf,
> 0);
>  		trigger_powermgt_suspend_cycle(i915, pm);
>  
> -		gem_execbuf_flush_store_dw(i915, ibb, ctx, fencebuf,
> -EACCES);
> +		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC)
> {
> +			/* after teardown, alloc new assets for
> everything
> +			 * except the bo to ensure the kernel is
> catching
> +			 * the invalidated bo
> +			 */
> +			ctx2 = create_protected_ctx(i915, true, true,
> true, false, 0);
> +			assert_ctx_protected_param(i915, ctx2, true);
> +			ibb2 = intel_bb_create_with_context(i915, ctx2,
> 4096);
> +			igt_assert(ibb2);
> +
> +			intel_bb_detach_intel_buf(ibb, fencebuf);
> +			intel_bb_add_intel_buf(ibb2, fencebuf, true);
> +			gem_execbuf_flush_store_dw(i915, ibb2, ctx2,
> fencebuf, -ENOEXEC);
> +		} else {
> +			gem_execbuf_flush_store_dw(i915, ibb, ctx,
> fencebuf, -EACCES);
> +		}
>  
>  		intel_bb_destroy(ibb);
>  		intel_buf_destroy(fencebuf);
>  		gem_close(i915, fencebo);
>  		gem_context_destroy(i915, ctx);
> +		if (test_cfg == SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC)
> {
> +			intel_bb_destroy(ibb2);
> +			gem_context_destroy(i915, ctx2);
> +		}
>  		buf_ops_destroy(bops);
>  		break;
>  
> @@ -842,6 +867,9 @@ igt_main
>  		igt_subtest("reject-old-prot-context-execution-after-
> suspend-resume") {
>  			test_protected_session_teardown(i915,
> SESSION_PMSUSPEND_STALEPROTCTX_BAN_EXEC, &pm);
>  		}
> +		igt_subtest("reject-old-prot-buffer-execution-after-
> suspend-resume") {
> +			test_protected_session_teardown(i915,
> SESSION_PMSUSPEND_STALEPROTBO_BAN_EXEC, &pm);
> 
Feedback from testing: combine this test together with
test_protected_session_teardown because the total execution time of
this IGT increases by ~15 seconds everytime we take a suspend-resume
(required for this test).



> +		}
>  	}
>  
>  	igt_fixture {
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Dixit, Ashutosh]

On Fri, 14 May 2021 02:40:26 -0700, Teres Alexis, Alan Previn wrote:
>
>
> Okay - i will fix them all and push a new rev.

Mostly coding style looks good now but an easy way to check for
style/indendation issues in the code is to run checkpatch from the kernel
on the IGT patches and fix all issues which are not kernel specific (and
also ignore those which don't make sense):

https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl



_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Teres Alexis, Alan Previn]

I have been using checkpatch.pl on last few revs with zero issues. One caveat though and I just realize this now:
- when I run checkpatch "--no-tree -f my_modified_file.c", I don't get any warnings.
- when I run but running on the patches I get warnings about "exceeding 100 columns".

What is the official hard rule? Are we at 120, 100 or 80?

...alan

-----Original Message-----
From: Dixit, Ashutosh <ashutosh.dixit@intel.com> 
Sent: Saturday, May 15, 2021 4:16 PM
To: Teres Alexis, Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

On Fri, 14 May 2021 02:40:26 -0700, Teres Alexis, Alan Previn wrote:
>
>
> Okay - i will fix them all and push a new rev.

Mostly coding style looks good now but an easy way to check for style/indendation issues in the code is to run checkpatch from the kernel on the IGT patches and fix all issues which are not kernel specific (and also ignore those which don't make sense):

https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl



_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc

[Dixit, Ashutosh]

On Sat, 15 May 2021 16:58:24 -0700, Teres Alexis, Alan Previn wrote:
>
> I have been using checkpatch.pl on last few revs with zero issues. One caveat though and I just realize this now:
> - when I run checkpatch "--no-tree -f my_modified_file.c", I don't get any warnings.
> - when I run but running on the patches I get warnings about "exceeding 100 columns".
>
> What is the official hard rule? Are we at 120, 100 or 80?

IGT follows the kernel coding style. The kernel originally had a 80 char
width limit which was recently increased to 100 that is why checkpatch now
complains about 100.

So yes it's either 80 or 100 however you want to do it. But the official
hard limit is now 100 I'd think. So I'd fix it to be within one of those
widths.

>
> ...alan
>
> -----Original Message-----
> From: Dixit, Ashutosh <ashutosh.dixit@intel.com>
> Sent: Saturday, May 15, 2021 4:16 PM
> To: Teres Alexis, Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t 03/16] Add basic PXP testing of buffer and context alloc
>
> On Fri, 14 May 2021 02:40:26 -0700, Teres Alexis, Alan Previn wrote:
> >
> >
> > Okay - i will fix them all and push a new rev.
>
> Mostly coding style looks good now but an easy way to check for style/indendation issues in the code is to run checkpatch from the kernel on the IGT patches and fix all issues which are not kernel specific (and also ignore those which don't make sense):
>
> https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl
>
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev