eb_slave: fix pass fifo overflow bug

It is indeed possible for the pass FIFO to have more than the tag FIFO. If the TX mux is blocked after popping the tag, but before popping the pass FIFO, the pass FIFO can have one more element. Thus, if you have all of the pending tags landing in the same FIFO (pass, wbm, cfg), you can end up with the situation where the tag FIFO is indeed less full and thus you refill the tag FIFO while overflowing the other FIFOs. The result of this bug is that when writing to slow slaves, a very homogeneous access pattern can end up with a corrupted word if you are unlucky. The worst part of this bug is that when I wrote that code, I was uncertain if this was safe and marked it to be reviewed. It never got reviewed. Doh.

eb_slave: fix pass fifo overflow bug
It is indeed possible for the pass FIFO to have more than the tag FIFO. If the TX mux is blocked after popping the tag, but before popping the pass FIFO, the pass FIFO can have one more element. Thus, if you have all of the pending tags landing in the same FIFO (pass, wbm, cfg), you can end up with the situation where the tag FIFO is indeed less full and thus you refill the tag FIFO while overflowing the other FIFOs. The result of this bug is that when writing to slow slaves, a very homogeneous access pattern can end up with a corrupted word if you are unlucky. The worst part of this bug is that when I wrote that code, I was uncertain if this was safe and marked it to be reviewed. It never got reviewed. Doh.
1274ae82 · Wesley W. Terpstra · e9202a2e · 1274ae82
Commit 1274ae82 authored Apr 09, 2015 by Wesley W. Terpstra
Hide whitespace changes
Inline Side-by-side

Showing with 2 additions and 4 deletions

eb_slave_fsm.vhd hdl/eb_slave_core/eb_slave_fsm.vhd +2 -4

No files found.
--- a/hdl/eb_slave_core/eb_slave_fsm.vhd
+++ b/hdl/eb_slave_core/eb_slave_fsm.vhd
@@ -108,12 +108,10 @@ begin
  master_o.sel <= r_rx_rec_hdr.sel(master_o.sel'range);
  
  -- Stall the RX path if:
-  --   Any TX FIFO is full (probably only tag matters)
+  --   Any TX FIFO is full
  --   We are pushing a strobe that is stalled
  --   We are waiting to lower the cycle line
-  -- 
-  -- !!! could be improved to allow pipeline progress until stb/cyc need to be raised again
-  s_stall <= tag_full_i OR -- pass_full_i OR cfg_full_i OR wbm_full_i OR 
+  s_stall <= tag_full_i OR pass_full_i OR cfg_full_i OR wbm_full_i OR 
             (r_master_stb_o and master_stall_i) OR
             (not r_master_cyc_o and s_wbm_busy);