[rom_ctrl,dv] Connect the block-level rom_ctrl env to chip tb

hw/dv/sv/dv_lib/dv_base_env_cfg.sv

@@ -119,6 +119,12 @@ class dv_base_env_cfg #(type RAL_T = dv_base_reg_block) extends uvm_object;
   extern function void pre_randomize();
   extern function void post_randomize();
 
+  // Configure whether this env_cfg is active
+  //
+  // Use this, rather than directly setting is_active, in order that any agents' configurations are
+  // also updated.
+  extern virtual function void set_is_active(bit active);
+
   // Initialise the object with RAL models and set it up for randomisation
   //
   // The addr_width, data_width and be_width arguments are used to configure the analogous fields of
@@ -202,6 +208,10 @@ function void dv_base_env_cfg::post_randomize();
   end
 endfunction
 
+function void dv_base_env_cfg::set_is_active(bit active);
+  is_active = active;
+endfunction
+
 function void dv_base_env_cfg::initialize_ral(int unsigned addr_width,
                                               int unsigned data_width,
                                               int unsigned be_width,

hw/ip/rom_ctrl/dv/env/rom_ctrl_env.core

@@ -16,6 +16,7 @@ filesets:
       - lowrisc:dv:kmac_app_agent
       - lowrisc:dv:reset_agent
       - lowrisc:dv:rom_ctrl_ifs
+      - "chip_level ? (lowrisc:dv:chip_ral)"
     files:
       - rom_ctrl_prim_ral_pkg.sv
       - rom_ctrl_env_pkg.sv

hw/ip/rom_ctrl/dv/env/rom_ctrl_env.sv

@@ -41,34 +41,55 @@ class rom_ctrl_env extends cip_base_env #(
 endclass
 
 function void rom_ctrl_env::build_phase(uvm_phase phase);
+  bit integrity_check_disabled;
+
   super.build_phase(phase);
 
-  // Get the rom_bkdr interface
+  // Get the rom_bkdr interface.
   if (!uvm_config_db#(rom_ctrl_bkdr_util)::get(this, "", "rom_ctrl_bkdr_util",
                                                cfg.rom_ctrl_bkdr_util_h))
