LinuxLists.cc - [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

2020-07-22 21:22:42

Subject: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

Some secure elements like NXP's SE050 sit on I2C buses. For OP-TEE to
control this type of cryptographic devices it needs coordinated access
to the bus, so collisions and RUNTIME_PM dont get in the way.

This trampoline driver allow OP-TEE to access them.
Tested on imx8mm LPDDR4

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
---
v6: compile out if CONFIG_I2C not enabled
v5: alphabetic order of includes
v4: remove unnecessary extra line in optee_msg.h
v3: use from/to msg param to support all types of memory
modify OPTEE_MSG_RPC_CMD_I2C_TRANSFER message id

drivers/tee/optee/optee_msg.h | 16 +++++++
drivers/tee/optee/rpc.c | 88 +++++++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+)

diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
index 795bc19ae17a..14b580f55356 100644
--- a/drivers/tee/optee/optee_msg.h
+++ b/drivers/tee/optee/optee_msg.h
@@ -419,4 +419,20 @@ 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/out] memref[1] buffer to exchange the transfer data
+ * with the secure world
+ *
+ * [out] param[0].u.value.a bytes transferred by the driver
+ */
+#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
+#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
+#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
+
#endif /* _OPTEE_MSG_H */
diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
index b4ade54d1f28..5fd5c6c93896 100644
--- a/drivers/tee/optee/rpc.c
+++ b/drivers/tee/optee/rpc.c
@@ -7,6 +7,7 @@

#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/tee_drv.h>
#include "optee_private.h"
@@ -49,6 +50,90 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg)
arg->ret = TEEC_ERROR_BAD_PARAMETERS;
}

+#if IS_ENABLED(CONFIG_I2C)
+static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
+ struct optee_msg_arg *arg)
+{
+ struct i2c_client client;
+ struct tee_param *params;
+ uint32_t type;
+ int i, ret;
+ size_t len;
+ char *buf;
+ uint32_t attr[] = {
+ TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
+ TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
+ TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
+ };
+
+ if (arg->num_params != ARRAY_SIZE(attr)) {
+ arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+ return;
+ }
+
+ params = kmalloc_array(arg->num_params, sizeof(struct tee_param),
+ GFP_KERNEL);
+ if (!params) {
+ arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
+ return;
+ }
+
+ if (optee_from_msg_param(params, arg->num_params, arg->params))
+ goto bad;
+
+ for (i = 0; i < arg->num_params; i++) {
+ type = params[i].attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK;
+ if (type != attr[i])
+ goto bad;
+ }
+
+ client.addr = params[0].u.value.c;
+ client.adapter = i2c_get_adapter(params[0].u.value.b);
+ if (!client.adapter)
+ goto bad;
+
+ snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
+
+ buf = params[1].u.memref.shm->kaddr;
+ len = params[1].u.memref.size;
+
+ switch (params[0].u.value.a) {
+ case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
+ ret = i2c_master_recv(&client, buf, len);
+ break;
+ case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
+ ret = i2c_master_send(&client, buf, len);
+ break;
+ default:
+ i2c_put_adapter(client.adapter);
+ goto bad;
+ }
+
+ if (ret >= 0) {
+ params[2].u.value.a = ret;
+ arg->ret = TEEC_SUCCESS;
+ } else {
+ arg->ret = TEEC_ERROR_COMMUNICATION;
+ }
+
+ if (optee_to_msg_param(arg->params, arg->num_params, params))
+ arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+
+ i2c_put_adapter(client.adapter);
+ kfree(params);
+ return;
+bad:
+ kfree(params);
+ arg->ret = TEEC_ERROR_BAD_PARAMETERS;
+}
+#else
+static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
+ struct optee_msg_arg *arg)
+{
+ arg->ret = TEEC_ERROR_COMMUNICATION;
+}
+#endif
+
static struct wq_entry *wq_entry_get(struct optee_wait_queue *wq, u32 key)
{
struct wq_entry *w;
@@ -382,6 +467,9 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
case OPTEE_MSG_RPC_CMD_SHM_FREE:
handle_rpc_func_cmd_shm_free(ctx, arg);
break;
+ case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
+ handle_rpc_func_cmd_i2c_transfer(ctx, arg);
+ break;
default:
handle_rpc_supp_cmd(ctx, arg);
}
--
2.17.1

