[keymgr/dv] Update testplan and covergroup plan

Reviewed this on V2 review meeting
Also added covergrop as discussed in the review.
Signed-off-by: Weicai Yang <weicai@google.com>
diff --git a/hw/dv/tools/dvsim/testplans/harden_countermeasures_testplan.hjson b/hw/dv/tools/dvsim/testplans/harden_countermeasures_testplan.hjson
new file mode 100644
index 0000000..7dea3a4
--- /dev/null
+++ b/hw/dv/tools/dvsim/testplans/harden_countermeasures_testplan.hjson
@@ -0,0 +1,52 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+{
+  // TODO, split this into several testplan for each CM
+  testpoints: [
+    {
+      name: one_hot_check
+      desc: ''' Verify design behavior is correct when one-hot coding is violated.
+
+            Stimulus:
+            - Backdoor force one-hot coding signals to not one-hot.
+            - Randomly flip the value back to ensure the error is latched and won't go away until
+              reset.
+
+            Checks:
+            - Check that fatal alert is triggered.
+            - Check that err_code/fault_status is updated correctly and preserved until reset.
+            - Check the following operation should be failed if applicable.'''
+      milestone: V2
+      tests: ["{name}_harden_countermeasures"]
+    }
+    {
+      name: redundant_coding_fsm_check
+      desc: ''' Verify design behavior is correct when the redundant FSM enters an invalid state.
+
+            Stimulus:
+            - Backdoor force the FSM to any of the undefined values.
+            - Randomly flip the value back to a defined state to ensure the error is latched and
+               won't go away until reset.
+
+            Same checks as `one_hot_check`'''
+      milestone: V2
+      tests: ["{name}_harden_countermeasures"]
+    }
+    {
+      name: hardened_counter_check
+      desc: ''' Verify design behavior is correct when the harden counter is changed to an
+            unexpected value.
+
+            Stimulus:
+            - At the falling edge (non-active edge), force the counter to a different value.
+            - Randomly flip the value back to any other value to ensure the error is latched and
+              won't go away until reset.
+
+            Same checks as `one_hot_check`'''
+      milestone: V2
+      tests: ["{name}_harden_countermeasures"]
+    }
+  ]
+}
+
diff --git a/hw/ip/keymgr/data/keymgr_testplan.hjson b/hw/ip/keymgr/data/keymgr_testplan.hjson
index 9fb1b1a..b25bd34 100644
--- a/hw/ip/keymgr/data/keymgr_testplan.hjson
+++ b/hw/ip/keymgr/data/keymgr_testplan.hjson
@@ -8,6 +8,7 @@
                      "hw/dv/tools/dvsim/testplans/alert_test_testplan.hjson",
                      "hw/dv/tools/dvsim/testplans/tl_device_access_types_testplan.hjson",
                      "hw/dv/tools/dvsim/testplans/shadow_reg_errors_testplan.hjson",
