[dv,top_earlgrey] Fix pwrmgr_rstmgr_sva_if bind for top

The assertions pwrmgr_rstmgr_sva_if need to be disabled when either aon or main_pok are inactive. It is more reliable to bind them to rstmgr for top-level simulations. This PR does that and connects rst_slow_ni to &rst_por_aon_n, since rst_por_aon_n is 2-bit wide (one per domain), and captures the rstmgr behavior more accurately. This also removes the rstmgr_unit_only* files for simplicity. Fixes lowRISC#23961 Signed-off-by: Guillermo Maturana <[email protected]>
matutem · Jul 12, 2024 · 23bae8d · 23bae8d
1 parent 85029ae
commit 23bae8d
Show file tree

Hide file tree

Showing 24 changed files with 162 additions and 95 deletions.
diff --git a/hw/ip_templates/pwrmgr/dv/pwrmgr_sim.core b/hw/ip_templates/pwrmgr/dv/pwrmgr_sim.core
@@ -12,6 +12,7 @@ filesets:
     depend:
       - lowrisc:dv:pwrmgr_test
       - lowrisc:dv:pwrmgr_sva
+      - lowrisc:dv:pwrmgr_unit_only_sva
     files:
       - tb.sv
       - cov/pwrmgr_cov_bind.sv

diff --git a/hw/ip_templates/pwrmgr/dv/pwrmgr_sim_cfg.hjson b/hw/ip_templates/pwrmgr/dv/pwrmgr_sim_cfg.hjson
@@ -49,6 +49,7 @@
   // Add additional tops for simulation.
   sim_tops: ["pwrmgr_bind",
              "pwrmgr_cov_bind",
+             "pwrmgr_unit_only_bind",
              "sec_cm_prim_count_bind",
              "sec_cm_prim_sparse_fsm_flop_bind",
              "sec_cm_prim_onehot_check_bind"]

diff --git a/hw/ip_templates/pwrmgr/dv/sva/pwrmgr_bind.sv b/hw/ip_templates/pwrmgr/dv/sva/pwrmgr_bind.sv
@@ -33,20 +33,7 @@ module pwrmgr_bind;
     .io_clk_en(pwr_ast_o.io_clk_en),
     .usb_clk_en(pwr_ast_o.usb_clk_en)
   );
-
-  bind pwrmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
-    .clk_i,
-    .rst_ni,
-    .clk_slow_i,
-    .rst_slow_ni,
-    // The outputs from pwrmgr.
-    .rst_lc_req(pwr_rst_o.rst_lc_req),
-    .rst_sys_req(pwr_rst_o.rst_sys_req),
-    // The inputs from rstmgr.
-    .rst_lc_src_n(pwr_rst_i.rst_lc_src_n),
-    .rst_sys_src_n(pwr_rst_i.rst_sys_src_n)
-  );
-
+
   bind pwrmgr clkmgr_pwrmgr_sva_if #(.IS_USB(0)) clkmgr_pwrmgr_io_sva_if (
     .clk_i,
     .rst_ni,

diff --git a/hw/ip_templates/pwrmgr/dv/sva/pwrmgr_unit_only_bind.sv b/hw/ip_templates/pwrmgr/dv/sva/pwrmgr_unit_only_bind.sv
@@ -0,0 +1,21 @@
+// Copyright lowRISC contributors (OpenTitan project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// This binds assertions that should not be bound at chip level.
+module pwrmgr_unit_only_bind;
+
+  bind pwrmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
+    .clk_i,
+    .rst_ni,
+    .clk_slow_i,
+    .rst_slow_ni,
+    // The outputs from pwrmgr.
+    .rst_lc_req(pwr_rst_o.rst_lc_req),
+    .rst_sys_req(pwr_rst_o.rst_sys_req),
+    // The inputs from rstmgr.
+    .rst_lc_src_n(pwr_rst_i.rst_lc_src_n),
+    .rst_sys_src_n(pwr_rst_i.rst_sys_src_n)
+  );
+
+endmodule : pwrmgr_unit_only_bind
diff --git a/...s/rstmgr/dv/sva/rstmgr_unit_only_sva.core → ...s/pwrmgr/dv/sva/pwrmgr_unit_only_sva.core b/...s/rstmgr/dv/sva/rstmgr_unit_only_sva.core → ...s/pwrmgr/dv/sva/pwrmgr_unit_only_sva.core
@@ -2,8 +2,8 @@ CAPI=2:
 # Copyright lowRISC contributors (OpenTitan project).
 # Licensed under the Apache License, Version 2.0, see LICENSE for details.
 # SPDX-License-Identifier: Apache-2.0
