All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] tee: define session login identifiers
@ 2021-05-12 15:05 Etienne Carriere
  2021-05-12 15:05 ` [PATCH 2/2] tee: optee: support session login as REE kernel Etienne Carriere
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Etienne Carriere @ 2021-05-12 15:05 UTC (permalink / raw)
  To: u-boot

TEE header file defines a clnt_login field in struct tee_open_session_arg
but does not define the values expected. This change define identifiers
for the field using a enumerated type. Back end TEE driver is expected to
convert these IDs into IDs meaningful to the TEE.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 include/tee.h | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/tee.h b/include/tee.h
index 99367b258e..b8297601b4 100644
--- a/include/tee.h
+++ b/include/tee.h
@@ -31,6 +31,19 @@
 #define TEE_PARAM_ATTR_MASK			(TEE_PARAM_ATTR_TYPE_MASK | \
 						 TEE_PARAM_ATTR_META)
 
+/*
+ * Value for tee_open_session_arg::clnt_login
+ */
+enum tee_session_login {
+	TEE_SESSION_LOGIN_PUBLIC = 0,
+	TEE_SESSION_LOGIN_USER,
+	TEE_SESSION_LOGIN_GROUP,
+	TEE_SESSION_LOGIN_APPLICATION,
+	TEE_SESSION_LOGIN_APPLICATION_USER,
+	TEE_SESSION_LOGIN_APPLICATION_GROUP,
+	TEE_SESSION_LOGIN_REE_KERNEL,
+};
+
 /*
  * Some Global Platform error codes which has a meaning if the
  * TEE_GEN_CAP_GP bit is returned by the driver in
@@ -135,8 +148,8 @@ struct tee_param {
 /**
  * struct tee_open_session_arg - extra arguments for tee_open_session()
  * @uuid:	[in] UUID of the Trusted Application
- * @clnt_uuid:	[in] Normally zeroes
- * @clnt_login:	[in] Normally 0
+ * @clnt_uuid:	[in] UUID of client, zeroes for PUBLIC/REE_KERNEL
+ * @clnt_login:	[in] Class of client TEE_SESSION_LOGIN_*
  * @session:	[out] Session id
  * @ret:	[out] return value
  * @ret_origin:	[out] origin of the return value
@@ -144,7 +157,7 @@ struct tee_param {
 struct tee_open_session_arg {
 	u8 uuid[TEE_UUID_LEN];
 	u8 clnt_uuid[TEE_UUID_LEN];
-	u32 clnt_login;
+	enum tee_session_login clnt_login;
 	u32 session;
 	u32 ret;
 	u32 ret_origin;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] tee: optee: support session login as REE kernel
  2021-05-12 15:05 [PATCH 1/2] tee: define session login identifiers Etienne Carriere
@ 2021-05-12 15:05 ` Etienne Carriere
  2021-05-17  6:08   ` Jens Wiklander
  2021-05-17  6:02 ` [PATCH 1/2] tee: define session login identifiers Jens Wiklander
  2021-11-08  9:29 ` Patrick DELAUNAY
  2 siblings, 1 reply; 6+ messages in thread
From: Etienne Carriere @ 2021-05-12 15:05 UTC (permalink / raw)
  To: u-boot

OP-TEE supports an API extension to allow client to open a TEE session
as REE kernel which OP-TEE uses to differentiate client application
services from system services that only the REE OS kernel can access.

This change allows U-Boot to invoke OP-TEE which such kernel identity
and therefore access kernel client specific services.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
 drivers/tee/optee/core.c      | 24 +++++++++++++++++++++++-
 drivers/tee/optee/optee_msg.h |  2 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 73dbb22ba0..526bf125a0 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -349,6 +349,28 @@ static int optee_close_session(struct udevice *dev, u32 session)
 	return 0;
 }
 
+static uint32_t optee_login_id(enum tee_session_login login)
+{
+	/* Treat invalid IDs as public login */
+	switch (login) {
+	case TEE_SESSION_LOGIN_USER:
+		return OPTEE_MSG_LOGIN_USER;
+	case TEE_SESSION_LOGIN_GROUP:
+		return OPTEE_MSG_LOGIN_GROUP;
+	case TEE_SESSION_LOGIN_APPLICATION:
+		return OPTEE_MSG_LOGIN_APPLICATION;
+	case TEE_SESSION_LOGIN_APPLICATION_USER:
+		return OPTEE_MSG_LOGIN_APPLICATION;
+	case TEE_SESSION_LOGIN_APPLICATION_GROUP:
+		return OPTEE_MSG_LOGIN_APPLICATION;
+	case TEE_SESSION_LOGIN_REE_KERNEL:
+		return OPTEE_MSG_LOGIN_REE_KERNEL;
+	case TEE_SESSION_LOGIN_PUBLIC:
+	default:
+		return OPTEE_MSG_LOGIN_PUBLIC;
+	}
+}
+
 static int optee_open_session(struct udevice *dev,
 			      struct tee_open_session_arg *arg,
 			      uint num_params, struct tee_param *params)
@@ -372,7 +394,7 @@ static int optee_open_session(struct udevice *dev,
 				  OPTEE_MSG_ATTR_META;
 	memcpy(&msg_arg->params[0].u.value, arg->uuid, sizeof(arg->uuid));
 	memcpy(&msg_arg->params[1].u.value, arg->uuid, sizeof(arg->clnt_uuid));
-	msg_arg->params[1].u.value.c = arg->clnt_login;
+	msg_arg->params[1].u.value.c = optee_login_id(arg->clnt_login);
 
 	rc = to_msg_param(msg_arg->params + 2, num_params, params);
 	if (rc)
diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
index 8d40ce60c2..17e8d28e52 100644
--- a/drivers/tee/optee/optee_msg.h
+++ b/drivers/tee/optee/optee_msg.h
@@ -95,6 +95,8 @@
 #define OPTEE_MSG_LOGIN_APPLICATION		0x00000004
 #define OPTEE_MSG_LOGIN_APPLICATION_USER	0x00000005
 #define OPTEE_MSG_LOGIN_APPLICATION_GROUP	0x00000006
+/* OP-TEE extension: log as REE kernel */
+#define OPTEE_MSG_LOGIN_REE_KERNEL		0x80000000
 
 /*
  * Page size used in non-contiguous buffer entries
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 1/2] tee: define session login identifiers
  2021-05-12 15:05 [PATCH 1/2] tee: define session login identifiers Etienne Carriere
  2021-05-12 15:05 ` [PATCH 2/2] tee: optee: support session login as REE kernel Etienne Carriere
