Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
zchamski opened this issue Feb 12, 2025 · 1 comment
Open
1 task done
Labels
CV32A65X Part: Embedded configuration Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system

Comments

@zchamski
Copy link
Contributor

zchamski commented Feb 12, 2025

Is there an existing CVA6 bug for this?

  • 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 zchamski added the Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system label Feb 12, 2025
@zchamski
Copy link
Contributor Author

Initial distribution list: @ASintzoff @JeanRochCoulon @MikeOpenHWGroup @AyoubJalali @AEzzejjari

@JeanRochCoulon JeanRochCoulon added the CV32A65X Part: Embedded configuration label Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CV32A65X Part: Embedded configuration Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system
Projects
None yet
Development

No branches or pull requests

2 participants