Skip to content

Add test suites for the Sspmp extension#1166

Open
ybc-alkaid wants to merge 32 commits intoriscv:act4from
ybc-alkaid:act4
Open

Add test suites for the Sspmp extension#1166
ybc-alkaid wants to merge 32 commits intoriscv:act4from
ybc-alkaid:act4

Conversation

@ybc-alkaid
Copy link
Copy Markdown
Member

@ybc-alkaid ybc-alkaid commented Mar 31, 2026

This PR introduces tests suites for the RISC-V S-level Physical Memory Protection (SPMP) extension.

A pre-built PDF of SPMP:
https://github.com/riscv/riscv-spmp/blob/main/rv-spmp-spec.pdf

A pre-built PDF of riscv-privileged manual with SPMP:
https://github.com/riscv/riscv-spmp/releases/download/v0.8.18/riscv-privileged-with-spmp.pdf

A pointer to the SPMP specification:
riscv/riscv-isa-manual#2573

A Spike implementation supporting the tests:
riscv-software-src/riscv-isa-sim#2249

A QEMU implementation supporting the SPMP:
https://patchew.org/QEMU/20260318185238.99143-1-luisccunha8@gmail.com/

A Sail implementation supporting the SPMP:
riscv/sail-riscv#1457

A pointer to Unified-DB with SPMP information:
riscv/riscv-unified-db#1817

A pointer to the SPMP test plan:
https://docs.google.com/spreadsheets/d/1A-WXaAMJItmS6xfvJTAbtvhiR0XQ4AMe1oX2GEr21JM/edit?gid=513661486#gid=513661486

A pre-built Spike for your convenience:
https://drive.google.com/file/d/13kjMH1wittWbnSUzB11lZ6631HQ2poLR/view?usp=drive_link

A pre-built Sail for your convenience:
https://drive.google.com/file/d/11evwb2hHg3eKC0bSWBu2bCSdKN8pXpzL/view?usp=drive_link

Test logs:
https://drive.google.com/drive/folders/1luWNCsNwH7Yt6xTPhAvRxxHNHDrywCN2?usp=drive_link

Copilot AI review requested due to automatic review settings March 31, 2026 09:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new privileged test suite for the SPMP-related extensions by introducing a Python test generator (Sspmp) and checking in the framework-generated standalone assembly tests, along with functional coverage (SV covergroups) and normative/parameter YAML mappings.

