From 42e78e12e601d8e4f826cbcf553ed216c88211b6 Mon Sep 17 00:00:00 2001 From: Chris Frantz Date: Wed, 7 Aug 2024 11:57:01 -0700 Subject: [PATCH] [rom_ext] Do not process `boot_svc` request for wakeups The ROM_EXT should not process `boot_svc` requests for low-power exit cases as resuming from sleep is not expected to change the state of the chip. Fixes: #24235 Signed-off-by: Chris Frantz (cherry picked from commit 51c2dc933a7125b514ef008b8db662861402f3f6) --- .../rom_ext/e2e/boot_svc/BUILD | 28 +++++ .../rom_ext/e2e/boot_svc/boot_svc_test_lib.h | 1 + .../e2e/boot_svc/boot_svc_wakeup_test.c | 106 ++++++++++++++++++ sw/device/silicon_creator/rom_ext/rom_ext.c | 19 ++-- 4 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_wakeup_test.c diff --git a/sw/device/silicon_creator/rom_ext/e2e/boot_svc/BUILD b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/BUILD index 03907a9c801ff..daaf7c0eb2185 100644 --- a/sw/device/silicon_creator/rom_ext/e2e/boot_svc/BUILD +++ b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/BUILD @@ -52,6 +52,34 @@ opentitan_test( ], ) +opentitan_test( + name = "boot_svc_wakeup_test", + srcs = ["boot_svc_wakeup_test.c"], + exec_env = { + "//hw/top_earlgrey:fpga_cw310_rom_ext": None, + }, + fpga = fpga_params( + assemble = "{rom_ext}@0 {firmware}@0x10000", + #exit_failure = "BFV|PASS|FAIL", + #exit_success = "FinalBootLog: 2:AA\r\n", + ), + linker_script = "//sw/device/lib/testing/test_framework:ottf_ld_silicon_owner_slot_virtual", + deps = [ + ":boot_svc_test_lib", + "//sw/device/lib/base:status", + "//sw/device/lib/dif:aon_timer", + "//sw/device/lib/dif:pwrmgr", + "//sw/device/lib/runtime:log", + "//sw/device/lib/testing:pwrmgr_testutils", + "//sw/device/lib/testing/test_framework:check", + "//sw/device/lib/testing/test_framework:ottf_main", + "//sw/device/silicon_creator/lib:boot_log", + "//sw/device/silicon_creator/lib/boot_svc:boot_svc_empty", + "//sw/device/silicon_creator/lib/drivers:retention_sram", + "//sw/device/silicon_creator/lib/drivers:rstmgr", + ], +) + opentitan_test( name = "boot_svc_next_test", srcs = ["boot_svc_next_test.c"], diff --git a/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_test_lib.h b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_test_lib.h index a0ad699f42bcf..2c73cc0bf8dea 100644 --- a/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_test_lib.h +++ b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_test_lib.h @@ -12,6 +12,7 @@ typedef enum boot_svc_test { kBootSvcTestNextBl0 = 2, kBootSvcTestBadNextBl0 = 3, kBootSvcTestBl0MinSecVer = 4, + kBootSvcTestWakeup = 5, } boot_svc_test_t; typedef enum boot_svc_test_state { diff --git a/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_wakeup_test.c b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_wakeup_test.c new file mode 100644 index 0000000000000..f74ef34a526fe --- /dev/null +++ b/sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_wakeup_test.c @@ -0,0 +1,106 @@ +// Copyright lowRISC contributors (OpenTitan project). +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +#include "sw/device/lib/base/mmio.h" +#include "sw/device/lib/base/status.h" +#include "sw/device/lib/dif/dif_aon_timer.h" +#include "sw/device/lib/dif/dif_pwrmgr.h" +#include "sw/device/lib/runtime/log.h" +#include "sw/device/lib/testing/pwrmgr_testutils.h" +#include "sw/device/lib/testing/test_framework/check.h" +#include "sw/device/lib/testing/test_framework/ottf_main.h" +#include "sw/device/silicon_creator/lib/boot_svc/boot_svc_empty.h" +#include "sw/device/silicon_creator/lib/drivers/retention_sram.h" +#include "sw/device/silicon_creator/lib/drivers/rstmgr.h" +#include "sw/device/silicon_creator/rom_ext/e2e/boot_svc/boot_svc_test_lib.h" + +#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h" + +OTTF_DEFINE_TEST_CONFIG(); + +static dif_pwrmgr_t pwrmgr; +static dif_aon_timer_t aon_timer; + +static status_t test_init(void) { + // Initialize aon timer to use the wdog. + CHECK_DIF_OK(dif_aon_timer_init( + mmio_region_from_addr(TOP_EARLGREY_AON_TIMER_AON_BASE_ADDR), &aon_timer)); + TRY(dif_pwrmgr_init(mmio_region_from_addr(TOP_EARLGREY_PWRMGR_AON_BASE_ADDR), + &pwrmgr)); + return OK_STATUS(); +} + +static status_t deep_sleep_enter(uint32_t wakeup_ticks) { + dif_pwrmgr_domain_config_t pwrmgr_domain_cfg = 0; + // WakeupSourceFive is the AonTimer source. + // See hw/top_earlgrey/ip_autogen/pwrmgr/dv/env/pwrmgr_env_pkg.sv%wakeup_e. + TRY(pwrmgr_testutils_enable_low_power( + &pwrmgr, kDifPwrmgrWakeupRequestSourceFive, pwrmgr_domain_cfg)); + TRY(dif_aon_timer_wakeup_start(&aon_timer, wakeup_ticks, 0)); + LOG_INFO("Going to sleep."); + wait_for_interrupt(); + LOG_INFO("Unexpected wakeup from deep sleep."); + return UNKNOWN(); +} + +static status_t deep_sleep_check(void) { + bool wkup = TRY(pwrmgr_testutils_is_wakeup_reason( + &pwrmgr, kDifPwrmgrWakeupRequestSourceFive)); + return OK_STATUS(wkup); +} + +static status_t initialize(retention_sram_t *retram, boot_svc_retram_t *state) { + boot_svc_msg_t msg = {0}; + boot_svc_empty_req_init(&msg.empty); + retram->creator.boot_svc_msg = msg; + state->state = kBootSvcTestStateCheckEmpty; + TRY(deep_sleep_enter((uint32_t)kClockFreqAonHz)); + return INTERNAL(); +} + +static status_t check_empty(retention_sram_t *retram, + boot_svc_retram_t *state) { + if (!TRY(deep_sleep_check())) { + LOG_ERROR("Expected wakup from deep sleep"); + return INTERNAL(); + } + boot_svc_msg_t msg = retram->creator.boot_svc_msg; + TRY(boot_svc_header_check(&msg.header)); + // We expect the `EmptyReqType` here because the ROM_EXT should not process + // boot_svc requests when waking from deep sleep. + TRY_CHECK(msg.header.type == kBootSvcEmptyReqType); + state->state = kBootSvcTestStateFinal; + return OK_STATUS(); +} + +static status_t empty_message_test(void) { + TRY(test_init()); + retention_sram_t *retram = retention_sram_get(); + TRY(boot_svc_test_init(retram, kBootSvcTestWakeup)); + boot_svc_retram_t *state = (boot_svc_retram_t *)&retram->owner; + + for (;;) { + LOG_INFO("Test state = %d", state->state); + switch (state->state) { + case kBootSvcTestStateInit: + TRY(initialize(retram, state)); + break; + case kBootSvcTestStateCheckEmpty: + TRY(check_empty(retram, state)); + break; + case kBootSvcTestStateFinal: + return OK_STATUS(); + default: + return UNKNOWN(); + } + } +} + +bool test_main(void) { + status_t sts = empty_message_test(); + if (status_err(sts)) { + LOG_ERROR("boot_svc_wakeup_test: %r", sts); + } + return status_ok(sts); +} diff --git a/sw/device/silicon_creator/rom_ext/rom_ext.c b/sw/device/silicon_creator/rom_ext/rom_ext.c index 9f9bf46387e03..7dcff4454bcc8 100644 --- a/sw/device/silicon_creator/rom_ext/rom_ext.c +++ b/sw/device/silicon_creator/rom_ext/rom_ext.c @@ -36,6 +36,7 @@ #include "sw/device/silicon_creator/lib/drivers/pinmux.h" #include "sw/device/silicon_creator/lib/drivers/retention_sram.h" #include "sw/device/silicon_creator/lib/drivers/rnd.h" +#include "sw/device/silicon_creator/lib/drivers/rstmgr.h" #include "sw/device/silicon_creator/lib/drivers/uart.h" #include "sw/device/silicon_creator/lib/epmp_state.h" #include "sw/device/silicon_creator/lib/manifest.h" @@ -863,13 +864,17 @@ static rom_error_t rom_ext_start(boot_data_t *boot_data, boot_log_t *boot_log) { // Handle any pending boot_svc commands. rom_error_t error; - error = handle_boot_svc(boot_data); - if (error == kErrorWriteBootdataThenReboot) { - // Boot services reports errors by writing a status code into the reply - // messages. Regardless of whether a boot service request produced an - // error, we want to continue booting unless the error specifically asks - // for a reboot. - return error; + uint32_t reset_reasons = retention_sram_get()->creator.reset_reasons; + uint32_t skip_boot_svc = reset_reasons & (1 << kRstmgrReasonLowPowerExit); + if (skip_boot_svc == 0) { + error = handle_boot_svc(boot_data); + if (error == kErrorWriteBootdataThenReboot) { + // Boot services reports errors by writing a status code into the reply + // messages. Regardless of whether a boot service request produced an + // error, we want to continue booting unless the error specifically asks + // for a reboot. + return error; + } } // Re-sync the boot_log entries that could be changed by boot services.