kendryte / k230_sdk

Kendryte K230 SDK
BSD 2-Clause "Simplified" License
165 stars 35 forks source link

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

Open cyyself opened 9 months ago

cyyself commented 9 months ago

Feature description

Remove these 3 lines which lock PMP entries: https://github.com/kendryte/k230_sdk/blob/3ade2d07710fbbd5ab2e0919aa29298696873750/src/little/uboot/arch/riscv/cpu/k230/cpu.c#L82-L84 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: https://github.com/kendryte/k230_sdk/blob/3ade2d07710fbbd5ab2e0919aa29298696873750/src/little/uboot/arch/riscv/cpu/k230/cpu.c#L45

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 commented 9 months ago

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 commented 8 months ago

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