[sival,rstmgr] Adjust rstmgr_alert_info_test

A minor cleanup of the test to expect an otp_ctrl error if not in rom_ext. There is still an unexpected flash_ctrl read error for cw310 upon entry to test_main, so it is pending from rom_ext. Add utility to log the bits set in flash_ctrl fault status. The issue #20589 has an explanation of this test's failures. Signed-off-by: Guillermo Maturana <[email protected]>
lowRISC · Jan 24, 2024 · e61d18f · e61d18f
1 parent da087bb
commit e61d18f
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 22 deletions.
diff --git a/hw/top_earlgrey/data/ip/chip_rstmgr_testplan.hjson b/hw/top_earlgrey/data/ip/chip_rstmgr_testplan.hjson
@@ -65,16 +65,14 @@
             after reset seems suitable. The `spi_host` IPs receive multiple resets so they will
             need special consideration.
 
-            SiVal: CPU must be enabled, but no other OTP or lifecycle dependencies.
+            SiVal: CPU and debug must be enabled, so it only works in TEST_UNLOCKED, DEV, and RMA.
             The rv_dm is an important tool for SiVal, so the stage is set to SV2.
             '''
       stage: V2
       si_stage: SV3
       lc_states: [
         "TEST_UNLOCKED",
         "DEV",
-        "PROD",
-        "PROD_END",
         "RMA",
       ]
       features: [
@@ -153,8 +151,7 @@
             Refer to `chip_sw_rstmgr_*sys_reset_info`.
 
             SiVal: CPU must be enabled, but no other OTP or lifecycle dependencies.
-            This can be an important diagnostic tool, so setting it to SV2.
-            This test already runs in CW310.
+            This can be an important diagnostic tool, so setting it to SV3.
             '''
       stage: V2
       si_stage: SV3
@@ -170,12 +167,7 @@
         "RSTMGR.ALERT_INFO.ENABLE",
       ]
       tests: ["chip_sw_rstmgr_alert_info"]
-      bazel: [
-# TODO(lowrisc/opentitan#20589): Enable these _sival tests when the bug is fixed
-#        "//sw/device/tests:rstmgr_alert_info_test_fpga_cw310_sival",
-#        "//sw/device/tests:rstmgr_alert_info_test_fpga_cw310_sival_rom_ext",
-        "//sw/device/tests:rstmgr_alert_info_test_fpga_cw310_test_rom",
-      ]
+      bazel: ["//sw/device/tests:rstmgr_alert_info_test"]
     }
     {
       name: chip_sw_rstmgr_sw_rst

diff --git a/sw/device/lib/testing/flash_ctrl_testutils.c b/sw/device/lib/testing/flash_ctrl_testutils.c
@@ -335,3 +335,26 @@ status_t flash_ctrl_testutils_backdoor_wait_update(const volatile uint8_t *addr,
   } while (new_data == prior_data);
   return OK_STATUS();
 }
