linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus
@ 2020-07-22 21:21 Jorge Ramirez-Ortiz
  2020-07-28 10:08 ` Jorge Ramirez-Ortiz, Foundries
  2020-08-05 13:35 ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 2 replies; 5+ messages in thread
From: Jorge Ramirez-Ortiz @ 2020-07-22 21:21 UTC (permalink / raw)
  To: jorge, jens.wiklander, sumit.garg; +Cc: ricardo, mike, tee-dev, linux-kernel

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 <jorge@foundries.io>
---
 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


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

* Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus
  2020-07-22 21:21 [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus Jorge Ramirez-Ortiz
@ 2020-07-28 10:08 ` Jorge Ramirez-Ortiz, Foundries
  2020-08-05 13:35 ` Jorge Ramirez-Ortiz, Foundries
  1 sibling, 0 replies; 5+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2020-07-28 10:08 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz
  Cc: jens.wiklander, sumit.garg, ricardo, mike, tee-dev, linux-kernel

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 <jorge@foundries.io>
> ---
>  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

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

* Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus
  2020-07-22 21:21 [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus Jorge Ramirez-Ortiz
  2020-07-28 10:08 ` Jorge Ramirez-Ortiz, Foundries
@ 2020-08-05 13:35 ` Jorge Ramirez-Ortiz, Foundries
  2020-08-05 14:24   ` Jens Wiklander
  1 sibling, 1 reply; 5+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2020-08-05 13:35 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz
  Cc: jens.wiklander, sumit.garg, ricardo, mike, tee-dev, linux-kernel

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 <jorge@foundries.io>
> ---
>  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?

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

* Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus
  2020-08-05 13:35 ` Jorge Ramirez-Ortiz, Foundries
@ 2020-08-05 14:24   ` Jens Wiklander
  2020-08-05 20:16     ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Wiklander @ 2020-08-05 14:24 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries
  Cc: sumit.garg, ricardo, mike, tee-dev, linux-kernel

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 <jorge@foundries.io>
> > ---
> >  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

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

* Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus
  2020-08-05 14:24   ` Jens Wiklander
@ 2020-08-05 20:16     ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 0 replies; 5+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2020-08-05 20:16 UTC (permalink / raw)
  To: Jens Wiklander
  Cc: Jorge Ramirez-Ortiz, Foundries, sumit.garg, ricardo, mike,
	tee-dev, linux-kernel

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 <jorge@foundries.io>
> > > ---
> > >  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

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

end of thread, other threads:[~2020-08-05 20:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 21:21 [PATCHv6] drivers: optee: allow op-tee to access devices on the i2c bus Jorge Ramirez-Ortiz
2020-07-28 10:08 ` Jorge Ramirez-Ortiz, Foundries
2020-08-05 13:35 ` Jorge Ramirez-Ortiz, Foundries
2020-08-05 14:24   ` Jens Wiklander
2020-08-05 20:16     ` Jorge Ramirez-Ortiz, Foundries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).