+                     "hw/dv/tools/dvsim/testplans/harden_countermeasures_testplan.hjson",
                      "hw/dv/tools/dvsim/testplans/stress_all_with_reset_testplan.hjson"]
   testpoints: [
     {
@@ -21,6 +22,8 @@
             - Issue gen-id, gen-sw-output operation in each state, including invalid operations in
               states other than normal operating states (`StCreatorRootKey`, `StOwnerIntKey` and
               `StOwnerRootKey`).
+            - Randomize `CDI_SEL` and `DEST_SEL`.
+            - Use default/fixed values for HW/SW inputs.
 
             Checks:
             - Check STATUS reg for each operation.
@@ -38,11 +41,13 @@
       name: random
       desc: '''
             Extend from smoke to randomize all SW input data
-            - Fully randomize SW input: rom_ext_desc_*, software_binding_*, salt_*, max_*_key_ver,
-              *_key_ver_en.
+            - Fully randomize SW inputs: rom_ext_desc_*, software_binding_*, salt_*, max_*_key_ver,
+              *_key_ver_regwen.
             - Randomize key_version any value less than max_*_key_ver, to avoid triggerring
               `invalid_kmac_input` error.
-            - Fully randomize HW input from flash, otp and life cycle.
+            - Fully randomize HW inputs from flash, otp and life cycle.
+            - Randomize *sw_binding_regwen. Ensure this gates the *_sw_binding and it will
+              be cleared after a successful advance operation.
 
             Most of other sequences are derived from this to have similar init and sequence.
 
@@ -54,22 +59,11 @@
     {
       name: cfgen_during_op
       desc: '''
-            CFGEN is RO reg and it gates bunch of write access of other registers, which isn't
-            tested in common CSR tests.
+            `cfg_regwen` is RO reg and it gates bunch of write access of other registers, which is
+            not tested in common CSR tests.
 
             Stimulus and checks:
-            Test command and reg access gated by CFGEN is ignored during operation.
-            '''
-      milestone: V2
-      tests: ["keymgr_cfg_regwen"]
-    }
-    {
-      name: cfgen_at_st_random
-      desc: '''
-            `StRandom` is a temporary state during init operation, which will set CSR CFGEN.
-
-            Stimulus and checks:
-            Test command and reg access gated by CFGEN is ignored during `StRandom`.
+            Test command and reg access gated by `cfg_regwen` is ignored during operation.
             '''
       milestone: V2
       tests: ["keymgr_cfg_regwen"]
@@ -77,10 +71,12 @@
     {
       name: sideload
       desc: '''
-            Keymgr contains HW sideload interfaces to output keys for KMAC, HMAC, AES.
+            Keymgr contains HW sideload interfaces to output keys for KMAC, AES, OTBN.
 
             Stimulus:
-            Generate a keymgr output to HW sideload interface, excising all the sideload interfaces.
+            - Generate a keymgr output to HW sideload interface, exercising all the sideload
+              interfaces.
+            - Randomly program any value to Sideload_clear after any operation.
 
             Checks:
             Verify the sideload data and status for correctness.
@@ -90,17 +86,6 @@
               "keymgr_sideload_aes", "keymgr_sideload_otbn"]
     }
     {
-      name: init_n_start
-      desc: '''
-            Stimulus and checks:
-            Verify all combinations of `init` and `start`.
-            - If both are set in `StReset`, check `INVALID_OP` error is triggered.
-            - If it's in other state, `init` is ignored.
-            '''
-      milestone: V2
-      tests: ["keymgr_random"]
-    }
-    {
       name: direct_to_disabled_state
       desc: '''
             Stimulus and checks:
@@ -120,35 +105,14 @@
             Checks:
             - If keymgr is not initialized, check it can't be initialized until life cycle enables
               keymgr.
-            - If keymgr is in a valid state after init, key output to KMAC is wiped immediately and
-              SW output will be invalid after OP is done.
+            - If keymgr is in a valid state after `StReset`, key output to KMAC is wiped immediately
+              and SW output will be invalid after OP is done.
             - If keymgr in disabled state, check the behavior is consistent with normal behavior.
             '''
       milestone: V2
       tests: ["keymgr_lc_disable"]
     }
     {
-      name: invalid_cmd
-      desc: '''
-            Verify `keymgr` security countermeasures on fault injection.
-
-            Stimulus:
-            Force one of these or both
-            - Force advance, gen_id and gen_out to not one-hot
-              - Path: `dut.u_ctrl.adv_en_o/id_en_o/gen_en_o`
-            - Force KMAC FSM to default branch to trigger FSM error
-              - Path: `dut.u_kmac_if.state_d`
-
-            Checks:
-            - Check interrupts `err` is triggered.
-            - Check alert `fatal_fault_err` is triggered and err_code is `INVALID_CMD`.
-            - Check KMAC output key and output data to SW are corrupted, also keymgr enters
-              StDisabled.
-            '''
-      milestone: V2
-      tests: ["keymgr_cmd_invalid"]
-    }
-    {
       name: kmac_error_response
       desc: '''
             Verify `keymgr` behavior on error response received from `KMAC` after sending data to
@@ -197,13 +161,109 @@
       tests: ["keymgr_hwsw_invalid_input"]
     }
     {
+      name: sync_async_fault_cross
+      desc: '''
+            Verify `keymgr` behavior with invalid data patterns.
+
+            Stimulus:
+            Create these 2 direct tests:
+            - Sync (transactional) fault occurs followed by async (non-transactional) fault.
+            - Async (non-transactional) fault occurs followed by sync (transactional) fault.
+
+            Checks:
+            - Check interrupts `err` is triggered.
+            - Check alert `fatal_fault_err` is triggered.
+            - Check `fault_status` is updated correctly.
+            '''
+      milestone: V2
+      tests: ["keymgr_sync_async_fault_cross"]
+    }
+    {
       name: stress_all
       desc: '''
             - Combine above sequences in one test to run sequentially, except csr sequence and
-              keymgr_cfg_regwen (requires zero_delays)
-            - Randomly add reset between each sequence'''
+              keymgr_cfg_regwen (requires zero_delays).
+            - Randomly add reset between each sequence.'''
       milestone: V2
       tests: ["keymgr_stress_all"]
     }
   ]
