On Mon, May 24, 2021 at 10:47:48PM -0700, Daniele Ceraolo Spurio wrote:

From: Vitaly Lubart vitaly.lubart@intel.com

Export PAVP client to work with i915 driver, for binding it uses kernel component framework.

Signed-off-by: Vitaly Lubart vitaly.lubart@intel.com Signed-off-by: Tomas Winkler tomas.winkler@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

---

drivers/misc/mei/Kconfig | 2 + drivers/misc/mei/Makefile | 1 + drivers/misc/mei/pxp/Kconfig | 13 ++ drivers/misc/mei/pxp/Makefile | 7 + drivers/misc/mei/pxp/mei_pxp.c | 233 +++++++++++++++++++++++++++++++++ drivers/misc/mei/pxp/mei_pxp.h | 18 +++ 6 files changed, 274 insertions(+) create mode 100644 drivers/misc/mei/pxp/Kconfig create mode 100644 drivers/misc/mei/pxp/Makefile create mode 100644 drivers/misc/mei/pxp/mei_pxp.c create mode 100644 drivers/misc/mei/pxp/mei_pxp.h

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index f5fd5b786607..0e0bcd0da852 100644 --- a/drivers/misc/mei/Kconfig +++ b/drivers/misc/mei/Kconfig @@ -47,3 +47,5 @@ config INTEL_MEI_TXE Intel Bay Trail

source "drivers/misc/mei/hdcp/Kconfig" +source "drivers/misc/mei/pxp/Kconfig"

diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index f1c76f7ee804..d8e5165917f2 100644 --- a/drivers/misc/mei/Makefile +++ b/drivers/misc/mei/Makefile @@ -26,3 +26,4 @@ mei-$(CONFIG_EVENT_TRACING) += mei-trace.o CFLAGS_mei-trace.o = -I$(src)

obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/ +obj-$(CONFIG_INTEL_MEI_PXP) += pxp/ diff --git a/drivers/misc/mei/pxp/Kconfig b/drivers/misc/mei/pxp/Kconfig new file mode 100644 index 000000000000..4029b96afc04 --- /dev/null +++ b/drivers/misc/mei/pxp/Kconfig @@ -0,0 +1,13 @@

+# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020, Intel Corporation. All rights reserved. +#

s> +config INTEL_MEI_PXP

• tristate "Intel PXP services of ME Interface"
• select INTEL_MEI_ME
• depends on DRM_I915
• help

•  MEI Support for PXP Services on Intel platforms.
  

•  Enables the ME FW services required for PXP support through
  

•  I915 display driver of Intel.
  

diff --git a/drivers/misc/mei/pxp/Makefile b/drivers/misc/mei/pxp/Makefile new file mode 100644 index 000000000000..0329950d5794 --- /dev/null +++ b/drivers/misc/mei/pxp/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2020, Intel Corporation. All rights reserved. +# +# Makefile - PXP client driver for Intel MEI Bus Driver.

+obj-$(CONFIG_INTEL_MEI_PXP) += mei_pxp.o diff --git a/drivers/misc/mei/pxp/mei_pxp.c b/drivers/misc/mei/pxp/mei_pxp.c new file mode 100644 index 000000000000..cacfbedb640a --- /dev/null +++ b/drivers/misc/mei/pxp/mei_pxp.c @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright © 2020 Intel Corporation
• */

+/**

• • DOC: MEI_PXP Client Driver
• • The mei_pxp driver acts as a translation layer between PXP
• • protocol implementer (I915) and ME FW by translating PXP
• • negotiation messages to ME FW command payloads and vice versa.
• */

+#include <linux/module.h> +#include <linux/slab.h> +#include <linux/uuid.h> +#include <linux/mei_cl_bus.h> +#include <linux/component.h> +#include <drm/drm_connector.h> +#include <drm/i915_component.h> +#include <drm/i915_pxp_tee_interface.h>

+#include "mei_pxp.h"

+/**

• • mei_pxp_send_message() - Sends a PXP message to ME FW.
• • @dev: device corresponding to the mei_cl_device
• • @message: a message buffer to send
• • @size: size of the message
• • Return: 0 on Success, <0 on Failure
• */

+static int +mei_pxp_send_message(struct device *dev, const void *message, size_t size) +{

• struct mei_cl_device *cldev;
• ssize_t byte;
• if (!dev || !message)

• return -EINVAL;
  

• cldev = to_mei_cl_device(dev);
• /* temporary drop const qualifier till the API is fixed */
• byte = mei_cldev_send(cldev, (u8 *)message, size);
• if (byte < 0) {

• dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte);
  

• return byte;
  

• }
• return 0;

+}

+/**

• • mei_pxp_receive_message() - Receives a PXP message from ME FW.
• • @dev: device corresponding to the mei_cl_device
• • @buffer: a message buffer to contain the received message
• • @size: size of the buffer
• • Return: bytes sent on Success, <0 on Failure
• */

+static int +mei_pxp_receive_message(struct device *dev, void *buffer, size_t size) +{

• struct mei_cl_device *cldev;
• ssize_t byte;
• if (!dev || !buffer)

• return -EINVAL;
  

• cldev = to_mei_cl_device(dev);
• byte = mei_cldev_recv(cldev, buffer, size);
• if (byte < 0) {

• dev_dbg(dev, "mei_cldev_recv failed. %zd\n", byte);
  

• return byte;
  

• }
• return byte;

+}

+static const struct i915_pxp_component_ops mei_pxp_ops = {

• .owner = THIS_MODULE,
• .send = mei_pxp_send_message,
• .recv = mei_pxp_receive_message,

+};

+static int mei_component_master_bind(struct device *dev) +{

• struct mei_cl_device *cldev = to_mei_cl_device(dev);
• struct i915_pxp_component *comp_master = mei_cldev_get_drvdata(cldev);
• int ret;
• dev_dbg(dev, "%s\n", __func__);
• comp_master->ops = &mei_pxp_ops;
• comp_master->tee_dev = dev;
• ret = component_bind_all(dev, comp_master);
• if (ret < 0)

• return ret;
  

• return 0;

+}

+static void mei_component_master_unbind(struct device *dev) +{

• struct mei_cl_device *cldev = to_mei_cl_device(dev);
• struct i915_pxp_component *comp_master = mei_cldev_get_drvdata(cldev);
• dev_dbg(dev, "%s\n", __func__);
• component_unbind_all(dev, comp_master);

+}

+static const struct component_master_ops mei_component_master_ops = {

• .bind = mei_component_master_bind,
• .unbind = mei_component_master_unbind,

+};

+/**

• • mei_pxp_component_match - compare function for matching mei pxp.
• • The function checks if the driver is i915, the subcomponent is PXP
• • and the grand parent of pxp and the parent of i915 are the same
• • PCH device.
• • @dev: master device
• • @subcomponent: subcomponent to match (I915_COMPONENT_PXP)
• • @data: compare data (mei pxp device)
• • Return:
• • • 1 - if components match
• • • 0 - otherwise
• */

+static int mei_pxp_component_match(struct device *dev, int subcomponent,

• 		   void *data)
  

+{

• struct device *base = data;
• if (subcomponent != I915_COMPONENT_PXP)

• return 0;
  

• if (strcmp(dev->driver->name, "i915") == 0) {

• base = base->parent;
  

• if (!base)
  

• 	return 0;
  

• base = base->parent;
  

• dev = dev->parent;
  

• return (base && dev && dev == base);
  

• }
• return 0;

+}

+static int mei_pxp_probe(struct mei_cl_device *cldev,

• 	 const struct mei_cl_device_id *id)
  

+{

• struct i915_pxp_component *comp_master;
• struct component_match *master_match;
• int ret;
• ret = mei_cldev_enable(cldev);
• if (ret < 0) {

• dev_err(&cldev->dev, "mei_cldev_enable Failed. %d\n", ret);
  

• goto enable_err_exit;
  

• }
• comp_master = kzalloc(sizeof(*comp_master), GFP_KERNEL);
• if (!comp_master) {

• ret = -ENOMEM;
  

• goto err_exit;
  

• }
• master_match = NULL;
• component_match_add_typed(&cldev->dev, &master_match,

• 		  mei_pxp_component_match, &cldev->dev);
  

• if (IS_ERR_OR_NULL(master_match)) {

• ret = -ENOMEM;
  

• goto err_exit;
  

• }
• mei_cldev_set_drvdata(cldev, comp_master);
• ret = component_master_add_with_match(&cldev->dev,

• 			      &mei_component_master_ops,
  

• 			      master_match);
  

• if (ret < 0) {

• dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
  

• goto err_exit;
  

• }
• return 0;

+err_exit:

• mei_cldev_set_drvdata(cldev, NULL);
• kfree(comp_master);
• mei_cldev_disable(cldev);

+enable_err_exit:

• return ret;

+}

+static void mei_pxp_remove(struct mei_cl_device *cldev) +{

• struct i915_pxp_component *comp_master = mei_cldev_get_drvdata(cldev);
• int ret;
• component_master_del(&cldev->dev, &mei_component_master_ops);
• kfree(comp_master);
• mei_cldev_set_drvdata(cldev, NULL);
• ret = mei_cldev_disable(cldev);
• if (ret)

• dev_warn(&cldev->dev, "mei_cldev_disable() failed\n");
  

+}

+/* fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1 : PAVP GUID*/

^ missing space

+#define MEI_GUID_PXP GUID_INIT(0xfbf6fcf1, 0x96cf, 0x4e2e, 0xA6, \

• 	       0xa6, 0x1b, 0xab, 0x8c, 0xbe, 0x36, 0xb1)
  

I don't have the spec to confirm this... that said...

the implementation looks clean and with the very little that I learned from MEI on the recent months I'm comfortable in adding:

Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com

Tomas, ack to get it from drm/drm-intel tree?

+static struct mei_cl_device_id mei_pxp_tbl[] = {

• { .uuid = MEI_GUID_PXP, .version = MEI_CL_VERSION_ANY },
• { }

+}; +MODULE_DEVICE_TABLE(mei, mei_pxp_tbl);

+static struct mei_cl_driver mei_pxp_driver = {

• .id_table = mei_pxp_tbl,
• .name = KBUILD_MODNAME,
• .probe = mei_pxp_probe,
• .remove = mei_pxp_remove,

+};

+module_mei_cl_driver(mei_pxp_driver);

+MODULE_AUTHOR("Intel Corporation"); +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("MEI PXP"); diff --git a/drivers/misc/mei/pxp/mei_pxp.h b/drivers/misc/mei/pxp/mei_pxp.h new file mode 100644 index 000000000000..e7b15373fefd --- /dev/null +++ b/drivers/misc/mei/pxp/mei_pxp.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*

• • Copyright © 2020 Intel Corporation
• • Authors:
• • Vitaly Lubart vitaly.lubart@intel.com
• */

+#ifndef __MEI_PXP_H__ +#define __MEI_PXP_H__

+/* me_pxp_status: Enumeration of all PXP Status Codes */ +enum me_pxp_status {

• ME_PXP_STATUS_SUCCESS = 0x0000,

+};

+#endif /* __MEI_PXP_H__ */

2.29.2

On 6/1/2021 1:20 PM, Rodrigo Vivi wrote:

On Mon, May 24, 2021 at 10:47:50PM -0700, Daniele Ceraolo Spurio wrote:

...

From: Chris Wilson chris@chris-wilson.co.uk

Allow internal clients to create a pinned context.

v2 (Daniele): export destructor as well, allow optional usage of custom vm for maximum flexibility.

Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

---

drivers/gpu/drm/i915/gt/intel_engine.h | 10 ++++++++ drivers/gpu/drm/i915/gt/intel_engine_cs.c | 29 +++++++++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h index 47ee8578e511..a64d28aba257 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine.h +++ b/drivers/gpu/drm/i915/gt/intel_engine.h @@ -18,7 +18,9 @@ #include "intel_workarounds.h"

struct drm_printer; +struct intel_context; struct intel_gt; +struct lock_class_key;

