[ottf,handler] Deprecate default ISRs and replace with OTTF ISRs.
In #9335, OTTF ISRs were separated into their own build target so that
only one set of default test ISRs would need to be maintained.
Specifically, device code can make use of the OTTF ISRs even WITHOUT the
OTTF being present.
This commit migrates all remaining device SW from the old default IRQ
handlers (i.e., ISRs) to the new default OTTF ISRs, and deprecates the
old handlers.
This partially addresses a task in #8015.
Signed-off-by: Timothy Trippel <ttrippel@google.com>
diff --git a/hw/top_englishbreakfast/util/sw_sources.patch b/hw/top_englishbreakfast/util/sw_sources.patch
index ad349de..bec9a48 100644
--- a/hw/top_englishbreakfast/util/sw_sources.patch
+++ b/hw/top_englishbreakfast/util/sw_sources.patch
@@ -75,7 +75,7 @@
simple_serial_process_packet();
}
diff --git a/sw/device/sca/lib/meson.build b/sw/device/sca/lib/meson.build
-index a6ce4d8af..8a107194c 100644
+index c4a65f7e0..3f6ee67f1 100644
--- a/sw/device/sca/lib/meson.build
+++ b/sw/device/sca/lib/meson.build
@@ -17,8 +17,6 @@ sw_sca_lib_sca = declare_dependency(
@@ -85,10 +85,10 @@
- sw_lib_dif_csrng,
- sw_lib_dif_edn,
sw_lib_irq,
- sw_lib_irq_handlers,
+ sw_lib_testing_ottf_isrs,
sw_lib_mmio,
diff --git a/sw/device/sca/lib/sca.c b/sw/device/sca/lib/sca.c
-index 0336455da..a46911701 100644
+index 3c0c86f26..7354aa9cb 100644
--- a/sw/device/sca/lib/sca.c
+++ b/sw/device/sca/lib/sca.c
@@ -7,8 +7,6 @@
@@ -154,9 +154,9 @@
-}
-
/**
- * Timer IRQ handler.
+ * Override default Timer ISR.
*
-@@ -172,20 +143,6 @@ void handler_irq_timer(void) {
+@@ -172,20 +143,6 @@ void ottf_timer_isr(void) {
* @param disable Set of peripherals to disable.
*/
void sca_disable_peripherals(sca_peripherals_t disable) {
@@ -227,7 +227,7 @@
CHECK_DIF_OK(
dif_aes_init(mmio_region_from_addr(TOP_EARLGREY_AES_BASE_ADDR), &aes));
diff --git a/sw/device/tests/meson.build b/sw/device/tests/meson.build
-index 62112f435..89d3d230c 100644
+index 2df371008..d9ab1416f 100644
--- a/sw/device/tests/meson.build
+++ b/sw/device/tests/meson.build
@@ -259,7 +259,6 @@ aes_smoketest_lib = declare_dependency(
diff --git a/sw/device/benchmarks/coremark/meson.build b/sw/device/benchmarks/coremark/meson.build
index b3cce1c..92f236e 100644
--- a/sw/device/benchmarks/coremark/meson.build
+++ b/sw/device/benchmarks/coremark/meson.build
@@ -21,8 +21,8 @@
sw_lib_dif_uart,
sw_lib_mem,
riscv_crt,
- sw_lib_irq_handlers,
device_lib,
+ sw_lib_testing_ottf_isrs,
sw_lib_testing_test_status,
],
# Set up coremark-specific defines.
diff --git a/sw/device/boot_rom/BUILD b/sw/device/boot_rom/BUILD
index 47f48b2..1655d84 100644
--- a/sw/device/boot_rom/BUILD
+++ b/sw/device/boot_rom/BUILD
@@ -89,10 +89,10 @@
],
deps = [
"//sw/device/exts/common",
- "//sw/device/lib:irq",
"//sw/device/lib/arch:device",
"//sw/device/lib/crt",
"//sw/device/lib/testing/test_framework",
"//sw/device/lib/testing/test_framework:main",
+ "//sw/device/lib/testing/test_framework:ottf_isrs",
],
)
diff --git a/sw/device/examples/hello_usbdev/meson.build b/sw/device/examples/hello_usbdev/meson.build
index 750ed4f..0410651 100644
--- a/sw/device/examples/hello_usbdev/meson.build
+++ b/sw/device/examples/hello_usbdev/meson.build
@@ -21,8 +21,8 @@
sw_lib_dif_uart,
sw_lib_usb,
riscv_crt,
- sw_lib_irq_handlers,
device_lib,
+ sw_lib_testing_ottf_isrs,
sw_lib_testing_test_status,
],
)
diff --git a/sw/device/examples/hello_world/BUILD b/sw/device/examples/hello_world/BUILD
index c48807f..19e55f0 100644
--- a/sw/device/examples/hello_world/BUILD
+++ b/sw/device/examples/hello_world/BUILD
@@ -28,7 +28,6 @@
"//hw/top_earlgrey/sw/autogen:top_earlgrey",
"//sw/device/examples:demos",
"//sw/device/exts/common",
- "//sw/device/lib:irq",
"//sw/device/lib:pinmux",
"//sw/device/lib/arch:device",
"//sw/device/lib/crt",
@@ -39,5 +38,6 @@
"//sw/device/lib/runtime:log",
"//sw/device/lib/runtime:print",
"//sw/device/lib/testing/test_framework",
+ "//sw/device/lib/testing/test_framework:ottf_isrs",
],
)
diff --git a/sw/device/examples/hello_world/meson.build b/sw/device/examples/hello_world/meson.build
index 903a581..c9f6212 100644
--- a/sw/device/examples/hello_world/meson.build
+++ b/sw/device/examples/hello_world/meson.build
@@ -18,8 +18,8 @@
sw_lib_dif_spi_device,
sw_lib_dif_uart,
riscv_crt,
- sw_lib_irq_handlers,
device_lib,
+ sw_lib_testing_ottf_isrs,
sw_lib_testing_test_status,
],
)
diff --git a/sw/device/exts/common/ibex_interrupt_vectors.S b/sw/device/exts/common/ibex_interrupt_vectors.S
index 40ff90b..1dbefe2 100644
--- a/sw/device/exts/common/ibex_interrupt_vectors.S
+++ b/sw/device/exts/common/ibex_interrupt_vectors.S
@@ -7,7 +7,8 @@
* This file contains Ibex-specific interrupt vectors.
*/
- // These functions are declared in `sw/device/lib/handler.h`.
+ // These symbols are declared in
+ // `sw/device/lib/testing/test_framework/ottf_isrs.S`.
.extern handler_exception
.extern handler_irq_software
.extern handler_irq_timer
@@ -28,9 +29,9 @@
* `crt_interrupt_vector` is the CRT-loaded interrupt vector for Ibex.
*
* It contains entries that point at the functions declared in
- * `sw/device/lib/handler.h`. If you are getting link errors for these symbols,
- * then it's likely you have forgotten to add `sw_lib_irq_handlers` as a
- * dependency for your executable.
+ * `sw/device/lib/testing/test_framework/ottf_isrs.S`. If you are getting link
+ * errors for these symbols, then it's likely you have forgotten to add
+ * `ottf_isrs` as a dependency for your executable.
*
* Interrupt vectors in Ibex have 32 entries for 32 possible interrupts. The
* vector must be 256-byte aligned, as Ibex's vectoring mechanism requires that.
diff --git a/sw/device/exts/common/meson.build b/sw/device/exts/common/meson.build
index dad84f0..a772d5a 100644
--- a/sw/device/exts/common/meson.build
+++ b/sw/device/exts/common/meson.build
@@ -25,7 +25,7 @@
# RISC-V startup library. Every RISC-V executable should depend on this target.
#
# The crt defines the interrupt vector, so requires definitions for all the
-# declarations in `sw/device/lib/handler.h`.
+# declarations in `sw/device/lib/testing/test_framework/ottf_isrs.S`.
riscv_crt = declare_dependency(
link_args: riscv_linker_args,
# The following assembly files need to be included as a source, not a
diff --git a/sw/device/lib/BUILD b/sw/device/lib/BUILD
index 9faade5..6e3aca2 100644
--- a/sw/device/lib/BUILD
+++ b/sw/device/lib/BUILD
@@ -20,24 +20,6 @@
)
cc_library(
- name = "irq",
- srcs = [
- "handler.c",
- "irq.c",
- ],
- hdrs = [
- "handler.h",
- "irq.h",
- ],
- target_compatible_with = [OPENTITAN_CPU],
- deps = [
- "//hw/top_earlgrey/sw/autogen:top_earlgrey",
- "//sw/device/lib/base",
- "//sw/device/lib/runtime:log",
- ],
-)
-
-cc_library(
name = "pinmux",
srcs = ["pinmux.c"],
hdrs = ["pinmux.h"],
diff --git a/sw/device/lib/handler.c b/sw/device/lib/handler.c
deleted file mode 100644
index cb5996b..0000000
--- a/sw/device/lib/handler.c
+++ /dev/null
@@ -1,113 +0,0 @@
-// Copyright lowRISC contributors.
-// Licensed under the Apache License, Version 2.0, see LICENSE for details.
-// SPDX-License-Identifier: Apache-2.0
-
-#include "sw/device/lib/handler.h"
-
-#include "sw/device/lib/base/csr.h"
-#include "sw/device/lib/base/stdasm.h"
-#include "sw/device/lib/runtime/log.h"
-
-/**
- * Return value of mtval
- */
-static uint32_t get_mtval(void) {
- uint32_t mtval;
- CSR_READ(CSR_REG_MTVAL, &mtval);
- return mtval;
-}
-
-/**
- * Default Error Handling
- * @param msg error message supplied by caller
- * TODO - this will be soon by a real print formatting
- */
-static void print_exc_msg(const char *msg) {
- LOG_INFO("%s", msg);
- LOG_INFO("MTVAL value is 0x%x", get_mtval());
- while (1) {
- };
-}
-
-// Below functions are default weak exception handlers meant to be overriden
-__attribute__((weak)) void handler_exception(void) {
- uint32_t mcause;
- exc_id_t exc_cause;
-
- CSR_READ(CSR_REG_MCAUSE, &mcause);
- exc_cause = (exc_id_t)(mcause & kIdMax);
-
- switch (exc_cause) {
- case kInstMisa:
- handler_instr_acc_fault();
- break;
- case kInstAccFault:
- handler_instr_acc_fault();
- break;
- case kInstIllegalFault:
- handler_instr_ill_fault();
- break;
- case kBkpt:
- handler_bkpt();
- break;
- case kLoadAccFault:
- handler_lsu_fault();
- break;
- case kStrAccFault:
- handler_lsu_fault();
- break;
- case kECall:
- handler_ecall();
- break;
- default:
- while (1) {
- };
- }
-}
-
-__attribute__((weak)) void handler_irq_software(void) {
- LOG_INFO("Software IRQ triggered!");
- while (1) {
- }
-}
-
-__attribute__((weak)) void handler_irq_timer(void) {
- LOG_INFO("Timer IRQ triggered!");
- while (1) {
- }
-}
-
-__attribute__((weak)) void handler_irq_external(void) {
- LOG_INFO("External IRQ triggered!");
- while (1) {
- }
-}
-
-__attribute__((weak)) void handler_instr_acc_fault(void) {
- const char fault_msg[] =
- "Instruction access fault, mtval shows fault address";
- print_exc_msg(fault_msg);
-}
-
-__attribute__((weak)) void handler_instr_ill_fault(void) {
- const char fault_msg[] =
- "Illegal Instruction fault, mtval shows instruction content";
- print_exc_msg(fault_msg);
-}
-
-__attribute__((weak)) void handler_bkpt(void) {
- const char exc_msg[] =
- "Breakpoint triggerd, mtval shows the breakpoint address";
- print_exc_msg(exc_msg);
-}
-
-__attribute__((weak)) void handler_lsu_fault(void) {
- const char exc_msg[] = "Load/Store fault, mtval shows the fault address";
- print_exc_msg(exc_msg);
-}
-
-__attribute__((weak)) void handler_ecall(void) {
- LOG_INFO("Environment call encountered");
- while (1) {
- }
-}
diff --git a/sw/device/lib/handler.h b/sw/device/lib/handler.h
deleted file mode 100644
index 014b4d3..0000000
--- a/sw/device/lib/handler.h
+++ /dev/null
@@ -1,129 +0,0 @@
-// Copyright lowRISC contributors.
-// Licensed under the Apache License, Version 2.0, see LICENSE for details.
-// SPDX-License-Identifier: Apache-2.0
-
-#ifndef OPENTITAN_SW_DEVICE_LIB_HANDLER_H_
-#define OPENTITAN_SW_DEVICE_LIB_HANDLER_H_
-
-typedef enum exc_id {
- kInstMisa = 0,
- kInstAccFault = 1,
- kInstIllegalFault = 2,
- kBkpt = 3,
- kLoadAccFault = 5,
- kStrAccFault = 7,
- kECall = 11,
- kIdMax = 31
-} exc_id_t;
-
-// The RISC-V interrupt vector will not include the addresses of the handlers,
-// instead, it includes (uncompressed) instructions. Thus the interrupt vector
-// will include `j <interrupt handler name>` for each handler.
-//
-// The only requirement on the symbol in the jump is that it must be correctly
-// aligned. If the processor supports the C extension, this can be 2-byte
-// aligned, but 4-byte aligned is compatible with all RISC-V processors.
-//
-// If the processor is not using interrupt vectoring, then there will be a
-// single address where interrupts jump to, which will either contain a function
-// (which will need to be aligned), or will contain a jump to a function, again
-// which will need to be aligned.
-//
-// You only need to use this ABI for handlers that are the first function called
-// in an interrupt handler. Subsequent functions can just use the regular RISC-V
-// calling convention.
-#define INTERRUPT_HANDLER_ABI __attribute__((aligned(4), interrupt))
-
-// The following `handler_*` functions have weak definitions, provided by
-// `handler.c`. This weak definition can be overriden at link-time by providing
-// an additional non-weak definition of each function. Executables and libraries
-// must not contain more than one weak definition of the same symbol.
-
-/**
- * Default exception handler.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-INTERRUPT_HANDLER_ABI void handler_exception(void);
-
-/**
- * SW IRQ handler.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-INTERRUPT_HANDLER_ABI void handler_irq_software(void);
-
-/**
- * Timer IRQ handler.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-INTERRUPT_HANDLER_ABI void handler_irq_timer(void);
-
-/**
- * external IRQ handler.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-INTERRUPT_HANDLER_ABI void handler_irq_external(void);
-
-/**
- * Instruction access fault.
- *
- * Called by default implementation of `handler_exception`. If that function is
- * overriden, this function may not be called.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-void handler_instr_acc_fault(void);
-
-/**
- * Illegal Instruction fault.
- *
- * Called by default implementation of `handler_exception`. If that function is
- * overriden, this function may not be called.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-void handler_instr_ill_fault(void);
-
-/**
- * Breakpoint handler.
- *
- * Called by default implementation of `handler_exception`. If that function is
- * overriden, this function may not be called.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-void handler_bkpt(void);
-
-/**
- * Load store unit fault.
- *
- * Called by default implementation of `handler_exception`. If that function is
- * overriden, this function may not be called.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-void handler_lsu_fault(void);
-
-/**
- * Exception call handler.
- *
- * Called by default implementation of `handler_exception`. If that function is
- * overriden, this function may not be called.
- *
- * `handler.c` provides a weak definition of this symbol, which can be overriden
- * at link-time by providing an additional non-weak definition.
- */
-void handler_ecall(void);
-
-#endif // OPENTITAN_SW_DEVICE_LIB_HANDLER_H_
diff --git a/sw/device/lib/meson.build b/sw/device/lib/meson.build
index ec154b1..bc9c9a8 100644
--- a/sw/device/lib/meson.build
+++ b/sw/device/lib/meson.build
@@ -60,28 +60,6 @@
)
)
-# IRQ Handlers Library
-#
-# handler.c contains various definitions with weak linkage, for interrupt
-# handler symbols referenced from riscv_crt.
-#
-# To override default handlers, implement the declarations in handler.h, and
-# link to this library to pick up the other default definitions, if needed.
-#
-# If you are getting errors for functions beginning `handler_` when linking a
-# device executable, it's likely because you forgot to link this library.
-sw_lib_irq_handlers = declare_dependency(
- link_with: static_library(
- 'irq_default_handlers_ot',
- sources: [
- 'handler.c',
- ],
- dependencies: [
- sw_lib_runtime_log,
- ],
- )
-)
-
# USB DEV library (sw_lib_usb)
sw_lib_usb = declare_dependency(
link_with: static_library(
diff --git a/sw/device/lib/testing/test_framework/BUILD b/sw/device/lib/testing/test_framework/BUILD
index da9e5b6..6dc555c 100644
--- a/sw/device/lib/testing/test_framework/BUILD
+++ b/sw/device/lib/testing/test_framework/BUILD
@@ -20,7 +20,7 @@
],
)
-# TODO use more specific targets to avoid unnecesary dependencies
+# TODO use more specific targets to avoid unnecessary dependencies
# https://github.com/lowRISC/opentitan/issues/9098
cc_library(
name = "test_framework",
@@ -44,3 +44,22 @@
"//sw/device/lib/runtime:print",
],
)
+
+cc_library(
+ name = "ottf_isrs",
+ srcs = [
+ "ottf_isrs.S",
+ "ottf_isrs.c",
+ ],
+ hdrs = [
+ "ottf_isrs.h",
+ "ottf_macros.h",
+ ],
+ target_compatible_with = [OPENTITAN_CPU],
+ deps = [
+ "//sw/device/lib/base",
+ "//sw/device/lib/runtime:hart",
+ "//sw/device/lib/runtime:ibex",
+ "//sw/device/lib/runtime:log",
+ ],
+)
diff --git a/sw/device/lib/testing/test_framework/ottf_isrs.c b/sw/device/lib/testing/test_framework/ottf_isrs.c
index 716a8b3..f378c46 100644
--- a/sw/device/lib/testing/test_framework/ottf_isrs.c
+++ b/sw/device/lib/testing/test_framework/ottf_isrs.c
@@ -16,8 +16,8 @@
* ISRs for testing, while also enabling writing tests that do not make use of
* the OTTF. See the `crt_test` in `sw/device/tests/crt_test.c` for an example.
*/
-OT_ATTR_WEAK uintptr_t pxCurrentTCB = (uintptr_t)NULL;
-OT_ATTR_WEAK uintptr_t kTestConfig = (uintptr_t)NULL;
+OT_ATTR_WEAK void *pxCurrentTCB = NULL;
+OT_ATTR_WEAK void *kTestConfig = NULL;
OT_ATTR_WEAK void ottf_exception_handler(void) {
uint32_t mcause = ibex_mcause_read();
diff --git a/sw/device/riscv_compliance_support/meson.build b/sw/device/riscv_compliance_support/meson.build
index 1e236ba..514adf1 100644
--- a/sw/device/riscv_compliance_support/meson.build
+++ b/sw/device/riscv_compliance_support/meson.build
@@ -14,8 +14,8 @@
dependencies: [
sw_lib_runtime_print,
sw_lib_runtime_log,
- sw_lib_irq_handlers,
sw_lib_mem,
+ sw_lib_testing_ottf_isrs,
sw_lib_testing_test_status,
device_lib,
],
diff --git a/sw/device/sca/lib/meson.build b/sw/device/sca/lib/meson.build
index a6ce4d8..c4a65f7 100644
--- a/sw/device/sca/lib/meson.build
+++ b/sw/device/sca/lib/meson.build
@@ -20,7 +20,7 @@
sw_lib_dif_csrng,
sw_lib_dif_edn,
sw_lib_irq,
- sw_lib_irq_handlers,
+ sw_lib_testing_ottf_isrs,
sw_lib_mmio,
sw_lib_pinmux,
sw_lib_runtime_log,
diff --git a/sw/device/sca/lib/sca.c b/sw/device/sca/lib/sca.c
index 0336455..3c0c86f 100644
--- a/sw/device/sca/lib/sca.c
+++ b/sw/device/sca/lib/sca.c
@@ -13,11 +13,11 @@
#include "sw/device/lib/dif/dif_gpio.h"
#include "sw/device/lib/dif/dif_rv_timer.h"
#include "sw/device/lib/dif/dif_uart.h"
-#include "sw/device/lib/handler.h"
#include "sw/device/lib/irq.h"
#include "sw/device/lib/pinmux.h"
#include "sw/device/lib/runtime/hart.h"
#include "sw/device/lib/runtime/print.h"
+#include "sw/device/lib/testing/test_framework/ottf_isrs.h"
#include "clkmgr_regs.h" // Generated
#include "csrng_regs.h" // Generated
@@ -147,11 +147,11 @@
}
/**
- * Timer IRQ handler.
+ * Override default Timer ISR.
*
* Disables the counter and clears pending interrupts.
*/
-void handler_irq_timer(void) {
+void ottf_timer_isr(void) {
// Return values of below functions are ignored to improve capture
// performance.
IGNORE_RESULT(dif_rv_timer_counter_set_enabled(&timer, kRvTimerHart,
