[otbn] Add register checking to otbn_core_model

This works by binding a "snooping" interface (otbn_rf_snooper) into
both the register files and checking a (text-based) dump from the ISS
to see whether it matches.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/otbn/dv/model/iss_wrapper.cc b/hw/ip/otbn/dv/model/iss_wrapper.cc
index 67a8463..277e7ce 100644
--- a/hw/ip/otbn/dv/model/iss_wrapper.cc
+++ b/hw/ip/otbn/dv/model/iss_wrapper.cc
@@ -171,6 +171,14 @@
   return std::string(model_path.get());
 }
 
+// Read 8 hex characters from str as a uint32_t.
+static uint32_t read_hex_32(const char *str) {
+  char buf[9];
+  memcpy(buf, str, 8);
+  buf[8] = '\0';
+  return strtoul(buf, nullptr, 16);
+}
+
 ISSWrapper::ISSWrapper() : tmpdir(new TmpDir()) {
   std::string model_path(find_otbn_model());
 
@@ -281,6 +289,87 @@
   return saw_busy_cleared(lines);
 }
 
+void ISSWrapper::get_regs(std::array<uint32_t, 32> *gprs,
+                          std::array<u256_t, 32> *wdrs) {
+  assert(gprs && wdrs);
+
+  std::vector<std::string> lines;
+  run_command("print_regs\n", &lines);
+
+  // A record of which registers we've seen (to check we see each
+  // register exactly once). GPR i sets bit i. WDR i sets bit 32 + i.
+  uint64_t seen_mask = 0;
+
+  // Lines look like
+  //
+  //  x3  = 0x12345678
+  //  w10 = 0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+
+  std::regex re("\\s*([wx][0-9]{1,2})\\s*=\\s*0x([0-9a-f]+)\n");
+  std::smatch match;
+
+  for (const std::string &line : lines) {
+    if (line == "PRINT_REGS\n")
+      continue;
+
+    if (!std::regex_match(line, match, re)) {
+      std::ostringstream oss;
+      oss << "Invalid line in ISS print_register output (`" << line << "').";
+      throw std::runtime_error(oss.str());
+    }
+
+    assert(match.size() == 3);
+
+    std::string reg_name = match[1].str();
+    std::string str_value = match[2].str();
+
+    assert(reg_name.size() <= 3);
+    assert(reg_name[0] == 'w' || reg_name[0] == 'x');
+    bool is_wide = reg_name[0] == 'w';
+    int reg_idx = atoi(reg_name.c_str() + 1);
+
+    assert(reg_idx >= 0);
+    if (reg_idx >= 32) {
+      std::ostringstream oss;
+      oss << "Invalid register name in ISS output (`" << reg_name
+          << "'). Line was `" << line << "'.";
+      throw std::runtime_error(oss.str());
+    }
+
+    unsigned idx_seen = reg_idx + (is_wide ? 32 : 0);
+    if ((seen_mask >> idx_seen) & 1) {
+      std::ostringstream oss;
+      oss << "Duplicate lines writing register " << reg_name << ".";
+      throw std::runtime_error(oss.str());
+    }
+
+    unsigned num_u32s = is_wide ? 8 : 1;
+    unsigned expected_value_len = 8 * num_u32s;
+    if (str_value.size() != expected_value_len) {
+      std::ostringstream oss;
+      oss << "Value for register " << reg_name << " has " << str_value.size()
+          << " hex characters, but we expected " << expected_value_len << ".";
+      throw std::runtime_error(oss.str());
+    }
+
+    uint32_t *dst = is_wide ? &(*wdrs)[reg_idx].words[7] : &(*gprs)[reg_idx];
+    for (unsigned i = 0; i < num_u32s; ++i) {
+      *dst = read_hex_32(&str_value[8 * i]);
+      --dst;
+    }
+
+    seen_mask |= ((uint64_t)1 << idx_seen);
+  }
+
+  // Check that we've seen all the registers
+  if (~seen_mask) {
+    std::ostringstream oss;
+    oss << "Some registers were missing from print_register output. Mask: 0x"
+        << std::hex << seen_mask << ".";
+    throw std::runtime_error(oss.str());
+  }
+}
+
 std::string ISSWrapper::make_tmp_path(const std::string &relative) const {
   return tmpdir->path + "/" + relative;
 }
diff --git a/hw/ip/otbn/dv/model/iss_wrapper.h b/hw/ip/otbn/dv/model/iss_wrapper.h
index 3a3be3b..79232ea 100644
--- a/hw/ip/otbn/dv/model/iss_wrapper.h
+++ b/hw/ip/otbn/dv/model/iss_wrapper.h
@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <array>
 #include <cstdint>
 #include <cstdio>
 #include <memory>
@@ -16,6 +17,12 @@
 
 // An object wrapping the ISS subprocess.
 struct ISSWrapper {
+  // A 256-bit unsigned integer value, stored in "LSB order". Thus, words[0]
+  // contains the LSB and words[7] contains the MSB.
+  struct u256_t {
+    uint32_t words[256 / 32];
+  };
+
   ISSWrapper();
   ~ISSWrapper();
 
@@ -33,6 +40,9 @@
   // finished (ECALL or error).
   bool step();
 
+  // Read contents of the register file
+  void get_regs(std::array<uint32_t, 32> *gprs, std::array<u256_t, 32> *wdrs);
+
   // Resolve a path relative to the convenience temporary directory.
   // relative should be a relative path (it is just appended to the
   // path of the temporary directory).
diff --git a/hw/ip/otbn/dv/model/otbn_core_model.sv b/hw/ip/otbn/dv/model/otbn_core_model.sv
index 660aa5d..3f36fa7 100644
--- a/hw/ip/otbn/dv/model/otbn_core_model.sv
+++ b/hw/ip/otbn/dv/model/otbn_core_model.sv
@@ -54,9 +54,10 @@
                              string  dmem_scope,
                              int     dmem_size);
   import "DPI-C" context function
-    int otbn_model_check_dmem(chandle model,
-                              string  dmem_scope,
-                              int     dmem_size);
+    int otbn_model_check(chandle model,
+                         string dmem_scope,
+                         int    dmem_size,
+                         string design_scope);
 
   chandle model_handle;
   initial begin
@@ -113,7 +114,7 @@
         if (DesignScope.len() == 0) begin
           void'(otbn_model_load_dmem(model_handle, DmemScope, DmemSizeWords));
         end else begin
-          void'(otbn_model_check_dmem(model_handle, DmemScope, DmemSizeWords));
+          void'(otbn_model_check(model_handle, DmemScope, DmemSizeWords, DesignScope));
         end
         new_run = 1'b0;
       end
@@ -137,4 +138,14 @@
   // Track negedges of running and expose that as a "done" output.
   assign done_o = running_r & ~running;
 
+  // If DesignScope is not empty, we have a design to check. Bind a copy of otbn_rf_snooper into
+  // each register file. The otbn_model_check() function will use these to extract memory contents.
+  generate if (DesignScope.len() != 0) begin
+    // TODO: This bind is by module, rather than by instance, because I couldn't get the by-instance
+    // syntax plus upwards name referencing to work with Verilator. Obviously, this won't work with
+    // multiple OTBN instances, so it would be nice to get it right.
+    bind otbn_rf_base_ff otbn_rf_snooper #(.Width (32), .Depth (32)) u_snooper (.rf (rf_reg));
+    bind otbn_rf_bignum_ff otbn_rf_snooper #(.Width (256), .Depth (32)) u_snooper (.rf (rf));
+  end endgenerate
+
 endmodule
diff --git a/hw/ip/otbn/dv/model/otbn_model.cc b/hw/ip/otbn/dv/model/otbn_model.cc
index b6c6772..06275c9 100644
--- a/hw/ip/otbn/dv/model/otbn_model.cc
+++ b/hw/ip/otbn/dv/model/otbn_model.cc
@@ -15,8 +15,9 @@
 #include "iss_wrapper.h"
 #include "sv_scoped.h"
 
-extern "C" int simutil_verilator_get_mem(int index, const svBitVecVal *val);
+extern "C" int simutil_verilator_get_mem(int index, svBitVecVal *val);
 extern "C" int simutil_verilator_set_mem(int index, const svBitVecVal *val);
+extern "C" int otbn_rf_peek(int index, svBitVecVal *val);
 
 // Use simutil_verilator_get_mem to read data one word at a time from the given
 // scope and collect the results up in a vector of uint8_t values.
@@ -207,52 +208,158 @@
   return 0;
 }
 
