Wrong MEPC saved with branch predictor configuration
Nobody has claimed this yet.
- Dominant language
- SystemVerilog
- Stars
- 2.1k
- Forks
- 810
- Avg merge
- 5d 23h
- Merged PRs (30d)
- 9
Description
## Observed Behavior
During long LSU stalls, predicted taken branches can cause the skid buffer to be loaded with a speculative address. Unfortunately, an interrupt taken during the stall may cause this value to be saved into `mepc`. When the ISR returns, incorrect behaviour can be observed.
To demonstrate, I have made a few changes to the simple system and written some basic software to demonstrate: https://github.com/sam-shahrestani/ibex/commit/ab12b99b38c917eb950433c2a581c74c4660d2d5
The relevant disassembly:
```assembly
0010033e :
#include "simple_system_common.h"
volatile int foo;
void memory_access_fail(void)
{
10033e: 1141 addi sp,sp,-16
puts("Test failed!\n");
100340: 00000517 auipc a0,0x0
100344: 11450513 addi a0,a0,276 # 100454
{
100348: c606 sw ra,12(sp)
puts("Test failed!\n");
10034a: 3399 jal 100090
sim_halt();
}
10034c: 40b2 lw ra,12(sp)
10034e: 0141 addi sp,sp,16
sim_halt();
100350: bba5 j 1000c8
00100352 :
/* Write foo once, then read it indefinitely, failing if the set value has changed */
void memory_access_loop(void)
{
foo = 0xdeadbeef;
100352: deadc737 lui a4,0xdeadc
100356: 00000797 auipc a5,0x0
10035a: 12278793 addi a5,a5,290 # 100478
10035e: eef70713 addi a4,a4,-273 # deadbeef
100362: c398 sw a4,0(a5)
asm volatile("" ::: "memory");
while (1)
{
asm volatile (
100364: a011 j 100368
100366: 3fe1 jal 10033e
100368: deadc2b7 lui t0,0xdeadc
10036c: eef28293 addi t0,t0,-273 # deadbeef
100370: 0007a303 lw t1,0(a5)
100374: fe6299e3 bne t0,t1,100366
while (1)
100378: b7f5 j 100364
```
`memory_access_loop` runs while timer interrupts are constantly going off. There is a branch targeting a (call to a) simulation halting function at `100366`, but the comparison is a constant to a memory value that never changes, so this branch will **never be taken**. It is a backwards branch, so it is **always predicted taken**.
At the time of the failing round, one can observe `100366` being saved into `mepc` because `pc_if_o` is set to a speculative address by `instr_skid_valid_q`. Instead, it should be set to the corrected `fetch_addr` after a mispredict is detected (i.e. `100378`).
I will attach a trace file in a comment for more detail.
## Expected Behavior
Do not save an incorrect MEPC.
## Steps to reproduce the issue
- Build the ibex simple system using the given branch with the configuration `fusesoc --cores-root=. run --target=sim --setup --build lowrisc:ibex:ibex_simple_system `./util/ibex_config.py experimental-branch-predictor fusesoc_opts``
- Build the software example with `make -C examples/sw/simple_system/mepc_test`
- Run the test with ` ./build/lowrisc_ibex_ibex_simple_system_0/sim-verilator/Vibex_simple_system -t --meminit=ram,./examples/sw/simple_system/mepc_test/mepc_test.elf`
- Observe the test does not loop indefinitely, it crashes with the log message "Test failed!"
- Additionally, using a BP-less config (such as small) or applying the suggested RTL fix in `ibex_if_stage.sv`, the test program will loop indefinitely (which is correct behaviour)
## My Environment
**Version of the Ibex source code:**
You can see this on master, i.e. `9e8b32a13802db36515a8ca042a5e9889cdf9218`, but only with LSU stalls. The [provided branch](https://github.com/sam-shahrestani/ibex/commit/ab12b99b38c917eb950433c2a581c74c4660d2d5) models this in a simple way using `simple_system`.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the failure with the experimental-branch-predictor simple-system build and the examples/sw/simple_system/mepc_test program. Then inspect ibex_if_stage.sv around instr_skid_valid_q, pc_if_o, and the corrected fetch_addr; done means the test loops indefinitely and does not save an incorrect MEPC.
Written by the indexing model from the issue text.
Assessment
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100