2024-02-13 14:54:44

by Balint Dobszay

[permalink] [raw]
Subject: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

The Trusted Services project provides a framework for developing and
deploying device Root of Trust services in FF-A Secure Partitions. The
FF-A SPs are accessible through the FF-A driver, but this doesn't
provide a user space interface. The goal of this TEE driver is to make
Trusted Services SPs accessible for user space clients.

All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
by TS. A TS SP can host one or more services, a service is identified by
its service UUID. The same type of service cannot be present twice in
the same SP. During SP boot each service in an SP is assigned an
interface ID, this is just a short ID to simplify message addressing.
There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
device is registered for each TS SP. This is required since contrary to
the generic TEE design where memory is shared with the whole TEE
implementation, in case of FF-A, memory is shared with a specific SP. A
user space client has to be able to separately share memory with each SP
based on its endpoint ID.

Signed-off-by: Balint Dobszay <[email protected]>
---
drivers/tee/Kconfig | 1 +
drivers/tee/Makefile | 1 +
drivers/tee/tstee/Kconfig | 11 +
drivers/tee/tstee/Makefile | 3 +
drivers/tee/tstee/core.c | 501 ++++++++++++++++++++++++++++++
drivers/tee/tstee/tstee_private.h | 92 ++++++
include/uapi/linux/tee.h | 1 +
7 files changed, 610 insertions(+)
create mode 100644 drivers/tee/tstee/Kconfig
create mode 100644 drivers/tee/tstee/Makefile
create mode 100644 drivers/tee/tstee/core.c
create mode 100644 drivers/tee/tstee/tstee_private.h

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index 73a147202e88..61b507c18780 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -15,5 +15,6 @@ if TEE

source "drivers/tee/optee/Kconfig"
source "drivers/tee/amdtee/Kconfig"
+source "drivers/tee/tstee/Kconfig"

endif
diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
index 68da044afbfa..5488cba30bd2 100644
--- a/drivers/tee/Makefile
+++ b/drivers/tee/Makefile
@@ -5,3 +5,4 @@ tee-objs += tee_shm.o
tee-objs += tee_shm_pool.o
obj-$(CONFIG_OPTEE) += optee/
obj-$(CONFIG_AMDTEE) += amdtee/
+obj-$(CONFIG_ARM_TSTEE) += tstee/
diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
new file mode 100644
index 000000000000..d32f91d47398
--- /dev/null
+++ b/drivers/tee/tstee/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config ARM_TSTEE
+ tristate "Arm Trusted Services TEE driver"
+ depends on ARM_FFA_TRANSPORT
+ default n
+ help
+ The Trusted Services project provides a framework for developing and
+ deploying device Root of Trust services in FF-A Secure Partitions.
+ This driver provides an interface to make Trusted Services Secure
+ Partitions accessible for user space clients, since the FF-A driver
+ doesn't implement a user space interface directly.
diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
new file mode 100644
index 000000000000..5227020ebd30
--- /dev/null
+++ b/drivers/tee/tstee/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+arm-tstee-objs := core.o
+obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
new file mode 100644
index 000000000000..8d6bbe4d03ed
--- /dev/null
+++ b/drivers/tee/tstee/core.c
@@ -0,0 +1,501 @@
+// 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_DIRECT_REQ_ARG_NUM 5
+#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,
+ /* FF-A endpoint ID only uses the lower 16 bits */
+ .impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
+ .gen_caps = 0,
+ };
+
+ *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[FFA_DIRECT_REQ_ARG_NUM] = {};
+ 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[FFA_DIRECT_REQ_ARG_NUM] = {};
+ int shm_id, rc;
+ u8 iface_id;
+ u64 handle;
+ u16 opcode;
+
+ mutex_lock(&ctxdata->mutex);
+ sess = find_session(ctxdata, arg->session);
+
+ /* Do this while holding the mutex to make sure that the session wasn't closed meanwhile */
+ if (sess)
+ iface_id = sess->iface_id;
+
+ mutex_unlock(&ctxdata->mutex);
+ if (!sess)
+ return -EINVAL;
+
+ opcode = lower_16_bits(arg->func);
+ shm_id = lower_32_bits(param[0].u.value.a);
+ req_len = lower_32_bits(param[0].u.value.b);
+
+ if (shm_id != 0) {
+ shm = tee_shm_get_from_id(ctx, shm_id);
+ if (IS_ERR(shm))
+ return PTR_ERR(shm);
+
+ if (shm->size < req_len) {
+ pr_err("request doesn't fit into shared memory buffer\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ handle = shm->sec_world_id;
+ } else {
+ handle = FFA_INVALID_MEM_HANDLE;
+ }
+
+ ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id, opcode);
+ ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
+ ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
+ ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
+ ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
+
+ arg_list_to_ffa_data(ffa_args, &ffa_data);
+ rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+ if (rc)
+ goto out;
+
+ arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+ if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
+ pr_err("invoke_func rpc status: %d\n", ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
+ if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
+ param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
+
+out:
+ if (shm)
+ tee_shm_put(shm);
+
+ return rc;
+}
+
+static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm, struct page **pages,
+ size_t num_pages, unsigned long start __always_unused)
+{
+ struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+ struct ffa_device *ffa_dev = tstee->ffa_dev;
+ struct ffa_mem_region_attributes mem_attr = {
+ .receiver = tstee->ffa_dev->vm_id,
+ .attrs = FFA_MEM_RW,
+ .flag = 0,
+ };
+ struct ffa_mem_ops_args mem_args = {
+ .attrs = &mem_attr,
+ .use_txbuf = true,
+ .nattrs = 1,
+ .flags = 0,
+ };
+ struct ffa_send_direct_data ffa_data;
+ struct sg_table sgt;
+ u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+ int rc;
+
+ rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0, num_pages * PAGE_SIZE,
+ GFP_KERNEL);
+ if (rc)
+ return rc;
+
+ mem_args.sg = sgt.sgl;
+ rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
+ sg_free_table(&sgt);
+ if (rc)
+ return rc;
+
+ shm->sec_world_id = mem_args.g_handle;
+
+ ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+ TS_RPC_OP_RETRIEVE_MEM);
+ ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
+ ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
+ ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
+ ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
+
+ arg_list_to_ffa_data(ffa_args, &ffa_data);
+ rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
+ if (rc) {
+ (void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
+ return rc;
+ }
+
+ arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+ if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
+ pr_err("shm_register rpc status: %d\n", ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
+ ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
+{
+ struct tstee *tstee = tee_get_drvdata(ctx->teedev);
+ struct ffa_device *ffa_dev = tstee->ffa_dev;
+ struct ffa_send_direct_data ffa_data;
+ u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+ int rc;
+
+ ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+ TS_RPC_OP_RELINQ_MEM);
+ ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
+ ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
+
+ 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_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
+ pr_err("shm_unregister rpc status: %d\n", ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
+ return -EINVAL;
+ }
+
+ rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
+
+ return rc;
+}
+
+static const struct tee_driver_ops tstee_ops = {
+ .get_version = tstee_get_version,
+ .open = tstee_open,
+ .release = tstee_release,
+ .open_session = tstee_open_session,
+ .close_session = tstee_close_session,
+ .invoke_func = tstee_invoke_func,
+ .shm_register = tstee_shm_register,
+ .shm_unregister = tstee_shm_unregister,
+};
+
+static const struct tee_desc tstee_desc = {
+ .name = "tstee-clnt",
+ .ops = &tstee_ops,
+ .owner = THIS_MODULE,
+};
+
+static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm, size_t size, size_t align)
+{
+ return tee_shm_pool_op_alloc_helper(pool, shm, size, align, tstee_shm_register);
+}
+
+static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
+{
+ tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
+}
+
+static void pool_op_destroy_pool(struct tee_shm_pool *pool)
+{
+ kfree(pool);
+}
+
+static const struct tee_shm_pool_ops pool_ops = {
+ .alloc = pool_op_alloc,
+ .free = pool_op_free,
+ .destroy_pool = pool_op_destroy_pool,
+};
+
+static struct tee_shm_pool *tstee_create_shm_pool(void)
+{
+ struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+
+ if (!pool)
+ return ERR_PTR(-ENOMEM);
+
+ pool->ops = &pool_ops;
+
+ return pool;
+}
+
+static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
+{
+ struct ffa_send_direct_data ffa_data;
+ u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
+
+ ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
+ TS_RPC_OP_GET_VERSION);
+
+ arg_list_to_ffa_data(ffa_args, &ffa_data);
+ if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
+ return false;
+
+ arg_list_from_ffa_data(&ffa_data, ffa_args);
+
+ return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
+}
+
+static void tstee_deinit_common(struct tstee *tstee)
+{
+ tee_device_unregister(tstee->teedev);
+ if (tstee->pool)
+ tee_shm_pool_free(tstee->pool);
+
+ kfree(tstee);
+}
+
+static int tstee_probe(struct ffa_device *ffa_dev)
+{
+ struct tstee *tstee;
+ int rc;
+
+ ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
+
+ if (!tstee_check_rpc_compatible(ffa_dev))
+ return -EINVAL;
+
+ tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
+ if (!tstee)
+ return -ENOMEM;
+
+ tstee->ffa_dev = ffa_dev;
+
+ tstee->pool = tstee_create_shm_pool();
+ if (IS_ERR(tstee->pool)) {
+ rc = PTR_ERR(tstee->pool);
+ tstee->pool = NULL;
+ goto err;
+ }
+
+ tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
+ if (IS_ERR(tstee->teedev)) {
+ rc = PTR_ERR(tstee->teedev);
+ tstee->teedev = NULL;
+ goto err;
+ }
+
+ rc = tee_device_register(tstee->teedev);
+ if (rc)
+ goto err;
+
+ ffa_dev_set_drvdata(ffa_dev, tstee);
+
+ pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);
+
+ return 0;
+
+err:
+ tstee_deinit_common(tstee);
+ return rc;
+}
+
+static void tstee_remove(struct ffa_device *ffa_dev)
+{
+ tstee_deinit_common(ffa_dev->dev.driver_data);
+}
+
+static const struct ffa_device_id tstee_device_ids[] = {
+ /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
+ { TS_RPC_UUID },
+ {}
+};
+
+static struct ffa_driver tstee_driver = {
+ .name = "arm_tstee",
+ .probe = tstee_probe,
+ .remove = tstee_remove,
+ .id_table = tstee_device_ids,
+};
+
+static int __init mod_init(void)
+{
+ return ffa_register(&tstee_driver);
+}
+module_init(mod_init)
+
+static void __exit mod_exit(void)
+{
+ ffa_unregister(&tstee_driver);
+}
+module_exit(mod_exit)
+
+MODULE_ALIAS("arm-tstee");
+MODULE_AUTHOR("Balint Dobszay <[email protected]>");
+MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
new file mode 100644
index 000000000000..81eeda220a5c
--- /dev/null
+++ b/drivers/tee/tstee/tstee_private.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023, Arm Limited
+ */
+
+#ifndef TSTEE_PRIVATE_H
+#define TSTEE_PRIVATE_H
+
+#include <linux/arm_ffa.h>
+#include <linux/bitops.h>
+#include <linux/idr.h>
+#include <linux/tee_drv.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+/* UUID of this protocol */
+#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
+ 0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
+
+/* Protocol version*/
+#define TS_RPC_PROTOCOL_VERSION (1)
+
+/* Status codes */
+#define TS_RPC_OK (0)
+
+/* RPC control register */
+#define TS_RPC_CTRL_REG (0)
+#define OPCODE_MASK GENMASK(15, 0)
+#define IFACE_ID_MASK GENMASK(23, 16)
+#define TS_RPC_CTRL_OPCODE(x) ((u16)(FIELD_GET(OPCODE_MASK, (x))))
+#define TS_RPC_CTRL_IFACE_ID(x) ((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
+#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o) \
+ (FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
+#define TS_RPC_CTRL_SAP_RC BIT(30)
+#define TS_RPC_CTRL_SAP_ERR BIT(31)
+
+/* Interface ID for RPC management operations */
+#define TS_RPC_MGMT_IFACE_ID (0xff)
+
+/* Management calls */
+#define TS_RPC_OP_GET_VERSION (0x0000)
+#define TS_RPC_GET_VERSION_RESP (1)
+
+#define TS_RPC_OP_RETRIEVE_MEM (0x0001)
+#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW (1)
+#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW (2)
+#define TS_RPC_RETRIEVE_MEM_TAG_LSW (3)
+#define TS_RPC_RETRIEVE_MEM_TAG_MSW (4)
+#define TS_RPC_RETRIEVE_MEM_RPC_STATUS (1)
+
+#define TS_RPC_OP_RELINQ_MEM (0x0002)
+#define TS_RPC_RELINQ_MEM_HANDLE_LSW (1)
+#define TS_RPC_RELINQ_MEM_HANDLE_MSW (2)
+#define TS_RPC_RELINQ_MEM_RPC_STATUS (1)
+
+#define TS_RPC_OP_SERVICE_INFO (0x0003)
+#define TS_RPC_SERVICE_INFO_UUID0 (1)
+#define TS_RPC_SERVICE_INFO_UUID1 (2)
+#define TS_RPC_SERVICE_INFO_UUID2 (3)
+#define TS_RPC_SERVICE_INFO_UUID3 (4)
+#define TS_RPC_SERVICE_INFO_RPC_STATUS (1)
+#define TS_RPC_SERVICE_INFO_IFACE (2)
+
+/* Service call */
+#define TS_RPC_SERVICE_MEM_HANDLE_LSW (1)
+#define TS_RPC_SERVICE_MEM_HANDLE_MSW (2)
+#define TS_RPC_SERVICE_REQ_LEN (3)
+#define TS_RPC_SERVICE_CLIENT_ID (4)
+#define TS_RPC_SERVICE_RPC_STATUS (1)
+#define TS_RPC_SERVICE_STATUS (2)
+#define TS_RPC_SERVICE_RESP_LEN (3)
+
+struct tstee {
+ struct ffa_device *ffa_dev;
+ struct tee_device *teedev;
+ struct tee_shm_pool *pool;
+};
+
+struct ts_session {
+ struct list_head list_node;
+ u32 session_id;
+ u8 iface_id;
+};
+
+struct ts_context_data {
+ struct list_head sess_list;
+ struct idr sess_ids;
+ /* Serializes access to this struct */
+ struct mutex mutex;
+};
+
+#endif /* TSTEE_PRIVATE_H */
diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
index 23e57164693c..d0430bee8292 100644
--- a/include/uapi/linux/tee.h
+++ b/include/uapi/linux/tee.h
@@ -56,6 +56,7 @@
*/
#define TEE_IMPL_ID_OPTEE 1
#define TEE_IMPL_ID_AMDTEE 2
+#define TEE_IMPL_ID_TSTEE 3

/*
* OP-TEE specific capabilities
--
2.34.1



2024-02-15 09:04:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

On 13/02/2024 15:52, Balint Dobszay wrote:
> The Trusted Services project provides a framework for developing and
> deploying device Root of Trust services in FF-A Secure Partitions. The
> FF-A SPs are accessible through the FF-A driver, but this doesn't
> provide a user space interface. The goal of this TEE driver is to make
> Trusted Services SPs accessible for user space clients.
>
> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
> by TS. A TS SP can host one or more services, a service is identified by
> its service UUID. The same type of service cannot be present twice in
> the same SP. During SP boot each service in an SP is assigned an
> interface ID, this is just a short ID to simplify message addressing.
> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
> device is registered for each TS SP. This is required since contrary to
> the generic TEE design where memory is shared with the whole TEE
> implementation, in case of FF-A, memory is shared with a specific SP. A
> user space client has to be able to separately share memory with each SP
> based on its endpoint ID.
>
> Signed-off-by: Balint Dobszay <[email protected]>
> ---


> +static int tstee_probe(struct ffa_device *ffa_dev)
> +{
> + struct tstee *tstee;
> + int rc;
> +
> + ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
> +
> + if (!tstee_check_rpc_compatible(ffa_dev))
> + return -EINVAL;
> +
> + tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
> + if (!tstee)
> + return -ENOMEM;
> +
> + tstee->ffa_dev = ffa_dev;
> +
> + tstee->pool = tstee_create_shm_pool();
> + if (IS_ERR(tstee->pool)) {
> + rc = PTR_ERR(tstee->pool);
> + tstee->pool = NULL;
> + goto err;

Is it logically correct to call here tee_device_unregister()?

> + }
> +
> + tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
> + if (IS_ERR(tstee->teedev)) {
> + rc = PTR_ERR(tstee->teedev);
> + tstee->teedev = NULL;
> + goto err;
> + }
> +
> + rc = tee_device_register(tstee->teedev);
> + if (rc)
> + goto err;
> +
> + ffa_dev_set_drvdata(ffa_dev, tstee);
> +
> + pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);

Don't print simple probe success messages. Anyway all prints in device
context should be dev_*.

> +
> + return 0;
> +
> +err:
> + tstee_deinit_common(tstee);
> + return rc;
> +}
> +
> +static void tstee_remove(struct ffa_device *ffa_dev)
> +{
> + tstee_deinit_common(ffa_dev->dev.driver_data);
> +}
> +
> +static const struct ffa_device_id tstee_device_ids[] = {
> + /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
> + { TS_RPC_UUID },
> + {}
> +};
> +
> +static struct ffa_driver tstee_driver = {
> + .name = "arm_tstee",
> + .probe = tstee_probe,
> + .remove = tstee_remove,
> + .id_table = tstee_device_ids,
> +};
> +
> +static int __init mod_init(void)
> +{
> + return ffa_register(&tstee_driver);
> +}
> +module_init(mod_init)
> +
> +static void __exit mod_exit(void)
> +{
> + ffa_unregister(&tstee_driver);
> +}
> +module_exit(mod_exit)
> +
> +MODULE_ALIAS("arm-tstee");

Why do you need this alias? I don't see MODULE_DEVICE_TABLE, so how this
bus handles module loading?


Best regards,
Krzysztof


2024-02-15 10:01:49

by Jens Wiklander

[permalink] [raw]
Subject: Re: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

Hi Balint,

On Tue, Feb 13, 2024 at 3:54 PM Balint Dobszay <[email protected]> wrote:
>
> The Trusted Services project provides a framework for developing and
> deploying device Root of Trust services in FF-A Secure Partitions. The
> FF-A SPs are accessible through the FF-A driver, but this doesn't
> provide a user space interface. The goal of this TEE driver is to make
> Trusted Services SPs accessible for user space clients.
>
> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
> by TS. A TS SP can host one or more services, a service is identified by
> its service UUID. The same type of service cannot be present twice in
> the same SP. During SP boot each service in an SP is assigned an
> interface ID, this is just a short ID to simplify message addressing.
> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
> device is registered for each TS SP. This is required since contrary to
> the generic TEE design where memory is shared with the whole TEE
> implementation, in case of FF-A, memory is shared with a specific SP. A
> user space client has to be able to separately share memory with each SP
> based on its endpoint ID.
>
> Signed-off-by: Balint Dobszay <[email protected]>
> ---
> drivers/tee/Kconfig | 1 +
> drivers/tee/Makefile | 1 +
> drivers/tee/tstee/Kconfig | 11 +
> drivers/tee/tstee/Makefile | 3 +
> drivers/tee/tstee/core.c | 501 ++++++++++++++++++++++++++++++
> drivers/tee/tstee/tstee_private.h | 92 ++++++
> include/uapi/linux/tee.h | 1 +
> 7 files changed, 610 insertions(+)
> create mode 100644 drivers/tee/tstee/Kconfig
> create mode 100644 drivers/tee/tstee/Makefile
> create mode 100644 drivers/tee/tstee/core.c
> create mode 100644 drivers/tee/tstee/tstee_private.h
>
> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> index 73a147202e88..61b507c18780 100644
> --- a/drivers/tee/Kconfig
> +++ b/drivers/tee/Kconfig
> @@ -15,5 +15,6 @@ if TEE
>
> source "drivers/tee/optee/Kconfig"
> source "drivers/tee/amdtee/Kconfig"
> +source "drivers/tee/tstee/Kconfig"
>
> endif
> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
> index 68da044afbfa..5488cba30bd2 100644
> --- a/drivers/tee/Makefile
> +++ b/drivers/tee/Makefile
> @@ -5,3 +5,4 @@ tee-objs += tee_shm.o
> tee-objs += tee_shm_pool.o
> obj-$(CONFIG_OPTEE) += optee/
> obj-$(CONFIG_AMDTEE) += amdtee/
> +obj-$(CONFIG_ARM_TSTEE) += tstee/
> diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
> new file mode 100644
> index 000000000000..d32f91d47398
> --- /dev/null
> +++ b/drivers/tee/tstee/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config ARM_TSTEE
> + tristate "Arm Trusted Services TEE driver"
> + depends on ARM_FFA_TRANSPORT
> + default n
> + help
> + The Trusted Services project provides a framework for developing and
> + deploying device Root of Trust services in FF-A Secure Partitions.
> + This driver provides an interface to make Trusted Services Secure
> + Partitions accessible for user space clients, since the FF-A driver
> + doesn't implement a user space interface directly.
> diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
> new file mode 100644
> index 000000000000..5227020ebd30
> --- /dev/null
> +++ b/drivers/tee/tstee/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +arm-tstee-objs := core.o
> +obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
> diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
> new file mode 100644
> index 000000000000..8d6bbe4d03ed
> --- /dev/null
> +++ b/drivers/tee/tstee/core.c
> @@ -0,0 +1,501 @@
> +// 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_DIRECT_REQ_ARG_NUM 5
> +#define FFA_INVALID_MEM_HANDLE U64_MAX
> +
> +static void arg_list_to_ffa_data(const u32 *args, struct ffa_send_direct_data *data)

The preferred limit on the length of a single line is 80 columns
https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings


> +{
> + 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,
> + /* FF-A endpoint ID only uses the lower 16 bits */
> + .impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
> + .gen_caps = 0,
> + };
> +
> + *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;

Since you have an idr, why not use idr_find() instead?

> +
> + 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[FFA_DIRECT_REQ_ARG_NUM] = {};
> + 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);

Is the cast needed?

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

This needs to be protected by the mutex.

> + 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[FFA_DIRECT_REQ_ARG_NUM] = {};
> + int shm_id, rc;
> + u8 iface_id;
> + u64 handle;
> + u16 opcode;
> +
> + mutex_lock(&ctxdata->mutex);
> + sess = find_session(ctxdata, arg->session);
> +
> + /* Do this while holding the mutex to make sure that the session wasn't closed meanwhile */
> + if (sess)
> + iface_id = sess->iface_id;
> +
> + mutex_unlock(&ctxdata->mutex);
> + if (!sess)
> + return -EINVAL;
> +
> + opcode = lower_16_bits(arg->func);
> + shm_id = lower_32_bits(param[0].u.value.a);
> + req_len = lower_32_bits(param[0].u.value.b);
> +
> + if (shm_id != 0) {
> + shm = tee_shm_get_from_id(ctx, shm_id);
> + if (IS_ERR(shm))
> + return PTR_ERR(shm);
> +
> + if (shm->size < req_len) {
> + pr_err("request doesn't fit into shared memory buffer\n");
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + handle = shm->sec_world_id;
> + } else {
> + handle = FFA_INVALID_MEM_HANDLE;
> + }
> +
> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id, opcode);
> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
> + ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
> + ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
> +
> + arg_list_to_ffa_data(ffa_args, &ffa_data);
> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> + if (rc)
> + goto out;
> +
> + arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> + if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
> + pr_err("invoke_func rpc status: %d\n", ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
> + if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
> + param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
> +
> +out:
> + if (shm)
> + tee_shm_put(shm);
> +
> + return rc;
> +}
> +
> +static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm, struct page **pages,
> + size_t num_pages, unsigned long start __always_unused)
> +{
> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> + struct ffa_device *ffa_dev = tstee->ffa_dev;
> + struct ffa_mem_region_attributes mem_attr = {
> + .receiver = tstee->ffa_dev->vm_id,
> + .attrs = FFA_MEM_RW,
> + .flag = 0,
> + };
> + struct ffa_mem_ops_args mem_args = {
> + .attrs = &mem_attr,
> + .use_txbuf = true,
> + .nattrs = 1,
> + .flags = 0,
> + };
> + struct ffa_send_direct_data ffa_data;
> + struct sg_table sgt;
> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> + int rc;
> +
> + rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0, num_pages * PAGE_SIZE,
> + GFP_KERNEL);
> + if (rc)
> + return rc;
> +
> + mem_args.sg = sgt.sgl;
> + rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
> + sg_free_table(&sgt);
> + if (rc)
> + return rc;
> +
> + shm->sec_world_id = mem_args.g_handle;
> +
> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> + TS_RPC_OP_RETRIEVE_MEM);
> + ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
> + ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
> + ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
> + ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
> +
> + arg_list_to_ffa_data(ffa_args, &ffa_data);
> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
> + if (rc) {
> + (void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
> + return rc;
> + }
> +
> + arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> + if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
> + pr_err("shm_register rpc status: %d\n", ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
> + ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
> +{
> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
> + struct ffa_device *ffa_dev = tstee->ffa_dev;
> + struct ffa_send_direct_data ffa_data;
> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> + int rc;
> +
> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> + TS_RPC_OP_RELINQ_MEM);
> + ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
> + ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
> +
> + 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_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
> + pr_err("shm_unregister rpc status: %d\n", ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
> + return -EINVAL;
> + }
> +
> + rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
> +
> + return rc;
> +}
> +
> +static const struct tee_driver_ops tstee_ops = {
> + .get_version = tstee_get_version,
> + .open = tstee_open,
> + .release = tstee_release,
> + .open_session = tstee_open_session,
> + .close_session = tstee_close_session,
> + .invoke_func = tstee_invoke_func,
> + .shm_register = tstee_shm_register,
> + .shm_unregister = tstee_shm_unregister,
> +};
> +
> +static const struct tee_desc tstee_desc = {
> + .name = "tstee-clnt",
> + .ops = &tstee_ops,
> + .owner = THIS_MODULE,
> +};
> +
> +static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm, size_t size, size_t align)
> +{
> + return tee_shm_pool_op_alloc_helper(pool, shm, size, align, tstee_shm_register);
> +}
> +
> +static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
> +{
> + tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
> +}
> +
> +static void pool_op_destroy_pool(struct tee_shm_pool *pool)
> +{
> + kfree(pool);
> +}
> +
> +static const struct tee_shm_pool_ops pool_ops = {
> + .alloc = pool_op_alloc,
> + .free = pool_op_free,
> + .destroy_pool = pool_op_destroy_pool,
> +};
> +
> +static struct tee_shm_pool *tstee_create_shm_pool(void)
> +{
> + struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> +
> + if (!pool)
> + return ERR_PTR(-ENOMEM);
> +
> + pool->ops = &pool_ops;
> +
> + return pool;
> +}
> +
> +static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
> +{
> + struct ffa_send_direct_data ffa_data;
> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
> +
> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
> + TS_RPC_OP_GET_VERSION);
> +
> + arg_list_to_ffa_data(ffa_args, &ffa_data);
> + if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
> + return false;
> +
> + arg_list_from_ffa_data(&ffa_data, ffa_args);
> +
> + return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
> +}
> +
> +static void tstee_deinit_common(struct tstee *tstee)
> +{
> + tee_device_unregister(tstee->teedev);
> + if (tstee->pool)
> + tee_shm_pool_free(tstee->pool);
> +
> + kfree(tstee);
> +}
> +
> +static int tstee_probe(struct ffa_device *ffa_dev)
> +{
> + struct tstee *tstee;
> + int rc;
> +
> + ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
> +
> + if (!tstee_check_rpc_compatible(ffa_dev))
> + return -EINVAL;
> +
> + tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
> + if (!tstee)
> + return -ENOMEM;
> +
> + tstee->ffa_dev = ffa_dev;
> +
> + tstee->pool = tstee_create_shm_pool();
> + if (IS_ERR(tstee->pool)) {
> + rc = PTR_ERR(tstee->pool);
> + tstee->pool = NULL;
> + goto err;
> + }
> +
> + tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
> + if (IS_ERR(tstee->teedev)) {
> + rc = PTR_ERR(tstee->teedev);
> + tstee->teedev = NULL;
> + goto err;
> + }
> +
> + rc = tee_device_register(tstee->teedev);
> + if (rc)
> + goto err;
> +
> + ffa_dev_set_drvdata(ffa_dev, tstee);
> +
> + pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);
> +
> + return 0;
> +
> +err:
> + tstee_deinit_common(tstee);
> + return rc;
> +}
> +
> +static void tstee_remove(struct ffa_device *ffa_dev)
> +{
> + tstee_deinit_common(ffa_dev->dev.driver_data);
> +}
> +
> +static const struct ffa_device_id tstee_device_ids[] = {
> + /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
> + { TS_RPC_UUID },
> + {}
> +};
> +
> +static struct ffa_driver tstee_driver = {
> + .name = "arm_tstee",
> + .probe = tstee_probe,
> + .remove = tstee_remove,
> + .id_table = tstee_device_ids,
> +};
> +
> +static int __init mod_init(void)
> +{
> + return ffa_register(&tstee_driver);
> +}
> +module_init(mod_init)
> +
> +static void __exit mod_exit(void)
> +{
> + ffa_unregister(&tstee_driver);
> +}
> +module_exit(mod_exit)
> +
> +MODULE_ALIAS("arm-tstee");
> +MODULE_AUTHOR("Balint Dobszay <[email protected]>");
> +MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
> new file mode 100644
> index 000000000000..81eeda220a5c
> --- /dev/null
> +++ b/drivers/tee/tstee/tstee_private.h
> @@ -0,0 +1,92 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2023, Arm Limited
> + */
> +
> +#ifndef TSTEE_PRIVATE_H
> +#define TSTEE_PRIVATE_H
> +
> +#include <linux/arm_ffa.h>
> +#include <linux/bitops.h>
> +#include <linux/idr.h>
> +#include <linux/tee_drv.h>
> +#include <linux/types.h>
> +#include <linux/uuid.h>
> +
> +/* UUID of this protocol */

