From fb57d7ce040fbc07197ee76e2afc46cb42cce5e3 Mon Sep 17 00:00:00 2001 From: Atul Khare Date: Mon, 22 May 2023 14:10:47 -0700 Subject: [PATCH 1/4] Add pre_v to processor state This adds the prev_v field to track the previous virtual mode state. We also assign it unconditionally to handle cases for trigger matching like the following (pointed out by Scott Johnson): 1) SRET from HS to VU: prev_v is set to 0 2) Trap from VU to VS: state.v/prev_v won't be assigned because of unchanged v, and remain 0. 3) An etrigger that's set to break on a VU-mode trap won't match properly because prev_v is incorrect This be used in a forthcoming patch for trigger matching. --- riscv/processor.cc | 23 +++++++++++------------ riscv/processor.h | 1 + 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/riscv/processor.cc b/riscv/processor.cc index 74a0b8fe..fce2c5c1 100644 --- a/riscv/processor.cc +++ b/riscv/processor.cc @@ -197,7 +197,7 @@ void state_t::reset(processor_t* const proc, reg_t max_isa) auto xlen = proc->get_isa().get_max_xlen(); prv = PRV_M; - v = false; + v = prev_v = false; csrmap[CSR_MISA] = misa = std::make_shared(proc, CSR_MISA, max_isa); mstatus = std::make_shared(proc, CSR_MSTATUS); @@ -747,17 +747,16 @@ void processor_t::set_virt(bool virt) if (state.prv == PRV_M) return; - if (state.v != virt) { - /* - * Ideally, we should flush TLB here but we don't need it because - * set_virt() is always used in conjucter with set_privilege() and - * set_privilege() will flush TLB unconditionally. - * - * The virtualized sstatus register also relies on this TLB flush, - * since changing V might change sstatus.MXR and sstatus.SUM. - */ - state.v = virt; - } + /* + * Ideally, we should flush TLB here but we don't need it because + * set_virt() is always used in conjucter with set_privilege() and + * set_privilege() will flush TLB unconditionally. + * + * The virtualized sstatus register also relies on this TLB flush, + * since changing V might change sstatus.MXR and sstatus.SUM. + */ + state.prev_v = state.v; + state.v = virt; } void processor_t::enter_debug_mode(uint8_t cause) diff --git a/riscv/processor.h b/riscv/processor.h index 1b74cc27..93e10f35 100644 --- a/riscv/processor.h +++ b/riscv/processor.h @@ -84,6 +84,7 @@ struct state_t std::unordered_map csrmap; reg_t prv; // TODO: Can this be an enum instead? bool v; + bool prev_v; misa_csr_t_p misa; mstatus_csr_t_p mstatus; csr_t_p mstatush; From ddae0f25a89f2d04d76a222505d00c39dc511505 Mon Sep 17 00:00:00 2001 From: Atul Khare Date: Mon, 22 May 2023 14:16:03 -0700 Subject: [PATCH 2/4] Add prev_prv to processor state This adds the prev_prv field to track the previous privilege. It will be used in a forthcoming patch for trigger matching. --- riscv/processor.cc | 3 ++- riscv/processor.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/riscv/processor.cc b/riscv/processor.cc index fce2c5c1..23284b87 100644 --- a/riscv/processor.cc +++ b/riscv/processor.cc @@ -196,7 +196,7 @@ void state_t::reset(processor_t* const proc, reg_t max_isa) // mstatus_csr_t::unlogged_write()): auto xlen = proc->get_isa().get_max_xlen(); - prv = PRV_M; + prv = prev_prv = PRV_M; v = prev_v = false; csrmap[CSR_MISA] = misa = std::make_shared(proc, CSR_MISA, max_isa); mstatus = std::make_shared(proc, CSR_MSTATUS); @@ -717,6 +717,7 @@ reg_t processor_t::legalize_privilege(reg_t prv) void processor_t::set_privilege(reg_t prv) { mmu->flush_tlb(); + state.prev_prv = state.prv; state.prv = legalize_privilege(prv); } diff --git a/riscv/processor.h b/riscv/processor.h index 93e10f35..34354c22 100644 --- a/riscv/processor.h +++ b/riscv/processor.h @@ -83,6 +83,7 @@ struct state_t // control and status registers std::unordered_map csrmap; reg_t prv; // TODO: Can this be an enum instead? + reg_t prev_prv; bool v; bool prev_v; misa_csr_t_p misa; From 31f5ede662303183d93f80e869379e49b7a01608 Mon Sep 17 00:00:00 2001 From: Atul Khare Date: Wed, 17 May 2023 12:40:01 -0700 Subject: [PATCH 3/4] Enhance mode_match() functionality The current version of mode_match() is based on the current privilege level. This adds an explicit privilege and virtual mode parameters in anticipation of an upcoming patch for matching trap triggers. --- riscv/triggers.cc | 11 ++++++----- riscv/triggers.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/riscv/triggers.cc b/riscv/triggers.cc index 86dcc81a..51dcf188 100644 --- a/riscv/triggers.cc +++ b/riscv/triggers.cc @@ -56,15 +56,16 @@ void trigger_t::tdata3_write(processor_t * const proc, const reg_t val) noexcept } bool trigger_t::common_match(processor_t * const proc) const noexcept { - return mode_match(proc->get_state()) && textra_match(proc); + auto state = proc->get_state(); + return mode_match(state->prv, state->v) && textra_match(proc); } -bool trigger_t::mode_match(state_t * const state) const noexcept +bool trigger_t::mode_match(reg_t prv, bool v) const noexcept { - switch (state->prv) { + switch (prv) { case PRV_M: return m; - case PRV_S: return state->v ? vs : s; - case PRV_U: return state->v ? vu : u; + case PRV_S: return v ? vs : s; + case PRV_U: return v ? vu : u; default: assert(false); } } diff --git a/riscv/triggers.h b/riscv/triggers.h index aeda4d58..94e7e5ce 100644 --- a/riscv/triggers.h +++ b/riscv/triggers.h @@ -102,7 +102,7 @@ protected: private: unsigned legalize_mhselect(bool h_enabled) const noexcept; - bool mode_match(state_t * const state) const noexcept; + bool mode_match(reg_t prv, bool v) const noexcept; bool textra_match(processor_t * const proc) const noexcept; struct mhselect_interpretation { From d9e30bb6970c3e387d719b3fd0808937889ca3a6 Mon Sep 17 00:00:00 2001 From: Atul Khare Date: Tue, 16 May 2023 15:11:44 -0700 Subject: [PATCH 4/4] triggers: Fix etrigger match on exceptions The etrigger match on exceptions doesn't work properly in cases like the following: 1) M-mode delegates ECALLs to S-mode 2) A CPU hardware point mechanism is used to place a breakpoint on the Umode instruction that executes the ECALL from Umode to Smode. In effect, this creates a breakpoint etrigger based on Umode. In the above, the expectation is that #2 will first cause an exit to the Smode handler (stvec), and the hardware breakpoint exception will be triggered following an entry into the handler. However, since etrigger currently checks the current privilege mode, we will never get a match on conditions like #2. The patch attempts to address the issue by using the stashed version of the previous privilege mode for the etrigger match. cc: YenHaoChen Signed-off-by: Atul Khare --- riscv/triggers.cc | 9 ++++++--- riscv/triggers.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/riscv/triggers.cc b/riscv/triggers.cc index 51dcf188..65ba4c9b 100644 --- a/riscv/triggers.cc +++ b/riscv/triggers.cc @@ -55,9 +55,11 @@ void trigger_t::tdata3_write(processor_t * const proc, const reg_t val) noexcept sselect = (sselect_t)((proc->extension_enabled_const('S') && get_field(val, CSR_TEXTRA_SSELECT(xlen)) <= SSELECT_MAXVAL) ? get_field(val, CSR_TEXTRA_SSELECT(xlen)) : SSELECT_IGNORE); } -bool trigger_t::common_match(processor_t * const proc) const noexcept { +bool trigger_t::common_match(processor_t * const proc, bool use_prev_prv) const noexcept { auto state = proc->get_state(); - return mode_match(state->prv, state->v) && textra_match(proc); + auto prv = use_prev_prv ? state->prev_prv : state->prv; + auto v = use_prev_prv ? state->prev_v : state->v; + return mode_match(prv, v) && textra_match(proc); } bool trigger_t::mode_match(reg_t prv, bool v) const noexcept @@ -398,7 +400,8 @@ void itrigger_t::tdata1_write(processor_t * const proc, const reg_t val, const b std::optional trap_common_t::detect_trap_match(processor_t * const proc, const trap_t& t) noexcept { - if (!common_match(proc)) + // Use the previous privilege for matching + if (!common_match(proc, true)) return std::nullopt; auto xlen = proc->get_xlen(); diff --git a/riscv/triggers.h b/riscv/triggers.h index 94e7e5ce..0bf6097a 100644 --- a/riscv/triggers.h +++ b/riscv/triggers.h @@ -90,7 +90,7 @@ public: protected: static action_t legalize_action(reg_t val, reg_t action_mask, reg_t dmode_mask) noexcept; - bool common_match(processor_t * const proc) const noexcept; + bool common_match(processor_t * const proc, bool use_prev_prv = false) const noexcept; bool allow_action(const state_t * const state) const; reg_t tdata2;