This RFC proposes an implementation of a remoteproc tee driver to communicate with a TEE trusted application in charge of authenticating and loading remoteproc firmware image in an Arm secure context.
The services implemented are the same as those offered by the Linux remoteproc framework: - load of a signed firmware - start/stop of a coprocessor - get the resource table
The OP-TEE code in charge of providing the service in a trusted application is proposed for upstream here: https://github.com/OP-TEE/optee_os/pull/6027
For more details on the implementation a presentation is available here: https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (4): tee: Re-enable vmalloc page support for shared memory remoteproc: Add TEE support dt-bindings: remoteproc: add compatibility for TEE support remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +- drivers/remoteproc/Kconfig | 9 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/stm32_rproc.c | 234 +++++++++-- drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++ drivers/tee/tee_shm.c | 24 +- include/linux/tee_remoteproc.h | 101 +++++ 7 files changed, 753 insertions(+), 46 deletions(-) create mode 100644 drivers/remoteproc/tee_remoteproc.c create mode 100644 include/linux/tee_remoteproc.h
This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support")
The firmware framework uses vmalloc page to store an image of a firmware, got from the file system. To be able to give this firmware to OP-TEE without an extra copy, the vmalloc page support needs to be reintroduce.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com --- drivers/tee/tee_shm.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index 673cf0359494..b2d349ac17b4 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -28,14 +28,26 @@ static int shm_get_kernel_pages(unsigned long start, size_t page_count, struct page *page; size_t n;
- if (WARN_ON_ONCE(is_vmalloc_addr((void *)start) || - is_kmap_addr((void *)start))) + if (WARN_ON_ONCE(is_kmap_addr((void *)start))) return -EINVAL;
- page = virt_to_page((void *)start); - for (n = 0; n < page_count; n++) { - pages[n] = page + n; - get_page(pages[n]); + if (is_vmalloc_addr((void *)start)) { + struct page *page; + + for (n = 0; n < page_count; n++) { + page = vmalloc_to_page((void *)(start + PAGE_SIZE * n)); + if (!page) + return -ENOMEM; + + get_page(page); + pages[n] = page; + } + } else { + page = virt_to_page((void *)start); + for (n = 0; n < page_count; n++) { + pages[n] = page + n; + get_page(pages[n]); + } }
return page_count;
On Tue, May 23, 2023 at 11:13:47AM +0200, Arnaud Pouliquen wrote:
This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support")
As per the discussion back then: don't just blindly do the same dumb thing again and fix the interfae to actually pass in a page array, or iov_iter or an actually useful container that fits.
Hello Christoph,
On 5/24/23 08:46, Christoph Hellwig wrote:
On Tue, May 23, 2023 at 11:13:47AM +0200, Arnaud Pouliquen wrote:
This patch revert commit c83900393aa1 ("tee: Remove vmalloc page support")
As per the discussion back then: don't just blindly do the same dumb thing again and fix the interfae to actually pass in a page array, or iov_iter or an actually useful container that fits.
I suppose your are speaking about this discussion: https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/
If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and register_shm_helper inernal function, right?
Seems that Jens has also pointed out the free part...
What about having equivalent of shm_get_kernel_pages in an external helper (to defined where to put it), could it be an alternative of the upadate of the tee_shm API?
Thanks, Arnaud
On Wed, May 24, 2023 at 04:01:14PM +0200, Arnaud POULIQUEN wrote:
As per the discussion back then: don't just blindly do the same dumb thing again and fix the interfae to actually pass in a page array, or iov_iter or an actually useful container that fits.
I suppose your are speaking about this discussion: https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/
Yes.
If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and register_shm_helper inernal function, right?
What about having equivalent of shm_get_kernel_pages in an external helper (to defined where to put it), could it be an alternative of the upadate of the tee_shm API?
I think the fundamentally right thing is to pass an iov_iter to register_shm_helper, and then use the new as of 6.3 iov_iter_extract_pages helper to extract the pages from that. For the kernel users you can then simply pass down an ITER_BVEC iter that you can fill with vmalloc pages if you want.
On 5/26/23 14:37, Christoph Hellwig wrote:
On Wed, May 24, 2023 at 04:01:14PM +0200, Arnaud POULIQUEN wrote:
As per the discussion back then: don't just blindly do the same dumb thing again and fix the interfae to actually pass in a page array, or iov_iter or an actually useful container that fits.
I suppose your are speaking about this discussion: https://lore.kernel.org/all/20221002002326.946620-3-ira.weiny@intel.com/
Yes.
If I'm not mistaken, I should modify at tee_shm_register_kernel_buf API and register_shm_helper inernal function, right?
What about having equivalent of shm_get_kernel_pages in an external helper (to defined where to put it), could it be an alternative of the upadate of the tee_shm API?
I think the fundamentally right thing is to pass an iov_iter to register_shm_helper, and then use the new as of 6.3 iov_iter_extract_pages helper to extract the pages from that. For the kernel users you can then simply pass down an ITER_BVEC iter that you can fill with vmalloc pages if you want.
Thanks for the advice!
Regards, Arnaud
Rework compatibility description according to the support of the authenticated firmware relying on OP-TEE authentication.
The expected behavior is: - with legacy compatible "st,stm32mp1-m4" the Linux kernel loads a non-signed (ELF) firmware image, - with compatible "st,stm32mp1-m4-tee" the Linux kernel load a signed firmware image. In this case it calls TEE services to manage the firmware loading and the remoteproc life-cycle.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com --- .../bindings/remoteproc/st,stm32-rproc.yaml | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml index 959a56f1b6c7..1671a90d5974 100644 --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml @@ -16,7 +16,12 @@ maintainers:
properties: compatible: - const: st,stm32mp1-m4 + enum: + - st,stm32mp1-m4 + - st,stm32mp1-m4-tee + description: + Use "st,stm32mp1-m4" for the Cortex-M4 coprocessor management by Linux + Use "st,stm32mp1-m4-tee" for the Cortex-M4 coprocessor management by OPTEE
reg: description: @@ -135,8 +140,28 @@ required: - compatible - reg - resets - - st,syscfg-holdboot - - st,syscfg-tz + +allOf: + - if: + properties: + compatible: + enum: + - st,stm32mp1-m4 + then: + required: + - memory-region + - st,syscfg-holdboot + - st,syscfg-tz + - resets + - if: + properties: + compatible: + enum: + - st,stm32mp1-m4-tee + then: + required: + - memory-region +
additionalProperties: false
@@ -148,6 +173,8 @@ examples: reg = <0x10000000 0x40000>, <0x30000000 0x40000>, <0x38000000 0x10000>; + memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>, + <&m_ipc_shm>, <&vdev0vring1>, <&vdev0buffer>; resets = <&rcc MCU_R>; st,syscfg-holdboot = <&rcc 0x10C 0x1>; st,syscfg-tz = <&rcc 0x000 0x1>;
On 23/05/2023 11:13, Arnaud Pouliquen wrote:
Rework compatibility description according to the support of the authenticated firmware relying on OP-TEE authentication.
The expected behavior is:
- with legacy compatible "st,stm32mp1-m4" the Linux kernel loads a non-signed (ELF) firmware image,
- with compatible "st,stm32mp1-m4-tee" the Linux kernel load a signed firmware image. In this case it calls TEE services to manage the firmware loading and the remoteproc life-cycle.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-)
Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel.
You missed at least DT list (maybe more), so this won't be tested. Please resend and include all necessary entries.
Because of above and RFC, I assume there is no need for review. Just to be clear - that's a no.
Best regards, Krzysztof
Hello Krzysztof,
On 5/30/23 13:50, Krzysztof Kozlowski wrote:
On 23/05/2023 11:13, Arnaud Pouliquen wrote:
Rework compatibility description according to the support of the authenticated firmware relying on OP-TEE authentication.
The expected behavior is:
- with legacy compatible "st,stm32mp1-m4" the Linux kernel loads a non-signed (ELF) firmware image,
- with compatible "st,stm32mp1-m4-tee" the Linux kernel load a signed firmware image. In this case it calls TEE services to manage the firmware loading and the remoteproc life-cycle.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-)
Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel.
You missed at least DT list (maybe more), so this won't be tested. Please resend and include all necessary entries.
Because of above and RFC, I assume there is no need for review. Just to be clear - that's a no.
I did not add DT list and maintainers intentionally to avoid that you review it. As in a first step the associated OP-TEE pull request has to be reviewed. And my plan was just to share the Linux implementation part until the OP-TEE review cycle is finished.
Now regarding your mail (and very interesting feedback from Christoph Hellwig), it was clearly not the good strategy. So my apologize and next time whatever the objective of the series I will add all peoples and lists in the loop.
Thanks, Arnaud
Best regards, Krzysztof
On 30/05/2023 17:00, Arnaud POULIQUEN wrote:
Hello Krzysztof,
On 5/30/23 13:50, Krzysztof Kozlowski wrote:
On 23/05/2023 11:13, Arnaud Pouliquen wrote:
Rework compatibility description according to the support of the authenticated firmware relying on OP-TEE authentication.
The expected behavior is:
- with legacy compatible "st,stm32mp1-m4" the Linux kernel loads a non-signed (ELF) firmware image,
- with compatible "st,stm32mp1-m4-tee" the Linux kernel load a signed firmware image. In this case it calls TEE services to manage the firmware loading and the remoteproc life-cycle.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-)
Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel.
You missed at least DT list (maybe more), so this won't be tested. Please resend and include all necessary entries.
Because of above and RFC, I assume there is no need for review. Just to be clear - that's a no.
I did not add DT list and maintainers intentionally to avoid that you review it. As in a first step the associated OP-TEE pull request has to be reviewed. And my plan was just to share the Linux implementation part until the OP-TEE review cycle is finished.
Sure, that's fine. I just don't know whether this is intentional or not. Many people skip list without such reason...
Now regarding your mail (and very interesting feedback from Christoph Hellwig), it was clearly not the good strategy. So my apologize and next time whatever the objective of the series I will add all peoples and lists in the loop.
No worries! Thanks.
Best regards, Krzysztof
Add a remoteproc TEE (Trusted Execution Environment) device driver that will be probed by the TEE bus. If the associated TEE Trusted application is supported, this device offers a client interface to load a signed firmware in the secure context.
An implementation of a trusted application is available the Arm TrustZone, in OP-TEE.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com --- drivers/remoteproc/Kconfig | 8 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++++++++++++ include/linux/tee_remoteproc.h | 101 +++++++ 4 files changed, 507 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 a850e9f486dd..2969f067e3c3 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -365,6 +365,14 @@ 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 trusted application" + depends on OPTEE + help + Support for signed remote processors firmware management. The firmware + authentication is 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..ba5831af0f42 --- /dev/null +++ b/drivers/remoteproc/tee_remoteproc.c @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2023 STMicroelectronics - All Rights Reserved + * Authors: Arnaud Pouliquen arnaud.pouliquen@foss.st.com + */ + +#include <linux/firmware.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_MEMB 4 + +/* + * Authentication of the firmware and load in the remote processor memory + * + * [in] params[0].value.a: unique 32bit identifier of the firmware + * [in] params[1].memref: buffer containing the image of the buffer + */ +#define TA_RPROC_CMD_LOAD_FW 1 + +/* + * start the remote processor + * + * [in] params[0].value.a: unique 32bit identifier of the firmware + */ +#define TA_RPROC_CMD_START_FW 2 + +/* + * stop the remote processor + * + * [in] params[0].value.a: unique 32bit identifier of the firmware + */ +#define TA_RPROC_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 firmware + * [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_CMD_GET_RSC_TABLE 4 + +/* + * return the address of the core dump + * + * [in] params[0].value.a: unique 32bit identifier of the firmware + * [out] params[1].memref: address of the core dump image if exist, + * else return Null + */ +#define TA_RPROC_CMD_GET_COREDUMP 5 + +struct tee_rproc_mem { + char name[20]; + void __iomem *cpu_addr; + phys_addr_t bus_addr; + u32 dev_addr; + size_t size; +}; + +struct tee_rproc_context { + struct list_head sessions; + struct tee_context *tee_ctx; + struct device *dev; +}; + +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_MEMB * 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->fw_id, + }; +} + +int tee_rproc_load_fw(struct tee_rproc *trproc, const struct firmware *fw) +{ + struct tee_ioctl_invoke_arg arg; + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB]; + struct tee_shm *fw_shm; + int ret; + + fw_shm = tee_shm_register_kernel_buf(tee_rproc_ctx->tee_ctx, (void *)fw->data, fw->size); + if (IS_ERR(fw_shm)) { + /* Fallback: Try to allocate memory in OP-TEE space */ + fw_shm = tee_shm_alloc_kernel_buf(tee_rproc_ctx->tee_ctx, fw->size); + if (IS_ERR(fw_shm)) + return PTR_ERR(fw_shm); + + memcpy(tee_shm_get_va(fw_shm, 0), fw->data, fw->size); + } + + prepare_args(trproc, TA_RPROC_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_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(tee_rproc_load_fw); + +int rproc_tee_get_rsc_table(struct tee_rproc *trproc) +{ + struct tee_ioctl_invoke_arg arg; + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB]; + struct rproc *rproc = trproc->rproc; + size_t rsc_size; + int ret; + + prepare_args(trproc, TA_RPROC_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_CMD_GET_RSC_TABLE invoke failed TEE err: %x, ret:%x\n", + arg.ret, ret); + return -EIO; + } + + rsc_size = param[2].u.value.a; + + /* + * Store the resource table address that would be updated by the remote + * core and the virtio. + */ + trproc->rsc_va = ioremap_wc(param[1].u.value.a, rsc_size); + if (IS_ERR_OR_NULL(trproc->rsc_va)) { + dev_err(tee_rproc_ctx->dev, "Unable to map memory region: %lld+%zx\n", + param[1].u.value.a, rsc_size); + trproc->rsc_va = NULL; + return -ENOMEM; + } + + /* + * A cached table is requested as the physical address is not mapped yet + * but remoteproc need to parse the table for resources. + */ + rproc->cached_table = kmemdup(trproc->rsc_va, rsc_size, GFP_KERNEL); + if (!rproc->cached_table) + return -ENOMEM; + + rproc->table_ptr = rproc->cached_table; + rproc->table_sz = rsc_size; + + return 0; +} +EXPORT_SYMBOL(rproc_tee_get_rsc_table); + +struct resource_table *tee_rproc_get_loaded_rsc_table(struct tee_rproc *trproc) +{ + return (struct resource_table *)trproc->rsc_va; +} +EXPORT_SYMBOL(tee_rproc_get_loaded_rsc_table); + +int tee_rproc_start(struct tee_rproc *trproc) +{ + struct tee_ioctl_invoke_arg arg; + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB]; + int ret; + + prepare_args(trproc, TA_RPROC_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_CMD_START_FW invoke failed TEE err: %x, ret:%x\n", + arg.ret, ret); + if (!ret) + ret = -EIO; + } + + return ret; +} +EXPORT_SYMBOL(tee_rproc_start); + +int tee_rproc_stop(struct tee_rproc *trproc) +{ + struct tee_ioctl_invoke_arg arg; + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB]; + int ret; + + prepare_args(trproc, TA_RPROC_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_CMD_STOP_FW invoke failed TEE err: %x, ret:%x\n", + arg.ret, ret); + if (!ret) + ret = -EIO; + } + if (trproc->rsc_va) + iounmap(trproc->rsc_va); + trproc->rsc_va = NULL; + + return ret; +} +EXPORT_SYMBOL(tee_rproc_stop); + +static const struct tee_client_device_id stm32_tee_fw_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 fw_id) +{ + struct tee_client_device *rproc_tee_device; + struct tee_ioctl_open_session_arg sess_arg; + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB]; + 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 nt 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); + + rproc_tee_device = to_tee_client_device(tee_rproc_ctx->dev); + memset(&sess_arg, 0, sizeof(sess_arg)); + + /* Open session with rproc_tee load Trusted App */ + memcpy(sess_arg.uuid, rproc_tee_device->id.uuid.b, TEE_IOCTL_UUID_LEN); + + /* + * TODO: should we replace TEE_IOCTL_LOGIN_PUBLIC by + * TEE_IOCTL_LOGIN_REE_KERNEL? + */ + 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 = fw_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->fw_id = fw_id; + trproc->session_id = sess_arg.session; + + list_add_tail(&trproc->node, &tee_rproc_ctx->sessions); + + return trproc; +} +EXPORT_SYMBOL(tee_rproc_register); + +int tee_rproc_unregister(struct tee_rproc *trproc) +{ + int ret; + + if (!tee_rproc_ctx) + return -ENODEV; + + 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(tee_rproc_unregister); + +static int tee_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; + + /* Only one RPROC OP-TEE device allowed */ + if (tee_rproc_ctx) { + dev_err(dev, "An RPROC OP-TEE device was already initialized: only one allowed\n"); + return -EBUSY; + } + + /* Open context with TEE driver */ + tee_ctx = tee_client_open_context(NULL, tee_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_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_fw_id_table); + +static struct tee_client_driver tee_rproc_fw_driver = { + .id_table = stm32_tee_fw_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("secure 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..a9e7327f2308 --- /dev/null +++ b/include/linux/tee_remoteproc.h @@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2023 STMicroelectronics - All Rights Reserved + */ + +#ifndef TEE_REMOTEPROC_H +#define TEE_REMOTEPROC_H + +#include <linux/remoteproc.h> +#include <linux/tee_drv.h> + +/** + * struct tee_rproc - TEE remoteproc structure + * @node: Reference in list + * @rproc: Remoteproc reference + * @parent: Parent device + * @fw_id: Identifier of the target firmware + * @session_id: TEE session identifier + * @rsc_va: Resource table virtual address. + */ +struct tee_rproc { + struct list_head node; + + struct rproc *rproc; + struct device *parent; + u32 fw_id; + u32 session_id; + void *rsc_va; +}; + +#if IS_ENABLED(CONFIG_TEE_REMOTEPROC) + +struct tee_rproc *tee_rproc_register(struct device *dev, unsigned int fw_id); +int tee_rproc_unregister(struct tee_rproc *trproc); + +int tee_rproc_load_fw(struct tee_rproc *trproc, const struct firmware *fw); +int rproc_tee_get_rsc_table(struct tee_rproc *trproc); +struct resource_table *tee_rproc_get_loaded_rsc_table(struct tee_rproc *trproc); +int tee_rproc_start(struct tee_rproc *trproc); +int tee_rproc_stop(struct tee_rproc *trproc); + +#else + +static inline struct tee_rproc *tee_rproc_register(struct device *dev, + unsigned int fw_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 tee_rproc *trproc, + const struct firmware *fw) +{ + /* This shouldn't be possible */ + WARN_ON(1); + + return 0; +} + +static inline int tee_rproc_start(struct tee_rproc *trproc) +{ + /* This shouldn't be possible */ + WARN_ON(1); + + return 0; +} + +static inline int tee_rproc_stop(struct tee_rproc *trproc) +{ + /* This shouldn't be possible */ + WARN_ON(1); + + return 0; +} + +static inline int rproc_tee_get_rsc_table(struct tee_rproc *trproc) +{ + /* This shouldn't be possible */ + WARN_ON(1); + + return 0; +} + +static inline struct resource_table * + tee_rproc_get_loaded_rsc_table(struct tee_rproc *trproc) +{ + /* This shouldn't be possible */ + WARN_ON(1); + + return NULL; +} + +#endif /* CONFIG_TEE_REMOTEPROC */ +#endif /* TEE_REMOTEPROC_H */
On Tue, May 23, 2023 at 11:13:48AM +0200, Arnaud Pouliquen wrote:
- struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
- struct tee_shm *fw_shm;
- int ret;
- fw_shm = tee_shm_register_kernel_buf(tee_rproc_ctx->tee_ctx, (void *)fw->data, fw->size);
- if (IS_ERR(fw_shm)) {
/* Fallback: Try to allocate memory in OP-TEE space */
fw_shm = tee_shm_alloc_kernel_buf(tee_rproc_ctx->tee_ctx, fw->size);
+EXPORT_SYMBOL(tee_rproc_load_fw);
Please stick to the EXPORT_SYMBOL_GPL of the underlying tee infrastructure.
Add the support of signed firmware using new TEE remoteproc device to manage a remote firmware in a secure trusted application.
The support of a signed or a non signed firmware is based on compatible: - with legacy compatible "st,stm32mp1-m4" the Linux kernel loads a non-signed (ELF) firmware image, - with compatible "st,stm32mp1-m4-tee" the Linux kernel load a signed firmware image. In this case it calls TEE services to manage the firmware loading and the remoteproc life-cycle.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com --- drivers/remoteproc/Kconfig | 1 + drivers/remoteproc/stm32_rproc.c | 234 ++++++++++++++++++++++++++----- 2 files changed, 198 insertions(+), 37 deletions(-)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 2969f067e3c3..e6f0ca4adf8a 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -316,6 +316,7 @@ config STM32_RPROC depends on ARCH_STM32 depends on REMOTEPROC select MAILBOX + select TEE_REMOTEPROC help Say y here to support STM32 MCU processors via the remote processor framework. diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c index 8746cbb1f168..d2c909905cba 100644 --- a/drivers/remoteproc/stm32_rproc.c +++ b/drivers/remoteproc/stm32_rproc.c @@ -20,6 +20,7 @@ #include <linux/remoteproc.h> #include <linux/reset.h> #include <linux/slab.h> +#include <linux/tee_remoteproc.h> #include <linux/workqueue.h>
#include "remoteproc_internal.h" @@ -49,6 +50,13 @@ #define M4_STATE_STANDBY 4 #define M4_STATE_CRASH 5
+/* + * Define a default index in future may come a global list of + * firmwares which list platforms and associated firmware(s) + */ + +#define STM32_MP1_FW_ID 0 + struct stm32_syscon { struct regmap *map; u32 reg; @@ -89,6 +97,8 @@ struct stm32_rproc { struct stm32_mbox mb[MBOX_NB_MBX]; struct workqueue_struct *workqueue; bool secured_soc; + bool fw_loaded; + struct tee_rproc *trproc; void __iomem *rsc_va; };
@@ -208,6 +218,139 @@ static int stm32_rproc_mbox_idx(struct rproc *rproc, const unsigned char *name) return -EINVAL; }
+static void stm32_rproc_request_shutdown(struct rproc *rproc) +{ + struct stm32_rproc *ddata = rproc->priv; + int err, dummy_data, idx; + + /* Request shutdown of the remote processor */ + if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) { + idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN); + if (idx >= 0 && ddata->mb[idx].chan) { + /* A dummy data is sent to allow to block on transmit. */ + err = mbox_send_message(ddata->mb[idx].chan, + &dummy_data); + if (err < 0) + dev_warn(&rproc->dev, "warning: remote FW shutdown without ack\n"); + } + } +} + +static int stm32_rproc_release(struct rproc *rproc) +{ + struct stm32_rproc *ddata = rproc->priv; + unsigned int err = 0; + + /* To allow platform Standby power mode, set remote proc Deep Sleep. */ + if (ddata->pdds.map) { + err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg, + ddata->pdds.mask, 1); + if (err) { + dev_err(&rproc->dev, "failed to set pdds\n"); + return err; + } + } + + /* Update coprocessor state to OFF if available. */ + if (ddata->m4_state.map) { + err = regmap_update_bits(ddata->m4_state.map, + ddata->m4_state.reg, + ddata->m4_state.mask, + M4_STATE_OFF); + if (err) { + dev_err(&rproc->dev, "failed to set copro state\n"); + return err; + } + } + + return err; +} + +static int stm32_rproc_tee_elf_sanity_check(struct rproc *rproc, + const struct firmware *fw) +{ + struct stm32_rproc *ddata = rproc->priv; + unsigned int ret = 0; + + if (rproc->state == RPROC_DETACHED) + return 0; + + ret = tee_rproc_load_fw(ddata->trproc, fw); + if (!ret) + ddata->fw_loaded = true; + + return ret; +} + +static int stm32_rproc_tee_elf_load(struct rproc *rproc, + const struct firmware *fw) +{ + struct stm32_rproc *ddata = rproc->priv; + unsigned int ret; + + /* + * This function can be called by remote proc for recovery + * without the sanity check. In this case we need to load the firmware + * else nothing done here as the firmware has been preloaded for the + * sanity check to be able to parse it for the resource table + */ + if (ddata->fw_loaded) + return 0; + + ret = tee_rproc_load_fw(ddata->trproc, fw); + if (ret) + return ret; + ddata->fw_loaded = true; + + /* update the resource table parameters */ + if (rproc_tee_get_rsc_table(ddata->trproc)) { + /* no resource table: reset the related fields */ + rproc->cached_table = NULL; + rproc->table_ptr = NULL; + rproc->table_sz = 0; + } + + return 0; +} + +static struct resource_table * +stm32_rproc_tee_elf_find_loaded_rsc_table(struct rproc *rproc, + const struct firmware *fw) +{ + struct stm32_rproc *ddata = rproc->priv; + + return tee_rproc_get_loaded_rsc_table(ddata->trproc); +} + +static int stm32_rproc_tee_start(struct rproc *rproc) +{ + struct stm32_rproc *ddata = rproc->priv; + + return tee_rproc_start(ddata->trproc); +} + +static int stm32_rproc_tee_attach(struct rproc *rproc) +{ + /* Nothing to do, remote proc already started by the secured context */ + return 0; +} + +static int stm32_rproc_tee_stop(struct rproc *rproc) +{ + struct stm32_rproc *ddata = rproc->priv; + int err; + + stm32_rproc_request_shutdown(rproc); + + err = tee_rproc_stop(ddata->trproc); + if (err) + return err; + + ddata->fw_loaded = false; + + return stm32_rproc_release(rproc); +} + static int stm32_rproc_prepare(struct rproc *rproc) { struct device *dev = rproc->dev.parent; @@ -270,7 +413,14 @@ static int stm32_rproc_prepare(struct rproc *rproc)
static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) { - if (rproc_elf_load_rsc_table(rproc, fw)) + struct stm32_rproc *ddata = rproc->priv; + int ret; + + if (ddata->trproc) + ret = rproc_tee_get_rsc_table(ddata->trproc); + else + ret = rproc_elf_load_rsc_table(rproc, fw); + if (ret) dev_warn(&rproc->dev, "no resource table found for this firmware\n");
return 0; @@ -503,17 +653,9 @@ static int stm32_rproc_detach(struct rproc *rproc) static int stm32_rproc_stop(struct rproc *rproc) { struct stm32_rproc *ddata = rproc->priv; - int err, idx; + int err;
- /* request shutdown of the remote processor */ - if (rproc->state != RPROC_OFFLINE && rproc->state != RPROC_CRASHED) { - idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN); - if (idx >= 0 && ddata->mb[idx].chan) { - err = mbox_send_message(ddata->mb[idx].chan, "detach"); - if (err < 0) - dev_warn(&rproc->dev, "warning: remote FW shutdown without ack\n"); - } - } + stm32_rproc_request_shutdown(rproc);
err = stm32_rproc_set_hold_boot(rproc, true); if (err) @@ -525,29 +667,8 @@ static int stm32_rproc_stop(struct rproc *rproc) return err; }
- /* to allow platform Standby power mode, set remote proc Deep Sleep */ - if (ddata->pdds.map) { - err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg, - ddata->pdds.mask, 1); - if (err) { - dev_err(&rproc->dev, "failed to set pdds\n"); - return err; - } - } - - /* update coprocessor state to OFF if available */ - if (ddata->m4_state.map) { - err = regmap_update_bits(ddata->m4_state.map, - ddata->m4_state.reg, - ddata->m4_state.mask, - M4_STATE_OFF); - if (err) { - dev_err(&rproc->dev, "failed to set copro state\n"); - return err; - } - }
- return 0; + return stm32_rproc_release(rproc); }
static void stm32_rproc_kick(struct rproc *rproc, int vqid) @@ -659,8 +780,22 @@ static const struct rproc_ops st_rproc_ops = { .get_boot_addr = rproc_elf_get_boot_addr, };
+static const struct rproc_ops st_rproc_tee_ops = { + .prepare = stm32_rproc_prepare, + .start = stm32_rproc_tee_start, + .stop = stm32_rproc_tee_stop, + .attach = stm32_rproc_tee_attach, + .kick = stm32_rproc_kick, + .parse_fw = stm32_rproc_parse_fw, + .find_loaded_rsc_table = stm32_rproc_tee_elf_find_loaded_rsc_table, + .get_loaded_rsc_table = stm32_rproc_get_loaded_rsc_table, + .sanity_check = stm32_rproc_tee_elf_sanity_check, + .load = stm32_rproc_tee_elf_load, +}; + static const struct of_device_id stm32_rproc_match[] = { - { .compatible = "st,stm32mp1-m4" }, + {.compatible = "st,stm32mp1-m4",}, + {.compatible = "st,stm32mp1-m4-tee",}, {}, }; MODULE_DEVICE_TABLE(of, stm32_rproc_match); @@ -801,6 +936,7 @@ static int stm32_rproc_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct stm32_rproc *ddata; struct device_node *np = dev->of_node; + struct tee_rproc *trproc; struct rproc *rproc; unsigned int state; int ret; @@ -809,11 +945,29 @@ static int stm32_rproc_probe(struct platform_device *pdev) if (ret) return ret;
- rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata)); - if (!rproc) - return -ENOMEM; + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) { + trproc = tee_rproc_register(dev, STM32_MP1_FW_ID); + if (IS_ERR_OR_NULL(trproc)) { + return PTR_ERR(trproc); + /* + * Delegate the firmware management to the secure context. + * The firmware loaded has to be signed. + */ + dev_info(dev, "Support of signed firmware only\n"); + } + } + rproc = rproc_alloc(dev, np->name, + trproc ? &st_rproc_tee_ops : &st_rproc_ops, + NULL, sizeof(*ddata)); + if (!rproc) { + ret = -ENOMEM; + goto free_tee; + }
ddata = rproc->priv; + ddata->trproc = trproc; + if (trproc) + ddata->trproc->rproc = rproc;
rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
@@ -864,6 +1018,10 @@ static int stm32_rproc_probe(struct platform_device *pdev) device_init_wakeup(dev, false); } rproc_free(rproc); +free_tee: + if (trproc) + tee_rproc_unregister(trproc); + return ret; }
@@ -885,6 +1043,8 @@ static int stm32_rproc_remove(struct platform_device *pdev) device_init_wakeup(dev, false); } rproc_free(rproc); + if (ddata->trproc) + tee_rproc_unregister(ddata->trproc);
return 0; }
On Tue, May 23, 2023 at 11:13:46AM +0200, Arnaud Pouliquen wrote:
This RFC proposes an implementation of a remoteproc tee driver to communicate with a TEE trusted application in charge of authenticating and loading remoteproc firmware image in an Arm secure context.
The services implemented are the same as those offered by the Linux remoteproc framework:
- load of a signed firmware
- start/stop of a coprocessor
- get the resource table
The OP-TEE code in charge of providing the service in a trusted application is proposed for upstream here: https://github.com/OP-TEE/optee_os/pull/6027
For more details on the implementation a presentation is available here: https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (4): tee: Re-enable vmalloc page support for shared memory remoteproc: Add TEE support dt-bindings: remoteproc: add compatibility for TEE support remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +- drivers/remoteproc/Kconfig | 9 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/stm32_rproc.c | 234 +++++++++-- drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++ drivers/tee/tee_shm.c | 24 +- include/linux/tee_remoteproc.h | 101 +++++ 7 files changed, 753 insertions(+), 46 deletions(-) create mode 100644 drivers/remoteproc/tee_remoteproc.c create mode 100644 include/linux/tee_remoteproc.h
Looking at comments from Christoph, there seems to be a good refactoring exercise in store for this pathset. As such I will wait for the next revision to look at it.
Thanks, Mathieu
-- 2.25.1
Hello Mathieu,
On 5/30/23 18:20, Mathieu Poirier wrote:
On Tue, May 23, 2023 at 11:13:46AM +0200, Arnaud Pouliquen wrote:
This RFC proposes an implementation of a remoteproc tee driver to communicate with a TEE trusted application in charge of authenticating and loading remoteproc firmware image in an Arm secure context.
The services implemented are the same as those offered by the Linux remoteproc framework:
- load of a signed firmware
- start/stop of a coprocessor
- get the resource table
The OP-TEE code in charge of providing the service in a trusted application is proposed for upstream here: https://github.com/OP-TEE/optee_os/pull/6027
For more details on the implementation a presentation is available here: https://resources.linaro.org/en/resource/6c5bGvZwUAjX56fvxthxds
Arnaud Pouliquen (4): tee: Re-enable vmalloc page support for shared memory remoteproc: Add TEE support dt-bindings: remoteproc: add compatibility for TEE support remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
.../bindings/remoteproc/st,stm32-rproc.yaml | 33 +- drivers/remoteproc/Kconfig | 9 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/stm32_rproc.c | 234 +++++++++-- drivers/remoteproc/tee_remoteproc.c | 397 ++++++++++++++++++ drivers/tee/tee_shm.c | 24 +- include/linux/tee_remoteproc.h | 101 +++++ 7 files changed, 753 insertions(+), 46 deletions(-) create mode 100644 drivers/remoteproc/tee_remoteproc.c create mode 100644 include/linux/tee_remoteproc.h
Looking at comments from Christoph, there seems to be a good refactoring exercise in store for this pathset.
Yes, a good opportunity to ramp-up on kernel memory management :)
As such I will wait for the next revision
to look at it.
That's fair. More than that I would prefer to focus first on OP-TEE part that provides the service. The OP-TEE pull request review could have significant impacts on the kernel implementation...
Thanks, Arnaud
Thanks, Mathieu
-- 2.25.1
op-tee@lists.trustedfirmware.org