[Libre-soc-dev] microwatt dcache potential bug (overlap r0 and r1)

[Luke Kenneth Casson Leighton]

On Thu, Jan 13, 2022 at 10:49 PM Paul Mackerras <paulus at ozlabs.org> wrote:

> I guess the first question is, what git commit in the microwatt repo
> are you working from?

good point! currently f636bb7c3999d

>  I would suggest you use dcache.vhdl from
> current master, because with the pipelined loadstore1.vhdl, microwatt
> can and does do back-to-back loads and stores successfully.

something i'll need to write a unit test for.

> If you can't use dcache.vhdl from the head of the master branch, then
> have a look at commit f636bb7c3999 in particular and also
> f812832ad78d, 1a9834c506a8 and 0b23a5e76000.

ah one of the big(ish) ones i've avoided was the 4-way selection
from wb bus, 2 forwarding and the cache. have to work up to it.

> > i suspect that the occasional LD corruption is down to r1.full
> > being set to zero, a batch of ACKs still being expected, but
> > a *new* r0 LD operation comes in, gets transferred to r1 *whilst
> > there are still ACKs pending from the previous LD*.
>
> The new ld would get into r1.req but shouldn't affect r1.wb until all
> the acks for the previous operation have come in.

a trace i'm currently looking at, STORE_WAIT_ACK ends early
due to "stbs_done and acks=1", which sets r1.state = IDLE
and also sets r1.dec_acks = 1.

            if wishbone_in.ack = '1' then
                        if stbs_done and acks = 1 then
                            r1.state <= IDLE;
                            r1.wb.cyc <= '0';
                            r1.wb.stb <= '0';
                        end if;
                        r1.dec_acks <= '1';

HOWEVER... on the next cycle (remember that
r1.dec_acks is set to 1...)

* the case r1.state=IDLE kicks in (because we just requested that,
   line 1382)
* line 1431, OP_STORE_MISS kicks us back to STORE_WAIT_ACK
  in the next cycle

BUT...

r1.dec_acks was, during that time, set to ZERO (line 1312).

thus, the number of acks gets out of sync by one, because of
this missed (tiny) window.

now, the question is: does this actually matter?  i can see some of
the state information in r1 being overwritten (r1.acks_pending=1,
r1.state= STORE_WAIT_ACK/RELOAD_WAIT_ACK etc), which
in theory should be ok.

it is however quite confusing, when looking at signal traces,
to find that r1.req.op stays in OP_STORE_MISS, dec_acks remains
at 1, acks_pending remains at 1.

l.