-// Grab contents of dmem from the model and compare it with the RTL. Prints
-// messages to stderr on failure or mismatch. Returns 1 for a match, 0 for a
-// mismatch, -1 for some other failure.
-extern "C" int otbn_model_check_dmem(ISSWrapper *model, const char *dmem_scope,
-                                     int dmem_words) {
+// Grab contents of dmem from the model and compare it with the RTL.
+// Prints messages to stderr on failure or mismatch. Returns true on
+// success; false on mismatch. Throws a std::runtime_error on failure.
+static bool otbn_model_check_dmem(ISSWrapper *model, const char *dmem_scope,
+                                  int dmem_words) {
   assert(model);
   assert(dmem_words >= 0);
 
   size_t dmem_bytes = dmem_words * 32;
   std::string dfname(model->make_tmp_path("dmem_out"));
-  try {
-    model->dump_d(dfname);
-    std::vector<uint8_t> iss_data = read_vector_from_file(dfname, dmem_bytes);
-    assert(iss_data.size() == dmem_bytes);
 
-    std::vector<uint8_t> rtl_data = get_sim_memory(dmem_scope, dmem_words, 32);
-    assert(rtl_data.size() == dmem_bytes);
+  model->dump_d(dfname);
+  std::vector<uint8_t> iss_data = read_vector_from_file(dfname, dmem_bytes);
+  assert(iss_data.size() == dmem_bytes);
 
-    // If the arrays match, we're done.
-    if (0 == memcmp(&iss_data[0], &rtl_data[0], dmem_bytes))
-      return 1;
+  std::vector<uint8_t> rtl_data = get_sim_memory(dmem_scope, dmem_words, 32);
+  assert(rtl_data.size() == dmem_bytes);
 
-    // If not, print out the first 10 mismatches
-    std::ios old_state(nullptr);
-    old_state.copyfmt(std::cerr);
-    std::cerr << "ERROR: Mismatches in dmem data:\n"
-              << std::hex << std::setfill('0');
-    int bad_count = 0;
-    for (size_t i = 0; i < dmem_bytes; ++i) {
-      if (iss_data[i] != rtl_data[i]) {
-        std::cerr << " @offset 0x" << std::setw(3) << i << ": rtl has 0x"
-                  << std::setw(2) << (int)rtl_data[i] << "; iss has 0x"
-                  << std::setw(2) << (int)iss_data[i] << "\n";
-        ++bad_count;
+  // If the arrays match, we're done.
+  if (0 == memcmp(&iss_data[0], &rtl_data[0], dmem_bytes))
+    return true;
 
-        if (bad_count == 10) {
-          std::cerr << " (skipping further errors...)\n";
-          break;
-        }
+  // If not, print out the first 10 mismatches
+  std::ios old_state(nullptr);
+  old_state.copyfmt(std::cerr);
+  std::cerr << "ERROR: Mismatches in dmem data:\n"
+            << std::hex << std::setfill('0');
+  int bad_count = 0;
+  for (size_t i = 0; i < dmem_bytes; ++i) {
+    if (iss_data[i] != rtl_data[i]) {
+      std::cerr << " @offset 0x" << std::setw(3) << i << ": rtl has 0x"
+                << std::setw(2) << (int)rtl_data[i] << "; iss has 0x"
+                << std::setw(2) << (int)iss_data[i] << "\n";
+      ++bad_count;
+
+      if (bad_count == 10) {
+        std::cerr << " (skipping further errors...)\n";
+        break;
       }
     }
-    std::cerr.copyfmt(old_state);
-    return 0;
+  }
+  std::cerr.copyfmt(old_state);
+  return false;
+}
 
-  } catch (const std::runtime_error &err) {
-    std::cerr << "Error when loading dmem from ISS: " << err.what() << "\n";
+template <typename T>
+static std::array<T, 32> get_rtl_regs(const std::string &reg_scope) {
+  std::array<T, 32> ret;
+  static_assert(sizeof(T) <= 256 / 8, "Can only copy 256 bits");
+
+  SVScoped scoped(reg_scope);
+
+  // otbn_rf_peek passes data as a packed array of svBitVecVal words (for a
+  // "bit [255:0]" argument). Allocate 256 bits (= 32 bytes) as
+  // 32/sizeof(svBitVecVal) words on the stack.
+  svBitVecVal buf[256 / 8 / sizeof(svBitVecVal)];
+
+  for (int i = 0; i < 32; ++i) {
+    if (!otbn_rf_peek(i, buf)) {
+      std::ostringstream oss;
+      oss << "Failed to peek into RTL to get value of register " << i
+          << " at scope `" << reg_scope << "'.";
+      throw std::runtime_error(oss.str());
+    }
+    memcpy(&ret[i], buf, sizeof(T));
+  }
+
+  return ret;
+}
+
+// Compare contents of ISS registers with those from the design. Prints
+// messages to stderr on failure or mismatch. Returns true on success; false on
+// mismatch. Throws a std::runtime_error on failure.
+static bool otbn_model_check_regs(ISSWrapper *model,
+                                  const std::string &design_scope) {
+  assert(model);
+
+  std::string base_scope =
+      design_scope + ".gen_rf_base_ff.u_otbn_rf_base.u_snooper";
+  std::string wide_scope =
+      design_scope + ".gen_rf_bignum_ff.u_otbn_rf_bignum.u_snooper";
+
+  auto rtl_gprs = get_rtl_regs<uint32_t>(base_scope);
+  auto rtl_wdrs = get_rtl_regs<ISSWrapper::u256_t>(wide_scope);
+
+  std::array<uint32_t, 32> iss_gprs;
+  std::array<ISSWrapper::u256_t, 32> iss_wdrs;
+  model->get_regs(&iss_gprs, &iss_wdrs);
+
+  bool good = true;
+
+  for (int i = 0; i < 32; ++i) {
+    if (rtl_gprs[i] != iss_gprs[i]) {
+      std::ios old_state(nullptr);
+      old_state.copyfmt(std::cerr);
+      std::cerr << std::setfill('0') << "RTL computed x" << i << " as 0x"
+                << std::hex << rtl_gprs[i] << ", but ISS got 0x" << iss_gprs[i]
+                << ".\n";
+      std::cerr.copyfmt(old_state);
+      good = false;
+    }
+  }
+  for (int i = 0; i < 32; ++i) {
+    if (0 != memcmp(rtl_wdrs[i].words, iss_wdrs[i].words,
+                    sizeof(rtl_wdrs[i].words))) {
+      std::ios old_state(nullptr);
+      old_state.copyfmt(std::cerr);
+      std::cerr << "RTL computed w" << i << " as 0x" << std::hex
+                << std::setfill('0');
+      for (int j = 0; j < 8; ++j) {
+        if (j)
+          std::cerr << "_";
+        std::cerr << std::setw(8) << rtl_wdrs[i].words[7 - j];
+      }
+      std::cerr << ", but ISS got 0x";
+      for (int j = 0; j < 8; ++j) {
+        if (j)
+          std::cerr << "_";
+        std::cerr << std::setw(8) << iss_wdrs[i].words[7 - j];
+      }
+      std::cerr << ".\n";
+      std::cerr.copyfmt(old_state);
+      good = false;
+    }
+  }
+
+  return good;
+}
+
+// Check model against RTL when a run has finished. Prints messages to
+// stderr on failure or mismatch. Returns 1 for a match, 0 for a
+// mismatch, -1 for some other failure.
+extern "C" int otbn_model_check(ISSWrapper *model, const char *dmem_scope,
+                                int dmem_words, const char *design_scope) {
+  assert(model);
+  assert(dmem_words >= 0);
+  assert(design_scope);
+
+  bool good = true;
+  try {
+    good &= otbn_model_check_dmem(model, dmem_scope, dmem_words);
+  } catch (const std::exception &err) {
+    std::cerr << "Failed to check DMEM: " << err.what() << "\n";
     return -1;
   }
+
+  try {
+    good &= otbn_model_check_regs(model, design_scope);
+  } catch (const std::exception &err) {
+    std::cerr << "Failed to check registers: " << err.what() << "\n";
+    return -1;
+  }
+
+  return good ? 1 : 0;
 }
diff --git a/hw/ip/otbn/dv/model/otbn_rf_snooper.sv b/hw/ip/otbn/dv/model/otbn_rf_snooper.sv
new file mode 100644
index 0000000..277782f
--- /dev/null
+++ b/hw/ip/otbn/dv/model/otbn_rf_snooper.sv
@@ -0,0 +1,32 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// Backdoor interface that can be bound into an OTBN register file and exports a function to peek at
+// the memory contents.
+
+interface otbn_rf_snooper #(
+  parameter int Width = 32,    // Memory width in bits
+  parameter int Depth = 32     // Number of registers
+) (
+   input logic [Width-1:0] rf [Depth]
+);
+
+  export "DPI-C" function otbn_rf_peek;
+
+  function automatic int otbn_rf_peek(input int index, output bit [255:0] val);
+    // Function only works for register files <= 256 bits wide
+    if (Width > 256) begin
+      return 0;
+    end
+
+    if (index > Depth) begin
+      return 0;
+    end
+
+    val = '0;
+    val[Width-1:0] = rf[index];
+    return 1;
+  endfunction
+
+endinterface
diff --git a/hw/ip/otbn/otbn.core b/hw/ip/otbn/otbn.core
index 77a9f41..4dff1d7 100644
--- a/hw/ip/otbn/otbn.core
+++ b/hw/ip/otbn/otbn.core
@@ -52,6 +52,7 @@
       - dv/model/iss_wrapper.cc
       - dv/model/iss_wrapper.h: { is_include_file: true }
       - dv/model/otbn_core_model.sv: { file_type: systemVerilogSource }
+      - dv/model/otbn_rf_snooper.sv: { file_type: systemVerilogSource }
     file_type: cppSource
 
   files_verilator_waiver: