IObundle / iob-cache

Verilog Configurable Cache
MIT License
167 stars 32 forks source link

'x' data read from cache #176

Closed retrhelo closed 2 years ago

retrhelo commented 2 years ago

I'm trying to connect iob_cache directly to a picorv32 core. But I noticed that, when reading from certain address, the rdata turns to be 'x'. It happens when the addr[3:0] == 4'b0000.

image

I create the iob_cache instance as follow:

iob_cache #(
  .N_WAYS(2),
  .LINE_OFF_W(7),
  .WORD_OFF_W(4),
  .WTBUF_DEPTH_W(5),
  .CTRL_CACHE(0),
  .CTRL_CNT(0)
) cache_inst (
  .clk(clk),
  .reset(!resetn),
  // Front-end interface
  .valid           ( pico_valid               ),
  .addr            ( pico_addr[31:2]          ),
  .wdata           ( pico_wdata               ),
  .wstrb           ( pico_wstrb               ),
  .rdata           ( pico_rdata               ),
  .ready           ( pico_ready               ),
  // Ctrl signals
  .force_inv_in    ( 1'b0                     ),
  .force_inv_out   ( /* not used */           ),
  .wtb_empty_in    ( 1'b1                     ),
  .wtb_empty_out   ( /* not used */           ),
  // Back-end interface
  .mem_valid       ( mem_valid                ),
  .mem_addr        ( mem_addr                 ),
  .mem_wdata       ( mem_wdata                ),
  .mem_wstrb       ( mem_wstrb                ),
  .mem_rdata       ( mem_rdata                ),
  .mem_ready       ( mem_ready                )
);

I wonder if there're some rules to follow when using iob_cache? The problem seems to have something to do with cache_memory module, but I'm not sure what exactly causes the x data.

retrhelo commented 2 years ago

As we can see from the waves, the appearance of 'x' data follows the rule that addr[3:0] == 4'b0000, and eventually leads to a stuck of picorv32.

image

jjts commented 2 years ago

Cache is connected to picorv32 in IObundle/iob-soc. Have you taken a look? I think the problem is the missing external memory model.

retrhelo commented 2 years ago

Have you taken a look?

Sure I have. But my memory model is a bit different from that. I connect iob_cache directly between the original testbench used by picorv32. The original structure looks like picorv32 - axi_adapter - axi_memory, and I make it picorv32 - iob_cache - axi_adapter - axi_memory.

I think the problem is the missing external memory model.

I don't think that the memory model is the key to problem, since picorv32 works fine with it. Since iob_cache is using the same native memory interface, it should also work fine.

P-Miranda commented 2 years ago

If you just want a single cache level with picorv interface on the CPU side and AXI interface on the memory side, you can try to use the iob-cache-axi.v module instead of iob_cache.v. Therefore you would have: picorv32 - iob-cache-axi - axi_memory (with iob-cache-axi you don't need axi-adapter anymore)

retrhelo commented 2 years ago

If you just want a single cache level with picorv interface on the CPU side and AXI interface on the memory side, you can try to use the iob-cache-axi.v module instead of iob_cache.v.

I'd prefer iob_cache.v, as it uses the picorv32's native memory interface. Thus it's easier to further implement mux/demux logic. And the axi_memory is using AXI Lite instead of AXI4 used by iob-cache-axi. I have to write some axi2axil logic first if I want to use it.

jjts commented 2 years ago

Our native interface, despite the signals having the same names as the picorv32 native interface, is perhaps semantically different.

Valid is a request and ready is in fact an acknowledge given to the request. Signal ready only has meaning if given 1 cycle or more after ready. Valid should remain high until ready is 0 and go low immediately after ready goes high. If Valid remains high after ready is high this is interpreted as a new request for the given address.

If your memory behaves as described you should be fine. Otherwise it will not work.

On Fri, May 13, 2022, 11:30 Artyom Liu @.***> wrote:

If you just want a single cache level with picorv interface on the CPU side and AXI interface on the memory side, you can try to use the iob-cache-axi.v module instead of iob_cache.v.

I'd prefer iob_cache.v, as it uses the picorv32's native memory interface. Thus it's easier to furhter implement mux/demux logic. And the axi_memory is using AXI Lite instead AXI4 used by iob-cache-axi. I have to write some axi2axil logic first if I want to use it.

— Reply to this email directly, view it on GitHub https://github.com/IObundle/iob-cache/issues/176#issuecomment-1125897033, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLUHOYMSU2ISEBIKTZ3VH3VJYVM7ANCNFSM5V2MQ4XQ . You are receiving this because you commented.Message ID: @.***>

retrhelo commented 2 years ago

Valid is a request and ready is in fact an acknowledge given to the request. Signal ready only has meaning if given 1 cycle or more after ready. Valid should remain high until ready is 0 and go low immediately after ready goes high. If Valid remains high after ready is high this is interpreted as a new request for the given address.

Well, I'm actually not very sure if picorv32's bus behaves this way... It seems that I mess these two different buses up, since they share the same name. It's really misleading though, in my opinion.

Anyway, I believe we've just found the answer to all the mess. I really appreciate all the kind replies, thank you very much. :D

jjts commented 2 years ago

Picorv32 bus does not work like this because it doesn't support pipeline reads and writes. Otherwise it would need something similar. It is unfortunate indeed that the two buses use the same names...

On Fri, May 13, 2022, 12:56 Artyom Liu @.***> wrote:

Closed #176 https://github.com/IObundle/iob-cache/issues/176.

— Reply to this email directly, view it on GitHub https://github.com/IObundle/iob-cache/issues/176#event-6605131646, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLUHO3JGC5UECWJHY3FFELVJY7PPANCNFSM5V2MQ4XQ . You are receiving this because you commented.Message ID: @.***>

jjts commented 2 years ago

Picorv32 takes 4 to 5 cycles for each read or write.

On Fri, May 13, 2022, 13:31 Jose T. de Sousa @.***> wrote:

Picorv32 bus does not work like this because it doesn't support pipeline reads and writes. Otherwise it would need something similar. It is unfortunate indeed that the two buses use the same names...

On Fri, May 13, 2022, 12:56 Artyom Liu @.***> wrote:

Closed #176 https://github.com/IObundle/iob-cache/issues/176.

— Reply to this email directly, view it on GitHub https://github.com/IObundle/iob-cache/issues/176#event-6605131646, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLUHO3JGC5UECWJHY3FFELVJY7PPANCNFSM5V2MQ4XQ . You are receiving this because you commented.Message ID: @.***>

jjts commented 2 years ago

I think if you use our iob-picorv32 wrapper uou can solve the problem, as it makes the 2 interfaces compatible.

On Fri, May 13, 2022, 13:32 Jose T. de Sousa @.***> wrote:

Picorv32 takes 4 to 5 cycles for each read or write.

On Fri, May 13, 2022, 13:31 Jose T. de Sousa @.***> wrote:

Picorv32 bus does not work like this because it doesn't support pipeline reads and writes. Otherwise it would need something similar. It is unfortunate indeed that the two buses use the same names...

On Fri, May 13, 2022, 12:56 Artyom Liu @.***> wrote:

Closed #176 https://github.com/IObundle/iob-cache/issues/176.

— Reply to this email directly, view it on GitHub https://github.com/IObundle/iob-cache/issues/176#event-6605131646, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLUHO3JGC5UECWJHY3FFELVJY7PPANCNFSM5V2MQ4XQ . You are receiving this because you commented.Message ID: @.***>

retrhelo commented 2 years ago

I think if you use our iob-picorv32 wrapper uou can solve the problem, as it makes the 2 interfaces compatible.

Yes, I'm currently working on it. But by doing so I have to find/modify a memory that supports AXI4, while supports some MMIO UART-like output for simulation purpose, like axi_memory in picorv32 project does. I guess it's just how life is, where people can't always find the perfect solution to the need lol.