Re: [RFC-v19 04/13] drm/i915/pxp: Create the arbitrary session after boot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rodrigo,

Quoted from Joonas at https://patchwork.freedesktop.org/patch/412397/?series=84620&rev=19 
" Generally the trend on the GT side is to contain in a .c file if there are
no shared users like these. So they should be at this spot, yet the rest
of the review comments apply."

So I think the define GEN12_KCR_SIP should stay in intel_pxp_arb.c since it's no shared users.
But anyway please let me know if you insist to move it to i915_reg.h

Best regards,
Sean

-----Original Message-----
From: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> 
Sent: Thursday, January 7, 2021 7:40 AM
To: Huang, Sean Z <sean.z.huang@xxxxxxxxx>; Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re:  [RFC-v19 04/13] drm/i915/pxp: Create the arbitrary session after boot

On Wed, 2021-01-06 at 15:12 -0800, Huang, Sean Z wrote:
> Create the arbitrary session, with the fixed session id 0xf, after 
> system boot, for the case that application allocates the protected 
> buffer without establishing any protection session. Because the 
> hardware requires at least one alive session for protected buffer 
> creation.  This arbitrary session needs to be re-created after 
> teardown or power event because hardware encryption key won't be valid 
> after such cases.
> 
> Signed-off-by: Huang, Sean Z <sean.z.huang@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/Makefile                |   1 +
>  drivers/gpu/drm/i915/pxp/intel_pxp.c         |   1 +
>  drivers/gpu/drm/i915/pxp/intel_pxp.h         |  16 +++
>  drivers/gpu/drm/i915/pxp/intel_pxp_arb.c     | 131
> +++++++++++++++++++
>  drivers/gpu/drm/i915/pxp/intel_pxp_arb.h     |  16 +++
>  drivers/gpu/drm/i915/pxp/intel_pxp_context.h |   1 +
>  drivers/gpu/drm/i915/pxp/intel_pxp_tee.c     |  73 +++++++++++
>  drivers/gpu/drm/i915/pxp/intel_pxp_tee.h     |   3 +
>  drivers/gpu/drm/i915/pxp/intel_pxp_types.h   |  26 ++++
>  9 files changed, 268 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_arb.c
>  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_arb.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile 
> b/drivers/gpu/drm/i915/Makefile index 5494c30cb54f..af9e87e4c63a 
> 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -262,6 +262,7 @@ i915-y += i915_perf.o  # Protected execution 
> platform (PXP) support
>  i915-$(CONFIG_DRM_I915_PXP) += \
>         pxp/intel_pxp.o \
> +       pxp/intel_pxp_arb.o \
>         pxp/intel_pxp_context.o \
>         pxp/intel_pxp_tee.o
>  
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index c819f3791ee4..3868e8c697f9 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -6,6 +6,7 @@
>  #include "intel_pxp.h"
>  #include "intel_pxp_context.h"
>  #include "intel_pxp_tee.h"
> +#include "intel_pxp_arb.h"
>  
>  /* KCR register definitions */
>  #define KCR_INIT            _MMIO(0x320f0)
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> index f47bc6bea34f..8fc91e900b16 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> @@ -8,6 +8,22 @@
>  
>  #include "intel_pxp_types.h"
>  
> +enum pxp_session_types {
> +       SESSION_TYPE_TYPE0 = 0,
> +       SESSION_TYPE_TYPE1 = 1,
> +
> +       SESSION_TYPE_MAX
> +};
> +
> +enum pxp_protection_modes {
> +       PROTECTION_MODE_NONE = 0,
> +       PROTECTION_MODE_LM   = 2,
> +       PROTECTION_MODE_HM   = 3,
> +       PROTECTION_MODE_SM   = 6,
> +
> +       PROTECTION_MODE_ALL
> +};
> +
>  #ifdef CONFIG_DRM_I915_PXP
>  void intel_pxp_init(struct intel_pxp *pxp);  void 
> intel_pxp_fini(struct intel_pxp *pxp); diff --git 
> a/drivers/gpu/drm/i915/pxp/intel_pxp_arb.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp_arb.c
> new file mode 100644
> index 000000000000..640d7103c04d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_arb.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2020, Intel Corporation. All rights reserved.
> + */
> +
> +#include "gt/intel_context.h"
> +#include "gt/intel_engine_pm.h"
> +
> +#include "intel_pxp_types.h"
> +#include "intel_pxp_arb.h"
> +#include "intel_pxp.h"
> +#include "intel_pxp_tee.h"
> +
> +#define GEN12_KCR_SIP _MMIO(0x32260) /* KCR type0 session in play 0-
> 31 */
> +
> +/* Arbitrary session */
> +#define ARB_SESSION_INDEX 0xf
> +#define ARB_SESSION_TYPE SESSION_TYPE_TYPE0

All reg defines should be in i915_reg.h

> +
> +bool intel_pxp_arb_session_is_in_play(struct intel_pxp *pxp) {
> +       u32 regval_sip = 0;
> +       intel_wakeref_t wakeref;
> +       struct intel_gt *gt = container_of(pxp, struct intel_gt,
> pxp);
> +
> +       with_intel_runtime_pm(&gt->i915->runtime_pm, wakeref) {
> +               regval_sip = intel_uncore_read(gt->uncore,
> GEN12_KCR_SIP);
> +       }
> +
> +       return regval_sip & BIT(ARB_SESSION_INDEX); }
> +
> +/* wait hw session_in_play reg to match the current sw state */ 
> +static int wait_arb_hw_sw_state(struct intel_pxp *pxp) {
> +       const int max_retry = 10;
> +       const int ms_delay = 10;
> +       int retry = 0;
> +       int ret;
> +       struct pxp_protected_session *arb = &pxp->ctx.arb_session;
> +
> +       ret = -EINVAL;
> +       for (retry = 0; retry < max_retry; retry++) {
> +               if (intel_pxp_arb_session_is_in_play(pxp) ==
> +                   arb->is_in_play) {
> +                       ret = 0;
> +                       break;
> +               }
> +
> +               msleep(ms_delay);
> +       }
> +
> +       return ret;
> +}
> +
> +static void arb_session_entry_init(struct intel_pxp *pxp) {
> +       struct pxp_protected_session *arb = &pxp->ctx.arb_session;
> +
> +       arb->type = ARB_SESSION_TYPE;
> +       arb->protection_mode = PROTECTION_MODE_HM;
> +       arb->index = ARB_SESSION_INDEX;
> +       arb->is_in_play = false;
> +}
> +
> +static int intel_pxp_arb_reserve_session(struct intel_pxp *pxp) {
> +       int ret;
> +
> +       lockdep_assert_held(&pxp->ctx.mutex);
> +
> +       arb_session_entry_init(pxp);
> +       ret = wait_arb_hw_sw_state(pxp);
> +
> +       return ret;
> +}
> +
> +/**
> + * intel_pxp_arb_mark_session_in_play - To put an reserved protected
> session to "in_play" state
> + * @pxp: pointer to pxp struct.
> + *
> + * Return: status. 0 means update is successful.
> + */
> +static int intel_pxp_arb_mark_session_in_play(struct intel_pxp *pxp) 
> +{
> +       struct pxp_protected_session *arb = &pxp->ctx.arb_session;
> +
> +       lockdep_assert_held(&pxp->ctx.mutex);
> +
> +       if (arb->is_in_play)
> +               return -EINVAL;
> +
> +       arb->is_in_play = true;
> +       return 0;
> +}
> +
> +int intel_pxp_arb_create_session(struct intel_pxp *pxp) {
> +       int ret;
> +       struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> +
> +       lockdep_assert_held(&pxp->ctx.mutex);
> +
> +       if (pxp->ctx.flag_display_hm_surface_keys) {
> +               drm_err(&gt->i915->drm, "%s: arb session is alive so
> skipping the creation\n",
> +                       __func__);
> +               return 0;
> +       }
> +
> +       ret = intel_pxp_arb_reserve_session(pxp);
> +       if (ret) {
> +               drm_err(&gt->i915->drm, "Failed to reserve arb
> session\n");
> +               return ret;
> +       }
> +
> +       ret = intel_pxp_tee_cmd_create_arb_session(pxp,
> ARB_SESSION_INDEX);
> +       if (ret) {
> +               drm_err(&gt->i915->drm, "Failed to send tee cmd for
> arb session creation\n");
> +               return ret;
> +       }
> +
> +       ret = intel_pxp_arb_mark_session_in_play(pxp);
> +       if (ret) {
> +               drm_err(&gt->i915->drm, "Failed to mark arb session
> status in play\n");
> +               return ret;
> +       }
> +
> +       pxp->ctx.flag_display_hm_surface_keys = true;
> +
> +       return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_arb.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp_arb.h
> new file mode 100644
> index 000000000000..2196153dd879
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_arb.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2020, Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __INTEL_PXP_ARB_H__
> +#define __INTEL_PXP_ARB_H__
> +
> +#include <linux/types.h>
> +
> +struct intel_pxp;
> +
> +int intel_pxp_arb_create_session(struct intel_pxp *pxp); bool 
> +intel_pxp_arb_session_is_in_play(struct intel_pxp *pxp);
> +
> +#endif /* __INTEL_PXP_ARB_H__ */
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_context.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp_context.h
> index f51021c33d45..bf2feb4aaf6d 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_context.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_context.h
> @@ -8,6 +8,7 @@
>  
>  #include <linux/mutex.h>
>  #include "intel_pxp_types.h"
> +#include "intel_pxp_arb.h"
>  
>  void intel_pxp_ctx_init(struct pxp_context *ctx);  void 
> intel_pxp_ctx_fini(struct pxp_context *ctx); diff --git 
> a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> index 5a1ffcc703e2..41d8a31bbb75 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> @@ -10,6 +10,32 @@
>  #include "intel_pxp.h"
>  #include "intel_pxp_context.h"
>  #include "intel_pxp_tee.h"
> +#include "intel_pxp_arb.h"
> +
> +#define PXP_TEE_APIVER 0x40002
> +#define PXP_TEE_ARB_CMDID 0x1e
> +#define PXP_TEE_ARB_PROTECTION_MODE 0x2
> +
> +/* PXP TEE message header */
> +struct pxp_tee_cmd_header {
> +       u32 api_version;
> +       u32 command_id;
> +       u32 status;
> +       /* Length of the message (excluding the header) */
> +       u32 buffer_len;
> +} __packed;
> +
> +/* PXP TEE message input to create a arbitrary session */ struct 
> +pxp_tee_create_arb_in {
> +       struct pxp_tee_cmd_header header;
> +       u32 protection_mode;
> +       u32 session_id;
> +} __packed;
> +
> +/* PXP TEE message output to create a arbitrary session */ struct 
> +pxp_tee_create_arb_out {
> +       struct pxp_tee_cmd_header header; } __packed;
>  
>  static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
>                                     void *msg_in, u32 msg_in_size, @@ 
> -70,7 +96,9 @@ 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)
>  {
> +       int ret = 0;
>         struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> +       struct intel_pxp *pxp = &i915->gt.pxp;
>  
>         if (!i915 || !tee_kdev || !data)
>                 return -EPERM;
> @@ -80,6 +108,19 @@ static int i915_pxp_tee_component_bind(struct
> device *i915_kdev,
>         i915->pxp_tee_master->tee_dev = tee_kdev;
>         mutex_unlock(&i915->pxp_tee_comp_mutex);
>  
> +       mutex_lock(&pxp->ctx.mutex);
> +
> +       /* Create arb session only if tee is ready, during system
> boot or sleep/resume */
> +       if (!intel_pxp_arb_session_is_in_play(pxp))
> +               ret = intel_pxp_arb_create_session(pxp);
> +
> +       mutex_unlock(&pxp->ctx.mutex);
> +
> +       if (ret) {
> +               drm_err(&i915->drm, "Failed to create arb session
> ret=[%d]\n", ret);
> +               return ret;
> +       }
> +
>         return 0;
>  }
>  
> @@ -135,3 +176,35 @@ void intel_pxp_tee_component_fini(struct
> intel_pxp *pxp)
>  
>         component_del(i915->drm.dev, &i915_pxp_tee_component_ops);  }
> +
> +int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
> +                                        int arb_session_id) {
> +       int ret;
> +       u32 msg_out_size_received = 0;
> +       struct pxp_tee_create_arb_in msg_in = {0};
> +       struct pxp_tee_create_arb_out msg_out = {0};
> +       struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> +       struct drm_i915_private *i915 = gt->i915;
> +
> +       msg_in.header.api_version = PXP_TEE_APIVER;
> +       msg_in.header.command_id = PXP_TEE_ARB_CMDID;
> +       msg_in.header.buffer_len = sizeof(msg_in) -
> sizeof(msg_in.header);
> +       msg_in.protection_mode = PXP_TEE_ARB_PROTECTION_MODE;
> +       msg_in.session_id = arb_session_id;
> +
> +       mutex_lock(&i915->pxp_tee_comp_mutex);
> +
> +       ret = intel_pxp_tee_io_message(pxp,
> +                                      &msg_in,
> +                                      sizeof(msg_in),
> +                                      &msg_out,
> &msg_out_size_received,
> +                                      sizeof(msg_out));
> +
> +       mutex_unlock(&i915->pxp_tee_comp_mutex);
> +
> +       if (ret)
> +               drm_err(&i915->drm, "Failed to send tee msg
> ret=[%d]\n", ret);
> +
> +       return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
> index 4b5e3edb1d9b..c46f9033f709 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.h
> @@ -11,4 +11,7 @@
>  void intel_pxp_tee_component_init(struct intel_pxp *pxp);  void 
> intel_pxp_tee_component_fini(struct intel_pxp *pxp);
>  
> +int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
> +                                        int arb_session_id);
> +
>  #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 f9b40ea98b1b..287ba1e0ed9d 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> @@ -8,12 +8,38 @@
>  
>  #include <linux/mutex.h>
>  
> +/**
> + * struct pxp_protected_session - structure to track all active
> sessions.
> + */
> +struct pxp_protected_session {
> +       /** @index: Numeric identifier for this protected session */
> +       int index;
> +       /** @type: Type of session */
> +       int type;
> +       /** @protection_mode: mode of protection requested */
> +       int protection_mode;
> +
> +       /**
> +        * @is_in_play: indicates whether the session has been
> established
> +        *              in the HW root of trust if this flag is
> false, it
> +        *              indicates an application has reserved this
> session,
> +        *              but has not * established the session in the
> +        *              hardware yet.
> +        */
> +       bool is_in_play;
> +};
> +
>  /* struct pxp_context - Represents combined view of driver and 
> logical HW states. */  struct pxp_context {
>         /** @mutex: mutex to protect the pxp context */
>         struct mutex mutex;
>  
>         bool inited;
> +
> +       struct pxp_protected_session arb_session;
> +       u32 arb_session_pxp_tag;
> +
> +       bool flag_display_hm_surface_keys;
>  };
>  
>  struct intel_pxp {


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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux