Make config settings non-global (#1535)

Move the `config_..` settings into `CLIOptions` and `ModelImpl` so they
are no longer global.

Author: Timmmm

c_emulator/CMakeLists.txt

@@ -30,7 +30,6 @@ add_library(riscv_model
     elf_loader.h
     riscv_callbacks_if.cpp
     riscv_callbacks_if.h
-    riscv_config.h
     riscv_platform_if.cpp
     riscv_platform_if.h
     riscv_model_impl.cpp

c_emulator/riscv_callbacks_log.cpp

@@ -1,5 +1,4 @@
 #include "riscv_callbacks_log.h"
-#include "riscv_config.h"
 #include "sail_riscv_model.h"
 #include <inttypes.h>
 #include <stdlib.h>

c_emulator/riscv_config.h

@@ -5,7 +5,6 @@
 #include <unistd.h>
 
 #include "riscv_callbacks_if.h"
-#include "riscv_config.h"
 #include "symbol_table.h"
 
 int term_fd = 1; // set during startup
@@ -226,36 +225,76 @@ unit ModelImpl::print_log_instr(const_sail_string s, uint64_t pc) {
 }
 
 unit ModelImpl::print_step(unit) {
-  if (config_print_step) {
+  if (m_config_print_step) {
     fprintf(trace_log, "\n");
   }
   return UNIT;
 }
 
 bool ModelImpl::get_config_print_instr(unit) {
-  return config_print_instr;
+  return m_config_print_instr;
 }
 
 bool ModelImpl::get_config_print_clint(unit) {
-  return config_print_clint;
+  return m_config_print_clint;
 }
+
 bool ModelImpl::get_config_print_exception(unit) {
-  return config_print_exception;
+  return m_config_print_exception;
 }
+
 bool ModelImpl::get_config_print_interrupt(unit) {
-  return config_print_interrupt;
+  return m_config_print_interrupt;
 }
+
 bool ModelImpl::get_config_print_htif(unit) {
-  return config_print_htif;
+  return m_config_print_htif;
 }
+
 bool ModelImpl::get_config_print_pma(unit) {
-  return config_print_pma;
+  return m_config_print_pma;
 }
 
 bool ModelImpl::get_config_rvfi(unit) {
-  return config_enable_rvfi;
+  return m_config_rvfi;
 }
 
 bool ModelImpl::get_config_use_abi_names(unit) {
-  return config_use_abi_names;
+  return m_config_use_abi_names;
+}
+
+void ModelImpl::set_config_print_instr(bool on) {
+  m_config_print_instr = on;
+}
+
+void ModelImpl::set_config_print_clint(bool on) {
+  m_config_print_clint = on;
+}
+
+void ModelImpl::set_config_print_exception(bool on) {
+  m_config_print_exception = on;
+}
+
+void ModelImpl::set_config_print_interrupt(bool on) {
+  m_config_print_interrupt = on;
+}
+
+void ModelImpl::set_config_print_htif(bool on) {
+  m_config_print_htif = on;
+}
+
+void ModelImpl::set_config_print_pma(bool on) {
+  m_config_print_pma = on;
+}
+
+void ModelImpl::set_config_rvfi(bool on) {
+  m_config_rvfi = on;
+}
+
+void ModelImpl::set_config_use_abi_names(bool on) {
+  m_config_use_abi_names = on;
+}
+
+void ModelImpl::set_config_print_step(bool on) {
+  m_config_print_step = on;
 }

c_emulator/riscv_model_impl.cpp

@@ -24,6 +24,17 @@ class ModelImpl final : public hart::Model {
   void set_enable_experimental_extensions(bool en);
   void set_reservation_set_size_exp(uint64_t exponent);
 
+  void set_config_print_instr(bool on);
+  void set_config_print_clint(bool on);
+  void set_config_print_exception(bool on);
+  void set_config_print_interrupt(bool on);
+  void set_config_print_htif(bool on);
+  void set_config_print_pma(bool on);
+  void set_config_rvfi(bool on);
+  void set_config_use_abi_names(bool on);
+
+  void set_config_print_step(bool on);
+
 private:
   // These functions are called by the Sail code.
 
@@ -79,6 +90,17 @@ class ModelImpl final : public hart::Model {
   bool get_config_rvfi(unit) override;
   bool get_config_use_abi_names(unit) override;
 
+  bool m_config_print_instr = false;
+  bool m_config_print_clint = false;
+  bool m_config_print_exception = false;
+  bool m_config_print_interrupt = false;
+  bool m_config_print_htif = false;
+  bool m_config_print_pma = false;
+  bool m_config_rvfi = false;
+  bool m_config_use_abi_names = false;
+
+  bool m_config_print_step = false;
+
   // TODO: Probably better with std::shared_ptr<callbacks_if>.
   std::vector<callbacks_if *> m_callbacks;
 

c_emulator/riscv_model_impl.h

@@ -68,23 +68,6 @@ ModelImpl g_model;
 
 FILE *trace_log = stdout;
 
-bool config_print_instr = false;
-bool config_print_reg = false;
-bool config_print_mem_access = false;
-bool config_print_clint = false;
-bool config_print_exception = false;
-bool config_print_interrupt = false;
-bool config_print_htif = false;
-bool config_print_pma = false;
-bool config_print_rvfi = false;
-bool config_print_step = false;
-bool config_print_ptw = false;
-
-bool config_use_abi_names = false;
-bool config_enable_rvfi = false;
-
-bool config_enable_experimental_extensions = false;
-
 static void print_dts() {
   char *dts = nullptr;
   g_model.zgenerate_dts(&dts, UNIT);
@@ -140,6 +123,22 @@ struct CLIOptions {
 #ifdef SAILCOV
   std::string sailcov_file;
 #endif
+
+  bool config_print_instr = false;
+  bool config_print_reg = false;
+  bool config_print_mem_access = false;
+  bool config_print_clint = false;
+  bool config_print_exception = false;
+  bool config_print_interrupt = false;
+  bool config_print_htif = false;
+  bool config_print_pma = false;
+  bool config_print_rvfi = false;
+  bool config_print_step = false;
+  bool config_print_ptw = false;
+
+  bool config_use_abi_names = false;
+
+  bool config_enable_experimental_extensions = false;
 };
 
 // Parse CLI options. This calls `exit()` on failure.
@@ -164,10 +163,10 @@ static CLIOptions parse_cli(int argc, char **argv) {
   app.add_flag("--print-isa-string", opts.do_print_isa, "Print ISA string");
   app.add_flag(
     "--enable-experimental-extensions",
-    config_enable_experimental_extensions,
+    opts.config_enable_experimental_extensions,
     "Enable experimental extensions"
   );
-  app.add_flag("--use-abi-names", config_use_abi_names, "Use ABI register names in trace log");
+  app.add_flag("--use-abi-names", opts.config_use_abi_names, "Use ABI register names in trace log");
 
   app.add_option("--device-tree-blob", opts.dtb_file, "Device tree blob file")
     ->check(CLI::ExistingFile)
@@ -186,44 +185,44 @@ static CLIOptions parse_cli(int argc, char **argv) {
   app.add_option("--sailcov-file", sailcov_file, "Sail coverage output file")->option_text("<file>");
 #endif
 
-  app.add_flag("--trace-instr", config_print_instr, "Enable trace output for instruction execution");
-  app.add_flag("--trace-ptw", config_print_ptw, "Enable trace output for Page Table walk");
-  app.add_flag("--trace-reg", config_print_reg, "Enable trace output for register access");
-  app.add_flag("--trace-mem", config_print_mem_access, "Enable trace output for memory accesses");
-  app.add_flag("--trace-rvfi", config_print_rvfi, "Enable trace output for RVFI");
-  app.add_flag("--trace-clint", config_print_clint, "Enable trace output for CLINT memory accesses and status");
-  app.add_flag("--trace-exception", config_print_exception, "Enable trace output for exceptions");
-  app.add_flag("--trace-interrupt", config_print_interrupt, "Enable trace output for interrupts");
-  app.add_flag("--trace-htif", config_print_htif, "Enable trace output for HTIF operations");
-  app.add_flag("--trace-pma", config_print_pma, "Enable trace output for PMA checks");
+  app.add_flag("--trace-instr", opts.config_print_instr, "Enable trace output for instruction execution");
+  app.add_flag("--trace-ptw", opts.config_print_ptw, "Enable trace output for Page Table walk");
+  app.add_flag("--trace-reg", opts.config_print_reg, "Enable trace output for register access");
+  app.add_flag("--trace-mem", opts.config_print_mem_access, "Enable trace output for memory accesses");
+  app.add_flag("--trace-rvfi", opts.config_print_rvfi, "Enable trace output for RVFI");
+  app.add_flag("--trace-clint", opts.config_print_clint, "Enable trace output for CLINT memory accesses and status");
+  app.add_flag("--trace-exception", opts.config_print_exception, "Enable trace output for exceptions");
+  app.add_flag("--trace-interrupt", opts.config_print_interrupt, "Enable trace output for interrupts");
+  app.add_flag("--trace-htif", opts.config_print_htif, "Enable trace output for HTIF operations");
+  app.add_flag("--trace-pma", opts.config_print_pma, "Enable trace output for PMA checks");
   app.add_flag_callback(
     "--trace-platform",
-    [] {
-      config_print_clint = true;
-      config_print_exception = true;
-      config_print_interrupt = true;
-      config_print_htif = true;
-      config_print_pma = true;
+    [&opts] {
+      opts.config_print_clint = true;
+      opts.config_print_exception = true;
+      opts.config_print_interrupt = true;
+      opts.config_print_htif = true;
+      opts.config_print_pma = true;
     },
     "Enable trace output for platform-level events (MMIO, interrupts, "
     "exceptions, CLINT, HTIF, PMA)"
   );
-  app.add_flag("--trace-step", config_print_step, "Add a blank line between steps in the trace output");
+  app.add_flag("--trace-step", opts.config_print_step, "Add a blank line between steps in the trace output");
 
   app.add_flag_callback(
     "--trace-all",
-    [] {
-      config_print_instr = true;
-      config_print_reg = true;
-      config_print_mem_access = true;
-      config_print_rvfi = true;
-      config_print_clint = true;
-      config_print_exception = true;
-      config_print_interrupt = true;
-      config_print_htif = true;
-      config_print_pma = true;
-      config_print_step = true;
-      config_print_ptw = true;
+    [&opts] {
+      opts.config_print_instr = true;
+      opts.config_print_reg = true;
+      opts.config_print_mem_access = true;
+      opts.config_print_rvfi = true;
+      opts.config_print_clint = true;
+      opts.config_print_exception = true;
+      opts.config_print_interrupt = true;
+      opts.config_print_htif = true;
+      opts.config_print_pma = true;
+      opts.config_print_step = true;
+      opts.config_print_ptw = true;
     },
     "Enable all trace output"
   );
@@ -433,11 +432,9 @@ void finish(const CLIOptions &opts) {
 }
 
 void flush_logs() {
-  if (config_print_instr) {
-    fflush(stderr);
-    fflush(stdout);
-    fflush(trace_log);
-  }
+  fflush(stderr);
+  fflush(stdout);
+  fflush(trace_log);
 }
 
 void run_sail(const CLIOptions &opts) {
@@ -454,7 +451,7 @@ void run_sail(const CLIOptions &opts) {
 
   while (!g_model.zhtif_done && (opts.insn_limit == 0 || total_insns < opts.insn_limit)) {
     if (rvfi.has_value()) {
-      switch (rvfi->pre_step(config_print_rvfi)) {
+      switch (rvfi->pre_step(opts.config_print_rvfi)) {
       case RVFI_prestep_continue:
         continue;
       case RVFI_prestep_eof:
@@ -477,17 +474,19 @@ void run_sail(const CLIOptions &opts) {
       if (g_model.have_exception) {
         break;
       }
-      flush_logs();
+      if (opts.config_print_instr) {
+        flush_logs();
+      }
       KILL(sail_int)(&sail_step);
       if (rvfi) {
-        rvfi->send_trace(config_print_rvfi);
+        rvfi->send_trace(opts.config_print_rvfi);
       }
     }
 
     g_model.call_post_step_callbacks(is_waiting);
 
     if (!is_waiting) {
-      if (config_print_step) {
+      if (opts.config_print_step) {
         fprintf(trace_log, "\n");
       }
       step_no++;
@@ -569,7 +568,6 @@ int inner_main(int argc, char **argv) {
     return EXIT_SUCCESS;
   }
   if (opts.rvfi_dii_port != 0) {
-    config_enable_rvfi = true;
     rvfi.emplace(opts.rvfi_dii_port, g_model);
   }
   if (opts.do_show_times) {
@@ -584,14 +582,25 @@ int inner_main(int argc, char **argv) {
   if (opts.signature_granularity != DEFAULT_SIGNATURE_GRANULARITY) {
     fprintf(stderr, "setting signature-granularity to %d bytes\n", opts.signature_granularity);
   }
-  if (config_enable_experimental_extensions) {
+  if (opts.config_enable_experimental_extensions) {
     fprintf(stderr, "enabling unratified extensions.\n");
     g_model.set_enable_experimental_extensions(true);
   }
   if (!opts.trace_log_path.empty()) {
     fprintf(stderr, "using %s for trace output.\n", opts.trace_log_path.c_str());
   }
 
+  g_model.set_config_print_instr(opts.config_print_instr);
+  g_model.set_config_print_clint(opts.config_print_clint);
+  g_model.set_config_print_exception(opts.config_print_exception);
+  g_model.set_config_print_interrupt(opts.config_print_interrupt);
+  g_model.set_config_print_htif(opts.config_print_htif);
+  g_model.set_config_print_pma(opts.config_print_pma);
+  g_model.set_config_rvfi(rvfi.has_value());
+  g_model.set_config_use_abi_names(opts.config_use_abi_names);
+
+  g_model.set_config_print_step(opts.config_print_step);
+
   // Always validate the schema conformance of the config.
   validate_config_schema(opts.config_file);
 
@@ -640,13 +649,19 @@ int inner_main(int argc, char **argv) {
   }
 
   init_logs(opts);
-  log_callbacks log_cbs(config_print_reg, config_print_mem_access, config_print_ptw, config_use_abi_names, trace_log);
+  log_callbacks log_cbs(
+    opts.config_print_reg,
+    opts.config_print_mem_access,
+    opts.config_print_ptw,
+    opts.config_use_abi_names,
+    trace_log
+  );
   g_model.register_callback(&log_cbs);
 
   init_start = steady_clock::now();
 
   if (rvfi) {
-    if (!rvfi->setup_socket(config_print_rvfi)) {
+    if (!rvfi->setup_socket(opts.config_print_rvfi)) {
       return 1;
     }
     g_model.register_callback(&rvfi_cbs);