From 83df3dfe83e860aa272294032725383923158801 Mon Sep 17 00:00:00 2001 From: Rose Thompson Date: Mon, 15 Jan 2024 16:02:37 -0600 Subject: [PATCH 1/5] Fixed the zifencei bug (part of issue 405). --- src/ieu/controller.sv | 2 +- src/lsu/lsu.sv | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ieu/controller.sv b/src/ieu/controller.sv index 3507ec3e9..e5cfff9ed 100644 --- a/src/ieu/controller.sv +++ b/src/ieu/controller.sv @@ -370,7 +370,7 @@ module controller import cvw::*; #(parameter cvw_t P) ( // Fences // Ordinary fence is presently a nop // fence.i flushes the D$ and invalidates the I$ if Zifencei is supported and I$ is implemented - if (P.ZIFENCEI_SUPPORTED & P.ICACHE_SUPPORTED) begin:fencei + if (P.ZIFENCEI_SUPPORTED & (P.ICACHE_SUPPORTED | P.DCACHE_SUPPORTED)) begin:fencei logic FenceID; assign FenceID = FenceXD & (Funct3D == 3'b001); // is it a FENCE.I instruction? assign InvalidateICacheD = FenceID; diff --git a/src/lsu/lsu.sv b/src/lsu/lsu.sv index e7d6707d6..06d64c154 100644 --- a/src/lsu/lsu.sv +++ b/src/lsu/lsu.sv @@ -342,7 +342,6 @@ module lsu import cvw::*; #(parameter cvw_t P) ( assign DCacheStallM = CacheStall & ~IgnoreRequestTLB; assign CacheBusRW = CacheBusRWTemp; - // *** add support for cboz ahbcacheinterface #(.AHBW(P.AHBW), .LLEN(P.LLEN), .PA_BITS(P.PA_BITS), .BEATSPERLINE(BEATSPERLINE), .AHBWLOGBWPL(AHBWLOGBWPL), .LINELEN(LINELEN), .LLENPOVERAHBW(LLENPOVERAHBW), .READ_ONLY_CACHE(0)) ahbcacheinterface( .HCLK(clk), .HRESETn(~reset), .Flush(FlushW | IgnoreRequestTLB), .HRDATA, .HWDATA(LSUHWDATA), .HWSTRB(LSUHWSTRB), From 614a83331f10032200eb2b6c61f262813e7e9fb2 Mon Sep 17 00:00:00 2001 From: Rose Thompson Date: Mon, 15 Jan 2024 17:29:00 -0600 Subject: [PATCH 2/5] Fixed part of issue #405. The non-cache version of the bus controller did not have the correct supression of BusCommitted for a read only controller. --- src/ebu/ahbinterface.sv | 2 +- src/ebu/busfsm.sv | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ebu/ahbinterface.sv b/src/ebu/ahbinterface.sv index de17f3553..d9892f21d 100644 --- a/src/ebu/ahbinterface.sv +++ b/src/ebu/ahbinterface.sv @@ -64,7 +64,7 @@ module ahbinterface #( assign HWSTRB = '0; end - busfsm busfsm(.HCLK, .HRESETn, .Flush, .BusRW, + busfsm #(~LSU) busfsm(.HCLK, .HRESETn, .Flush, .BusRW, .BusCommitted, .Stall, .BusStall, .CaptureEn, .HREADY, .HTRANS, .HWRITE); diff --git a/src/ebu/busfsm.sv b/src/ebu/busfsm.sv index 108cd546d..a2d4e42b2 100644 --- a/src/ebu/busfsm.sv +++ b/src/ebu/busfsm.sv @@ -28,7 +28,9 @@ //////////////////////////////////////////////////////////////////////////////////////////////// // HCLK and clk must be the same clock! -module busfsm ( +module busfsm #( + parameter READ_ONLY +)( input logic HCLK, input logic HRESETn, @@ -70,7 +72,7 @@ module busfsm ( // (CurrState == DATA_PHASE & ~BusRW[0]); // possible optimization here. fails uart test, but i'm not sure the failure is valid. (CurrState == DATA_PHASE); - assign BusCommitted = CurrState != ADR_PHASE; + assign BusCommitted = CurrState != ADR_PHASE & ~(READ_ONLY & CurrState == MEM3); assign HTRANS = (CurrState == ADR_PHASE & HREADY & |BusRW & ~Flush) ? AHB_NONSEQ : AHB_IDLE; assign HWRITE = BusRW[0]; From 82a786f1855998a82a27de422cf76e0973363402 Mon Sep 17 00:00:00 2001 From: Rose Thompson Date: Mon, 15 Jan 2024 17:36:01 -0600 Subject: [PATCH 3/5] Hmm. Verilator is complaining about the parameter width. I'm not sure why so I changed to 1 bit. --- src/ebu/ahbinterface.sv | 2 +- src/ebu/busfsm.sv | 4 ++-- src/ifu/ifu.sv | 2 +- src/lsu/lsu.sv | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ebu/ahbinterface.sv b/src/ebu/ahbinterface.sv index d9892f21d..fa5a6293e 100644 --- a/src/ebu/ahbinterface.sv +++ b/src/ebu/ahbinterface.sv @@ -29,7 +29,7 @@ module ahbinterface #( parameter XLEN, - parameter LSU = 0 // 1: LSU bus width is `XLEN, 0: IFU bus width is 32 bits + parameter logic LSU = 1'b0 // 1: LSU bus width is `XLEN, 0: IFU bus width is 32 bits )( input logic HCLK, HRESETn, // bus interface diff --git a/src/ebu/busfsm.sv b/src/ebu/busfsm.sv index a2d4e42b2..9ba159705 100644 --- a/src/ebu/busfsm.sv +++ b/src/ebu/busfsm.sv @@ -29,7 +29,7 @@ // HCLK and clk must be the same clock! module busfsm #( - parameter READ_ONLY + parameter logic READ_ONLY )( input logic HCLK, input logic HRESETn, @@ -72,7 +72,7 @@ module busfsm #( // (CurrState == DATA_PHASE & ~BusRW[0]); // possible optimization here. fails uart test, but i'm not sure the failure is valid. (CurrState == DATA_PHASE); - assign BusCommitted = CurrState != ADR_PHASE & ~(READ_ONLY & CurrState == MEM3); + assign BusCommitted = (CurrState != ADR_PHASE) & ~(READ_ONLY & CurrState == MEM3); assign HTRANS = (CurrState == ADR_PHASE & HREADY & |BusRW & ~Flush) ? AHB_NONSEQ : AHB_IDLE; assign HWRITE = BusRW[0]; diff --git a/src/ifu/ifu.sv b/src/ifu/ifu.sv index 107a4af8b..8e7d9a0d1 100644 --- a/src/ifu/ifu.sv +++ b/src/ifu/ifu.sv @@ -273,7 +273,7 @@ module ifu import cvw::*; #(parameter cvw_t P) ( assign BusRW = ~ITLBMissF & ~SelIROM ? IFURWF : '0; assign IFUHSIZE = 3'b010; - ahbinterface #(P.XLEN, 0) ahbinterface(.HCLK(clk), .Flush(FlushD), .HRESETn(~reset), .HREADY(IFUHREADY), + ahbinterface #(P.XLEN, 1'b0) ahbinterface(.HCLK(clk), .Flush(FlushD), .HRESETn(~reset), .HREADY(IFUHREADY), .HRDATA(HRDATA), .HTRANS(IFUHTRANS), .HWRITE(IFUHWRITE), .HWDATA(), .HWSTRB(), .BusRW, .ByteMask(), .WriteData('0), .Stall(GatedStallD), .BusStall, .BusCommitted(BusCommittedF), .FetchBuffer(FetchBuffer)); diff --git a/src/lsu/lsu.sv b/src/lsu/lsu.sv index 06d64c154..cf0fab9e8 100644 --- a/src/lsu/lsu.sv +++ b/src/lsu/lsu.sv @@ -367,7 +367,7 @@ module lsu import cvw::*; #(parameter cvw_t P) ( assign LSUHADDR = PAdrM; assign LSUHSIZE = LSUFunct3M; - ahbinterface #(P.XLEN, 1) ahbinterface(.HCLK(clk), .HRESETn(~reset), .Flush(FlushW), .HREADY(LSUHREADY), + ahbinterface #(P.XLEN, 1'b1) ahbinterface(.HCLK(clk), .HRESETn(~reset), .Flush(FlushW), .HREADY(LSUHREADY), .HRDATA(HRDATA), .HTRANS(LSUHTRANS), .HWRITE(LSUHWRITE), .HWDATA(LSUHWDATA), .HWSTRB(LSUHWSTRB), .BusRW, .ByteMask(ByteMaskM), .WriteData(LSUWriteDataM[P.XLEN-1:0]), .Stall(GatedStallW), .BusStall, .BusCommitted(BusCommittedM), .FetchBuffer(FetchBuffer)); From dfe5ef44276af447c32c510aae47e02350d16093 Mon Sep 17 00:00:00 2001 From: Rose Thompson Date: Mon, 15 Jan 2024 17:47:17 -0600 Subject: [PATCH 4/5] Added logic for the non-cache atomics. --- src/ebu/ahbinterface.sv | 3 ++- src/ebu/busfsm.sv | 26 ++++++++++++++++---------- src/ifu/ifu.sv | 2 +- src/lsu/lsu.sv | 2 +- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/ebu/ahbinterface.sv b/src/ebu/ahbinterface.sv index fa5a6293e..df84175f0 100644 --- a/src/ebu/ahbinterface.sv +++ b/src/ebu/ahbinterface.sv @@ -44,6 +44,7 @@ module ahbinterface #( input logic Stall, // Core pipeline is stalled input logic Flush, // Pipeline stage flush. Prevents bus transaction from starting input logic [1:0] BusRW, // Memory operation read/write control: 10: read, 01: write + input logic BusAtomic, // Uncache atomic memory operation input logic [XLEN/8-1:0] ByteMask, // Bytes enables within a word input logic [XLEN-1:0] WriteData, // IEU write data for a store output logic BusStall, // Bus is busy with an in flight memory operation @@ -64,7 +65,7 @@ module ahbinterface #( assign HWSTRB = '0; end - busfsm #(~LSU) busfsm(.HCLK, .HRESETn, .Flush, .BusRW, + busfsm #(~LSU) busfsm(.HCLK, .HRESETn, .Flush, .BusRW, .BusAtomic, .BusCommitted, .Stall, .BusStall, .CaptureEn, .HREADY, .HTRANS, .HWRITE); diff --git a/src/ebu/busfsm.sv b/src/ebu/busfsm.sv index 9ba159705..126759b0d 100644 --- a/src/ebu/busfsm.sv +++ b/src/ebu/busfsm.sv @@ -38,6 +38,7 @@ module busfsm #( input logic Stall, // Core pipeline is stalled input logic Flush, // Pipeline stage flush. Prevents bus transaction from starting input logic [1:0] BusRW, // Memory operation read/write control: 10: read, 01: write + input logic BusAtomic, // Uncache atomic memory operation output logic CaptureEn, // Enable updating the Fetch buffer with valid data from HRDATA output logic BusStall, // Bus is busy with an in flight memory operation output logic BusCommitted, // Bus is busy with an in flight memory operation and it is not safe to take an interrupt @@ -47,7 +48,7 @@ module busfsm #( output logic HWRITE // AHB 0: Read operation 1: Write operation ); - typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, MEM3} busstatetype; + typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, MEM3, ATOMIC_PHASE} busstatetype; typedef enum logic [1:0] {AHB_IDLE = 2'b00, AHB_BUSY = 2'b01, AHB_NONSEQ = 2'b10, AHB_SEQ = 2'b11} ahbtranstype; busstatetype CurrState, NextState; @@ -58,13 +59,16 @@ module busfsm #( always_comb begin case(CurrState) - ADR_PHASE: if(HREADY & |BusRW) NextState = DATA_PHASE; - else NextState = ADR_PHASE; - DATA_PHASE: if(HREADY) NextState = MEM3; - else NextState = DATA_PHASE; - MEM3: if(Stall) NextState = MEM3; - else NextState = ADR_PHASE; - default: NextState = ADR_PHASE; + ADR_PHASE: if(HREADY & |BusRW) NextState = DATA_PHASE; + else NextState = ADR_PHASE; + DATA_PHASE: if(HREADY & BusAtomic) NextState = ATOMIC_PHASE; + else if(HREADY & ~BusAtomic) NextState = MEM3; + else NextState = DATA_PHASE; + ATOMIC_PHASE: if(HREADY) NextState = MEM3; + else NextState = ATOMIC_PHASE; + MEM3: if(Stall) NextState = MEM3; + else NextState = ADR_PHASE; + default: NextState = ADR_PHASE; endcase end @@ -74,8 +78,10 @@ module busfsm #( assign BusCommitted = (CurrState != ADR_PHASE) & ~(READ_ONLY & CurrState == MEM3); - assign HTRANS = (CurrState == ADR_PHASE & HREADY & |BusRW & ~Flush) ? AHB_NONSEQ : AHB_IDLE; - assign HWRITE = BusRW[0]; + assign HTRANS = (CurrState == ADR_PHASE & HREADY & |BusRW & ~Flush) | + (CurrState == DATA_PHASE & BusAtomic) ? AHB_NONSEQ : AHB_IDLE; + assign HWRITE = (BusRW[0] & ~BusAtomic) | (CurrState == DATA_PHASE & BusAtomic); + assign CaptureEn = CurrState == DATA_PHASE; endmodule diff --git a/src/ifu/ifu.sv b/src/ifu/ifu.sv index 8e7d9a0d1..0bd899306 100644 --- a/src/ifu/ifu.sv +++ b/src/ifu/ifu.sv @@ -275,7 +275,7 @@ module ifu import cvw::*; #(parameter cvw_t P) ( ahbinterface #(P.XLEN, 1'b0) ahbinterface(.HCLK(clk), .Flush(FlushD), .HRESETn(~reset), .HREADY(IFUHREADY), .HRDATA(HRDATA), .HTRANS(IFUHTRANS), .HWRITE(IFUHWRITE), .HWDATA(), - .HWSTRB(), .BusRW, .ByteMask(), .WriteData('0), + .HWSTRB(), .BusRW, .BusAtomic('0), .ByteMask(), .WriteData('0), .Stall(GatedStallD), .BusStall, .BusCommitted(BusCommittedF), .FetchBuffer(FetchBuffer)); assign CacheCommittedF = '0; diff --git a/src/lsu/lsu.sv b/src/lsu/lsu.sv index cf0fab9e8..f86c62aea 100644 --- a/src/lsu/lsu.sv +++ b/src/lsu/lsu.sv @@ -369,7 +369,7 @@ module lsu import cvw::*; #(parameter cvw_t P) ( ahbinterface #(P.XLEN, 1'b1) ahbinterface(.HCLK(clk), .HRESETn(~reset), .Flush(FlushW), .HREADY(LSUHREADY), .HRDATA(HRDATA), .HTRANS(LSUHTRANS), .HWRITE(LSUHWRITE), .HWDATA(LSUHWDATA), - .HWSTRB(LSUHWSTRB), .BusRW, .ByteMask(ByteMaskM), .WriteData(LSUWriteDataM[P.XLEN-1:0]), + .HWSTRB(LSUHWSTRB), .BusRW, .BusAtomic(AtomicM[1]), .ByteMask(ByteMaskM), .WriteData(LSUWriteDataM[P.XLEN-1:0]), .Stall(GatedStallW), .BusStall, .BusCommitted(BusCommittedM), .FetchBuffer(FetchBuffer)); // Mux between the 2 sources of read data, 0: Bus, 1: DTIM From ff5554ca61d22087c6cd7e882652d8285a02f73f Mon Sep 17 00:00:00 2001 From: Rose Thompson Date: Tue, 16 Jan 2024 10:43:20 -0600 Subject: [PATCH 5/5] Atomics work correctly without a d cache. --- src/ebu/buscachefsm.sv | 24 ++++++++++++++---------- src/ebu/busfsm.sv | 12 ++++++++---- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/ebu/buscachefsm.sv b/src/ebu/buscachefsm.sv index 0368164ed..8d434c678 100644 --- a/src/ebu/buscachefsm.sv +++ b/src/ebu/buscachefsm.sv @@ -66,7 +66,7 @@ module buscachefsm #( output logic [2:0] HBURST // AHB burst length ); - typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, ATOMIC_PHASE, MEM3, CACHE_FETCH, CACHE_WRITEBACK} busstatetype; + typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, ATOMIC_READ_DATA_PHASE, ATOMIC_PHASE, MEM3, CACHE_FETCH, CACHE_WRITEBACK} busstatetype; typedef enum logic [1:0] {AHB_IDLE = 2'b00, AHB_BUSY = 2'b01, AHB_NONSEQ = 2'b10, AHB_SEQ = 2'b11} ahbtranstype; busstatetype CurrState, NextState; @@ -87,13 +87,15 @@ module buscachefsm #( always_comb begin case(CurrState) - ADR_PHASE: if (HREADY & |BusRW) NextState = DATA_PHASE; - else if (HREADY & BusWrite) NextState = CACHE_WRITEBACK; - else if (HREADY & CacheBusRW[1]) NextState = CACHE_FETCH; - else NextState = ADR_PHASE; - DATA_PHASE: if(HREADY & BusAtomic) NextState = ATOMIC_PHASE; - else if(HREADY & ~BusAtomic) NextState = MEM3; + ADR_PHASE: if (HREADY & |BusRW) NextState = DATA_PHASE; + else if (HREADY & BusWrite) NextState = CACHE_WRITEBACK; + else if (HREADY & CacheBusRW[1]) NextState = CACHE_FETCH; + else NextState = ADR_PHASE; + DATA_PHASE: if(HREADY & BusAtomic) NextState = ATOMIC_READ_DATA_PHASE; + else if(HREADY & ~BusAtomic) NextState = MEM3; else NextState = DATA_PHASE; + ATOMIC_READ_DATA_PHASE: if(HREADY) NextState = ATOMIC_PHASE; + else NextState = ATOMIC_READ_DATA_PHASE; ATOMIC_PHASE: if(HREADY) NextState = MEM3; else NextState = ATOMIC_PHASE; MEM3: if(Stall) NextState = MEM3; @@ -107,7 +109,7 @@ module buscachefsm #( else if(HREADY & FinalBeatCount & BusCMOZero) NextState = MEM3; else if(HREADY & FinalBeatCount & ~|CacheBusRW) NextState = ADR_PHASE; else NextState = CACHE_WRITEBACK; - default: NextState = ADR_PHASE; + default: NextState = ADR_PHASE; endcase end @@ -129,6 +131,7 @@ module buscachefsm #( //(CurrState == DATA_PHASE & ~BusRW[0]) | // *** replace the next line with this. Fails uart test but i think it's a test problem not a hardware problem. (CurrState == DATA_PHASE) | (CurrState == ATOMIC_PHASE) | + (CurrState == ATOMIC_READ_DATA_PHASE) | (CurrState == CACHE_FETCH & ~FinalBeatCount) | (CurrState == CACHE_WRITEBACK & ~FinalBeatCount); @@ -136,11 +139,11 @@ module buscachefsm #( // AHB bus interface assign HTRANS = (CurrState == ADR_PHASE & HREADY & ((|BusRW) | (|CacheBusRW) | BusCMOZero) & ~Flush) | - (CurrState == DATA_PHASE & BusAtomic) | + (CurrState == ATOMIC_READ_DATA_PHASE & BusAtomic) | (CacheAccess & FinalBeatCount & |CacheBusRW & HREADY & ~Flush) ? AHB_NONSEQ : // if we have a pipelined request (CacheAccess & |BeatCount) ? (`BURST_EN ? AHB_SEQ : AHB_NONSEQ) : AHB_IDLE; - assign HWRITE = ((BusRW[0] & ~BusAtomic) | BusWrite & ~Flush) | (CurrState == DATA_PHASE & BusAtomic) | + assign HWRITE = ((BusRW[0] & ~BusAtomic) | BusWrite & ~Flush) | (CurrState == ATOMIC_READ_DATA_PHASE & BusAtomic) | (CurrState == CACHE_WRITEBACK & |BeatCount); assign HBURST = `BURST_EN & ((|CacheBusRW & ~Flush) | (CacheAccess & |BeatCount)) ? LocalBurstType : 3'b0; @@ -159,6 +162,7 @@ module buscachefsm #( assign SelBusBeat = (CurrState == ADR_PHASE & (BusRW[0] | BusWrite)) | (CurrState == DATA_PHASE & BusRW[0]) | (CurrState == ATOMIC_PHASE & BusRW[0]) | + (CurrState == ATOMIC_READ_DATA_PHASE & BusRW[0]) | (CurrState == CACHE_WRITEBACK) | (CurrState == CACHE_FETCH); diff --git a/src/ebu/busfsm.sv b/src/ebu/busfsm.sv index 126759b0d..81d11715e 100644 --- a/src/ebu/busfsm.sv +++ b/src/ebu/busfsm.sv @@ -48,7 +48,7 @@ module busfsm #( output logic HWRITE // AHB 0: Read operation 1: Write operation ); - typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, MEM3, ATOMIC_PHASE} busstatetype; + typedef enum logic [2:0] {ADR_PHASE, DATA_PHASE, MEM3, ATOMIC_READ_DATA_PHASE, ATOMIC_PHASE} busstatetype; typedef enum logic [1:0] {AHB_IDLE = 2'b00, AHB_BUSY = 2'b01, AHB_NONSEQ = 2'b10, AHB_SEQ = 2'b11} ahbtranstype; busstatetype CurrState, NextState; @@ -61,9 +61,11 @@ module busfsm #( case(CurrState) ADR_PHASE: if(HREADY & |BusRW) NextState = DATA_PHASE; else NextState = ADR_PHASE; - DATA_PHASE: if(HREADY & BusAtomic) NextState = ATOMIC_PHASE; + DATA_PHASE: if(HREADY & BusAtomic) NextState = ATOMIC_READ_DATA_PHASE; else if(HREADY & ~BusAtomic) NextState = MEM3; else NextState = DATA_PHASE; + ATOMIC_READ_DATA_PHASE: if(HREADY) NextState = ATOMIC_PHASE; + else NextState = ATOMIC_READ_DATA_PHASE; ATOMIC_PHASE: if(HREADY) NextState = MEM3; else NextState = ATOMIC_PHASE; MEM3: if(Stall) NextState = MEM3; @@ -74,13 +76,15 @@ module busfsm #( assign BusStall = (CurrState == ADR_PHASE & |BusRW) | // (CurrState == DATA_PHASE & ~BusRW[0]); // possible optimization here. fails uart test, but i'm not sure the failure is valid. + (CurrState == ATOMIC_PHASE) | + (CurrState == ATOMIC_READ_DATA_PHASE) | (CurrState == DATA_PHASE); assign BusCommitted = (CurrState != ADR_PHASE) & ~(READ_ONLY & CurrState == MEM3); assign HTRANS = (CurrState == ADR_PHASE & HREADY & |BusRW & ~Flush) | - (CurrState == DATA_PHASE & BusAtomic) ? AHB_NONSEQ : AHB_IDLE; - assign HWRITE = (BusRW[0] & ~BusAtomic) | (CurrState == DATA_PHASE & BusAtomic); + (CurrState == ATOMIC_READ_DATA_PHASE & BusAtomic) ? AHB_NONSEQ : AHB_IDLE; + assign HWRITE = (BusRW[0] & ~BusAtomic) | (CurrState == ATOMIC_READ_DATA_PHASE & BusAtomic); assign CaptureEn = CurrState == DATA_PHASE;