merged coverage exclusions

sim/GetLineNum.do

@@ -0,0 +1,18 @@
+# Alec Vercruysse
+# 2023-04-12
+# Note that the target string is regex, and needs to be double-escaped.
+# e.g. to match a (, you need \\(.
+proc GetLineNum {fname target} {
+    set f [open $fname]
+    set linectr 1
+    while {[gets $f line] != -1} {
+         if {[regexp $target $line]} {
+             close $f
+             return $linectr
+         }
+         incr linectr
+     }
+    close $f
+    return -code error \
+         "target string not found"
+}

sim/coverage-exclusions-rv64gc.do

@@ -27,6 +27,8 @@
 # This file should be a last resort.  It's preferable to put 
 # // coverage off 
 # statements inline with the code whenever possible.
+# a hack to describe coverage exclusions without hardcoding linenumbers:
+do GetLineNum.do
 
 # LZA (i<64) statement confuses coverage tool 
 # This is ugly to exlcude the whole file - is there a better option?  // coverage off isn't working
@@ -35,6 +37,46 @@ coverage exclude -srcfile lzc.sv
 # FDIVSQRT has 
 coverage exclude -scope /core/fpu/fpu/fdivsqrt/fdivsqrtfsm -ftrans state DONE->BUSY
 
+### Exclude D$ states and logic for the I$ instance
+# This is cleaner than trying to set an I$-specific pragma in cachefsm.sv (which would exclude it for the D$ instance too)
+# Also exclude the write line to ready transition for the I$ since we can't get a flush during this operation.
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -fstate CurrState STATE_FLUSH STATE_FLUSH_WRITEBACK STATE_FLUSH_WRITEBACK STATE_WRITEBACK
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -ftrans CurrState STATE_WRITE_LINE->STATE_READY
+# exclude unused transitions from case statement. Unfortunately the whole branch needs to be excluded I think. Expression coverage should still work.
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache state-case"] -item b 1
+# exclude branch/condition coverage: LineDirty if statement
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache FETCHStatement"] -item bc 1
+# exclude the unreachable logic
+set start [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-start: icache case"]
+set end [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-end: icache case"]
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange $start-$end
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache WRITEBACKStatement"]
+# exclude Atomic Operation logic
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache storeAMO"] -item e 1 -fecexprrow 6
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache storeAMO1"] -item e 1 -fecexprrow 2-4
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache AnyUpdateHit"] -item e 1 -fecexprrow 2
+# cache write logic
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheW"] -item e 1 -fecexprrow 4
+# output signal logic
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache StallStates"] -item e 1 -fecexprrow 8 12 14
+set start [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-start: icache flushdirtycontrols"]
+set end [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag-end: icache flushdirtycontrols"]
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange $start-$end
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusW"]
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache SelAdrCauses"] -item e 1 -fecexprrow 4 10
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: icache CacheBusRCauses"] -item e 1 -fecexprrow 1-2 12
+# cache.sv AdrSelMux and CacheBusAdrMux, excluding unhit Flush branch
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/AdrSelMux -linerange [GetLineNum ../src/generic/mux.sv "exclusion-tag: mux3"] -item b 1
+coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheBusAdrMux -linerange [GetLineNum ../src/generic/mux.sv "exclusion-tag: mux3"] -item b 1 3
+# CacheWay Dirty logic. -scope does not accept wildcards.
+set numcacheways 4
+for {set i 0} {$i < $numcacheways} {incr i} {
+    coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SetDirtyWay"] -item e 1
+    coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SelectedWiteWordEn"] -item e 1 -fecexprrow 4 6
+    # below: flushD can't go high during an icache write b/c of pipeline stall
+    coverage exclude -scope /dut/core/ifu/bus/icache/icache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: icache SetValidEN"] -item e 1 -fecexprrow 4
+}
+
 
 # Excluding peripherals as sources of instructions for the ifu
 coverage exclude -scope /dut/core/ifu/immu/immu/pmachecker/adrdecs/clintdec

src/cache/cache.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// cache 
+// cache.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created: 7 July 2021
@@ -122,7 +122,7 @@ module cache #(parameter LINELEN,  NUMLINES,  NUMWAYS, LOGBWPL, WORDLEN, MUXINTE
   // Select victim way for associative caches
   if(NUMWAYS > 1) begin:vict
     cacheLRU #(NUMWAYS, SETLEN, OFFSETLEN, NUMLINES) cacheLRU(
-      .clk, .reset, .CacheEn, .FlushStage, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn,
+      .clk, .reset, .CacheEn, .HitWay, .ValidWay, .VictimWay, .CacheSet, .LRUWriteEn,
       .SetValid, .PAdr(PAdr[SETTOP-1:OFFSETLEN]), .InvalidateCache, .FlushCache);
   end else 
     assign VictimWay = 1'b1; // one hot.

src/cache/cacheLRU.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// dcache (data cache)
+// cacheLRU.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created: 20 July 2021
@@ -33,7 +33,6 @@ module cacheLRU
   #(parameter NUMWAYS = 4, SETLEN = 9, OFFSETLEN = 5, NUMLINES = 128) (
   input  logic                clk, 
   input  logic                reset,
-  input  logic                FlushStage,      // Pipeline flush of second stage (prevent writes and bus operations)
   input  logic                CacheEn,         // Enable the cache memory arrays.  Disable hold read data constant
   input  logic [NUMWAYS-1:0]  HitWay,          // Which way is valid and matches PAdr's tag
   input  logic [NUMWAYS-1:0]  ValidWay,        // Which ways for a particular set are valid, ignores tag
@@ -90,16 +89,26 @@ module cacheLRU
     assign WayExpanded[StartIndex : EndIndex] = {{DuplicationFactor}{WayEncoded[row]}};
   end
 
-  genvar               r, a, s;
+  genvar               node;
   assign LRUUpdate[NUMWAYS-2] = '1;
-  for(s = NUMWAYS-2; s >= NUMWAYS/2; s--) begin : enables
+  for(node = NUMWAYS-2; node >= NUMWAYS/2; node--) begin : enables
-    localparam p = NUMWAYS - s - 1;
+    localparam ctr = NUMWAYS - node - 1;
-    localparam g = log2(p);
+    localparam ctr_depth = log2(ctr);
-    localparam t0 = s - p;
+    localparam lchild = node - ctr;
-    localparam t1 = t0 - 1;
+    localparam rchild = lchild - 1;
-    localparam r = LOGNUMWAYS - g;
+    localparam r = LOGNUMWAYS - ctr_depth;
-    assign LRUUpdate[t0] = LRUUpdate[s] & ~WayEncoded[r];
+
-    assign LRUUpdate[t1] = LRUUpdate[s] & WayEncoded[r];
+    // the child node will be updated if its parent was updated and
+    // the WayEncoded bit was the correct value.
+    // The if statement is only there for coverage since LRUUpdate[root] is always 1.
+    if (node == NUMWAYS-2) begin
+      assign LRUUpdate[lchild] = ~WayEncoded[r];
+      assign LRUUpdate[rchild] = WayEncoded[r];
+    end
+    else begin
+      assign LRUUpdate[lchild] = LRUUpdate[node] & ~WayEncoded[r];
+      assign LRUUpdate[rchild] = LRUUpdate[node] & WayEncoded[r];
+    end
   end
 
   // The root node of the LRU tree will always be selected in LRUUpdate. No mux needed.
@@ -107,15 +116,15 @@ module cacheLRU
   mux2 #(1) LRUMuxes[NUMWAYS-3:0](CurrLRU[NUMWAYS-3:0], ~WayExpanded[NUMWAYS-3:0], LRUUpdate[NUMWAYS-3:0], NextLRU[NUMWAYS-3:0]);
 
   // Compute next victim way.
-  for(s = NUMWAYS-2; s >= NUMWAYS/2; s--) begin
+  for(node = NUMWAYS-2; node >= NUMWAYS/2; node--) begin
-    localparam t0 = 2*s - NUMWAYS;
+    localparam t0 = 2*node - NUMWAYS;
     localparam t1 = t0 + 1;
-    assign Intermediate[s] = CurrLRU[s] ? Intermediate[t0] : Intermediate[t1];
+    assign Intermediate[node] = CurrLRU[node] ? Intermediate[t0] : Intermediate[t1];
   end
-  for(s = NUMWAYS/2-1; s >= 0; s--) begin
+  for(node = NUMWAYS/2-1; node >= 0; node--) begin
-    localparam int0 = (NUMWAYS/2-1-s)*2;
+    localparam int0 = (NUMWAYS/2-1-node)*2;
     localparam int1 = int0 + 1;
-    assign Intermediate[s] = CurrLRU[s] ? int1[LOGNUMWAYS-1:0] : int0[LOGNUMWAYS-1:0];
+    assign Intermediate[node] = CurrLRU[node] ? int1[LOGNUMWAYS-1:0] : int0[LOGNUMWAYS-1:0];
   end
 
   logic [NUMWAYS-1:0] FirstZero;
@@ -134,11 +143,9 @@ module cacheLRU
   always_ff @(posedge clk) begin
     if (reset) for (int set = 0; set < NUMLINES; set++) LRUMemory[set] <= '0;
     if(CacheEn) begin
-      // if((InvalidateCache | FlushCache) & ~FlushStage) for (int set = 0; set < NUMLINES; set++) LRUMemory[set] <= '0;
+      if(LRUWriteEn)
-      if (LRUWriteEn & ~FlushStage) begin 
         LRUMemory[PAdr] <= NextLRU;
-      end
+      if(LRUWriteEn & (PAdr == CacheSet))
-      if(LRUWriteEn & ~FlushStage & (PAdr == CacheSet))
         CurrLRU <= #1 NextLRU;
       else 
         CurrLRU <= #1 LRUMemory[CacheSet];

src/cache/cachefsm.sv

@@ -1,11 +1,11 @@
 ///////////////////////////////////////////
-// dcache (data cache) fsm
+// cachefsm.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created: 25 August 2021
 // Modified: 20 January 2023
 //
-// Purpose: Controller for the dcache fsm
+// Purpose: Controller for the cache fsm
 //
 // Documentation: RISC-V System on Chip Design Chapter 7 (Figure 7.14 and Table 7.1)
 //
@@ -89,13 +89,13 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) (
   assign AMO = CacheAtomic[1] & (&CacheRW);
   assign StoreAMO = AMO | CacheRW[0];
 
-  assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache;
+  assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; // exclusion-tag: icache storeAMO
-  assign AnyUpdateHit = (StoreAMO) & CacheHit;
+  assign AnyUpdateHit = (StoreAMO) & CacheHit;                             // exclusion-tag: icache storeAMO1
-  assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit);  
+  assign AnyHit = AnyUpdateHit | (CacheRW[1] & CacheHit);                  // exclusion-tag: icache AnyUpdateHit
   assign FlushFlag = FlushAdrFlag & FlushWayFlag;
 
   // outputs for the performance counters.
-  assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY;
+  assign CacheAccess = (AMO | CacheRW[1] | CacheRW[0]) & CurrState == STATE_READY; // exclusion-tag: icache CacheW
   assign CacheMiss = CacheAccess & ~CacheHit;
 
   // special case on reset. When the fsm first exists reset the
@@ -109,17 +109,18 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) (
   
   always_comb begin
     NextState = STATE_READY;
-    case (CurrState)
+    case (CurrState)                                                                                        // exclusion-tag: icache state-case
       STATE_READY:           if(InvalidateCache)                               NextState = STATE_READY;
                              else if(FlushCache & ~READ_ONLY_CACHE)            NextState = STATE_FLUSH;
-                             else if(AnyMiss & (READ_ONLY_CACHE | ~LineDirty)) NextState = STATE_FETCH;
+                             else if(AnyMiss & (READ_ONLY_CACHE | ~LineDirty)) NextState = STATE_FETCH;     // exclusion-tag: icache FETCHStatement
-                             else if(AnyMiss & LineDirty)                      NextState = STATE_WRITEBACK;
+                             else if(AnyMiss & LineDirty)                      NextState = STATE_WRITEBACK; // exclusion-tag: icache WRITEBACKStatement
                              else                                              NextState = STATE_READY;
       STATE_FETCH:           if(CacheBusAck)                                   NextState = STATE_WRITE_LINE;
                              else                                              NextState = STATE_FETCH;
       STATE_WRITE_LINE:                                                        NextState = STATE_READ_HOLD;
       STATE_READ_HOLD:       if(Stall)                                         NextState = STATE_READ_HOLD;
                              else                                              NextState = STATE_READY;
+      // exclusion-tag-start: icache case
       STATE_WRITEBACK:       if(CacheBusAck)                                   NextState = STATE_FETCH;
                              else                                              NextState = STATE_WRITEBACK;
       // eviction needs a delay as the bus fsm does not correctly handle sending the write command at the same time as getting back the bus ack.
@@ -129,13 +130,14 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) (
       STATE_FLUSH_WRITEBACK: if(CacheBusAck & ~FlushFlag)                      NextState = STATE_FLUSH;
                              else if(CacheBusAck)                              NextState = STATE_READ_HOLD;
                              else                                              NextState = STATE_FLUSH_WRITEBACK;
+      // exclusion-tag-end: icache case
       default:                                                                 NextState = STATE_READY;
     endcase
   end
 
   // com back to CPU
   assign CacheCommitted = (CurrState != STATE_READY) & ~(READ_ONLY_CACHE & CurrState == STATE_READ_HOLD);
-  assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | 
+  assign CacheStall = (CurrState == STATE_READY & (FlushCache | AnyMiss)) | // exclusion-tag: icache StallStates
                       (CurrState == STATE_FETCH) |
                       (CurrState == STATE_WRITEBACK) |
                       (CurrState == STATE_WRITE_LINE) |  // this cycle writes the sram, must keep stalling so the next cycle can read the next hit/miss unless its a write.
@@ -143,12 +145,14 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) (
                       (CurrState == STATE_FLUSH_WRITEBACK);
   // write enables internal to cache
   assign SetValid = CurrState == STATE_WRITE_LINE;
-  assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) |
+  // coverage off -item e 1 -fecexprrow 8
-                    (CurrState == STATE_WRITE_LINE & (StoreAMO));
-  assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) |
-                      (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal.  Dirty must be cleared concurrently and atomically with writeback.  For single core cannot clear after writeback on bus ack and change flushadr.  Clears the wrong set.
   assign LRUWriteEn = (CurrState == STATE_READY & AnyHit) |
-                      (CurrState == STATE_WRITE_LINE);
+                      (CurrState == STATE_WRITE_LINE) & ~FlushStage;
+  // exclusion-tag-start: icache flushdirtycontrols
+  assign SetDirty = (CurrState == STATE_READY & AnyUpdateHit) |         // exclusion-tag: icache SetDirty
+                    (CurrState == STATE_WRITE_LINE & (StoreAMO));
+  assign ClearDirty = (CurrState == STATE_WRITE_LINE & ~(StoreAMO)) |   // exclusion-tag: icache ClearDirty
+                      (CurrState == STATE_FLUSH & LineDirty); // This is wrong in a multicore snoop cache protocal.  Dirty must be cleared concurrently and atomically with writeback.  For single core cannot clear after writeback on bus ack and change flushadr.  Clears the wrong set.
   // Flush and eviction controls
   assign SelWriteback = (CurrState == STATE_WRITEBACK & ~CacheBusAck) |
                     (CurrState == STATE_READY & AnyMiss & LineDirty);
@@ -162,20 +166,20 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) (
              (CurrState == STATE_FLUSH_WRITEBACK & CacheBusAck);
   assign FlushCntRst = (CurrState == STATE_FLUSH & FlushFlag & ~LineDirty) |
               (CurrState == STATE_FLUSH_WRITEBACK & FlushFlag & CacheBusAck);
+  // exclusion-tag-end: icache flushdirtycontrols
   // Bus interface controls
-  assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | 
+  assign CacheBusRW[1] = (CurrState == STATE_READY & AnyMiss & ~LineDirty) | // exclusion-tag: icache CacheBusRCauses
                          (CurrState == STATE_FETCH & ~CacheBusAck) | 
                          (CurrState == STATE_WRITEBACK & CacheBusAck);
-  assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) |
+  assign CacheBusRW[0] = (CurrState == STATE_READY & AnyMiss & LineDirty) | // exclusion-tag: icache CacheBusW
                           (CurrState == STATE_WRITEBACK & ~CacheBusAck) |
                      (CurrState == STATE_FLUSH_WRITEBACK & ~CacheBusAck);
 