@ 2021-05-17  6:02 ` Jens Wiklander
  2021-11-08  9:29 ` Patrick DELAUNAY
  2 siblings, 0 replies; 6+ messages in thread
From: Jens Wiklander @ 2021-05-17  6:02 UTC (permalink / raw)
  To: u-boot

On Wed, May 12, 2021 at 5:06 PM Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> TEE header file defines a clnt_login field in struct tee_open_session_arg
> but does not define the values expected. This change define identifiers
> for the field using a enumerated type. Back end TEE driver is expected to
> convert these IDs into IDs meaningful to the TEE.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>  include/tee.h | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/include/tee.h b/include/tee.h
> index 99367b258e..b8297601b4 100644
> --- a/include/tee.h
> +++ b/include/tee.h
> @@ -31,6 +31,19 @@
>  #define TEE_PARAM_ATTR_MASK                    (TEE_PARAM_ATTR_TYPE_MASK | \
>                                                  TEE_PARAM_ATTR_META)
>
> +/*
> + * Value for tee_open_session_arg::clnt_login
> + */
> +enum tee_session_login {
> +       TEE_SESSION_LOGIN_PUBLIC = 0,
> +       TEE_SESSION_LOGIN_USER,
> +       TEE_SESSION_LOGIN_GROUP,
> +       TEE_SESSION_LOGIN_APPLICATION,

This is defined as 4 in the spec.

> +       TEE_SESSION_LOGIN_APPLICATION_USER,
> +       TEE_SESSION_LOGIN_APPLICATION_GROUP,

Please make these USER_APPLICATION and GROUP_APPLICATION since that's
the order used in the spec.

> +       TEE_SESSION_LOGIN_REE_KERNEL,

The Linux kernel defines the REE kernel stuff as:
/*
 * Disallow user-space to use GP implementation specific login
 * method range (0x80000000 - 0xBFFFFFFF). This range is rather
 * being reserved for REE kernel clients or TEE implementation.
 */
#define TEE_IOCTL_LOGIN_REE_KERNEL_MIN          0x80000000
#define TEE_IOCTL_LOGIN_REE_KERNEL_MAX          0xBFFFFFFF
/* Private login method for REE kernel clients */
#define TEE_IOCTL_LOGIN_REE_KERNEL              0x80000000

We should at least try to be in the same implementation defined range.

> +};

I think that using a TEE_LOGIN_ prefix should be enough.

> +
>  /*
>   * Some Global Platform error codes which has a meaning if the
>   * TEE_GEN_CAP_GP bit is returned by the driver in
> @@ -135,8 +148,8 @@ struct tee_param {
>  /**
>   * struct tee_open_session_arg - extra arguments for tee_open_session()
>   * @uuid:      [in] UUID of the Trusted Application
> - * @clnt_uuid: [in] Normally zeroes
> - * @clnt_login:        [in] Normally 0
> + * @clnt_uuid: [in] UUID of client, zeroes for PUBLIC/REE_KERNEL
> + * @clnt_login:        [in] Class of client TEE_SESSION_LOGIN_*
>   * @session:   [out] Session id
>   * @ret:       [out] return value
>   * @ret_origin:        [out] origin of the return value
> @@ -144,7 +157,7 @@ struct tee_param {
>  struct tee_open_session_arg {
>         u8 uuid[TEE_UUID_LEN];
>         u8 clnt_uuid[TEE_UUID_LEN];
> -       u32 clnt_login;
> +       enum tee_session_login clnt_login;

Please keep this as an u32. It's part of the ABI.

Cheers,
Jens

>         u32 session;
>         u32 ret;
>         u32 ret_origin;
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] tee: optee: support session login as REE kernel
  2021-05-12 15:05 ` [PATCH 2/2] tee: optee: support session login as REE kernel Etienne Carriere
@ 2021-05-17  6:08   ` Jens Wiklander
  2021-05-17 17:22     ` Etienne Carriere
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Wiklander @ 2021-05-17  6:08 UTC (permalink / raw)
  To: u-boot

On Wed, May 12, 2021 at 5:06 PM Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> OP-TEE supports an API extension to allow client to open a TEE session
> as REE kernel which OP-TEE uses to differentiate client application
> services from system services that only the REE OS kernel can access.
>
> This change allows U-Boot to invoke OP-TEE which such kernel identity
> and therefore access kernel client specific services.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>  drivers/tee/optee/core.c      | 24 +++++++++++++++++++++++-
>  drivers/tee/optee/optee_msg.h |  2 ++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index 73dbb22ba0..526bf125a0 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -349,6 +349,28 @@ static int optee_close_session(struct udevice *dev, u32 session)
>         return 0;
>  }
>
> +static uint32_t optee_login_id(enum tee_session_login login)
> +{
> +       /* Treat invalid IDs as public login */
> +       switch (login) {
> +       case TEE_SESSION_LOGIN_USER:
> +               return OPTEE_MSG_LOGIN_USER;
> +       case TEE_SESSION_LOGIN_GROUP:
> +               return OPTEE_MSG_LOGIN_GROUP;
> +       case TEE_SESSION_LOGIN_APPLICATION:
> +               return OPTEE_MSG_LOGIN_APPLICATION;
> +       case TEE_SESSION_LOGIN_APPLICATION_USER:
> +               return OPTEE_MSG_LOGIN_APPLICATION;
> +       case TEE_SESSION_LOGIN_APPLICATION_GROUP:
> +               return OPTEE_MSG_LOGIN_APPLICATION;
> +       case TEE_SESSION_LOGIN_REE_KERNEL:
> +               return OPTEE_MSG_LOGIN_REE_KERNEL;
> +       case TEE_SESSION_LOGIN_PUBLIC:
> +       default:
> +               return OPTEE_MSG_LOGIN_PUBLIC;
> +       }
> +}
> +

I don't see any point in this translation, we could just as well use
the correct values from the start.

Cheers,
Jens