/* Early gen2 devices have a cacheline of just 32 bytes, using 64 is overkill,

• but keeps the logic simple. Indeed, the whole purpose of this macro is just

@@ -255,6 +257,14 @@ struct i915_request * intel_engine_find_active_request(struct intel_engine_cs *engine);

u32 intel_engine_context_size(struct intel_gt *gt, u8 class); +struct intel_context * +intel_engine_create_pinned_context(struct intel_engine_cs *engine,

• 		   struct i915_address_space *vm,
  

• 		   unsigned int ring_size,
  

• 		   unsigned int hwsp,
  

• 		   struct lock_class_key *key,
  

• 		   const char *name);
  

+void intel_engine_destroy_pinned_context(struct intel_context *ce);

void intel_engine_init_active(struct intel_engine_cs *engine, unsigned int subclass); diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index eba2da9679a5..8cbf11497e8e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -801,11 +801,13 @@ intel_engine_init_active(struct intel_engine_cs *engine, unsigned int subclass) #endif }

-static struct intel_context * -create_pinned_context(struct intel_engine_cs *engine,

•       unsigned int hwsp,
  

•       struct lock_class_key *key,
  

•       const char *name)
  

+struct intel_context * +intel_engine_create_pinned_context(struct intel_engine_cs *engine,

• 		   struct i915_address_space *vm,
  

• 		   unsigned int ring_size,
  

• 		   unsigned int hwsp,
  

• 		   struct lock_class_key *key,
  

• 		   const char *name)
  

  { struct intel_context *ce; int err;

@@ -816,6 +818,12 @@ create_pinned_context(struct intel_engine_cs *engine,

__set_bit(CONTEXT_BARRIER_BIT, &ce->flags); ce->timeline = page_pack_bits(NULL, hwsp);

• ce->ring = __intel_context_ring_size(ring_size);

why do we need this now and we didn't need before?

Since we're now exporting the function as a more "official" interface, the idea was to provide as much flexibility as possible. The ring size could be used if e.g. we decide to use more pxp sessions and therefore need more space in the ring to insert instructions. Same for the vm below.

Daniele

...

• if (vm) {

• i915_vm_put(ce->vm);
  

• ce->vm = i915_vm_get(vm);
  

• }

same question here...

...

err = intel_context_pin(ce); /* perma-pin so it is always available */ if (err) { @@ -834,7 +842,7 @@ create_pinned_context(struct intel_engine_cs *engine, return ce; }

-static void destroy_pinned_context(struct intel_context *ce) +void intel_engine_destroy_pinned_context(struct intel_context *ce) { struct intel_engine_cs *engine = ce->engine; struct i915_vma *hwsp = engine->status_page.vma; @@ -854,8 +862,9 @@ create_kernel_context(struct intel_engine_cs *engine) { static struct lock_class_key kernel;

• return create_pinned_context(engine, I915_GEM_HWS_SEQNO_ADDR,

• 		     &kernel, "kernel_context");
  

• return intel_engine_create_pinned_context(engine, NULL, SZ_4K,

• 				  I915_GEM_HWS_SEQNO_ADDR,
  

• 				  &kernel, "kernel_context");
  

  }

  /**

@@ -898,7 +907,7 @@ static int engine_init_common(struct intel_engine_cs *engine) return 0;

err_context:

• destroy_pinned_context(ce);

• intel_engine_destroy_pinned_context(ce); return ret; }

@@ -956,7 +965,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine) fput(engine->default_state);

if (engine->kernel_context)

• destroy_pinned_context(engine->kernel_context);
  

• intel_engine_destroy_pinned_context(engine->kernel_context);
  
  

  GEM_BUG_ON(!llist_empty(&engine->barrier_tasks)); cleanup_status_page(engine);

-- 2.29.2

On Mon, 2021-05-24 at 22:47 -0700, Daniele Ceraolo Spurio wrote:

From: "Huang, Sean Z" sean.z.huang@intel.com

Implement the funcs to create the TEE channel, so kernel can send the TEE commands directly to TEE for creating the arbitrary (default) session.

v2: fix locking, don't pollute dev_priv (Chris)

v3: wait for mei PXP component to be bound.

Signed-off-by: Huang, Sean Z sean.z.huang@intel.com Signed-off-by: Daniele Ceraolo Spurio < daniele.ceraolospurio@intel.com> Cc: Chris Wilson chris@chris-wilson.co.uk Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com #v2

---

drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/pxp/intel_pxp.c | 13 ++++ drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 87 ++++++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_tee.h | 14 ++++ drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 3 + 5 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_tee.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_tee.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index efd950122e40..0dfff52fea24 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -275,7 +275,8 @@ i915-y += i915_perf.o

# Protected execution platform (PXP) support i915-$(CONFIG_DRM_I915_PXP) += \

• pxp/intel_pxp.o

• pxp/intel_pxp.o \
• pxp/intel_pxp_tee.o

# Post-mortem debug and GPU hang state capture i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index 3255c6da34e8..5df2a09c9e4b 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -3,6 +3,7 @@

• Copyright(c) 2020 Intel Corporation.

*/ #include "intel_pxp.h" +#include "intel_pxp_tee.h" #include "gt/intel_context.h" #include "i915_drv.h"

@@ -50,7 +51,16 @@ void intel_pxp_init(struct intel_pxp *pxp) if (ret) return;

• ret = intel_pxp_tee_component_init(pxp);
• if (ret)

• goto out_context;
  

• drm_info(&gt->i915->drm, "Protected Xe Path (PXP) protected

content support initialized\n");

• return;

+out_context:

• destroy_vcs_context(pxp);

}

void intel_pxp_fini(struct intel_pxp *pxp) @@ -58,5 +68,8 @@ void intel_pxp_fini(struct intel_pxp *pxp) if (!intel_pxp_is_enabled(pxp)) return;

• intel_pxp_tee_component_fini(pxp);
• destroy_vcs_context(pxp);

} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c new file mode 100644 index 000000000000..4ed234d8584f --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: MIT +/*

• • Copyright(c) 2020 Intel Corporation.
• */

+#include <linux/component.h> +#include "drm/i915_pxp_tee_interface.h" +#include "drm/i915_component.h" +#include "i915_drv.h" +#include "intel_pxp.h" +#include "intel_pxp_tee.h"

+static inline struct intel_pxp *i915_dev_to_pxp(struct device *i915_kdev) +{

• return &kdev_to_i915(i915_kdev)->gt.pxp;

+}

+/**

• • i915_pxp_tee_component_bind - bind function to pass the function

pointers to pxp_tee

• • @i915_kdev: pointer to i915 kernel device
• • @tee_kdev: pointer to tee kernel device
• • @data: pointer to pxp_tee_master containing the function pointers
• • This bind function is called during the system boot or resume

from system sleep.

• • Return: return 0 if successful.
• */

+static int i915_pxp_tee_component_bind(struct device *i915_kdev,

• 		       struct device *tee_kdev, void
  

*data) +{

• struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
• pxp->pxp_component = data;
• pxp->pxp_component->tee_dev = tee_kdev;
• return 0;

+}

+static void i915_pxp_tee_component_unbind(struct device *i915_kdev,

• 			  struct device *tee_kdev, void
  

*data) +{

• struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
• pxp->pxp_component = NULL;

+}

+static const struct component_ops i915_pxp_tee_component_ops = {

• .bind = i915_pxp_tee_component_bind,
• .unbind = i915_pxp_tee_component_unbind,

+};

+int intel_pxp_tee_component_init(struct intel_pxp *pxp) +{

• int ret;
• struct intel_gt *gt = pxp_to_gt(pxp);
• struct drm_i915_private *i915 = gt->i915;
• ret = component_add_typed(i915->drm.dev,

&i915_pxp_tee_component_ops,

• 		  I915_COMPONENT_PXP);
  

• if (ret < 0) {

• drm_err(&i915->drm, "Failed to add PXP component
  

(%d)\n", ret);

• return ret;
  

• }
• /*

• * Adding the component does not guarantee that it will bind
  

properly,

• * so make sure to wait until it does.
  

• */
  

• ret = wait_for(pxp->pxp_component, 50);
• if (ret) {

• drm_err(&i915->drm, "Failed to bind PXP component 
  

Based on my testing with the 'wip-igt-pxp', this failed here because the mei-pxp was getting loaded multiple seconds later than i915. We might need to consider having a check for pxp_component availability on every IOCTL to ensure binding actually occured and succeed (==pxp- supported), or occured and failed (==pxp-failed, disable it), or didn't occure yet (==pending-retry?).

(%d)\n", ret);

• return ret;
  

• }
• return 0;

+}

+void intel_pxp_tee_component_fini(struct intel_pxp *pxp) +{

• struct intel_gt *gt = pxp_to_gt(pxp);
• struct drm_i915_private *i915 = gt->i915;
• if (!pxp->pxp_component)

• return;
  

• component_del(i915->drm.dev, &i915_pxp_tee_component_ops);

+} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h new file mode 100644 index 000000000000..23d050a5d3e7 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: MIT */ +/*

• • Copyright(c) 2020, Intel Corporation. All rights reserved.
• */

+#ifndef __INTEL_PXP_TEE_H__ +#define __INTEL_PXP_TEE_H__

+#include "intel_pxp.h"

+int intel_pxp_tee_component_init(struct intel_pxp *pxp); +void intel_pxp_tee_component_fini(struct intel_pxp *pxp);

+#endif /* __INTEL_PXP_TEE_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index bd12c520e60a..3e95d21513e8 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -7,8 +7,11 @@ #define __INTEL_PXP_TYPES_H__

struct intel_context; +struct i915_pxp_component;

struct intel_pxp {

• struct i915_pxp_component *pxp_component;
• struct intel_context *ce;

};

On Mon, May 24, 2021 at 10:47:56PM -0700, Daniele Ceraolo Spurio wrote:

From: "Huang, Sean Z" sean.z.huang@intel.com

The HW will generate a teardown interrupt when session termination is required, which requires i915 to submit a terminating batch. Once the HW is done with the termination it will generate another interrupt, at which point it is safe to re-create the session.

Since the termination and re-creation flow is something we want to trigger from the driver as well, use a common work function that can be called both from the irq handler and from the driver set-up flows, which has the addded benefit of allowing us to skip any extra locks because the work itself serializes the operations.

v2: use struct completion instead of bool (Chris) v3: drop locks, clean up functions and improve comments (Chris), move to common work function. v4: improve comments, simplify wait logic (Rodrigo)

Signed-off-by: Huang, Sean Z sean.z.huang@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Rodrigo Vivi rodrigo.vivi@intel.com

---

drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt_irq.c | 7 ++ drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/pxp/intel_pxp.c | 66 +++++++++++-- drivers/gpu/drm/i915/pxp/intel_pxp.h | 8 ++ drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 97 ++++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_irq.h | 32 +++++++ drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 54 ++++++++++- drivers/gpu/drm/i915/pxp/intel_pxp_session.h | 5 +- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 8 +- drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 18 ++++ 11 files changed, 281 insertions(+), 16 deletions(-) create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_irq.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_irq.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0fba97014512..29331bbb3e98 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -277,6 +277,7 @@ i915-y += i915_perf.o i915-$(CONFIG_DRM_I915_PXP) += \ pxp/intel_pxp.o \ pxp/intel_pxp_cmd.o \

• pxp/intel_pxp_irq.o \ pxp/intel_pxp_session.o \ pxp/intel_pxp_tee.o

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c index 9fc6c912a4e5..7c4ec8880b1a 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c @@ -13,6 +13,7 @@ #include "intel_lrc_reg.h" #include "intel_uncore.h" #include "intel_rps.h" +#include "pxp/intel_pxp_irq.h"

static void guc_irq_handler(struct intel_guc *guc, u16 iir) { @@ -106,6 +107,9 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 instance, if (instance == OTHER_GTPM_INSTANCE) return gen11_rps_irq_handler(&gt->rps, iir);

• if (instance == OTHER_KCR_INSTANCE)

• return intel_pxp_irq_handler(&gt->pxp, iir);
  

• WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n", instance, iir);

} @@ -232,6 +236,9 @@ void gen11_gt_irq_reset(struct intel_gt *gt) intel_uncore_write(uncore, GEN11_GPM_WGBOXPERF_INTR_MASK, ~0); intel_uncore_write(uncore, GEN11_GUC_SG_INTR_ENABLE, 0); intel_uncore_write(uncore, GEN11_GUC_SG_INTR_MASK, ~0);

• intel_uncore_write(uncore, GEN11_CRYPTO_RSVD_INTR_ENABLE, 0);
• intel_uncore_write(uncore, GEN11_CRYPTO_RSVD_INTR_MASK, ~0);

}

void gen11_gt_irq_postinstall(struct intel_gt *gt) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 4dbe79009c0e..297671d78076 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8023,6 +8023,7 @@ enum { /* irq instances for OTHER_CLASS */ #define OTHER_GUC_INSTANCE 0 #define OTHER_GTPM_INSTANCE 1 +#define OTHER_KCR_INSTANCE 4

#define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + ((x) * 4))

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index e48debb5ca22..6b0e7170c29b 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -2,7 +2,9 @@ /*

• Copyright(c) 2020 Intel Corporation.

*/ +#include <linux/workqueue.h> #include "intel_pxp.h" +#include "intel_pxp_irq.h" #include "intel_pxp_session.h" #include "intel_pxp_tee.h" #include "gt/intel_context.h" @@ -66,6 +68,16 @@ void intel_pxp_init(struct intel_pxp *pxp) if (!HAS_PXP(gt->i915)) return;

• /*

• * we'll use the completion to check if there is a termination pending,
  

• * so we start it as completed and we reinit it when a termination
  

• * is triggered.
  

• */
  

• init_completion(&pxp->termination);
• complete_all(&pxp->termination);
• INIT_WORK(&pxp->session_work, intel_pxp_session_work);
• ret = create_vcs_context(pxp); if (ret) return;

@@ -94,19 +106,61 @@ void intel_pxp_fini(struct intel_pxp *pxp) destroy_vcs_context(pxp); }

-void intel_pxp_init_hw(struct intel_pxp *pxp) +void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp) {

• int ret;

• pxp->arb_is_valid = false;
• reinit_completion(&pxp->termination);

+}

+static void intel_pxp_queue_termination(struct intel_pxp *pxp) +{

• struct intel_gt *gt = pxp_to_gt(pxp);
• /*

• * We want to get the same effect as if we received a termination
  

• * interrupt, so just pretend that we did.
  

• */
  

• spin_lock_irq(&gt->irq_lock);
• intel_pxp_mark_termination_in_progress(pxp);
• pxp->session_events |= PXP_TERMINATION_REQUEST;
• queue_work(system_unbound_wq, &pxp->session_work);
• spin_unlock_irq(&gt->irq_lock);

+}

+/*

• • the arb session is restarted from the irq work when we receive the
• • termination completion interrupt
• */

+int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +{

• if (!intel_pxp_is_enabled(pxp))

• return 0;
  

• if (!wait_for_completion_timeout(&pxp->termination,

• 			 msecs_to_jiffies(100)))
  

• return -ETIMEDOUT;
  

• if (!pxp->arb_is_valid)

• return -EIO;
  

• return 0;

+}

+void intel_pxp_init_hw(struct intel_pxp *pxp) +{ kcr_pxp_enable(pxp_to_gt(pxp));

• intel_pxp_irq_enable(pxp);

• /* always emit a full termination to clean the state */
• ret = intel_pxp_terminate_arb_session_and_global(pxp);
• if (!ret)

• intel_pxp_create_arb_session(pxp);
  

• /*

• * the session could've been attacked while we weren't loaded, so
  

• * handle it as if it was and re-create it.
  

• */
  

• intel_pxp_queue_termination(pxp);

}

void intel_pxp_fini_hw(struct intel_pxp *pxp) { kcr_pxp_disable(pxp_to_gt(pxp));

• intel_pxp_irq_disable(pxp);

} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 8eeb65af78b1..074b3b980957 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -30,6 +30,9 @@ void intel_pxp_fini(struct intel_pxp *pxp);

void intel_pxp_init_hw(struct intel_pxp *pxp); void intel_pxp_fini_hw(struct intel_pxp *pxp);

+void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); +int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) { @@ -38,6 +41,11 @@ static inline void intel_pxp_init(struct intel_pxp *pxp) static inline void intel_pxp_fini(struct intel_pxp *pxp) { }

+static inline int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +{

• return 0;

+} #endif

#endif /* __INTEL_PXP_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c new file mode 100644 index 000000000000..196449243515 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: MIT +/*

• • Copyright(c) 2020 Intel Corporation.
• */

+#include <linux/workqueue.h> +#include "intel_pxp.h" +#include "intel_pxp_irq.h" +#include "intel_pxp_session.h" +#include "gt/intel_gt_irq.h" +#include "i915_irq.h" +#include "i915_reg.h"

+/**

• • intel_pxp_irq_handler - Handles PXP interrupts.
• • @pxp: pointer to pxp struct
• • @iir: interrupt vector
• */

+void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir) +{

• struct intel_gt *gt = pxp_to_gt(pxp);
• if (GEM_WARN_ON(!intel_pxp_is_enabled(pxp)))

• return;
  

• lockdep_assert_held(&gt->irq_lock);
• if (unlikely(!iir))

• return;
  

• if (iir & (GEN12_DISPLAY_PXP_STATE_TERMINATED_INTERRUPT |

•    GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) {
  

• /* immediately mark PXP as inactive on termination */
  

• intel_pxp_mark_termination_in_progress(pxp);
  

• pxp->session_events |= PXP_TERMINATION_REQUEST;
  

• }
• if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)

• pxp->session_events |= PXP_TERMINATION_COMPLETE;
  

• if (pxp->session_events)

• queue_work(system_unbound_wq, &pxp->session_work);
  

+}

