openhwgroup / cv32e40s

4 stage, in-order, secure RISC-V core based on the CV32E40P
https://docs.openhwgroup.org/projects/cv32e40s-user-manual/en/latest/
Other
129 stars 22 forks source link

Dummy instructions: counter deciding when to insert dummy instructions resets unintentionally #456

Closed silabs-hfegran closed 1 year ago

silabs-hfegran commented 1 year ago

Dummy instruction insertion is suspended when the delay between insertions gets larger than a handful of instructions (around 10 instructions is observed in the trace below). Assertions (design and verif) depend on the same erroneous behavior and fails to catch the design error.

Cause

counter resets every time the prefetcher status is not valid (cf. code below)

cv32e40s_dummy_instr.sv:
  assign dummy_en   = ctrl_fsm_i.allow_dummy_instr && xsecure_ctrl_i.cpuctrl.rnddummy;

  assign cnt_rst        = !dummy_en                          ||      // Reset counter when dummy instructions are disabled
                          (dummy_insert_o && instr_issued_i) ||      // Reset counter when inserting dummy instruction which is propagated to the ID stage
                          xsecure_ctrl_i.cntrst;                     // Reset counter when requested by xsecure_ctrl (due to csr updates)

cv32e40s_controller_fsm.sv:
  assign  ctrl_fsm_o.allow_dummy_instr = !dcsr_i.step  &&  // Valid in IF because it can only be written in debug mode
                                         !debug_mode_q &&  // Valid in IF because pipeline is killed when entering and exiting debug
-                                         (first_op_nondummy_if_i && prefetch_valid_if_i);

Steps to Reproduce

  1. git hash: 8d049d2a286872c466dbfe12c64fa5b2d75628e3
  2. Program secure seed 0-2 CSRs with 80000057, 80000062, 8000007A respectively.
  3. Program cpuctrl to enable random dummy instructions and frequency 1-64
  4. Should work for any test that run a few thousand instructions (Only tested locally with a modified branch of core-v-verif, will likely be upstreamed in the near future) Screenshot 2023-05-23 at 10 03 08 AM