From df51d9908de0689e411668f9ba233f48a98b11ef Mon Sep 17 00:00:00 2001
From: bbracker <bbracker@hmc.edu>
Date: Thu, 18 Mar 2021 18:25:12 -0400
Subject: [PATCH] AHB bugfixes and sim waveview refactoring

---
 wally-pipelined/regression/sim-wally-rv32ic   |  2 +-
 wally-pipelined/regression/wally-pipelined.do | 52 +-------------
 .../regression/wave-dos/ahb-waves.do          | 70 +++++++++++++++++++
 .../regression/wave-dos/default-waves.do      | 51 ++++++++++++++
 wally-pipelined/src/ebu/ahblite.sv            | 61 ++++++++--------
 wally-pipelined/src/uncore/dtim.sv            | 35 ++++++----
 6 files changed, 175 insertions(+), 96 deletions(-)
 create mode 100644 wally-pipelined/regression/wave-dos/ahb-waves.do
 create mode 100644 wally-pipelined/regression/wave-dos/default-waves.do

diff --git a/wally-pipelined/regression/sim-wally-rv32ic b/wally-pipelined/regression/sim-wally-rv32ic
index a254c8246..b69fb3172 100755
--- a/wally-pipelined/regression/sim-wally-rv32ic
+++ b/wally-pipelined/regression/sim-wally-rv32ic
@@ -1,3 +1,3 @@
 vsim -c <<!
-do wally-pipelined-batch.do ../config/rv32ic
+do wally-pipelined-batch.do ../config/rv32ic rv32ic
 !
diff --git a/wally-pipelined/regression/wally-pipelined.do b/wally-pipelined/regression/wally-pipelined.do
index cabcfe153..a6833db59 100644
--- a/wally-pipelined/regression/wally-pipelined.do
+++ b/wally-pipelined/regression/wally-pipelined.do
@@ -42,61 +42,13 @@ vsim workopt
 view wave
 
 -- display input and output signals as hexidecimal values