+static inline void __pxp_set_interrupts(struct intel_gt *gt, u32 interrupts) +{

• struct intel_uncore *uncore = gt->uncore;
• const u32 mask = interrupts << 16;
• intel_uncore_write(uncore, GEN11_CRYPTO_RSVD_INTR_ENABLE, mask);
• intel_uncore_write(uncore, GEN11_CRYPTO_RSVD_INTR_MASK, ~mask);

+}

+static inline void pxp_irq_reset(struct intel_gt *gt) +{

• spin_lock_irq(&gt->irq_lock);
• gen11_gt_reset_one_iir(gt, 0, GEN11_KCR);
• spin_unlock_irq(&gt->irq_lock);

+}

+void intel_pxp_irq_enable(struct intel_pxp *pxp) +{

• struct intel_gt *gt = pxp_to_gt(pxp);
• spin_lock_irq(&gt->irq_lock);
• if (!pxp->irq_enabled) {

• WARN_ON_ONCE(gen11_gt_reset_one_iir(gt, 0, GEN11_KCR));
  

• __pxp_set_interrupts(gt, GEN12_PXP_INTERRUPTS);
  

• pxp->irq_enabled = true;
  

• }
• spin_unlock_irq(&gt->irq_lock);

+}

+void intel_pxp_irq_disable(struct intel_pxp *pxp) +{

• struct intel_gt *gt = pxp_to_gt(pxp);
• /*

• * We always need to submit a global termination when we re-enable the
  

• * interrupts, so there is no need to make sure that the session state
  

• * makes sense at the end of this function. Just make sure this is not
  

• * called in a path were the driver consider the session as valid and
  

• * doesn't call a termination on restart.
  

• */
  

• GEM_WARN_ON(intel_pxp_is_active(pxp));
• spin_lock_irq(&gt->irq_lock);
• pxp->irq_enabled = false;
• __pxp_set_interrupts(gt, 0);
• spin_unlock_irq(&gt->irq_lock);
• intel_synchronize_irq(gt->i915);
• pxp_irq_reset(gt);
• flush_work(&pxp->session_work);

+} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h new file mode 100644 index 000000000000..8b5793654844 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: MIT */

oh, forgot this:

// SPDX-License-Identifier: MIT

+/*

• • Copyright(c) 2020, Intel Corporation. All rights reserved.
• */

+#ifndef __INTEL_PXP_IRQ_H__ +#define __INTEL_PXP_IRQ_H__

+#include <linux/types.h>

+struct intel_pxp;

+#define GEN12_DISPLAY_PXP_STATE_TERMINATED_INTERRUPT BIT(1) +#define GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT BIT(2) +#define GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT BIT(3)

+#define GEN12_PXP_INTERRUPTS \

• (GEN12_DISPLAY_PXP_STATE_TERMINATED_INTERRUPT | \

• GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT | \
  

• GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)
  

+#ifdef CONFIG_DRM_I915_PXP +void intel_pxp_irq_enable(struct intel_pxp *pxp); +void intel_pxp_irq_disable(struct intel_pxp *pxp); +void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir); +#else +static inline void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir) +{ +} +#endif

+#endif /* __INTEL_PXP_IRQ_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index b8e24adeb1f3..e751122cb24a 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -48,7 +48,7 @@ static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla return ret; }

-int intel_pxp_create_arb_session(struct intel_pxp *pxp) +static int pxp_create_arb_session(struct intel_pxp *pxp) { struct intel_gt *gt = pxp_to_gt(pxp); int ret; @@ -77,12 +77,13 @@ int intel_pxp_create_arb_session(struct intel_pxp *pxp) return 0; }

-int intel_pxp_terminate_arb_session_and_global(struct intel_pxp *pxp) +static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp) { int ret; struct intel_gt *gt = pxp_to_gt(pxp);

• pxp->arb_is_valid = false;

• /* must mark termination in progress calling this function */

• GEM_WARN_ON(pxp->arb_is_valid);

  /* terminate the hw sessions */ ret = intel_pxp_terminate_session(pxp, ARB_SESSION);

@@ -101,3 +102,50 @@ int intel_pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)

return ret; }

+static void pxp_terminate(struct intel_pxp *pxp) +{

• int ret;
• pxp->global_state_attacked = true;
• /*

• * if we fail to submit the termination there is no point in waiting for
  

• * it to complete. PXP will be marked as non-active until the next
  

• * termination is issued.
  

• */
  

• ret = pxp_terminate_arb_session_and_global(pxp);
• if (ret)

• complete_all(&pxp->termination);
  

+}

+static void pxp_terminate_complete(struct intel_pxp *pxp) +{

• /* Re-create the arb session after teardown handle complete */
• if (fetch_and_zero(&pxp->global_state_attacked))

• pxp_create_arb_session(pxp);
  

• complete_all(&pxp->termination);

+}

+void intel_pxp_session_work(struct work_struct *work) +{

• struct intel_pxp *pxp = container_of(work, typeof(*pxp), session_work);
• struct intel_gt *gt = pxp_to_gt(pxp);
• u32 events = 0;
• spin_lock_irq(&gt->irq_lock);
• events = fetch_and_zero(&pxp->session_events);
• spin_unlock_irq(&gt->irq_lock);
• if (!events)

• return;
  

• if (events & PXP_TERMINATION_REQUEST) {

• events &= ~PXP_TERMINATION_COMPLETE;
  

• pxp_terminate(pxp);
  

• }
• if (events & PXP_TERMINATION_COMPLETE)

• pxp_terminate_complete(pxp);
  

+} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.h b/drivers/gpu/drm/i915/pxp/intel_pxp_session.h index 7354314b1cc4..ba4c9d2b94b7 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.h @@ -8,9 +8,8 @@

#include <linux/types.h>

-struct intel_pxp; +struct work_struct;

-int intel_pxp_create_arb_session(struct intel_pxp *pxp); -int intel_pxp_terminate_arb_session_and_global(struct intel_pxp *pxp); +void intel_pxp_session_work(struct work_struct *work);

#endif /* __INTEL_PXP_SESSION_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index 98a62666b377..e3d178c44e51 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -65,21 +65,21 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev, { struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);

• int ret;

  pxp->pxp_component = data; pxp->pxp_component->tee_dev = tee_kdev;

  /* the component is required to fully start the PXP HW */ intel_pxp_init_hw(pxp);

• if (!pxp->arb_is_valid) {

• ret = intel_pxp_wait_for_arb_start(pxp);
• if (ret) { drm_err(&i915->drm, "Failed to create arb session during bind\n"); intel_pxp_fini_hw(pxp); pxp->pxp_component = NULL;

• return -EIO;
  

  }

• return 0;

• return ret;

}

