[otbn,dv] Add flag write coverage points
This means we have to split up the "bna" and "bnaf" encodings, which
previously used the same coverage tracking code ("bna" instructions
don't write the carry flag).
There's also a little bit of cleverness in defining
DEF_MLZ_FLAGS_TOGGLE_COV, where we want to track bits 1, 2 and 3. The
call to DEF_TOGGLE_COV_1(..., 01) tracks bit 2'b01. The call to
DEF_TOGGLE_COV_2(..., 1) tracks bits 2'b10 and 2'b11.
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/otbn/doc/dv/index.md b/hw/ip/otbn/doc/dv/index.md
index a58e042..752f9ab 100644
--- a/hw/ip/otbn/doc/dv/index.md
+++ b/hw/ip/otbn/doc/dv/index.md
@@ -181,6 +181,16 @@
For example, `BN.ADD` can write to each of the flags `C`, `M`, `L` and `Z`.
This paragraph implies eight coverage points (four flags times two values) for that instruction.
+> Again, the code to track this is split by encoding schema in `otbn_env_cov`.
+> The trace interface takes a copy of flag write data.
+> It doesn't bother storing the flag write flags, since these are implied by the instruction anyway.
+> There is a coverage coverpoint tracking both values for each of the flags that can be written.
+> This is then crossed with the instruction mnemonic.
+> For example, the coverpoint for the C flag (bit zero) in the `bnaf` encoding used by `BN.ADD` is called `flags_00_cp`.
+> Some instructions only write the `M`, `L` and `Z` flags.
+> These are found in the `bna`, `bnan`, `bnaqs` and `bnaqw` encoding groups.
+> For these instructions, we only track bits `1`, `2` and `3` of the flags structure.
+
#### ADD
This instruction uses the `R` encoding schema, with covergroup `enc_r_cg`.
diff --git a/hw/ip/otbn/dv/uvm/env/otbn_env_cov.sv b/hw/ip/otbn/dv/uvm/env/otbn_env_cov.sv
index 59a0234..ef74440 100644
--- a/hw/ip/otbn/dv/uvm/env/otbn_env_cov.sv
+++ b/hw/ip/otbn/dv/uvm/env/otbn_env_cov.sv
@@ -146,6 +146,12 @@
`define DEF_WDR_TOGGLE_COV(BASE, BITS) \
`_DEF_TOGGLE_COV_128(BASE, BITS, 8, 0) \
`_DEF_TOGGLE_COV_128(BASE, BITS, 8, 1)
+`define DEF_FLAGS_TOGGLE_COV(BASE, BITS) \
+ `_DEF_TOGGLE_COV_2(BASE, BITS, 2, 0) \
+ `_DEF_TOGGLE_COV_2(BASE, BITS, 2, 1)
+`define DEF_MLZ_FLAGS_TOGGLE_COV(BASE, BITS) \
+ `_DEF_TOGGLE_COV_1(BASE, BITS, 2, 01) \
+ `_DEF_TOGGLE_COV_2(BASE, BITS, 2, 1)
// Macros to allow crossing the "toggle" coverpoints defined by the previous macros with the
// mnemonic coverpoint for some encoding schema. These work just as above and the entry points to
@@ -183,21 +189,22 @@
`define DEF_WDR_TOGGLE_CROSS(BASE) \
`_DEF_TOGGLE_CROSS_128(BASE, 0) \
`_DEF_TOGGLE_CROSS_128(BASE, 1)
+`define DEF_FLAGS_TOGGLE_CROSS(BASE) \
+ `_DEF_TOGGLE_CROSS_2(BASE, 0) \
+ `_DEF_TOGGLE_CROSS_2(BASE, 1)
+`define DEF_MLZ_FLAGS_TOGGLE_CROSS(BASE) \
+ `_DEF_TOGGLE_CROSS_1(BASE, 01) \
+ `_DEF_TOGGLE_CROSS_2(BASE, 1)
// Per-encoding covergroups
covergroup enc_bna_cg
- with function sample(mnem_str_t mnemonic,
- logic [31:0] insn_data,
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
logic [255:0] wdr_operand_a,
- logic [255:0] wdr_operand_b);
+ logic [255:0] wdr_operand_b,
+ flags_t flags_write_data [2]);
- // Used for bna and bnaf encodings (which have the same operand layout: the only difference is
- // in the layout of the fixed bits)
mnemonic_cp: coverpoint mnemonic {
- `DEF_MNEM_BIN(mnem_bn_add);
- `DEF_MNEM_BIN(mnem_bn_addc);
- `DEF_MNEM_BIN(mnem_bn_sub);
- `DEF_MNEM_BIN(mnem_bn_subb);
`DEF_MNEM_BIN(mnem_bn_and);
`DEF_MNEM_BIN(mnem_bn_or);
`DEF_MNEM_BIN(mnem_bn_xor);
@@ -215,12 +222,49 @@
`DEF_WDR_TOGGLE_COV(wrs2, wdr_operand_b)
`DEF_WDR_TOGGLE_CROSS(wrs1)
`DEF_WDR_TOGGLE_CROSS(wrs2)
+
+ // BNA instructions can write the M, L and Z flags, but do not affect the carry flag (bit 0 in
+ // the flags_t struct).
+ `DEF_MLZ_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
+ `DEF_MLZ_FLAGS_TOGGLE_CROSS(flags)
+ endgroup
+
+ covergroup enc_bnaf_cg
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
+ logic [255:0] wdr_operand_a,
+ logic [255:0] wdr_operand_b,
+ flags_t flags_write_data [2]);
+
+ mnemonic_cp: coverpoint mnemonic {
+ `DEF_MNEM_BIN(mnem_bn_add);
+ `DEF_MNEM_BIN(mnem_bn_addc);
+ `DEF_MNEM_BIN(mnem_bn_sub);
+ `DEF_MNEM_BIN(mnem_bn_subb);
+ illegal_bins other = default;
+ }
+
+ sb_cp: coverpoint insn_data[29:25] { bins extremes[] = {'0, '1}; }
+ st_cp: coverpoint insn_data[30];
+ fg_cp: coverpoint insn_data[31];
+ `DEF_MNEM_CROSS(sb)
+ `DEF_MNEM_CROSS(st)
+ `DEF_MNEM_CROSS(fg)
+
+ `DEF_WDR_TOGGLE_COV(wrs1, wdr_operand_a)
+ `DEF_WDR_TOGGLE_COV(wrs2, wdr_operand_b)
+ `DEF_WDR_TOGGLE_CROSS(wrs1)
+ `DEF_WDR_TOGGLE_CROSS(wrs2)
+
+ `DEF_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
+ `DEF_FLAGS_TOGGLE_CROSS(flags)
endgroup
covergroup enc_bnai_cg
- with function sample(mnem_str_t mnemonic,
- logic [31:0] insn_data,
- logic [255:0] wdr_operand_a);
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
+ logic [255:0] wdr_operand_a,
+ flags_t flags_write_data [2]);
mnemonic_cp: coverpoint mnemonic {
`DEF_MNEM_BIN(mnem_bn_addi);
@@ -235,6 +279,9 @@
`DEF_WDR_TOGGLE_COV(wrs, wdr_operand_a)
`DEF_WDR_TOGGLE_CROSS(wrs)
+
+ `DEF_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
+ `DEF_FLAGS_TOGGLE_CROSS(flags)
endgroup
covergroup enc_bnam_cg
@@ -256,9 +303,10 @@
endgroup
covergroup enc_bnan_cg
- with function sample(mnem_str_t mnemonic,
- logic [31:0] insn_data,
- logic [255:0] wdr_operand_a);
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
+ logic [255:0] wdr_operand_a,
+ flags_t flags_write_data [2]);
mnemonic_cp: coverpoint mnemonic {
`DEF_MNEM_BIN(mnem_bn_not);
@@ -270,6 +318,10 @@
fg_cp: coverpoint insn_data[31];
`DEF_WDR_TOGGLE_COV(wrs, wdr_operand_a)
+
+ // BN.NOT can write the M, L and Z flags, but does not affect the carry flag (bit 0 in the
+ // flags_t struct).
+ `DEF_MLZ_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
endgroup
covergroup enc_bnaq_cg
@@ -297,7 +349,8 @@
with function sample(mnem_str_t mnemonic,
logic [31:0] insn_data,
logic [255:0] wdr_operand_a,
- logic [255:0] wdr_operand_b);
+ logic [255:0] wdr_operand_b,
+ flags_t flags_write_data [2]);
// Used for BN.MULQACC.SO
mnemonic_cp: coverpoint mnemonic {
@@ -314,13 +367,18 @@
`DEF_WDR_TOGGLE_COV(wrs1, wdr_operand_a)
`DEF_WDR_TOGGLE_COV(wrs2, wdr_operand_b)
+
+ // BN.MULQACC.SO can write the M, L and Z flags, but does not affect the carry flag (bit 0 in
+ // the flags_t struct).
+ `DEF_MLZ_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
endgroup
covergroup enc_bnaqw_cg
- with function sample(mnem_str_t mnemonic,
- logic [31:0] insn_data,
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
logic [255:0] wdr_operand_a,
- logic [255:0] wdr_operand_b);
+ logic [255:0] wdr_operand_b,
+ flags_t flags_write_data [2]);
// Used for BN.MULQACC.WO
mnemonic_cp: coverpoint mnemonic {
@@ -336,13 +394,18 @@
`DEF_WDR_TOGGLE_COV(wrs1, wdr_operand_a)
`DEF_WDR_TOGGLE_COV(wrs2, wdr_operand_b)
+
+ // BN.MULQACC.WO can write the M, L and Z flags, but does not affect the carry flag (bit 0 in
+ // the flags_t struct).
+ `DEF_MLZ_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
endgroup
covergroup enc_bnc_cg
- with function sample(mnem_str_t mnemonic,
- logic [31:0] insn_data,
+ with function sample(mnem_str_t mnemonic,
+ logic [31:0] insn_data,
logic [255:0] wdr_operand_a,
- logic [255:0] wdr_operand_b);
+ logic [255:0] wdr_operand_b,
+ flags_t flags_write_data [2]);
mnemonic_cp: coverpoint mnemonic {
`DEF_MNEM_BIN(mnem_bn_cmp);
@@ -361,6 +424,9 @@
`DEF_WDR_TOGGLE_COV(wrs2, wdr_operand_b)
`DEF_WDR_TOGGLE_CROSS(wrs1)
`DEF_WDR_TOGGLE_CROSS(wrs2)
+
+ `DEF_FLAGS_TOGGLE_COV(flags, flags_write_data[insn_data[31]])
+ `DEF_FLAGS_TOGGLE_CROSS(flags)
endgroup
covergroup enc_bnmov_cg
@@ -672,6 +738,7 @@
super.new(name, parent);
enc_bna_cg = new;
+ enc_bnaf_cg = new;
enc_bnai_cg = new;
enc_bnam_cg = new;
enc_bnan_cg = new;
@@ -779,22 +846,38 @@
// Every instruction mnemonic should have an associated encoding schema.
encoding = insn_encodings[mnem];
case (encoding)
- "bna", "bnaf":
- enc_bna_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
+ "bna":
+ enc_bna_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a, rtl_item.wdr_operand_b,
+ rtl_item.flags_write_data);
+ "bnaf":
+ enc_bnaf_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a, rtl_item.wdr_operand_b,
+ rtl_item.flags_write_data);
"bnai":
- enc_bnai_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a);
+ enc_bnai_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a,
+ rtl_item.flags_write_data);
"bnam":
enc_bnam_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
"bnan":
- enc_bnan_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a);
+ enc_bnan_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a,
+ rtl_item.flags_write_data);
"bnaq":
enc_bnaq_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
- "bnaqw":
- enc_bnaqw_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
"bnaqs":
- enc_bnaqs_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
+ enc_bnaqs_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a, rtl_item.wdr_operand_b,
+ rtl_item.flags_write_data);
+ "bnaqw":
+ enc_bnaqw_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a, rtl_item.wdr_operand_b,
+ rtl_item.flags_write_data);
"bnc":
- enc_bnc_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a, rtl_item.wdr_operand_b);
+ enc_bnc_cg.sample(mnem, insn_data,
+ rtl_item.wdr_operand_a, rtl_item.wdr_operand_b,
+ rtl_item.flags_write_data);
"bnmov":
enc_bnmov_cg.sample(mnem, insn_data, rtl_item.wdr_operand_a);
"bnmovr":
diff --git a/hw/ip/otbn/dv/uvm/env/otbn_env_pkg.sv b/hw/ip/otbn/dv/uvm/env/otbn_env_pkg.sv
index cd41dab..3da1a70 100644
--- a/hw/ip/otbn/dv/uvm/env/otbn_env_pkg.sv
+++ b/hw/ip/otbn/dv/uvm/env/otbn_env_pkg.sv
@@ -17,6 +17,8 @@
import otbn_reg_pkg::*;
import otbn_ral_pkg::*;
+ import otbn_pkg::flags_t;
+
// macro includes
`include "uvm_macros.svh"
`include "dv_macros.svh"
diff --git a/hw/ip/otbn/dv/uvm/env/otbn_trace_item.sv b/hw/ip/otbn/dv/uvm/env/otbn_trace_item.sv
index e2e7404..80663f4 100644
--- a/hw/ip/otbn/dv/uvm/env/otbn_trace_item.sv
+++ b/hw/ip/otbn/dv/uvm/env/otbn_trace_item.sv
@@ -15,13 +15,17 @@
logic [255:0] wdr_operand_a;
logic [255:0] wdr_operand_b;
+ // Flag output data
+ otbn_pkg::flags_t flags_write_data [2];
+
`uvm_object_utils_begin(otbn_trace_item)
- `uvm_field_int (insn_addr, UVM_DEFAULT | UVM_HEX)
- `uvm_field_int (insn_data, UVM_DEFAULT | UVM_HEX)
- `uvm_field_int (gpr_operand_a, UVM_DEFAULT | UVM_HEX)
- `uvm_field_int (gpr_operand_b, UVM_DEFAULT | UVM_HEX)
- `uvm_field_int (wdr_operand_a, UVM_DEFAULT | UVM_HEX)
- `uvm_field_int (wdr_operand_b, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (insn_addr, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (insn_data, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (gpr_operand_a, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (gpr_operand_b, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (wdr_operand_a, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_int (wdr_operand_b, UVM_DEFAULT | UVM_HEX)
+ `uvm_field_sarray_int (flags_write_data, UVM_DEFAULT | UVM_HEX)
`uvm_object_utils_end
`uvm_object_new
diff --git a/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv b/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv
index ef234d6..ae628a0 100644
--- a/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv
+++ b/hw/ip/otbn/dv/uvm/env/otbn_trace_monitor.sv
@@ -31,6 +31,9 @@
item.gpr_operand_b = cfg.trace_vif.rf_base_rd_data_b;
item.wdr_operand_a = cfg.trace_vif.rf_bignum_rd_data_a;
item.wdr_operand_b = cfg.trace_vif.rf_bignum_rd_data_b;
+ item.flags_write_data = cfg.trace_vif.flags_write_data;
+
+ `uvm_info(`gfn, $sformatf("saw trace item:\n%0s", item.sprint()), UVM_HIGH)
analysis_port.write(item);
end
end
