From 9ef85c547b35c0cdae84631e5efbd5a1e3255cab Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Mon, 17 Apr 2023 14:12:58 -0700 Subject: [PATCH 1/5] fix unhit exclusion in fdivsqrtfsm --- sim/coverage-exclusions-rv64gc.do | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sim/coverage-exclusions-rv64gc.do b/sim/coverage-exclusions-rv64gc.do index 754d57db6..41345e6e6 100644 --- a/sim/coverage-exclusions-rv64gc.do +++ b/sim/coverage-exclusions-rv64gc.do @@ -35,7 +35,7 @@ do GetLineNum.do coverage exclude -srcfile lzc.sv # FDIVSQRT has -coverage exclude -scope /core/fpu/fpu/fdivsqrt/fdivsqrtfsm -ftrans state DONE->BUSY +coverage exclude -scope /dut/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) From cd803bfa443c50739a363028448adbaf72375460 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 19 Apr 2023 01:19:25 -0700 Subject: [PATCH 2/5] Cover CacheWay edge case: CacheDataMem we=1 while ce=0. This test basically triggers an i$ miss during a d$ (hit) store operation. It requires some tricky timing (e.g. a flushD right before the relevant store). I use a script to generate the test. --- testbench/tests.vh | 3 +- tests/coverage/dcache1.S | 83 +++++++++++++++++++++++++++++++++++++ tests/coverage/dcache1.py | 86 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 tests/coverage/dcache1.S create mode 100644 tests/coverage/dcache1.py diff --git a/testbench/tests.vh b/testbench/tests.vh index 6a0f80276..d2b8a9347 100644 --- a/testbench/tests.vh +++ b/testbench/tests.vh @@ -53,7 +53,8 @@ string tvpaths[] = '{ "lsu", "vm64check", "pmp", - "tlbKP" + "tlbKP", + "dcache1", }; string coremark[] = '{ diff --git a/tests/coverage/dcache1.S b/tests/coverage/dcache1.S new file mode 100644 index 000000000..4a9b3de15 --- /dev/null +++ b/tests/coverage/dcache1.S @@ -0,0 +1,83 @@ + #include "WALLY-init-lib.h" +main: + // start way test #1 + li t0, 0x80100000 +.align 6 + // i$ boundary, way test #1 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + sd zero, 0(t0) + sd zero, 0(t0) + .word 0x00000013 + .word 0x00000013 + // start way test #2 + li t0, 0x80101000 +.align 6 + // i$ boundary, way test #2 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + sd zero, 0(t0) + sd zero, 0(t0) + .word 0x00000013 + .word 0x00000013 + // start way test #3 + li t0, 0x80102000 +.align 6 + // i$ boundary, way test #3 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + sd zero, 0(t0) + sd zero, 0(t0) + .word 0x00000013 + .word 0x00000013 + // start way test #4 + li t0, 0x80103000 +.align 6 + // i$ boundary, way test #4 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + .word 0x00000013 + sd zero, 0(t0) + sd zero, 0(t0) + .word 0x00000013 + .word 0x00000013 + j done diff --git a/tests/coverage/dcache1.py b/tests/coverage/dcache1.py new file mode 100644 index 000000000..59259567b --- /dev/null +++ b/tests/coverage/dcache1.py @@ -0,0 +1,86 @@ +#################### +# dcache1.py +# +# Written: avercruysse@hmc.edu 18 April 2023 +# +# Purpose: Test Coverage for D$ +# (For each way, trigger a CacheDataMem write enable while chip enable is low) +# +# A component of the CORE-V-WALLY configurable RISC-V project. +# +# Copyright (C) 2021-23 Harvey Mudd College & Oklahoma State University +# +# SPDX-License-Identifier: Apache-2.0 WITH SHL-2.1 +# +# Licensed under the Solderpad Hardware License v 2.1 (the “License”); you may not use this file +# except in compliance with the License, or, at your option, the Apache License version 2.0. You +# may obtain a copy of the License at +# +# https://solderpad.org/licenses/SHL-2.1/ +# +# Unless required by applicable law or agreed to in writing, any work distributed under the +# License is distributed on an “AS IS” BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +# either express or implied. See the License for the specific language governing permissions +# and limitations under the License. +################################################ + +import os + +test_name = "dcache1.S" +dcache_num_ways = 4 +dcache_way_size_in_bytes = 4096 +# warning i$ line size is not currently parameterized. + +# arbitrary start location of where I send stores to. +mem_start_addr = 0x80100000 + +# pointer to the start of unused memory (strictly increasing) +mem_addr = mem_start_addr + + +def wl(line="", comment=None, fname=test_name): + with open(fname, "a") as f: + instr = False if (":" in line or + ".align" in line or + "# include" in line) else True + indent = 6 if instr else 0 + comment = "// " + comment if comment is not None else "" + to_write = " " * indent + line + comment + "\n" + f.write(to_write) + + +def write_repro_instrs(): + """ + Assumes that the store location has been fetched to d$, and is in t0. + """ + for i in range(16): # write a whole cache set. + if i == 12: + wl('sd zero, 0(t0)') # D$ write to set PCM = PCF + 8 for proper alignment (stallD will happen). + elif i == 13: + # the store in question happens here, at adresses 0x34, 0x74 + wl('sd zero, 0(t0)') # it should hit this time + else: + # can't be a NOP or anything else that is encoded as compressed. + # this is because the branch predictor will use the wrong address + # so the IFU cache miss will come late. + wl('.word 0x00000013') # addi x0, x0, 0 (canonical NOP, uncompressed). + +if __name__ == "__main__": + if os.path.exists(test_name): + os.remove(test_name) + # os.rename(test_name, test_name + ".old") + wl(comment="This file is generated by dcache1.py (run that script manually)") + wl('#include "WALLY-init-lib.h"') + wl('main:') + + # excercise all 4 D$ ways. If they're not all full, it uses the first empty. + # So we are sure all 4 ways are exercised. + for i in range(dcache_num_ways): + wl(comment=f"start way test #{i+1}") + wl(f'li t0, {hex(mem_addr)}') + wl(f'.align 6') # start at i$ set boundary. 6 lsb bits are zero. + wl(comment=f"i$ boundary, way test #{i+1}") + write_repro_instrs() + mem_addr += dcache_way_size_in_bytes # so that we excercise a new D$ way. + + wl("j done") From b3a3af8ed364919db5b4c13fca6d66fde2e5d904 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 19 Apr 2023 01:21:57 -0700 Subject: [PATCH 3/5] add D$ test case to trigger a FlushStage while SetDirtyWay=1 This hits some conditional coverage in each cacheway. A cache store hit happens at the same time as a StoreAmoMisalignedFault. --- testbench/tests.vh | 1 + tests/coverage/dcache2.S | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 tests/coverage/dcache2.S diff --git a/testbench/tests.vh b/testbench/tests.vh index d2b8a9347..fd48d6dc5 100644 --- a/testbench/tests.vh +++ b/testbench/tests.vh @@ -55,6 +55,7 @@ string tvpaths[] = '{ "pmp", "tlbKP", "dcache1", + "dcache2" }; string coremark[] = '{ diff --git a/tests/coverage/dcache2.S b/tests/coverage/dcache2.S new file mode 100644 index 000000000..58f97a2e4 --- /dev/null +++ b/tests/coverage/dcache2.S @@ -0,0 +1,49 @@ +/////////////////////////////////////////// +// dcache2.S +// +// Written: avercruysse@hmc.edu 18 April 2023 +// +// Purpose: Test Coverage for D$ +// (for all 4 cache ways, trigger a FlushStage while SetDirtyWay=1) +// +// A component of the CORE-V-WALLY configurable RISC-V project. +// +// Copyright (C) 2021-23 Harvey Mudd College & Oklahoma State University +// +// SPDX-License-Identifier: Apache-2.0 WITH SHL-2.1 +// +// Licensed under the Solderpad Hardware License v 2.1 (the “License”); you may not use this file +// except in compliance with the License, or, at your option, the Apache License version 2.0. You +// may obtain a copy of the License at +// +// https://solderpad.org/licenses/SHL-2.1/ +// +// Unless required by applicable law or agreed to in writing, any work distributed under the +// License is distributed on an “AS IS” BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +// either express or implied. See the License for the specific language governing permissions +// and limitations under the License. +//////////////////////////////////////////////////////////////////////////////////////////////// + +#include "WALLY-init-lib.h" +main: + // way 0 + li t0, 0x80100770 + sd zero, 0(t0) + sd zero, 1(t0) + + // way 1 + li t0, 0x80101770 + sd zero, 0(t0) + sd zero, 1(t0) + + // way 2 + li t0, 0x80102770 + sd zero, 0(t0) + sd zero, 1(t0) + + // way 3 + li t0, 0x80103770 + sd zero, 0(t0) + sd zero, 1(t0) + + j done From de93bd6937d31362fc2ef286c6dc83037cd1d436 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 19 Apr 2023 01:28:45 -0700 Subject: [PATCH 4/5] D$ scope-specific coverage exclusions (I$ logic that never fires) The InvalidateCache signal in the D$ is for I$ only, which causes some coverage issues that need exclusion. Another manual exclusion is due to the fact that D$ writeback, flush, write_line, or flush_writeback states can't be cancelled by a flush, so those transistions are excluded. There is some other small stuff to review (logic simplification, or an exclusion pragma if removing the redundent logic would make it harder to understand the code, as is the case in the FlushAdrCntEn assign statement, in my opinion). --- sim/coverage-exclusions-rv64gc.do | 15 ++++++++++++++- src/cache/cachefsm.sv | 8 +++++--- src/cache/cacheway.sv | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/sim/coverage-exclusions-rv64gc.do b/sim/coverage-exclusions-rv64gc.do index 41345e6e6..38c04231c 100644 --- a/sim/coverage-exclusions-rv64gc.do +++ b/sim/coverage-exclusions-rv64gc.do @@ -52,7 +52,7 @@ 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: cache AnyMiss"] -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 @@ -77,6 +77,19 @@ 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 SetValidEN"] -item e 1 -fecexprrow 4 } +## D$ Exclusions. +# InvalidateCache is I$ only: +coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: dcache InvalidateCheck"] -item b 2 +coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: dcache InvalidateCheck"] -item s 1 +coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: dcache CacheEn"] -item e 1 -fecexprrow 12 +coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/cachefsm -linerange [GetLineNum ../src/cache/cachefsm.sv "exclusion-tag: cache AnyMiss"] -item e 1 -fecexprrow 4 +set numcacheways 4 +for {set i 0} {$i < $numcacheways} {incr i} { + coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/CacheWays[$i] -linerange [GetLineNum ../src/cache/cacheway.sv "exclusion-tag: dcache invalidateway"] -item be 1 -fecexprrow 4 +} +# D$ writeback, flush, write_line, or flush_writeback states can't be cancelled by a flush +coverage exclude -scope /dut/core/lsu/bus/dcache/dcache/cachefsm -ftrans CurrState STATE_WRITEBACK->STATE_READY STATE_FLUSH->STATE_READY STATE_WRITE_LINE->STATE_READY STATE_FLUSH_WRITEBACK->STATE_READY + # Excluding peripherals as sources of instructions for the ifu coverage exclude -scope /dut/core/ifu/immu/immu/pmachecker/adrdecs/clintdec diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index 90d8eaad8..7cd8240c4 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -110,10 +110,10 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( always_comb begin NextState = STATE_READY; case (CurrState) // exclusion-tag: icache state-case - STATE_READY: if(InvalidateCache) NextState = STATE_READY; + STATE_READY: if(InvalidateCache) NextState = STATE_READY; // exclusion-tag: dcache InvalidateCheck else if(FlushCache & ~READ_ONLY_CACHE) NextState = STATE_FLUSH; else if(AnyMiss & (READ_ONLY_CACHE | ~LineDirty)) NextState = STATE_FETCH; // exclusion-tag: icache FETCHStatement - else if(AnyMiss & LineDirty) NextState = STATE_WRITEBACK; // exclusion-tag: icache WRITEBACKStatement + 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; @@ -160,6 +160,8 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( assign SelFlush = (CurrState == STATE_READY & FlushCache) | (CurrState == STATE_FLUSH) | (CurrState == STATE_FLUSH_WRITEBACK); + // coverage off -item e -fecexprrow 1 + // (state is always FLUSH_WRITEBACK when FlushWayFlag & CacheBusAck) assign FlushAdrCntEn = (CurrState == STATE_FLUSH_WRITEBACK & FlushWayFlag & CacheBusAck) | (CurrState == STATE_FLUSH & FlushWayFlag & ~LineDirty); assign FlushWayCntEn = (CurrState == STATE_FLUSH & ~LineDirty) | @@ -181,6 +183,6 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( (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; + assign CacheEn = (~Stall | FlushCache | AnyMiss) | (CurrState != STATE_READY) | reset | InvalidateCache; // exclusion-tag: dcache CacheEn endmodule // cachefsm diff --git a/src/cache/cacheway.sv b/src/cache/cacheway.sv index 79ec65e64..368c7b587 100644 --- a/src/cache/cacheway.sv +++ b/src/cache/cacheway.sv @@ -155,7 +155,7 @@ module cacheway #(parameter NUMLINES=512, LINELEN = 256, TAGLEN = 26, if (reset) ValidBits <= #1 '0; if(CacheEn) begin ValidWay <= #1 ValidBits[CacheSet]; - if(InvalidateCache) ValidBits <= #1 '0; + if(InvalidateCache) ValidBits <= #1 '0; // exclusion-tag: dcache invalidateway else if (SetValidEN) ValidBits[CacheSet] <= #1 SetValidWay; end end From faaf26655861423bb07180b21573f82217146c99 Mon Sep 17 00:00:00 2001 From: Alec Vercruysse Date: Wed, 19 Apr 2023 01:32:43 -0700 Subject: [PATCH 5/5] CacheFSM logic simplification for AMO operations Ran this by Ross. --- src/cache/cachefsm.sv | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cache/cachefsm.sv b/src/cache/cachefsm.sv index 7cd8240c4..34f1778f5 100644 --- a/src/cache/cachefsm.sv +++ b/src/cache/cachefsm.sv @@ -69,7 +69,7 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( ); logic resetDelay; - logic AMO, StoreAMO; + logic StoreAMO; logic AnyUpdateHit, AnyHit; logic AnyMiss; logic FlushFlag; @@ -86,16 +86,15 @@ module cachefsm #(parameter READ_ONLY_CACHE = 0) ( statetype CurrState, NextState; - assign AMO = CacheAtomic[1] & (&CacheRW); - assign StoreAMO = AMO | CacheRW[0]; + assign StoreAMO = CacheRW[0]; // AMO operations assert CacheRW[0] - assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; // exclusion-tag: icache storeAMO + assign AnyMiss = (StoreAMO | CacheRW[1]) & ~CacheHit & ~InvalidateCache; // exclusion-tag: cache AnyMiss assign AnyUpdateHit = (StoreAMO) & CacheHit; // exclusion-tag: icache storeAMO1 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; // exclusion-tag: icache CacheW + assign CacheAccess = (|CacheRW) & CurrState == STATE_READY; // exclusion-tag: icache CacheW assign CacheMiss = CacheAccess & ~CacheHit; // special case on reset. When the fsm first exists reset the