From ffe792bcfcab78c5b8040d5e6b0ef3a05e4918ac Mon Sep 17 00:00:00 2001 From: Ross Thompson Date: Mon, 20 Dec 2021 23:27:37 -0600 Subject: [PATCH 1/5] Fixed bug on icache spill. if the cpu stalled on the completion it was possible to use the wrong address for the sram read. Also miss spill hit always selected the wrong address. --- wally-pipelined/src/cache/icachefsm.sv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wally-pipelined/src/cache/icachefsm.sv b/wally-pipelined/src/cache/icachefsm.sv index dd4e2e4a..aa49a55e 100644 --- a/wally-pipelined/src/cache/icachefsm.sv +++ b/wally-pipelined/src/cache/icachefsm.sv @@ -294,8 +294,8 @@ module icachefsm ICacheStallF = 1'b0; LRUWriteEn = 1'b1; if(StallF) begin - NextState = STATE_CPU_BUSY; - SelAdr = 2'b01; + NextState = STATE_CPU_BUSY_SPILL; + SelAdr = 2'b10; end else begin NextState = STATE_READY; end From 3f62a64056e316c8e78994b25e0dd23ebc4975f4 Mon Sep 17 00:00:00 2001 From: Ross Thompson Date: Mon, 20 Dec 2021 23:45:55 -0600 Subject: [PATCH 2/5] Identified bug in the IFU which selects PCNextF when InvalidateICacheM is true. If the ID is invalid PCNextF should NOT be PCE. --- wally-pipelined/src/ifu/ifu.sv | 1 + 1 file changed, 1 insertion(+) diff --git a/wally-pipelined/src/ifu/ifu.sv b/wally-pipelined/src/ifu/ifu.sv index a6b63f24..896359bd 100644 --- a/wally-pipelined/src/ifu/ifu.sv +++ b/wally-pipelined/src/ifu/ifu.sv @@ -187,6 +187,7 @@ module ifu ( .s(BPPredWrongE), .y(PCNext1F)); + // *** December 20, 2021 BUG Ross Thompson, If instructions in ID and IF are already invalid we don't pick PCE on icache invalidate. mux2 #(`XLEN) pcmux2(.d0(PCNext1F), .d1(PCE), .s(InvalidateICacheM), From 8b97aaac3e9668320668b77ec2510c81a4b0f23e Mon Sep 17 00:00:00 2001 From: Ross Thompson Date: Tue, 21 Dec 2021 11:29:28 -0600 Subject: [PATCH 3/5] Fixed complex bug where FENCE is instruction class miss predicted as a taken branch. --- wally-pipelined/src/ifu/ifu.sv | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/wally-pipelined/src/ifu/ifu.sv b/wally-pipelined/src/ifu/ifu.sv index 896359bd..a0875e46 100644 --- a/wally-pipelined/src/ifu/ifu.sv +++ b/wally-pipelined/src/ifu/ifu.sv @@ -103,6 +103,9 @@ module ifu ( (* mark_debug = "true" *) logic [`PA_BITS-1:0] PCPFmmu, PCNextFPhys; // used to either truncate or expand PCPF and PCNextF into `PA_BITS width. logic [`XLEN+1:0] PCFExt; + logic [`XLEN-1:0] PCBPWrongInvalidate; + logic BPPredWrongM; + generate if (`XLEN==32) begin @@ -187,9 +190,13 @@ module ifu ( .s(BPPredWrongE), .y(PCNext1F)); - // *** December 20, 2021 BUG Ross Thompson, If instructions in ID and IF are already invalid we don't pick PCE on icache invalidate. + // December 20, 2021 Ross Thompson, If instructions in ID and IF are already invalid we don't pick PCE on icache invalidate. + // this only happens because of branch class miss prediction. The Fence instruction was incorrectly predicted as a branch + // this means on the previous cycle the BPPredWrongE updated PCNextF to the correct fall through address. + // to fix we need to select the correct address PCF as the next PCNextF. Unforunately we must still flush the instruction in IF + // as we are deliberately invalidating the icache. This address has to be refetched by the icache. mux2 #(`XLEN) pcmux2(.d0(PCNext1F), - .d1(PCE), + .d1(PCBPWrongInvalidate), .s(InvalidateICacheM), .y(PCNext2F)); @@ -206,6 +213,14 @@ module ifu ( flop #(1) resetReg (.clk(clk), .d(reset), .q(reset_q)); + + + flopenrc #(1) BPPredWrongMReg(.clk, .reset, .en(~StallM), .clear(FlushM), + .d(BPPredWrongE), .q(BPPredWrongM)); + + mux2 #(`XLEN) pcmuxBPWrongInvalidateFlush(.d0(PCE), .d1(PCF), + .s(BPPredWrongM & InvalidateICacheM), + .y(PCBPWrongInvalidate)); assign PCNextF = {UnalignedPCNextF[`XLEN-1:1], 1'b0}; // hart-SPEC p. 21 about 16-bit alignment From 7844d3f06478ebc164258653096d110f9f213c1f Mon Sep 17 00:00:00 2001 From: Ross Thompson Date: Tue, 21 Dec 2021 15:16:00 -0600 Subject: [PATCH 4/5] Fixed bug where the wrong address is read into the icache memory. --- wally-pipelined/src/cache/icachefsm.sv | 1 + 1 file changed, 1 insertion(+) diff --git a/wally-pipelined/src/cache/icachefsm.sv b/wally-pipelined/src/cache/icachefsm.sv index aa49a55e..1d579cd1 100644 --- a/wally-pipelined/src/cache/icachefsm.sv +++ b/wally-pipelined/src/cache/icachefsm.sv @@ -163,6 +163,7 @@ module icachefsm NextState = STATE_HIT_SPILL; end else if (~hit & ~spill) begin CntReset = 1'b1; + SelAdr = 2'b01; /// *********( NextState = STATE_MISS_FETCH_WDV; end else if (~hit & spill) begin CntReset = 1'b1; From 6a8e917e06c003d3e391fdfd9245fc00576675a9 Mon Sep 17 00:00:00 2001 From: Ross Thompson Date: Tue, 21 Dec 2021 15:59:56 -0600 Subject: [PATCH 5/5] It was possible for a load/store followed by tlb miss and update to have an exception and still commit its result to memory or register. --- wally-pipelined/src/cache/dcachefsm.sv | 18 +++++++----------- wally-pipelined/src/lsu/lsu.sv | 4 +++- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/wally-pipelined/src/cache/dcachefsm.sv b/wally-pipelined/src/cache/dcachefsm.sv index eb191b9a..f11b3c3e 100644 --- a/wally-pipelined/src/cache/dcachefsm.sv +++ b/wally-pipelined/src/cache/dcachefsm.sv @@ -169,7 +169,7 @@ module dcachefsm end // Flush dcache to next level of memory - else if(FlushDCacheM & ~(ExceptionM | PendingInterruptM)) begin + else if(FlushDCacheM) begin NextState = STATE_FLUSH; DCacheStall = 1'b1; SelAdrM = 2'b11; @@ -178,7 +178,7 @@ module dcachefsm end // amo hit - else if(AtomicM[1] & (&MemRWM) & CacheableM & ~(ExceptionM | PendingInterruptM) & CacheHit) begin + else if(AtomicM[1] & (&MemRWM) & CacheableM & CacheHit) begin SelAdrM = 2'b10; DCacheStall = 1'b0; @@ -194,7 +194,7 @@ module dcachefsm end end // read hit valid cached - else if(MemRWM[1] & CacheableM & ~(ExceptionM | PendingInterruptM) & CacheHit) begin + else if(MemRWM[1] & CacheableM & CacheHit) begin DCacheStall = 1'b0; LRUWriteEn = 1'b1; @@ -207,7 +207,7 @@ module dcachefsm end end // write hit valid cached - else if (MemRWM[0] & CacheableM & ~(ExceptionM | PendingInterruptM) & CacheHit) begin + else if (MemRWM[0] & CacheableM & CacheHit) begin SelAdrM = 2'b10; DCacheStall = 1'b0; SRAMWordWriteEnableM = 1'b1; @@ -223,29 +223,25 @@ module dcachefsm end end // read or write miss valid cached - else if((|MemRWM) & CacheableM & ~(ExceptionM | PendingInterruptM) & ~CacheHit) begin + else if((|MemRWM) & CacheableM & ~CacheHit) begin NextState = STATE_MISS_FETCH_WDV; CntReset = 1'b1; DCacheStall = 1'b1; end // uncached write - else if(MemRWM[0] & ~CacheableM & ~(ExceptionM | PendingInterruptM)) begin + else if(MemRWM[0] & ~CacheableM) begin NextState = STATE_UNCACHED_WRITE; CntReset = 1'b1; DCacheStall = 1'b1; AHBWrite = 1'b1; end // uncached read - else if(MemRWM[1] & ~CacheableM & ~(ExceptionM | PendingInterruptM)) begin + else if(MemRWM[1] & ~CacheableM) begin NextState = STATE_UNCACHED_READ; CntReset = 1'b1; DCacheStall = 1'b1; AHBRead = 1'b1; end - // fault - else if(AnyCPUReqM & (ExceptionM | PendingInterruptM)) begin - NextState = STATE_READY; - end else NextState = STATE_READY; end diff --git a/wally-pipelined/src/lsu/lsu.sv b/wally-pipelined/src/lsu/lsu.sv index 738124a9..ed7a9fcb 100644 --- a/wally-pipelined/src/lsu/lsu.sv +++ b/wally-pipelined/src/lsu/lsu.sv @@ -198,7 +198,9 @@ module lsu assign SelReplayCPURequest = (NextState == STATE_T0_REPLAY) | (NextState == STATE_T0_FAULT_REPLAY); assign SelHPTW = (CurrState == STATE_T3_DTLB_MISS) | (CurrState == STATE_T4_ITLB_MISS) | (CurrState == STATE_T5_ITLB_MISS) | (CurrState == STATE_T7_DITLB_MISS); - assign IgnoreRequest = CurrState == STATE_T0_READY & (ITLBMissF | DTLBMissM); + assign IgnoreRequest = (CurrState == STATE_T0_READY & (ITLBMissF | DTLBMissM | ExceptionM | PendingInterruptM)) | + ((CurrState == STATE_T0_REPLAY | CurrState == STATE_T0_FAULT_REPLAY) + & (ExceptionM | PendingInterruptM)); assign WalkerInstrPageFaultF = WalkerInstrPageFaultRaw | CurrState == STATE_T0_FAULT_REPLAY;