-  assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // changes if store delay hazard removed
+  assign SelAdr = (CurrState == STATE_READY & (StoreAMO | AnyMiss)) | // exclusion-tag: icache SelAdrCauses // changes if store delay hazard removed
                   (CurrState == STATE_FETCH) |
                   (CurrState == STATE_WRITEBACK) |
                   (CurrState == STATE_WRITE_LINE) |
                   resetDelay;
-
   assign SelFetchBuffer = CurrState == STATE_WRITE_LINE | CurrState == STATE_READ_HOLD;
   assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache;
                        

src/cache/cacheway.sv

@@ -97,18 +97,13 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26,
   /////////////////////////////////////////////////////////////////////////////////////////////
 
   assign SetValidWay = SetValid & SelData;
+  assign SetDirtyWay = SetDirty & SelData;                                 // exclusion-tag: icache SetDirtyWay
   assign ClearDirtyWay = ClearDirty & SelData;
-  if (!READ_ONLY_CACHE) begin
+  assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage;  // exclusion-tag: icache SelectedWiteWordEn
-    assign SetDirtyWay = SetDirty & SelData;
+  assign SetValidEN = SetValidWay & ~FlushStage;                           // exclusion-tag: icache SetValidEN
-    assign SelectedWriteWordEn = (SetValidWay | SetDirtyWay) & ~FlushStage;
-  end
-  else begin
-    assign SelectedWriteWordEn = SetValidWay & ~FlushStage;
-  end
 
   // If writing the whole line set all write enables to 1, else only set the correct word.
   assign FinalByteMask = SetValidWay ? '1 : LineByteMask; // OR
