[PATCH] RISC-V: fix page table mode detection on systems without TLB

[Xu, Zefan]

The previous page table mode dectection performed a risky operation: clearing the page talbe in memory before disabling MMU in CSR satp. This approach works fine on systems like QEMU, SPIKE or most CPUs, which have a TLB that caches PTEs. The TLB retains the cached address translation state until explicitly clearing by sfence.vma instruction.

However, on systems without TLB, such as OpenXiangShan/NEMU, clearing page table immediately affects virtual address traslation, causing the system would trap into endless page fault exception.

This patch modifies the detection procedure inspired by Linux：it immediately clears the satp register right after setting it. This patch also remove some redundant page table clearing operation.

Signed-off-by: "Xu, Zefan" <ceba_...@outlook.com>
---
.../riscv/cpu/generic/cpu_mmu_initial_pgtbl.c | 27 +++++++++++--------
1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/cpu/generic/cpu_mmu_initial_pgtbl.c b/arch/riscv/cpu/generic/cpu_mmu_initial_pgtbl.c
index 4b471736..64e646da 100644
--- a/arch/riscv/cpu/generic/cpu_mmu_initial_pgtbl.c
+++ b/arch/riscv/cpu/generic/cpu_mmu_initial_pgtbl.c
@@ -258,7 +258,7 @@ void __attribute__ ((section(".entry")))
{
#ifdef CONFIG_64BIT
u32 i, index;
- unsigned long satp;
+ unsigned long satp, hw_satp;
arch_pte_t *pgtbl =
(arch_pte_t *)to_load_pa((virtual_addr_t)&stage1_pgtbl_root);

@@ -280,20 +280,18 @@ void __attribute__ ((section(".entry")))
pgtbl[index] |= PGTBL_PTE_VALID_MASK;
satp = (unsigned long)pgtbl >> PGTBL_PAGE_SIZE_SHIFT;
satp |= SATP_MODE_SV57 << SATP_MODE_SHIFT;
+
+ /* Set and clear satp */
__sfence_vma_all();
csr_write(CSR_SATP, satp);
- if ((csr_read(CSR_SATP) >> SATP_MODE_SHIFT) == SATP_MODE_SV57) {
+ hw_satp = csr_swap(CSR_SATP, 0);
+ __sfence_vma_all();
+
+ if ((hw_satp >> SATP_MODE_SHIFT) == SATP_MODE_SV57) {
riscv_stage1_mode = SATP_MODE_SV57;
goto skip_sv48_test;
}

- /* Cleanup and disable MMU */
- for (i = 0; i < PGTBL_ROOT_ENTCNT; i++) {
- pgtbl[i] = 0x0ULL;
- }
- csr_write(CSR_SATP, 0);
- __sfence_vma_all();
-
/* Clear page table memory */
for (i = 0; i < PGTBL_ROOT_ENTCNT; i++) {
pgtbl[i] = 0x0ULL;
@@ -312,17 +310,24 @@ void __attribute__ ((section(".entry")))
pgtbl[index] |= PGTBL_PTE_VALID_MASK;
satp = (unsigned long)pgtbl >> PGTBL_PAGE_SIZE_SHIFT;
satp |= SATP_MODE_SV48 << SATP_MODE_SHIFT;
+
+ /* Set and clear satp */
__sfence_vma_all();
csr_write(CSR_SATP, satp);
- if ((csr_read(CSR_SATP) >> SATP_MODE_SHIFT) == SATP_MODE_SV48) {
+ hw_satp = csr_swap(CSR_SATP, 0);
+ __sfence_vma_all();
+
+ if ((hw_satp >> SATP_MODE_SHIFT) == SATP_MODE_SV48) {
riscv_stage1_mode = SATP_MODE_SV48;
}

skip_sv48_test:
- /* Cleanup and disable MMU */
+ /* Clear page table memory */
for (i = 0; i < PGTBL_ROOT_ENTCNT; i++) {
pgtbl[i] = 0x0ULL;
}
+
+ /* Disable MMU */
csr_write(CSR_SATP, 0);
__sfence_vma_all();
#endif
--
2.43.0

[Anup Patel]

On Tue, Sep 3, 2024 at 12:54 PM Xu, Zefan <ceba_...@outlook.com> wrote:
>
> The previous page table mode dectection performed a risky operation: clearing the page talbe in memory before disabling MMU in CSR satp. This approach works fine on systems like QEMU, SPIKE or most CPUs, which have a TLB that caches PTEs. The TLB retains the cached address translation state until explicitly clearing by sfence.vma instruction.

The only assumption in this page table mode detection code is that
Xvisor (just like Linux) is booted with MMU off (i.e. satp.mode =
Bare). There is certainly no assumption in this code about TLB
retaining cached entry.

The RISC-V privileged specification section "Supervisor Address
Translation and Protection (satp) Register" tells the following
"Implementations are not required to support all MODE settings, and if
satp is written with an unsupported MODE, the entire write has no
effect; no fields in satp are modified."

Please look at the code again. We are setting up identity mapping
before testing for a page table mode so only two things will happen:
1) If the page table mode is supported then HW will pick-up the
identity mapping and code will continue to run without faults.
2) If the page table mode is not supported then HW will fallback to
"bare" mode and code will continue to run without faults.

>
> However, on systems without TLB, such as OpenXiangShan/NEMU, clearing page table immediately affects virtual address traslation, causing the system would trap into endless page fault exception.

I think there is some HW issue with your TLB-less implementation.

Regards,
Anup

> --
> You received this message because you are subscribed to the Google Groups "Xvisor Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to xvisor-devel...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/xvisor-devel/SY8P282MB473257FF833C83C2EBFD1689E2932%40SY8P282MB4732.AUSP282.PROD.OUTLOOK.COM.