-name: "lowrisc:dv:rstmgr_unit_only_sva:0.1"
-description: "RSTMGR assertion modules not suitable for chip level and bind file."
+name: "lowrisc:dv:pwrmgr_unit_only_sva:0.1"
+gdescription: "PWRMGR assertion interfaces not suitable for chip level bind file."
 filesets:
   files_dv:
     depend:
@@ -12,18 +12,18 @@ filesets:
       - lowrisc:dv:pwrmgr_rstmgr_sva_if
 
     files:
-      - rstmgr_unit_only_bind.sv
+      - pwrmgr_unit_only_bind.sv
     file_type: systemVerilogSource
 
   files_formal:
     depend:
-      - lowrisc:systems:rstmgr
+      - lowrisc:ip_interfaces:pwrmgr
 
 generate:
   csr_assert_gen:
     generator: csr_assert_gen
     parameters:
-      spec: ../../data/rstmgr.hjson
+      spec: ../../data/pwrmgr.hjson
 
 targets:
   default: &default_target
@@ -36,4 +36,4 @@ targets:
     filesets:
       - files_formal
       - files_dv
-    toplevel: rstmgr
+    toplevel: pwrmgr
diff --git a/hw/ip_templates/rstmgr/dv/rstmgr_sim.core b/hw/ip_templates/rstmgr/dv/rstmgr_sim.core
@@ -13,7 +13,6 @@ filesets:
     depend:
       - lowrisc:dv:rstmgr_test
       - lowrisc:dv:rstmgr_sva
-      - lowrisc:dv:rstmgr_unit_only_sva
     files:
       - tb.sv
       - cov/rstmgr_cov_bind.sv

diff --git a/hw/ip_templates/rstmgr/dv/rstmgr_sim_cfg.hjson b/hw/ip_templates/rstmgr/dv/rstmgr_sim_cfg.hjson
@@ -53,8 +53,9 @@
   ]
 
   // Add additional tops for simulation.
-  sim_tops: ["rstmgr_bind", "rstmgr_unit_only_bind", "rstmgr_cov_bind",
-             "sec_cm_prim_sparse_fsm_flop_bind", "sec_cm_prim_onehot_check_bind"]
+  sim_tops: ["rstmgr_bind", "rstmgr_cov_bind",
+             "sec_cm_prim_sparse_fsm_flop_bind",
+	     "sec_cm_prim_onehot_check_bind"]
 
   // Default iterations for all tests - each test entry can override this.
   reseed: 50

diff --git a/hw/ip_templates/rstmgr/dv/sva/rstmgr_bind.sv b/hw/ip_templates/rstmgr/dv/sva/rstmgr_bind.sv
@@ -8,6 +8,11 @@ module rstmgr_bind;
     .EndpointType("Device")
   ) tlul_assert_device (.clk_i, .rst_ni, .h2d(tl_i), .d2h(tl_o));
 