static void i915_pxp_tee_component_unbind(struct device *i915_kdev, diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index 5f01eb3c3d2c..c059a17cbcfe 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -6,7 +6,9 @@ #ifndef __INTEL_PXP_TYPES_H__ #define __INTEL_PXP_TYPES_H__

+#include <linux/completion.h> #include <linux/types.h> +#include <linux/workqueue.h>

struct intel_context; struct i915_pxp_component; @@ -22,6 +24,22 @@ struct intel_pxp { * session to know if it's valid and need to track the status in SW. */ bool arb_is_valid;

• /*

• * If the HW perceives an attack on the integrity of the encryption it
  

• * will invalidate the keys and expect SW to re-initialize the session.
  

• * We keep track of this state to make sure we only re-start the arb
  

• * session when required.
  

• */
  

• bool global_state_attacked;
• bool irq_enabled;
• struct completion termination;
• struct work_struct session_work;
• u32 session_events; /* protected with gt->irq_lock */

+#define PXP_TERMINATION_REQUEST BIT(0) +#define PXP_TERMINATION_COMPLETE BIT(1) };

#endif /* __INTEL_PXP_TYPES_H__ */

2.29.2

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Mon, May 24, 2021 at 10:47:57PM -0700, Daniele Ceraolo Spurio wrote:

Extra tracking and checks around protected objects, coming in a follow-up patch, will be enabled only for contexts that opt in. Contexts can only be marked as using protected content at creation time and they must be both bannable and not recoverable.

When a PXP teardown occurs, all gem contexts marked this way that have been used at least once will be marked as invalid and all new submissions using them will be rejected. All intel contexts within the invalidated gem contexts will be marked banned. A new flag has been added to the RESET_STATS ioctl to report the invalidation to userspace.

v2: split to its own patch and improve doc (Chris), invalidate contexts on teardown

v3: improve doc, use -EACCES for execbuf fail (Chris), make protected context flag not mandatory in protected object execbuf to avoid abuse (Lionel)

Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Daniel Vetter daniel.vetter@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_context.c | 59 ++++++++++++++++++- drivers/gpu/drm/i915/gem/i915_gem_context.h | 18 ++++++ .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 + .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 18 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp.c | 48 +++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp.h | 1 + drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 + include/uapi/drm/i915_drm.h | 26 ++++++++

Thanks for pointing me at this patch, I overlooked that this here is the uapi.

8 files changed, 172 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 188dee13e017..11b67f1f6e96 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -76,6 +76,8 @@ #include "gt/intel_gpu_commands.h" #include "gt/intel_ring.h"

+#include "pxp/intel_pxp.h"

#include "i915_gem_context.h" #include "i915_globals.h" #include "i915_trace.h" @@ -1971,6 +1973,40 @@ static int set_priority(struct i915_gem_context *ctx, return 0; }

+static int set_protected(struct i915_gem_context *ctx,

• 	 const struct drm_i915_gem_context_param *args)
  

+{

• int ret = 0;
• if (!intel_pxp_is_enabled(&ctx->i915->gt.pxp))

• ret = -ENODEV;
  

• else if (ctx->file_priv) /* can't change this after creation! */

Nah please not. Jason is just now engaging in major surgery to make immutable contexts work properly with the proto-ctx, and it's a huge pain, and only because we put stuff into the magic ctx_setparam when we shouldn't.

Please change the uapi here so that it's impossible by design to set this flag anywhere else than in CREATE_CONTEXT_EXT. See the discussion on the media scalability rfc from Matt Brost (or just talk with Matt directly) for how to do this.

Also please coordinate with Jason's proto-ctx work to make sure you're not too badly colliding with each another.

Thanks, Daniel

• ret = -EEXIST;
  

• else if (args->size)

• ret = -EINVAL;
  

• else if (!args->value)

• clear_bit(UCONTEXT_PROTECTED, &ctx->user_flags);
  

• else if (i915_gem_context_is_recoverable(ctx) ||

•  !i915_gem_context_is_bannable(ctx))
  

• ret = -EPERM;
  

• else

• set_bit(UCONTEXT_PROTECTED, &ctx->user_flags);
  

• return ret;

+}

+static int get_protected(struct i915_gem_context *ctx,

• 	 struct drm_i915_gem_context_param *args)
  

+{

• if (!intel_pxp_is_enabled(&ctx->i915->gt.pxp))

• return -ENODEV;
  

• args->size = 0;
• args->value = i915_gem_context_uses_protected_content(ctx);
• return 0;

+}

static int ctx_setparam(struct drm_i915_file_private *fpriv, struct i915_gem_context *ctx, struct drm_i915_gem_context_param *args) @@ -2003,6 +2039,8 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, ret = -EPERM; else if (args->value) i915_gem_context_set_bannable(ctx);

• else if (i915_gem_context_uses_protected_content(ctx))
  

• 	ret = -EPERM; /* can't clear this for protected contexts */
  

  else i915_gem_context_clear_bannable(ctx); break;

@@ -2010,10 +2048,12 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, case I915_CONTEXT_PARAM_RECOVERABLE: if (args->size) ret = -EINVAL;

• else if (args->value)
  

• 	i915_gem_context_set_recoverable(ctx);
  

• else
  

• else if (!args->value)
  i915_gem_context_clear_recoverable(ctx);
  

• else if (i915_gem_context_uses_protected_content(ctx))
  

• 	ret = -EPERM; /* can't set this for protected contexts */
  

• else
  

• 	i915_gem_context_set_recoverable(ctx);
  

  break;

  case I915_CONTEXT_PARAM_PRIORITY:

@@ -2040,6 +2080,10 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, ret = set_ringsize(ctx, args); break;

• case I915_CONTEXT_PARAM_PROTECTED_CONTENT:

• ret = set_protected(ctx, args);
  

• break;
  

• case I915_CONTEXT_PARAM_BAN_PERIOD: default: ret = -EINVAL;

@@ -2493,6 +2537,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, ret = get_ringsize(ctx, args); break;

• case I915_CONTEXT_PARAM_PROTECTED_CONTENT:

• ret = get_protected(ctx, args);
  

• break;
  

• case I915_CONTEXT_PARAM_BAN_PERIOD: default: ret = -EINVAL;

@@ -2553,6 +2601,11 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, args->batch_active = atomic_read(&ctx->guilty_count); args->batch_pending = atomic_read(&ctx->active_count);

• /* re-use args->flags for output flags */
• args->flags = 0;
• if (i915_gem_context_invalidated(ctx))

• args->flags |= I915_CONTEXT_INVALIDATED;
  

• ret = 0;

out: rcu_read_unlock(); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h index b5c908f3f4f2..169d3fb49252 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h @@ -108,6 +108,24 @@ i915_gem_context_clear_user_engines(struct i915_gem_context *ctx) clear_bit(CONTEXT_USER_ENGINES, &ctx->flags); }

+static inline bool +i915_gem_context_invalidated(const struct i915_gem_context *ctx) +{

• return test_bit(CONTEXT_INVALID, &ctx->flags);

+}

+static inline void +i915_gem_context_set_invalid(struct i915_gem_context *ctx) +{

• set_bit(CONTEXT_INVALID, &ctx->flags);

+}

+static inline bool +i915_gem_context_uses_protected_content(const struct i915_gem_context *ctx) +{

• return test_bit(UCONTEXT_PROTECTED, &ctx->user_flags);

+}

/* i915_gem_context.c */ void i915_gem_init__contexts(struct drm_i915_private *i915);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index 340473aa70de..a0f80475da05 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -134,6 +134,7 @@ struct i915_gem_context { #define UCONTEXT_BANNABLE 2 #define UCONTEXT_RECOVERABLE 3 #define UCONTEXT_PERSISTENCE 4 +#define UCONTEXT_PROTECTED 5

/** * @flags: small set of booleans @@ -141,6 +142,7 @@ struct i915_gem_context { unsigned long flags; #define CONTEXT_CLOSED 0 #define CONTEXT_USER_ENGINES 1 +#define CONTEXT_INVALID 2

struct mutex mutex;

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 297143511f99..a11e9d5767bf 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -21,6 +21,8 @@ #include "gt/intel_gt_pm.h" #include "gt/intel_ring.h"

+#include "pxp/intel_pxp.h"

#include "i915_drv.h" #include "i915_gem_clflush.h" #include "i915_gem_context.h" @@ -746,6 +748,11 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(!ctx)) return -ENOENT;

• if (i915_gem_context_invalidated(ctx)) {

• i915_gem_context_put(ctx);
  

• return -EACCES;
  

• }
• eb->gem_context = ctx; if (rcu_access_pointer(ctx->vm)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;

@@ -2940,6 +2947,17 @@ eb_select_engine(struct i915_execbuffer *eb)

intel_gt_pm_get(ce->engine->gt);

• if (i915_gem_context_uses_protected_content(eb->gem_context)) {

• err = intel_pxp_wait_for_arb_start(&ce->engine->gt->pxp);
  

• if (err)
  

• 	goto err;
  

• if (i915_gem_context_invalidated(eb->gem_context)) {
  

• 	err = -EACCES;
  

• 	goto err;
  

• }
  

• }
• if (!test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) { err = intel_context_alloc_state(ce); if (err)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index 6b0e7170c29b..f713d3423cea 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -7,6 +7,7 @@ #include "intel_pxp_irq.h" #include "intel_pxp_session.h" #include "intel_pxp_tee.h" +#include "gem/i915_gem_context.h" #include "gt/intel_context.h" #include "i915_drv.h"

@@ -164,3 +165,50 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp)

intel_pxp_irq_disable(pxp); }

+void intel_pxp_invalidate(struct intel_pxp *pxp) +{

• struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;
• struct i915_gem_context *ctx, *cn;
• /* ban all contexts marked as protected */
• spin_lock_irq(&i915->gem.contexts.lock);
• list_for_each_entry_safe(ctx, cn, &i915->gem.contexts.list, link) {

• struct i915_gem_engines_iter it;
  

• struct intel_context *ce;
  

• if (!kref_get_unless_zero(&ctx->ref))
  

• 	continue;
  

• if (likely(!i915_gem_context_uses_protected_content(ctx)) ||
  

•     i915_gem_context_invalidated(ctx)) {
  

• 	i915_gem_context_put(ctx);
  

• 	continue;
  

• }
  

• spin_unlock_irq(&i915->gem.contexts.lock);
  

• /*
  

•  * Note that by the time we get here the HW keys are already
  

•  * long gone, so any batch using them that's already on the
  

•  * engines is very likely a lost cause (and it has probably
  

•  * already hung the engine). Therefore, we skip attempting to
  

•  * pull the running context out of the HW and we prioritize
  

•  * bringing the session back as soon as possible.
  

•  */
  

• for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
  

• 	/* only invalidate if at least one ce was allocated */
  

• 	if (test_bit(CONTEXT_ALLOC_BIT, &ce->flags)) {
  

• 		intel_context_set_banned(ce);
  

• 		i915_gem_context_set_invalid(ctx);
  

• 	}
  

• }
  

• i915_gem_context_unlock_engines(ctx);
  

• spin_lock_irq(&i915->gem.contexts.lock);
  

• list_safe_reset_next(ctx, cn, link);
  

• i915_gem_context_put(ctx);
  

• }
• spin_unlock_irq(&i915->gem.contexts.lock);

+}

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 074b3b980957..91c1a2056309 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -33,6 +33,7 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp);

void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp); +void intel_pxp_invalidate(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) { diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index e751122cb24a..e9fe757e368a 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -85,6 +85,9 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp) /* must mark termination in progress calling this function */ GEM_WARN_ON(pxp->arb_is_valid);

• /* invalidate protected objects */
• intel_pxp_invalidate(pxp);
• /* terminate the hw sessions */ ret = intel_pxp_terminate_session(pxp, ARB_SESSION); if (ret) {

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index fc4283c9b87c..3cc33fcbf520 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/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; -- 2.29.2

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Mon, May 24, 2021 at 10:47:58PM -0700, Daniele Ceraolo Spurio wrote:

Now that we can handle destruction and re-creation of the arb session, we can postpone the start of the session to the first submission that requires it, to avoid keeping it running with no user.

Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 ++-- drivers/gpu/drm/i915/pxp/intel_pxp.c | 37 ++++++++++++------- drivers/gpu/drm/i915/pxp/intel_pxp.h | 4 +- drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 2 +- drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 6 +-- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 10 +---- drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 3 ++ 7 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index a11e9d5767bf..c08e28847064 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2948,9 +2948,11 @@ eb_select_engine(struct i915_execbuffer *eb) intel_gt_pm_get(ce->engine->gt);

if (i915_gem_context_uses_protected_content(eb->gem_context)) {

• err = intel_pxp_wait_for_arb_start(&ce->engine->gt->pxp);
  

• if (err)
  

• 	goto err;
  

• if (!intel_pxp_is_active(&ce->engine->gt->pxp)) {
  

• 	err = intel_pxp_start(&ce->engine->gt->pxp);
  

• 	if (err)
  

• 		goto err;
  

• }
  
  

  if (i915_gem_context_invalidated(eb->gem_context)) { err = -EACCES;

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index f713d3423cea..2291c68fd3a0 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -77,6 +77,7 @@ void intel_pxp_init(struct intel_pxp *pxp) init_completion(&pxp->termination); complete_all(&pxp->termination);

• mutex_init(&pxp->arb_mutex); INIT_WORK(&pxp->session_work, intel_pxp_session_work);

  ret = create_vcs_context(pxp);

@@ -113,7 +114,7 @@ void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp) reinit_completion(&pxp->termination); }

-static void intel_pxp_queue_termination(struct intel_pxp *pxp) +static void pxp_queue_termination(struct intel_pxp *pxp) { struct intel_gt *gt = pxp_to_gt(pxp);

@@ -132,31 +133,41 @@ static void intel_pxp_queue_termination(struct intel_pxp *pxp)

• the arb session is restarted from the irq work when we receive the
• termination completion interrupt

*/ -int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +int intel_pxp_start(struct intel_pxp *pxp) {

• int ret = 0;
• if (!intel_pxp_is_enabled(pxp))

• return 0;
  

• return -ENODEV;
  

• mutex_lock(&pxp->arb_mutex);

• if (pxp->arb_is_valid)

• goto unlock;
  

• pxp_queue_termination(pxp);

  if (!wait_for_completion_timeout(&pxp->termination,

• 			 msecs_to_jiffies(100)))
  

• return -ETIMEDOUT;
  

• 			msecs_to_jiffies(100))) {
  

• ret = -ETIMEDOUT;
  

• goto unlock;
  

• }

• /* make sure the compiler doesn't optimize the double access */

• barrier();

  if (!pxp->arb_is_valid)

• return -EIO;
  

• ret = -EIO;
  
  

• return 0;

+unlock:

• mutex_unlock(&pxp->arb_mutex);
• return ret;

}

void intel_pxp_init_hw(struct intel_pxp *pxp) { kcr_pxp_enable(pxp_to_gt(pxp)); intel_pxp_irq_enable(pxp);

• /*

• * the session could've been attacked while we weren't loaded, so
  

• * handle it as if it was and re-create it.
  

• */
  

• intel_pxp_queue_termination(pxp);

}

void intel_pxp_fini_hw(struct intel_pxp *pxp) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 91c1a2056309..1f9871e64096 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -32,7 +32,7 @@ void intel_pxp_init_hw(struct intel_pxp *pxp); void intel_pxp_fini_hw(struct intel_pxp *pxp);

void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); -int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp); +int intel_pxp_start(struct intel_pxp *pxp); void intel_pxp_invalidate(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) @@ -43,7 +43,7 @@ static inline void intel_pxp_fini(struct intel_pxp *pxp) { }

-static inline int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +static inline int intel_pxp_start(struct intel_pxp *pxp) { return 0; } diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c index 196449243515..a230d0034e50 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c @@ -31,7 +31,7 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir) GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) { /* immediately mark PXP as inactive on termination */ intel_pxp_mark_termination_in_progress(pxp);

• pxp->session_events |= PXP_TERMINATION_REQUEST;
  

• pxp->session_events |= PXP_TERMINATION_REQUEST | PXP_INVAL_REQUIRED;
  

  }

  if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index e9fe757e368a..c21620916710 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -85,9 +85,6 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp) /* must mark termination in progress calling this function */ GEM_WARN_ON(pxp->arb_is_valid);

