Improving the priv func cov flow to run with --fcovpriv flag

bin/regression-wally

@@ -366,7 +366,7 @@ defaultsim = "verilator"   # Default simulator for all other tests
 parser = argparse.ArgumentParser()
 parser.add_argument("--ccov", help="Code Coverage", action="store_true")
 parser.add_argument("--fcov", help="Functional Coverage", action="store_true")
-parser.add_argument("--fcovrvvi", help="Functional Coverage RVVI", action="store_true")
+parser.add_argument("--fcovpriv", help="Privilege Functional Coverage", action="store_true")
 parser.add_argument("--nightly", help="Run large nightly regression", action="store_true")
 parser.add_argument("--buildroot", help="Include Buildroot Linux boot test (takes many hours, done along with --nightly)", action="store_true")
 parser.add_argument("--testfloat", help="Include Testfloat floating-point unit tests", action="store_true")
@@ -385,8 +385,8 @@ if (args.ccov):  # only run RV64GC tests in coverage mode
     coverStr = '--ccov'
 elif (args.fcov):  # only run RV64GC tests in lockstep in coverage mode
     coverStr = '--fcov'
-elif (args.fcovrvvi):  # only run RV64GC tests in rvvi coverage mode
-    coverStr = '--fcovrvvi'
+elif (args.fcovrvvi):  # only run RV64GC tests in lockstep in coverage mode
+    coverStr = '--fcovpriv'
 else:
    coverStr = ''
 
@@ -412,14 +412,10 @@ if (args.ccov):  # only run RV64GC tests on Questa in code coverage mode
     addTests(tests64gc_nofp, coveragesim)
     if (args.fp):
         addTests(tests64gc_fp, coveragesim)
-elif (args.fcov):  # only run RV64GC tests on Questa in lockstep in functional coverage mode
+elif (args.fcov or args.fcovpriv):  # only run RV64GC tests on Questa in lockstep in functional coverage mode
     addLockstepTestsByDir(WALLY+"/addins/cvw-arch-verif/tests/rv32/", "rv32gc", coveragesim, 1)
     addLockstepTestsByDir(WALLY+"/addins/cvw-arch-verif/tests/rv64/", "rv64gc", coveragesim, 1)
     # addLockstepTestsByDir(WALLY+"/tests/riscof/work/wally-riscv-arch-test/rv64i_m/privilege/src/", "rv64gc", coveragesim, 0)
-elif (args.fcovrvvi):  # only run RV64GC tests on Questa in rvvi coverage mode
-    addTests(tests64gc_nofp, coveragesim)
-    if (args.fp):
-        addTests(tests64gc_fp, coveragesim)
 else: 
     for sim in sims:
         if (not (args.buildroot and sim == defaultsim)):  # skip short buildroot sim if running long one
@@ -511,10 +507,7 @@ def main():
     if args.ccov:
         TIMEOUT_DUR = 20*60 # seconds
         os.system('rm -f questa/cov/*.ucdb')
-    elif args.fcovrvvi:
-        TIMEOUT_DUR = 20*60
-        os.system('rm -f questa/fcovrvvi_ucdb/* questa/fcovrvvi_logs/* questa/fcovrvvi/*')
-    elif args.fcov:
+    elif args.fcov or args.fcovpriv:
         TIMEOUT_DUR = 2*60
         os.system('rm -f questa/fcov_ucdb/* questa/fcov_logs/* questa/fcov/*')
     elif args.buildroot:
@@ -529,7 +522,7 @@ def main():
     # Scale the number of concurrent processes to the number of test cases, but
     # max out at a limited number of concurrent processes to not overwhelm the system
     # right now fcov, ccov, nightly all use Imperas
-    if (args.ccov or args.fcov or args.nightly):
+    if (args.ccov or args.fcov or args.fcovpriv or args.nightly):
         ImperasDVLicenseCount = 8 # limit number of concurrent processes to avoid overloading ImperasDV licenses
     else:
         ImperasDVLicenseCount = 10000 # effectively no license limit for non-lockstep tests
@@ -548,10 +541,8 @@ def main():
     # Coverage report
     if args.ccov:
        os.system('make QuestaCodeCoverage')
-    if args.fcov:
+    if args.fcov or args.fcovpriv:
        os.system('make -f '+WALLY+'/addins/cvw-arch-verif/Makefile merge')
