cva6 icon indicating copy to clipboard operation
cva6 copied to clipboard

[BUG] Too many ways of configuring Spike Tandem wrt. CV-X-IF verification

Open zchamski opened this issue 9 months ago • 1 comments

Is there an existing CVA6 bug for this?

  • [x] I have searched the existing bug issues

Bug Description

After the merge of openhwgroup/core-v-verif#2580, the 64b vcs-testharness and questa-testharness smoke tests are broken in CI. The direct cause of the failures is the inconsistency between RTL and Spike configurations. However, the root cause is that Spike is being configured in multiple ways and following inconsistent policies.

Background

The verification of the CV-X-IF is based on the execution of instructions which are not recognized by the CVA6 core and are delegated instead to a fake co-processor located behind the CV-X-IF interface (see the list in https://github.com/openhwgroup/cva6/blob/master/verif/env/uvme/cvxif_vseq/custom_instructions_cvxif_1_0_0.rst).

These instructions are defined for CV-X-IF verification purposes only and are implemented outside the base CVA6 core in two ways:

  • reference model: in Spike as a custom extension cvxif that provides a dedicated ISA extension Xcvxif;*
  • SystemVerilog: in CV-X-IF agent (UVM testbenches only).

Some of these instructions reuse opcodes of floating point instructions and therefore, the verification of the CV-X-IF (as currently defined) conflicts with the presence of the FPU presence in target core configuration.

The issue

Tests of FPU-enabled targets fail if Spike extension cvxif is activated. This Spike extension should only be activated for target configurations without FPU and preferably only when verifying CV-X-IF.

Analysis

Currently, the cvxif extension of Spike can be activated in four different ways:

  • on the command line (--extension=cvxif[,...]), in solo mode only;
  • in the Spike Yaml file as explicit custom extension (/top/core/<id>/extensions: cvxif[,...] or /top/cores/extensions: cvxif[,....]), in solo and tandem mode;
  • in the Spike Yaml file as part of the ISA string (..._xcvxif...), in solo and tandem mode;
  • when setting Spike default params (before Spike reads the Yaml config file) in tandem mode only, taking precedence over the Yaml file setting.

All four ways are currently being used, causing unexpected behaviors.

On the SystemVerilog side, the processing of CV-X-IF verification instructions in the dedicated UVM agent is enabled by adding the plus-arg +enabled_cvxif to the command line of the simulation. No support of CV-X-IF verification instructions is available in *-testharness configurations and hence, the Spike cvxif extension shall not be activated for these configurations.

Required changes

As of the merge point of openhwgroup/core-v-verif#2580, the following needs to be modified (list not yet exhaustive):

  1. [core-v-verif] Do not append _xcvxif to ISA string passed to Spike tandem when the CV-X-IF is enabled in RTL (function get_isa_str() in uvma_core_cntrl_utils.sv).
  2. [cva6] Do not force-add --extension=cvxif to Spike solo command line for target configurations that do NOT have an associated Spike Yaml config file under config/gen_from_riscv_config/<config_name>/spike/spike.yaml (see https://github.com/openhwgroup/cva6/blob/master/verif/sim/Makefile).
  3. [cva6] Do not force the extensions: field to contain cvxif in *-testharness testbench code (verif/tb/core/uvma_cva6pkg_utils.sv).
  4. [cva6] Activate the cvxif extension in Spike Yaml configuration file for target configurations that require CV-X-IF verification and do not include an FPU. Prefer to use the ISA approach (append _xvcxif to ISA string) since the ISA is effectively modified and ISA checks of the simultaneous presence of Xcvxif and F/D extensions are systematically performed.
  5. [cva6] Create Spike Yaml configuration files for all CVA6 target configurations.
  6. [cva6] Remove explicit setting of Spike default params from SystemVerilog code for all CVA6 configurations.

Limiting the removal of SV features to CVA6 configurations will preserve the existing way of operation for CVE2.

zchamski avatar Feb 12 '25 16:02 zchamski