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

[BUG] PMP config can not select NA4 mode while granularity is set to 4 #2465

Closed
1 task done
riscv914 opened this issue Aug 26, 2024 · 3 comments · Fixed by #2469
Closed
1 task done

[BUG] PMP config can not select NA4 mode while granularity is set to 4 #2465

riscv914 opened this issue Aug 26, 2024 · 3 comments · Fixed by #2469
Labels
Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system

Comments

@riscv914
Copy link

Is there an existing CVA6 bug for this?

  • I have searched the existing bug issues

Bug Description

While the granularity of the PMP can determine by following code is 4 bytes:

    asm volatile ("csrw pmpcfg0, x0");
    asm volatile ("addi a3,x0,-1");
    asm volatile ("csrw pmpaddr0, a3");
    asm volatile ("csrr a3, pmpaddr0"); // a3=0x003FFFFFFFFFFFFF -> 

a3 = 0x003FFFFFFFFFFFFF so, the least-significant bit that set is 0, so the PMP granularity is 4 bytes.

But CVA6 does not support setting the NA4 address matching mode. In other words, if we set the address matching mode to 0x2, the core rewrites the register with 0x0, which means no protection.

To reproduce, execute following instruction:

int main(void){
    asm volatile ("auipc	a3,0x0");
    asm volatile ("addi     a3,a3,0x24");
    asm volatile ("addi   t1,x0,0x2");
    asm volatile ("srl      t1,a3,t1");
    asm volatile ("csrw pmpaddr0, t1");
    asm volatile ("addi t0,x0,0x94");
    asm volatile ("csrw pmpcfg0, t0");

    asm volatile ("csrr a4, pmpcfg0");
    asm volatile ("csrr a4, pmpaddr0");
    asm volatile ("auipc    a3,0x0");
    asm volatile ("lbu a4,0(a3)");
}

However, if you execute same code on Spike you will get trap_load_access_fault exception.

- Spike version: `1.1.1-dev`
- version: `CVA6 commit: 7435cb310ee98c246edd15d00ec236ad2c6ff490`
- OS: `CentOS Linux release 7.9.2009 kernel: 5.15.0-78-generic`
- Simulator: `VCS_2023`
@riscv914 riscv914 added the Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system label Aug 26, 2024
@Moschn
Copy link
Contributor

Moschn commented Aug 27, 2024

This is expected behavior. CVA6 does not support low granularity for PMP, and NA4 is the lowest granularity. Spike on the other hand supports all granularities. Actually, the RISC-V privilege spec proposes a similar procedure as the one you posted to determine the granularity supported by a particular core:

Software may determine the PMP granularity by writing zero to pmp0cfg, then writing all ones to pmpaddr0, then reading back pmpaddr0. If G is the index of the least-significant bit set, the PMP granularity is 2G+2 bytes

@riscv914
Copy link
Author

@Moschn
Thank you for your reply. As you mentioned correctly, we can determine the granularity by proposed procedure. However, the return granularity shows that core should support 4 bytes granularity which is low and equivalent to NA4.
Here could be two possibilities, 1) Core reports wrong granularity to software. 2) Core reports correct granularity to software but did not implement that granularity correctly.

Please consider, in both cases the core has bug and should fix.

Moschn added a commit to Moschn/ariane that referenced this issue Aug 27, 2024
@Moschn
Copy link
Contributor

Moschn commented Aug 27, 2024

Oh, sorry for that, I did not read your issue thoroughly enough. I found the regression and proposed a fix in #2469.

@github-staff github-staff deleted a comment Aug 27, 2024
Moschn added a commit to Moschn/ariane that referenced this issue Sep 18, 2024
Moschn added a commit to Moschn/ariane that referenced this issue Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Bug For bugs in the RTL, Documentation, Verification environment or Tool and Build system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@Moschn @riscv914 and others