Changes:

  • Add Sspmp privileged test generator (SPMPSm.py) that can emit either a combined framework test or multiple standalone subtests.
  • Add new SPMP-related CSR encodings (e.g., CSR_SPMPEN, CSR_MPMPDELEG) to tests/env/encoding.h.
  • Add SPMP-specific functional coverage (SPMPSm_coverage*.svh) and normative/parameter mappings (SPMPSm.yaml).

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tests/priv/Sspmp/SPMPSmTorEntry0.S Standalone autogenerated subtest for TOR entry 0 lower-bound behavior.
tests/priv/Sspmp/SPMPSmSum.S Standalone autogenerated subtest for sstatus.SUM interactions.
tests/priv/Sspmp/SPMPSmSpmpen.S Standalone autogenerated subtest for spmpen behavior (Sspmpen).
tests/priv/Sspmp/SPMPSmShared.S Standalone autogenerated subtest for Shared-region encodings.
tests/priv/Sspmp/SPMPSmSfence.S Standalone autogenerated subtest for SFENCE.VMA ordering after SPMP CSR writes.
tests/priv/Sspmp/SPMPSmSatpBare.S Standalone autogenerated subtest related to satp Bare mode expectations.
tests/priv/Sspmp/SPMPSmReserved.S Standalone autogenerated subtest for reserved SPMP encodings.
tests/priv/Sspmp/SPMPSmPriority.S Standalone autogenerated subtest for priority/matching setup patterns.
tests/priv/Sspmp/SPMPSmPermUmode.S Standalone autogenerated subtest for U-mode rule encodings.
tests/priv/Sspmp/SPMPSmPermSmode.S Standalone autogenerated subtest for S-mode rule encodings.
tests/priv/Sspmp/SPMPSmOobAccess.S Standalone autogenerated subtest for out-of-bounds siselect indices.
tests/priv/Sspmp/SPMPSmNoMatch.S Standalone autogenerated subtest configuring “no match” conditions.
tests/priv/Sspmp/SPMPSmMxr.S Standalone autogenerated subtest for sstatus.MXR interactions.
tests/priv/Sspmp/SPMPSmMpmpdeleg.S Standalone autogenerated subtest for mpmpdeleg behavior (Smpmpdeleg).
tests/priv/Sspmp/SPMPSmMmodeBypass.S Standalone autogenerated subtest for M-mode bypass expectations.
tests/priv/Sspmp/SPMPSmMmodeAccess.S Standalone autogenerated subtest for M-mode indirect SPMP CSR access.
tests/priv/Sspmp/SPMPSmLock.S Standalone autogenerated subtest for SPMP lock semantics.
tests/priv/Sspmp/SPMPSmFault.S Standalone autogenerated subtest intended to trigger SPMP-related faults.
tests/priv/Sspmp/SPMPSmCsrAccess.S Standalone autogenerated subtest for S-mode indirect CSR access to SPMP state.
tests/priv/Sspmp/SPMPSmAddrMatch.S Standalone autogenerated subtest for address-matching mode configuration patterns.
tests/env/encoding.h Adds CSR/address constants for SPMPEN/SPMPENH/MPMPDELEG and siselect range helpers.
generators/testgen/src/testgen/priv/extensions/SPMPSm.py New generator implementing SPMP/SPMPEN/MPMPDELEG test emission (framework + standalone modes).
coverpoints/priv/SPMPSm_coverage.svh New SV covergroups for SPMP CSR, permission, matching, paging, and spmpen coverage.
coverpoints/priv/SPMPSm_coverage_init.svh Instantiates the new SPMP covergroups.
coverpoints/param/SPMPSm.yaml Parameter mapping metadata for the SPMP suite.
coverpoints/norm/SPMPSm.yaml Normative-rule-to-coverpoint mapping for the SPMP suite.

Comment thread coverpoints/priv/SspmpSm_coverage.svh
Comment thread coverpoints/priv/SspmpSm_coverage.svh
Comment thread generators/testgen/src/testgen/priv/extensions/SPMPSm.py Outdated
Comment thread generators/testgen/src/testgen/priv/extensions/SspmpSm.py
Comment thread generators/testgen/src/testgen/priv/extensions/SPMPSm.py Outdated
Comment thread generators/testgen/src/testgen/priv/extensions/SPMPSm.py Outdated
Comment thread generators/testgen/src/testgen/priv/extensions/SspmpSm.py
Comment thread generators/testgen/src/testgen/priv/extensions/SPMPSm.py Outdated
Comment thread generators/testgen/src/testgen/priv/extensions/SPMPSm.py Outdated
@jjscheel
Copy link
Copy Markdown

jjscheel commented Apr 1, 2026

Thanks, @ybc-alkaid! It looks like you've been busy!!!

Once again, @jordancarlin, @jamesbeyond, @UmerShahidengr, @davidharrishmc, the question becomes whether we have enough pieces here to reach a mergeable PR. From a process point of view, we've tried to ensure that coverage is correct and the tests reflect that coverage using the appropriate framework. We expect to do work on the testcases to refine their reability or ensure the technical correctness, but we want them to be reflect the coverage and be workable toward a final PR.

Please share your thoughts.

@davidharrishmc
Copy link
Copy Markdown
Collaborator

In addition to the trace in the test log, would it be possible to post the report from "make coverage" and the success message from "make spike" showing that the tests achieve 100% coverage and pass all of the tests?

@ybc-alkaid
Copy link
Copy Markdown
Member Author

ybc-alkaid commented Apr 2, 2026

@davidharrishmc Current test log was generated by running Spike manually.
To directly execute "make spike", I have to first submit a PR to riscv-unified-db.
Will update.

@jjscheel
Copy link
Copy Markdown

jjscheel commented Apr 9, 2026

To directly execute "make spike", I have to first submit a PR to riscv-unified-db.
Will update ASAP.

Can you use a local build of UDB with your patch applied instead? This is very similar to using a local Spike with your new support to run your tests.

@jjscheel
Copy link
Copy Markdown

