[dv,dvsim,tooling] Questa Support

Open

#24,341 opened on Aug 15, 2024 5 comments 0 reactions 1 assignee Claimed by @hcallahan-lowrisc View on GitHub

Component:DV Component:Tooling Help Wanted Tool:dvsim

Dominant language: SystemVerilog
Stars: 3.6k
Forks: 1.1k
Avg merge: 2d 22h
Merged PRs (30d): 141

Description

### Description

This is a bucket-issue for improving Questa support in OpenTitan.

> I want to use this issue to drive support forwards by gathering user feedback into a working branch of fixes, which can later be merged all at once.
> If you want to help, please [see the steps you can take below!](#how-you-can-help)

### Motive

I would like to get Questa support into a better place across the project.
Questa is a simulator that many people have access to, and a part of maximizing value of the OpenTitan project is to keep the barrier to entry as low as possible for anyone who is interested.
As lowRISC currently does not have access to questa licenses, nor is anyone regularly developing on OpenTitan with it or regressing the DV suite using it, the support there has always been limited and liable to break without notice.
I hope one day we can test our codebase against more simulators and catch breaking changes before they are merged, but up to now it has been a best-effort approach.

### Ctx

Questa support was added initially to our DV test tool 'dvsim' in [#10574 - [hw/dv] Feature/questa dv](https://github.com/lowRISC/opentitan/pull/10574), though I am not sure to what extent it was functional at the time. (i.e. which testbenches could be compiled/simulated, and which were incompatible).
Our signoff simulator for blocks/tops is [primarily](https://opentitan.org/book/doc/getting_started/setup_dv.html#supported-simulators) VCS (for example. see the earlgrey top-level testbench config file [chip_sim_cfg.hjson#L15](https://github.com/lowRISC/opentitan/blob/740308b2d652082415453eaecd75dfb62e92c973/hw/top_earlgrey/dv/chip_sim_cfg.hjson#L15)), although some blocks now use Xcelium for signoff as well.

'dvsim' is extensible, and the support added above allows for invoking simulations using the questa CLI when you use `--tool=questa`.
Fundamentally, dvsim should be able to assemble a valid CLI invocation for any tool it supports, with any number of arguments that are specific to the quirks of any program.
Some parts of this process are quite generic between tools. For example, we use fusesoc .core files to package blocks and components, and dvsim will invoke fusesoc to generate output collateral such as a specific filelist for the simulation/testbench of interest.
The tool-specific file [/hw/dv/tools/dvsim/questa.hjson](https://github.com/lowRISC/opentitan/blob/master/hw/dv/tools/dvsim/questa.hjson) is then responsible for tying-up the generic parts of the flow with the implementation details of a tool.

### Existing Questa Tickets

I did a little search of previous issues and pull requests related to questa support. I've pulled out the ones I think might be useful as a reference here...

#### Issues

https://github.com/lowRISC/opentitan/issues?q=is%3Aissue+questa+

| Ticket | Desc. |
|-------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------|
| OPEN | |
| [#24174 - [questa] Pls update questa.hjson file](https://github.com/lowRISC/opentitan/issues/24174) | Questa support request |
| [#23293 - [siemens] updated questa.hjson](https://github.com/lowRISC/opentitan/issues/23293) | General Questa questions |
| [#23268 - [documentation] generating scripts used to run Jasper Gold](https://github.com/lowRISC/opentitan/issues/23268) | Porting Jasper formal scripts to QuestaFormal |
| [#22755 - gpio_tl_intg_err test failing on Questa](https://github.com/lowRISC/opentitan/issues/22755) | Test passes on Xcelium, but not in Questa |
| [#22243 - [dvsim] Various questions](https://github.com/lowRISC/opentitan/issues/22243) | Questa/CentOS7 support questions, also dvsim/bazel questions |
| [#22217 - [dvgen] generating testbench w/in Ubuntu container but simulating in CentOS](https://github.com/lowRISC/opentitan/issues/22217) | Questa/CentOS7 support questions |
| [#18039 - [dvsim] Riviera/Questa support is broken - around otbn_memutil](https://github.com/lowRISC/opentitan/issues/18039) | Requesting Questa/Riviera support |
| CLOSED | |
| [#22162 - [ci] No module named 'google_verible_verilog_syntax_py (2024)](https://github.com/lowRISC/opentitan/issues/22162) | General questa/bazel support questions |
| [#21518 - [dvsim] Questa uart_smoke failing at tl_agent_pkg load](https://github.com/lowRISC/opentitan/issues/21518) | Questa SV / LRM incompatabilities, not sure a fix was merged but it may have been elsewhere. |
| [#16994 - [aes] aes_core simulation using ModelSim](https://github.com/lowRISC/opentitan/issues/16994) | Request for questa/modelsim support |
| [#9514 - Edalize and dvsim](https://github.com/lowRISC/opentitan/issues/9514) | Discusses some initial SV-incompatabilites before questa.hjson was merged in #10574 |
| [#4541 - [DV] missing has_edn check in cip_base_env.sv?](https://github.com/lowRISC/opentitan/issues/4541) | SV issue found while adding questa support. Fixed. |
| [#4529 - Sim: Illegal %s format specififer for class object](https://github.com/lowRISC/opentitan/issues/4529) | Questa incompatabilites -> fixed by adding '-suppress vsim-8323' in #10574 |
| [#4528 - Sim: Dangling virtual interface - Questa support](https://github.com/lowRISC/opentitan/issues/4528) | Questa SV incompatabilities -> fixed by adding '-permit_unmatched_virtual_intf' in #10574 |
| [#4427 - Sim: tlul_assert: SVA errors - QSTA UART Smoke test](https://github.com/lowRISC/opentitan/issues/4427) | Questa SV LRM incompatabilites in tl_host_driver.sv - closed due to lack of feedback |
| [#4377 - SIM: Questa support: FATAL error from RAL](https://github.com/lowRISC/opentitan/issues/4377) | OpenTitan does not support pre-compiled UVM -> problem found when trying to add Questa build support |
| [#4355 - Sim: Data type mismatch issue](https://github.com/lowRISC/opentitan/issues/4355) | Questa SV Support issue - closed as wontfix, pending vendor feedback on questa LRM interpretation |
| [#4340 - dvsim: QSTA support - sim_tops needs to be moved to run_opts](https://github.com/lowRISC/opentitan/issues/4340) | Some initial design questions about adding dvsim questa support |
| [#4334 - Renaming dir sim-vcs inside fusesoc.hjson](https://github.com/lowRISC/opentitan/issues/4334) | More dvsim+fusesoc oddities than questa |
| [#4332 - dvsim: Questa support - prim_clock_gating.v - is this missing in file-list?](https://github.com/lowRISC/opentitan/issues/4332) | Adding initial questa.hjson |
| [#4153 - EalrGrey Questa support?](https://github.com/lowRISC/opentitan/issues/4153) | Questa support request |

#### PRs

https://github.com/lowRISC/opentitan/pulls?q=is%3Apr+questa+

### Fixing outstanding issues

To get our DV flows working with Questa more broadly, the bulk of the work will be in SV / LRM differences in how tools interpret our codebase. A familiar hurdle for sure!

This process will probably raise a number of issues in dv base classes to begin with, and then follow onto testbench code specific to each block.

Probably the best approach will be to work block-by-block through the individual testbenches, first fixing any build issues, followed by any runtime differences.
The following table is a good starting list of some block level testbenches for non-parameterized blocks, and suggested invocations to test them.
(I've just worked down the list of IP in `hw/ip` to make a start. This list can become a more comprehensive checklist of tested-blocks in the future.)

| block | simulation command |
|-------------------|-----------------------------------------------------------------------------------------------------------|
| adc_ctrl | `./util/dvsim/dvsim.py hw/ip/adc_ctrl/dv/adc_ctrl_sim_cfg.hjson -i adc_ctrl_smoke --tool questa` |
| aes | `./util/dvsim/dvsim.py hw/ip/aes/dv/aes_masked_sim_cfg.hjson -i aes_smoke --tool questa` |
| aon_timer | `./util/dvsim/dvsim.py hw/ip/aon_timer/dv/aon_timer_sim_cfg.hjson -i aon_timer_smoke --tool questa` |
| csrng | `./util/dvsim/dvsim.py hw/ip/csrng/dv/csrng_sim_cfg.hjson -i csrng_smoke --tool questa` |
| edn | `./util/dvsim/dvsim.py hw/ip/edn/dv/edn_sim_cfg.hjson -i edn_smoke --tool questa` |
| entropy_src | `./util/dvsim/dvsim.py hw/ip/entropy_src/dv/entropy_src_sim_cfg.hjson -i entropy_src_smoke --tool questa` |
| gpio | `./util/dvsim/dvsim.py hw/ip/gpio/dv/gpio_sim_cfg.hjson -i gpio_smoke --tool questa` |
| hmac | `./util/dvsim/dvsim.py hw/ip/hmac/dv/hmac_sim_cfg.hjson -i hmac_smoke --tool questa` |
| i2c | `./util/dvsim/dvsim.py hw/ip/i2c/dv/i2c_sim_cfg.hjson -i i2c_host_smoke --tool questa` |
| keymgr | `./util/dvsim/dvsim.py hw/ip/keymgr/dv/keymgr_sim_cfg.hjson -i keymgr_smoke --tool questa` |
| kmac | `./util/dvsim/dvsim.py hw/ip/kmac/dv/kmac_masked_sim_cfg.hjson -i kmac_smoke --tool questa` |

The top-level testbench would eventually be desirable to fix, though I suspect it will be the most work to get there.

| top | simulation command |
|----------------|-----------------------------------------------------------------------------------------------------|
| earlgrey | `./util/dvsim/dvsim.py hw/top_earlgrey/dv/chip_sim_cfg.hjson -i chip_sw_rv_timer_irq --tool questa` |

After running smoketests for each block, probably the next thing would be to run 'all_once' regressions in each block.
E.g. `./util/dvsim/dvsim.py hw//dv/_sim_cfg.hjson -i all_once --tool questa`
This should exercise all of the stimulus vseq's listed in the `sim_cfg.hjson` configuration file.
Stimulus sequences are located in the following directory for each block-level testbench : `hw//dv/env/seq_lib/`.

Our existing convention has been to avoid `#ifdef`s unless absolutely necessary, so work in this area will need to prioritize finding common language constructs the tools agree on.
There are a very small number of `#ifdef XCELIUM` or `#ifdef VCS` switches in the codebase, and adding more of these should be considered a last-resort.
However, I think absorbing changes that use `#ifdef`s in the short-term, tracked in issues with pending vendor support feedback, may be workable.
For the working branch described below, short-term solutions using `#ifdef`s will be acceptable.

### How you can help

To start with, I've created [a draft PR](https://github.com/lowRISC/opentitan/pull/24331) to accumulate a working set of changes to fix bugs as they are found.
As bugs and suggested fixes come in, we can discuss them in this issue, and then propagate them over to that PR.

Steps:
1) Setup the opentitan repository & dependencies by following the instructions in steps 1-4 here : https://opentitan.org/book/doc/getting_started/index.html
- Test the above setup by using bazel to run a verilator simulation : `bazel test --test_output=streamed //sw/device/tests:uart_smoketest_sim_verilator`
- Ensure questa is on your path, and that QUESTA_HOME is set appropriately.
2) Checkout the changes from the working PR : https://github.com/lowRISC/opentitan/pull/24331
3) Run one of the [simulation commands above](#fixing-outstanding-issues), and report back the failure modes in comments on this issue with code/log snippets.
- We can discuss the failure modes to propose fixes or workarounds.
- Fixes will be propagated to the [working PR](https://github.com/lowRISC/opentitan/pull/24331) to unblock further work, with commits/comments refering back to the discussion here.
- If an immediate fix is not obvious, or requires some external feedback, then we can add temporary workarounds to the PR to unblock further work while a better fix is pending.

This will probably take a bit of time to get sorted out, as the feedback loop is inherently manual, and I don't have too many free cycles to put into this.
However, I hope that centralizing the discussions here will keep the process moving forwards, and eventually we can get up to a good parity with the other tools.

Thanks!

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

[dv,dvsim,tooling] Questa Support

Description

Contributor guide

Assessment

Get new issues in your inbox