-  assign SetValidEN = SetValidWay & ~FlushStage;
 
   /////////////////////////////////////////////////////////////////////////////////////////////
   // Tag Array

src/cache/subcachelineread.sv

@@ -1,11 +1,11 @@
 ///////////////////////////////////////////
-// subcachelineread
+// subcachelineread.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created: 4 February 2022
 // Modified: 20 January 2023
 //
-// Purpose: Muxes the cache line downto the word size.  Also include possilbe save/restore registers/muxes.
+// Purpose: Muxes the cache line down to the word size.  Also include possible save/restore registers/muxes.
 //
 // Documentation: RISC-V System on Chip Design Chapter 7
 
@@ -31,7 +31,6 @@
 
 module subcachelineread #(parameter LINELEN, WORDLEN, 
   parameter MUXINTERVAL )(     // The number of bits between mux. Set to 16 for I$ to support compressed.  Set to `LLEN for D$
-
   input  logic [$clog2(LINELEN/8) - $clog2(MUXINTERVAL/8) - 1 : 0] PAdr,       // Physical address 
   input  logic [LINELEN-1:0]                     ReadDataLine,// Read data of the whole cacheline
   output logic [WORDLEN-1:0]                     ReadDataWord // read data of selected word.

src/ebu/ahbcacheinterface.sv

@@ -51,11 +51,11 @@ module ahbcacheinterface #(
   
   // cache interface
   input  logic [`PA_BITS-1:0] CacheBusAdr,            // Address of cache line
-  input logic [`LLEN-1:0]     CacheReadDataWordM,      // one word of cache line during a writeback
+  input  logic [`LLEN-1:0]    CacheReadDataWordM,     // One word of cache line during a writeback
   input  logic                CacheableOrFlushCacheM, // Memory operation is cacheable or flushing D$
   input  logic                Cacheable,              // Memory operation is cachable
   input  logic [1:0]          CacheBusRW,             // Cache bus operation, 01: writeback, 10: fetch
-  output logic                CacheBusAck,             // Handshack to $ indicating bus transaction completed
+  output logic                CacheBusAck,            // Handshake to $ indicating bus transaction completed
   output logic [LINELEN-1:0]  FetchBuffer,            // Register to hold beats of cache line as the arrive from bus
   output logic [AHBWLOGBWPL-1:0] BeatCount,           // Beat position within the cache line in the Address Phase
   output logic                SelBusBeat,             // Tells the cache to select the word from ReadData or WriteData from BeatCount rather than PAdr

src/ebu/controllerinput.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// controller input stage
+// controllerinput.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created:  August 31, 2022
@@ -40,7 +40,7 @@ module controllerinput #(
   input  logic                HRESETn,
   input  logic                Save,     // Two or more managers requesting (HTRANS != 00) at the same time.  Save the non-granted manager inputs
   input  logic                Restore,  // Restore a saved manager inputs when it is finally granted
-  input logic                 Disable,  // Supress HREADY to the non-granted manager
+  input  logic                Disable,  // Suppress HREADY to the non-granted manager
   output logic                Request,  // This manager is making a request
   // controller input
   input  logic [1:0]          HTRANSIn,  // Manager input. AHB transaction type, 00: IDLE, 10 NON_SEQ, 11 SEQ
@@ -48,14 +48,14 @@ module controllerinput #(
   input  logic [2:0]          HSIZEIn,   // Manager input. AHB transaction width
   input  logic [2:0]          HBURSTIn,  // Manager input. AHB burst length
   input  logic [`PA_BITS-1:0] HADDRIn,   // Manager input. AHB address
-  output logic                HREADYOut, // Indicate to manager the peripherial is not busy and another manager does not have priority
+  output logic                HREADYOut, // Indicate to manager the peripheral is not busy and another manager does not have priority
   // controller output
-  output logic [1:0]          HTRANSOut, // Aribrated manager transaction. AHB transaction type, 00: IDLE, 10 NON_SEQ, 11 SEQ
+  output logic [1:0]          HTRANSOut, // Arbitrated manager transaction. AHB transaction type, 00: IDLE, 10 NON_SEQ, 11 SEQ
-  output logic                HWRITEOut, // Aribrated manager transaction. AHB 0: Read operation 1: Write operation 
+  output logic                HWRITEOut, // Arbitrated manager transaction. AHB 0: Read operation 1: Write operation 
-  output logic [2:0]          HSIZEOut,  // Aribrated manager transaction. AHB transaction width
+  output logic [2:0]          HSIZEOut,  // Arbitrated manager transaction. AHB transaction width
-  output logic [2:0]          HBURSTOut, // Aribrated manager transaction. AHB burst length 
+  output logic [2:0]          HBURSTOut, // Arbitrated manager transaction. AHB burst length 
-  output logic [`PA_BITS-1:0] HADDROut,  // Aribrated manager transaction. AHB address
+  output logic [`PA_BITS-1:0] HADDROut,  // Arbitrated manager transaction. AHB address
-  input logic                 HREADYIn   // Peripherial ready
+  input  logic                HREADYIn   // Peripheral ready
 );
 
   logic                       HWRITESave;