+  // In top-level testbench, do not bind the csr_assert_fpv to reduce simulation time.
+`ifndef TOP_LEVEL_DV
+  bind rstmgr rstmgr_csr_assert_fpv rstmgr_csr_assert (.clk_i, .rst_ni, .h2d(tl_i), .d2h(tl_o));
+`endif
+
   bind rstmgr rstmgr_cascading_sva_if rstmgr_cascading_sva_if (
     .clk_i,
     .clk_aon_i,
@@ -34,6 +39,19 @@ module rstmgr_bind;
     .expected_cpu_info_attr(($bits(cpu_dump_i) + 31) / 32)
   );
 
+  bind rstmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
+    .clk_i(clk_i),
+    .rst_ni(rst_ni),
+    .clk_slow_i(clk_aon_i),
+    .rst_slow_ni(&rst_por_aon_n),
+    // These are actually used for checks.
+    .rst_lc_req(pwr_i.rst_lc_req),
+    .rst_sys_req(pwr_i.rst_sys_req),
+    // The inputs from rstmgr.
+    .rst_lc_src_n(pwr_o.rst_lc_src_n),
+    .rst_sys_src_n(pwr_o.rst_sys_src_n)
+  );
+
   bind rstmgr rstmgr_sw_rst_sva_if rstmgr_sw_rst_sva_if (
     .clk_i({
       clk_io_div4_i,

diff --git a/hw/ip_templates/rstmgr/dv/sva/rstmgr_sva.core b/hw/ip_templates/rstmgr/dv/sva/rstmgr_sva.core
@@ -9,17 +9,25 @@ filesets:
     depend:
       - lowrisc:prim:mubi
       - lowrisc:ip_interfaces:rstmgr_pkg
+      - lowrisc:fpv:csr_assert_gen
       - lowrisc:dv:rstmgr_sva_ifs
 
     files:
       - rstmgr_bind.sv
-      - rstmgr_rst_en_track_sva_if.sv
     file_type: systemVerilogSource
 
+generate:
+  csr_assert_gen:
+    generator: csr_assert_gen
+    parameters:
+      spec: ../../data/rstmgr.hjson
+
 targets:
   default: &default_target
     filesets:
       - files_dv
+    generate:
+      - csr_assert_gen
   formal:
     <<: *default_target
     filesets:

diff --git a/hw/ip_templates/rstmgr/dv/sva/rstmgr_sva_ifs.core b/hw/ip_templates/rstmgr/dv/sva/rstmgr_sva_ifs.core
@@ -10,10 +10,12 @@ filesets:
       - lowrisc:ip:lc_ctrl_pkg
       - lowrisc:ip_interfaces:pwrmgr_pkg
       - lowrisc:ip_interfaces:rstmgr
+      - lowrisc:dv:pwrmgr_rstmgr_sva_if
 
     files:
       - rstmgr_attrs_sva_if.sv
       - rstmgr_cascading_sva_if.sv
+      - rstmgr_rst_en_track_sva_if.sv
       - rstmgr_sw_rst_sva_if.sv
     file_type: systemVerilogSource
 

diff --git a/hw/ip_templates/rstmgr/dv/sva/rstmgr_unit_only_bind.sv b/hw/ip_templates/rstmgr/dv/sva/rstmgr_unit_only_bind.sv
diff --git a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_main_power_glitch_vseq.sv b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_main_power_glitch_vseq.sv
@@ -14,8 +14,8 @@ class chip_sw_main_power_glitch_vseq extends chip_sw_base_vseq;
   virtual task pre_start();
     super.pre_start();
     // disable expected assertion error for power glitch test
-    $assertoff(1,"tb.dut.top_earlgrey.pwrmgr_rstmgr_sva_if.MainPwrRstOff_A");
-    $assertoff(1,"tb.dut.top_earlgrey.pwrmgr_rstmgr_sva_if.MainPwrRstOn_A");
+    $assertoff(1,"tb.dut.top_earlgrey.pwrmgr_rstreqs_sva_if.MainPwrRstOff_A");
+    $assertoff(1,"tb.dut.top_earlgrey.pwrmgr_rstreqs_sva_if.MainPwrRstOn_A");
   endtask
 
   virtual task body();

diff --git a/hw/top_earlgrey/ip_autogen/pwrmgr/dv/pwrmgr_sim.core b/hw/top_earlgrey/ip_autogen/pwrmgr/dv/pwrmgr_sim.core
@@ -12,6 +12,7 @@ filesets:
     depend:
       - lowrisc:dv:pwrmgr_test
       - lowrisc:dv:pwrmgr_sva
+      - lowrisc:dv:pwrmgr_unit_only_sva
     files:
       - tb.sv
       - cov/pwrmgr_cov_bind.sv

diff --git a/hw/top_earlgrey/ip_autogen/pwrmgr/dv/pwrmgr_sim_cfg.hjson b/hw/top_earlgrey/ip_autogen/pwrmgr/dv/pwrmgr_sim_cfg.hjson
@@ -49,6 +49,7 @@
   // Add additional tops for simulation.
   sim_tops: ["pwrmgr_bind",
              "pwrmgr_cov_bind",
+             "pwrmgr_unit_only_bind",
              "sec_cm_prim_count_bind",
              "sec_cm_prim_sparse_fsm_flop_bind",
              "sec_cm_prim_onehot_check_bind"]

diff --git a/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_bind.sv b/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_bind.sv
@@ -33,20 +33,7 @@ module pwrmgr_bind;
     .io_clk_en(pwr_ast_o.io_clk_en),
     .usb_clk_en(pwr_ast_o.usb_clk_en)
   );
-
-  bind pwrmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
-    .clk_i,
-    .rst_ni,
-    .clk_slow_i,
-    .rst_slow_ni,
-    // The outputs from pwrmgr.
-    .rst_lc_req(pwr_rst_o.rst_lc_req),
-    .rst_sys_req(pwr_rst_o.rst_sys_req),
-    // The inputs from rstmgr.
-    .rst_lc_src_n(pwr_rst_i.rst_lc_src_n),
-    .rst_sys_src_n(pwr_rst_i.rst_sys_src_n)
-  );
-
+
   bind pwrmgr clkmgr_pwrmgr_sva_if #(.IS_USB(0)) clkmgr_pwrmgr_io_sva_if (
     .clk_i,
     .rst_ni,

diff --git a/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_unit_only_bind.sv b/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_unit_only_bind.sv
@@ -0,0 +1,21 @@
+// Copyright lowRISC contributors (OpenTitan project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// This binds assertions that should not be bound at chip level.
+module pwrmgr_unit_only_bind;
+
+  bind pwrmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
+    .clk_i,
+    .rst_ni,
+    .clk_slow_i,
+    .rst_slow_ni,
+    // The outputs from pwrmgr.
+    .rst_lc_req(pwr_rst_o.rst_lc_req),
+    .rst_sys_req(pwr_rst_o.rst_sys_req),
+    // The inputs from rstmgr.
+    .rst_lc_src_n(pwr_rst_i.rst_lc_src_n),
+    .rst_sys_src_n(pwr_rst_i.rst_sys_src_n)
+  );
+
+endmodule : pwrmgr_unit_only_bind
diff --git a/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_unit_only_sva.core b/hw/top_earlgrey/ip_autogen/pwrmgr/dv/sva/pwrmgr_unit_only_sva.core
@@ -0,0 +1,39 @@
+CAPI=2:
+# Copyright lowRISC contributors (OpenTitan project).
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+name: "lowrisc:dv:pwrmgr_unit_only_sva:0.1"
+gdescription: "PWRMGR assertion interfaces not suitable for chip level bind file."
+filesets:
+  files_dv:
+    depend:
+      - lowrisc:tlul:headers
+      - lowrisc:fpv:csr_assert_gen
+      - lowrisc:dv:pwrmgr_rstmgr_sva_if
+
+    files:
+      - pwrmgr_unit_only_bind.sv
+    file_type: systemVerilogSource
+
+  files_formal:
+    depend:
+      - lowrisc:ip_interfaces:pwrmgr
+
+generate:
+  csr_assert_gen:
+    generator: csr_assert_gen
+    parameters:
+      spec: ../../data/pwrmgr.hjson
+
+targets:
+  default: &default_target
+    filesets:
+      - files_dv
+    generate:
+      - csr_assert_gen
+  formal:
+    <<: *default_target
+    filesets:
+      - files_formal
+      - files_dv
+    toplevel: pwrmgr
diff --git a/hw/top_earlgrey/ip_autogen/rstmgr/dv/rstmgr_sim.core b/hw/top_earlgrey/ip_autogen/rstmgr/dv/rstmgr_sim.core
@@ -13,7 +13,6 @@ filesets:
     depend:
       - lowrisc:dv:rstmgr_test
       - lowrisc:dv:rstmgr_sva
