chipsalliance / dromajo

RISC-V RV64GC emulator designed for RTL co-simulation
Apache License 2.0
210 stars 63 forks source link

dromajo lr/sc does not fully respect the riscv manual ? #64

Closed laurentiuduca closed 1 year ago

laurentiuduca commented 1 year ago
dromajo lr/sc does not fully respect the riscv manual ?
====
The SC must fail if a store to the reservation set from another hart can be observed to
occur between the LR and SC.

write printf to include/dromajo_template.h
====
#define OP_A(size)                                                                      \
..            case 2: /* lr.w */                                                          \
                printf("lr.w: addr=%x\n", addr);                                        \
..            case 3: /* sc.w */                                                          \
..              if (s->load_res == addr) {                                              \
..                  printf("sc.w success memval=%x hartid=%x\n",                        \
                                    read_reg(rs2), s->mhartid);                         \
                    val         = 0;                                                    \
                    s->load_res = ~0;                                                   \
                } else {                                                                \
                    printf("sc.w failed hartid=%x\n", s->mhartid);                      \
                    val = 1;                                                            \
                }                                                                       \

crt.s from dromajo/run vs laur.s
====
$ diff crt.S laur.S 
127,129c127,129
<   # for now, assume only 1 core
<   li a1, 1
< 1:bgeu a0, a1, 1b
---
>   # laur: run on all cores
> #  li a1, 1
> #1:bgeu a0, a1, 1b
139c139,148
<   j _init
---
> #  j _init
> 
>   # laur
>   # have each core add 1 to tohost
>   la a1, tohost
> 1: lr.w a4, (a1)
> addi a4, a4, 1
> sc.w a4, a4, (a1)
> bnez a4, 1b
> label1:j label1

compile and run
====
laur@laurPC-100:~/lucru/cn/riscv/64/dromajo/run$ riscv64-unknown-elf-gcc -march=rv64g -mabi=lp64 -static -mcmodel=medany -nostdlib -nostartfiles laur.S -lgcc -T test.ld
laur@laurPC-100:~/lucru/cn/riscv/64/dromajo/run$ ../build/dromajo --ncpus=4 ./a.out
lr.w: addr=80001000
lr.w: addr=80001000
lr.w: addr=80001000
lr.w: addr=80001000
sc.w success memval=1 hartid=0
sc.w success memval=1 hartid=1
sc.w success memval=1 hartid=2
sc.w success memval=1 hartid=3
Simulation speed:  0.29 MIPS (single-core)
Power off.
et-tommythorn commented 1 year ago

Thanks for the issue. You are right, this is broken. Unfortunately fixing this requires a bit more infrastructure for correct coherency between cores. (FWIW, atomics are ok with this, but LR/SC needs more help).

We'll look at this soon.

et-tommythorn commented 1 year ago

Sorry it took a while to get around to fixing this, but now (with slightly modified debug output):

../build/dromajo --ctrlc --ncpus=4 laur
0: lr.w: addr=80001000, S#0
1: lr.w: addr=80001000, S#0
2: lr.w: addr=80001000, S#0
3: lr.w: addr=80001000, S#0
0: sc.w success memval=1 S#0
1: sc.w failed memval=1 S#0 != S#1
2: sc.w failed memval=1 S#0 != S#1
3: sc.w failed memval=1 S#0 != S#1
Simulation speed:  0.47 MIPS (single-core)

Or

../build/dromajo --ctrlc --ncpus=4 laur
0: lr.w: addr=80001000, S#0
0: sc.w success memval=1 S#0
1: lr.w: addr=80001000, S#1
1: sc.w success memval=2 S#1
2: lr.w: addr=80001000, S#2
2: sc.w success memval=3 S#2
3: lr.w: addr=80001000, S#3
3: sc.w success memval=4 S#3
Simulation speed: 12.48 MIPS (single-core)

in the default configuration that doesn't run cores in lock-step (for better performance). Review request forth coming.