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
--
2.25.1
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 <[email protected]>
---
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;
}
--
2.25.1
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 <[email protected]>
---
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;
--
2.25.1
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 <[email protected]>
---
.../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>;
--
2.25.1
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/[email protected]/
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/[email protected]/
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/[email protected]/
>
> 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
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 <[email protected]>
> ---
> .../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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>>> ---
>>> .../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
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
>>