-    if args.fcovrvvi:
-        os.system('make QuestaFunctCoverageRvvi')
     # Count the number of failures
     if num_fail:
         print(f"{bcolors.FAIL}Regression failed with %s failed configurations{bcolors.ENDC}" % num_fail)

bin/wsim

@@ -29,7 +29,7 @@ parser.add_argument("--gui", "-g", help="Simulate with GUI", action="store_true"
 parser.add_argument("--ccov", "-c", help="Code Coverage", action="store_true")
 parser.add_argument("--fcovimp", "-f2", help="Functional Coverage with Imperas licensed riscvISACOV, implies lockstep", action="store_true")
 parser.add_argument("--fcov", "-f", help="Functional Coverage with cvw-arch-verif, implies lockstep", action="store_true")
-parser.add_argument("--fcovrvvi", "-fr", help="Functional Coverage RVVI", action="store_true")
+parser.add_argument("--fcovpriv", "-fpriv", help="Privileged Functional Coverage with cvw-arch-verif, implies lockstep", action="store_true")
 parser.add_argument("--args", "-a", help="Optional arguments passed to simulator via $value$plusargs", default="")
 parser.add_argument("--params", "-p", help="Optional top-level parameter overrides of the form param=value", default="")
 parser.add_argument("--vcd", "-v", help="Generate testbench.vcd", action="store_true")
@@ -71,7 +71,7 @@ if(args.lockstep and not args.testsuite.endswith('.elf')):
     exit(1)
 
 # Validate arguments
-if (args.gui or args.ccov or args.fcov or args.fcovimp or args.fcovrvvi or args.lockstep or args.lockstepverbose):
+if (args.gui or args.ccov or args.fcov or args.fcovimp or args.fcovpriv or args.lockstep or args.lockstepverbose):
     if args.sim not in ["questa", "vcs"]:
         print("Option only supported for Questa and VCS")
         exit(1)
@@ -89,7 +89,7 @@ if (args.tb == "testbench_fp"):
 if(int(args.locksteplog) >= 1): EnableLog = 1
 else: EnableLog = 0
 prefix = ""
-if (args.lockstep or args.lockstepverbose or args.fcov or args.fcovimp):
+if (args.lockstep or args.lockstepverbose or args.fcov or args.fcovimp or args.fcovpriv):
    if (args.sim == "questa" or args.sim == "vcs"):
         prefix = "IMPERAS_TOOLS=" + WALLY + "/config/"+args.config+"/imperas.ic"
    if (args.sim == "questa"):
@@ -104,7 +104,7 @@ if (args.lockstep or args.lockstepverbose):
         else: EnableLog = 0
         ImperasPlusArgs = " +IDV_TRACE2COV=" + str(EnableLog) + " +TRACE2LOG_AFTER=" + str(args.covlog) + " +TRACE2COV_ENABLE=" + CovEnableStr
         suffix = ""
-    if(args.fcov):
+    if(args.fcov or args.fcovpriv):
         CovEnableStr = "1" if int(args.covlog) > 0  else "0";
         if(args.covlog >= 1): EnableLog = 1
         else: EnableLog = 0
@@ -129,12 +129,12 @@ if (args.fcov):
     flags += " --fcov"
 if (args.fcovimp):
     flags += " --fcovimp"
-if (args.fcovrvvi):
-    flags += "--fcovrvvi"
+if (args.fcovpriv):
+    flags += "--fcovpriv"
 
 #  create the output sub-directories.
 regressionDir = WALLY + '/sim/'
-for d in ["logs", "wkdir", "cov", "ucdb", "fcov", "fcov_ucdb", "fcovrvvi", "fcovrvvi_ucdb"]:
+for d in ["logs", "wkdir", "cov", "ucdb", "fcov", "fcov_ucdb"]:
     try:
         os.mkdir(regressionDir+args.sim+"/"+d)
     except:

config/rv64gc/coverage.svh

@@ -9,11 +9,11 @@
 `include "RV64M_coverage.svh"
 `include "RV64F_coverage.svh"
 `include "RV64Zfh_coverage.svh"
-// `include "RV64VM_coverage.svh"
-// `include "RV64VM_PMP_coverage.svh"
-// `include "RV64CBO_VM_coverage.svh"
-// `include "RV64CBO_PMP_coverage.svh"
-// `include "RV64Zicbom_coverage.svh"
+`include "RV64VM_coverage.svh"
+`include "RV64VM_PMP_coverage.svh"
+`include "RV64CBO_VM_coverage.svh"
+`include "RV64CBO_PMP_coverage.svh"
+`include "RV64Zicbom_coverage.svh"
 `include "RV64Zicond_coverage.svh"
 `include "RV64Zca_coverage.svh"
 `include "RV64Zcb_coverage.svh"

sim/Makefile

@@ -27,7 +27,7 @@ deriv:
 
 
 
-.PHONY: QuestaCodeCoverage QuestaFunctCoverageRvvi collect_functcov combine_functcov remove_functcov_artifacts riscvdv riscvdv_functcov
+.PHONY: QuestaCodeCoverage collect_functcov combine_functcov remove_functcov_artifacts riscvdv riscvdv_functcov
 
 QuestaCodeCoverage: questa/ucdb/rv64gc_arch64i.ucdb
 	vcover merge -out questa/ucdb/cov.ucdb questa/ucdb/rv64gc_arch64i.ucdb questa/ucdb/rv64gc*.ucdb -logfile questa/cov/log
@@ -51,14 +51,6 @@ QuestaCodeCoverage: questa/ucdb/rv64gc_arch64i.ucdb
 #	vcover report -recursive questa/ucdb/cov.ucdb > questa/cov/rv64gc_recursive.rpt
 	vcover report -details -threshH 100 -html questa/ucdb/cov.ucdb
 
-QuestaFunctCoverageRvvi: ${WALLY}/addins/cvw-arch-verif/work/rv64gc_arch64i.ucdb 
-	vcover merge -out ${SIM}/questa/fcovrvvi_ucdb/fcovrvvi.ucdb ${WALLY}/addins/cvw-arch-verif/work/rv64gc_arch64i.ucdb ${WALLY}/addins/cvw-arch-verif/work/rv64gc_*.ucdb -logfile ${SIM}/questa/fcovrvvi/log
-	vcover report -details -html ${SIM}/questa/fcovrvvi_ucdb/fcovrvvi.ucdb
-	vcover report ${SIM}/questa/fcovrvvi_ucdb/fcovrvvi.ucdb -details -cvg > ${SIM}/questa/fcovrvvi/fcovrvvi.log
-	vcover report ${SIM}/questa/fcovrvvi_ucdb/fcovrvvi.ucdb -testdetails -cvg > ${SIM}/questa/fcovrvvi/fcovrvvi.testdetails.log
-	vcover report ${SIM}/questa/fcovrvvi_ucdb/fcovrvvi.ucdb -details -cvg | egrep "Coverpoint|Covergroup|Cross|TYPE"  > ${SIM}/questa/fcovrvvi/fcovrvvi.summary.log
-	grep "TOTAL COVERGROUP COVERAGE" ${SIM}/questa/fcovrvvi/fcovrvvi.log
-
 collect_functcov: remove_functcov_artifacts riscvdv_functcov combine_functcov
 
 riscvdv_functcov:

sim/questa/wally.do

@@ -51,8 +51,6 @@ if [file exists ${WKDIR}] {
     vdel -lib ${WKDIR} -all
 }
 vlib ${WKDIR}
-# Create directory for functional coverage data
-mkdir -p ${FCRVVI}
 
 set PlusArgs ""
 set ParamArgs ""
@@ -62,10 +60,8 @@ set ccov 0
 set CoverageVoptArg ""
 set CoverageVsimArg ""
 
-set FuncCovRVVI 0
-set FCdefineRVVI_COVERAGE ""
-
 set FunctCoverage 0
+set FCpriv 0
 set FCvlog ""
 set FCvopt ""
 set FCdefineCOVER_EXTS {}
@@ -108,10 +104,18 @@ if {[lcheck lst "--ccov"]} {
     set CoverageVsimArg "-coverage"
 }
 
-# if --fcovrvvi found set flag and remove from list
-if {[lcheck lst "--fcovrvvi"]} {
-    set FuncCovRVVI 1
-    set FCdefineRVVI_COVERAGE "+define+RVVI_COVERAGE"
+# if --fcovpriv found set flag and remove from list
+if {[lcheck lst "--fcovpriv"]} {
+    set FunctCoverage 1
+    set FCpriv 1
+         set FCvlog "+define+INCLUDE_TRACE2COV \
+                +define+IDV_INCLUDE_TRACE2COV \
+                +define+COVER_BASE_RV32I \
+                +define+COVER_PRIV \
+                +incdir+$env(WALLY)/addins/riscvISACOV/source \
+		"
+
+    set FCvopt "+TRACE2COV_ENABLE=1 +IDV_TRACE2COV=1"
 }
 
 # if --fcovimp found set flag and remove from list
@@ -138,11 +142,12 @@ if {[lcheck lst "--fcovimp"]} {
 if {[lcheck lst "--fcov"]} {
     set FunctCoverage 1
     # COVER_BASE_RV32I is just needed to keep riscvISACOV happy, but no longer affects tests
-    set FCvlog "+define+INCLUDE_TRACE2COV \
+         set FCvlog "+define+INCLUDE_TRACE2COV \
                 +define+IDV_INCLUDE_TRACE2COV \
                 +define+COVER_BASE_RV32I \
                 +incdir+$env(WALLY)/addins/riscvISACOV/source \
 		"
+
     set FCvopt "+TRACE2COV_ENABLE=1 +IDV_TRACE2COV=1"
 
 }
@@ -181,7 +186,6 @@ if {$DEBUG > 0} {
     echo "GUI = $GUI"
     echo "ccov = $ccov"
     echo "lockstep = $lockstep"
-    echo "FuncCovRVVI = $FuncCovRVVI"
     echo "FunctCoverage = $FunctCoverage"
     echo "remaining list = $lst"
     echo "Extra +args = $PlusArgs"
@@ -192,9 +196,9 @@ if {$DEBUG > 0} {
 # suppress spurious warnngs about
 # "Extra checking for conflicts with always_comb done at vopt time"
 # because vsim will run vopt
-set INC_DIRS "+incdir+${CONFIG}/${CFG} +incdir+${CONFIG}/deriv/${CFG} +incdir+${CONFIG}/shared +incdir+${FCRVVI} +incdir+${FCRVVI}/rv32 +incdir+${FCRVVI}/rv64 +incdir+${FCRVVI}/rv64_priv +incdir+${FCRVVI}/common +incdir+${FCRVVI}"
+set INC_DIRS "+incdir+${CONFIG}/${CFG} +incdir+${CONFIG}/deriv/${CFG} +incdir+${CONFIG}/shared +incdir+${FCRVVI} +incdir+${FCRVVI}/rv32 +incdir+${FCRVVI}/rv64 +incdir+${FCRVVI}/rv64_priv +incdir+${FCRVVI}/common"
 set SOURCES "${SRC}/cvw.sv ${TB}/${TESTBENCH}.sv ${TB}/common/*.sv ${SRC}/*/*.sv ${SRC}/*/*/*.sv ${WALLY}/addins/verilog-ethernet/*/*.sv ${WALLY}/addins/verilog-ethernet/*/*/*/*.sv"
-vlog -permissive -lint -work ${WKDIR} {*}${INC_DIRS} {*}${FCvlog} {*}${FCdefineCOVER_EXTS} {*}${lockstepvlog} ${FCdefineRVVI_COVERAGE} {*}${SOURCES} -suppress 2282,2583,7053,7063,2596,13286
+vlog -permissive -lint -work ${WKDIR} {*}${INC_DIRS} {*}${FCvlog} {*}${FCdefineCOVER_EXTS} {*}${lockstepvlog} {*}${SOURCES} -suppress 2282,2583,7053,7063,2596,13286
 
 # start and run simulation
 # remove +acc flag for faster sim during regressions if there is no need to access internal signals
@@ -216,13 +220,13 @@ if { ${GUI} } {
 }
 
 if {$FunctCoverage} {
-    set UCDB ${WALLY}/sim/questa/fcov_ucdb/${CFG}_${TESTSUITE}.ucdb
-    coverage save -onexit ${UCDB}
-}
-
-if {$FuncCovRVVI} {
-    set UCDB ${WALLY}/addins/cvw-arch-verif/work/${CFG}_${TESTSUITE}.ucdb
-    coverage save -onexit ${UCDB}
+    if {$FCpriv} {
+        set UCDB ${WALLY}/sim/questa/fcov_ucdb/${CFG}_${TESTSUITE}.priv.ucdb
+        coverage save -onexit ${UCDB}
+    } else {
+        set UCDB ${WALLY}/sim/questa/fcov_ucdb/${CFG}_${TESTSUITE}.ucdb
+        coverage save -onexit ${UCDB}
+    }
 }
 
 run -all