2020-07-28 10:12:37

by Jorge Ramirez-Ortiz, Foundries

[permalink] [raw]

Subject: Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

On 22/07/20, Jorge Ramirez-Ortiz wrote:
> Some secure elements like NXP's SE050 sit on I2C buses. For OP-TEE to
> control this type of cryptographic devices it needs coordinated access
> to the bus, so collisions and RUNTIME_PM dont get in the way.
>
> This trampoline driver allow OP-TEE to access them.
> Tested on imx8mm LPDDR4
>
> Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> ---
> v6: compile out if CONFIG_I2C not enabled
> v5: alphabetic order of includes
> v4: remove unnecessary extra line in optee_msg.h
> v3: use from/to msg param to support all types of memory
> modify OPTEE_MSG_RPC_CMD_I2C_TRANSFER message id
>
> drivers/tee/optee/optee_msg.h | 16 +++++++
> drivers/tee/optee/rpc.c | 88 +++++++++++++++++++++++++++++++++++
> 2 files changed, 104 insertions(+)
>
> diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> index 795bc19ae17a..14b580f55356 100644
> --- a/drivers/tee/optee/optee_msg.h
> +++ b/drivers/tee/optee/optee_msg.h
> @@ -419,4 +419,20 @@ 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/out] memref[1] buffer to exchange the transfer data
> + * with the secure world
> + *
> + * [out] param[0].u.value.a bytes transferred by the driver
> + */
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
> +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
> +
> #endif /* _OPTEE_MSG_H */
> diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> index b4ade54d1f28..5fd5c6c93896 100644
> --- a/drivers/tee/optee/rpc.c
> +++ b/drivers/tee/optee/rpc.c
> @@ -7,6 +7,7 @@
>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/i2c.h>
> #include <linux/slab.h>
> #include <linux/tee_drv.h>
> #include "optee_private.h"
> @@ -49,6 +50,90 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg)
> arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> }
>
> +#if IS_ENABLED(CONFIG_I2C)
> +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> + struct optee_msg_arg *arg)
> +{
> + struct i2c_client client;
> + struct tee_param *params;
> + uint32_t type;
> + int i, ret;
> + size_t len;
> + char *buf;
> + uint32_t attr[] = {
> + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> + TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
> + };
> +
> + if (arg->num_params != ARRAY_SIZE(attr)) {
> + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> + return;
> + }
> +
> + params = kmalloc_array(arg->num_params, sizeof(struct tee_param),
> + GFP_KERNEL);
> + if (!params) {
> + arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> + return;
> + }
> +
> + if (optee_from_msg_param(params, arg->num_params, arg->params))
> + goto bad;
> +
> + for (i = 0; i < arg->num_params; i++) {
> + type = params[i].attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK;
> + if (type != attr[i])
> + goto bad;
> + }
> +
> + client.addr = params[0].u.value.c;
> + client.adapter = i2c_get_adapter(params[0].u.value.b);
> + if (!client.adapter)
> + goto bad;
> +
> + snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
> +
> + buf = params[1].u.memref.shm->kaddr;
> + len = params[1].u.memref.size;
> +
> + switch (params[0].u.value.a) {
> + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
> + ret = i2c_master_recv(&client, buf, len);
> + break;
> + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
> + ret = i2c_master_send(&client, buf, len);
> + break;
> + default:
> + i2c_put_adapter(client.adapter);
> + goto bad;
> + }
> +
> + if (ret >= 0) {
> + params[2].u.value.a = ret;
> + arg->ret = TEEC_SUCCESS;
> + } else {
> + arg->ret = TEEC_ERROR_COMMUNICATION;
> + }
> +
> + if (optee_to_msg_param(arg->params, arg->num_params, params))
> + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> +
> + i2c_put_adapter(client.adapter);
> + kfree(params);
> + return;
> +bad:
> + kfree(params);
> + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> +}
> +#else
> +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> + struct optee_msg_arg *arg)
> +{
> + arg->ret = TEEC_ERROR_COMMUNICATION;
> +}
> +#endif
> +
> static struct wq_entry *wq_entry_get(struct optee_wait_queue *wq, u32 key)
> {
> struct wq_entry *w;
> @@ -382,6 +467,9 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
> case OPTEE_MSG_RPC_CMD_SHM_FREE:
> handle_rpc_func_cmd_shm_free(ctx, arg);
> break;
> + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> + handle_rpc_func_cmd_i2c_transfer(ctx, arg);
> + break;
> default:
> handle_rpc_supp_cmd(ctx, arg);
> }

