[otbn] Run the ISS as a "stepped" subprocess in otbn_model
This patch doesn't actually change the behaviour of the code at all,
but should allow us to do cleverer things in future. It runs the
stepped simulator (instead of otbnsim) but, instead of running it with
system(..), it spawns a subprocess and communicates over stdin/stdout.
With future patches, we'll be able to actually run the ISS one cycle
at a time. Detection of when the ISS finishes isn't particularly
pretty (see the regex code in ISSWrapper::saw_busy_cleared), but it
should work for now. I'm assuming we'll completely overhaul the
tracing format in the reasonably near future anyway.
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
new file mode 100644
index 0000000..8a4d329
--- /dev/null
+++ b/hw/ip/otbn/dv/model/iss_wrapper.cc
@@ -0,0 +1,300 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+#include "iss_wrapper.h"
+
+#include <cassert>
+#include <cstring>
+#include <fcntl.h>
+#include <iostream>
+#include <memory>
+#include <regex>
+#include <signal.h>
+#include <sstream>
+#include <sys/stat.h>
+#include <sys/wait.h>
+
+// Guard class to safely delete C strings
+namespace {
+struct CStrDeleter {
+ void operator()(char *p) const { std::free(p); }
+};
+}
+typedef std::unique_ptr<char, CStrDeleter> c_str_ptr;
+
+// Find the otbn Python model, based on our executable path, and
+// return it. On failure, throw a std::runtime_error with a
+// description of what went wrong.
+//
+// This works by searching upwards from the binary location to find a git
+// directory (which is assumed to be the OpenTitan toplevel). It won't work if
+// you copy the binary somewhere else: if we need to support that sort of
+// thing, we'll have to figure out a "proper" installation procedure.
+static std::string find_otbn_model() {
+ c_str_ptr self_path(realpath("/proc/self/exe", NULL));
+ if (!self_path) {
+ std::ostringstream oss;
+ oss << "Cannot resolve /proc/self/exe: " << strerror(errno);
+ throw std::runtime_error(oss.str());
+ }
+
+ // Take a copy of self_path as a std::string and modify it, walking backwards
+ // over '/' characters and appending .git each time. After the first
+ // iteration, last_pos is the position of the character before the final
+ // slash (where the path looks something like "/path/to/check/.git")
+ std::string path_buf(self_path.get());
+
+ struct stat git_dir_stat;
+
+ size_t last_pos = std::string::npos;
+ for (;;) {
+ size_t last_slash = path_buf.find_last_of('/', last_pos);
+
+ // self_path was absolute, so there should always be a '/' at position
+ // zero.
+ assert(last_slash != std::string::npos);
+ if (last_slash == 0) {
+ // We've got to the slash at the start of an absolute path (and "/.git"
+ // is probably not the path we want!). Give up.
+ std::ostringstream oss;
+ oss << "Cannot find a git top-level directory containing "
+ << self_path.get() << ".\n";
+ throw std::runtime_error(oss.str());
+ }
+
+ // Replace everything after last_slash with ".git". The first time around,
+ // this will turn "/path/to/elf-file" to "/path/to/.git". After that, it
+ // will turn "/path/to/check/.git" to "/path/to/.git". Note that last_slash
+ // is strictly less than the string length (because it's an element index),
+ // so last_slash + 1 won't fall off the end.
+ path_buf.replace(last_slash + 1, std::string::npos, ".git");
+ last_pos = last_slash - 1;
+
+ // Does path_buf name a directory? If so, we've found the enclosing git
+ // directory.
+ if (stat(path_buf.c_str(), &git_dir_stat) == 0 &&
+ S_ISDIR(git_dir_stat.st_mode)) {
+ break;
+ }
+ }
+
+ // If we get here, path_buf points at a .git directory. Resolve from there to
+ // the expected model name, then use realpath to canonicalise the path. If it
+ // fails, there was no script there.
+ path_buf += "/../hw/ip/otbn/dv/otbnsim/stepped.py";
+ char *model_path = realpath(path_buf.c_str(), NULL);
+ if (!model_path) {
+ std::ostringstream oss;
+ oss << "Cannot find otbnsim.py, at '" << path_buf
+ << "' (guessed by searching upwards from '" << self_path.get()
+ << "').\n";
+ throw std::runtime_error(oss.str());
+ }
+
+ return std::string(model_path);
+}
+
+ISSWrapper::ISSWrapper() {
+ std::string model_path(find_otbn_model());
+
+ // We want two pipes: one for writing to the child process, and the other for
+ // reading from it. We set the O_CLOEXEC flag so that the child process will
+ // drop all the fds when it execs.
+ int fds[4];
+ for (int i = 0; i < 2; ++i) {
+ if (pipe2(fds + 2 * i, O_CLOEXEC)) {
+ std::ostringstream oss;
+ oss << "Failed to open pipe " << i << " for ISS: " << strerror(errno);
+ throw std::runtime_error(oss.str());
+ }
+ }
+
+ // fds[0] and fds[2] are the read ends of two pipes, with write ends at
+ // fds[1] and fds[3], respectively.
+ //
+ // We'll attach fds[0] to the child's stdin and fds[3] to the child's stdout.
+ // That means we write to fds[1] to send data to the child and read from
+ // fds[2] to get data back.
+ pid_t pid = fork();
+ if (pid == -1) {
+ // Something went wrong.
+ std::ostringstream oss;
+ oss << "Failed to fork to create ISS process: " << strerror(errno);
+ throw std::runtime_error(oss.str());
+ }
+
+ if (pid == 0) {
+ // We are the child process. Attach stdin/stdout. (No need to close the
+ // pipe fds: we'll close them as part of the exec.)
+ close(0);
+ if (dup2(fds[0], 0) == -1) {
+ std::cerr << "Failed to set stdin in ISS subprocess: " << strerror(errno)
+ << "\n";
+ abort();
+ }
+ close(1);
+ if (dup2(fds[3], 1) == -1) {
+ std::cerr << "Failed to set stdout in ISS subprocess: " << strerror(errno)
+ << "\n";
+ abort();
+ }
+ // Finally, exec the ISS
+ execl(model_path.c_str(), model_path.c_str(), NULL);
+ }
+
+ // We are the parent process and pid is the PID of the child. Close the pipe
+ // ends that we don't need (because the child is using them)
+ close(fds[0]);
+ close(fds[3]);
+
+ child_pid = pid;
+
+ // Finally, construct FILE* streams for the fds (which will make life easier
+ // when we actually use them to communicate with the child process)
+ child_write_file = fdopen(fds[1], "w");
+ child_read_file = fdopen(fds[2], "r");
+
+ // The fdopen calls should have succeeded (because we know the fds are
+ // valid). Add an assertion to make sure nothing weird happens.
+ assert(child_write_file);
+ assert(child_read_file);
+}
+
+ISSWrapper::~ISSWrapper() {
+ // Stop the child process if it's still running. No need to be nice: we'll
+ // just send a SIGKILL. Also, no need to check whether it's running first: we
+ // can just fire off the signal and ignore whether it worked or not.
+ kill(child_pid, SIGKILL);
+
+ // Now wait for the child. This should be a very short wait.
+ waitpid(child_pid, NULL, 0);
+
+ // Close the child file handles.
+ fclose(child_write_file);
+ fclose(child_read_file);
+}
+
+void ISSWrapper::load_d(const char *path) {
+ std::ostringstream oss;
+ oss << "load_d " << path << "\n";
+ run_command(oss.str(), nullptr);
+}
+
+void ISSWrapper::load_i(const char *path) {
+ std::ostringstream oss;
+ oss << "load_i " << path << "\n";
+ run_command(oss.str(), nullptr);
+}
+
+void ISSWrapper::dump_d(const char *path) const {
+ std::ostringstream oss;
+ oss << "dump_d " << path << "\n";
+ run_command(oss.str(), nullptr);
+}
+
+void ISSWrapper::start(uint32_t addr) {
+ std::ostringstream oss;
+ oss << "start " << addr << "\n";
+ run_command(oss.str(), nullptr);
+}
+
+size_t ISSWrapper::run() {
+ size_t cycles = 0;
+ std::vector<std::string> lines;
+
+ for (;;) {
+ ++cycles;
+ lines.clear();
+ run_command("step\n", &lines);
+ if (saw_busy_cleared(lines))
+ break;
+ }
+ return cycles;
+}
+
+bool ISSWrapper::read_child_response(std::vector<std::string> *dst) const {
+ char buf[256];
+ bool continuation = false;
+
+ for (;;) {
+ // fgets reads a line, or fills buf, whichever happens first. It always
+ // writes the terminating null, so setting the second last position to \0
+ // beforehand can detect whether we filled buf without needing a call to
+ // strlen: buf is full if and only if this gets written with something
+ // other than a null.
+ buf[sizeof buf - 2] = '\0';
+
+ if (!fgets(buf, sizeof buf, child_read_file)) {
+ // Failed to read from child, or EOF
+ return false;
+ }
+
+ // If buf is ".\n", and we're not continuing another line, we're done.
+ if (!continuation && (0 == strcmp(buf, ".\n"))) {
+ return true;
+ }
+
+ // Otherwise it's some informative response from the child: take a copy if
+ // dst is not null.
+ if (dst) {
+ if (continuation) {
+ assert(dst->size());
+ dst->back() += buf;
+ } else {
+ dst->push_back(std::string(buf));
+ }
+ }
+
+ // Set the continuation flag if we filled buf without a newline. Our
+ // "canary" value at the end will be \0 or \n if and only if we got a
+ // newline (or EOF) before the end of the buffer.
+ char canary = buf[sizeof buf - 2];
+ continuation = !(canary == '\0' || canary == '\n');
+ }
+}
+
+bool ISSWrapper::run_command(const std::string &cmd,
+ std::vector<std::string> *dst) const {
+ assert(cmd.size() > 0);
+ assert(cmd.back() == '\n');
+
+ fputs(cmd.c_str(), child_write_file);
+ fflush(child_write_file);
+ return read_child_response(dst);
+}
+
+bool ISSWrapper::saw_busy_cleared(std::vector<std::string> &lines) const {
+ // We're interested in lines that show an update to otbn.STATUS. These look
+ // something like this:
+ //
+ // otbn.STATUS &= ~ 0x000001 (from HW) (now 0x000000)
+ //
+ // The \n picks up the newline that we expect at the end of each line.
+ std::regex re("\\s*otbn\\.STATUS.*0x[0-9]+\\)\n");
+
+ bool is_cleared = false;
+
+ for (const auto &line : lines) {
+ if (std::regex_match(line, re)) {
+ // Ahah! We have a match. At this point, we can cheat because we happen
+ // to know that the the busy bit is bit zero, so we just need to check
+ // whether the last character of the hex constant is even. Since the
+ // regex has a fixed number (2) of characters after the hex constant, we
+ // can just count back from the end of the string.
+ char last_digit = (&line.back())[-2];
+
+ int as_num;
+ if ('0' <= last_digit && last_digit <= '9') {
+ as_num = last_digit - '0';
+ } else {
+ assert('a' <= last_digit && last_digit <= 'f');
+ as_num = 10 + (last_digit - 'a');
+ }
+
+ is_cleared = !(as_num & 1);
+ }
+ }
+
+ return is_cleared;
+}
diff --git a/hw/ip/otbn/dv/model/iss_wrapper.h b/hw/ip/otbn/dv/model/iss_wrapper.h
new file mode 100644
index 0000000..6e24f9c
--- /dev/null
+++ b/hw/ip/otbn/dv/model/iss_wrapper.h
@@ -0,0 +1,53 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+
+#include <cstdint>
+#include <cstdio>
+#include <string>
+#include <unistd.h>
+#include <vector>
+
+// An object wrapping the ISS subprocess.
+struct ISSWrapper {
+ ISSWrapper();
+ ~ISSWrapper();
+
+ // No copy constructor or assignments: we're wrapping a child process.
+ ISSWrapper(const ISSWrapper &) = delete;
+ ISSWrapper &operator=(const ISSWrapper &) = delete;
+
+ // Load new contents of DMEM / IMEM
+ void load_d(const char *path);
+ void load_i(const char *path);
+
+ // Dump the contents of DMEM to a file
+ void dump_d(const char *path) const;
+
+ // Jump to a new address and start running
+ void start(uint32_t addr);
+
+ // Run simulation until ECALL or error. Return the number of cycles
+ // until that happened.
+ size_t run();
+
+ private:
+ // Read line by line from the child process until we get ".\n".
+ // Return true if we got the ".\n" terminator, false if EOF. If dst
+ // is not null, append to it each line that was read.
+ bool read_child_response(std::vector<std::string> *dst) const;
+
+ // Send a command to the child and wait for its response. Return
+ // value and dst argument behave as for read_child_response.
+ bool run_command(const std::string &cmd, std::vector<std::string> *dst) const;
+
+ // Return true if lines contain a line that shows otbn.STATUS having
+ // its busy flag cleared.
+ bool saw_busy_cleared(std::vector<std::string> &lines) const;
+
+ pid_t child_pid;
+ FILE *child_write_file;
+ FILE *child_read_file;
+};
diff --git a/hw/ip/otbn/dv/model/otbn_model.cc b/hw/ip/otbn/dv/model/otbn_model.cc
index d2c88d9..d0c1957 100644
--- a/hw/ip/otbn/dv/model/otbn_model.cc
+++ b/hw/ip/otbn/dv/model/otbn_model.cc
@@ -14,6 +14,8 @@
#include <svdpi.h>
#include <sys/stat.h>
+#include "iss_wrapper.h"
+
// Candidate for hw/dv/verilator/cpp
/**
* Guard class for SV Scope
@@ -35,12 +37,6 @@
~SVScoped() { svSetScope(prev_scope_); }
};
-// Guard class to safely delete C strings
-struct CStrDeleter {
- void operator()(char *p) const { std::free(p); }
-};
-typedef std::unique_ptr<char, CStrDeleter> c_str_ptr;
-
extern "C" int simutil_verilator_get_mem(int index, const svBitVecVal *val);
extern "C" int simutil_verilator_set_mem(int index, const svBitVecVal *val);
@@ -124,106 +120,6 @@
}
}
-// Read a 4-byte little-endian unsigned value from the given file which is
-// supposed to represent the number of cycles taken by the simulation and
-// return it.
-//
-// Raises a runtime_error on failure.
-static uint32_t load_cycles(const std::string &path) {
- std::filebuf fb;
- if (!fb.open(path.c_str(), std::ios::in | std::ios::binary)) {
- std::ostringstream oss;
- oss << "Cannot open the file '" << path << "'.";
- throw std::runtime_error(oss.str());
- }
-
- char buf[4];
- if (fb.sgetn(buf, 4) != 4) {
- std::ostringstream oss;
- oss << "Failed to read 4 bytes from '" << path << "'.";
- throw std::runtime_error(oss.str());
- }
-
- // Re-pack the little-endian value we just read.
- uint32_t ret = 0;
- for (int i = 0; i < 4; ++i) {
- ret += (uint32_t)buf[i] << 8 * i;
- }
- return ret;
-}
-
-// Find the otbn Python model, based on our executable path, and
-// return it. On failure, throw a std::runtime_error with a
-// description of what went wrong.
-//
-// This works by searching upwards from the binary location to find a git
-// directory (which is assumed to be the OpenTitan toplevel). It won't work if
-// you copy the binary somewhere else: if we need to support that sort of
-// thing, we'll have to figure out a "proper" installation procedure.
-static std::string find_otbn_model() {
- c_str_ptr self_path(realpath("/proc/self/exe", NULL));
- if (!self_path) {
- std::ostringstream oss;
- oss << "Cannot resolve /proc/self/exe: " << strerror(errno);
- throw std::runtime_error(oss.str());
- }
-
- // Take a copy of self_path as a std::string and modify it, walking backwards
- // over '/' characters and appending .git each time. After the first
- // iteration, last_pos is the position of the character before the final
- // slash (where the path looks something like "/path/to/check/.git")
- std::string path_buf(self_path.get());
-
- struct stat git_dir_stat;
-
- size_t last_pos = std::string::npos;
- for (;;) {
- size_t last_slash = path_buf.find_last_of('/', last_pos);
-
- // self_path was absolute, so there should always be a '/' at position
- // zero.
- assert(last_slash != std::string::npos);
- if (last_slash == 0) {
- // We've got to the slash at the start of an absolute path (and "/.git"
- // is probably not the path we want!). Give up.
- std::ostringstream oss;
- oss << "Cannot find a git top-level directory containing "
- << self_path.get() << ".\n";
- throw std::runtime_error(oss.str());
- }
-
- // Replace everything after last_slash with ".git". The first time around,
- // this will turn "/path/to/elf-file" to "/path/to/.git". After that, it
- // will turn "/path/to/check/.git" to "/path/to/.git". Note that last_slash
- // is strictly less than the string length (because it's an element index),
- // so last_slash + 1 won't fall off the end.
- path_buf.replace(last_slash + 1, std::string::npos, ".git");
- last_pos = last_slash - 1;
-
- // Does path_buf name a directory? If so, we've found the enclosing git
- // directory.
- if (stat(path_buf.c_str(), &git_dir_stat) == 0 &&
- S_ISDIR(git_dir_stat.st_mode)) {
- break;
- }
- }
-
- // If we get here, path_buf points at a .git directory. Resolve from there to
- // the expected model name, then use realpath to canonicalise the path. If it
- // fails, there was no script there.
- path_buf += "/../hw/ip/otbn/dv/otbnsim/otbnsim.py";
- char *model_path = realpath(path_buf.c_str(), NULL);
- if (!model_path) {
- std::ostringstream oss;
- oss << "Cannot find otbnsim.py, at '" << path_buf
- << "' (guessed by searching upwards from '" << self_path.get()
- << "').\n";
- throw std::runtime_error(oss.str());
- }
-
- return std::string(model_path);
-}
-
// Guard class to create (and possibly delete) temporary directories.
struct TmpDir {
std::string path;
@@ -313,11 +209,11 @@
assert(dmem_words >= 0);
assert(start_addr >= 0 && start_addr < (imem_words * 4));
- std::string model_path;
+ std::unique_ptr<ISSWrapper> wrapper;
try {
- model_path = find_otbn_model();
+ wrapper.reset(new ISSWrapper());
} catch (const std::runtime_error &err) {
- std::cerr << "Error when searching for OTBN model: " << err.what() << "\n";
+ std::cerr << "Error when constructing ISS wrapper: " << err.what() << "\n";
return 0;
}
@@ -331,7 +227,6 @@
std::string ifname(tmpdir.path + "/imem");
std::string dfname(tmpdir.path + "/dmem");
- std::string cfname(tmpdir.path + "/cycles");
std::string tfname(tmpdir.path + "/trace");
try {
@@ -342,20 +237,18 @@
return 0;
}
- std::ostringstream cmd;
- cmd << model_path << " " << imem_words << " " << ifname << " " << dmem_words
- << " " << dfname << " " << cfname << " " << tfname << " " << start_addr;
-
- if (std::system(cmd.str().c_str()) != 0) {
- std::cerr << "Failed to execute model (cmd was: '" << cmd.str() << "').\n";
- return 0;
- }
-
try {
+ wrapper->load_d(dfname.c_str());
+ wrapper->load_i(ifname.c_str());
+ wrapper->start(start_addr);
+ unsigned cycles = wrapper->run();
+ wrapper->dump_d(dfname.c_str());
+
load_memory(dfname, dmem_scope, dmem_words, 32);
- return load_cycles(cfname);
+ return cycles;
+
} catch (const std::runtime_error &err) {
- std::cerr << "Error when loading sim results: " << err.what() << "\n";
+ std::cerr << "Error when running ISS: " << err.what() << "\n";
return 0;
}
}
diff --git a/hw/ip/otbn/dv/otbnsim/otbnsim.py b/hw/ip/otbn/dv/otbnsim/otbnsim.py
deleted file mode 100755
index 54730e1..0000000
--- a/hw/ip/otbn/dv/otbnsim/otbnsim.py
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/usr/bin/env python3
-# Copyright lowRISC contributors.
-# Licensed under the Apache License, Version 2.0, see LICENSE for details.
-# SPDX-License-Identifier: Apache-2.0
-
-# Execute the simulation
-
-import argparse
-import struct
-import sys
-
-from sim.decode import decode_file
-from sim.model import OTBNModel
-from sim.sim import OTBNSim
-
-
-def main() -> int:
- parser = argparse.ArgumentParser()
- parser.add_argument("imem_words", type=int)
- parser.add_argument("imem_file")
- parser.add_argument("dmem_words", type=int)
- parser.add_argument("dmem_file")
- parser.add_argument("cycles_file")
- parser.add_argument("trace_file")
- parser.add_argument("start_addr", type=int)
-
- args = parser.parse_args()
- sim = OTBNSim(OTBNModel(verbose=args.trace_file))
-
- sim.load_program(decode_file(args.imem_file))
- with open(args.dmem_file, "rb") as f:
- sim.load_data(f.read())
-
- cycles = sim.run(start_addr=args.start_addr)
-
- with open(args.dmem_file, "wb") as f:
- f.write(sim.dump_data())
-
- with open(args.cycles_file, "wb") as f:
- f.write(struct.pack("<L", cycles))
-
- return 0
-
-
-if __name__ == '__main__':
- sys.exit(main())
diff --git a/hw/ip/otbn/dv/otbnsim/stepped.py b/hw/ip/otbn/dv/otbnsim/stepped.py
index 782a0d9..843ab4e 100755
--- a/hw/ip/otbn/dv/otbnsim/stepped.py
+++ b/hw/ip/otbn/dv/otbnsim/stepped.py
@@ -148,6 +148,7 @@
handler(sim, words[1:])
print('.')
+ sys.stdout.flush()
def main() -> int:
diff --git a/hw/ip/otbn/otbn.core b/hw/ip/otbn/otbn.core
index 287a92c..c7156b9 100644
--- a/hw/ip/otbn/otbn.core
+++ b/hw/ip/otbn/otbn.core
@@ -43,9 +43,11 @@
files_model:
files:
- - dv/model/otbn_model.cc: { file_type: cppSource }
- - dv/model/otbn_core_model.sv
- file_type: systemVerilogSource
+ - dv/model/otbn_model.cc
+ - dv/model/iss_wrapper.cc
+ - dv/model/iss_wrapper.h: { is_include_file: true }
+ - dv/model/otbn_core_model.sv: { file_type: systemVerilogSource }
+ file_type: cppSource
files_verilator_waiver:
depend:
