From 5db4710d89b61f6aee7f904e6eec0d4185f491a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Szczygie=C5=82?= Date: Mon, 4 Nov 2024 14:22:19 +0100 Subject: [PATCH] suit: Rework SDFW and SDFW Recovery sinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New implementation of SDFW and SDFW Recovery sinks. Supports fixed installation order (SDFW, then SDFW Recovery). Treats SDFW Recovery installation as optional. Envelope is saved when SDFW slot is successfully installed. Ref: NCSDK-29561 Signed-off-by: Adam Szczygieł --- subsys/suit/plat_err/include/suit_plat_err.h | 1 + .../suit/platform/sdfw/src/suit_plat_copy.c | 8 +- .../platform/src/suit_plat_error_convert.c | 3 + .../src/suit_sdfw_recovery_sink.c | 316 +++++++----------- .../stream/stream_sinks/src/suit_sdfw_sink.c | 199 ++++------- .../suit/utils/include/suit_plat_mem_util.h | 10 + subsys/suit/utils/src/suit_plat_mem_util.c | 60 +++- 7 files changed, 276 insertions(+), 321 deletions(-) diff --git a/subsys/suit/plat_err/include/suit_plat_err.h b/subsys/suit/plat_err/include/suit_plat_err.h index 7371edd95149..af9094d532ff 100644 --- a/subsys/suit/plat_err/include/suit_plat_err.h +++ b/subsys/suit/plat_err/include/suit_plat_err.h @@ -97,6 +97,7 @@ typedef int suit_plat_err_t; #define SUIT_PLAT_ERR_UNSUPPORTED -2010 /**< Attempt to perform an unsupported operation */ #define SUIT_PLAT_ERR_IPC -2011 /**< IPC error */ #define SUIT_PLAT_ERR_NO_RESOURCES -2012 /**< Not enough resources */ +#define SUIT_PLAT_ERR_SDRFW_FAILURE -2013 /**< Failure during SDFW Recovery update */ /** * If the error code is a common platform error code return it. diff --git a/subsys/suit/platform/sdfw/src/suit_plat_copy.c b/subsys/suit/platform/sdfw/src/suit_plat_copy.c index e88aed51bc45..7434ef99fb52 100644 --- a/subsys/suit/platform/sdfw/src/suit_plat_copy.c +++ b/subsys/suit/platform/sdfw/src/suit_plat_copy.c @@ -251,9 +251,11 @@ int suit_plat_copy(suit_component_t dst_handle, suit_component_t src_handle, } if (ret == SUIT_SUCCESS) { - ret = suit_generic_address_streamer_stream(payload_ptr, payload_size, &dst_sink); - if (ret != SUIT_PLAT_SUCCESS) { - LOG_ERR("memptr_streamer failed - error %i", ret); + plat_ret = + suit_generic_address_streamer_stream(payload_ptr, payload_size, &dst_sink); + if (plat_ret != SUIT_PLAT_SUCCESS) { + LOG_ERR("memptr_streamer failed - error %i", plat_ret); + ret = suit_plat_err_to_processor_err_convert(plat_ret); } } #endif /* CONFIG_SUIT_STREAM_SOURCE_MEMPTR */ diff --git a/subsys/suit/platform/src/suit_plat_error_convert.c b/subsys/suit/platform/src/suit_plat_error_convert.c index c1b0d1e20189..7d70c33a5bcd 100644 --- a/subsys/suit/platform/src/suit_plat_error_convert.c +++ b/subsys/suit/platform/src/suit_plat_error_convert.c @@ -24,6 +24,9 @@ int suit_plat_err_to_processor_err_convert(suit_plat_err_t plat_err) case SUIT_PLAT_ERR_CBOR_DECODING: proc_err = SUIT_ERR_DECODING; break; + case SUIT_PLAT_ERR_SDRFW_FAILURE: + proc_err = SUIT_FAIL_CONDITION; + break; /* To be extended */ default: /* Return SUIT_ERR_CRASH */ diff --git a/subsys/suit/stream/stream_sinks/src/suit_sdfw_recovery_sink.c b/subsys/suit/stream/stream_sinks/src/suit_sdfw_recovery_sink.c index 64541d54c78d..7e5e7e46ead5 100644 --- a/subsys/suit/stream/stream_sinks/src/suit_sdfw_recovery_sink.c +++ b/subsys/suit/stream/stream_sinks/src/suit_sdfw_recovery_sink.c @@ -14,18 +14,10 @@ #include #include -#include - #define SUIT_MAX_SDFW_RECOVERY_COMPONENTS 1 -#define SDFW_RECOVERY_SINK_ERR_AGAIN \ - 1 /* Reboot is needed before proceeding. Call the API again. \ - */ - LOG_MODULE_REGISTER(suit_sdfw_recovery_sink, CONFIG_SUIT_LOG_LEVEL); -typedef int sdf_sink_err_t; - struct sdfw_recovery_sink_context { bool in_use; bool write_called; @@ -44,55 +36,18 @@ static struct sdfw_recovery_sink_context *get_new_context(void) return NULL; } -static digest_sink_err_t verify_digest(uint8_t *buf, size_t buf_size, psa_algorithm_t algorithm, - uint8_t *expected_digest) +static suit_plat_err_t schedule_update(const uint8_t *buf, size_t size) { - struct stream_sink digest_sink; - suit_plat_err_t err = suit_digest_sink_get(&digest_sink, algorithm, expected_digest); - - if (err != SUIT_PLAT_SUCCESS) { - LOG_ERR("Failed to get digest sink: %d", err); - return err; - } - - err = digest_sink.write(digest_sink.ctx, buf, buf_size); - if (err != SUIT_PLAT_SUCCESS) { - LOG_ERR("Failed to write to stream: %d", err); - (void)digest_sink.release(digest_sink.ctx); - return err; - } - - digest_sink_err_t ret = suit_digest_sink_digest_match(digest_sink.ctx); - - err = digest_sink.release(digest_sink.ctx); - if (err != SUIT_PLAT_SUCCESS) { - LOG_WRN("Failed to release stream: %d", err); - } - - return ret; -} - -static suit_plat_err_t clear_urot_update_status(void) -{ - mram_erase((uintptr_t)&NRF_SICR->UROT.UPDATE, - sizeof(NRF_SICR->UROT.UPDATE) / CONFIG_SDFW_MRAM_WORD_SIZE); + int err = 0; - /* Clearing the registers is crucial for correct handling by SecROM. */ - /* Incorrect mram_erase behavior was observed on FPGA. */ - /* Since mram_erase returns void, there is a need for extra check and returning error code - * to handle such case. - */ - if (NRF_SICR->UROT.UPDATE.STATUS == SICR_UROT_UPDATE_STATUS_CODE_None && - NRF_SICR->UROT.UPDATE.OPERATION == SICR_UROT_UPDATE_OPERATION_OPCODE_Nop) { - return SUIT_PLAT_SUCCESS; - } else { - return SUIT_PLAT_ERR_IO; + if (!suit_plat_mem_clear_sicr_update_registers()) { + LOG_ERR("Failed to clear update registers"); + /* By design SDFW Recovery update error should not result in failing whole + * installation. + * Because of that, set specific error code instead of SUIT_PLAT_ERR_CRASH. + */ + return SUIT_PLAT_ERR_SDRFW_FAILURE; } -} - -static suit_plat_err_t schedule_sdfw_recovery_update(const uint8_t *buf, size_t size) -{ - int err = 0; const struct sdfw_update_blob update_blob = { .manifest_addr = @@ -122,126 +77,154 @@ static suit_plat_err_t schedule_sdfw_recovery_update(const uint8_t *buf, size_t if (err) { LOG_ERR("Failed to schedule SDFW Recovery update: %d", err); - return SUIT_PLAT_ERR_CRASH; + err = SUIT_PLAT_ERR_CRASH; + } else { + err = SUIT_PLAT_SUCCESS; } - LOG_INF("SDFW Recovery update scheduled"); - - return SUIT_PLAT_SUCCESS; + return err; } -static sdf_sink_err_t check_update_candidate(const uint8_t *buf, size_t size) +static void reboot_to_continue(void) { - uint8_t *candidate_binary_start = - (uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET); - uint8_t *candidate_digest_in_manifest = - (uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_DIGEST_OFFSET); - uint8_t *current_sdfw_recovery_digest = - (uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST); + if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { + LOG_INF("Reboot the system to continue update"); - /* First check if calculated digest of candidate matches the digest from Signed Manifest */ - digest_sink_err_t err = - verify_digest(candidate_binary_start, - size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET, - PSA_ALG_SHA_512, candidate_digest_in_manifest); - - if (err != SUIT_PLAT_SUCCESS) { - if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) { - LOG_ERR("Candidate inconsistent"); - } else { - LOG_ERR("Failed to calculate digest: %d", err); - } + LOG_PANIC(); - return SUIT_PLAT_ERR_CRASH; + sys_reboot(SYS_REBOOT_COLD); + } else { + LOG_DBG("Reboot disabled - perform manually"); } +} - LOG_DBG("Candidate consistent"); +static suit_plat_err_t schedule_update_and_reboot(const uint8_t *buf, size_t size) +{ + suit_plat_err_t err = schedule_update(buf, size); - /* Then compare candidate's digest with current SDFW Recovery digest */ - err = verify_digest(candidate_binary_start, - size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET, - PSA_ALG_SHA_512, current_sdfw_recovery_digest); if (err == SUIT_PLAT_SUCCESS) { - LOG_INF("Same candidate - skip update"); - return SUIT_PLAT_SUCCESS; - } else if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) { - LOG_INF("Different candidate"); - err = schedule_sdfw_recovery_update(buf, size); - if (err == SUIT_PLAT_SUCCESS) { - LOG_DBG("Update scheduled"); - err = SDFW_RECOVERY_SINK_ERR_AGAIN; + reboot_to_continue(); + if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { + /* If this code is reached, it means that reboot did not work. */ + /* In such case report an error. */ + LOG_ERR("Expected reboot did not happen"); + err = SUIT_PLAT_ERR_UNREACHABLE_PATH; } - return err; } - LOG_ERR("Failed to calculate digest: %d", err); - return SUIT_PLAT_ERR_CRASH; + return err; } -static void reboot_to_continue(void) +static suit_plat_err_t recovery_update_ongoing(const uint8_t *buf, size_t size) { - if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { - LOG_INF("Reboot the system to continue SDFW Recovery update"); + suit_plat_err_t err = SUIT_PLAT_SUCCESS; - LOG_PANIC(); + enum sdfw_update_status update_status = sdfw_update_initial_status_get(); - sys_reboot(SYS_REBOOT_COLD); - } else { - LOG_DBG("Reboot disabled - perform manually"); + /* Candidate is different than current FW but SDFW Recovery update was ongoing. */ + switch (update_status) { + case SDFW_UPDATE_STATUS_NONE: { + /* No pending operation even though operation indicates SDFW Recovery update. + * Yet candidate differs from current FW, so schedule the update. + */ + err = schedule_update_and_reboot(buf, size); + break; } + default: { + /* SecROM indicates error during update */ + LOG_ERR("Update failure: %08x", update_status); + /* By design SDFW Recovery update error should not result in failing whole + * installation. + * Because of that, set specific error code instead of SUIT_PLAT_ERR_CRASH. + */ + err = SUIT_PLAT_ERR_SDRFW_FAILURE; + break; + } + } + + return err; } -static suit_plat_err_t check_urot_none(const uint8_t *buf, size_t size) +static suit_plat_err_t sdfw_update_ongoing(const uint8_t *buf, size_t size) { - /* Detect update candidate. */ - /* It is enough to check Public Key Size field which occupies first 4B of Signed Manifest. - */ - if (*((uint32_t *)buf) == EMPTY_STORAGE_VALUE) { - LOG_INF("Update candidate not found"); - return SUIT_PLAT_ERR_NOT_FOUND; + suit_plat_err_t err = SUIT_PLAT_SUCCESS; + + enum sdfw_update_status update_status = sdfw_update_initial_status_get(); + + /* SDFW update was ongoing. */ + switch (update_status) { + case SDFW_UPDATE_STATUS_NONE: { + /* No pending operation even though operation indicates SDFW update. + * Proceed with SDFW Recovery update. + */ + err = schedule_update_and_reboot(buf, size); + break; + } + case SDFW_UPDATE_STATUS_UROT_ACTIVATED: { + /* SDFW was installed successfully. + * Proceed with SDFW Recovery update. + */ + err = schedule_update_and_reboot(buf, size); + break; + } + default: { + /* SecROM indicates error during SDFW update. + * By design SDFW update error should abort installation. + */ + LOG_ERR("SDFW update failure: %08x", update_status); + err = SUIT_PLAT_ERR_CRASH; + break; + } } - LOG_INF("Update candidate found"); + return err; +} - suit_plat_err_t err = check_update_candidate(buf, size); +static suit_plat_err_t update_needed_action(const uint8_t *buf, size_t size) +{ + suit_plat_err_t err = SUIT_PLAT_SUCCESS; - if (err == SDFW_RECOVERY_SINK_ERR_AGAIN) { - /* Update scheduled, continue after reboot */ - reboot_to_continue(); - if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { - /* If this code is reached, it means that reboot did not work. */ - /* In such case report an error and convert the error code. */ - LOG_ERR("Expected reboot did not happen"); - err = SUIT_PLAT_ERR_UNREACHABLE_PATH; - } + enum sdfw_update_operation initial_operation = sdfw_update_initial_operation_get(); + + switch (initial_operation) { + case SDFW_UPDATE_OPERATION_NOP: { + /* No previously running update or after the other slot update. */ + /* Schedule an update of this slot. */ + err = schedule_update_and_reboot(buf, size); + break; + } + case SDFW_UPDATE_OPERATION_UROT_ACTIVATE: { + /* SDFW update was ongoing */ + err = sdfw_update_ongoing(buf, size); + break; + } + case SDFW_UPDATE_OPERATION_RECOVERY_ACTIVATE: { + /* SDFW Recovery update was ongoing */ + err = recovery_update_ongoing(buf, size); + break; + } + default: { + LOG_ERR("Unhandled operation: %08x", initial_operation); + err = SUIT_PLAT_ERR_CRASH; + break; + } } return err; } -static suit_plat_err_t check_recovery_activated(const uint8_t *buf, size_t size) +static bool is_update_needed(const uint8_t *buf, size_t size) { - uint8_t *candidate_binary_start = - (uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET); - uint8_t *current_sdfw_digest = (uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST); - - /* Compare candidate's digest with current SDFW Recovery digest */ - digest_sink_err_t err = - verify_digest(candidate_binary_start, - size - (size_t)CONFIG_SUIT_SDFW_RECOVERY_UPDATE_FIRMWARE_OFFSET, - PSA_ALG_SHA_512, current_sdfw_digest); - if (err != SUIT_PLAT_SUCCESS) { - if (err == DIGEST_SINK_ERR_DIGEST_MISMATCH) { - LOG_ERR("Digest mismatch - update failure"); - return SUIT_PLAT_ERR_AUTHENTICATION; - } + const uint8_t *candidate_digest_in_manifest = + (uint8_t *)(buf + CONFIG_SUIT_SDFW_RECOVERY_UPDATE_DIGEST_OFFSET); + const uint8_t *current_sdfw_recovery_digest = + (uint8_t *)(NRF_SICR->UROT.RECOVERY.SM.TBS.FW.DIGEST); - LOG_ERR("Failed to calculate digest: %d", err); - return SUIT_PLAT_ERR_CRASH; - } + bool digests_match = memcmp(candidate_digest_in_manifest, current_sdfw_recovery_digest, + PSA_HASH_LENGTH(PSA_ALG_SHA_512)) == 0; - LOG_DBG("Digest match - update success"); - return SUIT_PLAT_SUCCESS; + /* Update is needed when candidate's digest doesn't match current FW digest */ + return !digests_match; } /* NOTE: Size means size of the SDFW binary to be updated, @@ -267,57 +250,12 @@ static suit_plat_err_t write(void *ctx, const uint8_t *buf, size_t size) context->write_called = true; suit_plat_err_t err = SUIT_PLAT_SUCCESS; - bool clear_registers = true; - switch (NRF_SICR->UROT.UPDATE.STATUS) { - case SICR_UROT_UPDATE_STATUS_CODE_None: { - err = check_urot_none(buf, size); - /* Potential start of update process - SecROM needs the registers to be set */ - clear_registers = false; - break; - } - - case SICR_UROT_UPDATE_STATUS_CODE_RecoveryActivated: { - err = check_recovery_activated(buf, size); - clear_registers = true; - break; - } - - /* TODO: Add handling of status RecoveryUnconfirmed and RecoveryConfirmed. - * For now the defines for these states are missing in mdk header files. - * NCSDK-26939 - */ - - case SICR_UROT_UPDATE_STATUS_CODE_UROTActivated: - case SICR_UROT_UPDATE_STATUS_CODE_VerifyOK: - case SICR_UROT_UPDATE_STATUS_CODE_AROTRecovery: { - LOG_ERR("Unsupported Recovery update status: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS); - err = SUIT_PLAT_ERR_INCORRECT_STATE; - clear_registers = true; - break; - } - - default: { - LOG_ERR("SDFW Recovery update failure: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS); - err = NRF_SICR->UROT.UPDATE.STATUS; - clear_registers = true; - break; - } - } - - if (clear_registers) { - suit_plat_err_t clear_err = clear_urot_update_status(); - - if (clear_err) { - LOG_ERR("Failed to clear UROT update status"); - /* If the only error was during register clearing - report it. */ - /* Otherwise report the original cause of failure. */ - if (err == SUIT_PLAT_SUCCESS) { - err = clear_err; - } - } else { - LOG_DBG("UROT update status cleared"); - } + if (is_update_needed(buf, size)) { + LOG_INF("Update needed"); + err = update_needed_action(buf, size); + } else { + LOG_INF("Update not needed"); } return err; diff --git a/subsys/suit/stream/stream_sinks/src/suit_sdfw_sink.c b/subsys/suit/stream/stream_sinks/src/suit_sdfw_sink.c index 93d0dc8a800e..5943bf999984 100644 --- a/subsys/suit/stream/stream_sinks/src/suit_sdfw_sink.c +++ b/subsys/suit/stream/stream_sinks/src/suit_sdfw_sink.c @@ -14,16 +14,10 @@ #include #include -#include - #define SUIT_MAX_SDFW_COMPONENTS 1 -#define SDFW_SINK_ERR_AGAIN 1 /* Reboot is needed before proceeding. Call the API again. */ - LOG_MODULE_REGISTER(suit_sdfw_sink, CONFIG_SUIT_LOG_LEVEL); -typedef int sdf_sink_err_t; - struct sdfw_sink_context { bool in_use; bool write_called; @@ -42,28 +36,28 @@ static struct sdfw_sink_context *get_new_context(void) return NULL; } -static suit_plat_err_t clear_urot_update_status(void) +static bool is_update_needed(const uint8_t *buf, size_t size) { - mram_erase((uintptr_t)&NRF_SICR->UROT.UPDATE, - sizeof(NRF_SICR->UROT.UPDATE) / CONFIG_SDFW_MRAM_WORD_SIZE); + const uint8_t *candidate_digest_in_manifest = + (uint8_t *)(buf + CONFIG_SUIT_SDFW_UPDATE_DIGEST_OFFSET); + const uint8_t *current_sdfw_digest = (uint8_t *)(NRF_SICR->UROT.SM.TBS.FW.DIGEST); - /* Clearing the registers is crucial for correct handling by SecROM. */ - /* Incorrect mram_erase behavior was observed on FPGA. */ - /* Since mram_erase returns void, there is a need for extra check and returning error code - * to handle such case. - */ - if (NRF_SICR->UROT.UPDATE.STATUS == SICR_UROT_UPDATE_STATUS_CODE_None && - NRF_SICR->UROT.UPDATE.OPERATION == SICR_UROT_UPDATE_OPERATION_OPCODE_Nop) { - return SUIT_PLAT_SUCCESS; - } else { - return SUIT_PLAT_ERR_IO; - } + bool digests_match = memcmp(candidate_digest_in_manifest, current_sdfw_digest, + PSA_HASH_LENGTH(PSA_ALG_SHA_512)) == 0; + + /* Update is needed when candidate's digest doesn't match current FW digest */ + return !digests_match; } -static suit_plat_err_t schedule_sdfw_update(const uint8_t *buf, size_t size) +static suit_plat_err_t schedule_update(const uint8_t *buf, size_t size) { int err = 0; + if (!suit_plat_mem_clear_sicr_update_registers()) { + LOG_ERR("Failed to clear update registers"); + return SUIT_PLAT_ERR_CRASH; + } + const struct sdfw_update_blob update_blob = { .manifest_addr = (uintptr_t)(buf + CONFIG_SUIT_SDFW_UPDATE_SIGNED_MANIFEST_OFFSET), .pubkey_addr = (uintptr_t)(buf + CONFIG_SUIT_SDFW_UPDATE_PUBLIC_KEY_OFFSET), @@ -88,37 +82,9 @@ static suit_plat_err_t schedule_sdfw_update(const uint8_t *buf, size_t size) if (err) { LOG_ERR("Failed to schedule SDFW update: %d", err); - return SUIT_PLAT_ERR_CRASH; - } - - LOG_INF("SDFW update scheduled"); - - return SUIT_PLAT_SUCCESS; -} - -static sdf_sink_err_t check_update_candidate(const uint8_t *buf, size_t size) -{ - uint8_t *candidate_digest_in_manifest = - (uint8_t *)(buf + CONFIG_SUIT_SDFW_UPDATE_DIGEST_OFFSET); - uint8_t *current_sdfw_digest = (uint8_t *)(NRF_SICR->UROT.SM.TBS.FW.DIGEST); - - sdf_sink_err_t err = SUIT_PLAT_SUCCESS; - - bool digests_match = - memcmp(candidate_digest_in_manifest, current_sdfw_digest, - PSA_HASH_LENGTH(PSA_ALG_SHA_512)) - == 0; - - if (digests_match) { - LOG_INF("Same candidate - skip update"); - return SUIT_PLAT_SUCCESS; + err = SUIT_PLAT_ERR_CRASH; } else { - LOG_INF("Different candidate"); - err = schedule_sdfw_update(buf, size); - if (err == SUIT_PLAT_SUCCESS) { - LOG_DBG("Update scheduled"); - err = SDFW_SINK_ERR_AGAIN; - } + err = SUIT_PLAT_SUCCESS; } return err; @@ -127,7 +93,7 @@ static sdf_sink_err_t check_update_candidate(const uint8_t *buf, size_t size) static void reboot_to_continue(void) { if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { - LOG_INF("Reboot the system to continue SDFW update"); + LOG_INF("Reboot the system to continue update"); LOG_PANIC(); @@ -137,26 +103,15 @@ static void reboot_to_continue(void) } } -static suit_plat_err_t check_urot_none(const uint8_t *buf, size_t size) +static suit_plat_err_t schedule_update_and_reboot(const uint8_t *buf, size_t size) { - /* Detect update candidate. */ - /* It is enough to check Public Key Size field which occupies first 4B of Signed Manifest. - */ - if (*((uint32_t *)buf) == EMPTY_STORAGE_VALUE) { - LOG_INF("Update candidate not found"); - return SUIT_PLAT_ERR_NOT_FOUND; - } + suit_plat_err_t err = schedule_update(buf, size); - LOG_INF("Update candidate found"); - - suit_plat_err_t err = check_update_candidate(buf, size); - - if (err == SDFW_SINK_ERR_AGAIN) { - /* Update scheduled, continue after reboot */ + if (err == SUIT_PLAT_SUCCESS) { reboot_to_continue(); if (IS_ENABLED(CONFIG_SUIT_UPDATE_REBOOT_ENABLED)) { /* If this code is reached, it means that reboot did not work. */ - /* In such case report an error and convert the error code. */ + /* In such case report an error. */ LOG_ERR("Expected reboot did not happen"); err = SUIT_PLAT_ERR_UNREACHABLE_PATH; } @@ -165,25 +120,59 @@ static suit_plat_err_t check_urot_none(const uint8_t *buf, size_t size) return err; } -static suit_plat_err_t check_urot_activated(const uint8_t *buf, size_t size) +static suit_plat_err_t sdfw_update_ongoing(const uint8_t *buf, size_t size) { - uint8_t *current_sdfw_digest = (uint8_t *)(NRF_SICR->UROT.SM.TBS.FW.DIGEST); - uint8_t *candidate_digest_in_manifest = - (uint8_t *)(buf + CONFIG_SUIT_SDFW_UPDATE_DIGEST_OFFSET); + suit_plat_err_t err = SUIT_PLAT_SUCCESS; - bool digests_match = - memcmp(candidate_digest_in_manifest, current_sdfw_digest, - PSA_HASH_LENGTH(PSA_ALG_SHA_512)) - == 0; + enum sdfw_update_status update_status = sdfw_update_initial_status_get(); - if (digests_match) { - LOG_DBG("Digest match - update success"); - return SUIT_PLAT_SUCCESS; + /* Candidate is different than current FW but SDFW update was ongoing. */ + switch (update_status) { + case SDFW_UPDATE_STATUS_NONE: { + /* No pending operation even though operation indicates SDFW update. + * Yet candidate differs from current FW, so schedule the update. + */ + err = schedule_update_and_reboot(buf, size); + break; + } + default: { + /* SecROM indicates error during update */ + LOG_ERR("Update failure: %08x", update_status); + err = SUIT_PLAT_ERR_CRASH; + break; + } } - LOG_ERR("Digest mismatch - update failure"); - return SUIT_PLAT_ERR_AUTHENTICATION; + return err; +} +static suit_plat_err_t update_needed_action(const uint8_t *buf, size_t size) +{ + suit_plat_err_t err = SUIT_PLAT_SUCCESS; + + enum sdfw_update_operation initial_operation = sdfw_update_initial_operation_get(); + + switch (initial_operation) { + case SDFW_UPDATE_OPERATION_NOP: + case SDFW_UPDATE_OPERATION_RECOVERY_ACTIVATE: { + /* No previously running update or after the other slot update. */ + /* Schedule an update of this slot. */ + err = schedule_update_and_reboot(buf, size); + break; + } + case SDFW_UPDATE_OPERATION_UROT_ACTIVATE: { + /* SDFW update was ongoing */ + err = sdfw_update_ongoing(buf, size); + break; + } + default: { + LOG_ERR("Unhandled operation: %08x", initial_operation); + err = SUIT_PLAT_ERR_CRASH; + break; + } + } + + return err; } /* NOTE: Size means size of the SDFW binary to be updated, @@ -209,52 +198,12 @@ static suit_plat_err_t write(void *ctx, const uint8_t *buf, size_t size) context->write_called = true; suit_plat_err_t err = SUIT_PLAT_SUCCESS; - bool clear_registers = true; - switch (NRF_SICR->UROT.UPDATE.STATUS) { - case SICR_UROT_UPDATE_STATUS_CODE_None: { - err = check_urot_none(buf, size); - /* Potential start of update process - SecROM needs the registers to be set */ - clear_registers = false; - break; - } - - case SICR_UROT_UPDATE_STATUS_CODE_UROTActivated: { - err = check_urot_activated(buf, size); - clear_registers = true; - break; - } - - case SICR_UROT_UPDATE_STATUS_CODE_VerifyOK: - case SICR_UROT_UPDATE_STATUS_CODE_RecoveryActivated: - case SICR_UROT_UPDATE_STATUS_CODE_AROTRecovery: { - LOG_ERR("Unsupported UROT update status: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS); - err = SUIT_PLAT_ERR_INCORRECT_STATE; - clear_registers = true; - break; - } - - default: { - LOG_ERR("SDFW update failure: 0x%08X", NRF_SICR->UROT.UPDATE.STATUS); - err = NRF_SICR->UROT.UPDATE.STATUS; - clear_registers = true; - break; - } - } - - if (clear_registers) { - suit_plat_err_t clear_err = clear_urot_update_status(); - - if (clear_err) { - LOG_ERR("Failed to clear UROT update status"); - /* If the only error was during register clearing - report it. */ - /* Otherwise report the original cause of failure. */ - if (err == SUIT_PLAT_SUCCESS) { - err = clear_err; - } - } else { - LOG_DBG("UROT update status cleared"); - } + if (is_update_needed(buf, size)) { + LOG_INF("Update needed"); + err = update_needed_action(buf, size); + } else { + LOG_INF("Update not needed"); } return err; diff --git a/subsys/suit/utils/include/suit_plat_mem_util.h b/subsys/suit/utils/include/suit_plat_mem_util.h index 3170a2c53470..130c300efa8e 100644 --- a/subsys/suit/utils/include/suit_plat_mem_util.h +++ b/subsys/suit/utils/include/suit_plat_mem_util.h @@ -85,6 +85,16 @@ uintptr_t suit_plat_mem_nvm_offset_get(uint8_t *ptr); */ uint8_t *suit_plat_mem_nvm_ptr_get(uintptr_t offset); +/** @brief Clear values of SICR.UROT.UPDATE.OPERATION and SICR.UROT.UPDATE.STATUS registers. + * + * @note The values are first checked and mramc access is done only when they need to be changed. + * Also mramc write mode is adjusted only when it is needed. + * + * @returns true when registers were cleared successfully or did not require clearing + * @returns false when any of the registers was not cleared + */ +bool suit_plat_mem_clear_sicr_update_registers(void); + #ifdef __cplusplus } #endif diff --git a/subsys/suit/utils/src/suit_plat_mem_util.c b/subsys/suit/utils/src/suit_plat_mem_util.c index cb6b335ee8f2..2d49fe26eef7 100644 --- a/subsys/suit/utils/src/suit_plat_mem_util.c +++ b/subsys/suit/utils/src/suit_plat_mem_util.c @@ -8,6 +8,9 @@ #ifdef CONFIG_FLASH_SIMULATOR #include #endif /* CONFIG_FLASH_SIMULATOR */ +#ifdef CONFIG_SDFW_IS_UROT +#include +#endif /* CONFIG_SDFW_IS_UROT */ uint8_t *suit_plat_mem_ptr_get(uintptr_t address) { @@ -71,9 +74,9 @@ uintptr_t suit_plat_mem_nvm_offset_get(uint8_t *ptr) uintptr_t address = suit_plat_mem_address_get(ptr); #if (DT_NODE_EXISTS(DT_NODELABEL(mram1x))) - address = (((address)&0xEFFFFFFFUL) - (DT_REG_ADDR(DT_NODELABEL(mram1x)) & 0xEFFFFFFFUL)); + address = (((address) & 0xEFFFFFFFUL) - (DT_REG_ADDR(DT_NODELABEL(mram1x)) & 0xEFFFFFFFUL)); #elif (DT_NODE_EXISTS(DT_NODELABEL(mram10))) - address = (((address)&0xEFFFFFFFUL) - (DT_REG_ADDR(DT_NODELABEL(mram10)) & 0xEFFFFFFFUL)); + address = (((address) & 0xEFFFFFFFUL) - (DT_REG_ADDR(DT_NODELABEL(mram10)) & 0xEFFFFFFFUL)); #endif return address; @@ -84,12 +87,61 @@ uint8_t *suit_plat_mem_nvm_ptr_get(uintptr_t offset) uintptr_t address; #if (DT_NODE_EXISTS(DT_NODELABEL(mram1x))) - address = (((offset)&0xEFFFFFFFUL) + (DT_REG_ADDR(DT_NODELABEL(mram1x)) & 0xEFFFFFFFUL)); + address = (((offset) & 0xEFFFFFFFUL) + (DT_REG_ADDR(DT_NODELABEL(mram1x)) & 0xEFFFFFFFUL)); #elif (DT_NODE_EXISTS(DT_NODELABEL(mram10))) - address = (((offset)&0xEFFFFFFFUL) + (DT_REG_ADDR(DT_NODELABEL(mram10)) & 0xEFFFFFFFUL)); + address = (((offset) & 0xEFFFFFFFUL) + (DT_REG_ADDR(DT_NODELABEL(mram10)) & 0xEFFFFFFFUL)); #else address = offset; #endif return suit_plat_mem_ptr_get(address); } + +#ifdef CONFIG_SDFW_IS_UROT +static nrf_mramc_mode_write_t set_mramc_write_mode(nrf_mramc_mode_write_t new_write_mode) +{ + NRF_MRAMC_Type * const mramc = (NRF_MRAMC_Type * const)DT_REG_ADDR(DT_NODELABEL(mramc)); + nrf_mramc_config_t mramc_config; + + nrf_mramc_config_get(mramc, &mramc_config); + + nrf_mramc_mode_write_t previous_write_mode = mramc_config.mode_write; + + if (new_write_mode != previous_write_mode) { + mramc_config.mode_write = new_write_mode; + nrf_mramc_config_set(mramc, &mramc_config); + } + + return previous_write_mode; +} + +bool suit_plat_mem_clear_sicr_update_registers(void) +{ + bool cleared = true; + + if (NRF_SICR->UROT.UPDATE.OPERATION != SICR_UROT_UPDATE_OPERATION_OPCODE_Nop || + NRF_SICR->UROT.UPDATE.STATUS != SICR_UROT_UPDATE_STATUS_CODE_None) { + + const nrf_mramc_mode_write_t new_write_mode = NRF_MRAMC_MODE_WRITE_DIRECT; + const nrf_mramc_mode_write_t previous_write_mode = + set_mramc_write_mode(new_write_mode); + + NRF_SICR->UROT.UPDATE.OPERATION = SICR_UROT_UPDATE_OPERATION_OPCODE_Nop; + NRF_SICR->UROT.UPDATE.STATUS = SICR_UROT_UPDATE_STATUS_CODE_None; + + /* Trigger MRAM write */ + NRF_SICR->UROT.UPDATE.SM.TBS.RFU[1] = (uint32_t)0xFFFFFFFF; + + if (previous_write_mode != new_write_mode) { + set_mramc_write_mode(previous_write_mode); + } + + if (NRF_SICR->UROT.UPDATE.OPERATION != SICR_UROT_UPDATE_OPERATION_OPCODE_Nop || + NRF_SICR->UROT.UPDATE.STATUS != SICR_UROT_UPDATE_STATUS_CODE_None) { + cleared = false; + } + } + + return cleared; +} +#endif /* CONFIG_SDFW_IS_UROT */