[otbn,dv] Connect status output to otbn_scoreboard
This ensures that the scoreboard knows when the ISS becomes locked.
There's a bit more wiring still to do (passing out things like
FATAL_ALERT_CAUSE), but this is hopefully a reasonable start.
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv b/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv
index 169d8b0..efe0056 100644
--- a/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv
+++ b/hw/ip/otbn/dv/uvm/env/otbn_scoreboard.sv
@@ -45,10 +45,15 @@
bit saw_start_tl_trans = 1'b0;
bit waiting_for_model = 1'b0;
- // The "running" field uses the OtbnModelStart and OtbnModelDone items on the model FIFO to track
- // whether we think OTBN is running at the moment. We know that this is in sync with the RTL
- // because of the MatchingDone_A assertion at testbench level.
- bit running = 1'b0;
+ // The mirrored STATUS register from the ISS.
+ bit [7:0] model_status = 1'b0;
+
+ // The "locked" field is used to track whether OTBN is "locked". For most operational state
+ // tracking, we go through the ISS, but OTBN can become locked without actually starting an
+ // operation (for example, there might be a malformed TL transaction). We spot that sort of thing
+ // here, updating the "locked" flag if either the ISS says to do so OR if we see something 'out of
+ // band'.
+ bit locked = 1'b0;
`uvm_component_new
@@ -87,6 +92,9 @@
// Clear saw_start_tl_trans. Any "start" TL transaction will have been discarded by the reset,
// so we shouldn't still be tracking it.
saw_start_tl_trans = 1'b0;
+
+ // Clear the locked bit (this is modelling RTL state that should be cleared on reset)
+ locked = 1'b0;
endfunction
task process_tl_access(tl_seq_item item, tl_channels_e channel, string ral_name);
@@ -152,8 +160,12 @@
end
"status": begin
- // Status register
- exp_read_data = '{upd: 1'b1, chk: 1'b1, val: {31'd0, running}};
+ // Status register. If we're locked, this should read 0xff. Otherwise, fall back to the
+ // expected operational state (IDLE = 0; BUSY_EXECUTE = 1; BUSY_SEC_WIPE_DMEM = 2;
+ // BUSY_SEC_WIPE_IMEM = 3).
+ //
+ // TODO: Track states other than IDLE and BUSY_EXECUTE.
+ exp_read_data = '{upd: 1'b1, chk: 1'b1, val: locked ? 32'hff : model_status};
end
"err_bits": begin
@@ -198,10 +210,13 @@
uvm_reg_addr_t csr_addr;
otbn_exp_read_data_t exp_read_data;
- // We're only interested in reads. Ignore any acks for memory or register writes that came in on
- // the A channel.
- if (item.is_write())
+ // The data-channel response to a write is just an ack, which isn't particularly interesting.
+ // Check for integrity errors (which should lock the block), but otherwise there's nothing to
+ // do.
+ if (item.is_write()) begin
+ if (item.d_error) locked = 1'b1;
return;
+ end
// We're also only interested in registers; the scoreboard doesn't explicitly model memories in
// the RAL. Look to see if this is a valid register address. If not, it was to a memory and we
@@ -247,18 +262,20 @@
// previous cycle.
`DV_CHECK_FATAL(waiting_for_model, "model started unbidden!")
waiting_for_model = 1'b0;
- running = 1'b1;
end
- OtbnModelDone: begin
- `DV_CHECK_FATAL(running, "Got done signal when we didn't think model was running.")
- running = 1'b0;
+ OtbnModelStatus: begin
+ model_status = item.status;
end
OtbnModelInsn: begin
// The model agent's monitor should be configured to only emit OtbnModelInsn items if
// coverage is enabled.
`DV_CHECK_FATAL(cfg.en_cov)
+ // We don't expect any instructions unless we're currently running something.
+ `DV_CHECK_EQ_FATAL(model_status, 1 /* BUSY_EXECUTE */,
+ "Saw instruction when not in BUSY_EXECUTE operational state.")
+
iss_trace_queue.push_back(item);
pop_trace_queues();
end
diff --git a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_agent_pkg.sv b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_agent_pkg.sv
index 035afda..90c1271 100644
--- a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_agent_pkg.sv
+++ b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_agent_pkg.sv
@@ -13,7 +13,7 @@
typedef enum {
OtbnModelStart,
- OtbnModelDone,
+ OtbnModelStatus,
OtbnModelInsn
} otbn_model_item_type_e;
diff --git a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_if.sv b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_if.sv
index 8896856..29bdf32 100644
--- a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_if.sv
+++ b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_if.sv
@@ -19,18 +19,16 @@
bit done; // Operation done
bit err; // Something went wrong
bit [31:0] stop_pc; // PC at end of operation
+
+ // Mirrored registers
+ bit [7:0] status; // STATUS register
+
chandle handle; // Handle for DPI calls to C model
- // Wait until done goes high. Stops early on reset
- task automatic wait_done();
- while (rst_ni === 1'b1 && !done) begin
- @(posedge clk_i or negedge rst_ni);
- end
- endtask
-
- // Wait until done goes low. Stops early on reset
- task automatic wait_not_done();
- while (rst_ni === 1'b1 && done) begin
+ // Wait until reset or change of status.
+ task automatic wait_status();
+ bit old_status = status;
+ while (rst_ni === 1'b1 && status == old_status) begin
@(posedge clk_i or negedge rst_ni);
end
endtask
diff --git a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_item.sv b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_item.sv
index 0a0cc86..7ea57f0 100644
--- a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_item.sv
+++ b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_item.sv
@@ -7,7 +7,8 @@
// What sort of transaction is this?
otbn_model_item_type_e item_type;
- // Only valid when item_type == OtbnModelDone.
+ // Only valid when item_type == OtbnModelStatus.
+ bit [7:0] status;
bit err;
// Only valid when item_type == OtbnModelInsn
@@ -16,6 +17,7 @@
`uvm_object_utils_begin(otbn_model_item)
`uvm_field_enum (otbn_model_item_type_e, item_type, UVM_DEFAULT)
+ `uvm_field_int (status, UVM_DEFAULT)
`uvm_field_int (err, UVM_DEFAULT)
`uvm_field_int (insn_addr, UVM_DEFAULT | UVM_HEX)
`uvm_field_string (mnemonic, UVM_DEFAULT)
diff --git a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv
index 8ada50e..c302d1b 100644
--- a/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv
+++ b/hw/ip/otbn/dv/uvm/otbn_model_agent/otbn_model_monitor.sv
@@ -22,7 +22,7 @@
protected task collect_trans(uvm_phase phase);
fork
collect_start();
- collect_done();
+ collect_status();
// TODO: Only run when coverage is enabled.
collect_insns();
join
@@ -38,6 +38,7 @@
if (cfg.vif.start) begin
trans = otbn_model_item::type_id::create("trans");
trans.item_type = OtbnModelStart;
+ trans.status = 0;
trans.err = 0;
trans.mnemonic = "";
analysis_port.write(trans);
@@ -46,23 +47,23 @@
end
endtask
- protected task collect_done();
+ protected task collect_status();
otbn_model_item trans;
forever begin
- // wait until vif signals done (or we are in reset)
- cfg.vif.wait_done();
+ // Wait until vif signals a change in status (or we are in reset)
+ cfg.vif.wait_status();
if (cfg.vif.rst_ni === 1'b1) begin
// We aren't in reset, so we've just seen the done signal go high.
trans = otbn_model_item::type_id::create("trans");
- trans.item_type = OtbnModelDone;
+ trans.item_type = OtbnModelStatus;
+ trans.status = cfg.vif.status;
trans.err = cfg.vif.err;
trans.mnemonic = "";
analysis_port.write(trans);
- cfg.vif.wait_not_done();
end else begin
- // We are in reset. Wait until we aren't (we need to do this because wait_done() returns
+ // We are in reset. Wait until we aren't (we need to do this because wait_status() returns
// immediately in reset)
wait(cfg.vif.rst_ni);
end
@@ -89,6 +90,7 @@
if (otbn_trace_checker_pop_iss_insn(insn_addr, insn_mnemonic)) begin
trans = otbn_model_item::type_id::create("trans");
trans.item_type = OtbnModelInsn;
+ trans.status = 0;
trans.err = 0;
trans.insn_addr = insn_addr;
trans.mnemonic = insn_mnemonic;
diff --git a/hw/ip/otbn/dv/uvm/tb.sv b/hw/ip/otbn/dv/uvm/tb.sv
index 3ba5021..9534b9e 100644
--- a/hw/ip/otbn/dv/uvm/tb.sv
+++ b/hw/ip/otbn/dv/uvm/tb.sv
@@ -191,7 +191,7 @@
.edn_rnd_data_i (dut.edn_rnd_data),
.edn_urnd_data_valid_i (edn_urnd_data_valid),
- .status_o (),
+ .status_o (model_if.status),
.insn_cnt_o (model_insn_cnt),
.err_o (model_if.err)
