[cov methodology] Functional coverage prototype
- Implements functional coverage for UART in an interface
- Proof of concept to drive a designer-led functional coverage
implementation methodology
- See link below for more details:
https://docs.google.com/presentation/d/1rUFpWzktORuBZNyG6eSG68kWxEWmxMkMaWdSCadhbQI
Signed-off-by: Srikrishna Iyer <sriyer@google.com>
diff --git a/hw/dv/sv/dv_utils/dv_macros.svh b/hw/dv/sv/dv_utils/dv_macros.svh
index f169faf..e9413f8 100644
--- a/hw/dv/sv/dv_utils/dv_macros.svh
+++ b/hw/dv/sv/dv_utils/dv_macros.svh
@@ -492,4 +492,29 @@
end
`endif
+// Instantiates a covergroup in an interface or module.
+//
+// This macro assumes that a covergroup of the same name as the __CG_NAME arg is defined in the
+// interface or module. It just adds some extra signals and logic to control the creation of the
+// covergroup instance with ~bit en_<cg_name>~. This defaults to 0. It is ORed with the external
+// __COND signal. The testbench can modify it at t = 0 based on the test being run.
+// NOTE: This is not meant to be invoked inside a class.
+//
+// __CG_NAME : Name of the covergroup.
+// __COND : External condition / expr that controls the creation of the covergroup.
+// __CG_ARGS : Arguments to covergroup instance, if any. Args MUST BE wrapped in (..).
+`ifndef DV_INSTANTIATE_CG
+`define DV_INSTANTIATE_CG(__CG_NAME, __COND = 1'b1, __CG_ARGS = ()) \
+ bit en_``__CG_NAME = 1'b0; \
+ __CG_NAME __CG_NAME``_inst; \
+ initial begin \
+ /* The #1 delay below allows any part of the tb to control the conditions first at t = 0. */ \
+ #1; \
+ if ((en_``__CG_NAME) || (__COND)) begin \
+ `DV_INFO({"Creating covergroup ", `"__CG_NAME`"}, UVM_MEDIUM) \
+ __CG_NAME``_inst = new``__CG_ARGS; \
+ end \
+ end
+`endif
+
`endif // __DV_MACROS_SVH__
diff --git a/hw/dv/sv/dv_utils/dv_utils.core b/hw/dv/sv/dv_utils/dv_utils.core
index 4751513..5efce93 100644
--- a/hw/dv/sv/dv_utils/dv_utils.core
+++ b/hw/dv/sv/dv_utils/dv_utils.core
@@ -15,6 +15,7 @@
files:
- dv_utils_pkg.sv
- dv_report_server.sv: {is_include_file: true}
+ - dv_vif_wrap.sv: {is_include_file: true}
file_type: systemVerilogSource
targets:
diff --git a/hw/dv/sv/dv_utils/dv_utils_pkg.sv b/hw/dv/sv/dv_utils/dv_utils_pkg.sv
index 46e2cc9..2c99462 100644
--- a/hw/dv/sv/dv_utils/dv_utils_pkg.sv
+++ b/hw/dv/sv/dv_utils/dv_utils_pkg.sv
@@ -152,9 +152,27 @@
endfunction
`endif
+ // Returns the hierarchical path to the interface / module N levels up.
+ //
+ // Meant to be invoked inside a module or interface.
+ // hier: String input of the interface / module, typically $sformatf("%m").
+ // n_levels_up: Integer number of levels up the hierarchy to omit.
+ // Example: if (hier = tb.dut.foo.bar, n_levels_up = 2), then return tb.dut
+ function automatic string get_parent_hier(string hier, int n_levels_up = 1);
+ int idx;
+ int level;
+ if (n_levels_up <= 0) return hier;
+ for (idx = hier.len() - 1; idx >= 0; idx--) begin
+ if (hier[idx] == ".") level++;
+ if (level == n_levels_up) break;
+ end
+ return (hier.substr(0, idx - 1));
+ endfunction
+
// sources
`ifdef UVM
`include "dv_report_server.sv"
+ `include "dv_vif_wrap.sv"
`endif
endpackage
diff --git a/hw/dv/sv/dv_utils/dv_vif_wrap.sv b/hw/dv/sv/dv_utils/dv_vif_wrap.sv
new file mode 100644
index 0000000..61d9947
--- /dev/null
+++ b/hw/dv/sv/dv_utils/dv_vif_wrap.sv
@@ -0,0 +1,96 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// Abstract class meant to hold arbitrary virtual interface handles.
+//
+// Written primarily for an interface which implements functional coverage, this could be used
+// for other purposes as well. This abstract class provides utilities & macros to retrieve
+// virtual interface handles that are bound to a DUT's sub-modules. These sub-module interfaces must
+// self-register using the `DV_VIF_WRAP_SET_VIF()` macro (see details below). The extended class
+// then implements the `get_vifs()` method using the `DV_VIF_WRAP_GET_VIF*` macros below to retrieve
+// the sub-module interface handles it maintains.
+virtual class dv_vif_wrap;
+ string hier; // Represents the hierarchy of the parent module or interface.
+ string name; // Name of the class instance.
+
+ function new(string hier, string name = "");
+ this.hier = hier;
+ this.name = name;
+ endfunction
+
+ // Abstract method implemented by the extended class. It is recommended to invoke the helper
+ // macros below rather than manually define it.
+ pure virtual task get_vifs();
+
+endclass
+
+// Helper macros.
+//
+// These are defined in the file itself since they are tightly coupled to the class definition
+// above. These are scoped globally so that extended classes can invoke them.
+
+// Enable an interface to register itself (set its handle into the config db).
+//
+// Meant to be invoked from an interface. The macros invocation causes the interface to register
+// itself into the uvm_resource_db pool. The derived class of dv_vif_wrap retrieves the handle to
+// that interface handle from the uvm_resource db pool.
+//
+// SV LRM does not yet allow referencing the instance of an interface within itself as one
+// would in case of a class using the ~this~ keyword. However, most major simulators actually
+// support this in their own custom way. On VCS, a reference to self within the interface can be set
+// using `interface::self()` construct. On Xcelium, this is achieved by simply referencing the
+// interface name.
+//
+// _IF: The SV interface
+// _VIF: The corresponding virtual interface handle name
+// _LEVEL: # of hierarchical levels the module to which the SV interface is bound, starting at the
+// top level DUT instance.
+`define DV_VIF_WRAP_SET_VIF(_IF, _VIF, _LEVEL = 0) \
+ import uvm_pkg::*; \
+ function automatic void self_register(); \
+ string path; \
+ virtual _IF vif; \
+ /* Initial block adds another level in the path hierarchy which needs to be split out. */ \
+ /* Add one more to go one level up the interface instance. */ \
+ /* Example: tb.dut.core.u_foo_if.self_register -> tb.dut.core. */ \
+ path = dv_utils_pkg::get_parent_hier(.hier($sformatf("%m")), .n_levels_up(2 + _LEVEL)); \
+`ifdef VCS \
+ vif = interface::self(); \
+`elsif XCELIUM \
+ vif = _IF; \
+`else \
+ vif = _IF; \
+`endif \
+ uvm_pkg::uvm_resource_db#(virtual _IF)::set(path, `"_VIF`", vif); \
+ endfunction \
+ initial self_register();
+
+// Enables the retrieval of individual vifs.
+//
+// The three macros below go together to define the _get_vifs() task in the extended class.
+`define DV_VIF_WRAP_GET_VIFS_BEGIN \
+ task get_vifs(); \
+ fork \
+
+// To avoid race condition between the instant when an interface handle is set into the config db
+// and the instant when it is retrieved (in the same time step, at t = 0), the macro below invokes
+// uvm_config_db#(..)::wait_modified() to ensure that the retrieval is done only after the set.
+`define DV_VIF_WRAP_GET_VIF(_IF, _VIF) \
+ begin \
+ bit vif_found; \
+ /* At most 2 retries. */ \
+ repeat (2) begin \
+ /* Force the evaluation at the end of the time step. */ \
+ #0; \
+ if (uvm_pkg::uvm_resource_db#(virtual _IF)::read_by_name(hier, `"_VIF`", _VIF)) begin \
+ vif_found = 1'b1; \
+ break; \
+ end \
+ end \
+ `DV_CHECK_FATAL(vif_found, {`"_VIF`", " not found in the resource db"}, hier) \
+ end
+
+`define DV_VIF_WRAP_GET_VIFS_END \
+ join \
+ endtask
diff --git a/hw/dv/tools/dvsim/vcs.hjson b/hw/dv/tools/dvsim/vcs.hjson
index 6193f0c..812eae8 100644
--- a/hw/dv/tools/dvsim/vcs.hjson
+++ b/hw/dv/tools/dvsim/vcs.hjson
@@ -47,7 +47,9 @@
// - Write (deposit) capability on registers and variables
// - Force capability on registers, variables, and nets
"-debug_access+f",
-
+ // Use this to conditionally compile for VCS (example: LRM interpretations differ
+ // across tools).
+ "+define+VCS",
// Upgrade below warnings to errors to make VCS more strict on syntax to avoid
// having issue with other simulators but passing with VCS
//
diff --git a/hw/dv/tools/dvsim/xcelium.hjson b/hw/dv/tools/dvsim/xcelium.hjson
index 29de0a6..c61101b 100644
--- a/hw/dv/tools/dvsim/xcelium.hjson
+++ b/hw/dv/tools/dvsim/xcelium.hjson
@@ -20,8 +20,9 @@
"-xmlibdirname {build_dir}/xcelium.d",
// for uvm_hdl_* used by csr backdoor
"-access +rw",
- "+define+XCELIUM",
- ]
+ // Use this to conditionally compile for Xcelium (example: LRM interpretations differ
+ // across tools).
+ "+define+XCELIUM"]
run_opts: ["-input {run_script}",
"-licqueue",
diff --git a/hw/ip/uart/dv/cov/uart_core_cov_if.sv b/hw/ip/uart/dv/cov/uart_core_cov_if.sv
new file mode 100644
index 0000000..8ae2068
--- /dev/null
+++ b/hw/ip/uart/dv/cov/uart_core_cov_if.sv
@@ -0,0 +1,15 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Implements functional coverage for uart_core.sv sub-module.
+interface uart_core_cov_if (
+ input logic tx_enable,
+ input logic rx_enable
+);
+
+ // This interface is bound to `uart_core` which is 1 level underneath `uart` which is the DUT top
+ // level module.
+ `DV_VIF_WRAP_SET_VIF(uart_core_cov_if, uart_core_cov_vif, 1)
+
+endinterface
diff --git a/hw/ip/uart/dv/cov/uart_cov.core b/hw/ip/uart/dv/cov/uart_cov.core
new file mode 100644
index 0000000..06d9f3c
--- /dev/null
+++ b/hw/ip/uart/dv/cov/uart_cov.core
@@ -0,0 +1,21 @@
+CAPI=2:
+# Copyright lowRISC contributors.
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+name: "lowrisc:dv:uart_cov"
+description: "UART functional coverage interface & bind."
+
+filesets:
+ files_dv:
+ depend:
+ - lowrisc:dv:dv_utils
+ files:
+ - uart_core_cov_if.sv
+ - uart_cov_if.sv
+ - uart_cov_bind.sv
+ file_type: systemVerilogSource
+
+targets:
+ default:
+ filesets:
+ - files_dv
diff --git a/hw/ip/uart/dv/cov/uart_cov_bind.sv b/hw/ip/uart/dv/cov/uart_cov_bind.sv
new file mode 100644
index 0000000..5179124
--- /dev/null
+++ b/hw/ip/uart/dv/cov/uart_cov_bind.sv
@@ -0,0 +1,12 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Binds UART functional coverage interaface to the top level UART module.
+module uart_cov_bind;
+
+ bind uart uart_cov_if u_uart_cov_if (.*);
+
+ bind uart_core uart_core_cov_if u_uart_core_cov_if (.*);
+
+endmodule
diff --git a/hw/ip/uart/dv/cov/uart_cov_if.sv b/hw/ip/uart/dv/cov/uart_cov_if.sv
new file mode 100644
index 0000000..823e897
--- /dev/null
+++ b/hw/ip/uart/dv/cov/uart_cov_if.sv
@@ -0,0 +1,55 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+//
+// Implements functional coverage for UART.
+interface uart_cov_if (
+ input logic clk_i
+);
+
+ import uvm_pkg::*;
+ import dv_utils_pkg::*;
+
+ bit en_full_cov = 1'b1;
+ bit en_intg_cov = 1'b1;
+
+ // If en_full_cov is set, then en_intg_cov must also be set since it is a subset.
+ bit en_intg_cov_loc;
+ assign en_intg_cov_loc = en_full_cov | en_intg_cov;
+
+ // References to cov interfaces bound to sub-modules need to be held inside a class.
+ class uart_cov_vifs_wrap extends dv_vif_wrap;
+ virtual uart_core_cov_if uart_core_cov_vif;
+
+ function new(string hier, string name);
+ super.new(hier, name);
+ endfunction
+
+ // The section below constructs the `get_vifs()` task using the helper macros. It retrieves the
+ // VIF handles bound to sub-modules from the uvm_resource_db.
+ `DV_VIF_WRAP_GET_VIFS_BEGIN
+ `DV_VIF_WRAP_GET_VIF(uart_core_cov_if, uart_core_cov_vif)
+ `DV_VIF_WRAP_GET_VIFS_END
+
+ endclass
+
+ uart_cov_vifs_wrap m_uart_cov_vifs_wrap;
+
+ initial begin
+ m_uart_cov_vifs_wrap = new(dv_utils_pkg::get_parent_hier(.hier($sformatf("%m"))),
+ "m_uart_cov_vifs_wrap");
+ m_uart_cov_vifs_wrap.get_vifs();
+ end
+
+ covergroup uart_op_cg @(posedge clk_i);
+ option.name = "uart_op_cg";
+ option.comment = "UART TX and RX operations";
+ option.per_instance = 1;
+
+ cp_tx_enable: coverpoint uart_core.tx_enable;
+ cp_rx_enable: coverpoint m_uart_cov_vifs_wrap.uart_core_cov_vif.rx_enable;
+ cr_tx_rx_enable: cross cp_tx_enable, cp_rx_enable;
+ endgroup
+ `DV_INSTANTIATE_CG(uart_op_cg, en_full_cov)
+
+endinterface
diff --git a/hw/ip/uart/dv/uart_sim.core b/hw/ip/uart/dv/uart_sim.core
index a279900..3ae0370 100644
--- a/hw/ip/uart/dv/uart_sim.core
+++ b/hw/ip/uart/dv/uart_sim.core
@@ -13,6 +13,7 @@
depend:
- lowrisc:dv:uart_test
- lowrisc:dv:uart_sva
+ - lowrisc:dv:uart_cov
files:
- tb/tb.sv
file_type: systemVerilogSource
diff --git a/hw/ip/uart/dv/uart_sim_cfg.hjson b/hw/ip/uart/dv/uart_sim_cfg.hjson
index 573b3db..9e1fdd4 100644
--- a/hw/ip/uart/dv/uart_sim_cfg.hjson
+++ b/hw/ip/uart/dv/uart_sim_cfg.hjson
@@ -33,7 +33,7 @@
"{proj_root}/hw/dv/tools/dvsim/tests/stress_tests.hjson"]
// Add additional tops for simulation.
- sim_tops: ["-top uart_bind"]
+ sim_tops: ["-top uart_bind", "-top uart_cov_bind"]
// Default iterations for all tests - each test entry can override this.
reseed: 50
