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

[Feature]: Consider remove PMP locking and don't set MXSTATUS.MAEE from U-Boot to support generic OpenSBI #48

Open
cyyself opened this issue Dec 12, 2023 · 2 comments · May be fixed by #53
Assignees

Comments

@cyyself
Copy link

cyyself commented Dec 12, 2023

Feature description

Remove these 3 lines which lock PMP entries:

csr_write(pmpaddr0, 0x24484dff);//start addr:0x24484c00<<2=0x91213000 len=1<<9 * 8 = 4KB
csr_write(pmpaddr1, 0x244851ff);//start addr:0x24485000<<2=0x91214000 len=1<<9 * 8 = 4KB
csr_write(pmpcfg0, 0x9999);

Change this line to csr_write(CSR_MXSTATUS, 0x438000) to disable the MAEE extension and move it to customized Kernel with T-HEAD MAEE Support:
csr_write(CSR_MXSTATUS, 0x638000);

Want resolve what problem

The locking PMP entries in U-Boot caused the generic OpenSBI not to enter S-Mode due to OpenSBI always assumes no PMP is locking. I tried to make OpenSBI probe locked PMP entries and send patches to the OpenSBI mailing list, a maintainer said "Please reconsider and fix your boot flow.". The best way to solve this is not to set locked PMPs in any previous boot stages before SBI.

Meanwhile, the U-Boot turned on MAXSTATUS.MAEE which changes the PTE format from RISC-V standard to T-HEAD MAEE standard, caused the unmodified OS kernel to fail to start. It will make the mainline OpenSBI and kernel support not easy.

Anything else

I finally agreed with this opinion from the OpenSBI maintainer after I tried to edit the subset of patches which does not ACKed by the maintainer. It will be hard for a generic SBI to support this situation like some PMPs are locked but we can turn on MSECCFG.RLB, so clean the existing locked PMPs. In addition, the T-Head RISC-V implementation raises a trap when writing to locked pmpaddr CSR making the software support even worse as I mentioned in this patch.

As for MAEE set by default. Everyone like me working on mainline Linux support may be knowledgeable with RISC-V specs but not T-Head spec, spending time to find out why the software failed on cache coherence issues as MAEE will add Cache attributes in the PTE and leaves them 0 refers to Non-cacheable memory, thus making the dirty cache line which exists before csr.satp changes lost after csr.satp changes, which might cause stack corruption like CacheWarp attack confuses the developer who does not know about T-Head Extensions.

@cyyself
Copy link
Author

cyyself commented Dec 12, 2023

Meanwhile, please also consider removing BIT(16) from CSR_MXSTATUS by default which enables User Mode Cache Management Extension which might be used for CacheWarp-like attacks.

@wangjianxin-canaan
Copy link

Thank you for your advice. I guess I will take it.

yf13 added a commit to yf13/k230_sdk that referenced this issue Jan 15, 2024
Per discussions in issue#48, we should leave PMP to firmware
or OS thus allowing mainline OpenSBI or different OS be loaded.

MAEE extension should also be turned off for standard compliance.

Signed-off-by: Yanfeng Liu <[email protected]>
@yf13 yf13 linked a pull request Jan 15, 2024 that will close this issue
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 a pull request may close this issue.

5 participants