-    `uvm_fatal(`gfn, "failed to get rom_ctrl_bkdr_util from uvm_config_db")
+    `uvm_fatal("config_db", "Failed to get rom_ctrl_bkdr_util from uvm_config_db")
 
   if (!uvm_config_db#(rom_ctrl_vif)::get(this, "", "rom_ctrl_vif", cfg.rom_ctrl_vif))
-    `uvm_fatal(`gfn, "failed to get rom_ctrl_vif from uvm_config_db")
+    `uvm_fatal("config_db", "Failed to get rom_ctrl_vif from uvm_config_db")
 
-  if (!uvm_config_db#(virtual rom_ctrl_fsm_if)::get(this, "", "rom_ctrl_fsm_vif", cfg.fsm_vif))
-    `uvm_fatal(`gfn, "failed to get rom_ctrl_fsm_vif from uvm_config_db")
+  // Check whether the integrity check has been disabled (because DISABLE_ROM_INTEGRITY_CHECK was
+  // defined).
+  if (!uvm_config_db#(bit)::get(this, "",
+                                "integrity_check_disabled", integrity_check_disabled)) begin
+    `uvm_fatal("config_db", "Failed to get integrity_check_disabled flag from uvm_config_db.")
+  end
 
-  if (!uvm_config_db#(virtual rom_ctrl_compare_if)::get(this, "",
-                                                        "rom_ctrl_compare_vif", cfg.compare_vif))
-    `uvm_fatal(`gfn, "failed to get rom_ctrl_compare_vif from uvm_config_db")
+  // If the integrity check has not been disabled, get the bound-in FSM interface. If it *is*
+  // disabled, this will be visible through cfg by fsm_vif being null.
+  if (!integrity_check_disabled) begin
+    if (!uvm_config_db#(virtual rom_ctrl_fsm_if)::get(this, "",
+                                                      "rom_ctrl_fsm_vif", cfg.fsm_vif)) begin
+      `uvm_fatal("config_db", "Failed to get rom_ctrl_fsm_vif from uvm_config_db.")
+    end
+  end
+
+  // If the environment is active, get hold of an interface that is bound into the compare module
+  // (because sequences will want to use it to inject faults).
+  if (cfg.is_active) begin
+    if (!uvm_config_db#(virtual rom_ctrl_compare_if)::get(this, "", "rom_ctrl_compare_vif",
+                                                          cfg.compare_vif))
+      `uvm_fatal("config_db",
+                 "Active environment could not get rom_ctrl_compare_vif from uvm_config_db")
+  end
 
   // Build the KMAC agent
   m_kmac_agent = kmac_app_device_agent::type_id::create("m_kmac_agent", this);
   uvm_config_db#(kmac_app_agent_cfg)::set(this, "m_kmac_agent", "cfg", cfg.m_kmac_agent_cfg);
 
   // Create a sequencer and driver for forcing the counter in the rom_ctrl FSM, but only if
-  // cfg.get_skip_middle() is true.
+  // cfg.get_skip_middle() is true (and fsm_vif is non-null, meaning that rom_ctrl is actually doing
+  // any integrity checking).
   //
   // Note that this does *not* depend on cfg.is_active: this "backdoor trickery" works by accessing
   // internals of rom_ctrl and doesn't interact with outside stimulus. As such, it makes perfect
   // sense to use when the environment is bound into a higher level testbench.
-  if (cfg.get_skip_middle()) begin
+  if (cfg.get_skip_middle() && cfg.fsm_vif != null) begin
     m_addr_force_sequencer = (rom_ctrl_addr_force_sequencer_t::type_id::
                               create("m_addr_force_sequencer", this));
 
@@ -78,9 +99,12 @@ function void rom_ctrl_env::build_phase(uvm_phase phase);
   // Create a sequencer and driver for overriding responses from kmac, but only if
   // cfg.get_force_expected_kmac_rsp() is true.
   //
-  // As with m_addr_force_sequencer, this does *not* depend on cfg.is_active: it is similarly
+  // As with m_addr_force_sequencer, this only runs if fsm_vif is non-null (meaning that rom_ctrl is
+  // actually sending anything to kmac).
+  //
+  // Also as with m_addr_force_sequencer, this does *not* depend on cfg.is_active: it is similarly
   // relevant when bound into a higher level testbench.
-  if (cfg.get_force_expected_kmac_rsp()) begin
+  if (cfg.get_force_expected_kmac_rsp() && cfg.fsm_vif != null) begin
     m_kmac_rsp_force_sequencer = (rom_ctrl_kmac_rsp_force_sequencer_t::type_id::
                                   create("m_kmac_rsp_force_sequencer", this));
     m_kmac_rsp_force_driver =
@@ -97,8 +121,14 @@ function void rom_ctrl_env::connect_phase(uvm_phase phase);
   m_kmac_agent.monitor.m_req_packet_analysis_port.connect(scoreboard.m_kmac_req_imp);
   m_kmac_agent.monitor.analysis_port.connect(scoreboard.m_kmac_txn_imp);
 
-  virtual_sequencer.kmac_sequencer_h = m_kmac_agent.sequencer;
+  if (cfg.is_active) begin
+    virtual_sequencer.kmac_sequencer_h = m_kmac_agent.sequencer;
+  end
 
+  // If there is an address forcing driver (because cfg.get_skip_middle() was true and the existence
+  // of fsm_vif means that there is integrity checking), connect the address forcing driver whether
+  // or not the environment is active: we want to be able to use it to convince rom_ctrl by the back
+  // door to jump over the middle of ROM.
   if (m_addr_force_driver != null) begin
     m_addr_force_driver.set_vif(cfg.fsm_vif);
     m_addr_force_driver.seq_item_port.connect(m_addr_force_sequencer.seq_item_export);

hw/ip/rom_ctrl/dv/env/rom_ctrl_env_cfg.sv

@@ -34,6 +34,9 @@ class rom_ctrl_env_cfg extends cip_base_env_cfg #(.RAL_T(rom_ctrl_regs_reg_block
   virtual rom_ctrl_compare_if compare_vif;
 
   // An interface bound into the rom_ctrl_fsm module
+  //
+  // This might be null. If so, the environment won't investigate integrity checks at all (probably
+  // because the FSM doesn't exist in the device).
   virtual rom_ctrl_fsm_if fsm_vif;
 
   // A handle to the scoreboard, used to flag expected errors.
@@ -56,6 +59,11 @@ class rom_ctrl_env_cfg extends cip_base_env_cfg #(.RAL_T(rom_ctrl_regs_reg_block
   extern function new (string name="");
   extern function void post_randomize();
 
+  // Configure whether the env_cfg is active (configuring the agents too)
+  //
+  // This extends the definition in dv_base_env_cfg.
+  extern function void set_is_active(bit active);
+
   extern virtual function void initialize(bit inherit_ral_models = 1'b0);
   extern virtual protected function dv_base_reg_block create_ral_by_name(string name);
 
@@ -118,6 +126,11 @@ function void rom_ctrl_env_cfg::post_randomize();
   m_kmac_agent_cfg.rsp_delay_max = m_kmac_rsp_delay_max;
 endfunction
 
+function void rom_ctrl_env_cfg::set_is_active(bit active);
+  super.set_is_active(active);
+  m_kmac_agent_cfg.is_active = active;
+endfunction
+
 function void rom_ctrl_env_cfg::initialize(bit inherit_ral_models = 1'b0);
   // Use the inherit_ral_models argument to infer the key for the memory uvm_reg_block in
   // ral_model_names. If inherit_ral_models is false, this is a block-level test and we should use

hw/ip/rom_ctrl/dv/tb/tb.sv

@@ -122,6 +122,10 @@ module tb;
         null, "*.env", "rom_ctrl_compare_vif",
         dut.gen_fsm_scramble_enabled.u_checker_fsm.u_compare.u_bound_if.gen_bound.u_compare_if);
 
+    // Pass a flag that tells the environment that we haven't built rom_ctrl without its integrity
+    // check FSM.
+    uvm_config_db#(bit)::set(null, "*.env", "integrity_check_disabled", 0);
+
     $timeformat(-12, 0, " ps", 12);
     run_test();
   end

hw/top_earlgrey/dv/chip_sim.core

@@ -42,6 +42,8 @@ filesets:
       - lowrisc:dv_dpi_c:usbdpi
       - lowrisc:dv_dpi_sv:usbdpi
       - lowrisc:dv:usb20_usbdpi
+      - lowrisc:dv:kmac_app_agent
+      - lowrisc:dv:rom_ctrl_ifs
     files:
       - tb/chip_hier_macros.svh: {is_include_file: true}
       - autogen/tb__xbar_connect.sv: {is_include_file: true}

hw/top_earlgrey/dv/env/chip_env.core

@@ -36,6 +36,7 @@ filesets:
       - lowrisc:dv:lc_ctrl_dv_utils
       - lowrisc:dv:top_earlgrey_chip_ral
       - lowrisc:systems:ast_pkg
+      - lowrisc:dv:rom_ctrl_env
     files:
       - chip_common_pkg.sv
       - chip_if.sv

hw/top_earlgrey/dv/env/chip_env.sv

@@ -19,6 +19,9 @@ class chip_env extends cip_base_env #(
   // spi host agent that transmits transactions to dut spi device
   spi_agent              m_spi_host_agent;
 
+  // A passive environment that monitors the rom_ctrl block
+  rom_ctrl_env_pkg::rom_ctrl_env m_rom_ctrl_env;
+
   `uvm_component_new
 
   function void build_phase(uvm_phase phase);
