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

Simon Glass sjg at chromium.org
Fri Aug 17 12:48:47 UTC 2018


Hi Jens,

On 13 August 2018 at 09:53, Jens Wiklander <jens.wiklander at 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 at 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
>

Please add a sandbox driver and a test in test/dm for any new uclass.

> 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 at linaro.org>
> +S:     Maintained
> +F:     drivers/tee/
> +F:     include/tee.h
> +
>  UBI
>  M:     Kyungmin Park <kmpark at infradead.org>
>  M:     Heiko Schocher <hs at 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;

Struct comment please

> +};
> +
> +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));

Rather than allocating memory here, can you use driver-model's
auto-alloc features?

> +       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;
> +
> +       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;
> +       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)

'find' should find the device, not probe anything. Is that what this does?

> +{
> +       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;
> +       }
> +
> +       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);

blank line before return

> +       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 */
>         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>
> +
> +#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 {

comments for these structs please.

Also the whole uclass needs a README or more docs. Perhaps I have missed this?

> +       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 {
> +       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);

Each of these needs a full function comment. Without that I cannot
really comment on the API. Same for functions below.

> +};
> +
> +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);
> +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


More information about the U-Boot mailing list