jjscheel commented Apr 9, 2026

Once again, @jordancarlin, @jamesbeyond, @UmerShahidengr, @davidharrishmc, the question becomes whether we have enough pieces here to reach a mergeable PR. From a process point of view, we've tried to ensure that coverage is correct and the tests reflect that coverage using the appropriate framework. We expect to do work on the testcases to refine their reability or ensure the technical correctness, but we want them to be reflect the coverage and be workable toward a final PR.

Please share your thoughts.

Ping...

I know we have asked for logs, but are those really required to know if we have something workable? To me, the coverpoints definition is more important that knowing ALL points were hit by the tests...

@jordancarlin
Copy link
Copy Markdown
Collaborator

Once again, @jordancarlin, @jamesbeyond, @UmerShahidengr, @davidharrishmc, the question becomes whether we have enough pieces here to reach a mergeable PR. From a process point of view, we've tried to ensure that coverage is correct and the tests reflect that coverage using the appropriate framework. We expect to do work on the testcases to refine their reability or ensure the technical correctness, but we want them to be reflect the coverage and be workable toward a final PR.
Please share your thoughts.

Ping...

Sorry about the delay. @jjscheel @ybc-alkaid I will get to this by Wednesday. I'll do a review of the code in the PR and try to put together a list of anything else that we definitely still need.

@jamesbeyond
Copy link
Copy Markdown
Collaborator

Hi @ybc-alkaid,
just want check with you where I can find the normative Rules for the spmp extension.
I looked into the spmp repo: https://github.com/riscv/riscv-spmp/blob/main/sspmp/spmp_spec.adoc
also the PR for ISA manual : riscv/riscv-isa-manual#2573

I don't see them defined there
however, in this PR, SspmpSm.yaml I can see those normative rules been referenced

@ybc-alkaid
Copy link
Copy Markdown
Member Author

ybc-alkaid commented Apr 17, 2026

Hi @jamesbeyond,
I find the riscv-arch-test defined far more normative rules than the ISA-manual.
So I think the normative rules should be reviewed and approved by ACT-Sig before being merged into the ISA manual. Therefore, to avoid cross-dependency, I am waiting for this PR to be acknowledged, after which I will update the ISA manual PR.
Am I missing anything, or is it the other way around?

@ybc-alkaid
Copy link
Copy Markdown
Member Author

ybc-alkaid commented Apr 21, 2026

@jordancarlin, @jamesbeyond, @UmerShahidengr, @davidharrishmc,

