-
Notifications
You must be signed in to change notification settings - Fork 199
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
Physical Memory Protection (32/64) Tests and Covergroups #462
Conversation
…d half word access, PMP Coverpoints updated to a new pattern
@allenjbaum Can you please review this version of coverpoints and the tests? The updated coverage report has also been attached above for your reference. |
Signed-off-by: Muhammad Hammad Bashir <139617104+MuhammadHammad001@users.noreply.github.com>
@MuhammadHammad001 , could have look at the CI checks, seems those added tests are failing |
@jamesbeyond The reason the CI is failing on the tests is because as mentioned in the description, these tests and covergroups are written following the new privilege architecture features support in RISC-V ISAC and RISCOF. Once, those PRs are merged, these tests will also pass the CI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little concerned that the helper macros are hardcoded as to the PMP registers numbers, Is there any way those can be passed in as a parameter? that will help other tests n the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the test structure is for each mode, different RWX settings with R,W, & X ops (R,W,RW,RX,RWX) or priority cases with same settings & a lower priority with RWX settings, or
a 3 level priority with the middle being no allowed accesses.
What is missing are for the first 2 cases: a noaccess case, for the second RWX cases (which is OK because its redundant).
In all case tests, the restricted case is higher priority than the all or none case.
I think you also need need tests where a restricted case is lower priority than no_Access or all access cases (see also comments on the first 2 tests).
I don't think there are tests here where the access is not matched by any entry, or paritally matched by any entry (without being unaligned). I think you can only do that with NA4 &ld, sd for RV64, and FLD,FSD in RV32 (so F-ext must be supported in that case)
Updated pmp_cfg_locked_write_unrelated coverpoint based on Allen comment Signed-off-by: Umer Shahid <umer.shahid@10xengineers.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug is fixed
Updated pmp_cfg_locked_write_unrelated coverpoint in rv64 to update a minor bug Signed-off-by: Umer Shahid <umer.shahid@10xengineers.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RV64 bugs fixed
@allenjbaum please approve this PR again, your previous approval was dismissed because of the new commits, now it is passing the CI, so it is good to go now. |
Looks like the riscof_work folder was added in the latest commit. This definitely shouldn’t be included. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is fixed, this is ready to go
Description
Related Issues
Ratified/Unratified Extensions
List Extensions
Reference Model Used
Mandatory Checklist:
Optional Checklist: