Re: [U-Boot] [PATCH 04/10] Add UCLASS_TEE for Trusted Execution Environment

Hi Jens,

On 13 August 2018 at 09:53, Jens Wiklander jens.wiklander@linaro.org wrote:

Adds a uclass to interface with a TEE (Trusted Execution Environment).

A TEE driver is a driver that interfaces with a trusted OS running in some secure environment, for example, TrustZone on ARM cpus, or a separate secure co-processor etc.

The TEE subsystem can serve a TEE driver for a Global Platform compliant TEE, but it's not limited to only Global Platform TEEs.

The over all design is based on the TEE subsystem in the Linux kernel, tailored for U-boot.

Signed-off-by: Jens Wiklander jens.wiklander@linaro.org

MAINTAINERS | 6 ++ drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/tee/Kconfig | 8 ++ drivers/tee/Makefile | 3 + drivers/tee/tee-uclass.c | 180 +++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/tee.h | 134 +++++++++++++++++++++++++++++ 8 files changed, 335 insertions(+) create mode 100644 drivers/tee/Kconfig create mode 100644 drivers/tee/Makefile create mode 100644 drivers/tee/tee-uclass.c create mode 100644 include/tee.h

diff --git a/MAINTAINERS b/MAINTAINERS index 58b61ac05882..7458c606ee92 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -571,6 +571,12 @@ TQ GROUP S: Orphaned (Since 2016-02) T: git git://git.denx.de/u-boot-tq-group.git

+TEE +M: Jens Wiklander jens.wiklander@linaro.org +S: Maintained +F: drivers/tee/ +F: include/tee.h

UBI M: Kyungmin Park kmpark@infradead.org M: Heiko Schocher hs@denx.de diff --git a/drivers/Kconfig b/drivers/Kconfig index c72abf893297..f3249ab1d143 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -94,6 +94,8 @@ source "drivers/spmi/Kconfig"

source "drivers/sysreset/Kconfig"

+source "drivers/tee/Kconfig"

source "drivers/thermal/Kconfig"

source "drivers/timer/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index d53208540ea6..0fcae36f50f7 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -103,6 +103,7 @@ obj-y += smem/ obj-y += soc/ obj-$(CONFIG_REMOTEPROC) += remoteproc/ obj-y += thermal/ +obj-$(CONFIG_TEE) += tee/

obj-$(CONFIG_MACH_PIC32) += ddr/microchip/ endif diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig new file mode 100644 index 000000000000..817ab331b0f8 --- /dev/null +++ b/drivers/tee/Kconfig @@ -0,0 +1,8 @@ +# Generic Trusted Execution Environment Configuration +config TEE

•   bool "Trusted Execution Environment support"
  

•   depends on ARM && (ARM64 || CPU_V7A)
  

•   select ARM_SMCCC
  

•   help
  

•     This implements a generic interface towards a Trusted Execution
  

•     Environment (TEE).
  

diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile new file mode 100644 index 000000000000..b6d8e16e6211 --- /dev/null +++ b/drivers/tee/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0+

+obj-y += tee-uclass.o diff --git a/drivers/tee/tee-uclass.c b/drivers/tee/tee-uclass.c new file mode 100644 index 000000000000..f0243a0f2e4e --- /dev/null +++ b/drivers/tee/tee-uclass.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2018 Linaro Limited
• */

+#include <common.h> +#include <dm.h> +#include <tee.h>

+struct tee_uclass_priv {

•   struct list_head list_shm;
  

+};

+static const struct tee_driver_ops *tee_get_ops(struct udevice *dev) +{

•   return device_get_ops(dev);
  

+}

+void tee_get_version(struct udevice *dev, struct tee_version_data *vers) +{

•   tee_get_ops(dev)->get_version(dev, vers);
  

+}

+int tee_open_session(struct udevice *dev, struct tee_open_session_arg *arg,

•                ulong num_param, struct tee_param *param)
  

+{

•   return tee_get_ops(dev)->open_session(dev, arg, num_param, param);
  

+}

+int tee_close_session(struct udevice *dev, u32 session) +{

•   return tee_get_ops(dev)->close_session(dev, session);
  

+}

+int tee_invoke_func(struct udevice *dev, struct tee_invoke_arg *arg,

•               ulong num_param, struct tee_param *param)
  

+{

•   return tee_get_ops(dev)->invoke_func(dev, arg, num_param, param);
  

+}

+struct tee_shm *__tee_shm_add(struct udevice *dev, ulong align, void *addr,

•                         ulong size, u32 flags)
  

+{

•   struct tee_shm *shm;
  

•   void *p = addr;
  

•   if (flags & TEE_SHM_ALLOC) {
  

•           if (align)
  

•                   p = memalign(align, size);
  

•           else
  

•                   p = malloc(size);
  

•   }
  

•   if (!p)
  

•           return NULL;
  

•   shm = calloc(1, sizeof(*shm));
  

•   if (!shm)
  

•           goto err;
  

•   shm->dev = dev;
  

•   shm->addr = p;
  

•   shm->size = size;
  

•   shm->flags = flags;
  

•   if ((flags & TEE_SHM_SEC_REGISTER) &&
  

•       tee_get_ops(dev)->shm_register(dev, shm))
  

•           goto err;
  

It seems like this can return errors other than -ENOMEM. How about changing this function to return an int?

•   if (flags & TEE_SHM_REGISTER) {
  

•           struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
  

•           list_add(&shm->link, &priv->list_shm);
  

•   }
  

•   return shm;
  

+err:

•   free(shm);
  

•   if (flags & TEE_SHM_ALLOC)
  

•           free(p);
  

•   return NULL;
  

+}

+struct tee_shm *tee_shm_alloc(struct udevice *dev, ulong size,

•                         u32 flags)
  

+{

•   u32 f = flags;
  

•   f |= TEE_SHM_SEC_REGISTER | TEE_SHM_REGISTER | TEE_SHM_ALLOC;
  

blank line before return

•   return __tee_shm_add(dev, 0, NULL, size, f);
  

+}

+struct tee_shm *tee_shm_register(struct udevice *dev, void *addr,

•                            ulong size, u32 flags)
  

+{

•   u32 f = flags & ~TEE_SHM_ALLOC;
  

•   f |= TEE_SHM_SEC_REGISTER | TEE_SHM_REGISTER;
  

•   return __tee_shm_add(dev, 0, addr, size, f);
  

+}

+void tee_shm_free(struct tee_shm *shm) +{

•   if (!shm)
  

•           return;
  

•   if (shm->flags & TEE_SHM_SEC_REGISTER)
  

•           tee_get_ops(shm->dev)->shm_unregister(shm->dev, shm);
  

•   if (shm->flags & TEE_SHM_REGISTER)
  

•           list_del(&shm->link);
  

•   if (shm->flags & TEE_SHM_ALLOC)
  

•           free(shm->addr);
  

•   free(shm);
  

+}

+bool tee_shm_is_registered(struct tee_shm *shm, struct udevice *dev) +{

•   struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
  

•   struct tee_shm *s;
  

•   list_for_each_entry(s, &priv->list_shm, link)
  

•           if (s == shm)
  

•                   return true;
  

•   return false;
  

+}

+struct udevice *tee_find_device(struct udevice *start,

•                           int (*match)(struct tee_version_data *vers,
  

•                                        const void *data),
  

•                           const void *data,
  

•                           struct tee_version_data *vers)
  

+{

•   struct udevice *dev = start;
  

•   struct tee_version_data lv;
  

•   struct tee_version_data *v = vers ? vers : &lv;
  

•   if (!dev)
  

•           uclass_first_device(UCLASS_TEE, &dev);
  

•   for (; dev; uclass_next_device(&dev)) {
  

•           tee_get_ops(dev)->get_version(dev, v);
  

•           if (!match || match(v, data))
  

•                   return dev;
  

This will probe each device as it looks through them. Is that what you want?

•   }
  

•   return NULL;
  

+}

+static int tee_pre_probe(struct udevice *dev) +{

•   struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
  

•   INIT_LIST_HEAD(&priv->list_shm);
  

•   return 0;
  

+}

+static int tee_pre_remove(struct udevice *dev) +{

•   struct tee_uclass_priv *priv = dev_get_uclass_priv(dev);
  

•   struct tee_shm *shm;
  

•   while (!list_empty(&priv->list_shm)) {
  

•           shm = list_first_entry(&priv->list_shm, struct tee_shm, link);
  

•           debug("%s: freeing leftover shm %p (size %lu, flags %#x)\n",
  

•                 __func__, (void *)shm, shm->size, shm->flags);
  

•           tee_shm_free(shm);
  

•   }
  

•   return 0;
  

+}

+UCLASS_DRIVER(tee) = {

•   .id = UCLASS_TEE,
  

•   .name = "tee",
  

•   .per_device_auto_alloc_size = sizeof(struct tee_uclass_priv),
  

•   .pre_probe = tee_pre_probe,
  

•   .pre_remove = tee_pre_remove,
  

+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index a39643ec5eef..955e0a915b87 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -81,6 +81,7 @@ enum uclass_id { UCLASS_SPI_GENERIC, /* Generic SPI flash target */ UCLASS_SYSCON, /* System configuration device */ UCLASS_SYSRESET, /* System reset device */

•   UCLASS_TEE,             /* Trusted Execution Environment device */
  

We need a bit more information about what this is for. It could go in a README or in the uclass header file.

    UCLASS_THERMAL,         /* Thermal sensor */
    UCLASS_TIMER,           /* Timer device */
    UCLASS_TPM,             /* Trusted Platform Module TIS interface */

diff --git a/include/tee.h b/include/tee.h new file mode 100644 index 000000000000..c2ac13e34128 --- /dev/null +++ b/include/tee.h @@ -0,0 +1,134 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (c) 2018 Linaro Limited
• */

+#ifndef __TEE_H +#define __TEE_H

+#include <common.h> +#include <dm.h>

These should be included by .c files.

+#define TEE_UUID_LEN 16

+#define TEE_GEN_CAP_GP BIT(0) /* GlobalPlatform compliant TEE */ +#define TEE_GEN_CAP_REG_MEM BIT(1) /* Supports registering shared memory */

+#define TEE_SHM_REGISTER BIT(0) +#define TEE_SHM_SEC_REGISTER BIT(1) +#define TEE_SHM_ALLOC BIT(2)

+#define TEE_PARAM_ATTR_TYPE_NONE 0 /* parameter not used */ +#define TEE_PARAM_ATTR_TYPE_VALUE_INPUT 1 +#define TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT 2 +#define TEE_PARAM_ATTR_TYPE_VALUE_INOUT 3 /* input and output */ +#define TEE_PARAM_ATTR_TYPE_MEMREF_INPUT 5 +#define TEE_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6 +#define TEE_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */ +#define TEE_PARAM_ATTR_TYPE_MASK 0xff +#define TEE_PARAM_ATTR_META 0x100 +#define TEE_PARAM_ATTR_MASK (TEE_PARAM_ATTR_TYPE_MASK | \

•                                            TEE_PARAM_ATTR_META)
  

+/*

• • Some Global Platform error codes which has a meaning if the
• • TEE_GEN_CAP_GP bit is returned by the driver.
• */

+#define TEE_SUCCESS 0x00000000 +#define TEE_ERROR_GENERIC 0xffff0000 +#define TEE_ERROR_BAD_PARAMETERS 0xffff0006 +#define TEE_ERROR_ITEM_NOT_FOUND 0xffff0008 +#define TEE_ERROR_NOT_IMPLEMENTED 0xffff0009 +#define TEE_ERROR_NOT_SUPPORTED 0xffff000a +#define TEE_ERROR_COMMUNICATION 0xffff000e +#define TEE_ERROR_OUT_OF_MEMORY 0xffff000c +#define TEE_ERROR_TARGET_DEAD 0xffff3024

+#define TEE_ORIGIN_COMMS 0x00000002

+struct tee_driver_ops;

+struct tee_shm {

Comment structure, and below

•   struct udevice *dev;
  

•   struct list_head link;
  

•   void *addr;
  

•   ulong size;
  

•   u32 flags;
  

+};

+struct tee_param_memref {

•   ulong shm_offs;
  

•   ulong size;
  

•   struct tee_shm *shm;
  

+};

+struct tee_param_value {

•   u64 a;
  

•   u64 b;
  

•   u64 c;
  

+};

+struct tee_param {

•   u64 attr;
  

•   union {
  

•           struct tee_param_memref memref;
  

•           struct tee_param_value value;
  

•   } u;
  

+};

+struct tee_open_session_arg {

•   u8 uuid[TEE_UUID_LEN];
  

•   u8 clnt_uuid[TEE_UUID_LEN];
  

•   u32 clnt_login;
  

•   u32 session;
  

•   u32 ret;
  

•   u32 ret_origin;
  

+};

+struct tee_invoke_arg {

•   u32 func;
  

•   u32 session;
  

•   u32 ret;
  

•   u32 ret_origin;
  

+};

+struct tee_version_data {

•   u32 gen_caps;
  

+};

+struct tee_driver_ops {

These all need full comments.

•   void (*get_version)(struct udevice *dev, struct tee_version_data *vers);
  

•   int (*open_session)(struct udevice *dev,
  

•                       struct tee_open_session_arg *arg, ulong num_param,
  

•                       struct tee_param *param);
  

•   int (*close_session)(struct udevice *dev, u32 session);
  

•   int (*invoke_func)(struct udevice *dev, struct tee_invoke_arg *arg,
  

•                      ulong num_param, struct tee_param *param);
  

•   int (*shm_register)(struct udevice *dev, struct tee_shm *shm);
  

•   int (*shm_unregister)(struct udevice *dev, struct tee_shm *shm);
  

+};

And these

+struct tee_shm *__tee_shm_add(struct udevice *dev, ulong align, void *addr,

•                         ulong size, u32 flags);
  

+struct tee_shm *tee_shm_alloc(struct udevice *dev, ulong size, u32 flags);

What are flags? All of this needs documentation please.

+struct tee_shm *tee_shm_register(struct udevice *dev, void *addr,

•                            ulong length, u32 flags);
  

+void tee_shm_free(struct tee_shm *shm); +bool tee_shm_is_registered(struct tee_shm *shm, struct udevice *dev);

+struct udevice *tee_find_device(struct udevice *start,

•                           int (*match)(struct tee_version_data *vers,
  

•                                        const void *data),
  

•                           const void *data,
  

•                           struct tee_version_data *vers);
  

+void tee_get_version(struct udevice *dev, struct tee_version_data *vers); +int tee_open_session(struct udevice *dev, struct tee_open_session_arg *arg,

•                ulong num_param, struct tee_param *param);
  

+int tee_close_session(struct udevice *dev, u32 session);

+int tee_invoke_func(struct udevice *dev, struct tee_invoke_arg *arg,

•               ulong num_param, struct tee_param *param);
  

+#endif /*__TEE_H*/

2.17.1

Regards, Simon