Pmp tests

[UmerShahidengr]

Dear All,
This PR contains Priv Arch compliance tests. This repo will be updated weekly with new priv arch tests. These short PRs will be easier to review on weekly basis. In the first iteration, a few basic PMP compliance tests have been pushed for review.

[allenjbaum]

What is going to be important in order to merge these tests are that - they compare between Sail and Spike (I think you already verified this) - that risc-isac can verify coverage. That means you need to be able to describe the coverage using a YAML ctg schema description. I think you'll find that this will be as hard as writing the tests, but it is important. Your written descriptions are a start, but the need to be fleshed out with the real corner cases in that YAML format (e.g. - 2 matches from both first and last PMP entry, - intervening entries being non-matching (including off) - the first is a wider range than the last, or a smaller or overlapping - 4 combinations of illegal/legal - for each of the permission bits. Another might be 3 entries: 2 abutting, and a 3rd overlapping the other two for just 8bytes (which I think can only occur on an RV64 with a doubleword data fetch) with - 6 different prioriity orders, - 3 different accesses (matching entirely in the 1st, 2nd or 3rd entry) - 8 different legal/illegal combinations, - both loads and stores

…

On Mon, Sep 19, 2022 at 10:28 AM Umer Shahid ***@***.***> wrote: Dear All, This PR contains Priv Arch compliance tests. This repo will be updated weekly with new priv arch tests. These short PRs will be easier to review on weekly basis. In the first iteration, a few basic PMP compliance tests have been pushed for review. ------------------------------ You can view, comment on, or merge this pull request online at: #284 Commit Summary - ecd5cc3 <ecd5cc3> PMP tests for RV32 & RV64 along with header macro files - f5d034a <f5d034a> header updated: trap handler issue, tests failing - 4503849 <4503849> Updated PMP Tests based on new arch_test file - f4eaf3f <f4eaf3f> Updated PMP tests, added verification tests in S-mode and U-mode - f496c9d <f496c9d> Merge branch 'riscv-non-isa:main' into pmp_tests - 4e01cd8 <4e01cd8> Updated pmp tests - 6f5ef61 <6f5ef61> Updated pmp tests File Changes (6 files <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files> ) - *D* riscv-test-suite/env/arch_test.h <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-62959c4d982b131e0dbdf5ffd0d074f4f71c0c28cdf12ad6f68321f719d5b491> (1466) - *A* riscv-test-suite/env/test_macros.h <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-86c00a2b5079b1636883cca4bc0c29ef31935d4faabbc72e46926e27004dee77> (818) - *A* riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-CFG-lock_bit.S <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-7f695ab627443208a37f0a9d0927f96556519b3988b4382ce9a292806dedfe0a> (257) - *A* riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-CFG-reg.S <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-051f6f7de272e6bf272aaa3cdeb2b5f5e1f4ee96e7b76ef3adb710f9409d3af1> (94) - *A* riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-CSR-access.S <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-38b1e3bda74767a0bb012ca96756297cd77215f38a53a78a85ce06381f223cc6> (292) - *A* riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-granularity.S <https://github.com/riscv-non-isa/riscv-arch-test/pull/284/files#diff-1f09a588c39f6cd29af3ba7162121ecd114ebac8cc63cf1ad825572fb82a6319> (128) Patch Links: - https://github.com/riscv-non-isa/riscv-arch-test/pull/284.patch - https://github.com/riscv-non-isa/riscv-arch-test/pull/284.diff — Reply to this email directly, view it on GitHub <#284>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJTUUIJK7E7SABW7QA3V7CPC3ANCNFSM6AAAAAAQQJ4QVM> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[pawks]

What is going to be important in order to merge these tests are that - they compare between Sail and Spike (I think you already verified this) - that risc-isac can verify coverage. That means you need to be able to describe the coverage using a YAML ctg schema description. I think you'll find that this will be as hard as writing the tests, but it is important.

