Test #1
Closed
Test #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a resource table is loaded by an external entity such as U-boot or OP-TEE, we do not necessarily get the device address(da) but the physical address(pa). This helper performs similar translation than the rproc_da_to_va() but based on a physical address. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.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 the secure part, this driver offers a client interface to load firmware
by the secure part.
This firmware could be authenticated by the secure trusted application.
A specificity of the implementation is that the firmware has to be
authenticated and optionally decrypted to access the resource table.
Consequently, the boot sequence is:
1) rproc_parse_fw --> rproc_tee_parse_fw
remoteproc TEE:
- Requests the TEE application to authenticate and load the firmware
in the remote processor memories.
- Requests the TEE application for the address of the resource table.
- Creates a copy of the resource table stored in rproc->cached_table.
2) rproc_load_segments --> rproc_tee_load_fw
remoteproc TEE:
- Requests the TEE application to load the firmware. Nothing is done
at the TEE application as the firmware is already loaded.
- In case of recovery, the TEE application has to reload the firmware.
3) rproc_tee_get_loaded_rsc_table
remoteproc TEE requests the TEE application for the address of the
resource table.
4) rproc_start --> rproc_tee_start
- Requests the TEE application to start the remote processor.
The shutdown sequence is:
5) rproc_stop --> rproc_tee_stop
- Requests the TEE application to stop the remote processor.
6) rproc_tee_release_fw
This function is used to request the TEE application to perform actions
to return to the initial state on stop or on error during the boot
sequence.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
Updates vs version [18]:
- rework/fix function headers
- use memremap instead of ioremap for the resource table.
- realign comments to 80 chars limit, with few exceptions for readability
- replace spinlock by mutex and and protect APIs from concurrent access
- add support of 64-bit address in rproc_tee_get_loaded_rsc_table()
- update copyright year
Updates vs version [17]:
Fix warning:
warning: EXPORT_SYMBOL() is used, but #include <linux/export.h> is missing
The release_fw operation allows releasing remote processor resources configured during rproc_parse_fw() and rproc_load_segments(). For example, it can release carveouts mapped during these operations. This operation is invoked in the following cases: - When an error occurs during remote processor boot. - When an error occurs during remote processor recovery start. - After stopping the remote processor. This operation is required for the remoteproc_tee implementation following a stop or upon encountering an error. Since the remoteproc image is loaded during resource table parsing, multiple failure scenarios may occur prior to remote processor startup, including issues with resource handling and carveout allocation. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> --- Updates from version 18: - Rewrite rproc_ops::release_fw documentation - Improve commit message Updates from version 16: - Removed the rproc:load_fw() ops introduced in previous version. - Removed duplicate calls to rproc_release_fw in rproc_fw_boot and rproc_boot.
The "st,stm32mp1-m4-tee" compatible is utilized in a system configuration where the Cortex-M4 firmware is loaded by the Trusted Execution Environment (TEE). For instance, this compatible is used in both the Linux and OP-TEE device trees: - In OP-TEE, a node is defined in the device tree with the "st,stm32mp1-m4-tee" compatible to support signed remoteproc firmware. Based on DT properties, the OP-TEE remoteproc framework is initiated to expose a trusted application service to authenticate and load the remote processor firmware provided by the Linux remoteproc framework, as well as to start and stop the remote processor. - In Linux, when the compatibility is set, the Cortex-M resets should not be declared in the device tree. In such a configuration, the reset is managed by the OP-TEE remoteproc driver and is no longer accessible from the Linux kernel. Associated with this new compatible, add the "st,proc-id" property to identify the remote processor. This ID is used to define a unique ID, common between Linux, U-Boot, and OP-TEE, to identify a coprocessor. This ID will be used in requests to the OP-TEE remoteproc Trusted Application to specify the remote processor. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
To prepare for the support of TEE remoteproc, create sub-functions that can be used in both cases, with and without remoteproc TEE support. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
The new TEE remoteproc driver is used to manage remote firmware in a secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is introduced to delegate the loading of the firmware to the trusted execution context. In such cases, the firmware should be signed and adhere to the image format defined by the TEE. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
There was a problem hiding this comment.
Pull Request Overview
Adds support for managing remote processors via a Trusted Execution Environment (TEE).
- Introduces a new
remoteproc_teeinterface and stubs in headers. - Implements a TEE client module (
remoteproc_tee.c) to handle firmware load, start/stop, and resource-table queries. - Integrates TEE operations into the STM32 remoteproc driver and updates core APIs (
release_fw,pa_to_va) to support cleanup and address translation.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| include/linux/remoteproc_tee.h | New TEE interface definitions and CONFIG_REMOTEPROC_TEE stubs |
| include/linux/remoteproc.h | Added release_fw to rproc_ops and rproc_pa_to_va prototype |
| drivers/remoteproc/stm32_rproc.c | Factored shutdown, added TEE ops, and probe/remove adjustments |
| drivers/remoteproc/remoteproc_tee.c | New implementation of TEE-backed remoteproc operations |
| drivers/remoteproc/remoteproc_internal.h | Added rproc_release_fw() helper |
| drivers/remoteproc/remoteproc_core.c | Implemented rproc_pa_to_va, calls to rproc_release_fw |
| drivers/remoteproc/Makefile | Built remoteproc_tee.o when enabled |
| drivers/remoteproc/Kconfig | New CONFIG_REMOTEPROC_TEE option |
| Documentation/devicetree/bindings/…/st,stm32-rproc.yaml | Extended DT binding for TEE-managed Cortex-M4 |
Comments suppressed due to low confidence (3)
drivers/remoteproc/stm32_rproc.c:902
- Error message references
st,rproc-idbut the code readsst,proc-id. Update the string to match the actual property name (st,proc-id).
dev_err(dev, "failed to read st,rproc-id property\n");
drivers/remoteproc/remoteproc_core.c:241
- The doc refers to
@dabut the function parameter is named@pa. Update the comment to reference@pafor consistency.
*
drivers/remoteproc/stm32_rproc.c:969
- Calling
rproc_tee_unregisterin the probe function immediately removes the TEE session you just created. This should only be invoked on remove or error paths.
rproc_tee_unregister(dev, rproc);
| .get_boot_addr = rproc_elf_get_boot_addr, | ||
| }; | ||
|
|
||
| static const struct rproc_ops st_rproc_tee_ops = { |
There was a problem hiding this comment.
st_rproc_tee_ops is missing a .get_boot_addr implementation (e.g., rproc_elf_get_boot_addr), so consumers of this ops struct will dereference a NULL pointer when computing the boot address.
| 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"); |
There was a problem hiding this comment.
[nitpick] Consider defining a constant or enum for the shutdown message payload instead of using a string literal directly to improve readability and maintainability.
Suggested change
| err = mbox_send_message(ddata->mb[idx].chan, "detach"); | |
| err = mbox_send_message(ddata->mb[idx].chan, STM32_MBX_DETACH_MSG); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.