any comments to this feature?

TIA

2020-08-05 16:49:11

by Jens Wiklander

[permalink] [raw]

Subject: Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

On Wed, Aug 05, 2020 at 03:35:01PM +0200, Jorge Ramirez-Ortiz, Foundries wrote:
> On 22/07/20, Jorge Ramirez-Ortiz wrote:
> > Some secure elements like NXP's SE050 sit on I2C buses. For OP-TEE to
> > control this type of cryptographic devices it needs coordinated access
> > to the bus, so collisions and RUNTIME_PM dont get in the way.
> >
> > This trampoline driver allow OP-TEE to access them.
> > Tested on imx8mm LPDDR4
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> > ---
> > v6: compile out if CONFIG_I2C not enabled
> > v5: alphabetic order of includes
> > v4: remove unnecessary extra line in optee_msg.h
> > v3: use from/to msg param to support all types of memory
> > modify OPTEE_MSG_RPC_CMD_I2C_TRANSFER message id
> >
> > drivers/tee/optee/optee_msg.h | 16 +++++++
> > drivers/tee/optee/rpc.c | 88 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 104 insertions(+)
> >
> > diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> > index 795bc19ae17a..14b580f55356 100644
> > --- a/drivers/tee/optee/optee_msg.h
> > +++ b/drivers/tee/optee/optee_msg.h
> > @@ -419,4 +419,20 @@ 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/out] memref[1] buffer to exchange the transfer data
> > + * with the secure world
> > + *
> > + * [out] param[0].u.value.a bytes transferred by the driver
> > + */
> > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
> > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
> > +
> > #endif /* _OPTEE_MSG_H */
> > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > index b4ade54d1f28..5fd5c6c93896 100644
> > --- a/drivers/tee/optee/rpc.c
> > +++ b/drivers/tee/optee/rpc.c
> > @@ -7,6 +7,7 @@
> >
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > +#include <linux/i2c.h>
> > #include <linux/slab.h>
> > #include <linux/tee_drv.h>
> > #include "optee_private.h"
> > @@ -49,6 +50,90 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg)
> > arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > }
> >
> > +#if IS_ENABLED(CONFIG_I2C)
> > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > + struct optee_msg_arg *arg)
> > +{
> > + struct i2c_client client;
> > + struct tee_param *params;
> > + uint32_t type;
> > + int i, ret;
> > + size_t len;
> > + char *buf;
> > + uint32_t attr[] = {
> > + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > + TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> > + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
> > + };
> > +
> > + if (arg->num_params != ARRAY_SIZE(attr)) {
> > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > + return;
> > + }
> > +
> > + params = kmalloc_array(arg->num_params, sizeof(struct tee_param),
> > + GFP_KERNEL);
> > + if (!params) {
> > + arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> > + return;
> > + }
> > +
> > + if (optee_from_msg_param(params, arg->num_params, arg->params))
> > + goto bad;
> > +
> > + for (i = 0; i < arg->num_params; i++) {
> > + type = params[i].attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK;
> > + if (type != attr[i])
> > + goto bad;
> > + }
> > +
> > + client.addr = params[0].u.value.c;
> > + client.adapter = i2c_get_adapter(params[0].u.value.b);
> > + if (!client.adapter)
> > + goto bad;
> > +
> > + snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
> > +
> > + buf = params[1].u.memref.shm->kaddr;
> > + len = params[1].u.memref.size;
> > +
> > + switch (params[0].u.value.a) {
> > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
> > + ret = i2c_master_recv(&client, buf, len);
> > + break;
> > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
> > + ret = i2c_master_send(&client, buf, len);
> > + break;
> > + default:
> > + i2c_put_adapter(client.adapter);
> > + goto bad;
> > + }
> > +
> > + if (ret >= 0) {
> > + params[2].u.value.a = ret;
> > + arg->ret = TEEC_SUCCESS;
> > + } else {
> > + arg->ret = TEEC_ERROR_COMMUNICATION;
> > + }
> > +
> > + if (optee_to_msg_param(arg->params, arg->num_params, params))
> > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > +
> > + i2c_put_adapter(client.adapter);
> > + kfree(params);
> > + return;
> > +bad:
> > + kfree(params);
> > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > +}
> > +#else
> > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > + struct optee_msg_arg *arg)
> > +{
> > + arg->ret = TEEC_ERROR_COMMUNICATION;
> > +}
> > +#endif
> > +
> > static struct wq_entry *wq_entry_get(struct optee_wait_queue *wq, u32 key)
> > {
> > struct wq_entry *w;
> > @@ -382,6 +467,9 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
> > case OPTEE_MSG_RPC_CMD_SHM_FREE:
> > handle_rpc_func_cmd_shm_free(ctx, arg);
> > break;
> > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> > + handle_rpc_func_cmd_i2c_transfer(ctx, arg);
> > + break;
> > default:
> > handle_rpc_supp_cmd(ctx, arg);
> > }
>
>
> any comments please?