• /* invalidate protected objects */
• intel_pxp_invalidate(pxp);
• /* terminate the hw sessions */ ret = intel_pxp_terminate_session(pxp, ARB_SESSION); if (ret) {

@@ -144,6 +141,9 @@ void intel_pxp_session_work(struct work_struct *work) if (!events) return;

• if (events & PXP_INVAL_REQUIRED)

• intel_pxp_invalidate(pxp);
  

doesn't this invalidation change deserves a separated patch? I'm not sure if I understood why we need this change...

if (events & PXP_TERMINATION_REQUEST) { events &= ~PXP_TERMINATION_COMPLETE; pxp_terminate(pxp); diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index e3d178c44e51..35b3fed4ca2f 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -63,23 +63,15 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp, static int i915_pxp_tee_component_bind(struct device *i915_kdev, struct device *tee_kdev, void *data) {

• struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);

• int ret;

  pxp->pxp_component = data; pxp->pxp_component->tee_dev = tee_kdev;

  /* the component is required to fully start the PXP HW */ intel_pxp_init_hw(pxp);

• ret = intel_pxp_wait_for_arb_start(pxp);

• if (ret) {

• drm_err(&i915->drm, "Failed to create arb session during bind\n");
  

• intel_pxp_fini_hw(pxp);
  

• pxp->pxp_component = NULL;
  

• }

• return ret;

• return 0;

}

static void i915_pxp_tee_component_unbind(struct device *i915_kdev, diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index c059a17cbcfe..b3ae49dd73a8 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -7,6 +7,7 @@ #define __INTEL_PXP_TYPES_H__

#include <linux/completion.h> +#include <linux/mutex.h> #include <linux/types.h> #include <linux/workqueue.h>

@@ -23,6 +24,7 @@ struct intel_pxp { * even if the keys are gone, so we can't rely on the HW state of the * session to know if it's valid and need to track the status in SW. */

• struct mutex arb_mutex; /* protects arb session start */ bool arb_is_valid;

  /*

@@ -40,6 +42,7 @@ struct intel_pxp { u32 session_events; /* protected with gt->irq_lock */ #define PXP_TERMINATION_REQUEST BIT(0) #define PXP_TERMINATION_COMPLETE BIT(1) +#define PXP_INVAL_REQUIRED BIT(2) };

#endif /* __INTEL_PXP_TYPES_H__ */

2.29.2

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Thu, Jun 10, 2021 at 03:44:37PM -0700, Daniele Ceraolo Spurio wrote:

On 6/2/2021 11:14 AM, Rodrigo Vivi wrote:

...

On Mon, May 24, 2021 at 10:47:58PM -0700, Daniele Ceraolo Spurio wrote:

...

Now that we can handle destruction and re-creation of the arb session, we can postpone the start of the session to the first submission that requires it, to avoid keeping it running with no user.

Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com

.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 ++-- drivers/gpu/drm/i915/pxp/intel_pxp.c | 37 ++++++++++++------- drivers/gpu/drm/i915/pxp/intel_pxp.h | 4 +- drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 2 +- drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 6 +-- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 10 +---- drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 3 ++ 7 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index a11e9d5767bf..c08e28847064 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2948,9 +2948,11 @@ eb_select_engine(struct i915_execbuffer *eb) intel_gt_pm_get(ce->engine->gt); if (i915_gem_context_uses_protected_content(eb->gem_context)) {

• err = intel_pxp_wait_for_arb_start(&ce->engine->gt->pxp);
  

• if (err)
  

• 	goto err;
  

• if (!intel_pxp_is_active(&ce->engine->gt->pxp)) {
  

• 	err = intel_pxp_start(&ce->engine->gt->pxp);
  

• 	if (err)
  

• 		goto err;
  

• }
  

  if (i915_gem_context_invalidated(eb->gem_context)) { err = -EACCES;

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index f713d3423cea..2291c68fd3a0 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -77,6 +77,7 @@ void intel_pxp_init(struct intel_pxp *pxp) init_completion(&pxp->termination); complete_all(&pxp->termination);

• mutex_init(&pxp->arb_mutex); INIT_WORK(&pxp->session_work, intel_pxp_session_work); ret = create_vcs_context(pxp);

@@ -113,7 +114,7 @@ void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp) reinit_completion(&pxp->termination); } -static void intel_pxp_queue_termination(struct intel_pxp *pxp) +static void pxp_queue_termination(struct intel_pxp *pxp) { struct intel_gt *gt = pxp_to_gt(pxp); @@ -132,31 +133,41 @@ static void intel_pxp_queue_termination(struct intel_pxp *pxp)

• the arb session is restarted from the irq work when we receive the
• termination completion interrupt

*/ -int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +int intel_pxp_start(struct intel_pxp *pxp) {

• int ret = 0;
• if (!intel_pxp_is_enabled(pxp))

• return 0;
  

• return -ENODEV;
  

• mutex_lock(&pxp->arb_mutex);
• if (pxp->arb_is_valid)

• goto unlock;
  

• pxp_queue_termination(pxp); if (!wait_for_completion_timeout(&pxp->termination,

• 			 msecs_to_jiffies(100)))
  

• return -ETIMEDOUT;
  

• 			msecs_to_jiffies(100))) {
  

• ret = -ETIMEDOUT;
  

• goto unlock;
  

• }
• /* make sure the compiler doesn't optimize the double access */
• barrier(); if (!pxp->arb_is_valid)

• return -EIO;
  

• ret = -EIO;
  

• return 0;

+unlock:

• mutex_unlock(&pxp->arb_mutex);
• return ret; } void intel_pxp_init_hw(struct intel_pxp *pxp) { kcr_pxp_enable(pxp_to_gt(pxp)); intel_pxp_irq_enable(pxp);

• /*

• * the session could've been attacked while we weren't loaded, so
  

• * handle it as if it was and re-create it.
  

• */
  

• intel_pxp_queue_termination(pxp); } void intel_pxp_fini_hw(struct intel_pxp *pxp)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 91c1a2056309..1f9871e64096 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -32,7 +32,7 @@ void intel_pxp_init_hw(struct intel_pxp *pxp); void intel_pxp_fini_hw(struct intel_pxp *pxp); void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); -int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp); +int intel_pxp_start(struct intel_pxp *pxp); void intel_pxp_invalidate(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) @@ -43,7 +43,7 @@ static inline void intel_pxp_fini(struct intel_pxp *pxp) { } -static inline int intel_pxp_wait_for_arb_start(struct intel_pxp *pxp) +static inline int intel_pxp_start(struct intel_pxp *pxp) { return 0; } diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c index 196449243515..a230d0034e50 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c @@ -31,7 +31,7 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir) GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) { /* immediately mark PXP as inactive on termination */ intel_pxp_mark_termination_in_progress(pxp);

• pxp->session_events |= PXP_TERMINATION_REQUEST;
  

• pxp->session_events |= PXP_TERMINATION_REQUEST | PXP_INVAL_REQUIRED;
  

  } if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index e9fe757e368a..c21620916710 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -85,9 +85,6 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp) /* must mark termination in progress calling this function */ GEM_WARN_ON(pxp->arb_is_valid);

• /* invalidate protected objects */
• intel_pxp_invalidate(pxp);
• /* terminate the hw sessions */ ret = intel_pxp_terminate_session(pxp, ARB_SESSION); if (ret) {

@@ -144,6 +141,9 @@ void intel_pxp_session_work(struct work_struct *work) if (!events) return;

• if (events & PXP_INVAL_REQUIRED)

• intel_pxp_invalidate(pxp);
  

doesn't this invalidation change deserves a separated patch? I'm not sure if I understood why we need this change...

Before this patch, we always did a full invalidation and an arb restart

oh, indeed, I had missed that part, sorry

back-to-back and therefore the invalidation was implicit in the PXP_TERMINATION_REQUEST flag. This patch changes this into different cases:

termination irq: invalidate objects, submit termination and restart the arb session suspend: invalidate objects first pxp submission: submit termination and start the arb session

Therefore we need to be able to select the invalidation as an independent case from the termination.

Thanks for the explanation, it makes sense

Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com

Daniele

...

...

if (events & PXP_TERMINATION_REQUEST) { events &= ~PXP_TERMINATION_COMPLETE; pxp_terminate(pxp); diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index e3d178c44e51..35b3fed4ca2f 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -63,23 +63,15 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp, static int i915_pxp_tee_component_bind(struct device *i915_kdev, struct device *tee_kdev, void *data) {

• struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
• int ret; pxp->pxp_component = data; pxp->pxp_component->tee_dev = tee_kdev; /* the component is required to fully start the PXP HW */ intel_pxp_init_hw(pxp);
• ret = intel_pxp_wait_for_arb_start(pxp);
• if (ret) {

• drm_err(&i915->drm, "Failed to create arb session during bind\n");
  

• intel_pxp_fini_hw(pxp);
  

• pxp->pxp_component = NULL;
  

• }
• return ret;

• return 0; } static void i915_pxp_tee_component_unbind(struct device *i915_kdev,

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index c059a17cbcfe..b3ae49dd73a8 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -7,6 +7,7 @@ #define __INTEL_PXP_TYPES_H__ #include <linux/completion.h> +#include <linux/mutex.h> #include <linux/types.h> #include <linux/workqueue.h> @@ -23,6 +24,7 @@ struct intel_pxp { * even if the keys are gone, so we can't rely on the HW state of the * session to know if it's valid and need to track the status in SW. */

• struct mutex arb_mutex; /* protects arb session start */ bool arb_is_valid; /*

@@ -40,6 +42,7 @@ struct intel_pxp { u32 session_events; /* protected with gt->irq_lock */ #define PXP_TERMINATION_REQUEST BIT(0) #define PXP_TERMINATION_COMPLETE BIT(1) +#define PXP_INVAL_REQUIRED BIT(2) };

#endif /* __INTEL_PXP_TYPES_H__ */

2.29.2

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx

On Mon, May 24, 2021 at 10:47:59PM -0700, Daniele Ceraolo Spurio wrote:

From: "Huang, Sean Z" sean.z.huang@intel.com

During the power event S3+ sleep/resume, hardware will lose all the encryption keys for every hardware session, even though the session state might still be marked as alive after resume. Therefore, we should consider the session as dead on suspend and invalidate all the objects. The session will be automatically restarted on the first protected submission on resume.

v2: runtime suspend also invalidates the keys v3: fix return codes, simplify rpm ops (Chris), use the new worker func v4: invalidate the objects on suspend, don't re-create the arb sesson on resume (delayed to first submission).

Signed-off-by: Huang, Sean Z sean.z.huang@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Rodrigo Vivi rodrigo.vivi@intel.com

---

drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt_pm.c | 15 +++++++- drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 11 ++++-- drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 40 ++++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_pm.h | 23 +++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 38 ++++++++++++++----- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 9 +++++ 8 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 29331bbb3e98..9cce0bf9a50f 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -278,6 +278,7 @@ i915-$(CONFIG_DRM_I915_PXP) += \ pxp/intel_pxp.o \ pxp/intel_pxp_cmd.o \ pxp/intel_pxp_irq.o \

• pxp/intel_pxp_pm.o \ pxp/intel_pxp_session.o \ pxp/intel_pxp_tee.o

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index aef3084e8b16..91151a02f7a2 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -19,6 +19,7 @@ #include "intel_rc6.h" #include "intel_rps.h" #include "intel_wakeref.h" +#include "pxp/intel_pxp_pm.h"

static void user_forcewake(struct intel_gt *gt, bool suspend) { @@ -265,6 +266,8 @@ int intel_gt_resume(struct intel_gt *gt)

intel_uc_resume(&gt->uc);

• intel_pxp_resume(&gt->pxp);
• user_forcewake(gt, false);

out_fw: @@ -299,6 +302,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt) user_forcewake(gt, true); wait_for_suspend(gt);

• intel_pxp_suspend(&gt->pxp); intel_uc_suspend(&gt->uc);

}

@@ -349,6 +353,7 @@ void intel_gt_suspend_late(struct intel_gt *gt)

void intel_gt_runtime_suspend(struct intel_gt *gt) {

• intel_pxp_suspend(&gt->pxp); intel_uc_runtime_suspend(&gt->uc);

  GT_TRACE(gt, "\n");

@@ -356,11 +361,19 @@ void intel_gt_runtime_suspend(struct intel_gt *gt)

int intel_gt_runtime_resume(struct intel_gt *gt) {

• int ret;
• GT_TRACE(gt, "\n"); intel_gt_init_swizzling(gt); intel_ggtt_restore_fences(gt->ggtt);

• return intel_uc_runtime_resume(&gt->uc);

• ret = intel_uc_runtime_resume(&gt->uc);
• if (ret)

• return ret;
  

• intel_pxp_resume(&gt->pxp);
• return 0;

}

static ktime_t __intel_gt_get_awake_time(const struct intel_gt *gt) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 2f06bb7b3ed2..6543e5577709 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -68,6 +68,8 @@ #include "gt/intel_gt_pm.h" #include "gt/intel_rc6.h"

+#include "pxp/intel_pxp_pm.h"

#include "i915_debugfs.h" #include "i915_drv.h" #include "i915_ioc32.h" diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c index a230d0034e50..9e5847c653f2 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c @@ -9,6 +9,7 @@ #include "gt/intel_gt_irq.h" #include "i915_irq.h" #include "i915_reg.h" +#include "intel_runtime_pm.h"

/**

• intel_pxp_irq_handler - Handles PXP interrupts.

@@ -62,11 +63,13 @@ void intel_pxp_irq_enable(struct intel_pxp *pxp) struct intel_gt *gt = pxp_to_gt(pxp);

spin_lock_irq(&gt->irq_lock);

• if (!pxp->irq_enabled) {

• if (!pxp->irq_enabled) WARN_ON_ONCE(gen11_gt_reset_one_iir(gt, 0, GEN11_KCR));

• __pxp_set_interrupts(gt, GEN12_PXP_INTERRUPTS);
  

• pxp->irq_enabled = true;
  

• }

• __pxp_set_interrupts(gt, GEN12_PXP_INTERRUPTS);
• pxp->irq_enabled = true;

why? and if we really need this maybe worth a squash on the other patch or a separated new one?

• spin_unlock_irq(&gt->irq_lock);

}

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c new file mode 100644 index 000000000000..400b3a9944c8 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +/*

• • Copyright(c) 2020 Intel Corporation.
• */

+#include "intel_pxp.h" +#include "intel_pxp_irq.h" +#include "intel_pxp_pm.h" +#include "intel_pxp_session.h"

+void intel_pxp_suspend(struct intel_pxp *pxp) +{

• if (!intel_pxp_is_enabled(pxp))

• return;
  

• pxp->arb_is_valid = false;
• /* invalidate protected objects */
• intel_pxp_invalidate(pxp);
• intel_pxp_fini_hw(pxp);
• pxp->global_state_attacked = false;

this attacked var name is really bad :)

and if we are setting this any time we disable, maybe the it would deserve to be in the fini function... but not sure...

+}

+void intel_pxp_resume(struct intel_pxp *pxp) +{

• if (!intel_pxp_is_enabled(pxp))

• return;
  

• /*

• * The PXP component gets automatically unbound when we go into S3 and
  

• * re-bound after we come out, so in that scenario we can defer the
  

• * hw init to the bind call.
  

• */
  

• if (!pxp->pxp_component)

• return;
  

• intel_pxp_init_hw(pxp);

+} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h new file mode 100644 index 000000000000..6f488789db6a --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: MIT */

// SPDX-License-Identifier: MIT

+/*

• • Copyright(c) 2020, Intel Corporation. All rights reserved.
• */

+#ifndef __INTEL_PXP_PM_H__ +#define __INTEL_PXP_PM_H__

+#include "i915_drv.h"

+#ifdef CONFIG_DRM_I915_PXP +void intel_pxp_suspend(struct intel_pxp *pxp); +void intel_pxp_resume(struct intel_pxp *pxp); +#else +static inline void intel_pxp_suspend(struct intel_pxp *pxp) +{ +} +static inline void intel_pxp_resume(struct intel_pxp *pxp) +{ +} +#endif

+#endif /* __INTEL_PXP_PM_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index c21620916710..893a2bf61b5c 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -21,29 +21,36 @@

static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id) {

• struct intel_gt *gt = pxp_to_gt(pxp);

• struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore; intel_wakeref_t wakeref; u32 sip = 0;

• with_intel_runtime_pm(gt->uncore->rpm, wakeref)

• sip = intel_uncore_read(gt->uncore, GEN12_KCR_SIP);
  

• /* if we're suspended the session is considered off */

• with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)

• sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
  
  

  return sip & BIT(id);

}

