From e3593800d989d736e79dd705225e74caff2a8d08 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 754d57db..41345e6e 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 cd9feb02603c7de65c2c9b7dc6ef6f375b6e9a5a 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 6a0f8027..d2b8a934 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 00000000..4a9b3de1 --- /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 00000000..59259567 --- /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 3de03abd9df709e7f8e1cd6dce197ccdaa22f01e 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 d2b8a934..fd48d6dc 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 00000000..58f97a2e --- /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 b52512b1ae0815dbb8c72cd4a485d6dc9107df00 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 41345e6e..38c04231 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 90d8eaad..7cd8240c 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 79ec65e6..368c7b58 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 7ba2bfd4b62dde1b22ee98caa462718d27052799 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 7cd8240c..34f1778f 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