Changed how caches are instantiated and configured. Since it is preferred to have absl::Flags all in one file (the file with, main()), how that flag is used to configure caches was changed to use config objects in the X_top classes. PiperOrigin-RevId: 679300970 Change-Id: Ic144279e37fb2e2c7730d63e7ff35a3326ea7c62
diff --git a/cheriot/BUILD b/cheriot/BUILD index 87dd61c..365d1c2 100644 --- a/cheriot/BUILD +++ b/cheriot/BUILD
@@ -506,6 +506,7 @@ "@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:config", "@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",
diff --git a/cheriot/cheriot_renode.cc b/cheriot/cheriot_renode.cc index 30ea54a..da29a83 100644 --- a/cheriot/cheriot_renode.cc +++ b/cheriot/cheriot_renode.cc
@@ -23,7 +23,6 @@ #include <ios> #include <iostream> #include <memory> -#include <new> #include <string> #include <string_view> @@ -79,6 +78,7 @@ namespace cheriot { using ::mpact::sim::proto::ComponentData; +using ::mpact::sim::proto::ComponentValueEntry; using ::mpact::sim::riscv::RiscVClint; using ::mpact::sim::riscv::RiscVCounterCsr; using ::mpact::sim::riscv::RiscVCounterCsrHigh; @@ -103,6 +103,8 @@ constexpr std::string_view kWaitForCLI = "waitForCLI"; constexpr std::string_view kInstProfile = "instProfile"; constexpr std::string_view kMemProfile = "memProfile"; +constexpr std::string_view kICache = "iCache"; +constexpr std::string_view kDCache = "dCache"; // Cpu names constexpr std::string_view kBaseName = "Mpact.Cheriot"; constexpr std::string_view kRvvName = "Mpact.CheriotRvv"; @@ -166,8 +168,8 @@ delete cheriot_renode_cli_top_; delete cheriot_cli_forwarder_; delete cheriot_decoder_; - delete cheriot_state_; delete cheriot_top_; + delete cheriot_state_; delete semihost_; delete router_; delete atomic_memory_; @@ -342,6 +344,8 @@ absl::Status CheriotRenode::SetConfig(const char *config_names[], const char *config_values[], int size) { + std::string icache_cfg; + std::string dcache_cfg; uint64_t tagged_memory_base = 0; uint64_t tagged_memory_size = 0; uint64_t revocation_memory_base = 0; @@ -353,32 +357,41 @@ for (int i = 0; i < size; ++i) { std::string name(config_names[i]); std::string str_value = config_values[i]; - auto res = ParseNumber(str_value); - uint64_t value = 0; - if (!res.ok()) { - return res.status(); - } - value = res.value(); - if (name == kTaggedMemoryBase) { - tagged_memory_base = value; - } else if (name == kTaggedMemorySize) { - tagged_memory_size = value; - } else if (name == kRevocationMemoryBase) { - revocation_memory_base = value; - } else if (name == kClintMMRBase) { - clint_mmr_base = value; - } else if (name == kClintPeriod) { - clint_period = value; - } else if (name == kCLIPort) { - cli_port = value; - } else if (name == kWaitForCLI) { - wait_for_cli = value; - } else if (name == kInstProfile) { - do_inst_profile = value != 0; - } else if (name == kMemProfile) { - mem_profiler_->set_is_enabled(value != 0); + // First check string valued config values. + if (name == kICache) { + icache_cfg = str_value; + } else if (name == kDCache) { + dcache_cfg = str_value; } else { - LOG(ERROR) << "Unknown config name: " << name << " " << config_values[i]; + // Numeric config values. + auto res = ParseNumber(str_value); + uint64_t value = 0; + if (!res.ok()) { + return res.status(); + } + value = res.value(); + if (name == kTaggedMemoryBase) { + tagged_memory_base = value; + } else if (name == kTaggedMemorySize) { + tagged_memory_size = value; + } else if (name == kRevocationMemoryBase) { + revocation_memory_base = value; + } else if (name == kClintMMRBase) { + clint_mmr_base = value; + } else if (name == kClintPeriod) { + clint_period = value; + } else if (name == kCLIPort) { + cli_port = value; + } else if (name == kWaitForCLI) { + wait_for_cli = value; + } else if (name == kInstProfile) { + do_inst_profile = value != 0; + } else if (name == kMemProfile) { + mem_profiler_->set_is_enabled(value != 0); + } else { + LOG(ERROR) << "Unknown config name: " << name << " " + << config_values[i]; + } } } if (tagged_memory_size == 0) { @@ -441,6 +454,22 @@ absl::StrCat("Failed to create socket CLI (", errno, ")")); } } + if (!icache_cfg.empty()) { + ComponentValueEntry icache_value; + icache_value.set_name("icache"); + icache_value.set_string_value(icache_cfg); + auto *cfg = cheriot_top_->GetConfig("icache"); + auto status = cfg->Import(&icache_value); + if (!status.ok()) return status; + } + if (!dcache_cfg.empty()) { + ComponentValueEntry dcache_value; + dcache_value.set_name("dcache"); + dcache_value.set_string_value(dcache_cfg); + auto *cfg = cheriot_top_->GetConfig("dcache"); + auto status = cfg->Import(&dcache_value); + if (!status.ok()) return status; + } return absl::OkStatus(); }
diff --git a/cheriot/cheriot_top.cc b/cheriot/cheriot_top.cc index 9c663d0..d683b61 100644 --- a/cheriot/cheriot_top.cc +++ b/cheriot/cheriot_top.cc
@@ -21,7 +21,6 @@ #include <thread> // NOLINT: third party code. #include <utility> -#include "absl/flags/flag.h" #include "absl/functional/any_invocable.h" #include "absl/functional/bind_front.h" #include "absl/log/check.h" @@ -55,9 +54,6 @@ #include "riscv//riscv_csr.h" #include "riscv//riscv_register.h" -// Flag to enable and configure instruction cache. -ABSL_FLAG(std::string, icache, "", "Instruction cache configuration"); - namespace mpact { namespace sim { namespace cheriot { @@ -78,8 +74,20 @@ counter_num_cycles_("num_cycles", 0), counter_pc_("pc", 0), cap_reg_re_{ - R"((\w+)\.(top|base|length|tag|permissions|object_type|reserved))"} { + R"((\w+)\.(top|base|length|tag|permissions|object_type|reserved))"}, + icache_config_("icache", ""), + dcache_config_("dcache", "") { CHECK_OK(AddChildComponent(*state_)); + // Register icache configuration, and set a callback for when the config + // entry is written to. + CHECK_OK(AddConfig(&icache_config_)); + icache_config_.AddValueWrittenCallback( + [this]() { ConfigureCache(icache_, icache_config_); }); + // Register dcache configuration, and set a callback for when the config + // entry is written to. + CHECK_OK(AddConfig(&dcache_config_)); + dcache_config_.AddValueWrittenCallback( + [this]() { ConfigureCache(dcache_, dcache_config_); }); Initialize(); } @@ -152,16 +160,7 @@ } return false; }); - // Instruction cache. - if (!absl::GetFlag(FLAGS_icache).empty()) { - icache_ = new Cache("icache", this); - absl::Status status = - icache_->Configure(absl::GetFlag(FLAGS_icache), &counter_num_cycles_); - if (!status.ok()) { - LOG(ERROR) << "Failed to configure instruction cache: " << status; - } - inst_db_ = state_->db_factory()->Allocate<uint32_t>(1); - } + inst_db_ = db_factory_.Allocate<uint32_t>(1); // Make sure the architectural and abi register aliases are added. std::string reg_name; @@ -191,6 +190,22 @@ } } +void CheriotTop::ConfigureCache(Cache *&cache, Config<std::string> &config) { + if (cache != nullptr) { + LOG(WARNING) << "Cache already configured - ignored"; + return; + } + auto cfg_str = config.GetValue(); + if (cfg_str.empty()) { + LOG(WARNING) << "Cache configuration is empty - ignored"; + } + cache = new Cache(config.name(), this); + absl::Status status = cache->Configure(cfg_str, &counter_num_cycles_); + if (!status.ok()) { + LOG(ERROR) << "Failed to configure instruction cache: " << status.message(); + } +} + bool CheriotTop::ExecuteInstruction(Instruction *inst) { // Check that pcc has tag set. if (!pcc_->tag()) {
diff --git a/cheriot/cheriot_top.h b/cheriot/cheriot_top.h index e187fac..131f531 100644 --- a/cheriot/cheriot_top.h +++ b/cheriot/cheriot_top.h
@@ -33,6 +33,7 @@ #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/config.h" #include "mpact/sim/generic/core_debug_interface.h" #include "mpact/sim/generic/counters.h" #include "mpact/sim/generic/data_buffer.h" @@ -51,6 +52,7 @@ using ::mpact::sim::generic::ActionPointManagerBase; using ::mpact::sim::generic::BreakpointManager; +using ::mpact::sim::generic::Config; using ::mpact::sim::generic::DecoderInterface; using ::mpact::sim::riscv::RiscVActionPointMemoryInterface; using ::mpact::sim::util::Cache; @@ -152,9 +154,14 @@ const std::string &halt_string() const { return halt_string_; } void set_halt_string(std::string halt_string) { halt_string_ = halt_string; } + Cache *icache() const { return icache_; } + Cache *dcache() const { return dcache_; } + private: // Initialize the top. void Initialize(); + // Configure cache helper method. + void ConfigureCache(Cache *&cache, Config<std::string> &config); // Execute instruction. Returns true if the instruction was executed (or // an exception was triggered). bool ExecuteInstruction(Instruction *inst); @@ -218,7 +225,11 @@ LazyRE2 cap_reg_re_; // Flag for breaking on a control flow change. bool break_on_control_flow_change_ = false; - // ICache. + // Configuration items. + Config<std::string> icache_config_; + Config<std::string> dcache_config_; + // ICache & DCache. + Cache *dcache_ = nullptr; Cache *icache_ = nullptr; DataBuffer *inst_db_ = nullptr; };
diff --git a/cheriot/mpact_cheriot.cc b/cheriot/mpact_cheriot.cc index 3401dd7..dc6185f 100644 --- a/cheriot/mpact_cheriot.cc +++ b/cheriot/mpact_cheriot.cc
@@ -20,7 +20,6 @@ #include <ios> #include <iostream> #include <memory> -#include <new> #include <optional> #include <ostream> #include <string> @@ -157,6 +156,9 @@ ABSL_FLAG(uint64_t, clint, 0x0200'0000ULL, "Base address of clint"); ABSL_FLAG(uint64_t, uart, 0x1000'0000ULL, "Base address of uart"); +// Flag to enable and configure the instruction cache. +ABSL_FLAG(std::string, icache, "", "Instruction cache configuration"); + constexpr char kStackEndSymbolName[] = "__stack_end"; constexpr char kStackSizeSymbolName[] = "__stack_size"; @@ -165,6 +167,7 @@ using HaltReason = ::mpact::sim::generic::CoreDebugInterface::HaltReason; using ::mpact::sim::cheriot::CheriotTop; using ::mpact::sim::generic::Instruction; +using ::mpact::sim::proto::ComponentValueEntry; using ::mpact::sim::riscv::RiscVArmSemihost; using ::mpact::sim::riscv::RiscVClint; using ::mpact::sim::util::AtomicMemoryOpInterface; @@ -289,6 +292,17 @@ CheriotTop cheriot_top("Cheriot", &cheriot_state, decoder); + if (!absl::GetFlag(FLAGS_icache).empty()) { + ComponentValueEntry icache_value; + icache_value.set_name("icache"); + icache_value.set_string_value(absl::GetFlag(FLAGS_icache)); + auto *cfg = cheriot_top.GetConfig("icache"); + auto status = cfg->Import(&icache_value); + if (!status.ok()) return -1; + } + + // TODO: enable dcache. + // Enable instruction profiling if the flag is set. InstructionProfiler *inst_profiler = nullptr; if (absl::GetFlag(FLAGS_inst_profile)) {