static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_play) {

• struct intel_gt *gt = pxp_to_gt(pxp);

• struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore; intel_wakeref_t wakeref; u32 mask = BIT(id); int ret;

• with_intel_runtime_pm(gt->uncore->rpm, wakeref)

• ret = intel_wait_for_register(gt->uncore,
  

• 			      GEN12_KCR_SIP,
  

• 			      mask,
  

• 			      in_play ? mask : 0,
  

• 			      100);
  

• /* if we're suspended the session is considered off */

• wakeref = intel_runtime_pm_get_if_in_use(uncore->rpm);

• if (!wakeref)

• return in_play ? -ENODEV : 0;
  

• ret = intel_wait_for_register(uncore,

• 		      GEN12_KCR_SIP,
  

• 		      mask,
  

• 		      in_play ? mask : 0,
  

• 		      100);
  

• intel_runtime_pm_put(uncore->rpm, wakeref);

  return ret;

} @@ -132,6 +139,7 @@ void intel_pxp_session_work(struct work_struct *work) { struct intel_pxp *pxp = container_of(work, typeof(*pxp), session_work); struct intel_gt *gt = pxp_to_gt(pxp);

• intel_wakeref_t wakeref; u32 events = 0;

  spin_lock_irq(&gt->irq_lock);

@@ -144,6 +152,14 @@ void intel_pxp_session_work(struct work_struct *work) if (events & PXP_INVAL_REQUIRED) intel_pxp_invalidate(pxp);

• /*

• * If we're processing an event while suspending then don't bother,
  

• * we're going to re-init everything on resume anyway.
  

• */
  

• wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
• if (!wakeref)

• return;
  

• if (events & PXP_TERMINATION_REQUEST) { events &= ~PXP_TERMINATION_COMPLETE; pxp_terminate(pxp);

@@ -151,4 +167,6 @@ void intel_pxp_session_work(struct work_struct *work)

if (events & PXP_TERMINATION_COMPLETE) pxp_terminate_complete(pxp);

• intel_runtime_pm_put(gt->uncore->rpm, wakeref);

} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index 35b3fed4ca2f..0b6098a7b75c 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -63,14 +63,23 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp, static int i915_pxp_tee_component_bind(struct device *i915_kdev, struct device *tee_kdev, void *data) {

• struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);

• intel_wakeref_t wakeref;

  pxp->pxp_component = data; pxp->pxp_component->tee_dev = tee_kdev;

• /* if we are suspended, the HW will be re-initialized on resume */

• wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);

• if (!wakeref)

• return 0;
  

• /* the component is required to fully start the PXP HW */ intel_pxp_init_hw(pxp);

• intel_runtime_pm_put(&i915->runtime_pm, wakeref);

• return 0;

}

-- 2.29.2

On Thu, Jun 10, 2021 at 03:58:13PM -0700, Daniele Ceraolo Spurio wrote:

On 6/2/2021 9:20 AM, Rodrigo Vivi wrote:

...

On Mon, May 24, 2021 at 10:47:59PM -0700, Daniele Ceraolo Spurio wrote:

...

From: "Huang, Sean Z" sean.z.huang@intel.com

During the power event S3+ sleep/resume, hardware will lose all the encryption keys for every hardware session, even though the session state might still be marked as alive after resume. Therefore, we should consider the session as dead on suspend and invalidate all the objects. The session will be automatically restarted on the first protected submission on resume.

v2: runtime suspend also invalidates the keys v3: fix return codes, simplify rpm ops (Chris), use the new worker func v4: invalidate the objects on suspend, don't re-create the arb sesson on resume (delayed to first submission).

Signed-off-by: Huang, Sean Z sean.z.huang@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Rodrigo Vivi rodrigo.vivi@intel.com

---

drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt_pm.c | 15 +++++++- drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 11 ++++-- drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 40 ++++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_pm.h | 23 +++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 38 ++++++++++++++----- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 9 +++++ 8 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.c create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_pm.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 29331bbb3e98..9cce0bf9a50f 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -278,6 +278,7 @@ i915-$(CONFIG_DRM_I915_PXP) += \ pxp/intel_pxp.o \ pxp/intel_pxp_cmd.o \ pxp/intel_pxp_irq.o \

• pxp/intel_pxp_pm.o \ pxp/intel_pxp_session.o \ pxp/intel_pxp_tee.o

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index aef3084e8b16..91151a02f7a2 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -19,6 +19,7 @@ #include "intel_rc6.h" #include "intel_rps.h" #include "intel_wakeref.h" +#include "pxp/intel_pxp_pm.h" static void user_forcewake(struct intel_gt *gt, bool suspend) { @@ -265,6 +266,8 @@ int intel_gt_resume(struct intel_gt *gt) intel_uc_resume(&gt->uc);

• intel_pxp_resume(&gt->pxp);
• user_forcewake(gt, false); out_fw:

@@ -299,6 +302,7 @@ void intel_gt_suspend_prepare(struct intel_gt *gt) user_forcewake(gt, true); wait_for_suspend(gt);

• intel_pxp_suspend(&gt->pxp); intel_uc_suspend(&gt->uc); }

@@ -349,6 +353,7 @@ void intel_gt_suspend_late(struct intel_gt *gt) void intel_gt_runtime_suspend(struct intel_gt *gt) {

• intel_pxp_suspend(&gt->pxp); intel_uc_runtime_suspend(&gt->uc); GT_TRACE(gt, "\n");

@@ -356,11 +361,19 @@ void intel_gt_runtime_suspend(struct intel_gt *gt) int intel_gt_runtime_resume(struct intel_gt *gt) {

• int ret;
• GT_TRACE(gt, "\n"); intel_gt_init_swizzling(gt); intel_ggtt_restore_fences(gt->ggtt);

• return intel_uc_runtime_resume(&gt->uc);

• ret = intel_uc_runtime_resume(&gt->uc);
• if (ret)

• return ret;
  

• intel_pxp_resume(&gt->pxp);
• return 0; } static ktime_t __intel_gt_get_awake_time(const struct intel_gt *gt)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 2f06bb7b3ed2..6543e5577709 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -68,6 +68,8 @@ #include "gt/intel_gt_pm.h" #include "gt/intel_rc6.h" +#include "pxp/intel_pxp_pm.h"

• #include "i915_debugfs.h" #include "i915_drv.h" #include "i915_ioc32.h"

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c index a230d0034e50..9e5847c653f2 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c @@ -9,6 +9,7 @@ #include "gt/intel_gt_irq.h" #include "i915_irq.h" #include "i915_reg.h" +#include "intel_runtime_pm.h" /**

• intel_pxp_irq_handler - Handles PXP interrupts.

@@ -62,11 +63,13 @@ void intel_pxp_irq_enable(struct intel_pxp *pxp) struct intel_gt *gt = pxp_to_gt(pxp); spin_lock_irq(&gt->irq_lock);

• if (!pxp->irq_enabled) {

• if (!pxp->irq_enabled) WARN_ON_ONCE(gen11_gt_reset_one_iir(gt, 0, GEN11_KCR));

• __pxp_set_interrupts(gt, GEN12_PXP_INTERRUPTS);
  

• pxp->irq_enabled = true;
  

• }

• __pxp_set_interrupts(gt, GEN12_PXP_INTERRUPTS);
• pxp->irq_enabled = true;

why? and if we really need this maybe worth a squash on the other patch or a separated new one?

I had some troubles with the driver resetting all interrupts on S3 exit behind the PXP code, so wanted to make sure we always enabled them when we entered this function.  I can squash this change in the patch that adds the irq logic if you think that's better.

yeap, please... squash there or a separated patch explaining your issue.

...

...

• spin_unlock_irq(&gt->irq_lock); }

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c new file mode 100644 index 000000000000..400b3a9944c8 --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +/*

• • Copyright(c) 2020 Intel Corporation.
• */

+#include "intel_pxp.h" +#include "intel_pxp_irq.h" +#include "intel_pxp_pm.h" +#include "intel_pxp_session.h"

+void intel_pxp_suspend(struct intel_pxp *pxp) +{

• if (!intel_pxp_is_enabled(pxp))

• return;
  

• pxp->arb_is_valid = false;
• /* invalidate protected objects */
• intel_pxp_invalidate(pxp);
• intel_pxp_fini_hw(pxp);
• pxp->global_state_attacked = false;

this attacked var name is really bad :)

pxp->hw_state_invalidated?

I like this more... but up to you!

...

and if we are setting this any time we disable, maybe the it would deserve to be in the fini function... but not sure...

I'd prefer to keep it out of the fini_hw, but I'll double check if I can wrap it up more nicely.

ack!

...

...

+}

+void intel_pxp_resume(struct intel_pxp *pxp) +{

• if (!intel_pxp_is_enabled(pxp))

• return;
  

• /*

• * The PXP component gets automatically unbound when we go into S3 and
  

• * re-bound after we come out, so in that scenario we can defer the
  

• * hw init to the bind call.
  

• */
  

• if (!pxp->pxp_component)

• return;
  

• intel_pxp_init_hw(pxp);

+} diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h new file mode 100644 index 000000000000..6f488789db6a --- /dev/null +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: MIT */

// SPDX-License-Identifier: MIT

Don't we use /* for SPDX in headers?

Duh! (facepalm) you are right /**/ for .h and // for .c https://www.kernel.org/doc/html/latest/process/license-rules.html

sorry

Daniele

...

...

+/*

• • Copyright(c) 2020, Intel Corporation. All rights reserved.
• */

+#ifndef __INTEL_PXP_PM_H__ +#define __INTEL_PXP_PM_H__

+#include "i915_drv.h"

+#ifdef CONFIG_DRM_I915_PXP +void intel_pxp_suspend(struct intel_pxp *pxp); +void intel_pxp_resume(struct intel_pxp *pxp); +#else +static inline void intel_pxp_suspend(struct intel_pxp *pxp) +{ +} +static inline void intel_pxp_resume(struct intel_pxp *pxp) +{ +} +#endif

+#endif /* __INTEL_PXP_PM_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index c21620916710..893a2bf61b5c 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -21,29 +21,36 @@ static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id) {

• struct intel_gt *gt = pxp_to_gt(pxp);

• struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore; intel_wakeref_t wakeref; u32 sip = 0;

• with_intel_runtime_pm(gt->uncore->rpm, wakeref)

• sip = intel_uncore_read(gt->uncore, GEN12_KCR_SIP);
  

• /* if we're suspended the session is considered off */
• with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)

• sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
  

  return sip & BIT(id); } static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_play) {

• struct intel_gt *gt = pxp_to_gt(pxp);

• struct intel_uncore *uncore = pxp_to_gt(pxp)->uncore; intel_wakeref_t wakeref; u32 mask = BIT(id); int ret;

• with_intel_runtime_pm(gt->uncore->rpm, wakeref)

• ret = intel_wait_for_register(gt->uncore,
  

• 			      GEN12_KCR_SIP,
  

• 			      mask,
  

• 			      in_play ? mask : 0,
  

• 			      100);
  

• /* if we're suspended the session is considered off */
• wakeref = intel_runtime_pm_get_if_in_use(uncore->rpm);
• if (!wakeref)

• return in_play ? -ENODEV : 0;
  

• ret = intel_wait_for_register(uncore,

• 		      GEN12_KCR_SIP,
  

• 		      mask,
  

• 		      in_play ? mask : 0,
  

• 		      100);
  

• intel_runtime_pm_put(uncore->rpm, wakeref); return ret; }

