From 7c0c48fac4114406fab20f4ed18a13558bf53cfc Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Fri, 26 Feb 2016 01:29:38 -0800 Subject: [PATCH 1/4] Resurrect the backup memory port We need this to work for our chip, and it's not been tested in a long time in upstream -- it didn't even used to build since the Nasti conversion. This makes a few changes: * Rather than calling the backup memory port parameters MEM_*, it calls them MIF_* (to match the MIT* paramater objects). A new name was necessary because the Nasti stuff is now dumped as MEM_*, which has similar names but incompatible values. * p(MIFDataBits) was changed back to 128, as otherwise the backup memory port doesn't work (it only send half a TileLink transaction). 64 also causes readmemh to bail out, but changing the elf2hex parameters works around that. * A configuration was added that enabled the backup memory port in the tester. While this is kind of an awkward way to do it, I want to make sure I can start testing this regularly and this makes it easy to integrate. --- src/main/scala/Configs.scala | 14 +++++++++----- vsim/Makefrag | 2 -- vsrc/backup_mem.v | 16 ++++++++-------- vsrc/rocketTestHarness.v | 14 +++++++------- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/main/scala/Configs.scala b/src/main/scala/Configs.scala index fd8fe032..161708fa 100644 --- a/src/main/scala/Configs.scala +++ b/src/main/scala/Configs.scala @@ -87,13 +87,15 @@ class DefaultConfig extends Config ( case PPNBits => site(PAddrBits) - site(PgIdxBits) case VAddrBits => site(VPNBits) + site(PgIdxBits) case ASIdBits => 7 - case MIFTagBits => // Bits needed at the L2 agent + case MIFTagBits => Dump("MIF_TAG_BITS", + // Bits needed at the L2 agent log2Up(site(NAcquireTransactors)+2) + // Bits added by NASTI interconnect max(log2Up(site(MaxBanksPerMemoryChannel)), - (if (site(UseDma)) 3 else 2)) - case MIFDataBits => 64 - case MIFAddrBits => site(PAddrBits) - site(CacheBlockOffsetBits) + (if (site(UseDma)) 3 else 2))) + case MIFDataBits => Dump("MIF_DATA_BITS", 128) + case MIFAddrBits => Dump("MIF_ADDR_BITS", + site(PAddrBits) - site(CacheBlockOffsetBits)) case MIFDataBeats => site(CacheBlockBytes) * 8 / site(MIFDataBits) case NastiKey => { Dump("MEM_STRB_BITS", site(MIFDataBits) / 8) @@ -447,5 +449,7 @@ class WithOneOrMaxChannels extends Config( case MemoryChannelMuxConfigs => Dump("MEMORY_CHANNEL_MUX_CONFIGS", List(1, site(NMemoryChannels))) } ) - class OneOrEightChannelBenchmarkConfig extends Config(new WithOneOrMaxChannels ++ new With8MemoryChannels ++ new SingleChannelBenchmarkConfig) + +class SimulateBackupMemConfig extends Config(){ Dump("MEM_BACKUP_EN", true) } +class BackupMemVLSIConfig extends Config(new SimulateBackupMemConfig ++ new DefaultVLSIConfig) diff --git a/vsim/Makefrag b/vsim/Makefrag index 683bd131..a6e9e143 100644 --- a/vsim/Makefrag +++ b/vsim/Makefrag @@ -67,8 +67,6 @@ $(simv_debug) : $(sim_vsrcs) $(sim_csrcs) $(sim_dir)/libdramsim.a $(consts_heade $(VCS) $(VCS_OPTS) -o $(simv_debug) \ +define+DEBUG -debug_pp \ -# +define+MEM_BACKUP_EN \ - #-------------------------------------------------------------------- # Run #-------------------------------------------------------------------- diff --git a/vsrc/backup_mem.v b/vsrc/backup_mem.v index ad6d037d..584f2db1 100644 --- a/vsrc/backup_mem.v +++ b/vsrc/backup_mem.v @@ -42,27 +42,27 @@ module BackupMemory input mem_req_valid, output mem_req_ready, input mem_req_rw, - input [`MEM_ADDR_BITS-1:0] mem_req_addr, - input [`MEM_TAG_BITS-1:0] mem_req_tag, + input [`MIF_ADDR_BITS-1:0] mem_req_addr, + input [`MIF_TAG_BITS-1:0] mem_req_tag, input mem_req_data_valid, output mem_req_data_ready, - input [`MEM_DATA_BITS-1:0] mem_req_data_bits, + input [`MIF_DATA_BITS-1:0] mem_req_data_bits, output reg mem_resp_valid, - output reg [`MEM_DATA_BITS-1:0] mem_resp_data, - output reg [`MEM_TAG_BITS-1:0] mem_resp_tag + output reg [`MIF_DATA_BITS-1:0] mem_resp_data, + output reg [`MIF_TAG_BITS-1:0] mem_resp_tag ); localparam DATA_CYCLES = 4; localparam DEPTH = 2*1024*1024; reg [`ceilLog2(DATA_CYCLES)-1:0] cnt; - reg [`MEM_TAG_BITS-1:0] tag; + reg [`MIF_TAG_BITS-1:0] tag; reg state_busy, state_rw; - reg [`MEM_ADDR_BITS-1:0] addr; + reg [`MIF_ADDR_BITS-1:0] addr; - reg [`MEM_DATA_BITS-1:0] ram [DEPTH-1:0]; + reg [`MIF_DATA_BITS-1:0] ram [DEPTH-1:0]; wire [`ceilLog2(DEPTH)-1:0] ram_addr = state_busy ? {addr[`ceilLog2(DEPTH/DATA_CYCLES)-1:0], cnt} : {mem_req_addr[`ceilLog2(DEPTH/DATA_CYCLES)-1:0], cnt}; wire do_read = mem_req_valid && mem_req_ready && !mem_req_rw || state_busy && !state_rw; diff --git a/vsrc/rocketTestHarness.v b/vsrc/rocketTestHarness.v index 204304d7..a1727eda 100644 --- a/vsrc/rocketTestHarness.v +++ b/vsrc/rocketTestHarness.v @@ -88,12 +88,12 @@ module rocketTestHarness; end wire mem_bk_req_valid, mem_bk_req_rw, mem_bk_req_data_valid; - wire [`MEM_ID_BITS-1:0] mem_bk_req_tag; - wire [`MEM_ADDR_BITS-1:0] mem_bk_req_addr; - wire [`MEM_DATA_BITS-1:0] mem_bk_req_data_bits; + wire [`MIF_TAG_BITS-1:0] mem_bk_req_tag; + wire [`MIF_ADDR_BITS-1:0] mem_bk_req_addr; + wire [`MIF_DATA_BITS-1:0] mem_bk_req_data_bits; wire mem_bk_req_ready, mem_bk_req_data_ready, mem_bk_resp_valid; - wire [`MEM_ID_BITS-1:0] mem_bk_resp_tag; - wire [`MEM_DATA_BITS-1:0] mem_bk_resp_data; + wire [`MIF_TAG_BITS-1:0] mem_bk_resp_tag; + wire [`MIF_DATA_BITS-1:0] mem_bk_resp_data; `ifdef MEM_BACKUP_EN memdessertMemDessert dessert @@ -150,9 +150,9 @@ module rocketTestHarness; assign mem_in_bits = {`HTIF_WIDTH {1'b0}}; assign mem_bk_req_valid = 1'b0; assign mem_bk_req_ready = 1'b0; - assign mem_bk_req_addr = {`MEM_ADDR_BITS {1'b0}}; + assign mem_bk_req_addr = {`MIF_ADDR_BITS {1'b0}}; assign mem_bk_req_rw = 1'b0; - assign mem_bk_req_tag = {`MEM_ID_BITS {1'b0}}; + assign mem_bk_req_tag = {`MIF_TAG_BITS {1'b0}}; assign mem_bk_req_data_valid = 1'b0; assign mem_bk_req_data_bits = 16'd0; `endif From 7319f430d0ddee5eac8928d074720cdbb43ace41 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Sat, 27 Feb 2016 10:35:22 -0800 Subject: [PATCH 2/4] Fix the backup memory port on multiple-channel configs The backup memory port doesn't work on multi-channel configurations, it just screws up the Nasti tag bits. This patch always instantiates a single-channel backup memory port, which relies on the memory channel selector to only enable a single memory channel when the backup memory port is enabled. There are some assertions to make sure this happens, as otherwise memory gets silently corrupted. While this is a bit of a hack, the backup memory port will be going away soon so I don't want to spend a whole lot of time fixing it. The generated hardware is actually very similar: we used to elaborate a Nasti arbiter inside the backup memory support, now there's one outside of it instead. --- src/main/scala/RocketChip.scala | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/scala/RocketChip.scala b/src/main/scala/RocketChip.scala index 2dcda621..11ac09ce 100644 --- a/src/main/scala/RocketChip.scala +++ b/src/main/scala/RocketChip.scala @@ -334,6 +334,13 @@ class OuterMemorySystem(implicit val p: Parameters) extends Module with HasTopLe if(p(UseBackupMemoryPort)) { VLSIUtils.doOuterMemorySystemSerdes( mem_channels, io.mem, io.mem_backup, io.mem_backup_en, - nMemChannels, htifW, p(CacheBlockOffsetBits)) + 1, htifW, p(CacheBlockOffsetBits)) + for (i <- 1 until nMemChannels) { io.mem(i) <> mem_channels(i) } + assert(!Vec(mem_channels.map{ io => io.r.valid }).toBits.orR || + !io.mem_backup_en || + Vec(channelConfigs.map{i => UInt(i)})(io.memory_channel_mux_select) === UInt(1), + "Backup memory port only works when 1 memory channel is enabled") + Predef.assert(channelConfigs.sortWith(_ < _)(0) == 1, + "Backup memory port requires a single memory port mux config") } else { io.mem <> mem_channels } } From 9ea8c4e78162d91183399ee376ad63f1a49b75e5 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Fri, 26 Feb 2016 01:33:25 -0800 Subject: [PATCH 3/4] Add an 8-channel backup memory port config Now that the backup memory port works I want to test it. --- src/main/scala/Configs.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/scala/Configs.scala b/src/main/scala/Configs.scala index 161708fa..8a16cc44 100644 --- a/src/main/scala/Configs.scala +++ b/src/main/scala/Configs.scala @@ -444,12 +444,16 @@ class DualChannelBenchmarkConfig extends Config(new With2MemoryChannels ++ new S class QuadChannelBenchmarkConfig extends Config(new With4MemoryChannels ++ new SingleChannelBenchmarkConfig) class OctoChannelBenchmarkConfig extends Config(new With8MemoryChannels ++ new SingleChannelBenchmarkConfig) +class EightChannelVLSIConfig extends Config(new With8MemoryChannels ++ new DefaultVLSIConfig) + class WithOneOrMaxChannels extends Config( (pname, site, here) => pname match { case MemoryChannelMuxConfigs => Dump("MEMORY_CHANNEL_MUX_CONFIGS", List(1, site(NMemoryChannels))) } ) class OneOrEightChannelBenchmarkConfig extends Config(new WithOneOrMaxChannels ++ new With8MemoryChannels ++ new SingleChannelBenchmarkConfig) +class OneOrEightChannelVLSIConfig extends Config(new WithOneOrMaxChannels ++ new EightChannelVLSIConfig) class SimulateBackupMemConfig extends Config(){ Dump("MEM_BACKUP_EN", true) } class BackupMemVLSIConfig extends Config(new SimulateBackupMemConfig ++ new DefaultVLSIConfig) +class OneOrEightChannelBackupMemVLSIConfig extends Config(new WithOneOrMaxChannels ++ new With8MemoryChannels ++ new BackupMemVLSIConfig) From a0f3189c7452760c7dadb0d7fd1866a3311beef4 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Sat, 27 Feb 2016 11:41:28 -0800 Subject: [PATCH 4/4] Change MIF_DATA_BITS back to 64 It turns out the Chisel C++ backend can't emit correct initialization code for a 128 bit wide NastiROM. Rather than trying to fix Chisel, I'm just going to hack up the backup memory port Verilog harness a bit more to make it work. Note that the backup memory port Verilog already couldn't take arbitrary parameters for MIF_*, so it's not like we're losing any flexibility here. --- src/main/scala/Configs.scala | 2 +- vsrc/backup_mem.v | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/scala/Configs.scala b/src/main/scala/Configs.scala index 8a16cc44..5022a6d6 100644 --- a/src/main/scala/Configs.scala +++ b/src/main/scala/Configs.scala @@ -93,7 +93,7 @@ class DefaultConfig extends Config ( // Bits added by NASTI interconnect max(log2Up(site(MaxBanksPerMemoryChannel)), (if (site(UseDma)) 3 else 2))) - case MIFDataBits => Dump("MIF_DATA_BITS", 128) + case MIFDataBits => Dump("MIF_DATA_BITS", 64) case MIFAddrBits => Dump("MIF_ADDR_BITS", site(PAddrBits) - site(CacheBlockOffsetBits)) case MIFDataBeats => site(CacheBlockBytes) * 8 / site(MIFDataBits) diff --git a/vsrc/backup_mem.v b/vsrc/backup_mem.v index 584f2db1..2d8ae383 100644 --- a/vsrc/backup_mem.v +++ b/vsrc/backup_mem.v @@ -54,7 +54,7 @@ module BackupMemory output reg [`MIF_TAG_BITS-1:0] mem_resp_tag ); - localparam DATA_CYCLES = 4; + localparam DATA_CYCLES = 8; localparam DEPTH = 2*1024*1024; reg [`ceilLog2(DATA_CYCLES)-1:0] cnt; @@ -62,7 +62,7 @@ module BackupMemory reg state_busy, state_rw; reg [`MIF_ADDR_BITS-1:0] addr; - reg [`MIF_DATA_BITS-1:0] ram [DEPTH-1:0]; + reg [127:0] ram [DEPTH-1:0]; wire [`ceilLog2(DEPTH)-1:0] ram_addr = state_busy ? {addr[`ceilLog2(DEPTH/DATA_CYCLES)-1:0], cnt} : {mem_req_addr[`ceilLog2(DEPTH/DATA_CYCLES)-1:0], cnt}; wire do_read = mem_req_valid && mem_req_ready && !mem_req_rw || state_busy && !state_rw; @@ -97,9 +97,15 @@ module BackupMemory cnt <= cnt + 1'b1; if (do_write) - ram[ram_addr] <= mem_req_data_bits; + if (ram_addr[0] == 1'b0) + ram[ram_addr/2][63:0] <= mem_req_data_bits; + else + ram[ram_addr/2][127:64] <= mem_req_data_bits; else - mem_resp_data <= ram[ram_addr]; + if (ram_addr[0] == 1'b0) + mem_resp_data <= ram[ram_addr/2][63:0]; + else + mem_resp_data <= ram[ram_addr/2][127:64]; if (reset) mem_resp_valid <= 1'b0;