Instruction Fetch Requests Generate X's during Fuzz Testing

[cnokes14]

Instruction Fetch Requests Generate X's during Fuzz Testing

During fuzz testing, instruction requests (instr_req_o and instr_reqpar_o) can have X's.

This seems to be because of a cyclic path of unclocked signals across stages:
lsu_ready_i (execute stage) controls wb_ready_o (execute stage)
wb_ready_o (execute stage) controls wb_ready_i (controller)
wb_ready_i (controller) controls lsu_ready_o (controller)
lsu_ready_o (controller) controls ready_0_i (load/store stage)
ready_0_i (load/store stage) controls ready_0_o (load/store stage)
ready_0_o (load/store stage) controls lsu_ready_i (execute stage)

In cases where instr_req_o is not X, it is because this path is cancelled out (IE: OR'd with 1, AND'd with 0) by additional logic somewhere along the way.

Additionally, if the PMP is enabled and configured, the X may occur during a cycle where instr_addr_o should not be accessible by the core (EX: if MMWP is set, the core may set instr_req_o = X while instr_addr_o is not covered by the PMP).

Component

Issue seems to span multiple RTL components, namely cv32e40s_ex_stage.sv, cv32e40s_controller.sv, cv32e40s_load_store_unit.sv, and cv32e40s_core.sv.

Steps to Reproduce

1. Use this fuzz test.
2. X's seem to occur relatively frequently.

This screenshot is from running the test I provided. Of note, the X that occurs at 185ns occurs alongside an instruction address which should be blocked by the PMP.

[Silabs-ArjanB]

Hi @cnokes14 , thank you for your detailed report. The cyclic path you report cannot be as stated as the execute stage does not control wb_ready_o (wb_ready comes from the WB stage), so the following parts in bold cannot be true:

lsu_ready_i (execute stage) controls wb_ready_o (execute stage)
wb_ready_o (execute stage) controls wb_ready_i (controller)

If there is a loop it must be a different one; could you please correct above loop info?

Are you 100% sure all inputs to the top level are non-X at all times (other then maybe time 0)? (I cannot see that from above screenshot)

Are you using https://github.com/openhwgroup/cv32e40s/blob/master/bhv/cv32e40s_sim_clock_gate.sv or did you add your own design?

What simulator are you using?

Best regards,
Arjan

[cnokes14]

Hi Arjan,

I'm working in Xilinx Vivado (2024.1) with a fresh download of the core. I am using sim_clock_gate, sim_sffr, and sim_sffs. The only file that isn't taken from the repository is the test bench I'm using.
No inputs are X's. I've included every signal at the top level over the first 30 clock cycles as a full sanity check. I'm sure this is more than you asked for, but better safe than sorry!

Sorry about the incorrect cyclic signals, I was copying things over from my very messy personal notes and it seems something was lost in translation. I'll double check that and get back.

[cnokes14]

In doing some further investigation, it appears my initial cyclic signals report was incorrect.
Instead, this error is generated by index-out-of-bounds errors occurring in cv32e40s_obi_integrity_fifo.sv and cv32e40s_lsu_response_filter.sv. Both fifo_q (cv32e40s_obi_integrity_fifo.sv) and outstanding_q (cv32e40s_lsu_response_filter.sv) are three elements long, but are indexed by two bit numbers with no protection.

I've implemented a hasty local fix (just stopping increments at 2'b10 and decrements at 2'b00), which seems to be preventing this bug from occurring. I can create a pull request some time in the next few days with a nicer fix.

[Silabs-ArjanB]

Hi @cnokes14 (Apart from the out of bound indexing) do you still think there is a cyclic path? (I got an email with more detailed signals but I don't see that in this github issue)

[cnokes14]

Hi Arjan,

No, I think the cyclic signals part was just a mistake on my end. I realized my original comment was in error and edited it to correct myself.
Following the path of X's through the core, they all end up originating from an out of bounds access. There's no sign of anything cyclic; my initial guess on cyclic signals probably just came from getting lost in similarly-named top level signals.