-      - lowrisc:dv:rstmgr_unit_only_sva
     files:
       - tb.sv
       - cov/rstmgr_cov_bind.sv

diff --git a/hw/top_earlgrey/ip_autogen/rstmgr/dv/rstmgr_sim_cfg.hjson b/hw/top_earlgrey/ip_autogen/rstmgr/dv/rstmgr_sim_cfg.hjson
@@ -53,8 +53,9 @@
   ]
 
   // Add additional tops for simulation.
-  sim_tops: ["rstmgr_bind", "rstmgr_unit_only_bind", "rstmgr_cov_bind",
-             "sec_cm_prim_sparse_fsm_flop_bind", "sec_cm_prim_onehot_check_bind"]
+  sim_tops: ["rstmgr_bind", "rstmgr_cov_bind",
+             "sec_cm_prim_sparse_fsm_flop_bind",
+	     "sec_cm_prim_onehot_check_bind"]
 
   // Default iterations for all tests - each test entry can override this.
   reseed: 50

diff --git a/hw/top_earlgrey/ip_autogen/rstmgr/dv/sva/rstmgr_bind.sv b/hw/top_earlgrey/ip_autogen/rstmgr/dv/sva/rstmgr_bind.sv
@@ -8,6 +8,11 @@ module rstmgr_bind;
     .EndpointType("Device")
   ) tlul_assert_device (.clk_i, .rst_ni, .h2d(tl_i), .d2h(tl_o));
 
