Simulated external ram C doesn't work in post implementation timing simulation under Vivado 2021.2

[H1alus]

I implemented the external ram c from the testbench.simple.vhd example in to my testbench as I intended to load the pixel values from an image in to it without having to have a much more complicated design and also since i wanted to exclude the external ram from the overall power consumption, I dont have a simple way to search for the exception since using UART after implementation would take a day for the simulation, is there anything I should do besides what's done in the testbench in order to have it working in post implementation? Or should I just have the external ram as part of the design sources?

Note: The behavioural simulation runs as expected.

[H1alus]

xbus_cyc_o is not synced with stb, this makes the post implementation simulation fail.

Looking at the code from the xbus it seems that xbus_stb_o as well as xbus_cyc_o are not pipelined.

[stnolting]

That's right, by default the cyc and stb signals are not driven directly by flip flops. You can enable an additional register stage in the XBUS module to make them sync:

neorv32/rtl/core/neorv32_top.vhd

Line 95 in 636cd6e

XBUS_REGSTAGE_EN : boolean := false; -- add XBUS register stage

[H1alus]

Damn, I totally overlooked that. Thanks

[H1alus]

Actually, the generic was already there set to true, so the waveform i sent previously refers to the pipelined signals

[stnolting]

the problem lies in the signals stb and cyc that are not synced at the rising edge of clock

But if you enable the XBUS register stage then they should be sync as all outgoing XBUS signals are fully registered:

neorv32/rtl/core/neorv32_xbus.vhd

Lines 66 to 83 in 636cd6e

	reg_stage_enable:
	if REGSTAGE_EN generate
	reg_stage: process(rstn_i, clk_i)
	begin
	if (rstn_i = '0') then
	bus_req <= req_terminate_c;
	bus_rsp_o <= rsp_terminate_c;
	elsif rising_edge(clk_i) then
	-- request --
	if (bus_req_i.stb = '1') then -- keep all signals stable ...
	bus_req <= bus_req_i;
	end if;
	bus_req.stb <= bus_req_i.stb; -- ... except for STB that is single-shot
	-- response --
	bus_rsp_o <= bus_rsp;
	end if;
	end process reg_stage;
	end generate;

I came up with a version of the RAM presented in testbench.simple.vhd were ack is first latched and then parsed through the queue:

Those TB memories were designed to simulate different access latencies. Do you really need this for your setup? If not, I recommend to use a different memory code, e.g. the one from the RISCOF testbench (which can also be initialized from a file if that is relevant for you):

https://github.com/stnolting/neorv32-riscof/blob/b2e518d1d8cd8eb061695d02948a1d2bbefb267c/sim/neorv32_riscof_tb.vhd#L195-L223

[H1alus]

There is a problem with the xbus actual output signals, bus_req is still an internal signal, so the regstate doesn't affect the actual module outputs for what I see:

  -- Bus Access -----------------------------------------------------------------------------
  -- -------------------------------------------------------------------------------------------
  xbus_adr_o <= bus_req.addr;
  xbus_dat_o <= bus_req.data;
  xbus_we_o  <= bus_req.rw;
  xbus_sel_o <= bus_req.ben;
  xbus_stb_o <= bus_req.stb;
  xbus_cyc_o <= bus_req.stb or pending;
  xbus_tag_o <= bus_req.src & '0' & bus_req.priv; -- instr/data, secure, privileged/unprivileged

the cyc_o comes from an OR gate that is not parsed through an actual register.
This waveform is from a post implementation simulation that ended 10 minutes before; the signals you see are from the actual TB global scope:

As you see the simulation accounts for the or delay in the clock cycle, the ack is working after the change I made to the memory. The behavioural simulation in vivado doesn't account for routing and overall slice submodules delays.

XBUS_REGSTAGE_EN => true in neorv32_top instance of my custom top module.

If there's nothing that compromises the simulation I'd keep the memory from the simple testbench with my change, at least for now, do you think it should still be ok? If not I'll work on changing it to the RISCOF testbench.

[H1alus]

Nah I have to check this again

[H1alus]

Ok I dont really get what's going on, I ONLY changed the XBUS_REGSTAGE_EN to false and re-run the implementation + simulation, I don't understand how I've got those waveforms the first time...

[stnolting]

Anyway, the main thing is that it works now! 👍

[H1alus]

A question about riscof template (I think this could be the right place for this). The file doesn't reference the wishbone error signal, is it set to 0 somewhere in the neorv32 design? If shouldn't the template handle the error? or at least set it to zero? I experienced unknown wishbone cyc signal value in post implementation as a result of err left undefined.

[H1alus]

Yeah I have to try and replicate that using the RISCOF RAM without the changes I made

[H1alus]

Ok, so no there is no timing violation during simulation, the only thing is that the ram doesn't respond to the request since "stb and cyc" condition is never met at rising edge due to their latency

[H1alus]

So, I took inspiration from you pointing to the synthetizer setting IO buffers for the top entity signals.
By adding the IOB attribute to the top entity signals of xbus I was able to fix the issue of the emulated RAM with 1 clock cycle delay (being it RISCOF or the other template from the testbench.simple)

There is still much delay but the read is fine now (unlike previously).

entity neorv32_mine is
  port (
    -- Global control --
    clk_i  : in  std_logic;
    rstn_i : in  std_logic;
    
    -- External bus interface (available if XBUS_EN = true) --
    xbus_adr_o     : out std_ulogic_vector(31 downto 0);                    -- address
    xbus_dat_o     : out std_ulogic_vector(31 downto 0);                    -- write data
    xbus_tag_o     : out std_ulogic_vector(2 downto 0);                     -- access tag
    xbus_we_o      : out std_ulogic;                                        -- read/write
    xbus_sel_o     : out std_ulogic_vector(3 downto 0);                     -- byte enable
    xbus_stb_o     : out std_ulogic;                                        -- strobe
    xbus_cyc_o     : out std_ulogic;                                        -- valid cycle
    xbus_dat_i     : in  std_ulogic_vector(31 downto 0) := (others => 'L'); -- read data
    xbus_ack_i     : in  std_ulogic := 'L';                                 -- transfer acknowledge
    xbus_err_i     : in  std_ulogic := 'L';                                 -- transfer error
    
    -- primary UART0 (available if IO_UART0_EN = true) --
    uart0_txd_o    : out std_ulogic;                                        -- UART0 send data
    uart0_rxd_i    : in  std_ulogic;                                 -- UART0 receive data
    uart0_rts_o    : out std_ulogic;                                        -- HW flow control: UART0.RX ready to receive ("RTR"), low-active, optional
    uart0_cts_i    : in  std_ulogic;                                 -- HW flow control: UART0.TX allowed to transmit, low-active, optional
    
    -- CPU interrupts (for chip-internal usage only) --
    mtime_irq_i    : in  std_ulogic := 'L';                                 -- machine timer interrupt, available if IO_MTIME_EN = false
    msw_irq_i      : in  std_ulogic := 'L';                                 -- machine software interrupt
    mext_irq_i     : in  std_ulogic := 'L'                                  -- machine external interrupt
  );
    attribute IOB : string; 
    attribute IOB of xbus_adr_o          : signal  is "TRUE";
    attribute IOB of xbus_dat_o          : signal  is "TRUE";
    attribute IOB of xbus_tag_o          : signal  is "TRUE";
    attribute IOB of xbus_we_o           : signal  is "TRUE";
    attribute IOB of xbus_sel_o          : signal  is "TRUE";
    attribute IOB of xbus_stb_o          : signal  is "TRUE";
    attribute IOB of xbus_cyc_o          : signal  is "TRUE";
    attribute IOB of xbus_dat_i          : signal  is "TRUE";
    attribute IOB of xbus_ack_i          : signal  is "TRUE";
    attribute IOB of xbus_err_i          : signal  is "TRUE";
end entity;

[H1alus]

Still, why does the cyc signal stay high after the read?

[stnolting]

Please excuse me, but I have completely lost sight of this post... 🙈
Is the problem still relevant?

[H1alus]

Please don't excuse yourself, I can't expect more from your work. Personally no, I think the problem might be vivado at this point, different versions provide very very different waveforms, a friend of mine working in Vivado as a day job told me post-implementation simulation is bugged under any version

[stnolting]

post-implementation simulation is bugged under any version

I already thought so... 😅

[H1alus]

Yeah but I didn't know better. A tool with milions of dollars behind should work as expected😅