Re: [PATCH v3 1/7] remoteproc: Add TEE support

Hello Mathieu,

On 2/23/24 19:27, Mathieu Poirier wrote:

On Wed, Feb 14, 2024 at 06:21:21PM +0100, Arnaud Pouliquen wrote:

...

From: Arnaud Pouliquen arnaud.pouliquen@st.com

Add a remoteproc TEE (Trusted Execution Environment) driver that will be probed by the TEE bus. If the associated Trusted application is supported on secure part this device offers a client interface to load a firmware in the secure part. This firmware could be authenticated and decrypted by the secure trusted application.

Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com

update from V2

• Use 'tee_rproc' prefix for all functions
• rename tee_rproc_get_loaded_rsc_table to tee_rproc_find_loaded_rsc_table
• redefine fonction to better match with the rproc_ops structure format
  • replace 'struct tee_rproc' parameter by 'struct rproc' parameter
  • rename 'rproc_tee_get_rsc_table()' to tee_rproc_get_loaded_rsc_table() and rework it to remove the cached_table management.
  • introduce tee_rproc_get_context() to get the tee_rproc struct from the rproc struct
  • rename tee_rproc_get_loaded_rsc_table() to tee_rproc_find_loaded_rsc_table()
• remove useless check on tee_rproc_ctx structure in tee_rproc_register() and tee_rproc_unregister()
• fix test on the return of tee_rproc_ctx = devm_kzalloc()
• remove useless includes and unused tee_rproc_mem structure.

---

drivers/remoteproc/Kconfig | 9 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++++++++++++ include/linux/tee_remoteproc.h | 102 +++++++ 4 files changed, 509 insertions(+) create mode 100644 drivers/remoteproc/tee_remoteproc.c create mode 100644 include/linux/tee_remoteproc.h

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 48845dc8fa85..85299606806c 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -365,6 +365,15 @@ config XLNX_R5_REMOTEPROC It's safe to say N if not interested in using RPU r5f cores.

+config TEE_REMOTEPROC

• tristate "trusted firmware support by a TEE application"
• depends on OPTEE
• help

•  Support for trusted remote processors firmware. The firmware
  

•  authentication and/or decryption are managed by a trusted application.
  

•  This can be either built-in or a loadable module.
  

endif # REMOTEPROC endmenu diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 91314a9b43ce..fa8daebce277 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o +obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o obj-$(CONFIG_TI_K3_R5_REMOTEPROC) += ti_k3_r5_remoteproc.o obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o diff --git a/drivers/remoteproc/tee_remoteproc.c b/drivers/remoteproc/tee_remoteproc.c new file mode 100644 index 000000000000..ac727e062d00 --- /dev/null +++ b/drivers/remoteproc/tee_remoteproc.c @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• • Copyright (C) STMicroelectronics 2023 - All Rights Reserved
• • Author: Arnaud Pouliquen arnaud.pouliquen@st.com
• */

+#include <linux/firmware.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/remoteproc.h> +#include <linux/slab.h> +#include <linux/tee_drv.h> +#include <linux/tee_remoteproc.h>

+#include "remoteproc_internal.h"

+#define MAX_TEE_PARAM_ARRY_MEMBER 4

+/*

• • Authentication of the firmware and load in the remote processor memory
• • [in] params[0].value.a: unique 32bit identifier of the remote processor
• • [in] params[1].memref: buffer containing the image of the buffer
• */

+#define TA_RPROC_FW_CMD_LOAD_FW 1

+/*

• • Start the remote processor
• • [in] params[0].value.a: unique 32bit identifier of the remote processor
• */

+#define TA_RPROC_FW_CMD_START_FW 2

+/*

• • Stop the remote processor
• • [in] params[0].value.a: unique 32bit identifier of the remote processor
• */

+#define TA_RPROC_FW_CMD_STOP_FW 3

+/*

• • Return the address of the resource table, or 0 if not found
• • No check is done to verify that the address returned is accessible by
• • the non secure context. If the resource table is loaded in a protected
• • memory the access by the non secure context will lead to a data abort.
• • [in] params[0].value.a: unique 32bit identifier of the remote processor
• • [out] params[1].value.a: 32bit LSB resource table memory address
• • [out] params[1].value.b: 32bit MSB resource table memory address
• • [out] params[2].value.a: 32bit LSB resource table memory size
• • [out] params[2].value.b: 32bit MSB resource table memory size
• */

+#define TA_RPROC_FW_CMD_GET_RSC_TABLE 4

+/*

• • Return the address of the core dump
• • [in] params[0].value.a: unique 32bit identifier of the remote processor
• • [out] params[1].memref: address of the core dump image if exist,

• • 		else return Null
    

• */

+#define TA_RPROC_FW_CMD_GET_COREDUMP 5

+struct tee_rproc__context {

• struct list_head sessions;
• struct tee_context *tee_ctx;
• struct device *dev;

+};

+static struct tee_rproc__context *tee_rproc_ctx;

+static void prepare_args(struct tee_rproc *trproc, int cmd,

• 	 struct tee_ioctl_invoke_arg *arg,
  

• 	 struct tee_param *param, unsigned int num_params)
  

+{

• memset(arg, 0, sizeof(*arg));
• memset(param, 0, MAX_TEE_PARAM_ARRY_MEMBER * sizeof(*param));
• arg->func = cmd;
• arg->session = trproc->session_id;
• arg->num_params = num_params + 1;
• param[0] = (struct tee_param) {

• .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
  

• .u.value.a = trproc->rproc_id,
  

• };

+}

+static struct tee_rproc *tee_rproc_get_context(struct rproc *rproc) +{

• struct tee_rproc *entry, *tmp;
• list_for_each_entry_safe(entry, tmp, &tee_rproc_ctx->sessions, node) {

• if (entry->rproc == rproc)
  

• 	return entry;
  

• }
• return NULL;

+}

+int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw) +{

• struct tee_ioctl_invoke_arg arg;
• struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
• struct tee_rproc *trproc = tee_rproc_get_context(rproc);
• struct tee_shm *fw_shm;
• int ret;
• if (!trproc)

• return -EINVAL;
  

• fw_shm = tee_shm_register_kernel_buf(tee_rproc_ctx->tee_ctx, (void *)fw->data, fw->size);
• if (IS_ERR(fw_shm))

• return PTR_ERR(fw_shm);
  

• prepare_args(trproc, TA_RPROC_FW_CMD_LOAD_FW, &arg, param, 1);
• /* Provide the address of the firmware image */
• param[1] = (struct tee_param) {

• .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT,
  

• .u.memref = {
  

• 	.shm = fw_shm,
  

• 	.size = fw->size,
  

• 	.shm_offs = 0,
  

• },
  

• };
• ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
• if (ret < 0 || arg.ret != 0) {

• dev_err(tee_rproc_ctx->dev,
  

• 	"TA_RPROC_FW_CMD_LOAD_FW invoke failed TEE err: %x, ret:%x\n",
  

• 	arg.ret, ret);
  

• if (!ret)
  

• 	ret = -EIO;
  

• }
• tee_shm_free(fw_shm);
• return ret;

+} +EXPORT_SYMBOL_GPL(tee_rproc_load_fw);

+struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz) +{

• struct tee_ioctl_invoke_arg arg;
• struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
• struct tee_rproc *trproc = tee_rproc_get_context(rproc);
• int ret;
• if (!trproc)

• return ERR_PTR(-EINVAL);
  

• prepare_args(trproc, TA_RPROC_FW_CMD_GET_RSC_TABLE, &arg, param, 2);
• param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
• param[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT;
• ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
• if (ret < 0 || arg.ret != 0) {

• dev_err(tee_rproc_ctx->dev,
  

• 	"TA_RPROC_FW_CMD_GET_RSC_TABLE invoke failed TEE err: %x, ret:%x\n",
  

• 	arg.ret, ret);
  

• return ERR_PTR(-EIO);
  

• }
• *table_sz = param[2].u.value.a;
• /* If the size is null no resource table defined in the image */
• if (!*table_sz)

• return NULL;
  

• /* Store the resource table address that would be updated by the remote core. */
• trproc->rsc_table = ioremap_wc(param[1].u.value.a, *table_sz);
• if (IS_ERR_OR_NULL(trproc->rsc_table)) {

• dev_err(tee_rproc_ctx->dev, "Unable to map memory region: %lld+%zx\n",
  

• 	param[1].u.value.a, *table_sz);
  

• return ERR_PTR(-ENOMEM);
  

• }
• return trproc->rsc_table;

+} +EXPORT_SYMBOL_GPL(tee_rproc_get_loaded_rsc_table);

Here we are missing:

tee_rproc_parse_fw()

Please tell me if I'm wrong but the aim of this ops is to parse the firmware before loading it in case of some resources shoukd be needed before. but here to parse it we load. I'm not sure that this function makes sense here.

{ tee_rproc_load_fw() resource_table = tee_rproc_get_loaded_rsc_table()

    //check error conditions here

    rproc->cached_table = resource_table;
    rproc->table_ptr = resource_table;

This seems to me that it is not possible regarding the "memcpy(loaded_table, rproc->cached_table, rproc->table_sz)"[1] in rproc_start() and the kfree(rproc->cached_table) [2] in rproc_shutdown(). We would copy with the same source and destibnation address. In this case a memory should be allocated for the rproc->cached_table.

}

This is essentially the same as rproc_elf_load_rsc_table(). That way we don't need rproc_alt_fw_boot() and rproc_load_segments() doesn't have to be moved around.

The trusted application simply needs to know that if the firmware is

already loaded, it has to return.

Today trying to load twice time is considered as an error in OP-TEE [3]. As it is a constraint introduced by Linux, I would prefer treated it in the Linux.

What about introduce a "fw_loaded" flag in tee_rproc__context?

[1] https://elixir.bootlin.com/linux/v6.8-rc6/source/drivers/remoteproc/remotepr... [2] https://elixir.bootlin.com/linux/v6.8-rc6/source/drivers/remoteproc/remotepr... [3] https://elixir.bootlin.com/op-tee/latest/source/ta/remoteproc/src/remoteproc...

Regards, Arnaud

...

+struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,

• 				       const struct firmware *fw)
  

+{

• struct tee_rproc *trproc = tee_rproc_get_context(rproc);
• size_t table_sz;
• if (!trproc)

• return ERR_PTR(-EINVAL);
  

• if (!trproc->rsc_table)

• trproc->rsc_table = tee_rproc_get_loaded_rsc_table(rproc, &table_sz);
  

• return trproc->rsc_table;

+} +EXPORT_SYMBOL_GPL(tee_rproc_find_loaded_rsc_table);

+int tee_rproc_start(struct rproc *rproc) +{

• struct tee_ioctl_invoke_arg arg;
• struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
• struct tee_rproc *trproc = tee_rproc_get_context(rproc);
• int ret;
• if (!trproc)

• return -EINVAL;
  

• prepare_args(trproc, TA_RPROC_FW_CMD_START_FW, &arg, param, 0);
• ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
• if (ret < 0 || arg.ret != 0) {

• dev_err(tee_rproc_ctx->dev,
  

• 	"TA_RPROC_FW_CMD_START_FW invoke failed TEE err: %x, ret:%x\n",
  

• 	arg.ret, ret);
  

• if (!ret)
  

• 	ret = -EIO;
  

• }
• return ret;

+} +EXPORT_SYMBOL_GPL(tee_rproc_start);