There is a small hitch in this case. The csr coverpoints are currently defined over the values in the csrs. So the coverpoints for the pmpaddr csr can be defined only based on the value contained in them. However, it is perfectly legal for the values to be different based on the implementation defined address space, meaning that the coverpoint itself changes between implementations. There are two possible ways to handle this:

• Allow for the coverpoints to be different based on the implementation and assume a default for SAIL. Since the coverage is primarily measured on SAIL, this allows us to easily measure coverage but the coverpoints are tied to a specific memory map.
• Define the coverpoints based on the behaviour of the pmp, making them independent of the actual addresses i.e use the effective address of the access and write expressions which mimic the pmp match and priority functions to check for coverage. Doing so will require some more discussion and adding support for the same in isac.

Your written descriptions are a start, but the need to be fleshed out with the real corner cases in that YAML format (e.g. - 2 matches from both first and last PMP entry, - intervening entries being non-matching (including off) - the first is a wider range than the last, or a smaller or overlapping - 4 combinations of illegal/legal - for each of the permission bits. Another might be 3 entries: 2 abutting, and a 3rd overlapping the other two for just 8bytes (which I think can only occur on an RV64 with a doubleword data fetch) with - 6 different prioriity orders, - 3 different accesses (matching entirely in the 1st, 2nd or 3rd entry) - 8 different legal/illegal combinations, - both loads and stores

The tests also should be easily configurable to account for the following:

• Granularity of the pmps
• Write-ability of the pmp* csrs
• Number of pmp csrs defined

Another corner case which needs to be considered is partial matches. This particular case is applicable for mem accesses only when the granularity of the pmps is 4 and the XLEN is 64 or the D extension exists. However, this is applicable for instruction accesses when the C extension exists regardless of the granularity of the csrs.

[UmerShahidengr]

• that risc-isac can verify coverage. That means you need to be able to describe the coverage using a YAML ctg schema description. I think you'll find that this will be as hard as writing the tests, but it is important.

Since ISAC has limitation on adding coverpoints. I added csr coverpoints which were defined over the values in the pmp registers, I submitted that PR at riscv-ctg with updated cfg files, I will update those cfg files according to the test cases for each upcoming test

[UmerShahidengr]

• Allow for the coverpoints to be different based on the implementation and assume a default for SAIL. Since the coverage is primarily measured on SAIL, this allows us to easily measure coverage but the coverpoints are tied to a specific memory map.
• Define the coverpoints based on the behaviour of the pmp, making them independent of the actual addresses i.e use the effective address of the access and write expressions which mimic the pmp match and priority functions to check for coverage. Doing so will require some more discussion and adding support for the same in isac.

Dear @pawks , I am not sure if I understand it fully. I'll dig in to it, but I might need more help in this regard. I will ping you offline in case of any query. In the meantime, can you refer me to some good documentation on understanding cgf files in more detail?

[UmerShahidengr]

• Granularity of the pmps
• Write-ability of the pmp* csrs
• Number of pmp csrs defined

Another corner case which needs to be considered is partial matches. This particular case is applicable for mem accesses only when the granularity of the pmps is 4 and the XLEN is 64 or the D extension exists. However, this is applicable for instruction accesses when the C extension exists regardless of the granularity of the csrs.

Point noted. We are actually following this testplan, you can review our testplan and give your feedback, we will add more tests to cover the corner point which you mentioned.

[pawks]

Dear @pawks , I am not sure if I understand it fully. I'll dig in to it, but I might need more help in this regard. I will ping you offline in case of any query. In the meantime, can you refer me to some good documentation on understanding cgf files in more detail?

A description of the various types of coverpoints and their functions can be found here.

[UmerShahidengr]

A description of the various types of coverpoints and their functions can be found here.

Thanks.

[pawks]

The tests should also mention the conditions for it to be enabled in the TEST_CASE macros. It would probably be better if the tests are organized in such a way that all related test cases for a particular pmpentry in a single test. All the tests in the PR for the L bit only test their warl behaviour. However, the same test can be leveraged to test for the matching and permission logic too.

[pawks]

Another thing which should probably be accounted for is the Smepmp spec as it changes the behavior of the pmps. They will definitely influence the conditions for these tests.

[UmerShahidengr]

Another thing which should probably be accounted for is the Smepmp spec as it changes the behavior of the pmps. They will definitely influence the conditions for these tests.

Agreed. The addition of Smepmp will definitely change the conditions and test cases of these tests (and we are also looking forward to work on it), but since Sail does not have full support of ePMP yet and we are still naive in this environment, so why not if you declare these tests as beta version (or something with partial completion status), and review it as first draft. We will keep improving these tests with the passage of time and with each update in golden model.

[UmerShahidengr]

The tests should also mention the conditions for it to be enabled in the TEST_CASE macros. It would probably be better if the tests are organized in such a way that all related test cases for a particular pmpentry in a single test. All the tests in the PR for the L bit only test their warl behaviour. However, the same test can be leveraged to test for the matching and permission logic too.

Agreed to this point too. Yes we can add all possible test cases of one particular pmpentry in one single test but that will complicate the test structure especially when we incorporate the behavior of each test case in different privilege mode (S,U, H).
We are following it step-by-step. In the first go, we have written tests to verify the warl behavior of L-bit in pmpcfg_lock_bit test (as you mentioned). Similarly, we are testing if pmp regs are accessible in only M-mode or not in pmp_csr_access test.Same can be said for pmp TOR mode tests, NAPOT mode tests, and NA4 mode tests. Combining all possible scenerios wrt particular pmp entry will complicate the understanding of these tests. If you see the test plan, things will become complicated in future PRs (when tests related to priorities & matching, and paging will come into play)

[allenjbaum]

I think this is a perfectly reasonable approach for development.

…

On Thu, Sep 22, 2022 at 5:31 AM Umer Shahid ***@***.***> wrote: The tests should also mention the conditions for it to be enabled in the TEST_CASE macros. It would probably be better if the tests are organized in such a way that all related test cases for a particular pmpentry in a single test. All the tests in the PR for the L bit only test their warl behaviour. However, the same test can be leveraged to test for the matching and permission logic too. Agreed to this point too. Yes we can add all possible test cases of one particular pmpentry in one single test but that will complicate the test structure especially when we incorporate the behavior of each test case in different privilege mode (S,U, H). We are following it step-by-step. In the first go, we have written tests to verify the warl behavior of L-bit in pmpcfg_lock_bit <https://github.com/Abdulwadoodd/riscv-arch-test/blob/pmp_tests/riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-CFG-lock_bit.S> test (as you mentioned). Similarly, we are testing if pmp regs are accessible in only M-mode or not in pmp_csr_access <https://github.com/Abdulwadoodd/riscv-arch-test/blob/pmp_tests/riscv-test-suite/rv32i_m/privilege/src/pmp32/PMP-CSR-access.S> test.Same can be said for pmp TOR mode tests, NAPOT mode tests, and NA4 mode tests. Combining all possible scenerios wrt particular pmp entry will complicate the understanding of these tests. If you see the test plan, things will become complicated in future PRs (when tests related to priorities & matching, and paging will come into play) — Reply to this email directly, view it on GitHub <#284 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJVBE3V5ZOAKFL4BFUDV7RGTLANCNFSM6AAAAAAQQJ4QVM> . You are receiving this because you commented.Message ID: ***@***.***>

[UmerShahidengr]

In this week's commit, the following changes are made:

• Based on Allen's comments, in PMP-CFG-reg.S tests for locked bit and zero bits are merged.
• code size is reduced by adding macros and .rept directive.
• coverpoints for pmpcfg tests are added in rv32i_priv.cgf.
  Hopefully, all other coverpoints and pmp tests will be merged till next week, contributors are requested to review the draft.