chipsalliance / rocket-chip

Rocket Chip Generator
Other
3.28k stars 1.14k forks source link

4496e8d Failing Uncore Assertion #17

Closed seldridge closed 9 years ago

seldridge commented 9 years ago

I just bumped my rocket-chip version and started getting a broadcast.scala (from uncore) assertion failure when running the C++ emulator. I bisected and it's originating in 4496e8d4e2bbd9eb9af4ecd5687a2a9676957073.

For a basic hello world C program, when running the C++ emulator on 56ecdff52d5b0097bc6be54a7ec22bf1c79212b5 (one behind 4496e8d4e2bbd9eb9af4ecd5687a2a9676957073), I see:

$ ./emulator-Top-DefaultCPPConfig pk hello.rv
hello

When switching to 4496e8d, I get:

$ ./emulator-Top-DefaultCPPConfig pk hello.rv
Assertion failed: AcquireTracker initialized with a tail data beat

I'm using the current rocket-chip submodule references for riscv-tools.

seldridge commented 9 years ago

Narrowed it down a little bit. The problem is that the default mem_mb method in htif.cc (riscv-fesvr) used to return 1024. The child class htif_emulator_t in htif_emulator.h (rocket-chip) now returns 4096, the MEM_SIZE set in mm.h (rocket-chip) for a host system that defines long as being > 4 bytes.

The following basic change gets everything working, but I doubt this is the right approach:

diff --git a/csrc/mm.h b/csrc/mm.h
index 8871870..30727a2 100644
--- a/csrc/mm.h
+++ b/csrc/mm.h
@@ -8,7 +8,7 @@
 #include <queue>

 const int LINE_SIZE = 64; // all cores assume this.
-const size_t MEM_SIZE = (sizeof(long) > 4 ? 4L : 1L) * 1024*1024*1024;
+const size_t MEM_SIZE = 1L * 1024*1024*1024;

 class mm_t
 {

I'll poke around in riscv-fesvr a little bit more to see if there's an equivalent of MEM_SIZE that can be set and hopefully clear this more intelligently.

zhemao commented 9 years ago

Sorry, this is a problem that I introduced with the recent memory-mapped IO changes. The generator by default allocates 1 GB of physical address spaced to memory, so having mem_mb set to > 1024 will break things. If you want pk to work without having to modify code, you can set the memory size on the command line by adding "+memsize=1024" to the command line args for emulator. When I was testing pk, bbl, and vmlinux in the emulator, I was always setting the memsize on the command-line, so I forgot about this problem.

I think your fix is the correct one, since the CPU can't address any memory not in its memory map anyway, so allocating the extra memory space is pointless.