@@ -132,6 +139,7 @@ void intel_pxp_session_work(struct work_struct *work) { struct intel_pxp *pxp = container_of(work, typeof(*pxp), session_work); struct intel_gt *gt = pxp_to_gt(pxp);

• intel_wakeref_t wakeref; u32 events = 0; spin_lock_irq(&gt->irq_lock);

@@ -144,6 +152,14 @@ void intel_pxp_session_work(struct work_struct *work) if (events & PXP_INVAL_REQUIRED) intel_pxp_invalidate(pxp);

• /*

• * If we're processing an event while suspending then don't bother,
  

• * we're going to re-init everything on resume anyway.
  

• */
  

• wakeref = intel_runtime_pm_get_if_in_use(gt->uncore->rpm);
• if (!wakeref)

• return;
  

• if (events & PXP_TERMINATION_REQUEST) { events &= ~PXP_TERMINATION_COMPLETE; pxp_terminate(pxp);

@@ -151,4 +167,6 @@ void intel_pxp_session_work(struct work_struct *work) if (events & PXP_TERMINATION_COMPLETE) pxp_terminate_complete(pxp);

• intel_runtime_pm_put(gt->uncore->rpm, wakeref); }

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index 35b3fed4ca2f..0b6098a7b75c 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -63,14 +63,23 @@ static int intel_pxp_tee_io_message(struct intel_pxp *pxp, static int i915_pxp_tee_component_bind(struct device *i915_kdev, struct device *tee_kdev, void *data) {

• struct drm_i915_private *i915 = kdev_to_i915(i915_kdev); struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
• intel_wakeref_t wakeref; pxp->pxp_component = data; pxp->pxp_component->tee_dev = tee_kdev;
• /* if we are suspended, the HW will be re-initialized on resume */
• wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);
• if (!wakeref)

• return 0;
  

• /* the component is required to fully start the PXP HW */ intel_pxp_init_hw(pxp);
• intel_runtime_pm_put(&i915->runtime_pm, wakeref);
• return 0; }

-- 2.29.2

On Mon, May 24, 2021 at 10:48:00PM -0700, Daniele Ceraolo Spurio wrote:

From: Bommu Krishnaiah krishnaiah.bommu@intel.com

This api allow user mode to create Protected buffers. Only contexts marked as protected are allowed to operate on protected buffers.

We only allow setting the flags at creation time.

All protected objects that have backing storage will be considered invalid when the session is destroyed and they won't be usable anymore.

Given that the PXP HW supports multiple modes (but we currently only care about one), a flag variable has been reserved in the structure used in the create_ext ioctl for possible future updates.

This is a rework of the original code by Bommu Krishnaiah. I've kept authorship unchanged since significant chunks have not been modified.

v2: split context changes, fix defines and improve documentation (Chris), add object invalidation logic v3: fix spinlock definition and usage, only validate objects when they're first added to a context lut, only remove them once (Chris), make protected context flag not mandatory in protected object execbuf to avoid abuse (Lionel) v4: rebase to new gem_create_ext

Signed-off-by: Bommu Krishnaiah krishnaiah.bommu@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Telukuntla Sreedhar sreedhar.telukuntla@intel.com Cc: Kondapally Kalyan kalyan.kondapally@intel.com Cc: Gupta Anshuman Anshuman.Gupta@intel.com Cc: Huang Sean Z sean.z.huang@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Daniel Vetter daniel.vetter@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_create.c | 26 ++++++++++++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 15 +++++++ drivers/gpu/drm/i915/gem/i915_gem_object.c | 6 +++ drivers/gpu/drm/i915/gem/i915_gem_object.h | 12 ++++++ .../gpu/drm/i915/gem/i915_gem_object_types.h | 13 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp.c | 41 +++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp.h | 13 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 5 +++ include/uapi/drm/i915_drm.h | 33 ++++++++++++++- 9 files changed, 163 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 548ddf39d853..c14be3882c35 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -6,6 +6,7 @@ #include "gem/i915_gem_ioctls.h" #include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h" +#include "pxp/intel_pxp.h"

#include "i915_drv.h" #include "i915_trace.h" @@ -99,7 +100,11 @@ i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)

GEM_BUG_ON(size != obj->base.size);

• if (obj->user_flags & I915_GEM_OBJECT_PROTECTED)

• intel_pxp_object_add(obj);
  

• trace_i915_gem_object_create(obj);
• return 0;

}

@@ -344,8 +349,29 @@ static int ext_set_placements(struct i915_user_extension __user *base, return set_placements(&ext, data); }

+static int ext_set_protected(struct i915_user_extension __user *base, void *data) +{

• struct drm_i915_gem_create_ext_protected_content ext;
• struct create_ext *ext_data = data;
• if (copy_from_user(&ext, base, sizeof(ext)))

• return -EFAULT;
  

• if (ext.flags)

• return -EINVAL;
  

• if (!intel_pxp_is_enabled(&ext_data->i915->gt.pxp))

• return -ENODEV;
  

• ext_data->vanilla_object->user_flags |= I915_GEM_OBJECT_PROTECTED;
• return 0;

+}

static const i915_user_extension_fn create_extensions[] = { [I915_GEM_CREATE_EXT_MEMORY_REGIONS] = ext_set_placements,

• [I915_GEM_CREATE_EXT_PROTECTED_CONTENT] = ext_set_protected,

};

/** diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index c08e28847064..5dd813d04a9f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -839,6 +839,21 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle) if (unlikely(!obj)) return ERR_PTR(-ENOENT);

• /*
  

•  * If the user has opted-in for protected-object tracking, make
  

•  * sure the object encryption can be used.
  

•  * We only need to do this when the object is first used with
  

•  * this context, because the context itself will be banned when
  

•  * the protected objects become invalid.
  

•  */
  

• if (i915_gem_context_uses_protected_content(eb->gem_context) &&
  

•     i915_gem_object_is_protected(obj)) {
  

• 	if (!intel_pxp_is_active(&vm->gt->pxp))
  

• 		return ERR_PTR(-ENODEV);
  

• 	if (!i915_gem_object_has_valid_protection(obj))
  

• 		return ERR_PTR(-ENOEXEC);
  

• }
  

• vma = i915_vma_instance(obj, vm, NULL); if (IS_ERR(vma)) { i915_gem_object_put(obj);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 28144410df86..b47fa0a7b25a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -25,6 +25,7 @@ #include <linux/sched/mm.h>

#include "display/intel_frontbuffer.h" +#include "pxp/intel_pxp.h" #include "i915_drv.h" #include "i915_gem_clflush.h" #include "i915_gem_context.h" @@ -70,6 +71,8 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&obj->lut_list); spin_lock_init(&obj->lut_lock);

• INIT_LIST_HEAD(&obj->pxp_link);
• spin_lock_init(&obj->mmo.lock); obj->mmo.offsets = RB_ROOT;

@@ -232,6 +235,9 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, spin_unlock(&obj->vma.lock); }

• if (i915_gem_object_has_valid_protection(obj))
  

• 	intel_pxp_object_remove(obj);
  

• __i915_gem_object_free_mmaps(obj);

  GEM_BUG_ON(!list_empty(&obj->lut_list));

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 2ebd79537aea..61b101560352 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -288,6 +288,18 @@ i915_gem_object_never_mmap(const struct drm_i915_gem_object *obj) return i915_gem_object_type_has(obj, I915_GEM_OBJECT_NO_MMAP); }

+static inline bool +i915_gem_object_is_protected(const struct drm_i915_gem_object *obj) +{

• return obj->user_flags & I915_GEM_OBJECT_PROTECTED;

+}

+static inline bool +i915_gem_object_has_valid_protection(const struct drm_i915_gem_object *obj) +{

• return i915_gem_object_is_protected(obj) && !list_empty(&obj->pxp_link);

+}

static inline bool i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 0727d0c76aa0..a698ad0ef7f6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -167,6 +167,11 @@ struct drm_i915_gem_object { } mmo;

I915_SELFTEST_DECLARE(struct list_head st_link);

• /**

• * @user_flags: small set of booleans set by the user
  

• */
  

• unsigned long user_flags;

+#define I915_GEM_OBJECT_PROTECTED BIT(0)

unsigned long flags; #define I915_BO_ALLOC_CONTIGUOUS BIT(0) @@ -294,6 +299,14 @@ struct drm_i915_gem_object { bool dirty:1; } mm;

• /*

• * When the PXP session is invalidated, we need to mark all protected
  

• * objects as invalid. To easily do so we add them all to a list. The
  

• * presence on the list is used to check if the encryption is valid or
  

• * not.
  

• */
  

• struct list_head pxp_link;
• /** Record of address bit 17 of each page at last unbind. */ unsigned long *bit_17;

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index 2291c68fd3a0..e6a59eb05eae 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -69,6 +69,9 @@ void intel_pxp_init(struct intel_pxp *pxp) if (!HAS_PXP(gt->i915)) return;

• spin_lock_init(&pxp->lock);
• INIT_LIST_HEAD(&pxp->protected_objects);
• /*
  • we'll use the completion to check if there is a termination pending,
  • so we start it as completed and we reinit it when a termination

@@ -177,11 +180,49 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp) intel_pxp_irq_disable(pxp); }

+int intel_pxp_object_add(struct drm_i915_gem_object *obj) +{

• struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
• if (!intel_pxp_is_enabled(pxp))

• return -ENODEV;
  

• if (!list_empty(&obj->pxp_link))

• return -EEXIST;
  

• spin_lock_irq(&pxp->lock);
• list_add(&obj->pxp_link, &pxp->protected_objects);
• spin_unlock_irq(&pxp->lock);
• return 0;

+}

+void intel_pxp_object_remove(struct drm_i915_gem_object *obj) +{

• struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
• if (!intel_pxp_is_enabled(pxp))

• return;
  

• spin_lock_irq(&pxp->lock);
• list_del_init(&obj->pxp_link);
• spin_unlock_irq(&pxp->lock);

+}

void intel_pxp_invalidate(struct intel_pxp *pxp) { struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;

• struct drm_i915_gem_object *obj, *tmp; struct i915_gem_context *ctx, *cn;

• /* delete objects that have been used with the invalidated session */

• spin_lock_irq(&pxp->lock);

• list_for_each_entry_safe(obj, tmp, &pxp->protected_objects, pxp_link) {

• if (i915_gem_object_has_pages(obj))
  

• 	list_del_init(&obj->pxp_link);
  

• }

• spin_unlock_irq(&pxp->lock);

• /* ban all contexts marked as protected */ spin_lock_irq(&i915->gem.contexts.lock); list_for_each_entry_safe(ctx, cn, &i915->gem.contexts.list, link) {

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 1f9871e64096..3500d7896058 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -9,6 +9,8 @@ #include "gt/intel_gt_types.h" #include "intel_pxp_types.h"

+struct drm_i915_gem_object;

static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) { return container_of(pxp, struct intel_gt, pxp); @@ -33,6 +35,9 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp);

void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); int intel_pxp_start(struct intel_pxp *pxp);

+int intel_pxp_object_add(struct drm_i915_gem_object *obj); +void intel_pxp_object_remove(struct drm_i915_gem_object *obj); void intel_pxp_invalidate(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) @@ -47,6 +52,14 @@ static inline int intel_pxp_start(struct intel_pxp *pxp) { return 0; }

+static inline int intel_pxp_object_add(struct drm_i915_gem_object *obj) +{

• return 0;

+} +static inline void intel_pxp_object_remove(struct drm_i915_gem_object *obj) +{ +} #endif

#endif /* __INTEL_PXP_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index b3ae49dd73a8..cc510416eac6 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -7,7 +7,9 @@ #define __INTEL_PXP_TYPES_H__

#include <linux/completion.h> +#include <linux/list.h> #include <linux/mutex.h> +#include <linux/spinlock.h> #include <linux/types.h> #include <linux/workqueue.h>

@@ -43,6 +45,9 @@ struct intel_pxp { #define PXP_TERMINATION_REQUEST BIT(0) #define PXP_TERMINATION_COMPLETE BIT(1) #define PXP_INVAL_REQUIRED BIT(2)

• spinlock_t lock; /* protects the objects list */
• struct list_head protected_objects;

};

#endif /* __INTEL_PXP_TYPES_H__ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 3cc33fcbf520..d39b8cb4bfc5 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h

For easier review it would be nice to have all the uapi and its kerneldoc in one patch. I feel like I've missed parts.

@@ -1768,7 +1768,7 @@ struct drm_i915_gem_context_param { __u64 value; };

-/* +/**

• Context SSEU programming
• It may be necessary for either functional or performance reason to configure

@@ -2669,8 +2669,12 @@ struct drm_i915_gem_create_ext { * * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see * struct drm_i915_gem_create_ext_memory_regions.

• *
  

• * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
  

• * struct drm_i915_gem_create_ext_protected_content.
  

  */

#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1 __u64 extensions; };

@@ -2728,6 +2732,33 @@ struct drm_i915_gem_create_ext_memory_regions { __u64 regions; };

