From e106a78ef00df4c70a1594a89520af07b939cd92 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 12:25:09 +0530 Subject: [PATCH 01/12] feat(fwu): update the URL links for the FWU specification Update the links for accessing the FWU Multi Bank update specification to point to the latest revision of the specification. Change-Id: I25f35556a94ca81ca0a7463aebfcbc2d84595e8f Signed-off-by: Sughosh Ganu --- docs/components/firmware-update.rst | 2 +- docs/getting_started/build-options.rst | 2 +- docs/index.rst | 2 +- include/drivers/fwu/fwu_metadata.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/components/firmware-update.rst b/docs/components/firmware-update.rst index 1ba1e1c6e..eda78525a 100644 --- a/docs/components/firmware-update.rst +++ b/docs/components/firmware-update.rst @@ -494,4 +494,4 @@ This is only allowed if the image is not being executed. .. _Universally Unique Identifier: https://tools.ietf.org/rfc/rfc4122.txt .. |Flow Diagram| image:: ../resources/diagrams/fwu_flow.png .. |FWU state machine| image:: ../resources/diagrams/fwu_states.png -.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/a/ +.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/latest/ diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst index 37545ce34..def9fda77 100644 --- a/docs/getting_started/build-options.rst +++ b/docs/getting_started/build-options.rst @@ -1345,7 +1345,7 @@ Firmware update options *Copyright (c) 2019-2024, Arm Limited. All rights reserved.* .. _DEN0115: https://developer.arm.com/docs/den0115/latest -.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/a/ +.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/latest/ .. _PSA DRTM specification: https://developer.arm.com/documentation/den0113/a .. _GCC: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html .. _Clang: https://clang.llvm.org/docs/DiagnosticsReference.html diff --git a/docs/index.rst b/docs/index.rst index cdb237aea..c05c0a506 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -95,4 +95,4 @@ have previously been raised against the software. .. _System Control and Management Interface (SCMI): http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/DEN0056A_System_Control_and_Management_Interface.pdf .. _Software Delegated Exception Interface (SDEI): http://infocenter.arm.com/help/topic/com.arm.doc.den0054a/ARM_DEN0054A_Software_Delegated_Exception_Interface.pdf .. _SMC Calling Convention: https://developer.arm.com/docs/den0028/latest -.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/a/ +.. _PSA FW update specification: https://developer.arm.com/documentation/den0118/latest/ diff --git a/include/drivers/fwu/fwu_metadata.h b/include/drivers/fwu/fwu_metadata.h index 2e88de5ec..7ff11763c 100644 --- a/include/drivers/fwu/fwu_metadata.h +++ b/include/drivers/fwu/fwu_metadata.h @@ -4,7 +4,7 @@ * SPDX-License-Identifier: BSD-3-Clause * * FWU metadata information as per the specification section 4.1: - * https://developer.arm.com/documentation/den0118/a/ + * https://developer.arm.com/documentation/den0118/latest/ * */ From 7ae16196cc73a580f298734bb98f2ccb210e3ba9 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 12:42:40 +0530 Subject: [PATCH 02/12] feat(fwu): document the config flag for including image info in the FWU metadata The version 2 of the FWU metadata structure is designed such that the information on the updatable images can be omitted from the metadata structure. Add a config flag, PSA_FWU_METADATA_FW_STORE_DESC, which is used to select whether the metadata structure has this information included or not. It's value is set to 1 by default. Change-Id: Id6c99455db768edd59b0a316051432a900d30076 Signed-off-by: Sughosh Ganu --- docs/getting_started/build-options.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst index def9fda77..a8b40ad8e 100644 --- a/docs/getting_started/build-options.rst +++ b/docs/getting_started/build-options.rst @@ -1340,6 +1340,15 @@ Firmware update options This flag is used in defining the firmware update metadata structure. This flag is by default set to '1'. +- ``PSA_FWU_METADATA_FW_STORE_DESC``: To be enabled when the FWU + metadata contains image description. The default value is 1. + + The version 2 of the FWU metadata allows for an opaque metadata + structure where a platform can choose to not include the firmware + store description in the metadata structure. This option indicates + if the firmware store description, which provides information on + the updatable images is part of the structure. + -------------- *Copyright (c) 2019-2024, Arm Limited. All rights reserved.* From a89d58bb204c00db260225859bce0b55aa5e2385 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 12:47:13 +0530 Subject: [PATCH 03/12] feat(fwu): migrate FWU metadata structure to version 2 The latest version of the FWU specification [1] has changes to the metadata structure. This is version 2 of the structure. Primary changes include - bank_state field in the top level structure - Total metadata size in the top level structure - Image description structures now optional - Number of banks and images per bank values part of the structure Make changes to the structure to align with version 2 of the structure defined in the specification. These changes also remove support for version 1 of the metadata structure. [1] - https://developer.arm.com/documentation/den0118/latest/ Change-Id: I84b4e742e463cae92375dde8b4603b4a581d62d8 Signed-off-by: Sughosh Ganu --- include/drivers/fwu/fwu.h | 4 +++ include/drivers/fwu/fwu_metadata.h | 44 +++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/include/drivers/fwu/fwu.h b/include/drivers/fwu/fwu.h index 9f18e221c..1414fec0f 100644 --- a/include/drivers/fwu/fwu.h +++ b/include/drivers/fwu/fwu.h @@ -9,6 +9,10 @@ #include +#define FWU_BANK_STATE_ACCEPTED 0xFCU +#define FWU_BANK_STATE_VALID 0xFEU +#define FWU_BANK_STATE_INVALID 0xFFU + void fwu_init(void); bool fwu_is_trial_run_state(void); const struct fwu_metadata *fwu_get_metadata(void); diff --git a/include/drivers/fwu/fwu_metadata.h b/include/drivers/fwu/fwu_metadata.h index 7ff11763c..cc2c863d5 100644 --- a/include/drivers/fwu/fwu_metadata.h +++ b/include/drivers/fwu/fwu_metadata.h @@ -14,6 +14,8 @@ #include #include +#define NR_OF_MAX_FW_BANKS 4 + /* Properties of image in a bank */ struct fwu_image_properties { @@ -45,6 +47,29 @@ struct fwu_image_entry { } __packed; +/* Firmware Image descriptor */ +struct fwu_fw_store_descriptor { + + /* Number of Banks */ + uint8_t num_banks; + + /* Reserved */ + uint8_t reserved; + + /* Number of images per bank */ + uint16_t num_images; + + /* Size of image_entry(all banks) in bytes */ + uint16_t img_entry_size; + + /* Size of image bank info structure in bytes */ + uint16_t bank_info_entry_size; + + /* Array of fwu_image_entry structs */ + struct fwu_image_entry img_entry[NR_OF_IMAGES_IN_FW_BANK]; + +} __packed; + /* * FWU metadata filled by the updater and consumed by TF-A for * various purposes as below: @@ -66,8 +91,25 @@ struct fwu_metadata { /* Previous bank index with which device booted successfully */ uint32_t previous_active_index; + /* Size of the entire metadata in bytes */ + uint32_t metadata_size; + + /* Offset of the image descriptor structure */ + uint16_t desc_offset; + + /* Reserved */ + uint16_t reserved1; + + /* Bank state */ + uint8_t bank_state[NR_OF_MAX_FW_BANKS]; + + /* Reserved */ + uint32_t reserved2; + +#if PSA_FWU_METADATA_FW_STORE_DESC /* Image entry information */ - struct fwu_image_entry img_entry[NR_OF_IMAGES_IN_FW_BANK]; + struct fwu_fw_store_descriptor fw_desc; +#endif } __packed; From 11d05a77295885f27530cf07029ebc2b36f49918 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 12:51:20 +0530 Subject: [PATCH 04/12] feat(fwu): add a config flag for including image info in the FWU metadata The version 2 of the FWU metadata structure is designed such that the information on the updatable images can be omitted from the metadata structure. Add a configuration flag, PSA_FWU_METADATA_FW_STORE_DESC, which is used to select whether the metadata structure has this information included or not. It's value is set to 1 by default. Change-Id: I4463a20c94d2c745ddb0b2cc8932c12d418fbd42 Signed-off-by: Sughosh Ganu --- Makefile | 2 ++ make_helpers/defaults.mk | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 6a1ea99a6..53d391e89 100644 --- a/Makefile +++ b/Makefile @@ -1185,6 +1185,7 @@ $(eval $(call assert_booleans,\ COT_DESC_IN_DTB \ USE_SP804_TIMER \ PSA_FWU_SUPPORT \ + PSA_FWU_METADATA_FW_STORE_DESC \ ENABLE_MPMM \ ENABLE_MPMM_FCONF \ FEATURE_DETECTION \ @@ -1360,6 +1361,7 @@ $(eval $(call add_defines,\ NR_OF_FW_BANKS \ NR_OF_IMAGES_IN_FW_BANK \ PSA_FWU_SUPPORT \ + PSA_FWU_METADATA_FW_STORE_DESC \ ENABLE_BRBE_FOR_NS \ ENABLE_TRBE_FOR_NS \ ENABLE_SYS_REG_TRACE_FOR_NS \ diff --git a/make_helpers/defaults.mk b/make_helpers/defaults.mk index 180207791..7fe8bf81d 100644 --- a/make_helpers/defaults.mk +++ b/make_helpers/defaults.mk @@ -351,6 +351,14 @@ NR_OF_IMAGES_IN_FW_BANK := 1 # Disable Firmware update support by default PSA_FWU_SUPPORT := 0 +# Enable image description in FWU metadata by default when PSA_FWU_SUPPORT +# is enabled. +ifeq ($(PSA_FWU_SUPPORT),1) +PSA_FWU_METADATA_FW_STORE_DESC := 1 +else +PSA_FWU_METADATA_FW_STORE_DESC := 0 +endif + # Dynamic Root of Trust for Measurement support DRTM_SUPPORT := 0 From 588b01b5e4726cd4a6d235e9f566a546ef17f631 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 16:56:27 +0530 Subject: [PATCH 05/12] feat(st): get the state of the active bank directly With version 2 of the FWU metadata structure, the state that a bank is in can be obtained from the bank_state field in the top level structure. Read the state of the active bank by referencing this field directly, instead of making an API call. Change-Id: Ib22c56acbe172923b1323c544801ded81f1598ec Signed-off-by: Sughosh Ganu --- plat/st/common/bl2_io_storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plat/st/common/bl2_io_storage.c b/plat/st/common/bl2_io_storage.c index 86795d71a..96d24d174 100644 --- a/plat/st/common/bl2_io_storage.c +++ b/plat/st/common/bl2_io_storage.c @@ -628,7 +628,7 @@ uint32_t plat_fwu_get_boot_idx(void) if (boot_idx == INVALID_BOOT_IDX) { boot_idx = data->active_index; - if (fwu_is_trial_run_state()) { + if (data->bank_state[boot_idx] == FWU_BANK_STATE_VALID) { if (stm32_get_and_dec_fwu_trial_boot_cnt() == 0U) { WARN("Trial FWU fails %u times\n", FWU_MAX_TRIAL_REBOOT); From 56724d09c2c55ee2b8486b7c706f5fb9d980df88 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Thu, 1 Feb 2024 16:59:01 +0530 Subject: [PATCH 06/12] feat(fwu): modify the check for getting the FWU bank's state The version 2 of the FWU metadata structure has a field bank_state in the top level of the structure which can be used to check if a given bank is in the either of Trial State, Accepted State, or in an Invalid State. This is different from the binary states of Valid/Accepted States that the bank could be in, as defined in the earlier version of the specification. Replace the fwu_is_trial_run_state() API with fwu_get_active_bank_state() to get the state the current active bank is in. The value returned by this API is then used by the caller to take appropriate action. Change-Id: I764f486840a3713bfe5f8e03d0634bfe09b23590 Signed-off-by: Sughosh Ganu --- drivers/auth/auth_mod.c | 10 +++++++--- drivers/fwu/fwu.c | 29 ++++++++++++----------------- include/drivers/fwu/fwu.h | 2 +- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/auth/auth_mod.c b/drivers/auth/auth_mod.c index 608866c86..8c5ff9d12 100644 --- a/drivers/auth/auth_mod.c +++ b/drivers/auth/auth_mod.c @@ -328,7 +328,6 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param, unsigned int data_len, len, i; unsigned int plat_nv_ctr; int rc; - bool is_trial_run = false; /* Get the counter value from current image. The AM expects the IPM * to return the counter value as a DER encoded integer */ @@ -388,9 +387,14 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param, return 1; } else if (*cert_nv_ctr > plat_nv_ctr) { #if PSA_FWU_SUPPORT && IMAGE_BL2 - is_trial_run = fwu_is_trial_run_state(); + if (fwu_get_active_bank_state() == FWU_BANK_STATE_ACCEPTED) { + *need_nv_ctr_upgrade = true; + } else { + *need_nv_ctr_upgrade = false; + } +#else + *need_nv_ctr_upgrade = true; #endif /* PSA_FWU_SUPPORT && IMAGE_BL2 */ - *need_nv_ctr_upgrade = !is_trial_run; } return 0; diff --git a/drivers/fwu/fwu.c b/drivers/fwu/fwu.c index ff432be8c..7bb76933f 100644 --- a/drivers/fwu/fwu.c +++ b/drivers/fwu/fwu.c @@ -133,28 +133,23 @@ exit: } /******************************************************************************* - * The system runs in the trial run state if any of the images in the active - * firmware bank has not been accepted yet. + * The platform can be in one of Valid, Invalid or Accepted states. * - * Returns true if the system is running in the trial state. + * Invalid - One or more images in the bank are corrupted, or partially + * overwritten. The bank is not to be used for booting. + * + * Valid - All images of the bank are valid but at least one image has not + * been accepted. This implies that the platform is in Trial State. + * + * Accepted - All images of the bank are valid and accepted. + * + * Returns the state of the current active bank ******************************************************************************/ -bool fwu_is_trial_run_state(void) +uint32_t fwu_get_active_bank_state(void) { - bool trial_run = false; - assert(is_metadata_initialized); - for (unsigned int i = 0U; i < NR_OF_IMAGES_IN_FW_BANK; i++) { - struct fwu_image_entry *entry = &metadata.img_entry[i]; - struct fwu_image_properties *img_props = - &entry->img_props[metadata.active_index]; - if (img_props->accepted == 0) { - trial_run = true; - break; - } - } - - return trial_run; + return metadata.bank_state[metadata.active_index]; } const struct fwu_metadata *fwu_get_metadata(void) diff --git a/include/drivers/fwu/fwu.h b/include/drivers/fwu/fwu.h index 1414fec0f..489d4a1e7 100644 --- a/include/drivers/fwu/fwu.h +++ b/include/drivers/fwu/fwu.h @@ -14,7 +14,7 @@ #define FWU_BANK_STATE_INVALID 0xFFU void fwu_init(void); -bool fwu_is_trial_run_state(void); +uint32_t fwu_get_active_bank_state(void); const struct fwu_metadata *fwu_get_metadata(void); #endif /* FWU_H */ From d2566cfb896672ea07c31c37e7acd9ef77abc4fb Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Wed, 17 Jan 2024 16:38:01 +0530 Subject: [PATCH 07/12] feat(fwu): add some sanity checks for the FWU metadata Add some sanity checks on the values read from the FWU metadata structure. This ensures that values in the metadata structure are inline with certain config symbol values. Change-Id: Ic4415da9048ac3980f8f811ed7852beb90683f7d Signed-off-by: Sughosh Ganu --- drivers/fwu/fwu.c | 59 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/drivers/fwu/fwu.c b/drivers/fwu/fwu.c index 7bb76933f..5c3ccf878 100644 --- a/drivers/fwu/fwu.c +++ b/drivers/fwu/fwu.c @@ -24,6 +24,17 @@ CASSERT((offsetof(struct fwu_metadata, crc_32) == 0), crc_32_must_be_first_member_of_structure); +/* + * Ensure that the NR_OF_FW_BANKS selected by the platform is not + * zero and not greater than the maximum number of banks allowed + * by the specification. + */ +CASSERT((NR_OF_FW_BANKS > 0) && (NR_OF_FW_BANKS <= NR_OF_MAX_FW_BANKS), + assert_fwu_num_banks_invalid_value); + +#define FWU_METADATA_VERSION 2U +#define FWU_FW_STORE_DESC_OFFSET 0x20U + static struct fwu_metadata metadata; static bool is_metadata_initialized __unused; @@ -51,16 +62,54 @@ static int fwu_metadata_crc_check(void) /******************************************************************************* * Check the sanity of FWU metadata. * - * return -1 on error, otherwise 0 + * return -EINVAL on error, otherwise 0 ******************************************************************************/ static int fwu_metadata_sanity_check(void) { - /* ToDo: add more conditions for sanity check */ - if ((metadata.active_index >= NR_OF_FW_BANKS) || - (metadata.previous_active_index >= NR_OF_FW_BANKS)) { - return -1; + if (metadata.version != FWU_METADATA_VERSION) { + WARN("Incorrect FWU Metadata version of %u\n", + metadata.version); + return -EINVAL; } + if (metadata.active_index >= NR_OF_FW_BANKS) { + WARN("Active Index value(%u) greater than the configured value(%d)", + metadata.active_index, NR_OF_FW_BANKS); + return -EINVAL; + } + + if (metadata.previous_active_index >= NR_OF_FW_BANKS) { + WARN("Previous Active Index value(%u) greater than the configured value(%d)", + metadata.previous_active_index, NR_OF_FW_BANKS); + return -EINVAL; + } + +#if PSA_FWU_METADATA_FW_STORE_DESC + if (metadata.fw_desc.num_banks != NR_OF_FW_BANKS) { + WARN("Number of Banks(%u) in FWU Metadata different from the configured value(%d)", + metadata.fw_desc.num_banks, NR_OF_FW_BANKS); + return -EINVAL; + } + + if (metadata.fw_desc.num_images != NR_OF_IMAGES_IN_FW_BANK) { + WARN("Number of Images(%u) in FWU Metadata different from the configured value(%d)", + metadata.fw_desc.num_images, NR_OF_IMAGES_IN_FW_BANK); + return -EINVAL; + } + + if (metadata.desc_offset != FWU_FW_STORE_DESC_OFFSET) { + WARN("Descriptor Offset(0x%x) in the FWU Metadata not equal to 0x20\n", + metadata.desc_offset); + return -EINVAL; + } +#else + if (metadata.desc_offset != 0U) { + WARN("Descriptor offset has non zero value of 0x%x\n", + metadata.desc_offset); + return -EINVAL; + } +#endif + return 0; } From 26aab79560a2281c4207b01102495459c2bddefc Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Wed, 7 Feb 2024 20:13:01 +0530 Subject: [PATCH 08/12] feat(fwu): add a function to obtain an alternate FWU bank to boot Add a function fwu_get_alternate_boot_bank() to return a valid bank to boot from. This function can be called by a platform to get an alternate bank to try to boot the platform in the unlikely scenario of the active bank being in an invalid state, or if the number of times the platform boots in trial state exceeds a pre-set count. Change-Id: I4bcd88e68e334c452882255bf028e01b090369d1 Signed-off-by: Sughosh Ganu --- drivers/fwu/fwu.c | 57 +++++++++++++++++++++++++++++++++++++++ include/drivers/fwu/fwu.h | 3 +++ 2 files changed, 60 insertions(+) diff --git a/drivers/fwu/fwu.c b/drivers/fwu/fwu.c index 5c3ccf878..b6f06e0a7 100644 --- a/drivers/fwu/fwu.c +++ b/drivers/fwu/fwu.c @@ -181,6 +181,63 @@ exit: return result; } +/******************************************************************************* + * Check for an alternate bank for the platform to boot from. This function will + * mostly be called whenever the count of the number of times a platform boots + * in the Trial State exceeds a pre-set limit. + * The function first checks if the platform can boot from the previously active + * bank. If not, it tries to find another bank in the accepted state. + * And finally, if both the checks fail, as a last resort, it tries to find + * a valid bank. + * + * Returns the index of a bank to boot, else returns invalid index + * INVALID_BOOT_IDX. + ******************************************************************************/ +uint32_t fwu_get_alternate_boot_bank(void) +{ + uint32_t i; + + /* First check if the previously active bank can be used */ + if (metadata.bank_state[metadata.previous_active_index] == + FWU_BANK_STATE_ACCEPTED) { + return metadata.previous_active_index; + } + + /* Now check for any other bank in the accepted state */ + for (i = 0U; i < NR_OF_FW_BANKS; i++) { + if (i == metadata.active_index || + i == metadata.previous_active_index) { + continue; + } + + if (metadata.bank_state[i] == FWU_BANK_STATE_ACCEPTED) { + return i; + } + } + + /* + * No accepted bank found. Now try booting from a valid bank. + * Give priority to the previous active bank. + */ + if (metadata.bank_state[metadata.previous_active_index] == + FWU_BANK_STATE_VALID) { + return metadata.previous_active_index; + } + + for (i = 0U; i < NR_OF_FW_BANKS; i++) { + if (i == metadata.active_index || + i == metadata.previous_active_index) { + continue; + } + + if (metadata.bank_state[i] == FWU_BANK_STATE_VALID) { + return i; + } + } + + return INVALID_BOOT_IDX; +} + /******************************************************************************* * The platform can be in one of Valid, Invalid or Accepted states. * diff --git a/include/drivers/fwu/fwu.h b/include/drivers/fwu/fwu.h index 489d4a1e7..18e8a3163 100644 --- a/include/drivers/fwu/fwu.h +++ b/include/drivers/fwu/fwu.h @@ -13,8 +13,11 @@ #define FWU_BANK_STATE_VALID 0xFEU #define FWU_BANK_STATE_INVALID 0xFFU +#define INVALID_BOOT_IDX 0xFFFFFFFFU + void fwu_init(void); uint32_t fwu_get_active_bank_state(void); +uint32_t fwu_get_alternate_boot_bank(void); const struct fwu_metadata *fwu_get_metadata(void); #endif /* FWU_H */ From 6e99fee43efa256bdac3b38864206c94bd9ae3c8 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Tue, 20 Feb 2024 14:17:57 +0530 Subject: [PATCH 09/12] feat(st): add a function to clear the FWU trial state counter Add an API stm32_clear_fwu_trial_boot_cnt() function to clear the trial state counter. This is called in the corner case scenario when the active index is in an Invalid state, thus needing a reset of the trial state counter. Change-Id: I2980135da88d0d947c222655c7958b51eb572d69 Signed-off-by: Sughosh Ganu --- plat/st/common/include/stm32mp_common.h | 1 + plat/st/stm32mp1/stm32mp1_private.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/plat/st/common/include/stm32mp_common.h b/plat/st/common/include/stm32mp_common.h index 0ff60929f..a1ed1addd 100644 --- a/plat/st/common/include/stm32mp_common.h +++ b/plat/st/common/include/stm32mp_common.h @@ -142,6 +142,7 @@ void stm32_display_board_info(uint32_t board_id); void stm32mp1_fwu_set_boot_idx(void); uint32_t stm32_get_and_dec_fwu_trial_boot_cnt(void); void stm32_set_max_fwu_trial_boot_cnt(void); +void stm32_clear_fwu_trial_boot_cnt(void); #endif /* PSA_FWU_SUPPORT */ #endif /* STM32MP_COMMON_H */ diff --git a/plat/st/stm32mp1/stm32mp1_private.c b/plat/st/stm32mp1/stm32mp1_private.c index 0e6951316..f098eb37e 100644 --- a/plat/st/stm32mp1/stm32mp1_private.c +++ b/plat/st/stm32mp1/stm32mp1_private.c @@ -714,4 +714,13 @@ void stm32_set_max_fwu_trial_boot_cnt(void) TAMP_BOOT_FWU_INFO_CNT_MSK); clk_disable(RTCAPB); } + +void stm32_clear_fwu_trial_boot_cnt(void) +{ + uintptr_t bkpr_fwu_cnt = tamp_bkpr(TAMP_BOOT_FWU_INFO_REG_ID); + + clk_enable(RTCAPB); + mmio_clrbits_32(bkpr_fwu_cnt, TAMP_BOOT_FWU_INFO_CNT_MSK); + clk_disable(RTCAPB); +} #endif /* PSA_FWU_SUPPORT */ From 6166051426638087b5433eff1739d26478313dff Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Tue, 20 Feb 2024 14:20:41 +0530 Subject: [PATCH 10/12] feat(st): add logic to boot the platform from an alternate bank In a few scenarios, there is a need to boot the platform from an alernate bank which is not the active bank. Call the API fwu_get_alernate_boot_bank() to select an alternate bank to boot the platform from. Calling this API function might be required in a couple of cases. One, in the unlikely scenario of the active bank being in an invalid state, or if the number of times the platform boots in trial state exceeds a pre-set count. Also add a debug print that indicates the bank that the platform is booting from. Change-Id: I688406540e64d1719af8d5c121821f5bb6335c06 Signed-off-by: Sughosh Ganu --- plat/st/common/bl2_io_storage.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/plat/st/common/bl2_io_storage.c b/plat/st/common/bl2_io_storage.c index 96d24d174..5513a2fc0 100644 --- a/plat/st/common/bl2_io_storage.c +++ b/plat/st/common/bl2_io_storage.c @@ -613,8 +613,6 @@ int plat_get_image_source(unsigned int image_id, uintptr_t *dev_handle, * - we already boot FWU_MAX_TRIAL_REBOOT times in trial mode. * we select the previous_active_index. */ -#define INVALID_BOOT_IDX 0xFFFFFFFFU - uint32_t plat_fwu_get_boot_idx(void) { /* @@ -622,20 +620,26 @@ uint32_t plat_fwu_get_boot_idx(void) * even if this function is called several times. */ static uint32_t boot_idx = INVALID_BOOT_IDX; - const struct fwu_metadata *data; - - data = fwu_get_metadata(); if (boot_idx == INVALID_BOOT_IDX) { + const struct fwu_metadata *data = fwu_get_metadata(); + boot_idx = data->active_index; + if (data->bank_state[boot_idx] == FWU_BANK_STATE_VALID) { if (stm32_get_and_dec_fwu_trial_boot_cnt() == 0U) { WARN("Trial FWU fails %u times\n", FWU_MAX_TRIAL_REBOOT); - boot_idx = data->previous_active_index; + boot_idx = fwu_get_alternate_boot_bank(); } - } else { + } else if (data->bank_state[boot_idx] == + FWU_BANK_STATE_ACCEPTED) { stm32_set_max_fwu_trial_boot_cnt(); + } else { + ERROR("The active bank(%u) of the platform is in Invalid State.\n", + boot_idx); + boot_idx = fwu_get_alternate_boot_bank(); + stm32_clear_fwu_trial_boot_cnt(); } } @@ -667,6 +671,7 @@ void plat_fwu_set_images_source(const struct fwu_metadata *metadata) boot_idx = plat_fwu_get_boot_idx(); assert(boot_idx < NR_OF_FW_BANKS); + VERBOSE("Selecting to boot from bank %u\n", boot_idx); for (i = 0U; i < NR_OF_IMAGES_IN_FW_BANK; i++) { img_type_uuid = &metadata->img_entry[i].img_type_uuid; From 37e81a603dc06c2b4c9579c02e773d3e7562f475 Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Fri, 2 Feb 2024 15:32:10 +0530 Subject: [PATCH 11/12] style(partition): use GUID values for GPT partition fields The GPT partition uses GUID values for identification of partition types and partitions. Change the relevant functions to use GUID values instead of UUID's. Change-Id: I30df66a8a02fb502e04b0285f34131b65977988e Signed-off-by: Sughosh Ganu --- drivers/partition/partition.c | 14 ++++++++------ include/drivers/partition/partition.h | 6 ++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/partition/partition.c b/drivers/partition/partition.c index 555fe7fd0..42e157bea 100644 --- a/drivers/partition/partition.c +++ b/drivers/partition/partition.c @@ -452,14 +452,15 @@ const partition_entry_t *get_partition_entry(const char *name) } /* - * Try retrieving a partition table entry based on the GUID. + * Try retrieving a partition table entry based on the partition type GUID. */ -const partition_entry_t *get_partition_entry_by_type(const uuid_t *type_uuid) +const partition_entry_t *get_partition_entry_by_type( + const struct efi_guid *type_guid) { int i; for (i = 0; i < list.entry_count; i++) { - if (guidcmp(type_uuid, &list.list[i].type_guid) == 0) { + if (guidcmp(type_guid, &list.list[i].type_guid) == 0) { return &list.list[i]; } } @@ -468,14 +469,15 @@ const partition_entry_t *get_partition_entry_by_type(const uuid_t *type_uuid) } /* - * Try retrieving a partition table entry based on the UUID. + * Try retrieving a partition table entry based on the unique partition GUID. */ -const partition_entry_t *get_partition_entry_by_uuid(const uuid_t *part_uuid) +const partition_entry_t *get_partition_entry_by_guid( + const struct efi_guid *part_guid) { int i; for (i = 0; i < list.entry_count; i++) { - if (guidcmp(part_uuid, &list.list[i].part_guid) == 0) { + if (guidcmp(part_guid, &list.list[i].part_guid) == 0) { return &list.list[i]; } } diff --git a/include/drivers/partition/partition.h b/include/drivers/partition/partition.h index d567d4cbb..4183570aa 100644 --- a/include/drivers/partition/partition.h +++ b/include/drivers/partition/partition.h @@ -46,8 +46,10 @@ typedef struct partition_entry_list { int load_partition_table(unsigned int image_id); const partition_entry_t *get_partition_entry(const char *name); -const partition_entry_t *get_partition_entry_by_type(const uuid_t *type_guid); -const partition_entry_t *get_partition_entry_by_uuid(const uuid_t *part_uuid); +const partition_entry_t *get_partition_entry_by_type( + const struct efi_guid *type_guid); +const partition_entry_t *get_partition_entry_by_guid( + const struct efi_guid *part_guid); const partition_entry_list_t *get_partition_entry_list(void); void partition_init(unsigned int image_id); int gpt_partition_init(void); From 8d08a1df1e0effc42501e13c22435e546a275dae Mon Sep 17 00:00:00 2001 From: Sughosh Ganu Date: Fri, 2 Feb 2024 15:35:18 +0530 Subject: [PATCH 12/12] style(fwu): change the metadata fields to align with specification Change the names of some FWU metadata structure members to have them align with the wording used in the corresponding specification. Use the GUID type instead of UUID as the fields described in the specification are GUIDs. Make corresponding changes to the code that accesses these fields. No functional changes are introduced by the patch. Change-Id: Id3544ed1633811b0eeee2bf99477f9b7e6667044 Signed-off-by: Sughosh Ganu --- include/drivers/fwu/fwu_metadata.h | 18 +++++++++--------- plat/st/common/bl2_io_storage.c | 28 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/include/drivers/fwu/fwu_metadata.h b/include/drivers/fwu/fwu_metadata.h index cc2c863d5..b441300e4 100644 --- a/include/drivers/fwu/fwu_metadata.h +++ b/include/drivers/fwu/fwu_metadata.h @@ -17,10 +17,10 @@ #define NR_OF_MAX_FW_BANKS 4 /* Properties of image in a bank */ -struct fwu_image_properties { +struct fwu_image_bank_info { - /* UUID of the image in this bank */ - uuid_t img_uuid; + /* GUID of the image in this bank */ + struct efi_guid img_guid; /* [0]: bit describing the image acceptance status – * 1 means the image is accepted @@ -36,14 +36,14 @@ struct fwu_image_properties { /* Image entry information */ struct fwu_image_entry { - /* UUID identifying the image type */ - uuid_t img_type_uuid; + /* GUID identifying the image type */ + struct efi_guid img_type_guid; - /* UUID of the storage volume where the image is located */ - uuid_t location_uuid; + /* GUID of the storage volume where the image is located */ + struct efi_guid location_guid; - /* Properties of images with img_type_uuid in the different FW banks */ - struct fwu_image_properties img_props[NR_OF_FW_BANKS]; + /* Properties of images with img_type_guid in the different FW banks */ + struct fwu_image_bank_info img_bank_info[NR_OF_FW_BANKS]; } __packed; diff --git a/plat/st/common/bl2_io_storage.c b/plat/st/common/bl2_io_storage.c index 5513a2fc0..f8a0c1879 100644 --- a/plat/st/common/bl2_io_storage.c +++ b/plat/st/common/bl2_io_storage.c @@ -493,12 +493,10 @@ int bl2_plat_handle_pre_image_load(unsigned int image_id) */ #if !PSA_FWU_SUPPORT const partition_entry_t *entry; - const struct efi_guid img_type_guid = STM32MP_FIP_GUID; - uuid_t img_type_uuid; + const struct efi_guid fip_guid = STM32MP_FIP_GUID; - guidcpy(&img_type_uuid, &img_type_guid); partition_init(GPT_IMAGE_ID); - entry = get_partition_entry_by_type(&img_type_uuid); + entry = get_partition_entry_by_type(&fip_guid); if (entry == NULL) { entry = get_partition_entry(FIP_IMAGE_NAME); if (entry == NULL) { @@ -646,12 +644,12 @@ uint32_t plat_fwu_get_boot_idx(void) return boot_idx; } -static void *stm32_get_image_spec(const uuid_t *img_type_uuid) +static void *stm32_get_image_spec(const struct efi_guid *img_type_guid) { unsigned int i; for (i = 0U; i < MAX_NUMBER_IDS; i++) { - if ((guidcmp(&policies[i].img_type_guid, img_type_uuid)) == 0) { + if ((guidcmp(&policies[i].img_type_guid, img_type_guid)) == 0) { return (void *)policies[i].image_spec; } } @@ -664,8 +662,9 @@ void plat_fwu_set_images_source(const struct fwu_metadata *metadata) unsigned int i; uint32_t boot_idx; const partition_entry_t *entry __maybe_unused; - const uuid_t *img_type_uuid; - const uuid_t *img_uuid __maybe_unused; + const struct fwu_image_entry *img_entry; + const void *img_type_guid; + const void *img_guid; io_block_spec_t *image_spec; const uint16_t boot_itf = stm32mp_get_boot_itf_selected(); @@ -673,12 +672,13 @@ void plat_fwu_set_images_source(const struct fwu_metadata *metadata) assert(boot_idx < NR_OF_FW_BANKS); VERBOSE("Selecting to boot from bank %u\n", boot_idx); + img_entry = (void *)&metadata->fw_desc.img_entry; for (i = 0U; i < NR_OF_IMAGES_IN_FW_BANK; i++) { - img_type_uuid = &metadata->img_entry[i].img_type_uuid; + img_type_guid = &img_entry[i].img_type_guid; - img_uuid = &metadata->img_entry[i].img_props[boot_idx].img_uuid; + img_guid = &img_entry[i].img_bank_info[boot_idx].img_guid; - image_spec = stm32_get_image_spec(img_type_uuid); + image_spec = stm32_get_image_spec(img_type_guid); if (image_spec == NULL) { ERROR("Unable to get image spec for the image in the metadata\n"); panic(); @@ -688,7 +688,7 @@ void plat_fwu_set_images_source(const struct fwu_metadata *metadata) #if (STM32MP_SDMMC || STM32MP_EMMC) case BOOT_API_CTX_BOOT_INTERFACE_SEL_FLASH_SD: case BOOT_API_CTX_BOOT_INTERFACE_SEL_FLASH_EMMC: - entry = get_partition_entry_by_uuid(img_uuid); + entry = get_partition_entry_by_guid(img_guid); if (entry == NULL) { ERROR("No partition with the uuid mentioned in metadata\n"); panic(); @@ -700,9 +700,9 @@ void plat_fwu_set_images_source(const struct fwu_metadata *metadata) #endif #if STM32MP_SPI_NOR case BOOT_API_CTX_BOOT_INTERFACE_SEL_FLASH_NOR_SPI: - if (guidcmp(img_uuid, &STM32MP_NOR_FIP_A_GUID) == 0) { + if (guidcmp(img_guid, &STM32MP_NOR_FIP_A_GUID) == 0) { image_spec->offset = STM32MP_NOR_FIP_A_OFFSET; - } else if (guidcmp(img_uuid, &STM32MP_NOR_FIP_B_GUID) == 0) { + } else if (guidcmp(img_guid, &STM32MP_NOR_FIP_B_GUID) == 0) { image_spec->offset = STM32MP_NOR_FIP_B_OFFSET; } else { ERROR("Invalid uuid mentioned in metadata\n");