Skip to content

PMM (Pointer Masking) ACTs #484

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

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

Conversation

UmerShahidengr
Copy link
Collaborator

@UmerShahidengr UmerShahidengr commented Aug 6, 2024

Description

This PR has ACTs for Pointer Masking Extension

Ratified/Unratified Extensions

  • Ratified
  • [] Unratified

List Extensions

Pointer Masking Extension specs (frozen) : available here
Sail PR: available here

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.

@MuhammadHammad001
Copy link
Contributor

Hi,
This SOW is complete. All the tests with 100% coverage are added in this PR(access the coverage report here).
@allenjbaum @jamesbeyond @UmerShahidengr

@MuhammadHammad001
Copy link
Contributor

To run the coverage locally, use the following command:

riscof coverage --config=/home/hammad//riscv-arch-test/config.ini --cgf-file=/home/hammad/riscv-arch-test/coverage/dataset.cgf --cgf-file=/home/hammad/Pointer_Masking/sir_umer_repo/riscv-arch-test/coverage/rv64_pmm.cgf --suite=/home/hammad/riscv-test-suite/rv64i_m/pmm --env=/home/hammad/artifacts/env -h /home/hammad/riscv-arch-test/coverage/header_file.yaml

@jamesbeyond jamesbeyond changed the title PMM ACTs PMM (Pointer Masking) ACTs Feb 24, 2025
@jamesbeyond jamesbeyond added ratified ratified specs and removed unratified labels Feb 24, 2025
@nadime15
Copy link

nadime15 commented May 27, 2025

Hi all!

I am currently using these tests to validate the implementation of the pointer masking extension for Sail.

I have come across a few issues that I wanted to share:

  1. The default configuration in Sail doesn't work as-is, you will encounter a load-access-fault (PMM_basic_01_M_sv48_tag11.S).
0x00000000800007A4 (0x00040483) lb x9, 0x0(x8)
mem[R,0x0000000080006900] -> 0x00200000000000DF
within_phys_mem: 0x0080000000000000 not within phys-mem:
  plat_rom_base: 0x0000000000001000
  plat_rom_size: 0x0000000000001000
  plat_ram_base: 0x0000000080000000
  plat_ram_size: 0x0000000080000000
trapping from M to M to handle load-access-fault

this is due to the size of the RAM, which is for these tests to small, changing the configuration and increasing the RAM size (in my case to 8 exabyte) will get rid of the problem:

"ram": {
  "base": 2147483648,
  "size": 9223372036854775808
}

  1. Spike will also "fail" with an illegal instruction exception. This happens because the Smmpm extension is not enabled (btw. all extensions, namely Smnpm, Smmpm and Ssnpm should be enabled), which is required for accessing the mseccfg CSR (PMM_basic_01_M_sv48_tag11.S):
core   0: 0x0000000080000818 (0x7476a073) csrs    mseccfg, a3
core   0: exception trap_illegal_instruction, epc 0x0000000080000818
core   0:           tval 0x000000007476a073

Snippet from Spike:

void mseccfg_csr_t::verify_permissions(insn_t insn, bool write) const {
  basic_csr_t::verify_permissions(insn, write);
  if (!proc->extension_enabled(EXT_SMEPMP) &&
      !proc->extension_enabled(EXT_SMMPM) &&
      !proc->extension_enabled(EXT_ZICFILP) &&
      !proc->extension_enabled(EXT_ZKR))
    throw trap_illegal_instruction(insn.bits());
}

I believe there are a few more issues that still need to be resolved, and I am currently investigating their root causes.

@MuhammadHammad001
Copy link
Contributor

@nadime15 Thank you for pointing out the issues. This is a really old PR and the sail model has changed a lot since then.

I will update the changes in the next commit.

Copy link

@nadime15 nadime15 left a comment

Choose a reason for hiding this comment

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

Hi @MuhammadHammad001,

Thanks for your reply! I just noticed a few additional points that might save you some time down the line:

  1. The test cases for pmm_float are not being executed.

  2. The test cases for Supervisor (and User modes) are triggering illegal instruction exceptions due to incorrectly defined addresses in the page table setup (details below).

I have also included a few more observations further down.

Comment on lines +1 to +8
/*
Verification Goal: Set PMM = 01 in the menvcfg and tag bits are 0xFFFF with bit[47]=1,
test whether or not pointer masking with PMLEN = 16 is enabled or not in M-Mode in sv48
Description:
If Pointer Masking is enabled, then the Effective Address will be masked accordingly, no exception will be generated,
If Pointer Masking is disabled, then the Effective Address will be valid, no exceptions will be created
due to the invalid Virtual Address,
*/

Choose a reason for hiding this comment

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

The filenames in the pmm_float directory aren't right for M-Mode, they use SV48 (uppercase), but everything else uses sv48.

Comment on lines +2 to +7
Verification Goal: Set PMM = 01 in the menvcfg and tag bits are 0xABAB with bit[47]=0,
test whether or not pointer masking with PMLEN = 16 is enabled or not in M-Mode in sv48
Description:
If Pointer Masking is enabled, then the Effective Address will be masked accordingly, no exception will be generated,
If Pointer Masking is disabled, then the Effective Address will be invalid, exceptions will be created
due to the invalid Virtual Address,

Choose a reason for hiding this comment

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

For pmm_atomics, the files:

  1. PMM_atomic_01_M_sv48_tag02.S

  2. PMM_atomic_01_M_sv48_tag03.S