src/ebu/ebufsmarb.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// ebufsmarb
+// ebufsmarb.sv
 //
 // Written: Ross Thompson ross1728@gmail.com
 // Created: 23 January 2023
@@ -86,7 +86,7 @@ module ebufsmarb (
   // Controller 1 (LSU)
   // When both the IFU and LSU request at the same time, the FSM will go into the arbitrate state.
   // Once the LSU request is done the fsm returns to IDLE.  To prevent the LSU from regaining
-  // priority and re issuing the same memroy operation, the delayed IFUReqD squashes the LSU request.
+  // priority and re-issuing the same memory operation, the delayed IFUReqD squashes the LSU request.
   // This is necessary because the pipeline is stalled for the entire duration of both transactions,
   // and the LSU memory request will stil be active.
   flopr #(1) ifureqreg(HCLK, ~HRESETn, IFUReq, IFUReqD);

src/fpu/fdivsqrt/fdivsqrtexpcalc.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// fdivsqrtpreproc.sv
+// fdivsqrtexpcalc.sv
 //
 // Written: David_Harris@hmc.edu, me@KatherineParry.com, cturek@hmc.edu
 // Modified:13 January 2022

src/generic/mux.sv

@@ -40,7 +40,7 @@ module mux3 #(parameter WIDTH = 8) (
   input  logic [1:0]       s, 
   output logic [WIDTH-1:0] y);
 
