cores
/
microwatt
mirror of https://github.com/antonblanchard/microwatt
3
1
Fork 1
Code Issues Projects Releases Wiki Activity

soc: Fix issues with 64-bit stores to IO bridge

Browse Source 
The IO bridge would latch the top half of write data and selection signals
when issuing the second downstream store. Unfortunately at this point the
bridge has already "accepted" the upstream store from the core (due to
stall being 0 on the cycle when stb/cyc are 1), so the values on the
wishbone signals aren't stable and might already reflect a subsequent
wishbone command.

This causes occasional data corruption of 64-bit stores through the IO
bridge.

While at it, take out a bunch of useless conditions on the data latch
path. It doesn't matter whether we is 0 or 1, we can just always latch
the data, the destination will decide whether to use the content or not,
which should save a bit of hardware.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
pull/413/head
Benjamin Herrenschmidt 2 years ago
parent 413f2dc5d6
commit 3f788e87dc
1 changed files with 33 additions and 22 deletions
Show Stats Download Patch File Download Diff File

55
soc.vhdl
Unescape Escape View File

@ -476,6 +476,8 @@ begin
variable end_cyc : std_ulogic; variable end_cyc : std_ulogic;
variable slave_io : slave_io_type; variable slave_io : slave_io_type;
variable match : std_ulogic_vector(31 downto 12); variable match : std_ulogic_vector(31 downto 12);
variable dat_latch : std_ulogic_vector(31 downto 0);
variable sel_latch : std_ulogic_vector(3 downto 0);
begin begin
if rising_edge(system_clk) then if rising_edge(system_clk) then
do_cyc := '0'; do_cyc := '0';
@ -488,6 +490,8 @@ begin
end_cyc := '1'; end_cyc := '1';
has_top := false; has_top := false;
has_bot := false; has_bot := false;
dat_latch := (others => '0');
sel_latch := (others => '0');
else else
case state is case state is
when IDLE => when IDLE =>
@ -497,7 +501,11 @@ begin
-- Do we have a cycle ? -- Do we have a cycle ?
if wb_io_in.cyc = '1' and wb_io_in.stb = '1' then if wb_io_in.cyc = '1' and wb_io_in.stb = '1' then
-- Stall master until we are done, we are't (yet) pipelining -- Stall master until we are done, we are't (yet) pipelining
-- this, it's all slow IOs. -- this, it's all slow IOs. Note: The current cycle has
-- already been accepted as "stall" was 0, this only blocks
-- the next one. This means that we must latch
-- everything we need from wb_io_in in *this* cycle.
--
wb_io_out.stall <= '1'; wb_io_out.stall <= '1';


-- Start cycle downstream -- Start cycle downstream
@ -512,21 +520,22 @@ begin
has_top := wb_io_in.sel(7 downto 4) /= "0000"; has_top := wb_io_in.sel(7 downto 4) /= "0000";
has_bot := wb_io_in.sel(3 downto 0) /= "0000"; has_bot := wb_io_in.sel(3 downto 0) /= "0000";


-- Remember the top word as it might be needed later
dat_latch := wb_io_in.dat(63 downto 32);
sel_latch := wb_io_in.sel(7 downto 4);

-- If we have a bottom word, handle it first, otherwise -- If we have a bottom word, handle it first, otherwise
-- send the top word down. XXX Split the actual mux out -- send the top word down.
-- and only generate a control signal.
if has_bot then if has_bot then
if wb_io_in.we = '1' then -- Always update out.dat, it doesn't matter if we
wb_sio_out.dat <= wb_io_in.dat(31 downto 0); -- update it on reads and it saves mux
end if; wb_sio_out.dat <= wb_io_in.dat(31 downto 0);
wb_sio_out.sel <= wb_io_in.sel(3 downto 0); wb_sio_out.sel <= wb_io_in.sel(3 downto 0);


-- Wait for ack -- Wait for ack
state := WAIT_ACK_BOT; state := WAIT_ACK_BOT;
else else
if wb_io_in.we = '1' then wb_sio_out.dat <= wb_io_in.dat(63 downto 32);
wb_sio_out.dat <= wb_io_in.dat(63 downto 32);
end if;
wb_sio_out.sel <= wb_io_in.sel(7 downto 4); wb_sio_out.sel <= wb_io_in.sel(7 downto 4);


-- Bump address -- Bump address
@ -544,18 +553,14 @@ begin


-- Handle ack -- Handle ack
if wb_sio_in.ack = '1' then if wb_sio_in.ack = '1' then
-- If it's a read, latch the data -- Always latch the data, it doesn't matter if it was
if wb_sio_out.we = '0' then -- a write and it saves a mux
wb_io_out.dat(31 downto 0) <= wb_sio_in.dat; wb_io_out.dat(31 downto 0) <= wb_sio_in.dat;
end if;


-- Do we have a "top" part as well ? -- Do we have a "top" part as well ?
if has_top then if has_top then
-- Latch data & sel wb_sio_out.dat <= dat_latch;
if wb_io_in.we = '1' then wb_sio_out.sel <= sel_latch;
wb_sio_out.dat <= wb_io_in.dat(63 downto 32);
end if;
wb_sio_out.sel <= wb_io_in.sel(7 downto 4);


-- Bump address and set STB -- Bump address and set STB
wb_sio_out.adr(0) <= '1'; wb_sio_out.adr(0) <= '1';
@ -583,10 +588,9 @@ begin


-- Handle ack -- Handle ack
if wb_sio_in.ack = '1' then if wb_sio_in.ack = '1' then
-- If it's a read, latch the data -- Always latch the data, it doesn't matter if it was
if wb_sio_out.we = '0' then -- a write and it saves a mux
wb_io_out.dat(63 downto 32) <= wb_sio_in.dat; wb_io_out.dat(63 downto 32) <= wb_sio_in.dat;
end if;


-- We are done, ack up, clear cyc downstram -- We are done, ack up, clear cyc downstram
end_cyc := '1'; end_cyc := '1';
@ -603,6 +607,13 @@ begin


-- Create individual registered cycle signals for the wishbones -- Create individual registered cycle signals for the wishbones
-- going to the various peripherals -- going to the various peripherals
--
-- Note: This needs to happen on the cycle matching state = IDLE,
-- as wb_io_in content can only be relied upon on that one cycle.
-- This works here because do_cyc is a variable, not a signal, and
-- thus here we observe the value set above in the state machine
-- on the same cycle rather than the next one.
--
if do_cyc = '1' or end_cyc = '1' then if do_cyc = '1' or end_cyc = '1' then
io_cycle_none <= '0'; io_cycle_none <= '0';
io_cycle_syscon <= '0'; io_cycle_syscon <= '0';

Write Preview
Loading…
Cancel
Save