end in decimal, but they should use the binary form instead, tag10.S and tag11.S.

Comment on lines +36 to +43
# --------------------------- Define Addresses -------------------------------
.set pa_rvtest_code_begin, 0x8000000000039c // 56-bit physical address of the code section
.set pa_rvtest_data_begin, 0x80000000003530 // 56-bit physical address of the data section
.set pa_rvtest_sig_begin, 0x80000000006218 // 56-bit physical address of the signature section
.set va_rvtest_code_begin, 0xFFFF80000000039c // 48-bit virtual address of the code section
.set va_rvtest_data_begin, 0xFFFF900000000000 // 48-bit virtual address of the data section
.set va_rvtest_sig_begin, 0xFFFF900000006218 // 48-bit virtual address of the signature section
.set va_rvtest_vmem_begin, 0xFFFF900000000000 // 48-bit virtual address of vmem
Copy link

@nadime15 nadime15 May 30, 2025

Choose a reason for hiding this comment

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

Ok this one is quite tricky and it took me literally days to find out what is going on here:

So the way it's set up right now you will run into an illegal instruction exception once you go from M-Mode to Supervisor-Mode:

[411] [S]: 0xFFFF800000000774 (0x0000) c.illegal 0x0 // From Sail, tries to execute first instruction of `vm_en`

This happens because of how the addresses were picked and the use of sv48, plus, the fact that PTE's at LEVEL3 are leaves with gigapages instead of using a multi-level page table with a page table walk and using a smaller page size.

So the problem is the following paddr:

.set pa_rvtest_code_begin, 0x8000000000039c // 56-bit physical address of the code section

this will eventually create a mapping from VA 0xFFFF900000000000 to PA 0x8000000000039C, which points to nowhere.

From what I can tell by reading the code, there is no remapping of the code from its initial addr:

0x80000000 <rvtest_entry_point>:

to

0x80000000000000 <rvtest_entry_point>:

so the initialization of the PTE down below:

LI (a0, pa_rvtest_code_begin)          // Load physical address of code
LI (a1, (PTE_V | PTE_A | PTE_W | PTE_R | PTE_D | PTE_X)) // Set permission bits
PTE_SETUP_RV64(a0, a1, t0, t1, va_rvtest_code_begin, LEVEL3, sv48) // Set up level 3 PTE

will not work as intended.

Unfortunately you can't just change the mapping from:

.set pa_rvtest_code_begin, 0x8000000000039c

to

.set pa_rvtest_code_begin, 0x8000039c

because as i said at the beginning, the fact that the code tries to set up gigabyte pages will result later in a page-fault because of the following rule:

5 ) A leaf PTE has been reached. If i>0 and pte.ppn[i-1:0] ≠ 0, this is a misaligned superpage; stop
and raise a page-fault exception corresponding to the original access type.

ppn[2:0] of 0x8000039c is non zero

So there are a few ways to get around this:

1 ) You actually have to remap your code from 0x8000039c to 0x8000000000039c (basically copy-paste it while in M-Mode)

2 ) You have to set up a proper multi-level page table and deal with smaller page sizes (meaning you have to go down to LEVEL2 or maybe even LEVEL0)

3 ) You modify the linker script to link the code at 0x80000000000000 instead of 0x80000000.

I decided to go for 3 ) which I only did to the get the test running, but we can not push this solution because this would modify the linker script.

On top of that you have to pay attention to these addresses:


.set va_rvtest_code_begin,  	0xFFFF80000000039c    // 48-bit virtual address of the code section						
.set va_rvtest_data_begin,  	0xFFFF900000000000    // 48-bit virtual address of the data section

They have different VPN[3] values, so va_rvtest_code_begin.VPN[3] != va_rvtest_data_begin.VPN[3], meaning each will map to a different memory address for their corresponding PTE entry.

And finally, I modify the defined addresses.:

Suggested change
# --------------------------- Define Addresses -------------------------------
.set pa_rvtest_code_begin, 0x8000000000039c // 56-bit physical address of the code section
.set pa_rvtest_data_begin, 0x80000000003530 // 56-bit physical address of the data section
.set pa_rvtest_sig_begin, 0x80000000006218 // 56-bit physical address of the signature section
.set va_rvtest_code_begin, 0xFFFF80000000039c // 48-bit virtual address of the code section
.set va_rvtest_data_begin, 0xFFFF900000000000 // 48-bit virtual address of the data section
.set va_rvtest_sig_begin, 0xFFFF900000006218 // 48-bit virtual address of the signature section
.set va_rvtest_vmem_begin, 0xFFFF900000000000 // 48-bit virtual address of vmem
# --------------------------- Define Addresses -------------------------------
.set pa_rvtest_code_begin, 0x8000000000039c // 56-bit physical address of the code section
.set pa_rvtest_data_begin, 0x80000000003530 // 56-bit physical address of the data section
.set pa_rvtest_sig_begin, 0x80000000006218 // 56-bit physical address of the signature section
.set va_rvtest_code_begin, 0xFFFF80000000039c // 48-bit virtual address of the code section
.set va_rvtest_data_begin, 0xFFFF800000003530 // 48-bit virtual address of the data section
.set va_rvtest_sig_begin, 0xFFFF800000006218 // 48-bit virtual address of the signature section
.set va_rvtest_vmem_begin, 0xFFFF800000000000 // 48-bit virtual address of vmem

After these changes I was able to successfully run the test.

similar changes in regard to the addresses must be made for the other 3 cases as well (tag00, tag01 and tag10)

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