+  covergroups: [
+    {
+      name: state_and_op_cg
+      desc: '''
+            - Cover all operations with `cdi_sel`, `dest_sel` and `op_status` (only fail or success)
+            at any of all working_states.
+            - This is sampled once an operation is done.'''
+    }
+    {
+      name: lc_disable_cg
+      desc: '''
+             - Cover LC disable occurs at any of all the states or during any of all the
+             operations.
+             - This is sampled once LC disables keymgr.'''
+    }
+    {
+      name: sideload_clear_cg
+      desc: '''
+            - Cover all the `sideload_clear` values are used after any of all the operations and in
+            any of all the states.
+            - Cover `sideload_clear` with any combination of availability of 3 sideload interfaces.
+            - This is sampled once sideload_clear is programmed after an operation.'''
+    }
+    {
+      name: reseed_interval_cg
+      desc: '''
+            - Cover small values of reseed_interval are used, so that TB can actually check EDN
+              request is sent in the right interval.
+            - Also Cover some large values to ensure all bits are toggled.'''
+    }
+    {
+      name: keymgr_sw_input_cg
+      desc: '''
+            - Cover all bits of SW inputs are toggled.
+            - SW input includes these CSRS: `*_sw_binding`, `salt`, `key_version`, `max_*_key_ver*`.
+            - Cross with the corresponding regwen.'''
+    }
+    {
+      name: control_w_regwen_cg
+      desc: '''
+            - Similar to keymgr_sw_input_cg, create a separate cg as there are some reserved fields
+              scattered in the CSR.
+            '''
+    }
+    {
+      name: err_code_cg
+      desc: '''
+            - Cover `err_codes` values except `invalid_shadow_update` as that is tested in a common
+              direct test.
+            - This is sampled when `err_codes` is read.'''
+    }
+    {
+      name: hw_invalid_input_cg
+      desc: '''
+            Cover all HW invalid inputs, including
+            - all ones/zeros on OTP root key.
+            - OTP root key valid is low.
+            - all ones/zeros on LC keymgr health state.
+            - all ones/zeros on ROM degist.
+            - ROM degist valid is low.
+            - all ones/zeros on flash creator seeds.
+            - all ones/zeros on flash owner seeds.'''
+    }
+    {
+      name: key_version_compare_cg
+      desc: '''
+            - Cover comparison results (equal, less, greater) of key_version and current max value.
+            - Cross with state and operation (gen-sw-out or gen-hw-out).'''
+    }
+    {
+      name: fault_status_cg
+      desc: '''
+            - Cover fault_status` values except `REGFILE_INTG` and `SHADOW` as they are tested in
+              a common direct test.
+            - Cover sync and async fault cross with each other.
+            - This is sampled when `fault_status` is read.'''
+    }
+  ]
 }