+  // In top-level testbench, do not bind the csr_assert_fpv to reduce simulation time.
+`ifndef TOP_LEVEL_DV
+  bind rstmgr rstmgr_csr_assert_fpv rstmgr_csr_assert (.clk_i, .rst_ni, .h2d(tl_i), .d2h(tl_o));
+`endif
+
   bind rstmgr rstmgr_cascading_sva_if rstmgr_cascading_sva_if (
     .clk_i,
     .clk_aon_i,
@@ -34,6 +39,19 @@ module rstmgr_bind;
     .expected_cpu_info_attr(($bits(cpu_dump_i) + 31) / 32)
   );
 
+  bind rstmgr pwrmgr_rstmgr_sva_if pwrmgr_rstmgr_sva_if (
+    .clk_i(clk_i),
+    .rst_ni(rst_ni),
+    .clk_slow_i(clk_aon_i),
+    .rst_slow_ni(&rst_por_aon_n),
+    // These are actually used for checks.
+    .rst_lc_req(pwr_i.rst_lc_req),
+    .rst_sys_req(pwr_i.rst_sys_req),
+    // The inputs from rstmgr.
+    .rst_lc_src_n(pwr_o.rst_lc_src_n),
+    .rst_sys_src_n(pwr_o.rst_sys_src_n)
+  );
+
   bind rstmgr rstmgr_sw_rst_sva_if rstmgr_sw_rst_sva_if (
     .clk_i({
       clk_io_div4_i,