It would be nice to have a link or reference to the description of the ABI.

> +#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
> + 0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
> +
> +/* Protocol version*/
> +#define TS_RPC_PROTOCOL_VERSION (1)
> +
> +/* Status codes */
> +#define TS_RPC_OK (0)
> +
> +/* RPC control register */
> +#define TS_RPC_CTRL_REG (0)
> +#define OPCODE_MASK GENMASK(15, 0)
> +#define IFACE_ID_MASK GENMASK(23, 16)
> +#define TS_RPC_CTRL_OPCODE(x) ((u16)(FIELD_GET(OPCODE_MASK, (x))))
> +#define TS_RPC_CTRL_IFACE_ID(x) ((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
> +#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o) \
> + (FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
> +#define TS_RPC_CTRL_SAP_RC BIT(30)
> +#define TS_RPC_CTRL_SAP_ERR BIT(31)
> +
> +/* Interface ID for RPC management operations */
> +#define TS_RPC_MGMT_IFACE_ID (0xff)
> +
> +/* Management calls */
> +#define TS_RPC_OP_GET_VERSION (0x0000)
> +#define TS_RPC_GET_VERSION_RESP (1)
> +
> +#define TS_RPC_OP_RETRIEVE_MEM (0x0001)
> +#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW (1)
> +#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW (2)
> +#define TS_RPC_RETRIEVE_MEM_TAG_LSW (3)
> +#define TS_RPC_RETRIEVE_MEM_TAG_MSW (4)
> +#define TS_RPC_RETRIEVE_MEM_RPC_STATUS (1)
> +
> +#define TS_RPC_OP_RELINQ_MEM (0x0002)
> +#define TS_RPC_RELINQ_MEM_HANDLE_LSW (1)
> +#define TS_RPC_RELINQ_MEM_HANDLE_MSW (2)
> +#define TS_RPC_RELINQ_MEM_RPC_STATUS (1)
> +
> +#define TS_RPC_OP_SERVICE_INFO (0x0003)
> +#define TS_RPC_SERVICE_INFO_UUID0 (1)
> +#define TS_RPC_SERVICE_INFO_UUID1 (2)
> +#define TS_RPC_SERVICE_INFO_UUID2 (3)
> +#define TS_RPC_SERVICE_INFO_UUID3 (4)
> +#define TS_RPC_SERVICE_INFO_RPC_STATUS (1)
> +#define TS_RPC_SERVICE_INFO_IFACE (2)
> +
> +/* Service call */
> +#define TS_RPC_SERVICE_MEM_HANDLE_LSW (1)
> +#define TS_RPC_SERVICE_MEM_HANDLE_MSW (2)
> +#define TS_RPC_SERVICE_REQ_LEN (3)
> +#define TS_RPC_SERVICE_CLIENT_ID (4)
> +#define TS_RPC_SERVICE_RPC_STATUS (1)
> +#define TS_RPC_SERVICE_STATUS (2)
> +#define TS_RPC_SERVICE_RESP_LEN (3)
> +
> +struct tstee {
> + struct ffa_device *ffa_dev;
> + struct tee_device *teedev;
> + struct tee_shm_pool *pool;
> +};
> +
> +struct ts_session {
> + struct list_head list_node;
> + u32 session_id;
> + u8 iface_id;
> +};
> +
> +struct ts_context_data {
> + struct list_head sess_list;
> + struct idr sess_ids;

Why do you need both a linked list and an IDR? Wouldn't the IDR be enough?

Cheers,
Jens

> + /* Serializes access to this struct */
> + struct mutex mutex;
> +};
> +
> +#endif /* TSTEE_PRIVATE_H */
> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> index 23e57164693c..d0430bee8292 100644
> --- a/include/uapi/linux/tee.h
> +++ b/include/uapi/linux/tee.h
> @@ -56,6 +56,7 @@
> */
> #define TEE_IMPL_ID_OPTEE 1
> #define TEE_IMPL_ID_AMDTEE 2
> +#define TEE_IMPL_ID_TSTEE 3
>
> /*
> * OP-TEE specific capabilities
> --
> 2.34.1
>

2024-02-15 11:09:12

by Jens Wiklander

[permalink] [raw]
Subject: Re: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

On Thu, Feb 15, 2024 at 9:59 AM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 13/02/2024 15:52, Balint Dobszay wrote:
> > The Trusted Services project provides a framework for developing and
> > deploying device Root of Trust services in FF-A Secure Partitions. The
> > FF-A SPs are accessible through the FF-A driver, but this doesn't
> > provide a user space interface. The goal of this TEE driver is to make
> > Trusted Services SPs accessible for user space clients.
> >
> > All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
> > by TS. A TS SP can host one or more services, a service is identified by
> > its service UUID. The same type of service cannot be present twice in
> > the same SP. During SP boot each service in an SP is assigned an
> > interface ID, this is just a short ID to simplify message addressing.
> > There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
> > device is registered for each TS SP. This is required since contrary to
> > the generic TEE design where memory is shared with the whole TEE
> > implementation, in case of FF-A, memory is shared with a specific SP. A
> > user space client has to be able to separately share memory with each SP
> > based on its endpoint ID.
> >
> > Signed-off-by: Balint Dobszay <[email protected]>
> > ---
>
>
> > +static int tstee_probe(struct ffa_device *ffa_dev)
> > +{
> > + struct tstee *tstee;
> > + int rc;
> > +
> > + ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
> > +
> > + if (!tstee_check_rpc_compatible(ffa_dev))
> > + return -EINVAL;
> > +
> > + tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
> > + if (!tstee)
> > + return -ENOMEM;
> > +
> > + tstee->ffa_dev = ffa_dev;
> > +
> > + tstee->pool = tstee_create_shm_pool();
> > + if (IS_ERR(tstee->pool)) {
> > + rc = PTR_ERR(tstee->pool);
> > + tstee->pool = NULL;
> > + goto err;
>
> Is it logically correct to call here tee_device_unregister()?

It is harmless since it ignores null pointers, but you have a point.
It doesn't make sense to call tee_device_unregister() before
tee_device_register() has been called.

Thanks,
Jens

>
> > + }
> > +
> > + tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
> > + if (IS_ERR(tstee->teedev)) {
> > + rc = PTR_ERR(tstee->teedev);
> > + tstee->teedev = NULL;
> > + goto err;
> > + }
> > +
> > + rc = tee_device_register(tstee->teedev);
> > + if (rc)
> > + goto err;
> > +
> > + ffa_dev_set_drvdata(ffa_dev, tstee);
> > +
> > + pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);
>
> Don't print simple probe success messages. Anyway all prints in device
> context should be dev_*.
>
> > +
> > + return 0;
> > +
> > +err:
> > + tstee_deinit_common(tstee);
> > + return rc;
> > +}
> > +
> > +static void tstee_remove(struct ffa_device *ffa_dev)
> > +{
> > + tstee_deinit_common(ffa_dev->dev.driver_data);
> > +}
> > +
> > +static const struct ffa_device_id tstee_device_ids[] = {
> > + /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
> > + { TS_RPC_UUID },
> > + {}
> > +};
> > +
> > +static struct ffa_driver tstee_driver = {
> > + .name = "arm_tstee",
> > + .probe = tstee_probe,
> > + .remove = tstee_remove,
> > + .id_table = tstee_device_ids,
> > +};
> > +
> > +static int __init mod_init(void)
> > +{
> > + return ffa_register(&tstee_driver);
> > +}
> > +module_init(mod_init)
> > +
> > +static void __exit mod_exit(void)
> > +{
> > + ffa_unregister(&tstee_driver);
> > +}
> > +module_exit(mod_exit)
> > +
> > +MODULE_ALIAS("arm-tstee");
>
> Why do you need this alias? I don't see MODULE_DEVICE_TABLE, so how this
> bus handles module loading?
>
>
> Best regards,
> Krzysztof
>

2024-02-21 10:04:18

by Balint Dobszay

[permalink] [raw]
Subject: Re: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

Hi Jens,

Thanks for the feedback. I noticed that IDR is deprecated now so for the
next version I replaced it with XArray, which also addresses some of the
issues you found.

On 15 Feb 2024, at 11:00, Jens Wiklander wrote:

> Hi Balint,
>
> On Tue, Feb 13, 2024 at 3:54 PM Balint Dobszay <[email protected]> wrote:
>>
>> The Trusted Services project provides a framework for developing and
>> deploying device Root of Trust services in FF-A Secure Partitions. The
>> FF-A SPs are accessible through the FF-A driver, but this doesn't
>> provide a user space interface. The goal of this TEE driver is to make
>> Trusted Services SPs accessible for user space clients.
>>
>> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
>> by TS. A TS SP can host one or more services, a service is identified by
>> its service UUID. The same type of service cannot be present twice in
>> the same SP. During SP boot each service in an SP is assigned an
>> interface ID, this is just a short ID to simplify message addressing.
>> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
>> device is registered for each TS SP. This is required since contrary to
>> the generic TEE design where memory is shared with the whole TEE
>> implementation, in case of FF-A, memory is shared with a specific SP. A
>> user space client has to be able to separately share memory with each SP
>> based on its endpoint ID.
>>
>> Signed-off-by: Balint Dobszay <[email protected]>
>> ---
>> drivers/tee/Kconfig | 1 +
>> drivers/tee/Makefile | 1 +
>> drivers/tee/tstee/Kconfig | 11 +
>> drivers/tee/tstee/Makefile | 3 +
>> drivers/tee/tstee/core.c | 501 ++++++++++++++++++++++++++++++
>> drivers/tee/tstee/tstee_private.h | 92 ++++++
>> include/uapi/linux/tee.h | 1 +
>> 7 files changed, 610 insertions(+)
>> create mode 100644 drivers/tee/tstee/Kconfig
>> create mode 100644 drivers/tee/tstee/Makefile
>> create mode 100644 drivers/tee/tstee/core.c
>> create mode 100644 drivers/tee/tstee/tstee_private.h
>>
>> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
>> index 73a147202e88..61b507c18780 100644
>> --- a/drivers/tee/Kconfig
>> +++ b/drivers/tee/Kconfig
>> @@ -15,5 +15,6 @@ if TEE
>>
>> source "drivers/tee/optee/Kconfig"
>> source "drivers/tee/amdtee/Kconfig"
>> +source "drivers/tee/tstee/Kconfig"
>>
>> endif
>> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
>> index 68da044afbfa..5488cba30bd2 100644
>> --- a/drivers/tee/Makefile
>> +++ b/drivers/tee/Makefile
>> @@ -5,3 +5,4 @@ tee-objs += tee_shm.o
>> tee-objs += tee_shm_pool.o
>> obj-$(CONFIG_OPTEE) += optee/
>> obj-$(CONFIG_AMDTEE) += amdtee/
>> +obj-$(CONFIG_ARM_TSTEE) += tstee/
>> diff --git a/drivers/tee/tstee/Kconfig b/drivers/tee/tstee/Kconfig
>> new file mode 100644
>> index 000000000000..d32f91d47398
>> --- /dev/null
>> +++ b/drivers/tee/tstee/Kconfig
>> @@ -0,0 +1,11 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +config ARM_TSTEE
>> + tristate "Arm Trusted Services TEE driver"
>> + depends on ARM_FFA_TRANSPORT
>> + default n
>> + help
>> + The Trusted Services project provides a framework for developing and
>> + deploying device Root of Trust services in FF-A Secure Partitions.
>> + This driver provides an interface to make Trusted Services Secure
>> + Partitions accessible for user space clients, since the FF-A driver
>> + doesn't implement a user space interface directly.
>> diff --git a/drivers/tee/tstee/Makefile b/drivers/tee/tstee/Makefile
>> new file mode 100644
>> index 000000000000..5227020ebd30
>> --- /dev/null
>> +++ b/drivers/tee/tstee/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +arm-tstee-objs := core.o
>> +obj-$(CONFIG_ARM_TSTEE) = arm-tstee.o
>> diff --git a/drivers/tee/tstee/core.c b/drivers/tee/tstee/core.c
>> new file mode 100644
>> index 000000000000..8d6bbe4d03ed
>> --- /dev/null
>> +++ b/drivers/tee/tstee/core.c
>> @@ -0,0 +1,501 @@
>> +// 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_DIRECT_REQ_ARG_NUM 5
>> +#define FFA_INVALID_MEM_HANDLE U64_MAX
>> +
>> +static void arg_list_to_ffa_data(const u32 *args, struct ffa_send_direct_data *data)
>
> The preferred limit on the length of a single line is 80 columns
> https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings

Sure, I'll reformat.

>> +{
>> + 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,
>> + /* FF-A endpoint ID only uses the lower 16 bits */
>> + .impl_caps = lower_16_bits(tstee->ffa_dev->vm_id),
>> + .gen_caps = 0,
>> + };
>> +
>> + *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;
>
> Since you have an idr, why not use idr_find() instead?

Good point, anyway this got removed due to using XArray.

>> +
>> + 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[FFA_DIRECT_REQ_ARG_NUM] = {};
>> + 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);
>
> Is the cast needed?

It's not needed, removed.

>> +
>> + 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);
>
> This needs to be protected by the mutex.

After replacing this with xa_alloc() my understanding is that the mutex
will not be needed here.

>> + 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[FFA_DIRECT_REQ_ARG_NUM] = {};
>> + int shm_id, rc;
>> + u8 iface_id;
>> + u64 handle;
>> + u16 opcode;
>> +
>> + mutex_lock(&ctxdata->mutex);
>> + sess = find_session(ctxdata, arg->session);
>> +
>> + /* Do this while holding the mutex to make sure that the session wasn't closed meanwhile */
>> + if (sess)
>> + iface_id = sess->iface_id;
>> +
>> + mutex_unlock(&ctxdata->mutex);
>> + if (!sess)
>> + return -EINVAL;
>> +
>> + opcode = lower_16_bits(arg->func);
>> + shm_id = lower_32_bits(param[0].u.value.a);
>> + req_len = lower_32_bits(param[0].u.value.b);
>> +
>> + if (shm_id != 0) {
>> + shm = tee_shm_get_from_id(ctx, shm_id);
>> + if (IS_ERR(shm))
>> + return PTR_ERR(shm);
>> +
>> + if (shm->size < req_len) {
>> + pr_err("request doesn't fit into shared memory buffer\n");
>> + rc = -EINVAL;
>> + goto out;
>> + }
>> +
>> + handle = shm->sec_world_id;
>> + } else {
>> + handle = FFA_INVALID_MEM_HANDLE;
>> + }
>> +
>> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(iface_id, opcode);
>> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_LSW] = lower_32_bits(handle);
>> + ffa_args[TS_RPC_SERVICE_MEM_HANDLE_MSW] = upper_32_bits(handle);
>> + ffa_args[TS_RPC_SERVICE_REQ_LEN] = req_len;
>> + ffa_args[TS_RPC_SERVICE_CLIENT_ID] = 0;
>> +
>> + arg_list_to_ffa_data(ffa_args, &ffa_data);
>> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> + if (rc)
>> + goto out;
>> +
>> + arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> + if (ffa_args[TS_RPC_SERVICE_RPC_STATUS] != TS_RPC_OK) {
>> + pr_err("invoke_func rpc status: %d\n", ffa_args[TS_RPC_SERVICE_RPC_STATUS]);
>> + rc = -EINVAL;
>> + goto out;
>> + }
>> +
>> + arg->ret = ffa_args[TS_RPC_SERVICE_STATUS];
>> + if (shm && shm->size >= ffa_args[TS_RPC_SERVICE_RESP_LEN])
>> + param[0].u.value.a = ffa_args[TS_RPC_SERVICE_RESP_LEN];
>> +
>> +out:
>> + if (shm)
>> + tee_shm_put(shm);
>> +
>> + return rc;
>> +}
>> +
>> +static int tstee_shm_register(struct tee_context *ctx, struct tee_shm *shm, struct page **pages,
>> + size_t num_pages, unsigned long start __always_unused)
>> +{
>> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> + struct ffa_device *ffa_dev = tstee->ffa_dev;
>> + struct ffa_mem_region_attributes mem_attr = {
>> + .receiver = tstee->ffa_dev->vm_id,
>> + .attrs = FFA_MEM_RW,
>> + .flag = 0,
>> + };
>> + struct ffa_mem_ops_args mem_args = {
>> + .attrs = &mem_attr,
>> + .use_txbuf = true,
>> + .nattrs = 1,
>> + .flags = 0,
>> + };
>> + struct ffa_send_direct_data ffa_data;
>> + struct sg_table sgt;
>> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> + int rc;
>> +
>> + rc = sg_alloc_table_from_pages(&sgt, pages, num_pages, 0, num_pages * PAGE_SIZE,
>> + GFP_KERNEL);
>> + if (rc)
>> + return rc;
>> +
>> + mem_args.sg = sgt.sgl;
>> + rc = ffa_dev->ops->mem_ops->memory_share(&mem_args);
>> + sg_free_table(&sgt);
>> + if (rc)
>> + return rc;
>> +
>> + shm->sec_world_id = mem_args.g_handle;
>> +
>> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> + TS_RPC_OP_RETRIEVE_MEM);
>> + ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
>> + ffa_args[TS_RPC_RETRIEVE_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
>> + ffa_args[TS_RPC_RETRIEVE_MEM_TAG_LSW] = 0;
>> + ffa_args[TS_RPC_RETRIEVE_MEM_TAG_MSW] = 0;
>> +
>> + arg_list_to_ffa_data(ffa_args, &ffa_data);
>> + rc = ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data);
>> + if (rc) {
>> + (void)ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
>> + return rc;
>> + }
>> +
>> + arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> + if (ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS] != TS_RPC_OK) {
>> + pr_err("shm_register rpc status: %d\n", ffa_args[TS_RPC_RETRIEVE_MEM_RPC_STATUS]);
>> + ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int tstee_shm_unregister(struct tee_context *ctx, struct tee_shm *shm)
>> +{
>> + struct tstee *tstee = tee_get_drvdata(ctx->teedev);
>> + struct ffa_device *ffa_dev = tstee->ffa_dev;
>> + struct ffa_send_direct_data ffa_data;
>> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> + int rc;
>> +
>> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> + TS_RPC_OP_RELINQ_MEM);
>> + ffa_args[TS_RPC_RELINQ_MEM_HANDLE_LSW] = lower_32_bits(shm->sec_world_id);
>> + ffa_args[TS_RPC_RELINQ_MEM_HANDLE_MSW] = upper_32_bits(shm->sec_world_id);
>> +
>> + 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_RELINQ_MEM_RPC_STATUS] != TS_RPC_OK) {
>> + pr_err("shm_unregister rpc status: %d\n", ffa_args[TS_RPC_RELINQ_MEM_RPC_STATUS]);
>> + return -EINVAL;
>> + }
>> +
>> + rc = ffa_dev->ops->mem_ops->memory_reclaim(shm->sec_world_id, 0);
>> +
>> + return rc;
>> +}
>> +
>> +static const struct tee_driver_ops tstee_ops = {
>> + .get_version = tstee_get_version,
>> + .open = tstee_open,
>> + .release = tstee_release,
>> + .open_session = tstee_open_session,
>> + .close_session = tstee_close_session,
>> + .invoke_func = tstee_invoke_func,
>> + .shm_register = tstee_shm_register,
>> + .shm_unregister = tstee_shm_unregister,
>> +};
>> +
>> +static const struct tee_desc tstee_desc = {
>> + .name = "tstee-clnt",
>> + .ops = &tstee_ops,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int pool_op_alloc(struct tee_shm_pool *pool, struct tee_shm *shm, size_t size, size_t align)
>> +{
>> + return tee_shm_pool_op_alloc_helper(pool, shm, size, align, tstee_shm_register);
>> +}
>> +
>> +static void pool_op_free(struct tee_shm_pool *pool, struct tee_shm *shm)
>> +{
>> + tee_shm_pool_op_free_helper(pool, shm, tstee_shm_unregister);
>> +}
>> +
>> +static void pool_op_destroy_pool(struct tee_shm_pool *pool)
>> +{
>> + kfree(pool);
>> +}
>> +
>> +static const struct tee_shm_pool_ops pool_ops = {
>> + .alloc = pool_op_alloc,
>> + .free = pool_op_free,
>> + .destroy_pool = pool_op_destroy_pool,
>> +};
>> +
>> +static struct tee_shm_pool *tstee_create_shm_pool(void)
>> +{
>> + struct tee_shm_pool *pool = kzalloc(sizeof(*pool), GFP_KERNEL);
>> +
>> + if (!pool)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + pool->ops = &pool_ops;
>> +
>> + return pool;
>> +}
>> +
>> +static bool tstee_check_rpc_compatible(struct ffa_device *ffa_dev)
>> +{
>> + struct ffa_send_direct_data ffa_data;
>> + u32 ffa_args[FFA_DIRECT_REQ_ARG_NUM] = {};
>> +
>> + ffa_args[TS_RPC_CTRL_REG] = TS_RPC_CTRL_PACK_IFACE_OPCODE(TS_RPC_MGMT_IFACE_ID,
>> + TS_RPC_OP_GET_VERSION);
>> +
>> + arg_list_to_ffa_data(ffa_args, &ffa_data);
>> + if (ffa_dev->ops->msg_ops->sync_send_receive(ffa_dev, &ffa_data))
>> + return false;
>> +
>> + arg_list_from_ffa_data(&ffa_data, ffa_args);
>> +
>> + return ffa_args[TS_RPC_GET_VERSION_RESP] == TS_RPC_PROTOCOL_VERSION;
>> +}
>> +
>> +static void tstee_deinit_common(struct tstee *tstee)
>> +{
>> + tee_device_unregister(tstee->teedev);
>> + if (tstee->pool)
>> + tee_shm_pool_free(tstee->pool);
>> +
>> + kfree(tstee);
>> +}
>> +
>> +static int tstee_probe(struct ffa_device *ffa_dev)
>> +{
>> + struct tstee *tstee;
>> + int rc;
>> +
>> + ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
>> +
>> + if (!tstee_check_rpc_compatible(ffa_dev))
>> + return -EINVAL;
>> +
>> + tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
>> + if (!tstee)
>> + return -ENOMEM;
>> +
>> + tstee->ffa_dev = ffa_dev;
>> +
>> + tstee->pool = tstee_create_shm_pool();
>> + if (IS_ERR(tstee->pool)) {
>> + rc = PTR_ERR(tstee->pool);
>> + tstee->pool = NULL;
>> + goto err;
>> + }
>> +
>> + tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
>> + if (IS_ERR(tstee->teedev)) {
>> + rc = PTR_ERR(tstee->teedev);
>> + tstee->teedev = NULL;
>> + goto err;
>> + }
>> +
>> + rc = tee_device_register(tstee->teedev);
>> + if (rc)
>> + goto err;
>> +
>> + ffa_dev_set_drvdata(ffa_dev, tstee);
>> +
>> + pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);
>> +
>> + return 0;
>> +
>> +err:
>> + tstee_deinit_common(tstee);
>> + return rc;
>> +}
>> +
>> +static void tstee_remove(struct ffa_device *ffa_dev)
>> +{
>> + tstee_deinit_common(ffa_dev->dev.driver_data);
>> +}
>> +
>> +static const struct ffa_device_id tstee_device_ids[] = {
>> + /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
>> + { TS_RPC_UUID },
>> + {}
>> +};
>> +
>> +static struct ffa_driver tstee_driver = {
>> + .name = "arm_tstee",
>> + .probe = tstee_probe,
>> + .remove = tstee_remove,
>> + .id_table = tstee_device_ids,
>> +};
>> +
>> +static int __init mod_init(void)
>> +{
>> + return ffa_register(&tstee_driver);
>> +}
>> +module_init(mod_init)
>> +
>> +static void __exit mod_exit(void)
>> +{
>> + ffa_unregister(&tstee_driver);
>> +}
>> +module_exit(mod_exit)
>> +
>> +MODULE_ALIAS("arm-tstee");
>> +MODULE_AUTHOR("Balint Dobszay <[email protected]>");
>> +MODULE_DESCRIPTION("Arm Trusted Services TEE driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/tee/tstee/tstee_private.h b/drivers/tee/tstee/tstee_private.h
>> new file mode 100644
>> index 000000000000..81eeda220a5c
>> --- /dev/null
>> +++ b/drivers/tee/tstee/tstee_private.h
>> @@ -0,0 +1,92 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2023, Arm Limited
>> + */
>> +
>> +#ifndef TSTEE_PRIVATE_H
>> +#define TSTEE_PRIVATE_H
>> +
>> +#include <linux/arm_ffa.h>
>> +#include <linux/bitops.h>
>> +#include <linux/idr.h>
>> +#include <linux/tee_drv.h>
>> +#include <linux/types.h>
>> +#include <linux/uuid.h>
>> +
>> +/* UUID of this protocol */
>
> It would be nice to have a link or reference to the description of the ABI.

The link for the ABI definition is available in the documentation, but
I'll add it here too.

>> +#define TS_RPC_UUID UUID_INIT(0xbdcd76d7, 0x825e, 0x4751, \
>> + 0x96, 0x3b, 0x86, 0xd4, 0xf8, 0x49, 0x43, 0xac)
>> +
>> +/* Protocol version*/
>> +#define TS_RPC_PROTOCOL_VERSION (1)
>> +
>> +/* Status codes */
>> +#define TS_RPC_OK (0)
>> +
>> +/* RPC control register */
>> +#define TS_RPC_CTRL_REG (0)
>> +#define OPCODE_MASK GENMASK(15, 0)
>> +#define IFACE_ID_MASK GENMASK(23, 16)
>> +#define TS_RPC_CTRL_OPCODE(x) ((u16)(FIELD_GET(OPCODE_MASK, (x))))
>> +#define TS_RPC_CTRL_IFACE_ID(x) ((u8)(FIELD_GET(IFACE_ID_MASK, (x))))
>> +#define TS_RPC_CTRL_PACK_IFACE_OPCODE(i, o) \
>> + (FIELD_PREP(IFACE_ID_MASK, (i)) | FIELD_PREP(OPCODE_MASK, (o)))
>> +#define TS_RPC_CTRL_SAP_RC BIT(30)
>> +#define TS_RPC_CTRL_SAP_ERR BIT(31)
>> +
>> +/* Interface ID for RPC management operations */
>> +#define TS_RPC_MGMT_IFACE_ID (0xff)
>> +
>> +/* Management calls */
>> +#define TS_RPC_OP_GET_VERSION (0x0000)
>> +#define TS_RPC_GET_VERSION_RESP (1)
>> +
>> +#define TS_RPC_OP_RETRIEVE_MEM (0x0001)
>> +#define TS_RPC_RETRIEVE_MEM_HANDLE_LSW (1)
>> +#define TS_RPC_RETRIEVE_MEM_HANDLE_MSW (2)
>> +#define TS_RPC_RETRIEVE_MEM_TAG_LSW (3)
>> +#define TS_RPC_RETRIEVE_MEM_TAG_MSW (4)
>> +#define TS_RPC_RETRIEVE_MEM_RPC_STATUS (1)
>> +
>> +#define TS_RPC_OP_RELINQ_MEM (0x0002)
>> +#define TS_RPC_RELINQ_MEM_HANDLE_LSW (1)
>> +#define TS_RPC_RELINQ_MEM_HANDLE_MSW (2)
>> +#define TS_RPC_RELINQ_MEM_RPC_STATUS (1)
>> +
>> +#define TS_RPC_OP_SERVICE_INFO (0x0003)
>> +#define TS_RPC_SERVICE_INFO_UUID0 (1)
>> +#define TS_RPC_SERVICE_INFO_UUID1 (2)
>> +#define TS_RPC_SERVICE_INFO_UUID2 (3)
>> +#define TS_RPC_SERVICE_INFO_UUID3 (4)
>> +#define TS_RPC_SERVICE_INFO_RPC_STATUS (1)
>> +#define TS_RPC_SERVICE_INFO_IFACE (2)
>> +
>> +/* Service call */
>> +#define TS_RPC_SERVICE_MEM_HANDLE_LSW (1)
>> +#define TS_RPC_SERVICE_MEM_HANDLE_MSW (2)
>> +#define TS_RPC_SERVICE_REQ_LEN (3)
>> +#define TS_RPC_SERVICE_CLIENT_ID (4)
>> +#define TS_RPC_SERVICE_RPC_STATUS (1)
>> +#define TS_RPC_SERVICE_STATUS (2)
>> +#define TS_RPC_SERVICE_RESP_LEN (3)
>> +
>> +struct tstee {
>> + struct ffa_device *ffa_dev;
>> + struct tee_device *teedev;
>> + struct tee_shm_pool *pool;
>> +};
>> +
>> +struct ts_session {
>> + struct list_head list_node;
>> + u32 session_id;
>> + u8 iface_id;
>> +};
>> +
>> +struct ts_context_data {
>> + struct list_head sess_list;
>> + struct idr sess_ids;
>
> Why do you need both a linked list and an IDR? Wouldn't the IDR be enough?

You're right, I added the IDR later and didn't realise this made the
linked list redundant. I removed both the linked list and IDR and just
use XArray instead.

Regards,
Balint

>> + /* Serializes access to this struct */
>> + struct mutex mutex;
>> +};
>> +
>> +#endif /* TSTEE_PRIVATE_H */
>> diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
>> index 23e57164693c..d0430bee8292 100644
>> --- a/include/uapi/linux/tee.h
>> +++ b/include/uapi/linux/tee.h
>> @@ -56,6 +56,7 @@
>> */
>> #define TEE_IMPL_ID_OPTEE 1
>> #define TEE_IMPL_ID_AMDTEE 2
>> +#define TEE_IMPL_ID_TSTEE 3
>>
>> /*
>> * OP-TEE specific capabilities
>> --
>> 2.34.1
>>

2024-02-22 16:43:11

by Balint Dobszay

[permalink] [raw]
Subject: Re: [PATCH 2/3] tee: tstee: Add Trusted Services TEE driver

Hi Krzysztof,

Thanks for the feedback.

On 15 Feb 2024, at 9:59, Krzysztof Kozlowski wrote:

> On 13/02/2024 15:52, Balint Dobszay wrote:
>> The Trusted Services project provides a framework for developing and
>> deploying device Root of Trust services in FF-A Secure Partitions. The
>> FF-A SPs are accessible through the FF-A driver, but this doesn't
>> provide a user space interface. The goal of this TEE driver is to make
>> Trusted Services SPs accessible for user space clients.
>>
>> All TS SPs have the same FF-A UUID, it identifies the RPC protocol used
>> by TS. A TS SP can host one or more services, a service is identified by
>> its service UUID. The same type of service cannot be present twice in
>> the same SP. During SP boot each service in an SP is assigned an
>> interface ID, this is just a short ID to simplify message addressing.
>> There is 1:1 mapping between TS SPs and TEE devices, i.e. a separate TEE
>> device is registered for each TS SP. This is required since contrary to
>> the generic TEE design where memory is shared with the whole TEE
>> implementation, in case of FF-A, memory is shared with a specific SP. A
>> user space client has to be able to separately share memory with each SP
>> based on its endpoint ID.
>>
>> Signed-off-by: Balint Dobszay <[email protected]>
>> ---
>
>
>> +static int tstee_probe(struct ffa_device *ffa_dev)
>> +{
>> + struct tstee *tstee;
>> + int rc;
>> +
>> + ffa_dev->ops->msg_ops->mode_32bit_set(ffa_dev);
>> +
>> + if (!tstee_check_rpc_compatible(ffa_dev))
>> + return -EINVAL;
>> +
>> + tstee = kzalloc(sizeof(*tstee), GFP_KERNEL);
>> + if (!tstee)
>> + return -ENOMEM;
>> +
>> + tstee->ffa_dev = ffa_dev;
>> +
>> + tstee->pool = tstee_create_shm_pool();
>> + if (IS_ERR(tstee->pool)) {
>> + rc = PTR_ERR(tstee->pool);
>> + tstee->pool = NULL;
>> + goto err;
>
> Is it logically correct to call here tee_device_unregister()?

As Jens mentioned it doesn't cause any issues. But I see you point, it's
not logically correct. I'll refactor this.

>> + }
>> +
>> + tstee->teedev = tee_device_alloc(&tstee_desc, NULL, tstee->pool, tstee);
>> + if (IS_ERR(tstee->teedev)) {
>> + rc = PTR_ERR(tstee->teedev);
>> + tstee->teedev = NULL;
>> + goto err;
>> + }
>> +
>> + rc = tee_device_register(tstee->teedev);
>> + if (rc)
>> + goto err;
>> +
>> + ffa_dev_set_drvdata(ffa_dev, tstee);
>> +
>> + pr_info("driver initialized for endpoint 0x%x\n", ffa_dev->vm_id);
>
> Don't print simple probe success messages. Anyway all prints in device
> context should be dev_*.

Okay, I'll remove this.

>> +
>> + return 0;
>> +
>> +err:
>> + tstee_deinit_common(tstee);
>> + return rc;
>> +}
>> +
>> +static void tstee_remove(struct ffa_device *ffa_dev)
>> +{
>> + tstee_deinit_common(ffa_dev->dev.driver_data);
>> +}
>> +
>> +static const struct ffa_device_id tstee_device_ids[] = {
>> + /* TS RPC protocol UUID: bdcd76d7-825e-4751-963b-86d4f84943ac */
>> + { TS_RPC_UUID },
>> + {}
>> +};
>> +
>> +static struct ffa_driver tstee_driver = {
>> + .name = "arm_tstee",
>> + .probe = tstee_probe,
>> + .remove = tstee_remove,
>> + .id_table = tstee_device_ids,
>> +};
>> +
>> +static int __init mod_init(void)
>> +{
>> + return ffa_register(&tstee_driver);
>> +}
>> +module_init(mod_init)
>> +
>> +static void __exit mod_exit(void)
>> +{
>> + ffa_unregister(&tstee_driver);
>> +}
>> +module_exit(mod_exit)
>> +
>> +MODULE_ALIAS("arm-tstee");
>
> Why do you need this alias? I don't see MODULE_DEVICE_TABLE, so how this
> bus handles module loading?

You're right, the alias is not needed, I'll remove it.
Regarding MODULE_DEVICE_TABLE, AFAIK this mechanism is currently not
supported for the arm_ffa bus type. Maybe Sudeep can chime in on this?

Regards,
Balint