From d8cdc72bf1208d51188b19f523f8b57efeee268c Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Fri, 12 Jan 2024 14:27:55 +0000 Subject: [PATCH] [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 --- sw/device/tests/BUILD | 4 +- sw/device/tests/sysrst_ctrl_in_irq_test.c | 109 ++++++++-- sw/host/tests/chip/sysrst_ctrl/BUILD | 18 ++ .../chip/sysrst_ctrl/sysrst_ctrl_in_irq.rs | 186 ++++++++++++++++++ 4 files changed, 303 insertions(+), 14 deletions(-) create mode 100644 sw/host/tests/chip/sysrst_ctrl/sysrst_ctrl_in_irq.rs diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD index 3cbf049b914be7..8b26dc7fdb23b1 100644 --- a/sw/device/tests/BUILD +++ b/sw/device/tests/BUILD @@ -4247,13 +4247,14 @@ 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, "//hw/top_earlgrey:sim_dv": None, }, deps = [ + ":sysrst_ctrl_lib", "//hw/top_earlgrey/sw/autogen:top_earlgrey", "//sw/device/lib/base:mmio", "//sw/device/lib/dif:pinmux", @@ -4263,6 +4264,7 @@ opentitan_test( "//sw/device/lib/runtime:log", "//sw/device/lib/testing:rv_plic_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 index b31da5213c6e59..23cee14743b50e 100644 --- a/sw/device/tests/sysrst_ctrl_in_irq_test.c +++ b/sw/device/tests/sysrst_ctrl_in_irq_test.c @@ -12,16 +12,21 @@ #include "sw/device/lib/testing/rv_plic_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 "sw/device/tests/sysrst_ctrl_lib.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, }; @@ -29,13 +34,20 @@ 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. +static volatile const uint8_t kCurrentTestPhaseDV = 0; +// 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 uint8_t kCurrentTestPhaseReal = 0xff; uint8_t phase = 0; enum { kOutputNumPads = 0x8, - kOutputNunMioPads = 0x6, + kOutputNumMioPads = 0x6, }; static const dif_pinmux_index_t kPeripheralInputs[] = { @@ -47,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 = { @@ -75,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 a 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."); @@ -103,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(); } /** @@ -111,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 = { @@ -124,15 +190,21 @@ 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); + // LOG_INFO("wait for host to send the glitch"); + 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."); @@ -151,6 +223,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(); } /** @@ -204,7 +281,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 index 0c0d1a1c58de25..04e86822ac7704 100644 --- 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", + ], +) diff --git a/sw/host/tests/chip/sysrst_ctrl/sysrst_ctrl_in_irq.rs b/sw/host/tests/chip/sysrst_ctrl/sysrst_ctrl_in_irq.rs new file mode 100644 index 00000000000000..9fe35feff0d2c3 --- /dev/null +++ b/sw/host/tests/chip/sysrst_ctrl/sysrst_ctrl_in_irq.rs @@ -0,0 +1,186 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::Result; +use clap::Parser; +use once_cell::sync::Lazy; +use std::collections::HashMap; +use std::fs; +use std::path::PathBuf; +use std::time::Duration; + +use object::{Object, ObjectSymbol}; +use opentitanlib::app::TransportWrapper; +use opentitanlib::io::uart::Uart; +use opentitanlib::test_utils::init::InitializeTest; +use opentitanlib::test_utils::mem::MemWriteReq; +use opentitanlib::test_utils::test_status::TestStatus; +use opentitanlib::uart::console::UartConsole; +use opentitanlib::{collection, execute_test}; + +mod sysrst_ctrl; + +use sysrst_ctrl::{Config, setup_pins, set_pins}; + +#[derive(Debug, Parser)] +struct Opts { + #[command(flatten)] + init: InitializeTest, + + /// Console receive timeout. + #[arg(long, value_parser = humantime::parse_duration, default_value = "10s")] + timeout: Duration, + + /// Path to the firmware's ELF file, for querying symbol addresses. + #[arg(value_name = "FIRMWARE_ELF")] + firmware_elf: PathBuf, +} + +static CONFIG: Lazy> = Lazy::new(|| { + collection! { + "hyper310" => Config { + /* The order of those pins must match the order in the DV's set_pad + * function, that is: power button, key0, key1, key2, AC, EC, WP. */ + pins: vec!["IOR5", "IOR10", "IOR11", "IOR12", "IOR6", "IOR8", "IOR9"], + open_drain: vec![false, false, false, false, false, true, true], + }, + } +}); + +fn sync_with_sw(opts: &Opts, uart: &dyn Uart) -> Result<()> { + UartConsole::wait_for(uart, &TestStatus::InWfi.wait_pattern(), opts.timeout)?; + Ok(()) +} + +struct TestParams<'a> { + opts: &'a Opts, + transport: &'a TransportWrapper, + uart: &'a dyn Uart, + config: &'a Config, + test_phase_addr: u32, +} + +fn set_pads_and_synch( + params: &TestParams, + pad_values_prev: u8, + pad_values_next: u8, + test_phase: &mut u8, +) -> Result<()> { + // The tests expects first a glitch that should not trigger the interrupt. + // Since we cannot generate a short enough glitch, simply do not change the pins + // and check that no interrupt is generated. + log::info!("=============================="); + log::info!("Test with prev={:x}, next={:x}", pad_values_prev, pad_values_next); + + // Set pins. + set_pins(params.transport, params.config, pad_values_prev)?; + // Advance phase to let device side setup the interrupt. + log::info!("Tell device to setup"); + MemWriteReq::execute(params.uart, params.test_phase_addr, &[*test_phase])?; + *test_phase += 1; + // Wait for device to notify that setup is done. + log::info!("Wait for device to finish setup"); + sync_with_sw(params.opts, params.uart)?; + // Here, do NOT generate a glitch. + // Advance phase to let device that we have "generated" the glitch. + log::info!("Tell device to wait for glitch"); + MemWriteReq::execute(params.uart, params.test_phase_addr, &[*test_phase])?; + *test_phase += 1; + // Wait for device to check that no interrupt was generated. + log::info!("Wait device to acknowledge undetected glitch"); + sync_with_sw(params.opts, params.uart)?; + // Change the input. + set_pins(params.transport, params.config, pad_values_next)?; + // Wait for device to acknowledge the interrupt. + log::info!("Wait device to acknowledge to interrupt"); + sync_with_sw(params.opts, params.uart)?; + + Ok(()) +} + +fn chip_sw_sysrst_ctrl_in_irq( + opts: &Opts, + transport: &TransportWrapper, + uart: &dyn Uart, + config: &Config, + test_phase_addr: u32, +) -> Result<()> { + /* Setup transport pins */ + setup_pins(transport, config)?; + + let params = TestParams { + opts, + transport, + uart, + config, + test_phase_addr, + }; + + // Test 7 H2L input transitions. + let mut test_phase = 0; + for i in 0..7 { + set_pads_and_synch(¶ms, 1 << i, 0, &mut test_phase)?; + } + // Test 7 L2H input transitions. + for i in 0..7 { + set_pads_and_synch(¶ms, 0, 1 << i, &mut test_phase)?; + } + + // Test 4 different combo key intr sources with 2, 3, 4 and 5 combo key + // transition H2L. + set_pads_and_synch(¶ms, 0b00000011, 0b00000000, &mut test_phase)?; + + set_pads_and_synch(¶ms, 0b00011100, 0b00000000, &mut test_phase)?; + + set_pads_and_synch(¶ms, 0b00011011, 0b00000000, &mut test_phase)?; + + set_pads_and_synch(¶ms, 0b00011111, 0b00000000, &mut test_phase)?; + + let _ = UartConsole::wait_for(uart, r"PASS!", opts.timeout)?; + Ok(()) +} + +fn main() -> Result<()> { + let opts = Opts::parse(); + opts.init.init_logging(); + let transport = opts.init.init_target()?; + + /* Load the ELF binary and get the address of the `kPhase` variable + * in example_sival.c */ + let elf_binary = fs::read(&opts.firmware_elf)?; + let elf_file = object::File::parse(&*elf_binary)?; + let mut symbols = HashMap::::new(); + for sym in elf_file.symbols() { + symbols.insert(sym.name()?.to_owned(), (sym.address() as u32, sym.size())); + } + let (test_phase_address, test_phase_size) = symbols + .get("kCurrentTestPhaseReal") + .expect("Provided ELF missing 'kCurrentTestPhaseReal' symbol"); + assert_eq!( + *test_phase_size, 1, + "symbol 'kCurrentTestPhaseReal' does not have the expected size" + ); + + let uart = transport.uart("console")?; + uart.set_flow_control(true)?; + let _ = UartConsole::wait_for(&*uart, r"Running [^\r\n]*", opts.timeout)?; + + log::info!( + "Use pin configuration for {:?}", + opts.init.backend_opts.interface + ); + let config = CONFIG + .get(opts.init.backend_opts.interface.as_str()) + .expect("interface"); + + execute_test!( + chip_sw_sysrst_ctrl_in_irq, + &opts, + &transport, + &*uart, + config, + *test_phase_address + ); + Ok(()) +}