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

Add read-only CSR MCONFIGPTR test #381

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

dansmathers
Copy link
Contributor

Description

mconfigptr is a read-only CSR defined in priv spec 1.12. The test attempts a read/write (illegal instruction exception expected)/read. The read data value is implementation dependent so the read data value is ignored.

Related Issues

Requires sail update for signatures to pass: riscv/sail-riscv#293

Ratified/Unratified Extensions

  • [X ] Ratified
  • Unratified

List Extensions

MCONFIGPTR exists in the priv spec 1.12 and used by the discovery task group.

Reference Model Used

  • SAIL
  • Spike
  • Other - < SPECIFY HERE >

Mandatory Checklist:

  • All tests are compliant with the test-format spec present in this repo ?
  • Ran the new tests on RISCOF with SAIL/Spike as reference model successfully ?
  • Ran the new tests on RISCOF in coverage mode
  • Link to Google-Drive folder containing the new coverage reports (See this for more info): < SPECIFY HERE >
  • Link to PR in RISCV-ISAC from which the reports were generated : < SPECIFY HERE >
  • Changelog entry created with a minor patch

Optional Checklist:

  • RISCV-V CTG PR link if tests were generated using it : < SPECIFY HERE >
  • Were the tests hand-written/modified ?
  • Have you run these on any hard DUT model ? Please specify name and provide link if possible in the description
  • If you have modified arch_test.h Please provide a detailed description of the changes in the Description section above.

mconfigptr is a read-only CSR defined in priv spec 1.12.  The test attempts a read/write (illegal instruction exception expected)/read.  The read data value is implementation dependent so the read data value is ignored.

Signed-off-by: Dan Smathers <[email protected]>
add mconfigptr csr test

Signed-off-by: Dan Smathers <[email protected]>
Copy link
Collaborator

@allenjbaum allenjbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs 2 nops after an op that might trap, because the trap handler always returns to the following double-word aligned instruction (deliberately, so we can write tests that do different things depending on whether a trap occurred or not, in addition to writing a trap signature

@allenjbaum
Copy link
Collaborator

allenjbaum commented Nov 10, 2023

This also needs a coverage definition, or this will fail when riscof runs the coverage command

cmuellner added a commit to cmuellner/riscv-arch-test that referenced this pull request Nov 15, 2023
The {m,s,h}envcfg CSRs have been defined a long time ago.
They contain bits that are defined by a the following ISA extensions:
  Zicbom_Zicboz_Zicsr_Ssdtso_Sstc

This patch introduces a test which attempts to write all of the defined bits
in menvcfg (similar to riscv-non-isa#381).  After the tests, the original value of
the CSR will be restored to avoid side-effects with other tests.

Future commits could introduce the following additional tests:
* write tests for the bits in senvcfg
* write tests for the bits in henvcfg
* test that {m,s,h}envcfg CSRs are only accessible in the relevant modes

Signed-off-by: Christoph Müllner <[email protected]>
cmuellner added a commit to cmuellner/riscv-arch-test that referenced this pull request Nov 17, 2023
The {m,s,h}envcfg CSRs have been defined a long time ago.
They contain bits that are defined by a the following ISA extensions:
  Zicbom_Zicboz_Zicsr_Ssdtso_Sstc

This patch introduces a test which attempts to write all of the defined bits
in menvcfg (similar to riscv-non-isa#381).  After the tests, the original value of
the CSR will be restored to avoid side-effects with other tests.

This commit is incomplete, as it does not test the illegal exceptions
which are triggered on access violations.

Signed-off-by: Christoph Müllner <[email protected]>
@UmerShahidengr
Copy link
Collaborator

This PR is on halt since last 3,4 months. @dansmathers please update the test and add coverpoint definitions so that it can be merged.

@jamesbeyond jamesbeyond changed the base branch from main to dev May 28, 2024 16:58
@jamesbeyond
Copy link
Collaborator

Sorry, removed the CHANGELOG.MD by mistake, need to be reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants