diff --git a/hw/dv/sv/dv_lib/dv_base_env_cfg.sv b/hw/dv/sv/dv_lib/dv_base_env_cfg.sv index 57220fe05ac67..531beefa31bb7 100644 --- a/hw/dv/sv/dv_lib/dv_base_env_cfg.sv +++ b/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, diff --git a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.core b/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.core index 65ce10c51e1e8..50ec7169abfe1 100644 --- a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.core +++ b/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 diff --git a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.sv b/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.sv index 8b09587ad477b..6730fd52f7cfe 100644 --- a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env.sv +++ b/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); diff --git a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env_cfg.sv b/hw/ip/rom_ctrl/dv/env/rom_ctrl_env_cfg.sv index 7a6298b0bc4e4..9fcf9f6001e91 100644 --- a/hw/ip/rom_ctrl/dv/env/rom_ctrl_env_cfg.sv +++ b/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 diff --git a/hw/ip/rom_ctrl/dv/tb/tb.sv b/hw/ip/rom_ctrl/dv/tb/tb.sv index 0aafe2608221b..e480835119a1e 100644 --- a/hw/ip/rom_ctrl/dv/tb/tb.sv +++ b/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 diff --git a/hw/top_earlgrey/dv/chip_sim.core b/hw/top_earlgrey/dv/chip_sim.core index ac82b404a1108..c088873b60ca5 100644 --- a/hw/top_earlgrey/dv/chip_sim.core +++ b/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} diff --git a/hw/top_earlgrey/dv/env/chip_env.core b/hw/top_earlgrey/dv/env/chip_env.core index 4fd59da6bef93..02bf4822cca13 100644 --- a/hw/top_earlgrey/dv/env/chip_env.core +++ b/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 diff --git a/hw/top_earlgrey/dv/env/chip_env.sv b/hw/top_earlgrey/dv/env/chip_env.sv index 946349c6d991d..80084220d67cd 100644 --- a/hw/top_earlgrey/dv/env/chip_env.sv +++ b/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] = diff --git a/hw/top_earlgrey/dv/env/chip_env_cfg.sv b/hw/top_earlgrey/dv/env/chip_env_cfg.sv index 849cb45db3cfa..45fffe720815f 100644 --- a/hw/top_earlgrey/dv/env/chip_env_cfg.sv +++ b/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. diff --git a/hw/top_earlgrey/dv/tb/tb.sv b/hw/top_earlgrey/dv/tb/tb.sv index d03fe8de13e46..1d020295a62a4 100644 --- a/hw/top_earlgrey/dv/tb/tb.sv +++ b/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