>  static int optee_open_session(struct udevice *dev,
>                               struct tee_open_session_arg *arg,
>                               uint num_params, struct tee_param *params)
> @@ -372,7 +394,7 @@ static int optee_open_session(struct udevice *dev,
>                                   OPTEE_MSG_ATTR_META;
>         memcpy(&msg_arg->params[0].u.value, arg->uuid, sizeof(arg->uuid));
>         memcpy(&msg_arg->params[1].u.value, arg->uuid, sizeof(arg->clnt_uuid));
> -       msg_arg->params[1].u.value.c = arg->clnt_login;
> +       msg_arg->params[1].u.value.c = optee_login_id(arg->clnt_login);
>
>         rc = to_msg_param(msg_arg->params + 2, num_params, params);
>         if (rc)
> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> index 8d40ce60c2..17e8d28e52 100644
> --- a/drivers/tee/optee/optee_msg.h
> +++ b/drivers/tee/optee/optee_msg.h
> @@ -95,6 +95,8 @@
>  #define OPTEE_MSG_LOGIN_APPLICATION            0x00000004
>  #define OPTEE_MSG_LOGIN_APPLICATION_USER       0x00000005
>  #define OPTEE_MSG_LOGIN_APPLICATION_GROUP      0x00000006
> +/* OP-TEE extension: log as REE kernel */
> +#define OPTEE_MSG_LOGIN_REE_KERNEL             0x80000000
>
>  /*
>   * Page size used in non-contiguous buffer entries
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] tee: optee: support session login as REE kernel
  2021-05-17  6:08   ` Jens Wiklander
@ 2021-05-17 17:22     ` Etienne Carriere
  0 siblings, 0 replies; 6+ messages in thread
From: Etienne Carriere @ 2021-05-17 17:22 UTC (permalink / raw)
  To: u-boot

On Mon, 17 May 2021 at 08:08, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> On Wed, May 12, 2021 at 5:06 PM Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > OP-TEE supports an API extension to allow client to open a TEE session
> > as REE kernel which OP-TEE uses to differentiate client application
> > services from system services that only the REE OS kernel can access.
> >
> > This change allows U-Boot to invoke OP-TEE which such kernel identity
> > and therefore access kernel client specific services.
> >
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >  drivers/tee/optee/core.c      | 24 +++++++++++++++++++++++-
> >  drivers/tee/optee/optee_msg.h |  2 ++
> >  2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> > index 73dbb22ba0..526bf125a0 100644
> > --- a/drivers/tee/optee/core.c
> > +++ b/drivers/tee/optee/core.c
> > @@ -349,6 +349,28 @@ static int optee_close_session(struct udevice *dev, u32 session)
> >         return 0;
> >  }
> >
> > +static uint32_t optee_login_id(enum tee_session_login login)
> > +{
> > +       /* Treat invalid IDs as public login */
> > +       switch (login) {
> > +       case TEE_SESSION_LOGIN_USER:
> > +               return OPTEE_MSG_LOGIN_USER;
> > +       case TEE_SESSION_LOGIN_GROUP:
> > +               return OPTEE_MSG_LOGIN_GROUP;
> > +       case TEE_SESSION_LOGIN_APPLICATION:
> > +               return OPTEE_MSG_LOGIN_APPLICATION;
> > +       case TEE_SESSION_LOGIN_APPLICATION_USER:
> > +               return OPTEE_MSG_LOGIN_APPLICATION;
> > +       case TEE_SESSION_LOGIN_APPLICATION_GROUP:
> > +               return OPTEE_MSG_LOGIN_APPLICATION;
> > +       case TEE_SESSION_LOGIN_REE_KERNEL:
> > +               return OPTEE_MSG_LOGIN_REE_KERNEL;
> > +       case TEE_SESSION_LOGIN_PUBLIC:
> > +       default:
> > +               return OPTEE_MSG_LOGIN_PUBLIC;
> > +       }
> > +}
> > +
>
> I don't see any point in this translation, we could just as well use
> the correct values from the start.
>
> Cheers,
> Jens

Right, i'll check that.

thanks
etienne

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] tee: define session login identifiers
  2021-05-12 15:05 [PATCH 1/2] tee: define session login identifiers Etienne Carriere
  2021-05-12 15:05 ` [PATCH 2/2] tee: optee: support session login as REE kernel Etienne Carriere
  2021-05-17  6:02 ` [PATCH 1/2] tee: define session login identifiers Jens Wiklander
@ 2021-11-08  9:29 ` Patrick DELAUNAY
  2 siblings, 0 replies; 6+ messages in thread
From: Patrick DELAUNAY @ 2021-11-08  9:29 UTC (permalink / raw)
  To: Etienne Carriere, u-boot; +Cc: Jens Wiklander, Simon Glass

Hi

On 5/12/21 5:05 PM, Etienne Carriere wrote:
> TEE header file defines a clnt_login field in struct tee_open_session_arg
> but does not define the values expected. This change define identifiers
> for the field using a enumerated type. Back end TEE driver is expected to
> convert these IDs into IDs meaningful to the TEE.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>   include/tee.h | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
>

Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Thanks
Patrick


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-08  9:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12 15:05 [PATCH 1/2] tee: define session login identifiers Etienne Carriere
2021-05-12 15:05 ` [PATCH 2/2] tee: optee: support session login as REE kernel Etienne Carriere
2021-05-17  6:08   ` Jens Wiklander
2021-05-17 17:22     ` Etienne Carriere
2021-05-17  6:02 ` [PATCH 1/2] tee: define session login identifiers Jens Wiklander
2021-11-08  9:29 ` Patrick DELAUNAY

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.