Ping...
From a testing perspective, are we ready to freeze the SPMP?
(Note: we only intend to freeze it, not ratify it now. So if we meet the essential requirement, please let us proceed...Eventually, we'll finish all tasks.)

@jordancarlin
Copy link
Copy Markdown
Collaborator

@ybc-alkaid @jjscheel It's a bit later than Wednesday, but I finally got a chance to look through this PR. I have lots of small comments on the code itself, but I will wait on that kind of detailed review until we are closer to merging this. My major notes on this are below.

Test Generator/Tests

  • Each extension (Sspmp, Sspmpen, Smpmpdeleg) needs a separate test generator/test so that each extension can be tested individually. For example, we can’t assume Sspmpen will always be implemented, so we can’t test it as part of Sspmp.
    • As part of this, rename the Python files to Sspmp.py, Sspmpen.py, etc.
  • Let’s drop all of the extra code that seems designed to sidestep the framework right now (the single test function, for example). If we need smaller tests, we should have multiple generators instead, though in general, we prefer to keep the number of separate test files to a minimum since the overhead of starting a simulation tends to be longer than most of the actual tests.
  • We need to test all of the SPMP registers, not just the first four of them. We can do most of our testing on a smaller subset of them but need at least some kind of smoke test for the rest of the registers.

Normative Rules

I see the list of normative rules in SspmpSm.yaml and the corresponding coverpoints, but I don't see those normative rules in the spec, which makes them hard to review. The Sspmp spec needs to be updated to include normative rules and then the YAML file needs to be updated to refer to those normative rules.

Coverpoints and Testplan

  • The current coverage is definitely insufficient for the final version of these tests, but it is a good start. We will need to add more extensive testing of accesses that should pass/fail with Spmp as we flush out a more complete testplan.
  • One of the major missing pieces right now is a test plan (and section in the CTP). Without a test plan, it is hard to interpret all of the coverpoints. See https://riscv.github.io/riscv-arch-test/ctp.html#_privileged_test_plan for an example of what the CTP section should look like. Each of the existing extensions in that section also has a link to a spreadsheet test plan.
  • Many of the coverpoints are not correctly covering what I think they intend to. For example, the cp_spmp_indirect_access coverpoint actually just checks that a csrrw, csrrs, and csrrc instruction were each executed without considering which CSR they are accessing.

Test Framework/Running the Tests

As far as I can tell from the provided logs, the test was manually compiled in non-self-checking mode and run on Spike without using the actual ACT4 test framework. This makes the log of limited use in reviewing this. It provides a trace of the executed instructions, but does not help in validating that the test is correct because none of the results were checked. In order to properly run the tests, support for SPMP needs to be added to UDB and then one of the configs in this repo (or a new config) should be updated to indicate that it supports SPMP. The test framework will then select and build the SPMP tests for that config and run them, first on the Sail model to generate expected results and then on the target platform (I suggest using a Spike config) using those expected results.

Freeze Status

The above list is lengthy and all of it (and more) will likely be needed for ratification. That still leaves open the question of what is needed for freeze. There is no official policy in place yet so I can't say for sure, but here is my personal opinion based on the state of this current PR.

  • A testplan spreadsheet in the style of the other priv tests (linked to from the CTP which is itself linked above) is absolutely necessary. The testplan is what we evaluate all of the tests and coverpoints against and should serve as the main source of truth for what we want to test. For freeze, the testplan should include everything that we will eventually want to test for the extension even if not all of those tests are completed yet.
  • The CTP section can probably wait until ratification.
  • The coverpoints seem like a reasonable start and are probably enough for freeze, but they will need significant work before ratification. It is also hard to evaluate them until the testplan is available.
  • Ideally, normative rules will be added to the specification before freeze.
  • I'd like to see the test restructuring based on extension mentioned above happen before freeze so we have a better sense of what the final tests will look like.
  • The big open question is actually running these tests using the framework. That is not an insignificant undertaking because it requires support in Sail, minimal support in UDB, support in GCC or Clang, and support in some other simulator (Spike, QEMU, Whisper, etc.), but I have a hard time approving these tests for any milestone without running them because it means we have no way of knowing if they are working as expected. It seems like there is (initial) support for all of these except UDB already, and the UDB support can be very minimal (limited to just a few YAML files declaring the extensions, no need for the CSR YAMLs), so I'm inclined to ask for this to be completed for freeze if at all possible. I'm happy to help walk through which specific parts are needed for UDB if that would be helpful.

I know this is quite a long response, so feel free to ask follow-up questions as we work through this and thanks for your flexibility as the first extension to be going through this new process.

@ybc-alkaid
Copy link
Copy Markdown
Member Author

@jordancarlin , thanks for the review.
You can evaluate tests using this pre-built Spike:
https://drive.google.com/file/d/1U7BTZ9kS6UQUlibtImdx4vKdY-gr69IM/view?usp=drive_link
It matches the PR version on Spike's GitHub, with my bug fixes included.
I'll go over the requirements.

@jordancarlin
Copy link
Copy Markdown
Collaborator

@jordancarlin , thanks for the review. You can evaluate tests using this pre-built Spike: https://drive.google.com/file/d/1U7BTZ9kS6UQUlibtImdx4vKdY-gr69IM/view?usp=drive_link It matches the PR version on Spike's GitHub, with my bug fixes included.

Without versions of the sail model and UDB that support the new extension, I don’t think I can run the tests. Spike on its own is not sufficient.

I'll go over the requirements.

Great. Reach out if you have any questions or need assistance with any of it.

@ybc-alkaid
Copy link
Copy Markdown
Member Author

ybc-alkaid commented Apr 22, 2026

@jordancarlin

Without versions of the sail model and UDB that support the new extension, I don’t think I can run the tests. Spike on its own is not sufficient.

I remember you mentioned in the last ACT meeting that proving all tests pass using either Sail or Spike is okay, at least for the current state.
So we need both now?

@jordancarlin
Copy link
Copy Markdown
Collaborator

@jordancarlin

Without versions of the sail model and UDB that support the new extension, I don’t think I can run the tests. Spike on its own is not sufficient.

I remember you mentioned in the last ACT meeting that proving all tests pass using either Sail or Spike is okay, at least for the current state. So we need both now?

I'm not sure what I said in the meeting, but as I mentioned above, running the test on Spike without first generating expected results means that the test is not actually checking anything, so it can't really pass or fail. When tests run, they should end by printing a message that looks like the following:

RVCP-SUMMARY: TEST PASSED - Test File "I-and-00.S"

If the message says SIGRUN instead of PASSED, it means that it was running in signature generation mode to create the expected results and was not checking anything. I don't see any of the UART output in the log you linked, but I assume it printed the SIGRUN version because I don't know how you would have generated the expected results.

@jordancarlin
Copy link
Copy Markdown
Collaborator

@ybc-alkaid This PR came up again during today's ACT SIG meeting. The consensus is that for freeze, we need a testplan (similar to the spreadsheets linked in this section of the CTP: https://riscv.github.io/riscv-arch-test/ctp.html#_privileged_test_plan). Once there is a testplan, the rest of this PR seems like a great initial effort and is more than sufficient for freeze. Let us know if you have any questions or need help with the testplan. Thanks again for all your work on this!

Copilot AI review requested due to automatic review settings May 3, 2026 17:14
@ybc-alkaid
Copy link
Copy Markdown
Member Author

Hi @jordancarlin @UmerShahidengr, many thanks for your comments.
I finally got time to update the test suites.

  • the UDB PR is created
  • the normative rules are updated to SPMP ISA PR
  • the sail model is updated
  • the test plan is created
  • all blocking items are updated
  • all non-blocking items are also updated

Now you can run make spike directly, and all tests shall pass.
image

Of course at current stage, you will need UDB, spike and sail that supports SPMP locally to run those tests. Don't worry, I build them for you:

A pre-built Spike for your convenience:
https://drive.google.com/file/d/13kjMH1wittWbnSUzB11lZ6631HQ2poLR/view?usp=drive_link

A pre-built Sail for your convenience:
https://drive.google.com/file/d/11evwb2hHg3eKC0bSWBu2bCSdKN8pXpzL/view?usp=drive_link

A pointer to Unified-DB with SPMP information:
riscv/riscv-unified-db#1817

A pointer to the SPMP test plan:
https://docs.google.com/spreadsheets/d/1A-WXaAMJItmS6xfvJTAbtvhiR0XQ4AMe1oX2GEr21JM/edit?gid=513661486#gid=513661486

Test logs:
https://drive.google.com/drive/folders/1luWNCsNwH7Yt6xTPhAvRxxHNHDrywCN2?usp=drive_link

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 38 out of 41 changed files in this pull request and generated 7 comments.

Comment thread scripts/sspmp_coverage_reachability.py Outdated
Comment thread scripts/sspmp_coverage_reachability.py Outdated
Comment thread tests/priv/Sspmp/SspmpSmSum.S
Comment thread tests/priv/Sspmp/SspmpSmPermUmode.S
Comment thread docs/ctp/src/spmp.adoc
Comment thread coverpoints/priv/SspmpSm_coverage.svh Outdated
Comment thread coverpoints/priv/SspmpSm_coverage.svh
@jordancarlin
Copy link
Copy Markdown
Collaborator

@ybc-alkaid Thank you for all of the work you have put into this. This is looking great and was clearly a very significant undertaking to put together. There is still more work to do before this will be ready to merge (I will try to do a detailed review at some point in the next few weeks), but I am more than happy to approve this for freeze at this point @jjscheel.

Copilot AI review requested due to automatic review settings May 6, 2026 06:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 38 out of 41 changed files in this pull request and generated 4 comments.

Comment thread coverpoints/priv/SspmpSm_coverage.svh
Comment thread coverpoints/param/SspmpSm.yaml
Comment thread tests/priv/Sspmp/SspmpSmSpmpen.S
Comment thread tests/priv/Sspmp/SspmpSmMpmpdeleg.S
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants