[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