+
+status_t flash_ctrl_testutils_show_faults(
+    const dif_flash_ctrl_state_t *flash_ctrl) {
+  dif_flash_ctrl_faults_t faults = {.memory_properties_error = false};
+  CHECK_DIF_OK(dif_flash_ctrl_get_faults(flash_ctrl, &faults));
+#define LOG_IF_FIELD_SET(_struct, _field)             \
+  if (_struct._field != 0) {                          \
+    LOG_INFO("Flash_ctrl fault status has " #_field); \
+  }
+
+  LOG_IF_FIELD_SET(faults, memory_properties_error);
+  LOG_IF_FIELD_SET(faults, read_error);
+  LOG_IF_FIELD_SET(faults, prog_window_error);
+  LOG_IF_FIELD_SET(faults, prog_type_error);
+  LOG_IF_FIELD_SET(faults, host_gnt_error);
+  LOG_IF_FIELD_SET(faults, register_integrity_error);
+  LOG_IF_FIELD_SET(faults, phy_integrity_error);
+  LOG_IF_FIELD_SET(faults, lifecycle_manager_error);
+  LOG_IF_FIELD_SET(faults, shadow_storage_error);
+#undef LOG_IF_FIELD_SET
+
+  return OK_STATUS();
+}
diff --git a/sw/device/lib/testing/flash_ctrl_testutils.h b/sw/device/lib/testing/flash_ctrl_testutils.h
@@ -267,4 +267,13 @@ status_t flash_ctrl_testutils_backdoor_wait_update(const volatile uint8_t *addr,
                                                    uint8_t prior_data,
                                                    size_t timeout_usec);
 
+/**
+ * Write to log any faults set in the status register.
+ *
+ * @param flash_state A flash_ctrl state handle.
+ */
+OT_WARN_UNUSED_RESULT
+status_t flash_ctrl_testutils_show_faults(
+    const dif_flash_ctrl_state_t *flash_state);
+
 #endif  // OPENTITAN_SW_DEVICE_LIB_TESTING_FLASH_CTRL_TESTUTILS_H_
diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD
@@ -3585,6 +3585,7 @@ opentitan_test(
     deps = [
         "//hw/top_earlgrey:alert_handler_regs",
         "//hw/top_earlgrey/sw/autogen:top_earlgrey",
+        "//sw/device/lib/arch:boot_stage",
         "//sw/device/lib/arch:device",
         "//sw/device/lib/base:memory",
         "//sw/device/lib/base:mmio",
@@ -3605,6 +3606,7 @@ opentitan_test(
         "//sw/device/lib/runtime:log",
         "//sw/device/lib/testing:alert_handler_testutils",
         "//sw/device/lib/testing:aon_timer_testutils",
+        "//sw/device/lib/testing:flash_ctrl_testutils",
         "//sw/device/lib/testing:keymgr_testutils",
         "//sw/device/lib/testing:ret_sram_testutils",
         "//sw/device/lib/testing:rstmgr_testutils",

diff --git a/sw/device/tests/rstmgr_alert_info_test.c b/sw/device/tests/rstmgr_alert_info_test.c
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0, see LICENSE for details.
 // SPDX-License-Identifier: Apache-2.0
 
+#include "sw/device/lib/arch/boot_stage.h"
 #include "sw/device/lib/base/math.h"
 #include "sw/device/lib/base/mmio.h"
 #include "sw/device/lib/dif/dif_alert_handler.h"
@@ -20,6 +21,7 @@
 #include "sw/device/lib/runtime/log.h"
 #include "sw/device/lib/testing/alert_handler_testutils.h"
 #include "sw/device/lib/testing/aon_timer_testutils.h"
+#include "sw/device/lib/testing/flash_ctrl_testutils.h"
 #include "sw/device/lib/testing/keymgr_testutils.h"
 #include "sw/device/lib/testing/ret_sram_testutils.h"
 #include "sw/device/lib/testing/rstmgr_testutils.h"
@@ -189,6 +191,8 @@ typedef struct test_alert_info {
   alert_handler_testutils_info_t alert_info;
 } test_alert_info_t;
 
+// The expected info is set for rom_ext, meaning kBootStage set to
+// kBootStageOwner. It is adjusted in init_expected_info_for_non_rom_ext.
 static test_alert_info_t kExpectedInfo[kRoundTotal] = {
     [kRound1] =
         {
@@ -205,9 +209,9 @@ static test_alert_info_t kExpectedInfo[kRoundTotal] = {
             .test_name = "Multi classes(ClassB,C)",
             .alert_info =
                 {
-                    .class_accum_cnt = {0, 1, 4, 0},
-                    .class_esc_state = {kCstateIdle, kCstatePhase1,
-                                        kCstatePhase0, kCstateIdle},
+                    .class_accum_cnt = {0, 0, 4, 0},
+                    .class_esc_state = {kCstateIdle, kCstateIdle, kCstateIdle,
+                                        kCstateIdle},
                 },
         },
     [kRound3] =
@@ -683,9 +687,6 @@ static void init_expected_cause(void) {
       .alert_info.alert_cause[kTopEarlgreyAlertIdUart2FatalFault] = 1;
   kExpectedInfo[kRound2]
       .alert_info.alert_cause[kTopEarlgreyAlertIdUart3FatalFault] = 1;
-  kExpectedInfo[kRound2]
-      .alert_info.alert_cause[kTopEarlgreyAlertIdOtpCtrlFatalBusIntegError] = 1;
-
   kExpectedInfo[kRound3]
       .alert_info.alert_cause[kTopEarlgreyAlertIdRvCoreIbexRecovSwErr] = 1;
   kExpectedInfo[kRound3]
@@ -695,6 +696,19 @@ static void init_expected_cause(void) {
   kExpectedInfo[kRound3]
       .alert_info.alert_cause[kTopEarlgreyAlertIdSpiHost0FatalFault] = 1;
 }
+
+// Modify kExpectedInfo for runs without rom_ext, so non-owner stages. The
+// difference is that without rom_ext we expect an otp alert in round 2.
+static void init_expected_info_for_non_rom_ext(void) {
+  if (kBootStage != kBootStageOwner) {
+    kExpectedInfo[kRound2].alert_info.class_accum_cnt[1] = 1;
+    kExpectedInfo[kRound2].alert_info.class_esc_state[1] = kCstatePhase1;
+    kExpectedInfo[kRound2]
+        .alert_info.alert_cause[kTopEarlgreyAlertIdOtpCtrlFatalBusIntegError] =
+        1;
+  }
+}
+
 bool test_main(void) {
   uint32_t event_idx = 0;
 
@@ -704,6 +718,7 @@ bool test_main(void) {
 
   // set expected values
   init_expected_cause();
+  init_expected_info_for_non_rom_ext();
 
   CHECK_DIF_OK(dif_rstmgr_init(
       mmio_region_from_addr(TOP_EARLGREY_RSTMGR_AON_BASE_ADDR), &rstmgr));
@@ -712,11 +727,12 @@ bool test_main(void) {
       &alert_handler));
   CHECK_DIF_OK(dif_rv_plic_init(
       mmio_region_from_addr(TOP_EARLGREY_RV_PLIC_BASE_ADDR), &plic));
-
   CHECK_DIF_OK(dif_flash_ctrl_init_state(
       &flash_ctrl,
       mmio_region_from_addr(TOP_EARLGREY_FLASH_CTRL_CORE_BASE_ADDR)));
 
+  CHECK_STATUS_OK(flash_ctrl_testutils_show_faults(&flash_ctrl));
+
   peripheral_init();
 
   // Enable all interrupts used in this test.
@@ -745,13 +761,14 @@ bool test_main(void) {
     CHECK_STATUS_OK(ret_sram_testutils_counter_get(kEventCounter, &event_idx));
     CHECK_STATUS_OK(ret_sram_testutils_counter_increment(kEventCounter));
     LOG_INFO("Test round %d", event_idx);
-    // We need to initialize the info FLASH partitions storing the Creator and
-    // Owner secrets to avoid getting the flash controller into a fatal error
-    // state.
-    if (kDeviceType == kDeviceFpgaCw310 && rst_info & kDifRstmgrResetInfoPor) {
+    // If not running rom_ext we need to initialize the info FLASH partitions
+    // storing the Creator and Owner secrets to avoid getting the flash
+    // controller into a fatal error state.
+    if (kBootStage != kBootStageOwner) {
       CHECK_STATUS_OK(keymgr_testutils_flash_init(&flash_ctrl, &kCreatorSecret,
                                                   &kOwnerSecret));
     }
+    CHECK_STATUS_OK(flash_ctrl_testutils_show_faults(&flash_ctrl));
 
     global_test_round = kRound1;
     prgm_alert_handler_round1();