-  assign y = s[1] ? d2 : (s[0] ? d1 : d0); 
+  assign y = s[1] ? d2 : (s[0] ? d1 : d0); // exclusion-tag: mux3
 endmodule
 
 module mux4 #(parameter WIDTH = 8) (

src/generic/onehotdecoder.sv

@@ -1,5 +1,5 @@
 ///////////////////////////////////////////
-// oneHotDecoder.sv
+// onehotdecoder.sv
 //
 // Written: ross1728@gmail.com July 09, 2021
 // Modified: 

src/ifu/bpred/bpred.sv

@@ -187,7 +187,7 @@ module bpred (
   // Correct branch/jump target.
   mux2 #(`XLEN) pccorrectemux(PCLinkE, IEUAdrE, PCSrcE, PCCorrectE);
   
-  // If the fence/csrw was predicted as a taken branch then we select PCF, rather PCE.
+  // If the fence/csrw was predicted as a taken branch then we select PCF, rather than PCE.
   // Effectively this is PCM+4 or the non-existant PCLinkM
   if(`INSTR_CLASS_PRED) mux2 #(`XLEN) pcmuxBPWrongInvalidateFlush(PCE, PCF, BPWrongM, NextValidPCE);
   else  assign NextValidPCE = PCE;
@@ -201,7 +201,7 @@ module bpred (
     // 3. target ras    (ras target wrong / class[2])
     // 4. direction     (br dir wrong / class[0])
 
-    // Unforuantely we can't use PCD to infer the correctness of the BTB or RAS because the class prediction 
+    // Unfortunately we can't use PCD to infer the correctness of the BTB or RAS because the class prediction 
     // could be wrong or the fall through address selected for branch predict not taken.
     // By pipeline the BTB's PC and RAS address through the pipeline we can measure the accuracy of
     // both without the above inaccuracies.

src/ifu/bpred/btb.sv

@@ -1,7 +1,7 @@
 ///////////////////////////////////////////
 // btb.sv
 //
-// Written: Ross Thomposn ross1728@gmail.com
+// Written: Ross Thompson ross1728@gmail.com
 // Created: February 15, 2021
 // Modified: 24 January 2023 
 //
@@ -34,7 +34,7 @@ module btb #(parameter Depth = 10 ) (
   input  logic             clk,
   input  logic             reset,
   input  logic             StallF, StallD, StallE, StallM, StallW, FlushD, FlushE, FlushM, FlushW,
-  input  logic [`XLEN-1:0] PCNextF, PCF, PCD, PCE, PCM,// PC at various stages
+  input  logic [`XLEN-1:0] PCNextF, PCF, PCD, PCE, PCM, // PC at various stages
   output logic [`XLEN-1:0] BPBTAF,                      // BTB's guess at PC
   output logic [`XLEN-1:0] BPBTAD,
   output logic [`XLEN-1:0] BPBTAE,
@@ -73,7 +73,7 @@ module btb #(parameter Depth = 10 ) (
 
   // must output a valid PC and valid bit during reset.  Because only PCF, not PCNextF is reset, PCNextF is invalid
   // during reset.  The BTB must produce a non X PC1NextF to allow the simulation to run.
-  // While thie mux could be included in IFU it is not necessary for the IROM/I$/bus.
+  // While the mux could be included in IFU it is not necessary for the IROM/I$/bus.
   // For now it is optimal to leave it here.
   assign ResetPC = `RESET_VECTOR;
   assign PCNextFIndex = reset ? ResetPC[Depth+1:2] : {PCNextF[Depth+1] ^ PCNextF[1], PCNextF[Depth:2]}; 

src/privileged/csrsr.sv

@@ -198,9 +198,12 @@ module csrsr (
         STATUS_UBE <= #1 CSRWriteValM[6]  & `U_SUPPORTED & `BIGENDIAN_SUPPORTED;
         STATUS_MBE <= #1 nextMBE;
         STATUS_SBE <= #1 nextSBE;
+      // coverage off
+      // MSTATUSH only exists in 32-bit configurations, will not be hit on rv64gc
       end else if (WriteMSTATUSHM) begin
         STATUS_MBE <= #1 CSRWriteValM[5] & `BIGENDIAN_SUPPORTED;
         STATUS_SBE <= #1 CSRWriteValM[4] & `S_SUPPORTED & `BIGENDIAN_SUPPORTED;
+      // coverage on
       end else if (WriteSSTATUSM) begin // write a subset of the STATUS bits
         STATUS_MXR_INT <= #1 CSRWriteValM[19];
         STATUS_SUM_INT <= #1 CSRWriteValM[18];

src/privileged/trap.sv

@@ -81,11 +81,14 @@ module trap (
   ///////////////////////////////////////////
   
   assign BothInstrAccessFaultM = InstrAccessFaultM | HPTWInstrAccessFaultM;
+  // coverage off -item e 1 -fecexprrow 2
+  // excludes InstrMisalignedFaultM from coverage of this line, since misaligned instructions cannot occur in rv64gc.
   assign ExceptionM = InstrMisalignedFaultM | BothInstrAccessFaultM | IllegalInstrFaultM |
                       LoadMisalignedFaultM | StoreAmoMisalignedFaultM |
                       InstrPageFaultM | LoadPageFaultM | StoreAmoPageFaultM |
                       BreakpointFaultM | EcallFaultM |
                       LoadAccessFaultM | StoreAmoAccessFaultM;
+  // coverage on
   assign TrapM = ExceptionM | InterruptM; 
   assign RetM = mretM | sretM;
 

testbench/testbench.sv

@@ -169,7 +169,7 @@ module testbench;
   logic        InitializingMemories;
   integer      ResetCount, ResetThreshold;
   logic        InReset;
-  logic        Begin;
+  logic        BeginSample;
   
   // instantiate device to be tested
   assign GPIOIN = 0;
@@ -225,7 +225,8 @@ module testbench;
       totalerrors = 0;
       testadr = 0;
       testadrNoBase = 0;
-      // riscof tests have a different signature, tests[0] == "1" refers to RiscvArchTests and  tests[0] == "2" refers to WallyRiscvArchTests 
+      // riscof tests have a different signature, tests[0] == "1" refers to RiscvArchTests 
+      // and tests[0] == "2" refers to WallyRiscvArchTests 
       riscofTest = tests[0] == "1" | tests[0] == "2"; 
       // fill memory with defined values to reduce Xs in simulation
       // Quick note the memory will need to be initialized.  The C library does not
@@ -265,8 +266,9 @@ module testbench;
         ProgramAddrMapFile = {pathname, tests[test], ".elf.objdump.addr"};
         ProgramLabelMapFile = {pathname, tests[test], ".elf.objdump.lab"};
       end
-      // declare memory labels that interest us, the updateProgramAddrLabelArray task will find the addr of each label and fill the array
+      // declare memory labels that interest us, the updateProgramAddrLabelArray task will find 
-      // to expand, add more elements to this array and initialize them to zero (also initilaize them to zero at the start of the next test)
+      // the addr of each label and fill the array. To expand, add more elements to this array 
+      // and initialize them to zero (also initilaize them to zero at the start of the next test)
       if(!`FPGA) begin
         updateProgramAddrLabelArray(ProgramAddrMapFile, ProgramLabelMapFile, ProgramAddrLabelArray);
         $display("Read memfile %s", memfilename);
@@ -311,8 +313,10 @@ module testbench;
           #600; // give time for instructions in pipeline to finish
           if (TEST == "embench") begin
             // Writes contents of begin_signature to .sim.output file
-            // this contains instret and cycles for start and end of test run, used by embench python speed script to calculate embench speed score
+            // this contains instret and cycles for start and end of test run, used by embench 
-            // also begin_signature contains the results of the self checking mechanism, which will be read by the python script for error checking
+            // python speed script to calculate embench speed score. 
+            // also, begin_signature contains the results of the self checking mechanism, 
+            // which will be read by the python script for error checking
             $display("Embench Benchmark: %s is done.", tests[test]);
             if (riscofTest) outputfile = {pathname, tests[test], "/ref/ref.sim.output"};
             else outputfile = {pathname, tests[test], ".sim.output"};
@@ -366,15 +370,14 @@ module testbench;
                 errors = errors+1;
                 $display("  Error on test %s result %d: adr = %h sim (D$) %h sim (DTIM_SUPPORTED) = %h, signature = %h", 
 						    tests[test], i, (testadr+i)*(`XLEN/8), DCacheFlushFSM.ShadowRAM[testadr+i], sig, signature[i]);
-				$stop;//***debug
+                $stop; //***debug
               end
               i = i + 1;
             end
             /* verilator lint_on INFINITELOOP */
             if (errors == 0) begin
               $display("%s succeeded.  Brilliant!!!", tests[test]);
-			end
+            end else begin
-			else begin
               $display("%s failed with %d errors. :(", tests[test], errors);
               totalerrors = totalerrors+1;
             end
@@ -385,8 +388,7 @@ module testbench;
           if (totalerrors == 0) $display("SUCCESS! All tests ran without failures.");
           else $display("FAIL: %d test programs had errors", totalerrors);
           $stop;
-          end
+          end else begin
-          else begin
             InitializingMemories = 1;
             // If there are still additional tests to run, read in information for the next test
             //pathname = tvpaths[tests[0]];
@@ -480,10 +482,9 @@ module testbench;
       assign EndSample = DCacheFlushStart & ~DCacheFlushDone;
 
       flop #(1) BeginReg(clk, StartSampleFirst, BeginDelayed);
-	  assign Begin = StartSampleFirst & ~BeginDelayed;
+      assign BeginSample = StartSampleFirst & ~BeginDelayed;
 
     end
-	
     always @(negedge clk) begin
       if(StartSample) begin
         for(HPMCindex = 0; HPMCindex < 32; HPMCindex += 1) begin
@@ -552,14 +553,15 @@ module testbench;
         end
       end
     end
-end
+  end
 
 
   if (`ICACHE_SUPPORTED && `I_CACHE_ADDR_LOGGER) begin : ICacheLogger
     int    file;
     string LogFile;
     logic  resetD, resetEdge;
-    logic  Enable, InvalDelayed;
+    logic  Enable;
+    logic  InvalDelayed, InvalEdge;
     
     assign Enable = dut.core.ifu.bus.icache.icache.cachefsm.LRUWriteEn & 
                     dut.core.ifu.immu.immu.pmachecker.Cacheable &
@@ -581,7 +583,7 @@ end
                            dut.core.ifu.bus.icache.icache.vict.cacheLRU.AllValid ? "E" : "M";
     always @(posedge clk) begin
     if(resetEdge) $fwrite(file, "TRAIN\n");
-	  if(Begin) $fwrite(file, "BEGIN %s\n", memfilename);
+    if(BeginSample) $fwrite(file, "BEGIN %s\n", memfilename);
     if(Enable) begin  // only log i cache reads
       $fwrite(file, "%h R %s\n", dut.core.ifu.PCPF, HitMissString);
     end
@@ -621,7 +623,7 @@ end
     end
     always @(posedge clk) begin
       if(resetEdge) $fwrite(file, "TRAIN\n");
-	  if(Begin) $fwrite(file, "BEGIN %s\n", memfilename);
+      if(BeginSample) $fwrite(file, "BEGIN %s\n", memfilename);
       if(Enabled) begin
         $fwrite(file, "%h %s %s\n", dut.core.lsu.PAdrM, AccessTypeString, HitMissString);
       end
@@ -656,7 +658,7 @@ end
     end
   end
 
-  // check for hange up.
+  // check for hang up.
   logic [`XLEN-1:0] OldPCW;
   integer           WatchDogTimerCount;
   localparam        WatchDogTimerThreshold = 1000000;
@@ -806,8 +808,7 @@ task automatic updateProgramAddrLabelArray;
       integer returncode;
       returncode = $fscanf(ProgramLabelMapFP, "%s\n", label);
       returncode = $fscanf(ProgramAddrMapFP, "%s\n", adrstr);
-      if (ProgramAddrLabelArray.exists(label)) 
+      if (ProgramAddrLabelArray.exists(label)) ProgramAddrLabelArray[label] = adrstr.atohex();
-        ProgramAddrLabelArray[label] = adrstr.atohex();
     end
   end
   $fclose(ProgramLabelMapFP);

tests/coverage/priv.S

@@ -143,6 +143,65 @@ main:
     csrw frm, t0
     csrw fflags, t0
     
+    # CSRC MCOUNTEREN Register
+    # Go to machine mode
+    li a0, 3
+    ecall
+    # Activate HPM3
+    li t0, -1
+    csrw mcounteren, t0
+    csrw scounteren, t0
+
+    # Go to supervisor
+    li a0, 1
+    ecall
+    #try to write to HPMs
+    csrw 333, t0
+    #go to user mode
+    li a0, 0
+    ecall
+    csrr t0, hpmcounter22
+
+    # setting registers bits to 0
+    li a0, 3 # back to machine mode
+    ecall
+    li t0, 0
+    csrw mcounteren, t0
+    csrw scounteren, t0
+
+    # Write to satp when status.TVM is 1 from machine mode
+    bseti t0, zero, 20 
+    csrs mstatus, t0
+
+    csrw satp, t0
+
+
+
+    # Test checking privilege for reading counters (using counter 22 as an example)
+
+    # Go to machine mode
+    li a0, 3
+    ecall
+
+    # Set SCOUNTEREN to all 0s, MCOUNTEREN to all 1s
+    li t0, 0
+    csrw scounteren, t0
+    li t1, -1
+    csrw mcounteren, t1
+
+    # Go to supervisor mode
+    li a0, 1
+    ecall
+
+    # try to read from HPM22
+    csrr t0, hpmcounter22
+
+    # go to user mode
+    li a0, 0
+    ecall
+
+    csrr t0, hpmcounter22
+
     j done