Firmware handoff #6308

raymo200915 · 2023-09-20T12:24:48Z

Add supports for Transfer List into core, which are compliant to the Firmware Handoff
specification v0.9 (https://github.com/FirmwareHandoff/firmware_handoff/releases/tag/v0.9).

The motivation for this patch set is to unify the way of arguments handoff between all boot stages (BL2/BL31/BL32/BL33).
In current practice, when the Transfer List is enabled between BL2 and BL33 (See the related patch sets for TF-A below for reference),
we need this patch to allow the DTB nodes appended by OP-TEE can be reflected into the Transfer List memory and passed to BL33.

Reference:

Patch to introduce Transfer List into TF-A: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22215
Patch to enable Transfer List between the handoff from BL2 to BL33 on qemu: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/22178

jenswi-linaro · 2023-09-20T15:39:20Z

Please rebase to resolve the conflicts. We also need to decide if firmware handoff will be used by OP-TEE or not. A good use case is enough motivation to include this.

etienne-lms

Some generic coding style comments that could be addressed before you rebase.
Could you add a link the the Firmware Handoff Transfer List specification?

TL is too vague I think as acronym. If you want an acronym, maybe FHTL would be better (Firmware Handoff Transfer List, as its the name of the spec).

core/include/mm/core_mmu.h

core/include/kernel/transfer_list.h

core/kernel/transfer_list.c

mk/config.mk

core/include/kernel/transfer_list.h

raymo200915 · 2023-09-21T09:26:55Z

Updated the patch set with:

Fix to enhance the backwards compatibility for legacy handoff if no Transfer List is detected through the arguments.
Fix of coding style issues (False-positive spelling warnings are still remained).
Rebase to master.

raymo200915 · 2023-09-21T09:28:34Z

Please rebase to resolve the conflicts. We also need to decide if firmware handoff will be used by OP-TEE or not. A good use case is enough motivation to include this.

Rebased and add description on the motivation, links of specification and reference.

raymo200915 · 2023-09-21T09:55:13Z

Some generic coding style comments that could be addressed before you rebase. Could you add a link the the Firmware Handoff Transfer List specification?

TL is too vague I think as acronym. If you want an acronym, maybe FHTL would be better (Firmware Handoff Transfer List, as its the name of the spec).

Fix in the commit [fixup] core: init dt when no DTB observed from TL

I guess at current stage I would prefer keep it named as "Transfer List", since the specification is named as "Firmware Handoff specification", generally "handoff" is a common terms and can by different implementations - The current one is implemented via "Transfer List" - That is the reason I use "TL" for the acronym, similiarly "TE" for "Transfer Entry".

raymo200915 · 2023-09-21T13:11:05Z

All comments have been addressed.

jenswi-linaro · 2023-09-26T14:38:24Z

Please squash the fixups.

raymo200915 · 2023-09-26T14:56:56Z

Squash done.

core/include/kernel/transfer_list.h

core/kernel/transfer_list.c

core/arch/arm/kernel/entry_a64.S

core/arch/arm/kernel/boot.c

core/kernel/dt.c

core/arch/arm/kernel/entry_a32.S

core/include/kernel/transfer_list.h

core/kernel/transfer_list.c

raymo200915 · 2023-10-02T15:03:00Z

All comments have been addressed.

core/arch/arm/kernel/boot.c

core/arch/arm/kernel/entry_a32.S

core/arch/arm/kernel/boot.c

core/kernel/transfer_list.c

core/kernel/dt.c

jenswi-linaro · 2023-10-03T08:33:21Z

The handling of boot arguments becomes even more complicated with this. I'm about to propose a PR to use common handling of boot arguments in C.

jenswi-linaro · 2023-10-03T09:53:28Z

#6335

raymo200915 · 2023-10-03T15:46:22Z

The handling of boot arguments becomes even more complicated with this. I'm about to propose a PR to use common handling of boot arguments in C.

Just took a quick look at the PR, seems that all arguments are saved instead of passing into individual boot functions.
I can hold this patch after addressing remained comments and rebase all after your PR is merged.

raymo200915 · 2023-10-06T14:01:27Z

All comments have been addressed.

core/include/kernel/transfer_list.h

core/kernel/transfer_list.c

raymo200915 · 2023-10-10T17:48:26Z

All comments have been addressed.
If there no upcoming comments I will squash all [review] commits and add Reviewed-by tags.

etienne-lms

I fine you squash the fixup commits.
Some minor late comments.

core/arch/arm/kernel/boot.c

core/kernel/dt.c

core/kernel/transfer_list.c

raymo200915 · 2023-10-12T15:37:55Z

All comments have been addressed.
Rebased to master, squashed all fixup commits, added Reviewed-by tags.

core/arch/arm/kernel/boot.c

etienne-lms · 2023-10-12T18:06:33Z

@raymo200915, actually no review tag were posted yet for the 3 commits. We only posted comments.
Please update mine with Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> for the 3 commits.
(edited: my A-b tag assuming #6308 (comment) is irrelevant or fixed)

etienne-lms · 2023-10-12T18:08:52Z

Checkpatch report are all false positive, all about missspelled te:

WARNING: 'te' may be misspelled - perhaps 'the'?
#55: FILE: core/arch/arm/kernel/boot.c:1262:
+	struct transfer_list_entry *te = NULL;
 	                            ^^

raymo200915 · 2023-10-12T19:45:37Z

@raymo200915, actually no review tag were posted yet for the 3 commits. We only posted comments. Please update mine with Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> for the 3 commits. (edited: my A-b tag assuming #6308 (comment) is irrelevant or fixed)

Done. Sorry, I misunderstand the purpose of Reviewed-by tag.

raymo200915 · 2023-11-03T15:13:53Z

For "core: add memory area for transfer list" please apply: Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Done.

raymo200915 · 2023-11-03T15:16:27Z

All comments have been addressed.

jenswi-linaro · 2023-11-06T09:43:43Z

For "core: add support for transfer list" please apply:
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

For "core: add transfer list API" please apply:
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

raymo200915 · 2023-11-06T14:19:13Z

For "core: add support for transfer list" please apply: Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

For "core: add transfer list API" please apply: Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Thanks @jenswi-linaro , tags added.

jforissier · 2023-11-06T17:06:41Z

There is a link error in "CI / make (multi-platform)"
How about renaming te to e to please checkpatch? e for entry is as good as te for transfer list entry IMO.
The Hafnium CI error is probably unrelated, I'll look into it

etienne-lms

comment for commit "core: add transfer list API".

core/kernel/transfer_list.c

raymo200915 · 2023-11-06T20:39:57Z

There is a link error in "CI / make (multi-platform)"

How about renaming te to e to please checkpatch? e for entry is as good as te for transfer list entry IMO.

The Hafnium CI error is probably unrelated, I'll look into it

Fixed the clang build error in multi-platform.
All te are renamed to tl_e for 'transfer list entry', no more checkpatch complains observed.

raymo200915 · 2023-11-06T20:44:49Z

Minor fixes applied:
Adding a stub function "boot_save_transfer_list()" to fix the CI clang build error.
Optimize function transfer_list_add().
Clean up checkpatch false positive warnings.

raymo200915 · 2023-11-07T15:29:08Z

Fixed another CI multi-platform build error.

etienne-lms · 2023-11-07T15:51:23Z

@raymo200915, it's hard to review again the whole changes after your latest updates since we don't really know which piece of code you updated. This is where appending fixup commits on top of an series of the patches we have reviewed (acked or not acked, not matter) is quite handy for the dear reviewers.
Nevertheless i'll have another review round...

@jforissier, could you enable CI tests for this P-R?
@raymo200915, it would be nice the CI includes a build test with CFG_TRANSFER_LIST=y. You can ~~archive~~ achieve that by patching file .github/workflows/ci.yaml, see that file Git history for examples.

(edited ;-)>

raymo200915 · 2023-11-07T22:05:57Z

@raymo200915, it's hard to review again the whole changes after your latest updates since we don't really know which piece of code you updated. This is where appending fixup commits on top of an series of the patches we have reviewed (acked or not acked, not matter) is quite handy for the dear reviewers. Nevertheless i'll have another review round...

@jforissier, could you enable CI tests for this P-R? @raymo200915, it would be nice the CI includes a build test with CFG_TRANSFER_LIST=y. You can ~~archive~~ achieve that by patching file .github/workflows/ci.yaml, see that file Git history for examples.

(edited ;-)>

Sorry for my last force-push...
Applied a minor fix for the stub function as @jenswi-linaro suggested.
Added CI build with CFG_TRANSFER_LIST and CFG_MAP_EXT_DT_SECURE.

etienne-lms

Thanks @raymo200915.
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> for the series.

jenswi-linaro · 2023-11-08T14:40:34Z

The latest fixups look good.

Adding a new area to map a transfer list if it is handed over from previous boot stage Signed-off-by: Raymond Mao <raymond.mao@linaro.org> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

Introduce Transfer List API into kernel to implement Firmware Handoff specification Link: https://github.com/FirmwareHandoff/firmware_handoff Signed-off-by: Raymond Mao <raymond.mao@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

Add supports for Transfer List on both aarch32/64. Fetch arguments from {x,r}{0-3} and check if a valid Transfer List exists, which compliant to the Firmware Handoff specification. The Transfer List will be mapped during early initialization and unmapped before exiting to next boot stage. DTB and pagable address will be parsed from the Transfer List if they exist as Transfer Entries. If Transfer List does not exist or is invalid, legacy argument handoff is backwards compatible. Signed-off-by: Raymond Mao <raymond.mao@linaro.org> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

Add CI multi-platform build with CFG_TRANSFER_LIST Signed-off-by: Raymond Mao <raymond.mao@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

raymo200915 · 2023-11-08T15:29:29Z

Squashed fixup commits and added Etienne's tag.

jforissier · 2023-11-09T14:32:42Z

Thanks @raymo200915

etienne-lms reviewed Sep 21, 2023

View reviewed changes

raymo200915 force-pushed the firmware_handoff branch from fca57fc to 6fb3c95 Compare September 21, 2023 09:06

raymo200915 force-pushed the firmware_handoff branch from 5a08df2 to 06abaf6 Compare September 26, 2023 14:55

raymo200915 force-pushed the firmware_handoff branch from 06abaf6 to cb0a351 Compare September 26, 2023 15:44

jenswi-linaro reviewed Sep 27, 2023

View reviewed changes

etienne-lms reviewed Sep 29, 2023

View reviewed changes

manish-pandey-arm mentioned this pull request Sep 29, 2023

Firmware handoff library hosting place FirmwareHandoff/firmware_handoff#20

Open

etienne-lms reviewed Oct 3, 2023

View reviewed changes

jenswi-linaro reviewed Oct 6, 2023

View reviewed changes

etienne-lms reviewed Oct 10, 2023

View reviewed changes

core/arch/arm/kernel/boot.c Show resolved Hide resolved

core/kernel/dt.c Outdated Show resolved Hide resolved

core/kernel/dt.c Outdated Show resolved Hide resolved

core/kernel/transfer_list.c Show resolved Hide resolved

raymo200915 force-pushed the firmware_handoff branch from beb77e0 to e7bbddd Compare October 12, 2023 15:36

etienne-lms reviewed Oct 12, 2023

View reviewed changes

core/arch/arm/kernel/boot.c Outdated Show resolved Hide resolved

raymo200915 force-pushed the firmware_handoff branch from 9628980 to aca63db Compare October 12, 2023 19:41

raymo200915 force-pushed the firmware_handoff branch from 63dad43 to c8a3f89 Compare November 3, 2023 15:13

raymo200915 force-pushed the firmware_handoff branch from c8a3f89 to 10c59e5 Compare November 6, 2023 14:18

etienne-lms reviewed Nov 6, 2023

View reviewed changes

core/kernel/transfer_list.c Show resolved Hide resolved

etienne-lms reviewed Nov 6, 2023

View reviewed changes

core/kernel/transfer_list.c Outdated Show resolved Hide resolved

core/kernel/transfer_list.c Outdated Show resolved Hide resolved

raymo200915 force-pushed the firmware_handoff branch 2 times, most recently from 40b48a0 to 993ab14 Compare November 6, 2023 20:35

raymo200915 force-pushed the firmware_handoff branch from 993ab14 to bb7b8b4 Compare November 7, 2023 15:28

etienne-lms reviewed Nov 8, 2023

View reviewed changes

raymo200915 added 4 commits November 8, 2023 07:26

ci: vexpress-qemu_armv8a: add transfer list build

274fc26

Add CI multi-platform build with CFG_TRANSFER_LIST Signed-off-by: Raymond Mao <raymond.mao@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>

raymo200915 force-pushed the firmware_handoff branch from b50bb94 to 274fc26 Compare November 8, 2023 15:28

jforissier merged commit 8bf1002 into OP-TEE:master Nov 9, 2023
8 checks passed

This was referenced Nov 10, 2023

[RFC] Enable Firmware Handoff in Qemu-Arm via bloblist raymo200915/u-boot#2

Draft

[RFC] Enable Firmware Handoff in Qemu-Arm via Transfer List library raymo200915/u-boot#1

Draft

raymo200915 mentioned this pull request Dec 15, 2023

[RFC] Enable Firmware Handoff in Qemu-Arm via bloblist v2 raymo200915/u-boot#3

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firmware handoff #6308

Firmware handoff #6308

raymo200915 commented Sep 20, 2023 •

edited

jenswi-linaro commented Sep 20, 2023

etienne-lms left a comment

raymo200915 commented Sep 21, 2023

raymo200915 commented Sep 21, 2023

raymo200915 commented Sep 21, 2023 •

edited

raymo200915 commented Sep 21, 2023

jenswi-linaro commented Sep 26, 2023

raymo200915 commented Sep 26, 2023

raymo200915 commented Oct 2, 2023

jenswi-linaro commented Oct 3, 2023

jenswi-linaro commented Oct 3, 2023

raymo200915 commented Oct 3, 2023

raymo200915 commented Oct 6, 2023

raymo200915 commented Oct 10, 2023

etienne-lms left a comment

raymo200915 commented Oct 12, 2023

etienne-lms commented Oct 12, 2023 •

edited

etienne-lms commented Oct 12, 2023

raymo200915 commented Oct 12, 2023

raymo200915 commented Nov 3, 2023

raymo200915 commented Nov 3, 2023

jenswi-linaro commented Nov 6, 2023

raymo200915 commented Nov 6, 2023

jforissier commented Nov 6, 2023

etienne-lms left a comment

raymo200915 commented Nov 6, 2023 •

edited

raymo200915 commented Nov 6, 2023

raymo200915 commented Nov 7, 2023

etienne-lms commented Nov 7, 2023 •

edited

raymo200915 commented Nov 7, 2023

etienne-lms left a comment

jenswi-linaro commented Nov 8, 2023

raymo200915 commented Nov 8, 2023

jforissier commented Nov 9, 2023

Firmware handoff #6308

Firmware handoff #6308

Conversation

raymo200915 commented Sep 20, 2023 • edited

jenswi-linaro commented Sep 20, 2023

etienne-lms left a comment

Choose a reason for hiding this comment

raymo200915 commented Sep 21, 2023

raymo200915 commented Sep 21, 2023

raymo200915 commented Sep 21, 2023 • edited

raymo200915 commented Sep 21, 2023

jenswi-linaro commented Sep 26, 2023

raymo200915 commented Sep 26, 2023

raymo200915 commented Oct 2, 2023

jenswi-linaro commented Oct 3, 2023

jenswi-linaro commented Oct 3, 2023

raymo200915 commented Oct 3, 2023

raymo200915 commented Oct 6, 2023

raymo200915 commented Oct 10, 2023

etienne-lms left a comment

Choose a reason for hiding this comment

raymo200915 commented Oct 12, 2023

etienne-lms commented Oct 12, 2023 • edited

etienne-lms commented Oct 12, 2023

raymo200915 commented Oct 12, 2023

raymo200915 commented Nov 3, 2023

raymo200915 commented Nov 3, 2023

jenswi-linaro commented Nov 6, 2023

raymo200915 commented Nov 6, 2023

jforissier commented Nov 6, 2023

etienne-lms left a comment

Choose a reason for hiding this comment

raymo200915 commented Nov 6, 2023 • edited

raymo200915 commented Nov 6, 2023

raymo200915 commented Nov 7, 2023

etienne-lms commented Nov 7, 2023 • edited

raymo200915 commented Nov 7, 2023

etienne-lms left a comment

Choose a reason for hiding this comment

jenswi-linaro commented Nov 8, 2023

raymo200915 commented Nov 8, 2023

jforissier commented Nov 9, 2023

raymo200915 commented Sep 20, 2023 •

edited

raymo200915 commented Sep 21, 2023 •

edited

etienne-lms commented Oct 12, 2023 •

edited

raymo200915 commented Nov 6, 2023 •

edited

etienne-lms commented Nov 7, 2023 •

edited