From 7475815f4b3697f6c61868e4ae6680baee8b93e2 Mon Sep 17 00:00:00 2001 From: "levi.yun" Date: Mon, 13 May 2024 10:27:17 +0100 Subject: [PATCH] feat(handoff): fix register convention r1/x1 value on transfer list According to recently firmware handsoff spec [1]'s "Register usage at handoff boundary", Transfer List's signature value was changed from 0x40_b10b (3 bytes) to 4a0f_b10b (4 bytes). As updating of TL's signature, register value of x1/r1 should be: In aarch32's r1 value should be R1[23:0]: set to the TL signature (4a0f_b10b -> masked range value: 0f_b10b) R1[31:24]: version of the register convention == 1 and In aarch64's x1 value should be X1[31:0]: set to the TL signature (4a0f_b10b) X1[39:32]: version of the register convention == 1 X1[63:40]: MBZ (See the [2] and [3]). Therefore, it requires to separate mask and shift value for register convention version field when sets each r1/x1. This patch fix two problems: 1. breaking X1 value with updated specification in aarch64 - change of length of signature field. 2. previous error value set in R1 in arm32. - length of signature should be 24, but it uses 32bit signature. This change is breaking change. It requires some patch for other softwares (u-boot[4], optee[5]). Link: https://github.com/FirmwareHandoff/firmware_handoff [1] Link: https://github.com/FirmwareHandoff/firmware_handoff/issues/32 [2] Link: https://github.com/FirmwareHandoff/firmware_handoff/commit/5aa7aa1d3a1db75213e458d392b751f0707de027 [3] Link: https://lists.denx.de/pipermail/u-boot/2024-July/558628.html [4] Link: https://github.com/OP-TEE/optee_os/pull/6933 [5] Signed-off-by: Levi Yun Change-Id: Ie417e054a7a4c192024a2679419e99efeded1705 --- include/lib/transfer_list.h | 17 ++++++++++++++++- lib/transfer_list/transfer_list.c | 21 +++++++++++---------- plat/qemu/common/qemu_bl2_setup.c | 16 ++++++++++++---- services/spd/opteed/opteed_main.c | 7 ++++--- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/include/lib/transfer_list.h b/include/lib/transfer_list.h index 5bea2705c..1b5ec2d63 100644 --- a/include/lib/transfer_list.h +++ b/include/lib/transfer_list.h @@ -29,7 +29,22 @@ * Version of the register convention used. * Set to 1 for both AArch64 and AArch32 according to fw handoff spec v0.9 */ -#define REGISTER_CONVENTION_VERSION_MASK (1 << 24) +#define REGISTER_CONVENTION_VERSION_SHIFT_64 UL(32) +#define REGISTER_CONVENTION_VERSION_SHIFT_32 UL(24) +#define REGISTER_CONVENTION_VERSION_MASK UL(0xff) +#define REGISTER_CONVENTION_VERSION UL(1) + +#define TRANSFER_LIST_HANDOFF_X1_VALUE(__version) \ + ((TRANSFER_LIST_SIGNATURE & \ + ((1UL << REGISTER_CONVENTION_VERSION_SHIFT_64) - 1)) | \ + (((__version) & REGISTER_CONVENTION_VERSION_MASK) << \ + REGISTER_CONVENTION_VERSION_SHIFT_64)) + +#define TRANSFER_LIST_HANDOFF_R1_VALUE(__version) \ + ((TRANSFER_LIST_SIGNATURE & \ + ((1UL << REGISTER_CONVENTION_VERSION_SHIFT_32) - 1)) | \ + (((__version) & REGISTER_CONVENTION_VERSION_MASK) << \ + REGISTER_CONVENTION_VERSION_SHIFT_32)) #ifndef __ASSEMBLER__ diff --git a/lib/transfer_list/transfer_list.c b/lib/transfer_list/transfer_list.c index b7fedfa69..8d82d259f 100644 --- a/lib/transfer_list/transfer_list.c +++ b/lib/transfer_list/transfer_list.c @@ -63,20 +63,21 @@ transfer_list_set_handoff_args(struct transfer_list_header *tl, te = transfer_list_find(tl, TL_TAG_FDT); dt = transfer_list_entry_data(te); - ep_info->args.arg1 = TRANSFER_LIST_SIGNATURE | - REGISTER_CONVENTION_VERSION_MASK; - ep_info->args.arg3 = (uintptr_t)tl; - - if (GET_RW(ep_info->spsr) == MODE_RW_32) { - /* aarch32 */ - ep_info->args.arg0 = 0; - ep_info->args.arg2 = (uintptr_t)dt; - } else { - /* aarch64 */ +#ifdef __aarch64__ + if (GET_RW(ep_info->spsr) == MODE_RW_64) { ep_info->args.arg0 = (uintptr_t)dt; + ep_info->args.arg1 = TRANSFER_LIST_HANDOFF_X1_VALUE(REGISTER_CONVENTION_VERSION); ep_info->args.arg2 = 0; + } else +#endif + { + ep_info->args.arg0 = 0; + ep_info->args.arg1 = TRANSFER_LIST_HANDOFF_R1_VALUE(REGISTER_CONVENTION_VERSION); + ep_info->args.arg2 = (uintptr_t)dt; } + ep_info->args.arg3 = (uintptr_t)tl; + return ep_info; } diff --git a/plat/qemu/common/qemu_bl2_setup.c b/plat/qemu/common/qemu_bl2_setup.c index d752b6cca..c96e4b956 100644 --- a/plat/qemu/common/qemu_bl2_setup.c +++ b/plat/qemu/common/qemu_bl2_setup.c @@ -357,12 +357,20 @@ static int qemu_bl2_handle_post_image_load(unsigned int image_id) case BL31_IMAGE_ID: /* * arg0 is a bl_params_t reserved for bl31_early_platform_setup2 - * we just need arg1 and arg3 for BL31 to update th TL from S + * we just need arg1 and arg3 for BL31 to update the TL from S * to NS memory before it exits */ - bl_mem_params->ep_info.args.arg1 = - TRANSFER_LIST_SIGNATURE | - REGISTER_CONVENTION_VERSION_MASK; +#ifdef __aarch64__ + if (GET_RW(bl_mem_params->ep_info.spsr) == MODE_RW_64) { + bl_mem_params->ep_info.args.arg1 = + TRANSFER_LIST_HANDOFF_X1_VALUE(REGISTER_CONVENTION_VERSION); + } else +#endif + { + bl_mem_params->ep_info.args.arg1 = + TRANSFER_LIST_HANDOFF_R1_VALUE(REGISTER_CONVENTION_VERSION); + } + bl_mem_params->ep_info.args.arg3 = (uintptr_t)bl2_tl; break; #endif diff --git a/services/spd/opteed/opteed_main.c b/services/spd/opteed/opteed_main.c index d6c004057..9e8384844 100644 --- a/services/spd/opteed/opteed_main.c +++ b/services/spd/opteed/opteed_main.c @@ -552,13 +552,14 @@ static int32_t opteed_handle_smc_load(uint64_t data_size, uint32_t data_pa) if (opteed_rw == OPTEE_AARCH64) { arg0 = (uint64_t)dt; + arg1 = TRANSFER_LIST_HANDOFF_X1_VALUE(REGISTER_CONVENTION_VERSION); arg2 = 0; } else { - arg2 = (uint64_t)dt; arg0 = 0; + arg1 = TRANSFER_LIST_HANDOFF_R1_VALUE(REGISTER_CONVENTION_VERSION); + arg2 = (uint64_t)dt; } - arg1 = TRANSFER_LIST_SIGNATURE | - REGISTER_CONVENTION_VERSION_MASK; + arg3 = (uint64_t)bl31_tl; } else { /* Default handoff arguments */