Hi Jorge,

On Mon, Dec 21, 2020 at 07:15:40PM +0100, Jorge Ramirez-Ortiz wrote:
> This commit gives the secure world access to the I2C bus so it can
> communicate with I2C slaves (tipically those would be secure elements
> like the NXP SE050).
> 
> Tested on imx8mmevk.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jo...@foundries.io>
> ---
>  drivers/tee/optee/Makefile               |  1 +
>  drivers/tee/optee/i2c.c                  | 88 ++++++++++++++++++++++++
>  drivers/tee/optee/optee_msg.h            | 22 ++++++
>  drivers/tee/optee/optee_msg_supplicant.h |  5 ++
>  drivers/tee/optee/optee_private.h        | 12 ++++
>  drivers/tee/optee/supplicant.c           |  3 +
>  6 files changed, 131 insertions(+)
>  create mode 100644 drivers/tee/optee/i2c.c
> 
> diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile
> index 928d3f8002..068c6e7aa1 100644
> --- a/drivers/tee/optee/Makefile
> +++ b/drivers/tee/optee/Makefile
> @@ -2,4 +2,5 @@
>  
>  obj-y += core.o
>  obj-y += supplicant.o
> +obj-$(CONFIG_DM_I2C) += i2c.o
>  obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
> diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c
> new file mode 100644
> index 0000000000..2ebbf1ff7c
> --- /dev/null
> +++ b/drivers/tee/optee/i2c.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (c) 2020 Foundries.io Ltd
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <i2c.h>
> +#include <tee.h>
> +#include "optee_msg.h"
> +#include "optee_private.h"
> +
> +static struct {
> +     struct udevice *dev;
> +     int chip;
> +     int bus;
> +} xfer;
> +
> +void optee_suppl_cmd_i2c_transfer(struct udevice *dev,
> +                               struct optee_msg_arg *arg)
> +{
> +     const uint64_t attr[] = {
A u8 instead of uint64_t would give the same result.

> +             OPTEE_MSG_ATTR_TYPE_VALUE_INPUT,
> +             OPTEE_MSG_ATTR_TYPE_VALUE_INPUT,
> +             OPTEE_MSG_ATTR_TYPE_RMEM_INOUT,
> +             OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT,
> +     };
> +     struct udevice *chip_dev = NULL;
> +     struct tee_shm *shm = NULL;
> +     uint8_t *buf = NULL;
> +     size_t len = 0;
> +     int chip = -1;
> +     int bus = -1;
> +     int ret = -1;
> +
> +     if (arg->num_params != ARRAY_SIZE(attr) ||
> +         arg->params[0].attr != attr[0] ||
> +         arg->params[1].attr != attr[1] ||
> +         arg->params[2].attr != attr[2] ||
> +         arg->params[3].attr != attr[3]) {
> +             arg->ret = TEE_ERROR_BAD_PARAMETERS;
> +             return;
> +     }
> +
> +     len = arg->params[2].u.tmem.size;
> +     shm = (struct tee_shm *)(unsigned long)arg->params[2].u.tmem.shm_ref;
Please replace tmem with rmem. The OPTEE_MSG_ATTR_TYPE_RMEM_INOUT above
indicates that we're dealing with a struct optee_msg_param_rmem.

> +     buf = shm->addr;
> +     if (!buf || !len)
> +             goto bad;
> +
> +     bus = (int)arg->params[0].u.value.b;
> +     chip = (int)arg->params[0].u.value.c;
> +
> +     if (!xfer.dev || xfer.chip != chip || xfer.bus != bus) {
> +             if (i2c_get_chip_for_busnum(bus, chip, 0, &chip_dev))
> +                     goto bad;
> +
> +             xfer.dev = chip_dev;
> +             xfer.chip = chip;
> +             xfer.bus = bus;
Is this caching safe? No risk of using stale data?

Thanks,
Jens

> +     }
> +
> +     if (arg->params[1].u.value.a & OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT)
> +             if (i2c_set_chip_flags(xfer.dev, DM_I2C_CHIP_10BIT))
> +                     goto bad;
> +
> +     switch (arg->params[0].u.value.a) {
> +     case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
> +             ret = dm_i2c_read(xfer.dev, 0, buf, len);
> +             break;
> +     case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
> +             ret = dm_i2c_write(xfer.dev, 0, buf, len);
> +             break;
> +     default:
> +             goto bad;
> +     }
> +
> +     if (ret) {
> +             arg->ret = TEE_ERROR_COMMUNICATION;
> +     } else {
> +             arg->params[3].u.value.a = len;
> +             arg->ret = TEE_SUCCESS;
> +     }
> +
> +     return;
> +bad:
> +     arg->ret = TEE_ERROR_BAD_PARAMETERS;
> +}
> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> index 24c60960fc..7cedb59a82 100644
> --- a/drivers/tee/optee/optee_msg.h
> +++ b/drivers/tee/optee/optee_msg.h
> @@ -422,4 +422,26 @@ struct optee_msg_arg {
>   */
>  #define OPTEE_MSG_RPC_CMD_SHM_FREE   7
>  
> +/*
> + * Access a device on an i2c bus
> + *
> + * [in]  param[0].u.value.a          mode: RD(0), WR(1)
> + * [in]  param[0].u.value.b          i2c adapter
> + * [in]  param[0].u.value.c          i2c chip
> + *
> + * [in]  param[1].u.value.a          i2c control flags
> + * [in]  param[1].u.value.b          i2c retry (optional)
> + *
> + * [in/out] memref[2]                        buffer to exchange the transfer 
> data
> + *                                   with the secure world
> + *
> + * [out]  param[3].u.value.a         bytes transferred by the driver
> + */
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> +/* I2C master transfer modes */
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
> +/* I2C master control flags */
> +#define OPTEE_MSG_RPC_CMD_I2C_FLAGS_TEN_BIT  BIT(0)
> +
>  #endif /* _OPTEE_MSG_H */
> diff --git a/drivers/tee/optee/optee_msg_supplicant.h 
> b/drivers/tee/optee/optee_msg_supplicant.h
> index a0fb8063c8..963cfd4782 100644
> --- a/drivers/tee/optee/optee_msg_supplicant.h
> +++ b/drivers/tee/optee/optee_msg_supplicant.h
> @@ -147,6 +147,11 @@
>  #define OPTEE_MSG_RPC_CMD_SHM_ALLOC  6
>  #define OPTEE_MSG_RPC_CMD_SHM_FREE   7
>  
> +/*
> + * I2C bus access
> + */
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> +
>  /*
>   * Was OPTEE_MSG_RPC_CMD_SQL_FS, which isn't supported any longer
>   */
> diff --git a/drivers/tee/optee/optee_private.h 
> b/drivers/tee/optee/optee_private.h
> index 9442d1c176..d7ab1f593f 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -60,6 +60,18 @@ static inline void optee_suppl_rpmb_release(struct udevice 
> *dev)
>  }
>  #endif
>  
> +#ifdef CONFIG_DM_I2C
> +void optee_suppl_cmd_i2c_transfer(struct udevice *dev,
> +                               struct optee_msg_arg *arg);
> +#else
> +void optee_suppl_cmd_i2c_transfer(struct udevice *dev,
> +                               struct optee_msg_arg *arg)
> +{
> +     debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER not implemented\n");
> +     arg->ret = TEE_ERROR_NOT_IMPLEMENTED;
> +}
> +#endif
> +
>  void *optee_alloc_and_init_page_list(void *buf, ulong len, u64 
> *phys_buf_ptr);
>  
>  #endif /* __OPTEE_PRIVATE_H */
> diff --git a/drivers/tee/optee/supplicant.c b/drivers/tee/optee/supplicant.c
> index ae042b9a20..f7738983cd 100644
> --- a/drivers/tee/optee/supplicant.c
> +++ b/drivers/tee/optee/supplicant.c
> @@ -89,6 +89,9 @@ void optee_suppl_cmd(struct udevice *dev, struct tee_shm 
> *shm_arg,
>       case OPTEE_MSG_RPC_CMD_RPMB:
>               optee_suppl_cmd_rpmb(dev, arg);
>               break;
> +     case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> +             optee_suppl_cmd_i2c_transfer(dev, arg);
> +             break;
>       default:
>               arg->ret = TEE_ERROR_NOT_IMPLEMENTED;
>       }
> -- 
> 2.17.1
> 

Reply via email to