@@ -89,6 +92,11 @@ class chip_env extends cip_base_env #(
                                           cfg.m_uart_agent_cfgs[i]);
     end
 
+    // Create the passive rom_ctrl_env. This can be given m_cfg.m_rom_ctrl_env_cfg (which has
+    // already been created and initialised by the test object's build_phase) as a cfg object.
+    m_rom_ctrl_env = rom_ctrl_env_pkg::rom_ctrl_env::type_id::create("m_rom_ctrl_env", this);
+    m_rom_ctrl_env.cfg = cfg.m_rom_ctrl_env_cfg;
+
     // dut spi host, tb spi device
     foreach (m_spi_device_agents[i]) begin
       m_spi_device_agents[i] =

hw/top_earlgrey/dv/env/chip_env_cfg.sv

@@ -114,6 +114,9 @@ class chip_env_cfg #(type RAL_T = chip_ral_pkg::chip_reg_block) extends cip_base
   // and skip cpu_init in chip_sw_base_vseq::body
   bit early_cpu_init = 0;
 
+  // An env_cfg for a passive bound-in rom_ctrl environment
+  rom_ctrl_env_pkg::rom_ctrl_env_cfg m_rom_ctrl_env_cfg;
+
   // NOTE: The clk_freq_mhz variable created in the base class was meant to be used by clk_rst_vif
   // interface that is passed by default by the testbench (retrieved by dv_base_env class). It was
   // meant for a CIP-compliant testbench to drive the clock and reset to the DUT. The chip level
@@ -125,7 +128,11 @@ class chip_env_cfg #(type RAL_T = chip_ral_pkg::chip_reg_block) extends cip_base
     foreach (clk_freqs_mhz[i]) clk_freqs_mhz[i] == clk_freq_mhz;
   }
 
-  `uvm_object_new
+  function new (string name = "");
+    super.new(name);
+    m_rom_ctrl_env_cfg = rom_ctrl_env_pkg::rom_ctrl_env_cfg::type_id::create("m_rom_ctrl_env_cfg");
+    m_rom_ctrl_env_cfg.set_is_active(1'b0);
+  endfunction
 
   `uvm_object_param_utils_begin(chip_env_cfg#(RAL_T))
     `uvm_field_object(m_jtag_riscv_agent_cfg, UVM_DEFAULT)
@@ -213,6 +220,14 @@ class chip_env_cfg #(type RAL_T = chip_ral_pkg::chip_reg_block) extends cip_base
     num_ram_main_tiles = 1;
     num_ram_ret_tiles = 1;
     num_otbn_dmem_tiles = 1;
+
+    // Copy handles to rom_ctrl's register blocks into its block-level environment config
+    m_rom_ctrl_env_cfg.ral_models["rom_ctrl_regs_reg_block"] = ral.rom_ctrl_regs;
+    m_rom_ctrl_env_cfg.ral_models["rom_ctrl_rom_reg_block"] = ral.rom_ctrl_rom;
+
+    // Set up the config for the bound-in rom_ctrl environment, passing inherit_ral_models=1 so that
+    // it uses the register blocks whose handles we just copied.
+    m_rom_ctrl_env_cfg.initialize(1'b1);
   endfunction
 
   // Configure the environment to run a DMI agent over a JTAG connection.

hw/top_earlgrey/dv/tb/tb.sv

@@ -456,6 +456,116 @@ module tb;
     end
   end
 
+  if (1) begin : gen_rom_ctrl
+       // Note: Splitting out PD_MAIN_PATH separately is to work around a problem with the linting
+       // tool. With Verible 3622 (the version currently used in CI) a bind statement where the
+       // target is just a macro expansion (`ROM_CTRL_PATH) causes an unrelated lint error.
+       // Splitting things like this means that we only have to repeat "u_rom_ctrl".
+`define PD_MAIN_PATH      dut.top_earlgrey.earlgrey_pd_main
+`define ROM_CTRL_PATH     `PD_MAIN_PATH.u_rom_ctrl
+`define ROM_CTRL_MEM_HIER `ROM_CTRL_PATH.gen_rom_scramble_enabled.u_rom.u_rom.u_prim_rom.mem
+
+    localparam string EnvPath = "*.env.m_rom_ctrl_env";
+
+    // The information that we pass to the environment will depend on whether
+    // DISABLE_ROM_INTEGRITY_CHECK is defined. To make it possible to reason about this in the
+    // environment, we define a bit showing whether it is defined and will pass it through
+    // uvm_config_db below.
+`ifdef DISABLE_ROM_INTEGRITY_CHECK
+   localparam bit RomIntegrityCheckDisabled = 1;
+`else
+   localparam bit RomIntegrityCheckDisabled = 0;
+`endif
+
+    wire clk, rst_n;
+    assign clk = `ROM_CTRL_PATH.clk_i;
+    assign rst_n = `ROM_CTRL_PATH.rst_ni;
+
+    clk_rst_if  clk_rst_if (.clk(clk), .rst_n(rst_n));
+    tl_if       rom_tl_if (.clk(clk), .rst_n(rst_n));
+    tl_if       regs_tl_if (.clk(clk), .rst_n(rst_n));
+
+    assign rom_tl_if.if_mode = Monitor;
+    assign rom_tl_if.h2d = `ROM_CTRL_PATH.rom_tl_i;
+    assign rom_tl_if.d2h = `ROM_CTRL_PATH.rom_tl_o;
+
+    assign regs_tl_if.if_mode = Monitor;
+    assign regs_tl_if.h2d = `ROM_CTRL_PATH.regs_tl_i;
+    assign regs_tl_if.d2h = `ROM_CTRL_PATH.regs_tl_o;
+
+    wire kmac_pkg::app_rsp_t kmac_data_in;
+    wire kmac_pkg::app_req_t kmac_data_out;
+    assign kmac_data_in  = `ROM_CTRL_PATH.kmac_data_i;
+    assign kmac_data_out = `ROM_CTRL_PATH.kmac_data_o;
+
+    // Note: The req and rsp inout ports get driven with kmac_data_in/kmac_data_out from the design,
+    //       and the interface should be in Monitor mode. We don't assign kmac_app_if.if_mode here:
+    //       this will be set by the build phase of the kmac_app_*_agent that we instantiate.
+    kmac_app_if u_app_if (.clk_i (clk), .rst_ni (rst_n), .req (kmac_data_out), .rsp (kmac_data_in));
+
+    bind `PD_MAIN_PATH.u_rom_ctrl
+      rom_ctrl_bound_if #(.Bound(1), .SecDisableScrambling(SecDisableScrambling))
+      u_bound_if (.clk_i, .rst_ni);
+
+    // We only bind in the FSM interface if scrambling is enabled (otherwise there won't actually be
+    // an FSM module). If DISABLE_ROM_INTEGRITY_CHECK is not defined, the
+    // SecRomCtrlDisableScrambling parameter to dut will not be set, so scrambling will be enabled
+    // and there will be an FSM at `FSM_PATH.
+`ifndef DISABLE_ROM_INTEGRITY_CHECK
+`define FSM_PATH `ROM_CTRL_PATH.gen_fsm_scramble_enabled.u_checker_fsm
+    bind `ROM_CTRL_PATH.gen_fsm_scramble_enabled.u_checker_fsm
+      rom_ctrl_fsm_bound_if #(.Bound(1), .TopCount(TopCount))
+      u_bound_if (.clk_i, .rst_ni);
+
+    initial begin
+      uvm_config_db#(virtual rom_ctrl_fsm_if)::set(null, EnvPath, "rom_ctrl_fsm_vif",
+                                                   `FSM_PATH.u_bound_if.gen_bound.u_fsm_if);
+
+      uvm_config_db#(bit [127:0])::set(null, EnvPath, "scramble_key",
+                                       `ROM_CTRL_PATH.RndCnstScrKey);
+      uvm_config_db#(bit [63:0])::set(null, EnvPath, "scramble_nonce",
+                                      `ROM_CTRL_PATH.RndCnstScrNonce);
+    end
+`undef FSM_PATH
+`endif
+
+    // Register the various interfaces so that they can be picked up by the passive rom_ctrl
+    // environment.
+    //
+    // There will be a rom_ctrl_bkdr_util created in the section that creates backdoor utilities
+    // below, which will pass that to the rom_ctrl environment too.
+    initial begin
+      automatic string rom_reg_block_name = "rom_ctrl_rom_reg_block";
+      automatic string regs_reg_block_name = "rom_ctrl_regs_reg_block";
+
+      uvm_config_db#(virtual clk_rst_if)::set(null, EnvPath, "clk_rst_vif", clk_rst_if);
+      uvm_config_db#(virtual rom_ctrl_if)::set(null, EnvPath, "rom_ctrl_vif",
+                                               `ROM_CTRL_PATH.u_bound_if.gen_bound.u_rom_ctrl_if);
+
+      uvm_config_db#(virtual tl_if)::set(null, {EnvPath, ".m_tl_agent_", rom_reg_block_name},
+                                         "vif", rom_tl_if);
+      uvm_config_db#(virtual tl_if)::set(null, {EnvPath, ".m_tl_agent_", regs_reg_block_name},
+                                         "vif", regs_tl_if);
+      uvm_config_db#(virtual clk_rst_if)::set(null, EnvPath, {"clk_rst_vif_", rom_reg_block_name},
+                                              clk_rst_if);
+
+      uvm_config_db#(virtual kmac_app_if)::set(null, {EnvPath, ".m_kmac_agent"}, "vif", u_app_if);
+
+      // Connect rom_ctrl's alert interface to the passive environment (which will instantiate an
+      // extra alert agent and run it in passive mode).
+      uvm_config_db#(virtual alert_esc_if)::set(null, {EnvPath, ".m_alert_agent_fatal"},
+                                                "vif", alert_if[TopEarlgreyAlertIdRomCtrlFatal]);
+
+      // Pass a flag that tells the environment whether to expect rom_ctrl_fsm_vif.
+      uvm_config_db#(bit)::set(null, EnvPath,
+                               "integrity_check_disabled", RomIntegrityCheckDisabled);
+    end
+
+`undef ROM_CTRL_MEM_HIER
+`undef ROM_CTRL_PATH
+`undef PD_MAIN_PATH
+  end
+
   // Instantiate the memory backdoor util instances.
   if (prim_pkg::PrimTechName == "Generic") begin : gen_generic
     initial begin
@@ -672,6 +782,10 @@ module tb;
             null, "*.env", m_mem_bkdr_util[mem].get_name(), m_mem_bkdr_util[mem]);
         mem = mem.next();
       end while (mem != mem.first());
+
+      // Pass the rom_ctrl_bkdr_util to the bound-in block-level environment
+      uvm_config_db#(rom_ctrl_bkdr_util)::set(null, "*.env.m_rom_ctrl_env",
+                                              "rom_ctrl_bkdr_util", rom);
     end
   end : gen_generic