From e13d027b7059f29bc8e6163b2244ab7af59c3ff7 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Fri, 27 Sep 2024 16:04:55 +0000 Subject: [PATCH 1/8] refactor(handoff): downgrade error messages Some APIs, like `transfer_list_check_header`, are used preemptively to determine if a new TL needs to be initialized. If we validate a TL and anticipate its contents to be invalid or corrupted, logging these as error message isn't helpful. Change-Id: Ic22378828548d48f73aa74d494f110fbd11857f4 Signed-off-by: Harrison Mutai --- lib/transfer_list/transfer_list.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/transfer_list/transfer_list.c b/lib/transfer_list/transfer_list.c index 381786185..4d4a167ed 100644 --- a/lib/transfer_list/transfer_list.c +++ b/lib/transfer_list/transfer_list.c @@ -176,35 +176,32 @@ transfer_list_check_header(const struct transfer_list_header *tl) } if (tl->signature != TRANSFER_LIST_SIGNATURE) { - ERROR("Bad transfer list signature 0x%x\n", tl->signature); + VERBOSE("Bad transfer list signature 0x%x\n", tl->signature); return TL_OPS_NON; } if (!tl->max_size) { - ERROR("Bad transfer list max size 0x%x\n", - tl->max_size); + VERBOSE("Bad transfer list max size 0x%x\n", tl->max_size); return TL_OPS_NON; } if (tl->size > tl->max_size) { - ERROR("Bad transfer list size 0x%x\n", tl->size); + VERBOSE("Bad transfer list size 0x%x\n", tl->size); return TL_OPS_NON; } if (tl->hdr_size != sizeof(struct transfer_list_header)) { - ERROR("Bad transfer list header size 0x%x\n", - tl->hdr_size); + VERBOSE("Bad transfer list header size 0x%x\n", tl->hdr_size); return TL_OPS_NON; } if (!transfer_list_verify_checksum(tl)) { - ERROR("Bad transfer list checksum 0x%x\n", - tl->checksum); + VERBOSE("Bad transfer list checksum 0x%x\n", tl->checksum); return TL_OPS_NON; } if (tl->version == 0) { - ERROR("Transfer list version is invalid\n"); + VERBOSE("Transfer list version is invalid\n"); return TL_OPS_NON; } else if (tl->version == TRANSFER_LIST_VERSION) { INFO("Transfer list version is valid for all operations\n"); From 2948d1f81904f02034a0d12faf9b8c7f34b05795 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Mon, 23 Dec 2024 16:18:58 +0000 Subject: [PATCH 2/8] fix(arm): reinit secure and non-secure tls Initializing the transfer list using `transfer_list_ensure` allows reuse of an already initialized transfer list. While this is beneficial when receiving a transfer list and ensuring one exists, it causes issues during a system RESET if the old content of SRAM is not cleared. To prevent this, at least one step in the reset path must zero intialise the transfer list memory. Unless a previous stage explicitly provides a transfer list via boot arguments, a fresh transfer list should be created. This change ensures that BL1 and BL31 properly reinitialize the transfer lists, preserving correctness for secure and non-secure handoffs in TF-A. Change-Id: I3bfaa9e76df932a637031d645e4a22d857a094a5 Signed-off-by: Harrison Mutai --- plat/arm/common/arm_bl1_setup.c | 2 +- plat/arm/common/arm_bl31_setup.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plat/arm/common/arm_bl1_setup.c b/plat/arm/common/arm_bl1_setup.c index b8e502760..75d6a5360 100644 --- a/plat/arm/common/arm_bl1_setup.c +++ b/plat/arm/common/arm_bl1_setup.c @@ -90,7 +90,7 @@ void arm_bl1_early_platform_setup(void) bl1_tzram_layout.total_size = ARM_BL_RAM_SIZE; #if TRANSFER_LIST - secure_tl = transfer_list_ensure((void *)PLAT_ARM_EL3_FW_HANDOFF_BASE, + secure_tl = transfer_list_init((void *)PLAT_ARM_EL3_FW_HANDOFF_BASE, PLAT_ARM_FW_HANDOFF_SIZE); assert(secure_tl != NULL); #endif diff --git a/plat/arm/common/arm_bl31_setup.c b/plat/arm/common/arm_bl31_setup.c index 0503acf10..b67df36b0 100644 --- a/plat/arm/common/arm_bl31_setup.c +++ b/plat/arm/common/arm_bl31_setup.c @@ -382,8 +382,8 @@ void arm_bl31_platform_setup(void) struct transfer_list_entry *te __unused; #if TRANSFER_LIST && !RESET_TO_BL31 - ns_tl = transfer_list_ensure((void *)FW_NS_HANDOFF_BASE, - PLAT_ARM_FW_HANDOFF_SIZE); + ns_tl = transfer_list_init((void *)FW_NS_HANDOFF_BASE, + PLAT_ARM_FW_HANDOFF_SIZE); if (ns_tl == NULL) { ERROR("Non-secure transfer list initialisation failed!\n"); panic(); From af61b50c1077b6d936c8ed741c1d0b8e43eb2b19 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Thu, 12 Dec 2024 18:33:54 +0000 Subject: [PATCH 3/8] fix(aarch32): avoid using r12 to store boot params The current implementation uses the `r12` register as temporary storage for r4. However, `r12` is a call-clobbered register, meaning its contents are not preserved across function calls. This becomes problematic when we later call the `zeromem` function, as any information stored in `r12` will be lost. To address this issue, we should avoid using `r12` to store boot parameters. Change-Id: If94b7fc3a01bc617ceadaaa704d5aa5e5accfd3f Signed-off-by: Harrison Mutai --- bl2/aarch32/bl2_entrypoint.S | 18 +++++++++--------- changelog.yaml | 3 +++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/bl2/aarch32/bl2_entrypoint.S b/bl2/aarch32/bl2_entrypoint.S index 678d9c2f0..91fc682e6 100644 --- a/bl2/aarch32/bl2_entrypoint.S +++ b/bl2/aarch32/bl2_entrypoint.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2022, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2016-2024, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -29,10 +29,10 @@ func bl2_entrypoint * use. * --------------------------------------------- */ - mov r9, r0 - mov r10, r1 - mov r11, r2 - mov r12, r3 + mov r8, r0 + mov r9, r1 + mov r10, r2 + mov r11, r3 /* --------------------------------------------- * Set the exception vector to something sane. @@ -114,10 +114,10 @@ func bl2_entrypoint * Perform BL2 setup * --------------------------------------------- */ - mov r0, r9 - mov r1, r10 - mov r2, r11 - mov r3, r12 + mov r0, r8 + mov r1, r9 + mov r2, r10 + mov r3, r11 bl bl2_setup diff --git a/changelog.yaml b/changelog.yaml index 422b9dafe..36ba4ea6f 100644 --- a/changelog.yaml +++ b/changelog.yaml @@ -1273,6 +1273,9 @@ subsections: - title: AArch64 scope: aarch64 + - title: AArch32 + scope: aarch32 + - title: Debug scope: debug From 7ffc1d6cf3c3981d74a3ac830f8a57f953b4ff03 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Mon, 16 Dec 2024 12:52:29 +0000 Subject: [PATCH 4/8] feat(handoff): add 32-bit variant of ep info Add the 32-bit version of the entry_point_info structure used to pass the boot arguments for future executables, added to the spec under the PR: https://github.com/FirmwareHandoff/firmware_handoff/pull/54. Change-Id: Id98e0f98db6ffd4790193e201f24e62101450e20 Signed-off-by: Harrison Mutai --- include/lib/transfer_list.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/lib/transfer_list.h b/include/lib/transfer_list.h index c403031a0..c864526f1 100644 --- a/include/lib/transfer_list.h +++ b/include/lib/transfer_list.h @@ -62,6 +62,7 @@ enum transfer_list_tag_id { TL_TAG_EXEC_EP_INFO64 = 0x102, TL_TAG_SRAM_LAYOUT64 = 0x104, TL_TAG_MBEDTLS_HEAP_INFO = 0x105, + TL_TAG_EXEC_EP_INFO32 = 0x106, }; enum transfer_list_ops { From 8001247ce267a583ae6a24a37a77f17427bd5204 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Mon, 16 Dec 2024 12:55:15 +0000 Subject: [PATCH 5/8] feat(handoff): add 32-bit variant of SRAM layout Introduce the 32-bit variant of the SRAM layout used by BL1 to communicate available free SRAM to BL2. This layout was added to the specification in: https://github.com/FirmwareHandoff/firmware_handoff/pull/54. Change-Id: I559fb8a00725eaedf01856af42d73029802aa095 Signed-off-by: Harrison Mutai --- include/lib/transfer_list.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/lib/transfer_list.h b/include/lib/transfer_list.h index c864526f1..bdc63490b 100644 --- a/include/lib/transfer_list.h +++ b/include/lib/transfer_list.h @@ -63,6 +63,7 @@ enum transfer_list_tag_id { TL_TAG_SRAM_LAYOUT64 = 0x104, TL_TAG_MBEDTLS_HEAP_INFO = 0x105, TL_TAG_EXEC_EP_INFO32 = 0x106, + TL_TAG_SRAM_LAYOUT32 = 0x107, }; enum transfer_list_ops { From 79e7aae82dd173d1ccc63e5d553222f1d58f12f5 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Mon, 16 Dec 2024 13:03:28 +0000 Subject: [PATCH 6/8] feat(handoff): add lib to sp-min sources Add firmware handoff to BL32 sources to provide support for the framework in SP-MIN. Change-Id: Ida8713fef8ba8fa72146004e41bf40f1a6b6f5ca Signed-off-by: Harrison Mutai --- bl32/sp_min/sp_min.mk | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bl32/sp_min/sp_min.mk b/bl32/sp_min/sp_min.mk index b1f4343f4..44d1e66b4 100644 --- a/bl32/sp_min/sp_min.mk +++ b/bl32/sp_min/sp_min.mk @@ -1,5 +1,5 @@ # -# Copyright (c) 2016-2024, Arm Limited and Contributors. All rights reserved. +# Copyright (c) 2016-2025, Arm Limited and Contributors. All rights reserved. # # SPDX-License-Identifier: BSD-3-Clause # @@ -83,3 +83,7 @@ $(eval $(call assert_boolean,RESET_TO_SP_MIN)) SP_MIN_WITH_SECURE_FIQ ?= 0 $(eval $(call add_define,SP_MIN_WITH_SECURE_FIQ)) $(eval $(call assert_boolean,SP_MIN_WITH_SECURE_FIQ)) + +ifeq (${TRANSFER_LIST},1) +BL32_SOURCES += $(TRANSFER_LIST_SOURCES) +endif From 3fabca724a724266f41a210d377d79072b36e140 Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Tue, 18 Feb 2025 14:26:08 +0000 Subject: [PATCH 7/8] feat(bl32): enable r3 usage for boot args `r3` is used to pass the base address of the transfer list. Make sure we update the context structure with this register value so it is populated with this information prior to executing the next stage. Change-Id: Ie1eedbd2eb68b592df30779625691e8975d987bf Signed-off-by: Harrison Mutai --- bl32/sp_min/sp_min_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bl32/sp_min/sp_min_main.c b/bl32/sp_min/sp_min_main.c index a85b35515..9add23920 100644 --- a/bl32/sp_min/sp_min_main.c +++ b/bl32/sp_min/sp_min_main.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2016-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -113,6 +113,7 @@ static void copy_cpu_ctx_to_smc_stx(const regs_t *cpu_reg_ctx, next_smc_ctx->r0 = read_ctx_reg(cpu_reg_ctx, CTX_GPREG_R0); next_smc_ctx->r1 = read_ctx_reg(cpu_reg_ctx, CTX_GPREG_R1); next_smc_ctx->r2 = read_ctx_reg(cpu_reg_ctx, CTX_GPREG_R2); + next_smc_ctx->r3 = read_ctx_reg(cpu_reg_ctx, CTX_GPREG_R3); next_smc_ctx->lr_mon = read_ctx_reg(cpu_reg_ctx, CTX_LR); next_smc_ctx->spsr_mon = read_ctx_reg(cpu_reg_ctx, CTX_SPSR); next_smc_ctx->scr = read_ctx_reg(cpu_reg_ctx, CTX_SCR); From 8921349894e5b1b844f3e3bb685cb86536b4b12b Mon Sep 17 00:00:00 2001 From: Harrison Mutai Date: Thu, 13 Mar 2025 18:20:14 +0000 Subject: [PATCH 8/8] refactor(arm): simplify early platform setup functions Refactor `arm_sp_min_early_platform_setup` to accept generic `u_register_r` values to support receiving firmware handoff boot arguments in common code. This has the added benefit of simplifying the interface into common early platform setup. Change-Id: Idfc3d41f94f2bf3a3a0c7ca39f6b9b0013836e3a Signed-off-by: Harrison Mutai --- include/plat/arm/common/plat_arm.h | 4 ++-- plat/arm/board/a5ds/sp_min/a5ds_sp_min_setup.c | 4 ++-- .../corstone700/sp_min/corstone700_sp_min_setup.c | 4 ++-- plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c | 4 ++-- plat/arm/board/fvp_ve/sp_min/fvp_ve_sp_min_setup.c | 4 ++-- plat/arm/common/sp_min/arm_sp_min_setup.c | 10 +++++----- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/plat/arm/common/plat_arm.h b/include/plat/arm/common/plat_arm.h index 1d7a59df0..2e72de2a2 100644 --- a/include/plat/arm/common/plat_arm.h +++ b/include/plat/arm/common/plat_arm.h @@ -298,8 +298,8 @@ void arm_transfer_list_get_heap_info(void **heap_addr, size_t *heap_size); void arm_tsp_early_platform_setup(void); /* SP_MIN utility functions */ -void arm_sp_min_early_platform_setup(void *from_bl2, uintptr_t tos_fw_config, - uintptr_t hw_config, void *plat_params_from_bl2); +void arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, + u_register_t arg2, u_register_t arg3); void arm_sp_min_plat_runtime_setup(void); void arm_sp_min_plat_arch_setup(void); diff --git a/plat/arm/board/a5ds/sp_min/a5ds_sp_min_setup.c b/plat/arm/board/a5ds/sp_min/a5ds_sp_min_setup.c index a951dc7b4..1b8699a0c 100644 --- a/plat/arm/board/a5ds/sp_min/a5ds_sp_min_setup.c +++ b/plat/arm/board/a5ds/sp_min/a5ds_sp_min_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, ARM Limited. All rights reserved. + * Copyright (c) 2019-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -11,7 +11,7 @@ void plat_arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { - arm_sp_min_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3); + arm_sp_min_early_platform_setup(arg0, arg1, arg2, arg3); /* enable snoop control unit */ enable_snoop_ctrl_unit(A5DS_SCU_BASE); diff --git a/plat/arm/board/corstone700/sp_min/corstone700_sp_min_setup.c b/plat/arm/board/corstone700/sp_min/corstone700_sp_min_setup.c index 2fc0e0dec..221e13298 100644 --- a/plat/arm/board/corstone700/sp_min/corstone700_sp_min_setup.c +++ b/plat/arm/board/corstone700/sp_min/corstone700_sp_min_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2019-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -9,5 +9,5 @@ void plat_arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { - arm_sp_min_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3); + arm_sp_min_early_platform_setup(arg0, arg1, arg2, arg3); } diff --git a/plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c b/plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c index 705ec384c..76ca5b5b7 100644 --- a/plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c +++ b/plat/arm/board/fvp/sp_min/fvp_sp_min_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2023, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2016-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -34,7 +34,7 @@ void plat_arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, } #endif /* !RESET_TO_SP_MIN && !RESET_TO_BL2 */ - arm_sp_min_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3); + arm_sp_min_early_platform_setup(arg0, arg1, arg2, arg3); /* Initialize the platform config for future decision making */ fvp_config_setup(); diff --git a/plat/arm/board/fvp_ve/sp_min/fvp_ve_sp_min_setup.c b/plat/arm/board/fvp_ve/sp_min/fvp_ve_sp_min_setup.c index e6a1bbec2..eba122f77 100644 --- a/plat/arm/board/fvp_ve/sp_min/fvp_ve_sp_min_setup.c +++ b/plat/arm/board/fvp_ve/sp_min/fvp_ve_sp_min_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Arm Limited. All rights reserved. + * Copyright (c) 2019-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -11,5 +11,5 @@ void plat_arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { - arm_sp_min_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3); + arm_sp_min_early_platform_setup(arg0, arg1, arg2, arg3); } diff --git a/plat/arm/common/sp_min/arm_sp_min_setup.c b/plat/arm/common/sp_min/arm_sp_min_setup.c index 4cd514bf1..78fc88e48 100644 --- a/plat/arm/common/sp_min/arm_sp_min_setup.c +++ b/plat/arm/common/sp_min/arm_sp_min_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2024, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2016-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -61,8 +61,8 @@ entry_point_info_t *sp_min_plat_get_bl33_ep_info(void) /******************************************************************************* * Utility function to perform early platform setup. ******************************************************************************/ -void arm_sp_min_early_platform_setup(void *from_bl2, uintptr_t tos_fw_config, - uintptr_t hw_config, void *plat_params_from_bl2) +void arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, + u_register_t arg2, u_register_t arg3) { /* Initialize the console to provide early debug support */ arm_console_boot_init(); @@ -99,7 +99,7 @@ void arm_sp_min_early_platform_setup(void *from_bl2, uintptr_t tos_fw_config, /* * Check params passed from BL2 should not be NULL, */ - bl_params_t *params_from_bl2 = (bl_params_t *)from_bl2; + bl_params_t *params_from_bl2 = (bl_params_t *)arg0; assert(params_from_bl2 != NULL); assert(params_from_bl2->h.type == PARAM_BL_PARAMS); assert(params_from_bl2->h.version >= VERSION_2); @@ -132,7 +132,7 @@ void arm_sp_min_early_platform_setup(void *from_bl2, uintptr_t tos_fw_config, void plat_arm_sp_min_early_platform_setup(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { - arm_sp_min_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3); + arm_sp_min_early_platform_setup(arg0, arg1, arg2, arg3); /* * Initialize Interconnect for this cluster during cold boot.