[sival] Add sysrst_ctrl_in_irq_test test

This tests follows the DV flow with two modifications: to avoid the various stages interfering with each other, add a sync step at the end of each stage so the host does not move forward until after the device code has seen and disabled the interrupt. The second problem is that on real device, the interrupt could occur between the phase change and the call to `wait_for_interrupt`. If that's the case, `wait_for_interrupt` will block forever. Modify the code so that the safely checks if the interrupt has already been handled before calling WFI. Signed-off-by: Amaury Pouly <[email protected]>
lowRISC · Jan 22, 2024 · 563ce4f · 563ce4f
1 parent a6e88c1
commit 563ce4f
Show file tree

Hide file tree

Showing 4 changed files with 300 additions and 14 deletions.
diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD
@@ -4232,7 +4232,7 @@ opentitan_test(
             --bootstrap="{firmware}"
             "{firmware:elf}"
         """,
-        test_harness = "//sw/host/tests/chip/sysrst_ctrl",
+        test_harness = "//sw/host/tests/chip/sysrst_ctrl:sysrst_ctrl_in_irq",
     ),
     exec_env = {
         "//hw/top_earlgrey:fpga_cw310_sival": None,
@@ -4249,6 +4249,7 @@ opentitan_test(
         "//sw/device/lib/testing:rv_plic_testutils",
         "//sw/device/lib/testing:sysrst_ctrl_testutils",
         "//sw/device/lib/testing/test_framework:ottf_main",
+        "//sw/device/lib/testing/test_framework:ottf_utils",
     ],
 )
 

diff --git a/sw/device/tests/sysrst_ctrl_in_irq_test.c b/sw/device/tests/sysrst_ctrl_in_irq_test.c
@@ -13,30 +13,41 @@
 #include "sw/device/lib/testing/sysrst_ctrl_testutils.h"
 #include "sw/device/lib/testing/test_framework/check.h"
 #include "sw/device/lib/testing/test_framework/ottf_main.h"
+#include "sw/device/lib/testing/test_framework/ottf_utils.h"
 
 #include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"
 
-OTTF_DEFINE_TEST_CONFIG();
+/* We need control flow for the ujson messages exchanged
+ * with the host in OTTF_WAIT_FOR on real devices. */
+OTTF_DEFINE_TEST_CONFIG(.enable_uart_flow_control = true);
 
 static dif_sysrst_ctrl_t sysrst_ctrl;
 static dif_rv_plic_t plic;
 
 enum {
-  kCurrentTestPhaseTimeoutUsec = 20,
+  kCurrentTestPhaseTimeoutUsecDV = 20,
+  kCurrentTestPhaseTimeoutUsecReal = 1000000,
   kPlicTarget = kTopEarlgreyPlicTargetIbex0,
 };
 
 static volatile dif_sysrst_ctrl_irq_t irq;
 static volatile top_earlgrey_plic_peripheral_t peripheral;
 dif_rv_plic_irq_id_t irq_id;
 
-// Test phase written by testbench.
-static volatile const uint8_t kCurrentTestPhase = 0;
+// On DV, we must use variables in flash.
+// On a real device, we must use variables in RAM.
+// In DV, the sequence can ensure that the pins are set even before the test
+// runs. On a real device, this is not the case and if the initial value of
+// kCurrentTestPhaseReal is 0, the very first OTTF_WAIT_FOR could succeed before
+// the host can set the pins. To avoid this, and only on real devices, set the
+// initial value to an invalid value so that we have to wait for the host.
+static volatile const uint8_t kCurrentTestPhaseDV = 0;
+static volatile uint8_t kCurrentTestPhaseReal = 0xff;
 uint8_t phase = 0;
 
 enum {
   kOutputNumPads = 0x8,
-  kOutputNunMioPads = 0x6,
+  kOutputNumMioPads = 0x6,
 };
 
 static const dif_pinmux_index_t kPeripheralInputs[] = {
@@ -48,25 +59,61 @@ static const dif_pinmux_index_t kPeripheralInputs[] = {
     kTopEarlgreyPinmuxPeripheralInSysrstCtrlAonLidOpen,
 };
 
-static const dif_pinmux_index_t kInputPads[] = {
+static const dif_pinmux_index_t kInputPadsDV[] = {
     kTopEarlgreyPinmuxInselIob3, kTopEarlgreyPinmuxInselIob6,
     kTopEarlgreyPinmuxInselIob8, kTopEarlgreyPinmuxInselIor13,
     kTopEarlgreyPinmuxInselIoc7, kTopEarlgreyPinmuxInselIoc9,
 };
 
+// We need different pins on the hyperdebug boards since certain
+// pins are not routed to the hyperdebug.
+static const dif_pinmux_index_t kInputPadsReal[] = {
+    kTopEarlgreyPinmuxInselIor10, kTopEarlgreyPinmuxInselIor11,
+    kTopEarlgreyPinmuxInselIor12, kTopEarlgreyPinmuxInselIor5,
+    kTopEarlgreyPinmuxInselIor6,  kTopEarlgreyPinmuxInselIor7,
+};
+
 void test_phase_sync(void) {
   test_status_set(kTestStatusInTest);
   test_status_set(kTestStatusInWfi);
 }
 
+static void wait_for_any_interrupt(void) {
+  // Disable interrupts to be certain interrupt doesn't occur between while
+  // condition check and `wait_for_interrupt` (so WFI misses that interrupt).
+  irq_global_ctrl(false);
+
+  // Only enter WFI loop if we haven't already seen the interrupt.
+  while (peripheral == UINT32_MAX) {
+    wait_for_interrupt();
+    // WFI ignores global interrupt enable, so enable it now and then
+    // immediately disable it. If there is an interrupt pending it will be
+    // taken here between the enable and disable. This confines the interrupt
+    // to a known place avoiding missed wakeup issues.
+    irq_global_ctrl(true);
+    irq_global_ctrl(false);
+  }
+  irq_global_ctrl(true);
+}
+
 /**
  * Configure for input change detection, sync with DV side, wait for input
  * change interrupt, check the interrupt cause and clear it.
  */
 void sysrst_ctrl_input_change_detect(
     dif_sysrst_ctrl_key_intr_src_t expected_key_intr_src) {
+  const uint32_t kCurrentTestPhaseTimeoutUsec =
+      kDeviceType == kDeviceSimDV ? kCurrentTestPhaseTimeoutUsecDV
+                                  : kCurrentTestPhaseTimeoutUsecReal;
+  const volatile uint8_t *kCurrentTestPhase = kDeviceType == kDeviceSimDV
+                                                  ? &kCurrentTestPhaseDV
+                                                  : &kCurrentTestPhaseReal;
+
   peripheral = UINT32_MAX;
-  IBEX_SPIN_FOR(phase++ == kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  LOG_INFO("Wait for test to start: want phase %d", phase);
+  OTTF_WAIT_FOR(phase == *kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  phase++;
+  LOG_INFO("Setup sysrst_ctrl");
 
   // Configure for input change.
   dif_sysrst_ctrl_input_change_config_t config = {
@@ -76,15 +123,18 @@ void sysrst_ctrl_input_change_detect(
   CHECK_DIF_OK(
       dif_sysrst_ctrl_input_change_detect_configure(&sysrst_ctrl, config));
 
+  LOG_INFO("Tell host we are ready");
   test_phase_sync();
 
-  IBEX_SPIN_FOR(phase++ == kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  OTTF_WAIT_FOR(phase == *kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  phase++;
   // Check that the interrupt isn't triggered at the first part of the test.
   CHECK(peripheral == UINT32_MAX,
         "The interrupt is triggered during input glitch.");
+  LOG_INFO("Tell host we did not detect the glitch");
   test_phase_sync();
 
-  wait_for_interrupt();
+  wait_for_any_interrupt();
   // Check that the interrupt is triggered at the second part of the test.
   CHECK(peripheral == kTopEarlgreyPlicPeripheralSysrstCtrlAon,
         "The interrupt is not triggered during the test.");
@@ -104,6 +154,11 @@ void sysrst_ctrl_input_change_detect(
   config.input_changes = 0;
   CHECK_DIF_OK(
       dif_sysrst_ctrl_input_change_detect_configure(&sysrst_ctrl, config));
+
+  // Tell host to finish the test (only on real devices).
+  LOG_INFO("Tell host to finish the test");
+  if (kDeviceType != kDeviceSimDV)
+    test_phase_sync();
 }
 
 /**
@@ -112,8 +167,18 @@ void sysrst_ctrl_input_change_detect(
  */
 void sysrst_ctrl_key_combo_detect(dif_sysrst_ctrl_key_combo_t key_combo,
                                   uint32_t combo_keys) {
+  const uint32_t kCurrentTestPhaseTimeoutUsec =
+      kDeviceType == kDeviceSimDV ? kCurrentTestPhaseTimeoutUsecDV
+                                  : kCurrentTestPhaseTimeoutUsecReal;
+  const volatile uint8_t *kCurrentTestPhase = kDeviceType == kDeviceSimDV
+                                                  ? &kCurrentTestPhaseDV
+                                                  : &kCurrentTestPhaseReal;
+
   peripheral = UINT32_MAX;
-  IBEX_SPIN_FOR(phase++ == kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  LOG_INFO("wait for test to start");
+  OTTF_WAIT_FOR(phase == *kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  phase++;
+  LOG_INFO("configure sysrst interrupt");
 
   // Configure for key combo
   dif_sysrst_ctrl_key_combo_config_t sysrst_ctrl_key_combo_config = {
@@ -125,15 +190,20 @@ void sysrst_ctrl_key_combo_detect(dif_sysrst_ctrl_key_combo_t key_combo,
   CHECK_DIF_OK(dif_sysrst_ctrl_key_combo_detect_configure(
       &sysrst_ctrl, key_combo, sysrst_ctrl_key_combo_config));
 
+  LOG_INFO("tell host we are ready");
   test_phase_sync();
 
-  IBEX_SPIN_FOR(phase++ == kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  OTTF_WAIT_FOR(phase == *kCurrentTestPhase, kCurrentTestPhaseTimeoutUsec);
+  phase++;
   // Check that the interrupt isn't triggered at the first part of the test.
   CHECK(peripheral == UINT32_MAX,
         "The interrupt is triggered during input glitch.");
+  LOG_INFO("tell host we did not detect the glitch");
   test_phase_sync();
 
-  wait_for_interrupt();
+  LOG_INFO("wait for interrupt");
+  wait_for_any_interrupt();
+  LOG_INFO("interrupt triggered, checks causes");
   // Check that the interrupt is triggered at the second part of the test.
   CHECK(peripheral == kTopEarlgreyPlicPeripheralSysrstCtrlAon,
         "The interrupt is not triggered during the test.");
@@ -152,6 +222,11 @@ void sysrst_ctrl_key_combo_detect(dif_sysrst_ctrl_key_combo_t key_combo,
   sysrst_ctrl_key_combo_config.keys = 0;
   CHECK_DIF_OK(dif_sysrst_ctrl_key_combo_detect_configure(
       &sysrst_ctrl, key_combo, sysrst_ctrl_key_combo_config));
+
+  // Tell host to finish the test (only on real devices).
+  LOG_INFO("Tell host to finish the test");
+  if (kDeviceType != kDeviceSimDV)
+    test_phase_sync();
 }
 
 /**
@@ -205,7 +280,13 @@ bool test_main(void) {
   dif_pinmux_t pinmux;
   CHECK_DIF_OK(dif_pinmux_init(
       mmio_region_from_addr(TOP_EARLGREY_PINMUX_AON_BASE_ADDR), &pinmux));
-  for (int i = 0; i < kOutputNunMioPads; ++i) {
+
+  // On real devices, we also need to configure the DIO pins.
+  if (kDeviceType != kDeviceSimDV)
+    setup_dio_pins(&pinmux, &sysrst_ctrl);
+  const dif_pinmux_index_t *kInputPads =
+      kDeviceType == kDeviceSimDV ? kInputPadsDV : kInputPadsReal;
+  for (int i = 0; i < kOutputNumMioPads; ++i) {
     CHECK_DIF_OK(
         dif_pinmux_input_select(&pinmux, kPeripheralInputs[i], kInputPads[i]));
   }

diff --git a/sw/host/tests/chip/sysrst_ctrl/BUILD b/sw/host/tests/chip/sysrst_ctrl/BUILD
@@ -23,3 +23,21 @@ rust_binary(
         "@crate_index//:serde_json",
     ],
 )
+
+rust_binary(
+    name = "sysrst_ctrl_in_irq",
+    srcs = [
+        "sysrst_ctrl.rs",
+        "sysrst_ctrl_in_irq.rs",
+    ],
+    deps = [
+        "//sw/host/opentitanlib",
+        "@crate_index//:anyhow",
+        "@crate_index//:clap",
+        "@crate_index//:humantime",
+        "@crate_index//:log",
+        "@crate_index//:object",
+        "@crate_index//:once_cell",
+        "@crate_index//:serde_json",
+    ],
+)