diff --git a/hw/ip/keymgr/dv/env/keymgr_env_cov.sv b/hw/ip/keymgr/dv/env/keymgr_env_cov.sv
index d43e19e..e791613 100644
--- a/hw/ip/keymgr/dv/env/keymgr_env_cov.sv
+++ b/hw/ip/keymgr/dv/env/keymgr_env_cov.sv
@@ -73,7 +73,10 @@
   // Covergroup to sample sideload_clear in all the states and followed by all the operations
   covergroup sideload_clear_cg with function sample(bit [2:0] sideload_clear,
                                                     keymgr_pkg::keymgr_working_state_e state,
-                                                    keymgr_pkg::keymgr_ops_e op);
+                                                    keymgr_pkg::keymgr_ops_e op,
+                                                    bit aes_sl_avail,
+                                                    bit kmac_sl_avail,
+                                                    bit otbn_sl_avail);
     sideload_clear_cp: coverpoint sideload_clear {
       bins clear_none  = {0};
       bins clear_one[] = {[1:3]};
@@ -83,7 +86,12 @@
     state_cp:          coverpoint state;
     // the operation followed by sideload_clear
     op_cp:             coverpoint op;
+    aes_sl_avail_cp:   coverpoint aes_sl_avail;
+    kmac_sl_avail_cp:  coverpoint kmac_sl_avail;
+    otbn_sl_avail_cp:  coverpoint otbn_sl_avail;
     sideload_clear_x_state_op_cross: cross sideload_clear, state, op;
+    sideload_clear_x_sl_avail_cross: cross sideload_clear_cp, aes_sl_avail, kmac_sl_avail,
+                                           otbn_sl_avail;
   endgroup
 
   covergroup err_code_cg with function sample(keymgr_pkg::keymgr_err_pos_e err_code);
diff --git a/hw/ip/keymgr/dv/env/keymgr_env_pkg.sv b/hw/ip/keymgr/dv/env/keymgr_env_pkg.sv
index 7ed72c7..009e927 100644
--- a/hw/ip/keymgr/dv/env/keymgr_env_pkg.sv
+++ b/hw/ip/keymgr/dv/env/keymgr_env_pkg.sv
@@ -48,6 +48,12 @@
     FlashOwnerSeedInvalid
   } keymgr_invalid_hw_input_type_e;
 
+  typedef enum bit[1:0] {
+    SideLoadNotAvail,
+    SideLoadAvail,
+    SideLoadClear
+  } keymgr_sideload_status_e;
+
   string msg_id = "keymgr_env_pkg";
   // functions
   // state is incremental, if it's not in defined enum, consider as StDisabled
diff --git a/hw/ip/keymgr/dv/env/keymgr_if.sv b/hw/ip/keymgr/dv/env/keymgr_if.sv
index 6428e8d..d9d818e 100644
--- a/hw/ip/keymgr/dv/env/keymgr_if.sv
+++ b/hw/ip/keymgr/dv/env/keymgr_if.sv
@@ -15,12 +15,6 @@
   // Status can't be directly changed from SideLoadClear to SideLoadAvail.
   // When status is SideLoadClear due to SIDELOAD_CLEAR programmed, need to write CSR to 0 to reset
   // it so that status is changed to SideLoadNotAvail, then we may set it to SideLoadAvail again
-  typedef enum bit[1:0] {
-    SideLoadNotAvail,
-    SideLoadAvail,
-    SideLoadClear
-  } keymgr_sideload_status_e;
-
   lc_ctrl_pkg::lc_tx_t            keymgr_en;
   lc_ctrl_pkg::lc_keymgr_div_t    keymgr_div;
   otp_ctrl_pkg::otp_device_id_t   otp_device_id;
diff --git a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
index 61a4f10..3c9b7d4 100644
--- a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
+++ b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv
@@ -676,7 +676,12 @@
       "sideload_clear": begin
         if (addr_phase_write) begin
           if (cfg.en_cov) begin
-            cov.sideload_clear_cg.sample(`gmv(ral.sideload_clear.val), current_state, get_operation());
+            cov.sideload_clear_cg.sample(`gmv(ral.sideload_clear.val),
+                                         current_state,
+                                         get_operation(),
+                                         cfg.keymgr_vif.aes_sideload_status == SideLoadAvail,
+                                         cfg.keymgr_vif.kmac_sideload_status == SideLoadAvail,
+                                         cfg.keymgr_vif.otbn_sideload_status == SideLoadAvail);
           end
 
           fork