+/**

• • struct drm_i915_gem_create_ext_protected_content - The
• • I915_OBJECT_PARAM_PROTECTED_CONTENT extension.
• • If this extension is provided, buffer contents are 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 ysing this extension will cause the ioctl to fail
• • and return -ENODEV. The flags parameter is reserved for future expansion and
• • must currently be set to zero.
• • The buffer contents are considered invalid after a PXP session teardown.
• • The encryption is guaranteed to be processed correctly only if the object
• • is submitted with a context created using the
• • I915_CONTEXT_PARAM_PROTECTED_CONTENT flag. This will also enable extra checks

Not finding the kerneldoc for this. Also is this a CTX_CREATE_EXT flag, so guaranteed to be immutable, or is this another CTX_SETPARAM flag, which would be mutable and we really want to get away from that paradigm?

• • at submission time on the validity of the objects involved, which can lead to
• • the following errors being returned from the execbuf ioctl:

I think this also should be put into the execbuf ioctl structs. Plus I think RESET_STATS should also explain how this interacts and all that. Here we should just reference this other sections imo.

Yes this means a bit more to type because not even the bare bones of these other ioctl kerneldocs exists, but we have to start somewhere.

Cheers, Daniel

• • -ENODEV: PXP session not currently active
• • -ENOEXEC: 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

-- 2.29.2

-----Original Message----- From: Intel-gfx intel-gfx-bounces@lists.freedesktop.org On Behalf Of Daniele Ceraolo Spurio Sent: Monday, May 24, 2021 10:48 PM To: intel-gfx@lists.freedesktop.org Cc: Vetter, Daniel daniel.vetter@intel.com; Huang Sean Z sean.z.huang@intel.com; dri-devel@lists.freedesktop.org; Chris Wilson chris@chris-wilson.co.uk; Kondapally Kalyan kalyan.kondapally@intel.com; Bommu, Krishnaiah krishnaiah.bommu@intel.com Subject: [Intel-gfx] [PATCH v4 14/17] drm/i915/pxp: User interface for Protected buffer

From: Bommu Krishnaiah krishnaiah.bommu@intel.com

This api allow user mode to create Protected buffers. Only contexts marked as protected are allowed to operate on protected buffers.

We only allow setting the flags at creation time.

All protected objects that have backing storage will be considered invalid when the session is destroyed and they won't be usable anymore.

Then these protected objects will be hanging in the system till user call gem_close() to free them? If the objects won't be usable anymore, why don't we automatically free these objects when the session is destroyed?

How is a session started/destroyed? From the code, intel_pxp_init() is called when loading i915 driver, so I think session lifetime is the same as i915 driver lifetime. Can we start multiple sessions after loading the driver?

--CQ

Given that the PXP HW supports multiple modes (but we currently only care about one), a flag variable has been reserved in the structure used in the create_ext ioctl for possible future updates.

This is a rework of the original code by Bommu Krishnaiah. I've kept authorship unchanged since significant chunks have not been modified.

v2: split context changes, fix defines and improve documentation (Chris), add object invalidation logic v3: fix spinlock definition and usage, only validate objects when they're first added to a context lut, only remove them once (Chris), make protected context flag not mandatory in protected object execbuf to avoid abuse (Lionel) v4: rebase to new gem_create_ext

Signed-off-by: Bommu Krishnaiah krishnaiah.bommu@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Telukuntla Sreedhar sreedhar.telukuntla@intel.com Cc: Kondapally Kalyan kalyan.kondapally@intel.com Cc: Gupta Anshuman Anshuman.Gupta@intel.com Cc: Huang Sean Z sean.z.huang@intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: Daniel Vetter daniel.vetter@intel.com

---

drivers/gpu/drm/i915/gem/i915_gem_create.c | 26 ++++++++++++ .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 15 +++++++ drivers/gpu/drm/i915/gem/i915_gem_object.c | 6 +++ drivers/gpu/drm/i915/gem/i915_gem_object.h | 12 ++++++ .../gpu/drm/i915/gem/i915_gem_object_types.h | 13 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp.c | 41 +++++++++++++++++++ drivers/gpu/drm/i915/pxp/intel_pxp.h | 13 ++++++ drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 5 +++ include/uapi/drm/i915_drm.h | 33 ++++++++++++++- 9 files changed, 163 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 548ddf39d853..c14be3882c35 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -6,6 +6,7 @@ #include "gem/i915_gem_ioctls.h" #include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h" +#include "pxp/intel_pxp.h"

#include "i915_drv.h" #include "i915_trace.h" @@ -99,7 +100,11 @@ i915_gem_setup(struct drm_i915_gem_object *obj, u64 size)

GEM_BUG_ON(size != obj->base.size);

• if (obj->user_flags & I915_GEM_OBJECT_PROTECTED)

• intel_pxp_object_add(obj);
  

• trace_i915_gem_object_create(obj);
• return 0;

}

@@ -344,8 +349,29 @@ static int ext_set_placements(struct i915_user_extension __user *base, return set_placements(&ext, data); }

+static int ext_set_protected(struct i915_user_extension __user *base, +void *data) {

• struct drm_i915_gem_create_ext_protected_content ext;
• struct create_ext *ext_data = data;
• if (copy_from_user(&ext, base, sizeof(ext)))

• return -EFAULT;
  

• if (ext.flags)

• return -EINVAL;
  

• if (!intel_pxp_is_enabled(&ext_data->i915->gt.pxp))

• return -ENODEV;
  

• ext_data->vanilla_object->user_flags |=

I915_GEM_OBJECT_PROTECTED;

• return 0;

+}

static const i915_user_extension_fn create_extensions[] = { [I915_GEM_CREATE_EXT_MEMORY_REGIONS] = ext_set_placements,

• [I915_GEM_CREATE_EXT_PROTECTED_CONTENT] =

ext_set_protected, };

/** diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index c08e28847064..5dd813d04a9f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -839,6 +839,21 @@ static struct i915_vma *eb_lookup_vma(struct i915_execbuffer *eb, u32 handle) if (unlikely(!obj)) return ERR_PTR(-ENOENT);

• /*
  

•  * If the user has opted-in for protected-object tracking,
  

make

•  * sure the object encryption can be used.
  

•  * We only need to do this when the object is first used with
  

•  * this context, because the context itself will be banned
  

when

•  * the protected objects become invalid.
  

•  */
  

• if (i915_gem_context_uses_protected_content(eb-
  

...

gem_context) &&

•     i915_gem_object_is_protected(obj)) {
  

• 	if (!intel_pxp_is_active(&vm->gt->pxp))
  

• 		return ERR_PTR(-ENODEV);
  

• 	if (!i915_gem_object_has_valid_protection(obj))
  

• 		return ERR_PTR(-ENOEXEC);
  

• }
  

• vma = i915_vma_instance(obj, vm, NULL); if (IS_ERR(vma)) { i915_gem_object_put(obj);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index 28144410df86..b47fa0a7b25a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -25,6 +25,7 @@ #include <linux/sched/mm.h>

#include "display/intel_frontbuffer.h" +#include "pxp/intel_pxp.h" #include "i915_drv.h" #include "i915_gem_clflush.h" #include "i915_gem_context.h" @@ -70,6 +71,8 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&obj->lut_list); spin_lock_init(&obj->lut_lock);

• INIT_LIST_HEAD(&obj->pxp_link);
• spin_lock_init(&obj->mmo.lock); obj->mmo.offsets = RB_ROOT;

@@ -232,6 +235,9 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, spin_unlock(&obj->vma.lock); }

• if (i915_gem_object_has_valid_protection(obj))
  

• 	intel_pxp_object_remove(obj);
  

• __i915_gem_object_free_mmaps(obj);

  GEM_BUG_ON(!list_empty(&obj->lut_list));

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 2ebd79537aea..61b101560352 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -288,6 +288,18 @@ i915_gem_object_never_mmap(const struct drm_i915_gem_object *obj) return i915_gem_object_type_has(obj, I915_GEM_OBJECT_NO_MMAP); }

+static inline bool +i915_gem_object_is_protected(const struct drm_i915_gem_object *obj) {

• return obj->user_flags & I915_GEM_OBJECT_PROTECTED; }

+static inline bool +i915_gem_object_has_valid_protection(const struct drm_i915_gem_object +*obj) {

• return i915_gem_object_is_protected(obj) &&

+!list_empty(&obj->pxp_link); }

static inline bool i915_gem_object_is_framebuffer(const struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 0727d0c76aa0..a698ad0ef7f6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -167,6 +167,11 @@ struct drm_i915_gem_object { } mmo;

I915_SELFTEST_DECLARE(struct list_head st_link);

• /**

• * @user_flags: small set of booleans set by the user
  

• */
  

• unsigned long user_flags;

+#define I915_GEM_OBJECT_PROTECTED BIT(0)

unsigned long flags; #define I915_BO_ALLOC_CONTIGUOUS BIT(0) @@ -294,6 +299,14 @@ struct drm_i915_gem_object { bool dirty:1; } mm;

• /*

• * When the PXP session is invalidated, we need to mark all
  

protected

• * objects as invalid. To easily do so we add them all to a list. The
  

• * presence on the list is used to check if the encryption is valid or
  

• * not.
  

• */
  

• struct list_head pxp_link;
• /** Record of address bit 17 of each page at last unbind. */ unsigned long *bit_17;

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index 2291c68fd3a0..e6a59eb05eae 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -69,6 +69,9 @@ void intel_pxp_init(struct intel_pxp *pxp) if (!HAS_PXP(gt->i915)) return;

• spin_lock_init(&pxp->lock);
• INIT_LIST_HEAD(&pxp->protected_objects);
• /*
  • we'll use the completion to check if there is a termination pending,
  • so we start it as completed and we reinit it when a termination

@@ -177,11 +180,49 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp) intel_pxp_irq_disable(pxp); }

+int intel_pxp_object_add(struct drm_i915_gem_object *obj) {

• struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
• if (!intel_pxp_is_enabled(pxp))

• return -ENODEV;
  

• if (!list_empty(&obj->pxp_link))

• return -EEXIST;
  

• spin_lock_irq(&pxp->lock);
• list_add(&obj->pxp_link, &pxp->protected_objects);
• spin_unlock_irq(&pxp->lock);
• return 0;

+}

+void intel_pxp_object_remove(struct drm_i915_gem_object *obj) {

• struct intel_pxp *pxp = &to_i915(obj->base.dev)->gt.pxp;
• if (!intel_pxp_is_enabled(pxp))

• return;
  

• spin_lock_irq(&pxp->lock);
• list_del_init(&obj->pxp_link);
• spin_unlock_irq(&pxp->lock);

+}

void intel_pxp_invalidate(struct intel_pxp *pxp) { struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;

• struct drm_i915_gem_object *obj, *tmp; struct i915_gem_context *ctx, *cn;

• /* delete objects that have been used with the invalidated session

*/

• spin_lock_irq(&pxp->lock);
• list_for_each_entry_safe(obj, tmp, &pxp->protected_objects,

pxp_link) {

• if (i915_gem_object_has_pages(obj))
  

• 	list_del_init(&obj->pxp_link);
  

• }
• spin_unlock_irq(&pxp->lock);
• /* ban all contexts marked as protected */ spin_lock_irq(&i915->gem.contexts.lock); list_for_each_entry_safe(ctx, cn, &i915->gem.contexts.list, link)

{ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h index 1f9871e64096..3500d7896058 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h @@ -9,6 +9,8 @@ #include "gt/intel_gt_types.h" #include "intel_pxp_types.h"

+struct drm_i915_gem_object;

static inline struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp) { return container_of(pxp, struct intel_gt, pxp); @@ -33,6 +35,9 @@ void intel_pxp_fini_hw(struct intel_pxp *pxp);

void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp); int intel_pxp_start(struct intel_pxp *pxp);

+int intel_pxp_object_add(struct drm_i915_gem_object *obj); void +intel_pxp_object_remove(struct drm_i915_gem_object *obj); void intel_pxp_invalidate(struct intel_pxp *pxp); #else static inline void intel_pxp_init(struct intel_pxp *pxp) @@ -47,6 +52,14 @@ static inline int intel_pxp_start(struct intel_pxp *pxp) { return 0; }

+static inline int intel_pxp_object_add(struct drm_i915_gem_object *obj) +{

• return 0;

+} +static inline void intel_pxp_object_remove(struct drm_i915_gem_object +*obj) { } #endif

#endif /* __INTEL_PXP_H__ */ diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h index b3ae49dd73a8..cc510416eac6 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h @@ -7,7 +7,9 @@ #define __INTEL_PXP_TYPES_H__

#include <linux/completion.h> +#include <linux/list.h> #include <linux/mutex.h> +#include <linux/spinlock.h> #include <linux/types.h> #include <linux/workqueue.h>

@@ -43,6 +45,9 @@ struct intel_pxp { #define PXP_TERMINATION_REQUEST BIT(0) #define PXP_TERMINATION_COMPLETE BIT(1) #define PXP_INVAL_REQUIRED BIT(2)

• spinlock_t lock; /* protects the objects list */
• struct list_head protected_objects;

};

#endif /* __INTEL_PXP_TYPES_H__ */ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 3cc33fcbf520..d39b8cb4bfc5 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1768,7 +1768,7 @@ struct drm_i915_gem_context_param { __u64 value; };

-/* +/**

• Context SSEU programming
• It may be necessary for either functional or performance reason to

configure @@ -2669,8 +2669,12 @@ struct drm_i915_gem_create_ext { * * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see * struct drm_i915_gem_create_ext_memory_regions.

• *
  

• * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
  

• * struct drm_i915_gem_create_ext_protected_content.
  

  */

#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1 __u64 extensions; };

@@ -2728,6 +2732,33 @@ struct drm_i915_gem_create_ext_memory_regions { __u64 regions; };

+/**

• • struct drm_i915_gem_create_ext_protected_content - The
• • I915_OBJECT_PARAM_PROTECTED_CONTENT extension.
• • If this extension is provided, buffer contents are 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 ysing this extension will cause the ioctl to

+fail

• • and return -ENODEV. The flags parameter is reserved for future

+expansion and

• • must currently be set to zero.
• • The buffer contents are considered invalid after a PXP session teardown.
• • The encryption is guaranteed to be processed correctly only if the

+object

• • is submitted with a context created using the
• • I915_CONTEXT_PARAM_PROTECTED_CONTENT flag. This will also enable

+extra checks

• • at submission time on the validity of the objects involved, which

+can lead to

• • the following errors being returned from the execbuf ioctl:
• • -ENODEV: PXP session not currently active
• • -ENOEXEC: 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

-- 2.29.2

---

Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx