From bcf81f81754924625fd9f964d54d02b531a32493 Mon Sep 17 00:00:00 2001 From: Martin Velay Date: Tue, 22 Oct 2024 15:40:13 +0000 Subject: [PATCH] [hmac,sw] Workaround after hang - Implement a workaround in C cryptolib as HW is hanged after a stop in certain conditions. - Refer to issue #24767 Signed-off-by: Martin Velay --- sw/device/lib/crypto/drivers/BUILD | 1 + sw/device/lib/crypto/drivers/hmac.c | 76 ++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/sw/device/lib/crypto/drivers/BUILD b/sw/device/lib/crypto/drivers/BUILD index 0fe8705302d425..819f946d518306 100644 --- a/sw/device/lib/crypto/drivers/BUILD +++ b/sw/device/lib/crypto/drivers/BUILD @@ -183,6 +183,7 @@ cc_library( "//sw/device/lib/base:macros", "//sw/device/lib/base:memory", "//sw/device/lib/crypto/impl:status", + "//sw/device/lib/runtime:ibex", ], ) diff --git a/sw/device/lib/crypto/drivers/hmac.c b/sw/device/lib/crypto/drivers/hmac.c index 3d10c2d962f6e2..86de9f99c8d482 100644 --- a/sw/device/lib/crypto/drivers/hmac.c +++ b/sw/device/lib/crypto/drivers/hmac.c @@ -9,6 +9,7 @@ #include "sw/device/lib/base/hardened.h" #include "sw/device/lib/base/memory.h" #include "sw/device/lib/crypto/impl/status.h" +#include "sw/device/lib/runtime/ibex.h" #include "hmac_regs.h" // Generated. #include "hw/top_earlgrey/sw/autogen/top_earlgrey.h" @@ -67,6 +68,13 @@ OT_ASSERT_ENUM_VALUE(HMAC_DIGEST_15_REG_OFFSET, HMAC_DIGEST_14_REG_OFFSET + 4); enum { /* The beginning of the address space of HMAC. */ kHmacBaseAddr = TOP_EARLGREY_HMAC_BASE_ADDR, + /* Timeout value for the polling + * As max 4 clock cycles are required to perform a read access to the + * register. And as it should take less than 64 clock cycles in SHA2-256 + * and 80 clock cycles in SHA2-384/512, let's take some margin and consider + * that 100 loops are a way enough to see IDLE status. + */ + kNumIterTimeout = 100, }; /** @@ -75,8 +83,6 @@ enum { * It returns error if HMAC HWIP becomes idle without firing `hmac_done` * interrupt. * - * TODO(#23191): It might be beneficial to have a timeout value for the polling. - * * @return Result of the operation. */ OT_WARN_UNUSED_RESULT @@ -85,8 +91,13 @@ static status_t hmac_idle_wait(void) { // Initialize `status_reg = 0` so that the loop starts with the assumption // that HMAC HWIP is not idle. uint32_t status_reg = 0; + uint32_t attempt_cnt = 0; while (bitfield_bit32_read(status_reg, HMAC_STATUS_HMAC_IDLE_BIT) == 0) { status_reg = abs_mmio_read32(kHmacBaseAddr + HMAC_STATUS_REG_OFFSET); + attempt_cnt++; + if (attempt_cnt >= kNumIterTimeout) { + return OTCRYPTO_FATAL_ERR; + } } // Verify that HMAC HWIP raises `hmac_done` bit. @@ -256,6 +267,38 @@ static void msg_fifo_write(const uint8_t *message, size_t message_len) { } } +/** + * Temporary workaround linked to issue #24767 + * + * The HMAC HWIP is not told to stop. This will cause the HW to be in an + * unexpected state, which could be exited by trigerring a simple HASH process + * operation. Before doing that, the context should be saved (message length + * and digest). This context is only available after a duration equivalent to + * 64 clock cycles in SHA2-256 and 80 clock cycles in SHA2-384/512, after the + * message length is on a block boundary (512 for SHA2-256 or 1024 bits for + * SHA2-384/512). + * + * @param[out] ctx Context to which values are written. + */ +static void tmp_avoid_hw_hang(hmac_ctx_t *ctx) { + // Insert delay which should be equivalent to at least 80 clock cycles + ibex_timeout_t timeout; + timeout.cycles = kNumIterTimeout; + timeout.start = ibex_mcycle_read(); + while (!ibex_timeout_check(&timeout)) { + // NULL statement + } + + // Save current context as it is updated after each block even if stop is not + // triggered + context_save(ctx); + + // Trigger hash_process + uint32_t cmd_reg = abs_mmio_read32(kHmacBaseAddr + HMAC_CMD_REG_OFFSET); + cmd_reg = bitfield_bit32_write(cmd_reg, HMAC_CMD_HASH_PROCESS_BIT, true); + abs_mmio_write32(kHmacBaseAddr + HMAC_CMD_REG_OFFSET, cmd_reg); +} + /** * For given `hmac_mode`, derive the matching CFG value and block/digest * lengths. @@ -397,17 +440,24 @@ status_t hmac_update(hmac_ctx_t *ctx, const uint8_t *data, size_t len) { // Keep writing incoming bytes msg_fifo_write(data, len - leftover_len); - // Time to tell HMAC HWIP to stop, because we do not have enough message - // bytes for another round. - uint32_t cmd_reg = - bitfield_bit32_write(HMAC_CMD_REG_RESVAL, HMAC_CMD_HASH_STOP_BIT, 1); - abs_mmio_write32(kHmacBaseAddr + HMAC_CMD_REG_OFFSET, cmd_reg); - - // Wait for HMAC HWIP operation to be completed. - HARDENED_TRY(hmac_idle_wait()); - - // Store context into `ctx`. - context_save(ctx); + /* + * TODO should be uncommented once the issue #24767 will be solved in the HW + * and tmp_avoid_hw_hang should be removed. + * + * // Time to tell HMAC HWIP to stop, because we do not have enough message + * // bytes for another round. + * uint32_t cmd_reg = + * bitfield_bit32_write(HMAC_CMD_REG_RESVAL, HMAC_CMD_HASH_STOP_BIT, 1); + * abs_mmio_write32(kHmacBaseAddr + HMAC_CMD_REG_OFFSET, cmd_reg); + * + * // Wait for HMAC HWIP operation to be completed. + * HARDENED_TRY(hmac_idle_wait()); + * + * // Store context into `ctx`. + * context_save(ctx); + */ + + tmp_avoid_hw_hang(ctx); // Write leftover bytes to `partial_block`, so that future update/final call // can feed them to HMAC HWIP.