+int tee_rproc_stop(struct rproc *rproc) +{

• struct tee_ioctl_invoke_arg arg;
• struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
• struct tee_rproc *trproc = tee_rproc_get_context(rproc);
• int ret;
• if (!trproc)

• return -EINVAL;
  

• prepare_args(trproc, TA_RPROC_FW_CMD_STOP_FW, &arg, param, 0);
• ret = tee_client_invoke_func(tee_rproc_ctx->tee_ctx, &arg, param);
• if (ret < 0 || arg.ret != 0) {

• dev_err(tee_rproc_ctx->dev,
  

• 	"TA_RPROC_FW_CMD_STOP_FW invoke failed TEE err: %x, ret:%x\n",
  

• 	arg.ret, ret);
  

• if (!ret)
  

• 	ret = -EIO;
  

• }
• if (trproc->rsc_table)

• iounmap(trproc->rsc_table);
  

• trproc->rsc_table = NULL;
• return ret;

+} +EXPORT_SYMBOL_GPL(tee_rproc_stop);

+static const struct tee_client_device_id stm32_tee_rproc_id_table[] = {

• {UUID_INIT(0x80a4c275, 0x0a47, 0x4905,

•    0x82, 0x85, 0x14, 0x86, 0xa9, 0x77, 0x1a, 0x08)},
  

• {}

+};

+struct tee_rproc *tee_rproc_register(struct device *dev, unsigned int rproc_id) +{

• struct tee_client_device *tee_device;
• struct tee_ioctl_open_session_arg sess_arg;
• struct tee_param param[MAX_TEE_PARAM_ARRY_MEMBER];
• struct tee_rproc *trproc;
• int ret;
• /*

• * The device is not probed by the TEE bus. We ignore the reason (bus could be not yet
  

• * probed or service not available in the secure firmware)
  

• * Assumption here is that the TEE bus is not probed.
  

• */
  

• if (!tee_rproc_ctx)

• return ERR_PTR(-EPROBE_DEFER);
  

• trproc = devm_kzalloc(dev, sizeof(*trproc), GFP_KERNEL);
• if (!trproc)

• return ERR_PTR(-ENOMEM);
  

• tee_device = to_tee_client_device(tee_rproc_ctx->dev);
• memset(&sess_arg, 0, sizeof(sess_arg));
• /* Open session with rproc_tee load the OP-TEE Trusted Application */
• memcpy(sess_arg.uuid, tee_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
• sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
• sess_arg.num_params = 1;
• param[0] = (struct tee_param) {

• .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
  

• .u.value.a = rproc_id,
  

• };
• ret = tee_client_open_session(tee_rproc_ctx->tee_ctx, &sess_arg, param);
• if (ret < 0 || sess_arg.ret != 0) {

• dev_err(dev, "tee_client_open_session failed, err: %x\n", sess_arg.ret);
  

• return ERR_PTR(-EINVAL);
  

• }
• trproc->parent = dev;
• trproc->rproc_id = rproc_id;
• trproc->session_id = sess_arg.session;
• list_add_tail(&trproc->node, &tee_rproc_ctx->sessions);
• return trproc;

+} +EXPORT_SYMBOL_GPL(tee_rproc_register);

+int tee_rproc_unregister(struct tee_rproc *trproc) +{

• int ret;
• ret = tee_client_close_session(tee_rproc_ctx->tee_ctx, trproc->session_id);
• if (ret < 0)

• dev_err(trproc->parent,	"tee_client_close_session failed, err: %x\n", ret);
  

• list_del(&trproc->node);
• return ret;

+} +EXPORT_SYMBOL_GPL(tee_rproc_unregister);

+static int tee_rproc_ctx_match(struct tee_ioctl_version_data *ver, const void *data) +{

• /* Today we support only the OP-TEE, could be extend to other tees */
• return (ver->impl_id == TEE_IMPL_ID_OPTEE);

+}

+static int tee_rproc_probe(struct device *dev) +{

• struct tee_context *tee_ctx;
• int ret;
• /* Open context with TEE driver */
• tee_ctx = tee_client_open_context(NULL, tee_rproc_ctx_match, NULL, NULL);
• if (IS_ERR(tee_ctx))

• return PTR_ERR(tee_ctx);
  

• tee_rproc_ctx = devm_kzalloc(dev, sizeof(*tee_ctx), GFP_KERNEL);
• if (!tee_rproc_ctx) {

• ret = -ENOMEM;
  

• goto err;
  

• }
• tee_rproc_ctx->dev = dev;
• tee_rproc_ctx->tee_ctx = tee_ctx;
• INIT_LIST_HEAD(&tee_rproc_ctx->sessions);
• return 0;

+err:

• tee_client_close_context(tee_ctx);
• return ret;

+}

+static int tee_rproc_remove(struct device *dev) +{

• struct tee_rproc *entry, *tmp;
• list_for_each_entry_safe(entry, tmp, &tee_rproc_ctx->sessions, node) {

• tee_client_close_session(tee_rproc_ctx->tee_ctx, entry->session_id);
  

• list_del(&entry->node);
  

• kfree(entry);
  

• }
• tee_client_close_context(tee_rproc_ctx->tee_ctx);
• return 0;

+}

+MODULE_DEVICE_TABLE(tee, stm32_tee_rproc_id_table);

+static struct tee_client_driver tee_rproc_fw_driver = {

• .id_table = stm32_tee_rproc_id_table,
• .driver = {

• .name		= KBUILD_MODNAME,
  

• .bus		= &tee_bus_type,
  

• .probe		= tee_rproc_probe,
  

• .remove		= tee_rproc_remove,
  

• },

+};

+static int __init tee_rproc_fw_mod_init(void) +{

• return driver_register(&tee_rproc_fw_driver.driver);

+}

+static void __exit tee_rproc_fw_mod_exit(void) +{

• driver_unregister(&tee_rproc_fw_driver.driver);

+}

+module_init(tee_rproc_fw_mod_init); +module_exit(tee_rproc_fw_mod_exit);

+MODULE_DESCRIPTION(" TEE remote processor control driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/tee_remoteproc.h b/include/linux/tee_remoteproc.h new file mode 100644 index 000000000000..7c9e91e989ba --- /dev/null +++ b/include/linux/tee_remoteproc.h @@ -0,0 +1,102 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*

• • Copyright(c) 2023 STMicroelectronics - All Rights Reserved
• */

+#ifndef TEE_REMOTEPROC_H +#define TEE_REMOTEPROC_H

+#include <linux/tee_drv.h> +#include <linux/firmware.h> +#include <linux/remoteproc.h>

+struct rproc;

+/**

• • struct tee_rproc - TEE remoteproc structure
• • @node: Reference in list
• • @rproc: Remoteproc reference
• • @parent: Parent device
• • @rproc_id: Identifier of the target firmware
• • @session_id: TEE session identifier
• • @rsc_table: Resource table virtual address.
• */

+struct tee_rproc {

• struct list_head node;
• struct rproc *rproc;
• struct device *parent;
• u32 rproc_id;
• u32 session_id;
• struct resource_table *rsc_table;

+};

+#if IS_ENABLED(CONFIG_TEE_REMOTEPROC)

+struct tee_rproc *tee_rproc_register(struct device *dev, unsigned int rproc_id); +int tee_rproc_unregister(struct tee_rproc *trproc);

+int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw); +struct resource_table *tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz); +struct resource_table *tee_rproc_find_loaded_rsc_table(struct rproc *rproc,

• 				       const struct firmware *fw);
  

+int tee_rproc_start(struct rproc *rproc); +int tee_rproc_stop(struct rproc *rproc);

+#else

+static inline struct tee_rproc *tee_rproc_register(struct device *dev, unsigned int rproc_id) +{

• return ERR_PTR(-ENODEV);

+}

+static inline int tee_rproc_unregister(struct tee_rproc *trproc) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return 0;

+}

+static inline int tee_rproc_load_fw(struct rproc *rproc, const struct firmware *fw) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return 0;

+}

+static inline int tee_rproc_start(struct rproc *rproc) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return 0;

+}

+static inline int tee_rproc_stop(struct rproc *rproc) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return 0;

+}

+static inline struct resource_table * +tee_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return NULL;

+}

+static inline struct resource_table * +tee_rproc_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw) +{

• /* This shouldn't be possible */
• WARN_ON(1);
• return NULL;

+} +#endif /* CONFIG_TEE_REMOTEPROC */

+#endif /* TEE_REMOTEPROC_H */

2.25.1