Refactored breakpoint and action points.
PiperOrigin-RevId: 643345868
Change-Id: I192800fbbc04ad9c98e21339c57b940f54d9c459
diff --git a/cheriot/BUILD b/cheriot/BUILD
index e2750df..4bb61f5 100644
--- a/cheriot/BUILD
+++ b/cheriot/BUILD
@@ -151,8 +151,9 @@
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/synchronization",
- "@com_google_mpact-riscv//riscv:riscv_breakpoint",
+ "@com_google_mpact-riscv//riscv:riscv_action_point_memory_interface",
"@com_google_mpact-riscv//riscv:riscv_state",
+ "@com_google_mpact-sim//mpact/sim/generic:action_points",
"@com_google_mpact-sim//mpact/sim/generic:component",
"@com_google_mpact-sim//mpact/sim/generic:core",
"@com_google_mpact-sim//mpact/sim/generic:core_debug_interface",
diff --git a/cheriot/cheriot_top.cc b/cheriot/cheriot_top.cc
index 5321f0d..2078de5 100644
--- a/cheriot/cheriot_top.cc
+++ b/cheriot/cheriot_top.cc
@@ -36,6 +36,8 @@
#include "cheriot/cheriot_state.h"
#include "cheriot/riscv_cheriot_enums.h"
#include "cheriot/riscv_cheriot_register_aliases.h"
+#include "mpact/sim/generic/action_point_manager_base.h"
+#include "mpact/sim/generic/breakpoint_manager.h"
#include "mpact/sim/generic/component.h"
#include "mpact/sim/generic/core_debug_interface.h"
#include "mpact/sim/generic/counters.h"
@@ -49,8 +51,7 @@
#include "mpact/sim/util/memory/tagged_memory_interface.h"
#include "mpact/sim/util/memory/tagged_memory_watcher.h"
#include "re2/re2.h"
-#include "riscv//riscv_action_point.h"
-#include "riscv//riscv_breakpoint.h"
+#include "riscv//riscv_action_point_memory_interface.h"
#include "riscv//riscv_csr.h"
#include "riscv//riscv_register.h"
@@ -60,6 +61,9 @@
constexpr char kCheriotName[] = "CherIoT";
+using ::mpact::sim::generic::ActionPointManagerBase;
+using ::mpact::sim::generic::BreakpointManager;
+using ::mpact::sim::riscv::RiscVActionPointMemoryInterface;
using EC = ::mpact::sim::cheriot::ExceptionCode;
using PB = ::mpact::sim::cheriot::CheriotRegister::PermissionBits;
@@ -127,20 +131,21 @@
CHECK_OK(AddCounter(&counter_pc_)) << "Failed to register pc counter";
// Breakpoints.
- rv_action_point_manager_ = new riscv::RiscVActionPointManager(
+ rv_ap_memory_if_ = new RiscVActionPointMemoryInterface(
memory, absl::bind_front(&generic::DecodeCache::Invalidate,
cheriot_decode_cache_));
- rv_bp_manager_ = new riscv::RiscVBreakpointManager(
- rv_action_point_manager_,
- [this](HaltReason halt_reason) { RequestHalt(halt_reason, nullptr); });
+ rv_ap_manager_ = new ActionPointManagerBase(rv_ap_memory_if_);
+ rv_bp_manager_ = new BreakpointManager(rv_ap_manager_, [this]() {
+ RequestHalt(HaltReason::kSoftwareBreakpoint, nullptr);
+ });
// Set the software action callback.
state_->AddEbreakHandler([this](const Instruction *inst) {
- if (rv_action_point_manager_->IsActionPointActive(inst->address())) {
+ if (rv_ap_manager_->IsActionPointActive(inst->address())) {
// Need to request a halt so that the action point can be stepped past
// after executing the actions. However, an action may override the
// particular halt reason (e.g., breakpoints).
RequestHalt(HaltReason::kActionPoint, inst);
- rv_action_point_manager_->PerformActions(inst->address());
+ rv_ap_manager_->PerformActions(inst->address());
return true;
}
return false;
@@ -220,7 +225,7 @@
absl::Status CheriotTop::StepPastBreakpoint() {
uint64_t pc = state_->pc_operand()->AsUint64(0);
// Disable the breakpoint.
- rv_action_point_manager_->WriteOriginalInstruction(pc);
+ (void)rv_ap_manager_->ap_memory_interface()->WriteOriginalInstruction(pc);
// Execute the real instruction.
auto real_inst = cheriot_decode_cache_->GetDecodedInstruction(pc);
real_inst->IncRef();
@@ -236,7 +241,7 @@
counter_num_instructions_.Increment(1);
real_inst->DecRef();
// Re-enable the breakpoint.
- rv_action_point_manager_->WriteBreakpointInstruction(pc);
+ (void)rv_ap_manager_->ap_memory_interface()->WriteBreakpointInstruction(pc);
// Get the next pc value.
if (state_->branch()) {
state_->set_branch(false);
@@ -760,37 +765,37 @@
return absl::OkStatus();
}
-// Methods for Action points forward to the rv_action_point_manager_ methods.
+// Methods for Action points forward to the rv_ap_manager_ methods.
absl::StatusOr<int> CheriotTop::SetActionPoint(
uint64_t address, absl::AnyInvocable<void(uint64_t, int)> action) {
- if (rv_action_point_manager_ == nullptr) {
+ if (rv_ap_manager_ == nullptr) {
return absl::InternalError("Action points are not enabled");
}
- auto res = rv_action_point_manager_->SetAction(address, std::move(action));
+ auto res = rv_ap_manager_->SetAction(address, std::move(action));
if (!res.ok()) return res;
return res.value();
}
absl::Status CheriotTop::ClearActionPoint(uint64_t address, int id) {
- if (rv_action_point_manager_ == nullptr) {
+ if (rv_ap_manager_ == nullptr) {
return absl::InternalError("Action points are not enabled");
}
- return rv_action_point_manager_->ClearAction(address, id);
+ return rv_ap_manager_->ClearAction(address, id);
}
absl::Status CheriotTop::EnableAction(uint64_t address, int id) {
- if (rv_action_point_manager_ == nullptr) {
+ if (rv_ap_manager_ == nullptr) {
return absl::InternalError("Action points are not enabled");
}
- return rv_action_point_manager_->EnableAction(address, id);
+ return rv_ap_manager_->EnableAction(address, id);
}
absl::Status CheriotTop::DisableAction(uint64_t address, int id) {
- if (rv_action_point_manager_ == nullptr) {
+ if (rv_ap_manager_ == nullptr) {
return absl::InternalError("Action points are not enabled");
}
- return rv_action_point_manager_->DisableAction(address, id);
+ return rv_ap_manager_->DisableAction(address, id);
}
// Set a data watchpoint for the given address range and access type.
@@ -898,16 +903,18 @@
// If requesting the disassembly for an instruction at an action point, we
// need to write the original instruction back to memory before getting the
// disassembly.
- bool inst_swap = rv_action_point_manager_->IsActionPointActive(address);
+ bool inst_swap = rv_ap_manager_->IsActionPointActive(address);
if (inst_swap) {
- rv_action_point_manager_->WriteOriginalInstruction(address);
+ (void)rv_ap_manager_->ap_memory_interface()->WriteOriginalInstruction(
+ address);
}
// Get the decoded instruction.
inst = cheriot_decode_cache_->GetDecodedInstruction(address);
auto disasm = inst != nullptr ? inst->AsString() : "Invalid instruction";
// Swap back if required.
if (inst_swap) {
- rv_action_point_manager_->WriteBreakpointInstruction(address);
+ (void)rv_ap_manager_->ap_memory_interface()->WriteBreakpointInstruction(
+ address);
}
return disasm;
}
diff --git a/cheriot/cheriot_top.h b/cheriot/cheriot_top.h
index 4fc8d17..eb7415d 100644
--- a/cheriot/cheriot_top.h
+++ b/cheriot/cheriot_top.h
@@ -31,7 +31,8 @@
#include "cheriot/cheriot_decoder.h"
#include "cheriot/cheriot_register.h"
#include "cheriot/cheriot_state.h"
-#include "cheriot/riscv_cheriot_enums.h"
+#include "mpact/sim/generic/action_point_manager_base.h"
+#include "mpact/sim/generic/breakpoint_manager.h"
#include "mpact/sim/generic/component.h"
#include "mpact/sim/generic/core_debug_interface.h"
#include "mpact/sim/generic/counters.h"
@@ -40,16 +41,18 @@
#include "mpact/sim/generic/decoder_interface.h"
#include "mpact/sim/util/memory/memory_interface.h"
#include "mpact/sim/util/memory/memory_watcher.h"
-#include "mpact/sim/util/memory/tagged_memory_interface.h"
#include "mpact/sim/util/memory/tagged_memory_watcher.h"
#include "re2/re2.h"
-#include "riscv//riscv_action_point.h"
-#include "riscv//riscv_breakpoint.h"
+#include "riscv//riscv_action_point_memory_interface.h"
namespace mpact {
namespace sim {
namespace cheriot {
+using ::mpact::sim::generic::ActionPointManagerBase;
+using ::mpact::sim::generic::BreakpointManager;
+using ::mpact::sim::riscv::RiscVActionPointMemoryInterface;
+
struct BranchTraceEntry {
uint32_t from;
uint32_t to;
@@ -165,10 +168,12 @@
CheriotState *state_;
// Flag that indicates an instruction needs to be stepped over.
bool need_to_step_over_ = false;
+ // Action point memory interface.
+ RiscVActionPointMemoryInterface *rv_ap_memory_if_ = nullptr;
// Action point manager.
- riscv::RiscVActionPointManager *rv_action_point_manager_ = nullptr;
+ ActionPointManagerBase *rv_ap_manager_ = nullptr;
// Breakpoint manager.
- riscv::RiscVBreakpointManager *rv_bp_manager_ = nullptr;
+ BreakpointManager *rv_bp_manager_ = nullptr;
// Textual description of halt reason.
std::string halt_string_;
// The pc register instance.
