[Intel-gfx] [RFC-v19 03/13] drm/i915/pxp: Implement funcs to create the TEE channel
Huang, Sean Z
sean.z.huang at intel.com
Mon Jan 11 22:47:40 UTC 2021
Hi Rodrigo,
I removed the debug print print_hex_dump() in this patch, this change will reflect at rev20.
Regarding to the Chris's question for mutex, actually what I'm doing the similar with the existing hdcp_comp_added and hdcp_comp_mutex in https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/display/intel_hdcp.c#L2002
During the bind/unbind, we would need to protect pxp_tee_comp_added with pxp_tee_comp_mutex, to make sure no race condition between intel_pxp_tee_component_init() and intel_pxp_tee_component_fini().
So hopefully this mutex is acceptable. Thank you!
Best regards,
Sean
-----Original Message-----
From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
Sent: Thursday, January 7, 2021 7:36 AM
To: Huang, Sean Z <sean.z.huang at intel.com>; Intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [RFC-v19 03/13] drm/i915/pxp: Implement funcs to create the TEE channel
On Wed, 2021-01-06 at 15:12 -0800, Huang, Sean Z wrote:
> Implement the funcs to create the TEE channel, so kernel can send the
> TEE commands directly to TEE for creating the arbitrary
> (defualt) session.
>
> Signed-off-by: Huang, Sean Z <sean.z.huang at intel.com>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 6 +
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 5 +
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 137
> +++++++++++++++++++++++
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.h | 14 +++
> include/drm/i915_component.h | 1 +
> include/drm/i915_pxp_tee_interface.h | 45 ++++++++
> 8 files changed, 211 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
> create mode 100644 include/drm/i915_pxp_tee_interface.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile index cbf2f0594b4d..5494c30cb54f
> 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -262,7 +262,8 @@ i915-y += i915_perf.o # Protected execution
> platform (PXP) support
> i915-$(CONFIG_DRM_I915_PXP) += \
> pxp/intel_pxp.o \
> - pxp/intel_pxp_context.o
> + pxp/intel_pxp_context.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/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3e504247f2da..207d50226e64 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -322,6 +322,7 @@ static int i915_driver_early_probe(struct
> drm_i915_private *dev_priv)
> mutex_init(&dev_priv->wm.wm_mutex);
> mutex_init(&dev_priv->pps_mutex);
> mutex_init(&dev_priv->hdcp_comp_mutex);
> + mutex_init(&dev_priv->pxp_tee_comp_mutex);
>
> i915_memcpy_init_early(dev_priv);
> intel_runtime_pm_init_early(&dev_priv->runtime_pm);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 5e5bcef20e33..c2f47daef5a5
> 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1209,6 +1209,12 @@ struct drm_i915_private {
> /* Mutex to protect the above hdcp component related values.
> */
> struct mutex hdcp_comp_mutex;
>
> + struct i915_pxp_comp_master *pxp_tee_master;
> + bool pxp_tee_comp_added;
> +
> + /* Mutex to protect the above pxp_tee component related
> values. */
> + struct mutex pxp_tee_comp_mutex;
> +
> I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
>
> /*
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index f566a4fda044..c819f3791ee4 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -5,6 +5,7 @@
> #include "i915_drv.h"
> #include "intel_pxp.h"
> #include "intel_pxp_context.h"
> +#include "intel_pxp_tee.h"
>
> /* KCR register definitions */
> #define KCR_INIT _MMIO(0x320f0)
> @@ -23,6 +24,8 @@ void intel_pxp_init(struct intel_pxp *pxp)
>
> intel_uncore_write(gt->uncore, KCR_INIT,
> KCR_INIT_ALLOW_DISPLAY_ME_WRITES);
>
> + intel_pxp_tee_component_init(pxp);
> +
> drm_info(>->i915->drm, "Protected Xe Path (PXP) protected
> content support initialized\n"); }
>
> @@ -33,5 +36,7 @@ void intel_pxp_fini(struct intel_pxp *pxp)
> if (INTEL_GEN(gt->i915) < 12)
> return;
>
> + intel_pxp_tee_component_fini(pxp);
> +
> intel_pxp_ctx_fini(&pxp->ctx); } 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..5a1ffcc703e2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> @@ -0,0 +1,137 @@
> +// 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_context.h"
> +#include "intel_pxp_tee.h"
> +
> +static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
> + void *msg_in, u32 msg_in_size,
> + void *msg_out, u32
> *msg_out_size_ptr,
> + u32 msg_out_buf_size) {
> + int ret;
> + struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> + struct drm_i915_private *i915 = gt->i915;
> + struct i915_pxp_comp_master *pxp_tee_master = i915-
> >pxp_tee_master;
> +
> + if (!pxp_tee_master || !msg_in || !msg_out ||
> !msg_out_size_ptr)
> + return -EINVAL;
> +
> + lockdep_assert_held(&i915->pxp_tee_comp_mutex);
> +
> + if (drm_debug_enabled(DRM_UT_DRIVER))
> + print_hex_dump(KERN_DEBUG, "TEE input message
> binaries:",
> + DUMP_PREFIX_OFFSET, 4, 4, msg_in,
> msg_in_size, true);
why not use drm debug prints directly?
Also, this patch didn't address any of the concerns that Chris had raised on v4.
> +
> + ret = pxp_tee_master->ops->send(pxp_tee_master->tee_dev,
> msg_in, msg_in_size);
> + if (ret) {
> + drm_err(&i915->drm, "Failed to send TEE message\n");
> + return -EFAULT;
> + }
> +
> + ret = pxp_tee_master->ops->receive(pxp_tee_master->tee_dev,
> msg_out, msg_out_buf_size);
> + if (ret < 0) {
> + drm_err(&i915->drm, "Failed to receive TEE
> message\n");
> + return -EFAULT;
> + }
> +
> + if (ret > msg_out_buf_size) {
> + drm_err(&i915->drm, "Failed to receive TEE message
> due to unexpected output size\n");
> + return -EFAULT;
> + }
> +
> + *msg_out_size_ptr = ret;
> + ret = 0;
> +
> + if (drm_debug_enabled(DRM_UT_DRIVER))
> + print_hex_dump(KERN_DEBUG, "TEE output message
> binaries:",
> + DUMP_PREFIX_OFFSET, 4, 4, msg_out,
> *msg_out_size_ptr, true);
> +
> + return ret;
> +}
> +
> +/**
> + * i915_pxp_tee_component_bind - bind funciton 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 drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> +
> + if (!i915 || !tee_kdev || !data)
> + return -EPERM;
> +
> + mutex_lock(&i915->pxp_tee_comp_mutex);
> + i915->pxp_tee_master = (struct i915_pxp_comp_master *)data;
> + i915->pxp_tee_master->tee_dev = tee_kdev;
> + mutex_unlock(&i915->pxp_tee_comp_mutex);
> +
> + return 0;
> +}
> +
> +static void i915_pxp_tee_component_unbind(struct device *i915_kdev,
> + struct device *tee_kdev,
> void *data)
> +{
> + struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> +
> + if (!i915 || !tee_kdev || !data)
> + return;
> +
> + mutex_lock(&i915->pxp_tee_comp_mutex);
> + i915->pxp_tee_master = NULL;
> + mutex_unlock(&i915->pxp_tee_comp_mutex);
> +}
> +
> +static const struct component_ops i915_pxp_tee_component_ops = {
> + .bind = i915_pxp_tee_component_bind,
> + .unbind = i915_pxp_tee_component_unbind, };
> +
> +void intel_pxp_tee_component_init(struct intel_pxp *pxp) {
> + int ret;
> + struct intel_gt *gt = container_of(pxp, typeof(*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 at component add(%d)\n",
> ret);
> + return;
> + }
> +
> + mutex_lock(&i915->pxp_tee_comp_mutex);
> + i915->pxp_tee_comp_added = true;
> + mutex_unlock(&i915->pxp_tee_comp_mutex);
> +}
> +
> +void intel_pxp_tee_component_fini(struct intel_pxp *pxp) {
> + struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> + struct drm_i915_private *i915 = gt->i915;
> +
> + mutex_lock(&i915->pxp_tee_comp_mutex);
> + if (!i915->pxp_tee_comp_added) {
> + mutex_unlock(&i915->pxp_tee_comp_mutex);
> + return;
> + }
> +
> + i915->pxp_tee_comp_added = false;
> + mutex_unlock(&i915->pxp_tee_comp_mutex);
> +
> + 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..4b5e3edb1d9b
> --- /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"
> +
> +void 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/include/drm/i915_component.h
> b/include/drm/i915_component.h index 55c3b123581b..c1e2a43d2d1e 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -29,6 +29,7 @@
> enum i915_component_type {
> I915_COMPONENT_AUDIO = 1,
> I915_COMPONENT_HDCP,
> + I915_COMPONENT_PXP
> };
>
> /* MAX_PORT is the number of port
> diff --git a/include/drm/i915_pxp_tee_interface.h
> b/include/drm/i915_pxp_tee_interface.h
> new file mode 100644
> index 000000000000..3999e255e145
> --- /dev/null
> +++ b/include/drm/i915_pxp_tee_interface.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + *
> + * Authors:
> + * Vitaly Lubart <vitaly.lubart at intel.com> */
> +
> +#ifndef _I915_PXP_TEE_INTERFACE_H_
> +#define _I915_PXP_TEE_INTERFACE_H_
> +
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +
> +/**
> + * struct i915_pxp_component_ops - ops for PXP services.
> + * @owner: Module providing the ops
> + * @send: sends data to PXP
> + * @receive: receives data from PXP
> + */
> +struct i915_pxp_component_ops {
> + /**
> + * @owner: owner of the module provding the ops
> + */
> + struct module *owner;
> +
> + int (*send)(struct device *dev, const void *message, size_t
> size);
> + int (*receive)(struct device *dev, void *buffer, size_t
> size);
> +};
> +
> +/**
> + * struct i915_pxp_component_master - Used for communication between
> i915
> + * and TEE drivers for the PXP services
> + * @tee_dev: device that provide the PXP service from TEE Bus.
> + * @pxp_ops: Ops implemented by TEE driver, used by i915 driver.
> + */
> +struct i915_pxp_comp_master {
> + struct device *tee_dev;
> + const struct i915_pxp_component_ops *ops;
> +
> + /* To protect the above members. */
> + struct mutex mutex;
> +};
> +
> +#endif /* _I915_TEE_PXP_INTERFACE_H_ */
More information about the Intel-gfx
mailing list