[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 ®_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: