chipsalliance / VeeRwolf

FuseSoC-based SoC for VeeR EH1 and EL2
284 stars 64 forks source link

Is it OK to simulate under VCS? #4

Closed zhanjf closed 4 years ago

zhanjf commented 4 years ago

It is ok for verilator.

but if I want to use VCS, fusesoc run --target=sim --tool=vcs --run swervolf --ram_init_file=../cores/Cores-SweRVolf/sw/zephyr_hello.vh

It seems the zephyr did not load into risc-v.

olofk commented 4 years ago

In theory that should work, but unfortunately I don't have access to VCS myself. Will try to find some help debugging this

zhanjf commented 4 years ago

Thanks,I will try to do it myself.

saw235 commented 4 years ago

I tried to do it under VCS and it seems like it loads the hex file into memory but the simulation never outputs anything like it does in verilator. I don't have access to modelsim so I was unable to find anything else to compare the result with.

I've checked the ram contents, it seems that it was able to load the ram init file successfully but the o_uart_tx wire in swervolf_core_tb is not toggling like it does in Verilator.

Updates: I looked into it more and it seems like an issue with the instruction fetch unit being stuck in WAIT_FOR_MEMORY state.

olofk commented 4 years ago

@saw235 Thanks for looking into this. As SweRV itself runs fine under VCS I guess it's some issue with another part of the SoC. Can't help out much without VCS though :( It should run fine under Questasim

operoutka commented 4 years ago

Apparently VCS has problem with structure _axi_metat in axi_node_arbiter. It causes port connection width mismatch between stream_arbiter and stream_arbiter_flushable. Simulation runs fine when _axi_metat is not defined as a structure.

olofk commented 4 years ago

I just looked at the upstream axi_node core and apparently it has been deprecated in favor of https://github.com/pulp-platform/axi

I will need to investigate how difficult it will be to migrate to this interconnect instead. If you have a quick fix, I'm happy to apply that in the meantime

operoutka commented 4 years ago

There is the diff:


--- a/src/axi_node_arbiter.sv
+++ b/src/axi_node_arbiter.sv
@@ -27,17 +27,14 @@ module axi_node_arbiter #(
   input  logic                                oup_ready_i
 );

-  typedef struct packed {
-    logic [AUX_WIDTH-1:0] aux;
-    logic [ID_WIDTH-1:0]  id;
-  } axi_meta_t;
+  typedef logic [AUX_WIDTH+ID_WIDTH-1:0] axi_meta_t;

   axi_meta_t [N_MASTER-1:0] inp_meta;
   axi_meta_t oup_meta;

   for (genvar i = 0; i < N_MASTER; i++) begin: gen_inp_meta
-    assign inp_meta[i].aux = inp_aux_i[i];
-    assign inp_meta[i].id = inp_id_i[i];
+    assign inp_meta[i][AUX_WIDTH+ID_WIDTH-1:ID_WIDTH] = inp_aux_i[i];
+    assign inp_meta[i][ID_WIDTH-1:0] = inp_id_i[i];
   end

   stream_arbiter #(
@@ -54,8 +51,8 @@ module axi_node_arbiter #(
     .oup_ready_i  (oup_ready_i)
   );

-  assign oup_id_o = oup_meta.id;
-  assign oup_aux_o = oup_meta.aux;
+  assign oup_id_o = oup_meta[ID_WIDTH-1:0];
+  assign oup_aux_o = oup_meta[AUX_WIDTH+ID_WIDTH-1:ID_WIDTH];

 // pragma translate_off
 `ifndef VERILATOR
olofk commented 4 years ago

Great! Thanks. I'll just give it a test with Vivado, Verilator and Questasim to make sure those still work. Perhaps you could prepare a PR against https://github.com/olofk/axi_node/tree/fusesoc in the meantime?

operoutka commented 4 years ago

Sure, it is created now.

operoutka commented 4 years ago

OK, so I just noticed that the original issue was with zephyr_hello.vh init file and not for the default one.

For Zephyr to run you need the fix above + set uart decoder baud rate to 57870, then it runs ok for me.

olofk commented 4 years ago

Ah..! I see the problem. The testbench still runs on a 25MHz clock. Could you just quickly check if changing 20 to 10 here https://github.com/chipsalliance/Cores-SweRVolf/blob/master/tb/swervolf_core_tb.v#L45 will make it work without changing the baud rate. If so, I will push out that as a fix

operoutka commented 4 years ago

Yep, that works.

olofk commented 4 years ago

Fantastic! Pushed that now. I believe this means that we can mark the bug as solved (and I will make a new release shortly). Thank you everyone involved and please reopen if I missed anything