On Fri, Jun 27, 2025 at 6:02 AM Andrew Jones andrew.jones@linux.dev wrote:
On Wed, Jun 18, 2025 at 08:44:52AM -0700, Jesse Taube wrote:
Add tests for the DBTR SBI extension.
Signed-off-by: Jesse Taube jesse@rivosinc.com Reviewed-by: Charlie Jenkins charlie@rivosinc.com Tested-by: Charlie Jenkins charlie@rivosinc.com
V1 -> V2:
- Call report_prefix_pop before returning
- Disable compressed instructions in exec_call, update related comment
- Remove extra "| 1" in dbtr_test_load
- Remove extra newlines
- Remove extra tabs in check_exec
- Remove typedefs from enums
- Return when dbtr_install_trigger fails
- s/avalible/available/g
- s/unistall/uninstall/g
V2 -> V3:
- Change SBI_DBTR_SHMEM_INVALID_ADDR to -1UL
- Move all dbtr functions to sbi-dbtr.c
- Move INSN_LEN to processor.h
- Update include list
- Use C-style comments
V3 -> V4:
- Include libcflat.h
- Remove #define SBI_DBTR_SHMEM_INVALID_ADDR
V4 -> V5:
- Sort includes
- Add kfail for update triggers
V5 -> V6:
- Add assert in gen_tdata1
- Add prefix to dbtr_test_type
- Add TRIG_STATE_DMODE
- Add TRIG_STATE_RESERVED
- Align function paramaters with opening parenthesis
- Change OpenSBI < v1.7 to < v1.5
- Constantly use spaces in prefix rather than _
- Export split_phys_addr
- Fix MCONTROL_U and MCONTROL_M mix up
- Fix swapped VU and VS
- Move /* to own line
- Print type in dbtr_test_type
- Remove _BIT suffix from macros
- Remove duplicate MODE_S
- Remove spaces before include
- Rename tdata1,2 to trigger and control in dbtr_install_trigger
- Report skip in dbtr_test_multiple
- Report variables in info not pass or fail
- s/save/store/g
- sbi_debug_set_shmem use split_phys_addr
- Use if (!report(... in dbtr_test_disable_enable
lib/riscv/asm/sbi.h | 1 + riscv/Makefile | 1 + riscv/sbi-dbtr.c | 839 ++++++++++++++++++++++++++++++++++++++++++++ riscv/sbi-tests.h | 2 + riscv/sbi.c | 3 +- 5 files changed, 845 insertions(+), 1 deletion(-) create mode 100644 riscv/sbi-dbtr.c
diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h index a5738a5c..78fd6e2a 100644 --- a/lib/riscv/asm/sbi.h +++ b/lib/riscv/asm/sbi.h @@ -51,6 +51,7 @@ enum sbi_ext_id { SBI_EXT_SUSP = 0x53555350, SBI_EXT_FWFT = 0x46574654, SBI_EXT_SSE = 0x535345,
SBI_EXT_DBTR = 0x44425452,};
enum sbi_ext_base_fid { diff --git a/riscv/Makefile b/riscv/Makefile index 11e68eae..55c7ac93 100644 --- a/riscv/Makefile +++ b/riscv/Makefile @@ -20,6 +20,7 @@ all: $(tests) $(TEST_DIR)/sbi-deps += $(TEST_DIR)/sbi-asm.o $(TEST_DIR)/sbi-deps += $(TEST_DIR)/sbi-fwft.o $(TEST_DIR)/sbi-deps += $(TEST_DIR)/sbi-sse.o +$(TEST_DIR)/sbi-deps += $(TEST_DIR)/sbi-dbtr.o
My OCD says this should come after the sbi-asm to be alphabetical.
all_deps += $($(TEST_DIR)/sbi-deps)
diff --git a/riscv/sbi-dbtr.c b/riscv/sbi-dbtr.c new file mode 100644 index 00000000..7837553f --- /dev/null +++ b/riscv/sbi-dbtr.c @@ -0,0 +1,839 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- SBI DBTR testsuite
- Copyright (C) 2025, Rivos Inc., Jesse Taube jesse@rivosinc.com
- */
+#include <libcflat.h> +#include <bitops.h>
+#include <asm/io.h> +#include <asm/processor.h>
+#include "sbi-tests.h"
+#define RV_MAX_TRIGGERS 32
+#define SBI_DBTR_TRIG_STATE_MAPPED BIT(0) +#define SBI_DBTR_TRIG_STATE_U BIT(1) +#define SBI_DBTR_TRIG_STATE_S BIT(2) +#define SBI_DBTR_TRIG_STATE_VU BIT(3) +#define SBI_DBTR_TRIG_STATE_VS BIT(4) +#define SBI_DBTR_TRIG_STATE_HAVE_HW_TRIG BIT(5) +#define SBI_DBTR_TRIG_STATE_RESERVED GENMASK(7, 6)
+#define SBI_DBTR_TRIG_STATE_HW_TRIG_IDX_SHIFT 8 +#define SBI_DBTR_TRIG_STATE_HW_TRIG_IDX(trig_state) (trig_state >> SBI_DBTR_TRIG_STATE_HW_TRIG_IDX_SHIFT)
+#define SBI_DBTR_TDATA1_TYPE_SHIFT (__riscv_xlen - 4) +#define SBI_DBTR_TDATA1_DMODE BIT_UL(__riscv_xlen - 5)
+#define SBI_DBTR_TDATA1_MCONTROL6_LOAD BIT(0) +#define SBI_DBTR_TDATA1_MCONTROL6_STORE BIT(1) +#define SBI_DBTR_TDATA1_MCONTROL6_EXECUTE BIT(2) +#define SBI_DBTR_TDATA1_MCONTROL6_U BIT(3) +#define SBI_DBTR_TDATA1_MCONTROL6_S BIT(4) +#define SBI_DBTR_TDATA1_MCONTROL6_M BIT(6) +#define SBI_DBTR_TDATA1_MCONTROL6_SELECT BIT(21) +#define SBI_DBTR_TDATA1_MCONTROL6_VU BIT(23) +#define SBI_DBTR_TDATA1_MCONTROL6_VS BIT(24)
+#define SBI_DBTR_TDATA1_MCONTROL_LOAD BIT(0) +#define SBI_DBTR_TDATA1_MCONTROL_STORE BIT(1) +#define SBI_DBTR_TDATA1_MCONTROL_EXECUTE BIT(2) +#define SBI_DBTR_TDATA1_MCONTROL_U BIT(3) +#define SBI_DBTR_TDATA1_MCONTROL_S BIT(4) +#define SBI_DBTR_TDATA1_MCONTROL_M BIT(6) +#define SBI_DBTR_TDATA1_MCONTROL_SELECT BIT(19)
+enum McontrolType {
SBI_DBTR_TDATA1_TYPE_NONE = (0UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_LEGACY = (1UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_MCONTROL = (2UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_ICOUNT = (3UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_ITRIGGER = (4UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_ETRIGGER = (5UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_MCONTROL6 = (6UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_TMEXTTRIGGER = (7UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_RESERVED0 = (8UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_RESERVED1 = (9UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_RESERVED2 = (10UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_RESERVED3 = (11UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_CUSTOM0 = (12UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_CUSTOM1 = (13UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_CUSTOM2 = (14UL << SBI_DBTR_TDATA1_TYPE_SHIFT),SBI_DBTR_TDATA1_TYPE_DISABLED = (15UL << SBI_DBTR_TDATA1_TYPE_SHIFT),+};
+enum Tdata1Value {
VALUE_NONE = 0,VALUE_LOAD = BIT(0),VALUE_STORE = BIT(1),VALUE_EXECUTE = BIT(2),+};
+enum Tdata1Mode {
MODE_NONE = 0,MODE_M = BIT(0),MODE_U = BIT(1),MODE_S = BIT(2),MODE_VU = BIT(3),MODE_VS = BIT(4),+};
+enum sbi_ext_dbtr_fid {
SBI_EXT_DBTR_NUM_TRIGGERS = 0,SBI_EXT_DBTR_SETUP_SHMEM,SBI_EXT_DBTR_TRIGGER_READ,SBI_EXT_DBTR_TRIGGER_INSTALL,SBI_EXT_DBTR_TRIGGER_UPDATE,SBI_EXT_DBTR_TRIGGER_UNINSTALL,SBI_EXT_DBTR_TRIGGER_ENABLE,SBI_EXT_DBTR_TRIGGER_DISABLE,+};
+struct sbi_dbtr_data_msg {
unsigned long tstate;unsigned long tdata1;unsigned long tdata2;unsigned long tdata3;+};
+struct sbi_dbtr_id_msg {
unsigned long idx;+};
+/* SBI shared mem messages layout */ +struct sbi_dbtr_shmem_entry {
union {struct sbi_dbtr_data_msg data;struct sbi_dbtr_id_msg id;};+};
+static bool dbtr_handled;
+/* Expected to be leaf function as not to disrupt frame-pointer */ +static __attribute__((naked)) void exec_call(void) +{
/* skip over nop when triggered instead of ret. */asm volatile (".option push\n"".option arch, -c\n""nop\n""ret\n"".option pop\n");+}
+static void dbtr_exception_handler(struct pt_regs *regs) +{
dbtr_handled = true;/* Reading *epc may cause a fault, skip over nop */if ((void *)regs->epc == exec_call) {regs->epc += 4;return;}/* WARNING: Skips over the trapped intruction */regs->epc += RV_INSN_LEN(readw((void *)regs->epc));+}
+static bool do_store(void *tdata2) +{
bool ret;writel(0, tdata2);ret = dbtr_handled;dbtr_handled = false;return ret;+}
+static bool do_load(void *tdata2) +{
bool ret;readl(tdata2);ret = dbtr_handled;dbtr_handled = false;return ret;+}
+static bool do_exec(void) +{
bool ret;exec_call();ret = dbtr_handled;dbtr_handled = false;return ret;+}
+static unsigned long gen_tdata1_mcontrol(enum Tdata1Mode mode, enum Tdata1Value value) +{
unsigned long tdata1 = SBI_DBTR_TDATA1_TYPE_MCONTROL;if (value & VALUE_LOAD)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_LOAD;if (value & VALUE_STORE)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_STORE;if (value & VALUE_EXECUTE)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_EXECUTE;if (mode & MODE_M)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_M;if (mode & MODE_U)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_U;if (mode & MODE_S)tdata1 |= SBI_DBTR_TDATA1_MCONTROL_S;return tdata1;+}
+static unsigned long gen_tdata1_mcontrol6(enum Tdata1Mode mode, enum Tdata1Value value) +{
unsigned long tdata1 = SBI_DBTR_TDATA1_TYPE_MCONTROL6;if (value & VALUE_LOAD)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_LOAD;if (value & VALUE_STORE)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_STORE;if (value & VALUE_EXECUTE)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_EXECUTE;if (mode & MODE_M)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_M;if (mode & MODE_U)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_U;if (mode & MODE_S)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_S;if (mode & MODE_VU)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_VU;if (mode & MODE_VS)tdata1 |= SBI_DBTR_TDATA1_MCONTROL6_VS;return tdata1;+}
+static unsigned long gen_tdata1(enum McontrolType type, enum Tdata1Value value, enum Tdata1Mode mode) +{
switch (type) {case SBI_DBTR_TDATA1_TYPE_MCONTROL:return gen_tdata1_mcontrol(mode, value);case SBI_DBTR_TDATA1_TYPE_MCONTROL6:return gen_tdata1_mcontrol6(mode, value);default:assert_msg(false, "Invalid mcontrol type: %lu", type);return 0;Can drop the 'return 0' line now that there's an unconditional assert.
}+}
+static struct sbiret sbi_debug_num_triggers(unsigned long trig_tdata1) +{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS, trig_tdata1, 0, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_set_shmem_raw(unsigned long shmem_phys_lo,
unsigned long shmem_phys_hi,unsigned long flags)+{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM, shmem_phys_lo,shmem_phys_hi, flags, 0, 0, 0);+}
+static struct sbiret sbi_debug_set_shmem(void *shmem) +{
unsigned long base_addr_lo, base_addr_hi;split_phys_addr(virt_to_phys(shmem), &base_addr_hi, &base_addr_lo);return sbi_debug_set_shmem_raw(base_addr_lo, base_addr_hi, 0);+}
+static struct sbiret sbi_debug_read_triggers(unsigned long trig_idx_base,
unsigned long trig_count)+{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_READ, trig_idx_base,trig_count, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_install_triggers(unsigned long trig_count) +{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_INSTALL, trig_count, 0, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_update_triggers(unsigned long trig_count) +{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_UPDATE, trig_count, 0, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_uninstall_triggers(unsigned long trig_idx_base,
unsigned long trig_idx_mask)+{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_UNINSTALL, trig_idx_base,trig_idx_mask, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_enable_triggers(unsigned long trig_idx_base,
unsigned long trig_idx_mask)+{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_ENABLE, trig_idx_base,trig_idx_mask, 0, 0, 0, 0);+}
+static struct sbiret sbi_debug_disable_triggers(unsigned long trig_idx_base,
unsigned long trig_idx_mask)+{
return sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIGGER_DISABLE, trig_idx_base,trig_idx_mask, 0, 0, 0, 0);+}
+static bool dbtr_install_trigger(struct sbi_dbtr_shmem_entry *shmem, void *trigger,
unsigned long control)+{
struct sbiret sbi_ret;bool ret;shmem->data.tdata1 = control;shmem->data.tdata2 = (unsigned long)trigger;sbi_ret = sbi_debug_install_triggers(1);ret = sbiret_report_error(&sbi_ret, SBI_SUCCESS, "sbi_debug_install_triggers");if (ret)install_exception_handler(EXC_BREAKPOINT, dbtr_exception_handler);return ret;+}
+static bool dbtr_uninstall_trigger(void) +{
struct sbiret ret;install_exception_handler(EXC_BREAKPOINT, NULL);ret = sbi_debug_uninstall_triggers(0, 1);return sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");+}
+static unsigned long dbtr_test_num_triggers(void) +{
struct sbiret ret;unsigned long tdata1 = 0;/* sbi_debug_num_triggers will return trig_max in sbiret.value when trig_tdata1 == 0 *//* should be at least one trigger. */ret = sbi_debug_num_triggers(tdata1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_num_triggers");if (ret.value == 0) {report_fail("sbi_debug_num_triggers: Returned 0 triggers available");} else {report_pass("sbi_debug_num_triggers: Returned triggers available");report_info("sbi_debug_num_triggers: Returned %lu triggers available", ret.value);Rather than put the sbi_debug_num_triggers prefix in each report call, we can just push it.
}return ret.value;+}
+static enum McontrolType dbtr_test_type(unsigned long *num_trig) +{
struct sbiret ret;unsigned long tdata1 = SBI_DBTR_TDATA1_TYPE_MCONTROL6;report_prefix_push("test type");ret = sbi_debug_num_triggers(tdata1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_num_triggers: mcontrol6");*num_trig = ret.value;if (ret.value > 0) {report_pass("sbi_debug_num_triggers: Returned mcontrol6 triggers available");report_info("sbi_debug_num_triggers: Returned %lu mcontrol6 triggers available",Same comment about pushing "sbi_debug_num_triggers".
ret.value);report_prefix_pop();If we push sbi_debug_num_triggers, then this can be a report_prefix_popn(2).
return tdata1;}tdata1 = SBI_DBTR_TDATA1_TYPE_MCONTROL;ret = sbi_debug_num_triggers(tdata1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_num_triggers: mcontrol");*num_trig = ret.value;if (ret.value > 0) {report_pass("sbi_debug_num_triggers: Returned mcontrol triggers available");report_info("sbi_debug_num_triggers: Returned %lu mcontrol triggers available",ret.value);report_prefix_pop();return tdata1;}report_fail("sbi_debug_num_triggers: Returned 0 mcontrol(6) triggers available");report_prefix_pop();return SBI_DBTR_TDATA1_TYPE_NONE;+}
+static struct sbiret dbtr_test_store_install_uninstall(struct sbi_dbtr_shmem_entry *shmem,
enum McontrolType type)+{
static unsigned long test;struct sbiret ret;report_prefix_push("store trigger");shmem->data.tdata1 = gen_tdata1(type, VALUE_STORE, MODE_S);shmem->data.tdata2 = (unsigned long)&test;ret = sbi_debug_install_triggers(1);if (!sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_install_triggers")) {report_prefix_pop();return ret;}install_exception_handler(EXC_BREAKPOINT, dbtr_exception_handler);report(do_store(&test), "triggered");if (do_load(&test))report_fail("triggered by load");ret = sbi_debug_uninstall_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");if (do_store(&test))report_fail("triggered after uninstall");install_exception_handler(EXC_BREAKPOINT, NULL);report_prefix_pop();return ret;+}
+static void dbtr_test_update(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;struct sbiret ret;bool kfail;report_prefix_push("update trigger");if (!dbtr_install_trigger(shmem, NULL, gen_tdata1(type, VALUE_NONE, MODE_NONE))) {report_prefix_pop();return;}shmem->id.idx = 0;shmem->data.tdata1 = gen_tdata1(type, VALUE_STORE, MODE_S);shmem->data.tdata2 = (unsigned long)&test;ret = sbi_debug_update_triggers(1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_update_triggers");/** Known broken update_triggers.* https://lore.kernel.org/opensbi/aDdp1UeUh7GugeHp@ghost/T/#t*/kfail = __sbi_get_imp_id() == SBI_IMPL_OPENSBI &&__sbi_get_imp_version() < sbi_impl_opensbi_mk_version(1, 7);report_kfail(kfail, do_store(&test), "triggered");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_load(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;report_prefix_push("load trigger");if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_LOAD, MODE_S))) {report_prefix_pop();return;}report(do_load(&test), "triggered");if (do_store(&test))report_fail("triggered by store");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_disable_enable(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;struct sbiret ret;report_prefix_push("disable trigger");if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}ret = sbi_debug_disable_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_disable_triggers");if (!report(!do_store(&test), "should not trigger")) {dbtr_uninstall_trigger();report_prefix_pop();report_skip("enable trigger: no disable");return;}report_prefix_pop();report_prefix_push("enable trigger");ret = sbi_debug_enable_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_enable_triggers");report(do_store(&test), "triggered");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_exec(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;report_prefix_push("exec trigger");/* check if loads and stores trigger exec */if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_EXECUTE, MODE_S))) {report_prefix_pop();return;}if (do_load(&test))report_fail("triggered by load");if (do_store(&test))report_fail("triggered by store");dbtr_uninstall_trigger();/* Check if exec works */if (!dbtr_install_trigger(shmem, exec_call, gen_tdata1(type, VALUE_EXECUTE, MODE_S))) {report_prefix_pop();return;}report(do_exec(), "triggered");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_read(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
const unsigned long tstatus_expected = SBI_DBTR_TRIG_STATE_S | SBI_DBTR_TRIG_STATE_MAPPED;const unsigned long tdata1 = gen_tdata1(type, VALUE_STORE, MODE_S);static unsigned long test;struct sbiret ret;report_prefix_push("read trigger");if (!dbtr_install_trigger(shmem, &test, tdata1)) {report_prefix_pop();return;}ret = sbi_debug_read_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_read_triggers");report(shmem->data.tdata1 == tdata1, "tdata1 expected: 0x%016lx", tdata1);report_info("tdata1 found: 0x%016lx", shmem->data.tdata1);report(shmem->data.tdata2 == ((unsigned long)&test), "tdata2 expected: 0x%016lx",(unsigned long)&test);report_info("tdata2 found: 0x%016lx", shmem->data.tdata2);report(shmem->data.tstate == tstatus_expected, "tstate expected: 0x%016lx", tstatus_expected);report_info("tstate found: 0x%016lx", shmem->data.tstate);These don't need to be split into report/report_info pairs because the report is checking for a specific value. We only split when report is checking for some nonzero value and we also want to output what that specific value was for informational purposes.
I'm a bit confused. Should it only report_info when the test fails?
dbtr_uninstall_trigger();report_prefix_pop();+}
+static void check_exec(unsigned long base) +{
struct sbiret ret;report(do_exec(), "exec triggered");ret = sbi_debug_uninstall_triggers(base, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");+}
+static void dbtr_test_multiple(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type,
unsigned long num_trigs)+{
static unsigned long test[2];struct sbiret ret;bool have_three = num_trigs > 2;if (num_trigs < 2) {report_skip("test multiple");return;}report_prefix_push("test multiple");if (!dbtr_install_trigger(shmem, &test[0], gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}if (!dbtr_install_trigger(shmem, &test[1], gen_tdata1(type, VALUE_LOAD, MODE_S)))goto error;if (have_three &&!dbtr_install_trigger(shmem, exec_call, gen_tdata1(type, VALUE_EXECUTE, MODE_S))) {ret = sbi_debug_uninstall_triggers(1, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");goto error;}report(do_store(&test[0]), "store triggered");if (do_load(&test[0]))report_fail("store triggered by load");report(do_load(&test[1]), "load triggered");if (do_store(&test[1]))report_fail("load triggered by store");if (have_three)check_exec(2);ret = sbi_debug_uninstall_triggers(1, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");if (do_load(&test[1]))report_fail("load triggered after uninstall");report(do_store(&test[0]), "store triggered");if (!have_three &&dbtr_install_trigger(shmem, exec_call, gen_tdata1(type, VALUE_EXECUTE, MODE_S)))check_exec(1);+error:
ret = sbi_debug_uninstall_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_uninstall_triggers");install_exception_handler(EXC_BREAKPOINT, NULL);report_prefix_pop();+}
+static void dbtr_test_multiple_types(struct sbi_dbtr_shmem_entry *shmem, unsigned long type) +{
static unsigned long test;report_prefix_push("test multiple types");/* check if loads and stores trigger exec */if (!dbtr_install_trigger(shmem, &test,gen_tdata1(type, VALUE_EXECUTE | VALUE_LOAD | VALUE_STORE, MODE_S))) {report_prefix_pop();return;}report(do_load(&test), "load triggered");report(do_store(&test), "store triggered");dbtr_uninstall_trigger();/* Check if exec works */if (!dbtr_install_trigger(shmem, exec_call,gen_tdata1(type, VALUE_EXECUTE | VALUE_LOAD | VALUE_STORE, MODE_S))) {report_prefix_pop();return;}report(do_exec(), "exec triggered");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_disable_uninstall(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;struct sbiret ret;report_prefix_push("disable uninstall");if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}ret = sbi_debug_disable_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_disable_triggers");dbtr_uninstall_trigger();if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}report(do_store(&test), "triggered");dbtr_uninstall_trigger();report_prefix_pop();+}
+static void dbtr_test_uninstall_enable(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;struct sbiret ret;report_prefix_push("uninstall enable");if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}dbtr_uninstall_trigger();ret = sbi_debug_enable_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_enable_triggers");install_exception_handler(EXC_BREAKPOINT, dbtr_exception_handler);report(!do_store(&test), "should not trigger");install_exception_handler(EXC_BREAKPOINT, NULL);report_prefix_pop();+}
+static void dbtr_test_uninstall_update(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
static unsigned long test;struct sbiret ret;bool kfail;report_prefix_push("uninstall update");if (!dbtr_install_trigger(shmem, NULL, gen_tdata1(type, VALUE_NONE, MODE_NONE))) {report_prefix_pop();return;}dbtr_uninstall_trigger();shmem->id.idx = 0;shmem->data.tdata1 = gen_tdata1(type, VALUE_STORE, MODE_S);shmem->data.tdata2 = (unsigned long)&test;/** Known broken update_triggers.* https://lore.kernel.org/opensbi/aDdp1UeUh7GugeHp@ghost/T/#t*/kfail = __sbi_get_imp_id() == SBI_IMPL_OPENSBI &&__sbi_get_imp_version() < sbi_impl_opensbi_mk_version(1, 7);ret = sbi_debug_update_triggers(1);sbiret_kfail_error(kfail, &ret, SBI_ERR_FAILURE, "sbi_debug_update_triggers");install_exception_handler(EXC_BREAKPOINT, dbtr_exception_handler);report(!do_store(&test), "should not trigger");install_exception_handler(EXC_BREAKPOINT, NULL);report_prefix_pop();+}
+static void dbtr_test_disable_read(struct sbi_dbtr_shmem_entry *shmem, enum McontrolType type) +{
const unsigned long tstatus_expected = SBI_DBTR_TRIG_STATE_S | SBI_DBTR_TRIG_STATE_MAPPED;const unsigned long tdata1 = gen_tdata1(type, VALUE_STORE, MODE_NONE);static unsigned long test;struct sbiret ret;report_prefix_push("disable read");if (!dbtr_install_trigger(shmem, &test, gen_tdata1(type, VALUE_STORE, MODE_S))) {report_prefix_pop();return;}ret = sbi_debug_disable_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_disable_triggers");ret = sbi_debug_read_triggers(0, 1);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_read_triggers");report(shmem->data.tdata1 == tdata1, "tdata1 expected: 0x%016lx", tdata1);report_info("tdata1 found: 0x%016lx", shmem->data.tdata1);report(shmem->data.tdata2 == ((unsigned long)&test), "tdata2 expected: 0x%016lx",(unsigned long)&test);report_info("tdata2 found: 0x%016lx", shmem->data.tdata2);report(shmem->data.tstate == tstatus_expected, "tstate expected: 0x%016lx", tstatus_expected);report_info("tstate found: 0x%016lx", shmem->data.tstate);Same comment about not needing to split this.
dbtr_uninstall_trigger();report_prefix_pop();+}
+void check_dbtr(void) +{
static struct sbi_dbtr_shmem_entry shmem[RV_MAX_TRIGGERS] = {};unsigned long num_trigs;enum McontrolType trig_type;struct sbiret ret;report_prefix_push("dbtr");if (!sbi_probe(SBI_EXT_DBTR)) {report_skip("extension not available");report_prefix_pop();return;Could rename the 'error' label to something else and goto it from here too.
Is `dtbr_exit_test` ok?
}if (__sbi_get_imp_id() == SBI_IMPL_OPENSBI &&__sbi_get_imp_version() < sbi_impl_opensbi_mk_version(1, 5)) {report_skip("OpenSBI < v1.5 detected, skipping tests");report_prefix_pop();return;Why do we need to do this? Isn't the DBTR probe above enough?
I copied it from the sbi-sse.c file none of the other tests do this so I'll remove it.
Thanks, Jesse Taube
}num_trigs = dbtr_test_num_triggers();if (!num_trigs)goto error;trig_type = dbtr_test_type(&num_trigs);if (trig_type == SBI_DBTR_TDATA1_TYPE_NONE)goto error;ret = sbi_debug_set_shmem(shmem);sbiret_report_error(&ret, SBI_SUCCESS, "sbi_debug_set_shmem");ret = dbtr_test_store_install_uninstall(&shmem[0], trig_type);/* install or uninstall failed */if (ret.error != SBI_SUCCESS)goto error;dbtr_test_load(&shmem[0], trig_type);dbtr_test_exec(&shmem[0], trig_type);dbtr_test_read(&shmem[0], trig_type);dbtr_test_disable_enable(&shmem[0], trig_type);dbtr_test_update(&shmem[0], trig_type);dbtr_test_multiple_types(&shmem[0], trig_type);dbtr_test_multiple(shmem, trig_type, num_trigs);dbtr_test_disable_uninstall(&shmem[0], trig_type);dbtr_test_uninstall_enable(&shmem[0], trig_type);dbtr_test_uninstall_update(&shmem[0], trig_type);dbtr_test_disable_read(&shmem[0], trig_type);+error:
report_prefix_pop();+} diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h index d5c4ae70..c1ebf016 100644 --- a/riscv/sbi-tests.h +++ b/riscv/sbi-tests.h @@ -97,8 +97,10 @@ static inline bool env_enabled(const char *env) return s && (*s == '1' || *s == 'y' || *s == 'Y'); }
+void split_phys_addr(phys_addr_t paddr, unsigned long *hi, unsigned long *lo); void sbi_bad_fid(int ext); void check_sse(void); +void check_dbtr(void);
#endif /* __ASSEMBLER__ */ #endif /* _RISCV_SBI_TESTS_H_ */ diff --git a/riscv/sbi.c b/riscv/sbi.c index edb1a6be..3b8aadce 100644 --- a/riscv/sbi.c +++ b/riscv/sbi.c @@ -105,7 +105,7 @@ static int rand_online_cpu(prng_state *ps) return cpu; }
-static void split_phys_addr(phys_addr_t paddr, unsigned long *hi, unsigned long *lo) +void split_phys_addr(phys_addr_t paddr, unsigned long *hi, unsigned long *lo) { *lo = (unsigned long)paddr; *hi = 0; @@ -1561,6 +1561,7 @@ int main(int argc, char **argv) check_susp(); check_sse(); check_fwft();
check_dbtr(); return report_summary();}
2.43.0
Thanks, drew