As you know we're still reviewing the secure world counterpart at
https://github.com/OP-TEE/optee_os/pull/3905
Where we're sorting out the ABI. Thanks for your patience.

Cheers,
Jens

2020-08-05 19:55:47

by Jorge Ramirez-Ortiz, Foundries

[permalink] [raw]

Subject: Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

2020-08-05 20:19:10

by Jorge Ramirez-Ortiz, Foundries

[permalink] [raw]

Subject: Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus

On 05/08/20, Jens Wiklander wrote:
> On Wed, Aug 05, 2020 at 03:35:01PM +0200, Jorge Ramirez-Ortiz, Foundries wrote:
> > On 22/07/20, Jorge Ramirez-Ortiz wrote:
> > > Some secure elements like NXP's SE050 sit on I2C buses. For OP-TEE to
> > > control this type of cryptographic devices it needs coordinated access
> > > to the bus, so collisions and RUNTIME_PM dont get in the way.
> > >
> > > This trampoline driver allow OP-TEE to access them.
> > > Tested on imx8mm LPDDR4
> > >
> > > Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
> > > ---
> > > v6: compile out if CONFIG_I2C not enabled
> > > v5: alphabetic order of includes
> > > v4: remove unnecessary extra line in optee_msg.h
> > > v3: use from/to msg param to support all types of memory
> > > modify OPTEE_MSG_RPC_CMD_I2C_TRANSFER message id
> > >
> > > drivers/tee/optee/optee_msg.h | 16 +++++++
> > > drivers/tee/optee/rpc.c | 88 +++++++++++++++++++++++++++++++++++
> > > 2 files changed, 104 insertions(+)
> > >
> > > diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> > > index 795bc19ae17a..14b580f55356 100644
> > > --- a/drivers/tee/optee/optee_msg.h
> > > +++ b/drivers/tee/optee/optee_msg.h
> > > @@ -419,4 +419,20 @@ 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/out] memref[1] buffer to exchange the transfer data
> > > + * with the secure world
> > > + *
> > > + * [out] param[0].u.value.a bytes transferred by the driver
> > > + */
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
> > > +
> > > #endif /* _OPTEE_MSG_H */
> > > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > > index b4ade54d1f28..5fd5c6c93896 100644
> > > --- a/drivers/tee/optee/rpc.c
> > > +++ b/drivers/tee/optee/rpc.c
> > > @@ -7,6 +7,7 @@
> > >
> > > #include <linux/delay.h>
> > > #include <linux/device.h>
> > > +#include <linux/i2c.h>
> > > #include <linux/slab.h>
> > > #include <linux/tee_drv.h>
> > > #include "optee_private.h"
> > > @@ -49,6 +50,90 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg)
> > > arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > }
> > >
> > > +#if IS_ENABLED(CONFIG_I2C)
> > > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > > + struct optee_msg_arg *arg)
> > > +{
> > > + struct i2c_client client;
> > > + struct tee_param *params;
> > > + uint32_t type;
> > > + int i, ret;
> > > + size_t len;
> > > + char *buf;
> > > + uint32_t attr[] = {
> > > + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > > + TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> > > + TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
> > > + };
> > > +
> > > + if (arg->num_params != ARRAY_SIZE(attr)) {
> > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > + return;
> > > + }
> > > +
> > > + params = kmalloc_array(arg->num_params, sizeof(struct tee_param),
> > > + GFP_KERNEL);
> > > + if (!params) {
> > > + arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> > > + return;
> > > + }
> > > +
> > > + if (optee_from_msg_param(params, arg->num_params, arg->params))
> > > + goto bad;
> > > +
> > > + for (i = 0; i < arg->num_params; i++) {
> > > + type = params[i].attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK;
> > > + if (type != attr[i])
> > > + goto bad;
> > > + }
> > > +
> > > + client.addr = params[0].u.value.c;
> > > + client.adapter = i2c_get_adapter(params[0].u.value.b);
> > > + if (!client.adapter)
> > > + goto bad;
> > > +
> > > + snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
> > > +
> > > + buf = params[1].u.memref.shm->kaddr;
> > > + len = params[1].u.memref.size;
> > > +
> > > + switch (params[0].u.value.a) {
> > > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
> > > + ret = i2c_master_recv(&client, buf, len);
> > > + break;
> > > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
> > > + ret = i2c_master_send(&client, buf, len);
> > > + break;
> > > + default:
> > > + i2c_put_adapter(client.adapter);
> > > + goto bad;
> > > + }
> > > +
> > > + if (ret >= 0) {
> > > + params[2].u.value.a = ret;
> > > + arg->ret = TEEC_SUCCESS;
> > > + } else {
> > > + arg->ret = TEEC_ERROR_COMMUNICATION;
> > > + }
> > > +
> > > + if (optee_to_msg_param(arg->params, arg->num_params, params))
> > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +
> > > + i2c_put_adapter(client.adapter);
> > > + kfree(params);
> > > + return;
> > > +bad:
> > > + kfree(params);
> > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +}
> > > +#else
> > > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > > + struct optee_msg_arg *arg)
> > > +{
> > > + arg->ret = TEEC_ERROR_COMMUNICATION;
> > > +}
> > > +#endif
> > > +
> > > static struct wq_entry *wq_entry_get(struct optee_wait_queue *wq, u32 key)
> > > {
> > > struct wq_entry *w;
> > > @@ -382,6 +467,9 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
> > > case OPTEE_MSG_RPC_CMD_SHM_FREE:
> > > handle_rpc_func_cmd_shm_free(ctx, arg);
> > > break;
> > > + case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> > > + handle_rpc_func_cmd_i2c_transfer(ctx, arg);
> > > + break;
> > > default:
> > > handle_rpc_supp_cmd(ctx, arg);
> > > }
> >
> >
> > any comments please?
>
> As you know we're still reviewing the secure world counterpart at
> https://github.com/OP-TEE/optee_os/pull/3905

yep

> Where we're sorting out the ABI. Thanks for your patience.

after the initial comments on the first patch it was not clear why
there were none on the follow up ones. but sure, thanks for taking the
time.

>
> Cheers,
> Jens