[RFC PATCH] tee: tstee: Add initial Trusted Services TEE driver
Jens Wiklander
jens.wiklander at linaro.org
Mon Oct 23 23:22:25 PDT 2023
On Fri, Oct 20, 2023 at 3:52 PM Balint Dobszay <balint.dobszay at arm.com> wrote:
>
> On 13 Oct 2023, at 14:47, Sumit Garg wrote:
> > On Wed, 27 Sept 2023 at 20:56, Balint Dobszay <balint.dobszay at arm.com> wrote:
>
> [snip]
>
> >> diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
> >> new file mode 100644
> >> index 000000000000..c0194638b7da
> >> --- /dev/null
> >> +++ b/drivers/tee/tstee/core.c
> >> @@ -0,0 +1,473 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (c) 2023, Arm Limited
> >> + */
> >> +
> >> +#define DRIVER_NAME "Arm TSTEE"
> >> +#define pr_fmt(fmt) DRIVER_NAME ": " fmt
> >> +
> >> +#include <linux/arm_ffa.h>
> >> +#include <linux/err.h>
> >> +#include <linux/errno.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/limits.h>
> >> +#include <linux/list.h>
> >> +#include <linux/mm.h>
> >> +#include <linux/module.h>
> >> +#include <linux/scatterlist.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/stat.h>
> >> +#include <linux/tee_drv.h>
> >> +#include <linux/types.h>
> >> +#include <linux/uaccess.h>
> >> +
> >> +#include "tstee_private.h"
> >> +
> >> +#define FFA_INVALID_MEM_HANDLE U64_MAX
> >> +
> >> +static void arg_list_to_ffa_data(const u32 *args, struct ffa_send_direct_data *data)
> >> +{
> >> + data->data0 = args[0];
> >> + data->data1 = args[1];
> >> + data->data2 = args[2];
> >> + data->data3 = args[3];
> >> + data->data4 = args[4];
> >> +}
> >> +
> >> +static void arg_list_from_ffa_data(const struct ffa_send_direct_data *data, u32 *args)
> >> +{
> >> + args[0] = lower_32_bits(data->data0);
> >> + args[1] = lower_32_bits(data->data1);
> >> + args[2] = lower_32_bits(data->data2);
> >> + args[3] = lower_32_bits(data->data3);
> >> + args[4] = lower_32_bits(data->data4);
> >> +}
> >> +
> >> +static void tstee_get_version(struct tee_device *teedev, struct tee_ioctl_version_data *vers)
> >> +{
> >> + struct tstee *tstee = tee_get_drvdata(teedev);
> >> + struct tee_ioctl_version_data v = {
> >> + .impl_id = TEE_IMPL_ID_TSTEE,
> >> + .impl_caps = tstee->ffa_dev->vm_id,
> >
> > So while exploring the user-space interface, I observed an anomaly
> > here. The ".impl_caps" refers to "Implementation specific
> > capabilities" meant to support backwards compatibility of a particular
> > TEE implementation. But here I observe you are using it instead for
> > endpoint_id. Can you provide the reasoning behind it? Also, do you
> > plan to support multiple endpoints via this driver?
>
> The mapping between Trusted Services SPs and TEE devices is 1:1, i.e.
> each /dev/tee<n> device represents exactly one FF-A endpoint. To answer
> your second question, each instance of the driver represents a single
> endpoint, multiple endpoints are supported by having multiple instances
> of the driver. Therefore we always return a single endpoint ID in this
> field.
>
> The reason behind the usage of this field is that somehow user space has
> to be able to discover which TEE device represents which FF-A endpoint.
> This is required when a client wants to invoke an SP with a specific
> endpoint ID, e.g. in a system where the SP's endpoint IDs is static and
> known by the client.
>
> I understand this is not a conventional usage of this field, I couldn't
> find a better way to "fit" this information into the ABI. My
> understanding is this solution shouldn't cause any issues for other
> TEEs, since this implementation specific field should only be parsed by
> a client if it has already matched on the TEE implementation ID. Please
> correct me if this assumption is wrong, or if you have any suggestions
> on other ways to expose the endpoint ID to user space.
impl_caps is a u32 and an SP ID can never use more than 16 bits. How
about making this explicit so it's clear that the upper bits are still
free for other usage?
Cheers,
Jens
>
> >> + .gen_caps = 0,
> >
> > Do you plan to support TEE_GEN_CAP_REG_MEM? IOW, do you expect
> > user-space to leverage TEE_IOC_SHM_REGISTER? I can see this driver
> > already provides tstee_shm_register() and tstee_shm_unregister()
> > callbacks.
>
> We didn't have a use case yet that would require this feature. I will
> give it some more thought, whether it's likely that it will be needed in
> the future.
>
> >> + };
> >> +
> >> + *vers = v;
> >> +}
> >> +
> >> +static int tstee_open(struct tee_context *ctx)
> >> +{
> >> + struct ts_context_data *ctxdata;
> >> +
> >> + ctxdata = kzalloc(sizeof(*ctxdata), GFP_KERNEL);
> >> + if (!ctxdata)
> >> + return -ENOMEM;
> >> +
> >> + mutex_init(&ctxdata->mutex);
> >> + idr_init(&ctxdata->sess_ids);
> >> + INIT_LIST_HEAD(&ctxdata->sess_list);
> >> +
> >> + ctx->data = ctxdata;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void tstee_release(struct tee_context *ctx)
> >> +{
> >> + struct ts_context_data *ctxdata = ctx->data;
> >> + struct ts_session *sess, *sess_tmp;
> >> +
> >> + if (!ctxdata)
> >> + return;
> >> +
> >> + list_for_each_entry_safe(sess, sess_tmp, &ctxdata->sess_list, list_node) {
> >> + list_del(&sess->list_node);
> >> + idr_remove(&ctxdata->sess_ids, sess->session_id);
> >> + kfree(sess);
> >> + }
> >> +
> >> + idr_destroy(&ctxdata->sess_ids);
> >> + mutex_destroy(&ctxdata->mutex);
> >> +
> >> + kfree(ctxdata);
> >> + ctx->data = NULL;
> >> +}
> >> +
> >> +static struct ts_session *find_session(struct ts_context_data *ctxdata, u32 session_id)
> >> +{
> >> + struct ts_session *sess;
> >> +
> >> + list_for_each_entry(sess, &ctxdata->sess_list, list_node)
> >> + if (sess->session_id == session_id)
> >> + return sess;
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +static int tstee_open_session(struct tee_context *ctx, struct tee_ioctl_open_session_arg *arg,
> >> + struct tee_param *param __always_unused)
> >> +{
> >> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> >> + struct ffa_device *ffa_dev = tstee->ffa_dev;
> >> + struct ts_context_data *ctxdata = ctx->data;
> >> + struct ffa_send_direct_data ffa_data;
> >> + struct ts_session *sess = NULL;
> >> + u32 ffa_args[5] = {};
> >
> > Please don't use magic numbers.
>
> Ack, will fix in the next version.
>
> >> + int sess_id;
> >> + int rc;
> >> +
> >> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> >> + TS_RPC_OP_SERVICE_INFO);
> >> +
> >> + memcpy((u8 *)(ffa_args + TS_RPC_SERVICE_INFO_UUID0), arg->uuid, UUID_SIZE);
> >> +
> >> + arg_list_to_ffa_data(ffa_args, &ffa_data);
> >> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> >> + if (rc)
> >> + return rc;
> >> +
> >> + arg_list_from_ffa_data(&ffa_data, ffa_args);
> >> +
> >> + if (ffa_args[TS_RPC_SERVICE_INFO_RPC_STATUS] != TS_RPC_OK)
> >> + return -ENODEV;
> >> +
> >> + if (ffa_args[TS_RPC_SERVICE_INFO_IFACE] > U8_MAX)
> >> + return -EINVAL;
> >> +
> >> + sess = kzalloc(sizeof(*sess), GFP_KERNEL);
> >> + if (!sess)
> >> + return -ENOMEM;
> >> +
> >> + sess_id = idr_alloc(&ctxdata->sess_ids, sess, 1, 0, GFP_KERNEL);
> >> + if (sess_id < 0) {
> >> + kfree(sess);
> >> + return sess_id;
> >> + }
> >> +
> >> + sess->session_id = sess_id;
> >> + sess->iface_id = ffa_args[TS_RPC_SERVICE_INFO_IFACE];
> >> +
> >> + mutex_lock(&ctxdata->mutex);
> >> + list_add(&sess->list_node, &ctxdata->sess_list);
> >> + mutex_unlock(&ctxdata->mutex);
> >> +
> >> + arg->session = sess_id;
> >> + arg->ret = 0;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int tstee_close_session(struct tee_context *ctx, u32 session)
> >> +{
> >> + struct ts_context_data *ctxdata = ctx->data;
> >> + struct ts_session *sess;
> >> +
> >> + mutex_lock(&ctxdata->mutex);
> >> + sess = find_session(ctxdata, session);
> >> + if (sess)
> >> + list_del(&sess->list_node);
> >> +
> >> + mutex_unlock(&ctxdata->mutex);
> >> +
> >> + if (!sess)
> >> + return -EINVAL;
> >> +
> >> + idr_remove(&ctxdata->sess_ids, sess->session_id);
> >> + kfree(sess);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int tstee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
> >> + struct tee_param *param)
> >> +{
> >> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> >> + struct ffa_device *ffa_dev = tstee->ffa_dev;
> >> + struct ts_context_data *ctxdata = ctx->data;
> >> + struct ffa_send_direct_data ffa_data;
> >> + struct tee_shm *shm = NULL;
> >> + struct ts_session *sess;
> >> + u32 req_len, ffa_args[5] = {};
> >
> > ditto.
>
> Ack.
>
> Regards,
> Balint
More information about the linux-arm-kernel
mailing list