-# Diplays All Signals recursively
-add wave /testbench/clk
-add wave /testbench/reset
-add wave -divider
-#add wave /testbench/dut/hart/ebu/IReadF
-add wave /testbench/dut/hart/DataStall
-add wave /testbench/dut/hart/InstrStall
-add wave /testbench/dut/hart/StallF
-add wave /testbench/dut/hart/StallD
-add wave /testbench/dut/hart/StallE
-add wave /testbench/dut/hart/StallM
-add wave /testbench/dut/hart/StallW
-add wave /testbench/dut/hart/FlushD
-add wave /testbench/dut/hart/FlushE
-add wave /testbench/dut/hart/FlushM
-add wave /testbench/dut/hart/FlushW
-
-add wave -divider
-add wave -hex /testbench/dut/hart/ifu/PCF
-add wave -hex /testbench/dut/hart/ifu/InstrF
-add wave /testbench/InstrFName
-add wave -hex /testbench/dut/hart/ifu/PCD
-add wave -hex /testbench/dut/hart/ifu/InstrD
-add wave /testbench/InstrDName
-add wave -divider
-add wave -hex /testbench/dut/hart/ifu/PCE
-add wave -hex /testbench/dut/hart/ifu/InstrE
-add wave /testbench/InstrEName
-add wave -hex /testbench/dut/hart/ieu/dp/SrcAE
-add wave -hex /testbench/dut/hart/ieu/dp/SrcBE
-add wave -hex /testbench/dut/hart/ieu/dp/ALUResultE
-#add wave /testbench/dut/hart/ieu/dp/PCSrcE
-add wave -divider
-add wave -hex /testbench/dut/hart/ifu/PCM
-add wave -hex /testbench/dut/hart/ifu/InstrM
-add wave /testbench/InstrMName
-add wave /testbench/dut/uncore/dtim/memwrite
-add wave -hex /testbench/dut/uncore/HADDR
-add wave -hex /testbench/dut/uncore/HWDATA
-add wave -divider
-add wave -hex /testbench/dut/hart/ifu/PCW
-add wave -hex /testbench/dut/hart/ifu/InstrW
-add wave /testbench/InstrWName
-add wave /testbench/dut/hart/ieu/dp/RegWriteW
-add wave -hex /testbench/dut/hart/ieu/dp/ResultW
-add wave -hex /testbench/dut/hart/ieu/dp/RdW
-add wave -divider
-#add ww
-add wave -hex -r /testbench/*
+do ./wave-dos/default-waves.do
 
 -- Set Wave Output Items 
 TreeUpdate [SetDefaultTree]
 WaveRestoreZoom {0 ps} {100 ps}
 configure wave -namecolwidth 250
-configure wave -valuecolwidth 120
+configure wave -valuecolwidth 140
 configure wave -justifyvalue left
 configure wave -signalnamewidth 0
 configure wave -snapdistance 10
diff --git a/wally-pipelined/regression/wave-dos/ahb-waves.do b/wally-pipelined/regression/wave-dos/ahb-waves.do
new file mode 100644
index 000000000..f24def65d
--- /dev/null
+++ b/wally-pipelined/regression/wave-dos/ahb-waves.do
@@ -0,0 +1,70 @@
+add wave /testbench/clk
+add wave /testbench/reset
+add wave -divider
+
+#add wave /testbench/dut/hart/ebu/IReadF
+add wave /testbench/dut/hart/DataStall
+add wave /testbench/dut/hart/InstrStall
+add wave /testbench/dut/hart/StallF
+add wave /testbench/dut/hart/StallD
+add wave /testbench/dut/hart/StallE
+add wave /testbench/dut/hart/StallM
+add wave /testbench/dut/hart/StallW
+add wave /testbench/dut/hart/FlushD
+add wave /testbench/dut/hart/FlushE
+add wave /testbench/dut/hart/FlushM
+add wave /testbench/dut/hart/FlushW
+add wave -divider
+
+add wave -hex /testbench/dut/hart/ifu/PCF
+add wave -hex /testbench/dut/hart/ifu/InstrF
+add wave /testbench/InstrFName
+add wave -hex /testbench/dut/hart/ifu/PCD
+add wave -hex /testbench/dut/hart/ifu/InstrD
+add wave /testbench/InstrDName
+add wave -divider
+
+add wave -hex /testbench/dut/hart/ifu/PCE
+add wave -hex /testbench/dut/hart/ifu/InstrE
+add wave /testbench/InstrEName
+add wave -hex /testbench/dut/hart/ieu/dp/SrcAE
+add wave -hex /testbench/dut/hart/ieu/dp/SrcBE
+add wave -hex /testbench/dut/hart/ieu/dp/ALUResultE
+#add wave /testbench/dut/hart/ieu/dp/PCSrcE
+add wave -divider
+
+add wave -hex /testbench/dut/hart/ifu/PCM
+add wave -hex /testbench/dut/hart/ifu/InstrM
+add wave /testbench/InstrMName
+add wave /testbench/dut/uncore/dtim/memwrite
+add wave -hex /testbench/dut/uncore/HADDR
+add wave -hex /testbench/dut/uncore/HWDATA
+add wave -divider
+
+add wave -hex /testbench/dut/hart/ebu/MemReadM
+add wave -hex /testbench/dut/hart/ebu/InstrReadF
+add wave -hex /testbench/dut/hart/ebu/BusState
+add wave -hex /testbench/dut/hart/ebu/NextBusState
+add wave -hex /testbench/dut/hart/ebu/HADDR
+add wave -hex /testbench/dut/hart/ebu/HREADY
+add wave -hex /testbench/dut/hart/ebu/HTRANS
+add wave -hex /testbench/dut/hart/ebu/HRDATA
+add wave -hex /testbench/dut/hart/ebu/HWRITE
+add wave -hex /testbench/dut/hart/ebu/HWDATA
+add wave -hex /testbench/dut/hart/ebu/CaptureDataM
+add wave -hex /testbench/dut/hart/ebu/InstrStall
+add wave -divider
+
+add wave -hex /testbench/dut/hart/ifu/PCW
+add wave -hex /testbench/dut/hart/ifu/InstrW
+add wave /testbench/InstrWName
+add wave /testbench/dut/hart/ieu/dp/RegWriteW
+add wave -hex /testbench/dut/hart/ebu/ReadDataW
+add wave -hex /testbench/dut/hart/ieu/dp/ResultW
+add wave -hex /testbench/dut/hart/ieu/dp/RdW
+add wave -divider
+
+add wave -hex /testbench/dut/uncore/dtim/*
+add wave -divider
+
+add wave -hex -r /testbench/*
\ No newline at end of file
diff --git a/wally-pipelined/regression/wave-dos/default-waves.do b/wally-pipelined/regression/wave-dos/default-waves.do
new file mode 100644
index 000000000..bdc9bf457
--- /dev/null
+++ b/wally-pipelined/regression/wave-dos/default-waves.do
@@ -0,0 +1,51 @@
+# These were ripped from wally-pipelined.do
+
+# Diplays All Signals recursively
+add wave /testbench/clk
+add wave /testbench/reset
+add wave -divider
+#add wave /testbench/dut/hart/ebu/IReadF
+add wave /testbench/dut/hart/DataStall
+add wave /testbench/dut/hart/InstrStall
+add wave /testbench/dut/hart/StallF
+add wave /testbench/dut/hart/StallD
+add wave /testbench/dut/hart/StallE
+add wave /testbench/dut/hart/StallM
+add wave /testbench/dut/hart/StallW
+add wave /testbench/dut/hart/FlushD
+add wave /testbench/dut/hart/FlushE
+add wave /testbench/dut/hart/FlushM
+add wave /testbench/dut/hart/FlushW
+
+add wave -divider
+add wave -hex /testbench/dut/hart/ifu/PCF
+add wave -hex /testbench/dut/hart/ifu/InstrF
+add wave /testbench/InstrFName
+add wave -hex /testbench/dut/hart/ifu/PCD
+add wave -hex /testbench/dut/hart/ifu/InstrD
+add wave /testbench/InstrDName
+add wave -divider
+add wave -hex /testbench/dut/hart/ifu/PCE
+add wave -hex /testbench/dut/hart/ifu/InstrE
+add wave /testbench/InstrEName
+add wave -hex /testbench/dut/hart/ieu/dp/SrcAE
+add wave -hex /testbench/dut/hart/ieu/dp/SrcBE
+add wave -hex /testbench/dut/hart/ieu/dp/ALUResultE
+#add wave /testbench/dut/hart/ieu/dp/PCSrcE
+add wave -divider
+add wave -hex /testbench/dut/hart/ifu/PCM
+add wave -hex /testbench/dut/hart/ifu/InstrM
+add wave /testbench/InstrMName
+add wave /testbench/dut/uncore/dtim/memwrite
+add wave -hex /testbench/dut/uncore/HADDR
+add wave -hex /testbench/dut/uncore/HWDATA
+add wave -divider
+add wave -hex /testbench/dut/hart/ifu/PCW
+add wave -hex /testbench/dut/hart/ifu/InstrW
+add wave /testbench/InstrWName
+add wave /testbench/dut/hart/ieu/dp/RegWriteW
+add wave -hex /testbench/dut/hart/ieu/dp/ResultW
+add wave -hex /testbench/dut/hart/ieu/dp/RdW
+add wave -divider
+#add ww
+add wave -hex -r /testbench/*
\ No newline at end of file
diff --git a/wally-pipelined/src/ebu/ahblite.sv b/wally-pipelined/src/ebu/ahblite.sv
index bec919ece..2f5c4f7ba 100644
--- a/wally-pipelined/src/ebu/ahblite.sv
+++ b/wally-pipelined/src/ebu/ahblite.sv
@@ -76,7 +76,7 @@ module ahblite (
 
   logic GrantData;
   logic [2:0] ISize;
-  logic [`AHBW-1:0] HRDATAMasked, ReadDataM, ReadDataPreW, WriteData;
+  logic [`AHBW-1:0] HRDATAMasked, ReadDataM, ReadDataNewW, ReadDataOldW, WriteData;
   logic IReady, DReady;
   logic CaptureDataM;
 
@@ -89,42 +89,40 @@ module ahblite (
   // Data accesses have priority over instructions.  However, if a data access comes
   // while an instruction read is occuring, the instruction read finishes before
   // the data access can take place.
-  typedef enum {IDLE, MEMREAD, MEMWRITE, INSTRREAD, INSTRREADMEMPENDING, ATOMICREAD, ATOMICWRITE} statetype;
+  typedef enum {IDLE, MEMREAD, MEMWRITE, INSTRREAD, INSTRREADC, ATOMICREAD, ATOMICWRITE} statetype;
   statetype BusState, NextBusState;
 
   flopenl #(.TYPE(statetype)) busreg(HCLK, ~HRESETn, 1'b1, NextBusState, IDLE, BusState);
 
   always_comb 
     case (BusState) 
-      IDLE: if      (AtomicM[1]) NextBusState = ATOMICREAD;
-            else if (MemReadM)   NextBusState = MEMREAD;  // Memory has pirority over instructions
-            else if (MemWriteM)  NextBusState = MEMWRITE;
-            else if (InstrReadF) NextBusState = INSTRREAD;
-            else                 NextBusState = IDLE;
-      ATOMICREAD: if (~HREADY)   NextBusState = ATOMICREAD;
-            else                 NextBusState = ATOMICWRITE;
-      ATOMICWRITE: if (~HREADY)  NextBusState = ATOMICWRITE;
-            else if (InstrReadF) NextBusState = INSTRREAD;
-            else                 NextBusState = IDLE;
-      MEMREAD: if (~HREADY)      NextBusState = MEMREAD;
-            else if (InstrReadF) NextBusState = INSTRREAD;
-            else                 NextBusState = IDLE;
-      MEMWRITE: if (~HREADY)     NextBusState = MEMWRITE;
-            else if (InstrReadF) NextBusState = INSTRREAD;
-            else                 NextBusState = IDLE;
-      INSTRREAD: //if (~HREADY & (MemReadM | MemWriteM))  NextBusState = INSTRREADMEMPENDING; // *** shouldn't happen, delete
-            if (~HREADY)    NextBusState = INSTRREAD;
-            else                 NextBusState = IDLE;  // if (InstrReadF still high)
-      INSTRREADMEMPENDING: if (~HREADY) NextBusState = INSTRREADMEMPENDING; // *** shouldn't happen, delete
-            else if (MemReadM)   NextBusState = MEMREAD;
-            else                 NextBusState = MEMWRITE; // must be write if not a read.  Don't return to idle.
+      IDLE: if      (AtomicM[1])  NextBusState = ATOMICREAD;
+            else if (MemReadM)    NextBusState = MEMREAD;  // Memory has pirority over instructions
+            else if (MemWriteM)   NextBusState = MEMWRITE;
+            else if (InstrReadF)  NextBusState = INSTRREAD;
+            else                  NextBusState = IDLE;
+      ATOMICREAD: if (~HREADY)    NextBusState = ATOMICREAD;
+            else                  NextBusState = ATOMICWRITE;
+      ATOMICWRITE: if (~HREADY)   NextBusState = ATOMICWRITE;
+            else if (InstrReadF)  NextBusState = INSTRREAD;
+            else                  NextBusState = IDLE;
+      MEMREAD: if (~HREADY)       NextBusState = MEMREAD;
+            else if (InstrReadF)  NextBusState = INSTRREADC;
+            else                  NextBusState = IDLE;
+      MEMWRITE: if (~HREADY)      NextBusState = MEMWRITE;
+            else if (InstrReadF)  NextBusState = INSTRREAD;
+            else                  NextBusState = IDLE;
+      INSTRREAD:
+            if (~HREADY)          NextBusState = INSTRREAD;
+            else                  NextBusState = IDLE;  // if (InstrReadF still high)
+      INSTRREADC: if (~HREADY)    NextBusState = INSTRREADC; // "C" for "competing", meaning please don't mess up the memread in the W stage.
+            else                  NextBusState = IDLE;
     endcase
 
   // stall signals
   assign #2 DataStall = (NextBusState == MEMREAD) || (NextBusState == MEMWRITE) || 
-                        (NextBusState == ATOMICREAD) || (NextBusState == ATOMICWRITE) ||
-                        (NextBusState == INSTRREADMEMPENDING);
-  assign #1 InstrStall = (NextBusState == INSTRREAD);
+                        (NextBusState == ATOMICREAD) || (NextBusState == ATOMICWRITE);
+  assign #1 InstrStall = (NextBusState == INSTRREAD) || (NextBusState == INSTRREADC);
        
   //  bus outputs
   assign #1 GrantData = (NextBusState == MEMREAD) || (NextBusState == MEMWRITE) || 
@@ -151,9 +149,10 @@ module ahblite (
   assign ReadDataM = HRDATAMasked; // changed from W to M dh 2/7/2021
   assign CaptureDataM = ((BusState == MEMREAD) && (NextBusState != MEMREAD)) ||
                         ((BusState == ATOMICREAD) && (NextBusState == ATOMICWRITE));
-  // *** check if this introduces an unnecessary cycle of latency in memory accesses
-  flopenr #(`XLEN) ReadDataPreWReg(clk, reset, CaptureDataM, ReadDataM, ReadDataPreW); // *** this may break when there is no instruction read after data read
-  flopenr #(`XLEN) ReadDataWReg(clk, reset, ~StallW, ReadDataPreW, ReadDataW);
+  // We think this introduces an unnecessary cycle of latency in memory accesses
+  flopenr #(`XLEN) ReadDataNewWReg(clk, reset, CaptureDataM,    ReadDataM, ReadDataNewW); // *** check that this does not break when there is no instruction read after data read
+  flopenr #(`XLEN) ReadDataOldWReg(clk, reset, CaptureDataM, ReadDataNewW, ReadDataOldW); // *** check that this does not break when there is no instruction read after data read
+  assign ReadDataW = (BusState == INSTRREADC) ? ReadDataOldW : ReadDataNewW;
 
   // Extract and sign-extend subwords if necessary
   subwordread swr(.*);
@@ -164,7 +163,7 @@ module ahblite (
       logic [`XLEN-1:0] AMOResult;
 //      amoalu amoalu(.a(HRDATA), .b(WriteDataM), .funct(Funct7M), .width(MemSizeM), 
 //                    .result(AMOResult));
-      amoalu amoalu(.srca(ReadDataPreW), .srcb(WriteDataM), .funct(Funct7M), .width(MemSizeM), 
+      amoalu amoalu(.srca(ReadDataW), .srcb(WriteDataM), .funct(Funct7M), .width(MemSizeM), 
                     .result(AMOResult));
       mux2 #(`XLEN) wdmux(WriteDataM, AMOResult, AtomicM[1], WriteData);
     end else
diff --git a/wally-pipelined/src/uncore/dtim.sv b/wally-pipelined/src/uncore/dtim.sv
index d9e0d81cb..a5c4574ee 100644
--- a/wally-pipelined/src/uncore/dtim.sv
+++ b/wally-pipelined/src/uncore/dtim.sv
@@ -30,6 +30,8 @@ module dtim #(parameter BASE=0, RANGE = 65535) (
   input  logic             HSELTim,
   input  logic [31:0]      HADDR,
   input  logic             HWRITE,
+  input  logic             HREADY,
+  input  logic [1:0]       HTRANS,
   input  logic [`XLEN-1:0] HWDATA,
   output logic [`XLEN-1:0] HREADTim,
   output logic             HRESPTim, HREADYTim
@@ -40,27 +42,26 @@ module dtim #(parameter BASE=0, RANGE = 65535) (
   logic [`XLEN-1:0] HREADTim0;
 
 //  logic [`XLEN-1:0] write;
-  logic [31:0] HADDRd;
-  logic        newAdr;
+  logic        prevHREADYTim, risingHREADYTim;
+  logic        initTrans;
   logic [15:0] entry;
   logic        memread, memwrite;
   logic [3:0]  busycount;
 
-  always_ff @(posedge HCLK) begin
-    memread <= HSELTim & ~ HWRITE;
-    memwrite <= HSELTim & HWRITE;
-    A <= HADDR;
-    HADDRd <= HADDR;
-  end
+  assign initTrans = HREADY & HSELTim & (HTRANS != 2'b00);
 
-  assign newAdr = HADDR!=HADDRd;
+  // *** this seems like a weird way to use reset
+  flopenr #(1)  memreadreg(HCLK, 1'b0, initTrans | ~HRESETn, HSELTim & ~HWRITE, memread);
+  flopenr #(1) memwritereg(HCLK, 1'b0, initTrans | ~HRESETn, HSELTim &  HWRITE, memwrite);
+  flopenr #(32)   haddrreg(HCLK, 1'b0, initTrans | ~HRESETn, HADDR, A);
 
   // busy FSM to extend READY signal
   always_ff @(posedge HCLK, negedge HRESETn) 
     if (~HRESETn) begin
-      HREADYTim <= 1;
+      busycount <= 0;
+      HREADYTim <= #1 0;
     end else begin
-      if ((HREADYTim | newAdr) & HSELTim) begin
+      if (initTrans) begin
         busycount <= 0;
         HREADYTim <= #1 0;
       end else if (~HREADYTim) begin
@@ -71,8 +72,14 @@ module dtim #(parameter BASE=0, RANGE = 65535) (
         end
       end
     end
-
   assign HRESPTim = 0; // OK
+  
+  // Rising HREADY edge detector
+  //   Indicates when dtim is finishing up
+  //   Needed because HREADY may go high for other reasons,
+  //   and we only want to write data when finishing up.
+  flopr #(1) prevhreadytimreg(HCLK,~HRESETn,HREADYTim,prevHREADYTim);
+  assign risingHREADYTim = HREADYTim & ~prevHREADYTim;
 
   // Model memory read and write
   generate
@@ -80,13 +87,13 @@ module dtim #(parameter BASE=0, RANGE = 65535) (
       always_ff @(posedge HCLK) begin
         HWADDR <= A;
         HREADTim0 <= RAM[A[31:3]];
-        if (memwrite && HREADYTim) RAM[HWADDR[31:3]] <= HWDATA;
+        if (memwrite && risingHREADYTim) RAM[HWADDR[31:3]] <= HWDATA;
       end
     end else begin 
       always_ff @(posedge HCLK) begin
         HWADDR <= A;  
         HREADTim0 <= RAM[A[31:2]];
-        if (memwrite && HREADYTim) RAM[HWADDR[31:2]] <= HWDATA;
+        if (memwrite && risingHREADYTim) RAM[HWADDR[31:2]] <= HWDATA;
       end
     end
   endgenerate