[Libre-soc-dev] [OpenPOWER-HDL-Cores] microwatt dcache potential bug (overlap r0 and r1)
paulus at ozlabs.org
Sat Jan 15 07:25:00 GMT 2022
On Fri, Jan 14, 2022 at 11:25:02PM +0000, Luke Kenneth Casson Leighton wrote:
> On Fri, Jan 14, 2022 at 10:26 PM lkcl <luke.leighton at gmail.com> wrote:
> > >The VHDL certainly is designed to be able to handle a store coming in
> > >while r1.full = 0 and the state machine is in RELOAD_WAIT_ACK state.
> > hmm then i need to look at that closely.
> arrrgh, i think i've got it.
> everything's "good"... the RELOAD_WAIT_ACKs completed, set
> r1.state == IDLE, no problem.
> this here tries to set r1.full = 1, again, looks like it's fine:
> -- Note that r1.full = 1 implies req_op = OP_NONE
> if req_op = OP_LOAD_MISS or req_op = OP_LOAD_NC or
> req_op = OP_STORE_MISS or req_op = OP_STORE_HIT then
> r1.req <= req;
> r1.full <= '1';
> end if;
> but... butbutbut... down below, bear in mind r1.state is currently
> RELOAD_WAIT_ACK, and bear in mind ld_stbs_done=true, this
> code *also* activates:
> if req.valid = '1' and req.same_tag = '1' and
> ((r1.dcbz = '1' and req.dcbz = '1') or
> (r1.dcbz = '0' and req.op = OP_LOAD_MISS)) and
> r1.store_row = get_row(req.real_addr) then
> r1.full <= '0';
> and it *overwrites* r1.full back to zero.
Ummm, it shouldn't, because the store should have req.op =
OP_STORE_HIT or OP_STORE_MISS, not OP_LOAD_MISS, so the condition of
the if should be false.
I see this at line 1561 of dcache.py:
(~r1.dcbz & (r1.req.op == Op.OP_LOAD_MISS))) &
Notice you have r1.req.op there whereas the VHDL has req.op. I think
that's your bug. (Similarly line 1560 has r1.req.dcbz not req.dcbz,
and line 1559 has r1.req.same_tag not req.same_tag.)
More information about the Libre-soc-dev