Updated debug interface to allow for halting with a 'cause'. PiperOrigin-RevId: 667625877 Change-Id: I42e732f2859d0d482f892ac8bcf0a806258cf9fd
diff --git a/cheriot/BUILD b/cheriot/BUILD index 8feb88d..3e2156a 100644 --- a/cheriot/BUILD +++ b/cheriot/BUILD
@@ -591,6 +591,7 @@ "@com_google_mpact-sim//mpact/sim/generic:core", "@com_google_mpact-sim//mpact/sim/generic:core_debug_interface", "@com_google_mpact-sim//mpact/sim/generic:counters", + "@com_google_mpact-sim//mpact/sim/generic:debug_command_shell_interface", "@com_google_mpact-sim//mpact/sim/generic:instruction", "@com_google_mpact-sim//mpact/sim/proto:component_data_cc_proto", "@com_google_mpact-sim//mpact/sim/util/memory", @@ -708,6 +709,7 @@ "@com_google_mpact-riscv//riscv:stoull_wrapper", "@com_google_mpact-sim//mpact/sim/generic:core", "@com_google_mpact-sim//mpact/sim/generic:core_debug_interface", + "@com_google_mpact-sim//mpact/sim/generic:debug_command_shell_interface", "@com_google_mpact-sim//mpact/sim/generic:type_helpers", "@com_google_mpact-sim//mpact/sim/proto:component_data_cc_proto", "@com_google_mpact-sim//mpact/sim/util/memory",
diff --git a/cheriot/cheriot_cli_forwarder.cc b/cheriot/cheriot_cli_forwarder.cc index 633f763..6c5075b 100644 --- a/cheriot/cheriot_cli_forwarder.cc +++ b/cheriot/cheriot_cli_forwarder.cc
@@ -80,6 +80,15 @@ absl::Status CheriotCLIForwarder::Halt() { return cheriot_cli_top_->CLIHalt(); } +absl::Status CheriotCLIForwarder::Halt(HaltReason halt_reason) { + cheriot_cli_top_->CLIRequestHalt(halt_reason, nullptr); + return absl::OkStatus(); +} + +absl::Status CheriotCLIForwarder::Halt(HaltReasonValueType halt_reason) { + cheriot_cli_top_->CLIRequestHalt(halt_reason, nullptr); + return absl::OkStatus(); +} absl::StatusOr<int> CheriotCLIForwarder::Step(int num) { return cheriot_cli_top_->CLIStep(num); }
diff --git a/cheriot/cheriot_cli_forwarder.h b/cheriot/cheriot_cli_forwarder.h index a07386a..3da7fb6 100644 --- a/cheriot/cheriot_cli_forwarder.h +++ b/cheriot/cheriot_cli_forwarder.h
@@ -71,6 +71,8 @@ void SetBreakOnControlFlowChange(bool value) override; // Request that core stop running. absl::Status Halt() override; + absl::Status Halt(HaltReason halt_reason) override; + absl::Status Halt(HaltReasonValueType halt_reason) override; // Step the core by num instructions. absl::StatusOr<int> Step(int num) override; // Allow the core to free-run. The loop to run the instructions should be
diff --git a/cheriot/cheriot_renode.cc b/cheriot/cheriot_renode.cc index aa417f8..88d2ea1 100644 --- a/cheriot/cheriot_renode.cc +++ b/cheriot/cheriot_renode.cc
@@ -426,7 +426,7 @@ cmd_shell_, cheriot_top_, mem_profiler_); cmd_shell_->AddCore( {static_cast<CheriotDebugInterface *>(cheriot_cli_forwarder_), - [this]() { return program_loader_; }}); + [this]() { return program_loader_; }, cheriot_state_}); cmd_shell_->AddCommand( instrumentation_control_->Usage(), absl::bind_front(&CheriotInstrumentationControl::PerformShellCommand,
diff --git a/cheriot/cheriot_state.cc b/cheriot/cheriot_state.cc index e0c194d..685e6d0 100644 --- a/cheriot/cheriot_state.cc +++ b/cheriot/cheriot_state.cc
@@ -655,9 +655,28 @@ set_branch(true); // TODO(torerik): set next pc mstatus_->Submit(); + // Set up interrupt info before incrementing the interrupt counter. That way + // any code that is triggered by the interrupt counter will see the updated + // interrupt info. + InterruptInfo info; + info.is_interrupt = is_interrupt; + info.cause = mcause_->GetUint32(); + info.tval = mtval_->GetUint32(); + info.epc = epc; + interrupt_info_list_.push_back(info); + counter_interrupts_taken_.Increment(1); } +// Called upon returning from an interrupt or exception. +void CheriotState::SignalReturnFromInterrupt() { + // First increment the interrupt return counter. Then pop the interrupt info. + // This way any code that is triggered by the interrupt return counter will + // be able to access the interrupt info. + counter_interrupt_returns_.Increment(1); + interrupt_info_list_.pop_back(); +} + // CheckForInterrupt is called whenever any relevant bits in the interrupt // enable and set bits are changed. It should always be scheduled to execute // from the function_delay_line, that way it is executed after an instruction
diff --git a/cheriot/cheriot_state.h b/cheriot/cheriot_state.h index b24f5e3..6045039 100644 --- a/cheriot/cheriot_state.h +++ b/cheriot/cheriot_state.h
@@ -19,6 +19,7 @@ #include <any> #include <cstdint> +#include <deque> #include <string> #include <string_view> #include <utility> @@ -157,6 +158,15 @@ using ::mpact::sim::generic::operator*; // NOLINT: used below (clang error). +// Struct to track interrupt/trap information. +struct InterruptInfo { + bool is_interrupt; + uint32_t cause; + uint32_t tval; + uint32_t epc; +}; +using InterruptInfoList = std::deque<InterruptInfo>; + class CheriotState : public generic::ArchState { public: static int constexpr kCapRegQueueSizeMask = 0x11; @@ -290,7 +300,7 @@ // Indicates that the program has returned from handling an interrupt. This // decrements the interrupt handler depth and should be called by the // implementations of mret, sret, and uret. - void SignalReturnFromInterrupt() { counter_interrupt_returns_.Increment(1); } + void SignalReturnFromInterrupt(); // Returns the depth of the interrupt handler currently being executed, or // zero if no interrupt handler is being executed. @@ -399,7 +409,9 @@ CheriotRegister *temp_reg() { return temp_reg_; } RiscVCsrInterface *mcause() { return mcause_; } RiscVCheri32PcSourceOperand *pc_src_operand() { return pc_src_operand_; } - + const InterruptInfoList &interrupt_info_list() const { + return interrupt_info_list_; + } uint64_t revocation_mem_base() const { return revocation_mem_base_; } uint64_t revocation_ram_base() const { return revocation_ram_base_; } @@ -451,6 +463,7 @@ SimpleCounter<int64_t> counter_interrupts_taken_; SimpleCounter<int64_t> counter_interrupt_returns_; InterruptCode available_interrupt_code_ = InterruptCode::kNone; + InterruptInfoList interrupt_info_list_; // By default, execute in machine mode. PrivilegeMode privilege_mode_ = PrivilegeMode::kMachine; // Handles to frequently used CSRs.
diff --git a/cheriot/cheriot_top.cc b/cheriot/cheriot_top.cc index 14d1cbf..6df56ad 100644 --- a/cheriot/cheriot_top.cc +++ b/cheriot/cheriot_top.cc
@@ -216,6 +216,16 @@ return absl::OkStatus(); } +absl::Status CheriotTop::Halt(HaltReason halt_reason) { + RequestHalt(halt_reason, nullptr); + return absl::OkStatus(); +} + +absl::Status CheriotTop::Halt(HaltReasonValueType halt_reason) { + RequestHalt(halt_reason, nullptr); + return absl::OkStatus(); +} + absl::Status CheriotTop::StepPastBreakpoint() { uint64_t pc = state_->pc_operand()->AsUint64(0); // Disable the breakpoint.
diff --git a/cheriot/cheriot_top.h b/cheriot/cheriot_top.h index 660d188..655bc94 100644 --- a/cheriot/cheriot_top.h +++ b/cheriot/cheriot_top.h
@@ -73,6 +73,8 @@ // Methods inherited from CoreDebugInterface. absl::Status Halt() override; + absl::Status Halt(HaltReason halt_reason) override; + absl::Status Halt(HaltReasonValueType halt_reason) override; absl::StatusOr<int> Step(int num) override; absl::Status Run() override; absl::Status Wait() override;
diff --git a/cheriot/debug_command_shell.cc b/cheriot/debug_command_shell.cc index e1ea869..927bc34 100644 --- a/cheriot/debug_command_shell.cc +++ b/cheriot/debug_command_shell.cc
@@ -35,6 +35,7 @@ #include "absl/strings/string_view.h" #include "cheriot/cheriot_debug_interface.h" #include "cheriot/cheriot_register.h" +#include "cheriot/cheriot_state.h" #include "cheriot/cheriot_top.h" #include "cheriot/riscv_cheriot_enums.h" #include "cheriot/riscv_cheriot_register_aliases.h" @@ -56,56 +57,37 @@ DebugCommandShell::InterruptListener::InterruptListener(CoreAccess *core_access) : core_access_(core_access), - top_(static_cast<CheriotTop *>(core_access->debug_interface)), + state_(static_cast<CheriotState *>(core_access_->state)), + dbg_if_(static_cast<CheriotTop *>(core_access->debug_interface)), taken_listener_( absl::bind_front(&InterruptListener::SetTakenValue, this)), return_listener_( absl::bind_front(&InterruptListener::SetReturnValue, this)) { - top_->state()->counter_interrupts_taken()->AddListener(&taken_listener_); - top_->state()->counter_interrupt_returns()->AddListener(&return_listener_); + state_->counter_interrupts_taken()->AddListener(&taken_listener_); + state_->counter_interrupt_returns()->AddListener(&return_listener_); } void DebugCommandShell::InterruptListener::SetReturnValue(int64_t value) { - if (interrupt_info_list_.empty()) { + auto &interrupt_info_list = state_->interrupt_info_list(); + if (interrupt_info_list.empty()) { LOG(ERROR) << "Interrupt stack is empty"; return; } - auto info = interrupt_info_list_.front(); - interrupt_info_list_.pop_front(); + auto info = interrupt_info_list.back(); // If breakpoints are enabled, then request a halt of the appropriate type. if (info.is_interrupt && interrupts_enabled_) - top_->RequestHalt(kInterruptReturn, nullptr); + (void)dbg_if_->Halt(kInterruptReturn); if (!info.is_interrupt && exceptions_enabled_) - top_->RequestHalt(kExceptionReturn, nullptr); + (void)dbg_if_->Halt(kExceptionReturn); } void DebugCommandShell::InterruptListener::SetTakenValue(int64_t value) { - InterruptInfo info; - bool ok = true; - // Read the values of the interrupt registers. - auto res = core_access_->debug_interface->ReadRegister("mcause"); - ok &= res.ok(); - if (ok) info.cause = res.value(); - - res = core_access_->debug_interface->ReadRegister("mtval"); - ok &= res.ok(); - if (ok) info.tval = res.value(); - - res = core_access_->debug_interface->ReadRegister("mepcc"); - ok &= res.ok(); - if (ok) info.epc = res.value(); - - if (!ok) { - LOG(ERROR) << "Failed to read interrupt registers"; - return; - } - info.is_interrupt = (info.cause & 0x8000'0000u) != 0; + InterruptInfo info = state_->interrupt_info_list().back(); // If breakpoints are enabled, the request a halt of the appropriate type. if (info.is_interrupt && interrupts_enabled_) - top_->RequestHalt(kInterruptTaken, nullptr); + (void)dbg_if_->Halt(kInterruptTaken); if (!info.is_interrupt && exceptions_enabled_) - top_->RequestHalt(kExceptionTaken, nullptr); - interrupt_info_list_.push_front(info); + (void)dbg_if_->Halt(kExceptionTaken); } // The constructor initializes all the regular expressions and the help string. @@ -332,8 +314,9 @@ } absl::StrAppend(&prompt, "\n"); } - auto &info_list = - interrupt_listeners_[current_core_]->interrupt_info_list(); + auto cheriot_state = + static_cast<CheriotState *>(core_access_[current_core_].state); + auto &info_list = cheriot_state->interrupt_info_list(); int count = 0; for (auto iter = info_list.rbegin(); iter != info_list.rend(); ++iter) { auto const &info = *iter;
diff --git a/cheriot/debug_command_shell.h b/cheriot/debug_command_shell.h index 71fc7b0..b58b1fe 100644 --- a/cheriot/debug_command_shell.h +++ b/cheriot/debug_command_shell.h
@@ -32,7 +32,8 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "cheriot/cheriot_top.h" +#include "cheriot/cheriot_debug_interface.h" +#include "cheriot/cheriot_state.h" #include "mpact/sim/generic/core_debug_interface.h" #include "mpact/sim/generic/counters_base.h" #include "mpact/sim/generic/debug_command_shell_interface.h" @@ -88,14 +89,6 @@ bool is_enabled; }; - // Struct to track interrupt/trap information. - struct InterruptInfo { - bool is_interrupt; - uint32_t cause; - uint32_t tval; - uint32_t epc; - }; - // The interrupt listener class is used to track interrupts/exceptions and // returns from interrupts/exceptions, so that breakpoints can be set on these // events. @@ -112,7 +105,6 @@ absl::AnyInvocable<void(int64_t)> callback_; }; - using InterruptInfoList = std::deque<InterruptInfo>; static constexpr uint32_t kInterruptTaken = *HaltReason::kUserSpecifiedMin + 1; static constexpr uint32_t kInterruptReturn = @@ -128,19 +120,15 @@ bool AreExceptionsEnabled() const { return exceptions_enabled_; } bool AreInterruptsEnabled() const { return interrupts_enabled_; } - const InterruptInfoList &interrupt_info_list() const { - return interrupt_info_list_; - } - private: void SetReturnValue(int64_t value); void SetTakenValue(int64_t value); CoreAccess *core_access_; - CheriotTop *top_; + CheriotState *state_ = nullptr; + CheriotDebugInterface *dbg_if_ = nullptr; bool interrupts_enabled_ = false; bool exceptions_enabled_ = false; - InterruptInfoList interrupt_info_list_; Listener taken_listener_; Listener return_listener_; };
diff --git a/cheriot/mpact_cheriot.cc b/cheriot/mpact_cheriot.cc index a15cfee..a24abce 100644 --- a/cheriot/mpact_cheriot.cc +++ b/cheriot/mpact_cheriot.cc
@@ -464,7 +464,8 @@ CheriotInstrumentationControl *cheriot_instrumentation_control = nullptr; if (interactive) { mpact::sim::cheriot::DebugCommandShell cmd_shell; - cmd_shell.AddCore({&cheriot_top, [&elf_loader]() { return &elf_loader; }}); + cmd_shell.AddCore({&cheriot_top, [&elf_loader]() { return &elf_loader; }, + &cheriot_state}); cheriot_instrumentation_control = new CheriotInstrumentationControl( &cmd_shell, &cheriot_top, memory_use_profiler); // Add custom command to interactive debug command shell.