[sw] Remove kDeviceStopAddress from device.h
- Removed kDeviceStopAddress since it does the same thing as
kDeviceTestStatusAddress
- Also update kDeviceTestStatusAddress to 0x30000000
- KDeviceLogBypassUartAddress to 0x30000004
- Updated `hart.c` to remove extra writes on abort()
- Updated boot_rom to invoke `abort()` on return from flash
- Added helper function in test_status.c
- Replicated some of the changes from PR #3598
- Fixes #3619
Signed-off-by: Srikrishna Iyer <sriyer@google.com>
diff --git a/sw/device/boot_rom/boot_rom.c b/sw/device/boot_rom/boot_rom.c
index 1b5f17f..ce01d05 100644
--- a/sw/device/boot_rom/boot_rom.c
+++ b/sw/device/boot_rom/boot_rom.c
@@ -9,6 +9,7 @@
#include "sw/device/lib/dif/dif_gpio.h"
#include "sw/device/lib/dif/dif_uart.h"
#include "sw/device/lib/pinmux.h"
+#include "sw/device/lib/runtime/hart.h"
#include "sw/device/lib/runtime/log.h"
#include "sw/device/lib/runtime/print.h"
#include "sw/device/lib/testing/check.h"
@@ -65,12 +66,6 @@
// return.
_flash_header.entry();
- // If there's a stop address, write to it.
- if (kDeviceStopAddress != 0) {
- mmio_region_t end_sim_addr = mmio_region_from_addr(kDeviceStopAddress);
- // We write `0xFFFFFFFE` to differentiate from `abort` (which writes
- // `0xFFFFFFFF`), and the `main` function in the flash image (which is
- // likely to return `0x0` or `0x1` to match unix exit codes).
- mmio_region_write32(end_sim_addr, 0x0, 0xFFFFFFFE);
- }
+ // If the flash image returns, we should abort anyway.
+ abort();
}
diff --git a/sw/device/exts/common/flash_crt.S b/sw/device/exts/common/flash_crt.S
index 6f639aa..4cd7fec 100644
--- a/sw/device/exts/common/flash_crt.S
+++ b/sw/device/exts/common/flash_crt.S
@@ -18,9 +18,9 @@
// is allocated space in ROM by the linker.
.section .crt, "ax", @progbits
+ .extern abort
.extern main
.extern crt_interrupt_vector
- .extern kDeviceStopAddress
/**
* Callable entry point for flash.
@@ -117,13 +117,5 @@
li a1, 0x0 // argv = NULL
call main
- // Load `kDeviceStopAddress` into `t0`. If it is non-zero, write the return
- // value from `main` (in `a0`) into it, otherwise, skip to `wfi` loop.
- lw t0, kDeviceStopAddress
- beqz t0, wfi_loop
- sw a0, 0(t0)
-
- // Loop forever if main somehow returns.
-wfi_loop:
- wfi
- j wfi_loop
+ // Halt the core (wfi loop)
+ tail abort
diff --git a/sw/device/lib/arch/device.h b/sw/device/lib/arch/device.h
index 3cbafde..ade03d8 100644
--- a/sw/device/lib/arch/device.h
+++ b/sw/device/lib/arch/device.h
@@ -76,17 +76,13 @@
extern const uint64_t kUartBaudrate;
/**
- * An address to write to to cause execution to stop.
- *
- * If this is zero, there is no address you can write to to stop execution.
- */
-extern const uintptr_t kDeviceStopAddress;
-
-/**
* An address to write to to report test status.
*
* If this is zero, there is no address to write to to report test status.
*
+ * Depending on the simulation environment and the value written to this
+ * address, the simulation may stop.
+ *
* @see #test_status_set
*/
extern const uintptr_t kDeviceTestStatusAddress;
diff --git a/sw/device/lib/arch/device_fpga_nexysvideo.c b/sw/device/lib/arch/device_fpga_nexysvideo.c
index 9db43c6..3f95219 100644
--- a/sw/device/lib/arch/device_fpga_nexysvideo.c
+++ b/sw/device/lib/arch/device_fpga_nexysvideo.c
@@ -19,9 +19,6 @@
const uint64_t kUartBaudrate = 115200;
-// No Device Stop Address in our FPGA implementation.
-const uintptr_t kDeviceStopAddress = 0;
-
const uintptr_t kDeviceTestStatusAddress = 0;
const uintptr_t kDeviceLogBypassUartAddress = 0;
diff --git a/sw/device/lib/arch/device_sim_dv.c b/sw/device/lib/arch/device_sim_dv.c
index 18c483c..b68485f 100644
--- a/sw/device/lib/arch/device_sim_dv.c
+++ b/sw/device/lib/arch/device_sim_dv.c
@@ -21,9 +21,8 @@
const uint64_t kUartBaudrate = 1 * (1 << 20); // 1Mbps
-// No Device Stop Address in our DV simulator.
-const uintptr_t kDeviceStopAddress = 0;
+// Defined in `hw/top_earlgrey/dv/env/chip_env_pkg.sv`
+const uintptr_t kDeviceTestStatusAddress = 0x30000000;
-const uintptr_t kDeviceTestStatusAddress = 0x1000fff8;
-
-const uintptr_t kDeviceLogBypassUartAddress = 0x1000fffc;
+// Defined in `hw/top_earlgrey/dv/env/chip_env_pkg.sv`
+const uintptr_t kDeviceLogBypassUartAddress = 0x30000004;
diff --git a/sw/device/lib/arch/device_sim_verilator.c b/sw/device/lib/arch/device_sim_verilator.c
index 5f13e63..0f30c6d 100644
--- a/sw/device/lib/arch/device_sim_verilator.c
+++ b/sw/device/lib/arch/device_sim_verilator.c
@@ -20,8 +20,6 @@
const uint64_t kUartBaudrate = 7200;
// Defined in `hw/top_earlgrey/top_earlgrey_verilator.core`
-const uintptr_t kDeviceStopAddress = 0x1000fff4;
-
-const uintptr_t kDeviceTestStatusAddress = 0;
+const uintptr_t kDeviceTestStatusAddress = 0x30000000;
const uintptr_t kDeviceLogBypassUartAddress = 0;
diff --git a/sw/device/lib/runtime/hart.c b/sw/device/lib/runtime/hart.c
index b5d1b55..6c344f7 100644
--- a/sw/device/lib/runtime/hart.c
+++ b/sw/device/lib/runtime/hart.c
@@ -7,7 +7,6 @@
#include <stdbool.h>
#include "sw/device/lib/arch/device.h"
-#include "sw/device/lib/base/mmio.h"
#include "sw/device/lib/runtime/ibex.h"
extern void wait_for_interrupt(void);
@@ -20,14 +19,6 @@
}
noreturn void abort(void) {
- if (kDeviceStopAddress != 0) {
- mmio_region_t end_sim_addr = mmio_region_from_addr(kDeviceStopAddress);
- // Write all ones to the address, to signal that it was `abort`. The
- // simulator doesn't do anything with the info, but it's good to
- // differentiate from likely `main` return codes (0, 1).
- mmio_region_write32(end_sim_addr, 0x0, 0xFFFFFFFF);
- }
-
while (true) {
wait_for_interrupt();
}
diff --git a/sw/device/lib/testing/test_status.c b/sw/device/lib/testing/test_status.c
index 0b37b60..9f87753 100644
--- a/sw/device/lib/testing/test_status.c
+++ b/sw/device/lib/testing/test_status.c
@@ -9,26 +9,35 @@
#include "sw/device/lib/runtime/hart.h"
#include "sw/device/lib/runtime/log.h"
-void test_status_set(test_status_t test_status) {
+/**
+ * Writes the test status to the test status device address.
+ *
+ * @param test_status current status of the test.
+ */
+static void test_status_device_write(test_status_t test_status) {
if (kDeviceTestStatusAddress != 0) {
- mmio_region_t sw_dv_test_status_addr =
+ mmio_region_t test_status_device_addr =
mmio_region_from_addr(kDeviceTestStatusAddress);
- mmio_region_write32(sw_dv_test_status_addr, 0x0, (uint32_t)test_status);
+ mmio_region_write32(test_status_device_addr, 0x0, (uint32_t)test_status);
}
+}
+void test_status_set(test_status_t test_status) {
switch (test_status) {
case kTestStatusPassed: {
LOG_INFO("PASS!");
+ test_status_device_write(test_status);
abort();
break;
}
case kTestStatusFailed: {
LOG_INFO("FAIL!");
+ test_status_device_write(test_status);
abort();
break;
}
default: {
- // Do nothing.
+ test_status_device_write(test_status);
break;
}
}
diff --git a/sw/device/lib/testing/test_status.h b/sw/device/lib/testing/test_status.h
index 3aa6b84..04723eb 100644
--- a/sw/device/lib/testing/test_status.h
+++ b/sw/device/lib/testing/test_status.h
@@ -19,46 +19,58 @@
typedef enum test_status {
/**
* Indicates that the CPU has started executing the boot_rom code.
+ *
+ * Writing this value to #kDeviceTestStatusAddress must not stop simulation of
+ * the current device.
*/
kTestStatusInBootRom = 0xb090, // 'bogo', BOotrom GO
/**
* Indicates that the CPU has started executing the test code.
+ *
+ * Writing this value to #kDeviceTestStatusAddress must not stop simulation of
+ * the current device.
*/
kTestStatusInTest = 0x4354, // 'test'
/**
* Indicates that the CPU is in the WFI state.
+ *
+ * Writing this value to #kDeviceTestStatusAddress must not stop simulation of
+ * the current device.
*/
kTestStatusInWfi = 0x1d1e, // 'idle'
/**
* This indicates that the test has passed. This is a terminal state. Any code
* appearing after this value is set is unreachable.
+ *
+ * Writing this value to #kDeviceTestStatusAddress may stop simulation of the
+ * current device.
*/
kTestStatusPassed = 0x900d, // 'good'
/**
* This indicates that the test has failed. This is a terminal state. Any code
* appearing after this value is set is unreachable.
+ *
+ * Writing this value to #kDeviceTestStatusAddress may stop simulation of the
+ * current device.
*/
kTestStatusFailed = 0xbaad // 'baad'
} test_status_t;
/**
- * Sets the test status.
- *
* This signals the status of the software test with `test_status` value.
- * In DV testing, this function writes the `test_status` to a specific
- * location in the RAM. In non-DV testing, this converts to a log message for
- * the terminal states. For the other intermediates states, the function returns
- * immediately.
*
- * At minimum, it is mandatory to set the status of the software to explicitly
- * indicate whether the software test passed or failed, with
- * `kTestStatusPassed` and `kTestStatusFailed` literals. If one these terminal
- * status is passed as the argument, the function calls `abort()` to ensure that
- * the core stops executing anymore.
+ * It is mandatory for tests to indicate whether they passed or failed using
+ * this function with #kTestStatusPassed and #kTestStatusFailed.
+ *
+ * In simulated testing (Verilator, DV), this function writes `test_status` to
+ * a specific address, which may cause the simulation to end.
+ *
+ * In environments with a null #kDeviceTestStatusAddress, this logs a message
+ * for terminal states and calls abort. Otherwise, the function returns safely.
*
* @param test_status current status of the test.
*/
diff --git a/sw/device/mask_rom/mask_rom_start.S b/sw/device/mask_rom/mask_rom_start.S
index f4a65f4..f473da2 100644
--- a/sw/device/mask_rom/mask_rom_start.S
+++ b/sw/device/mask_rom/mask_rom_start.S
@@ -205,9 +205,7 @@
// `t0` is the address to start zeroing at.
// `t1` is the address to stop zeroing at.
li t0, TOP_EARLGREY_RAM_MAIN_BASE_ADDR
- // TODO: Ensure we do zero all of RAM, for the moment we're not zeroing the
- // test area. https://github.com/lowRISC/opentitan/issues/3619
- la t1, _test_reserved_start
+ li t1, (TOP_EARLGREY_RAM_MAIN_BASE_ADDR + TOP_EARLGREY_RAM_MAIN_SIZE_BYTES)
bgeu t0, t1, sram_zero_end
sram_zero_loop:
// TODO: Unroll loop
