search

lowRISC / ibex

Ibex is a small 32 bit RISC-V CPU core, previously known as zero-riscy.
https://www.lowrisc.org
Apache License 2.0
1.34k stars 520 forks source link

[rtl] Behaviour of debug single stepping with dummy instruction insertion may be surprising #2186

Open GregAC opened 2 months ago

GregAC commented 2 months ago

I have observed a scenario where Ibex is single stepping whilst dummy instruction insertion is enabled. When a dummy instruction is inserted just as Ibex returns from debug mode to single step the next instruction, it single steps the dummy instruction instead. The depc is set correctly (i.e. it will rerun the instruction that should have been single stepped after a dret) but it could cause confusion for a debugger user and might break the debugger itself (that may reasonably assume a single step will always occur) but the instruction won't be incorrectly skipped.

This results in a mismatch with the cosim as it doesn't observe the dummy instruction on the RVFI interface so it sees the debug vector being re-entered without the single stepped instruction being executed.

To reproduce run:

make TEST=riscv_debug_single_step_test SEED=6250 ITERATIONS=1 WAVES=1 against commit 3384bf4c421781a2f5129307b912ac586c784977

There's three options I see here:

  1. Declare this behaviour is reasonable
  2. Force dummy instruction insertion to be disabled when running in single step
  3. Alter the controller behaviour so we'll execute the dummy instruction and then single step

Option 3 is the most complex, option 2 should be pretty simple though does require an RTL change. Option 1 seems reasonable for OpenTitan (so no RTL fix needed for Earlgrey).

For any of these options we should look at DV in this area to ensure this scenario is properly covered.

GregAC commented 2 months ago

@vogelpi @andreaskurth please note and in particular give your views on whether you're happy to leave this behaviour in earlgrey.

andreaskurth commented 2 months ago

I think option 1 is reasonable for this release since this doesn't seem to be a severe problem, but in a future release we may want to implement option 3 or 2.

vogelpi commented 2 months ago

Thanks for creating the issue and describing the behavior @GregAC . I share @andreaskurth 's view that this behavior is acceptable for Earlgrey. When debugging, user can also disable dummy instruction insertion but it's good to know that if it stays enabled, single stepping isn't completely broken or similar. For a future release, I would favor Option 3.