From 8e34f567ef9c6a6f371492e8b9fdb2234107d71d Mon Sep 17 00:00:00 2001 From: Baptiste Le Duc Date: Tue, 27 Oct 2026 08:59:18 +0100 Subject: [PATCH] xen/riscv: fix A/D bits in Xen's page-table mappings Xen does not handle page faults caused by clear A/D bits, so it presets them when creating PTEs. pt_update_entry() does so, but three places build leaf PTEs directly without going through it: setup_initial_mapping() (the boot page tables), check_pgtbl_mode_support() (the temporary root entry used to probe SATP mode support) and arch_pmap_map() (the fixmap). On implementations that fault on clear A/D bits (Svade), all three fault on first access. For check_pgtbl_mode_support(), the fault is raised by the instruction fetch right after the SATP write, before any trap handler is set. Add PTE_ACCESSED and PTE_DIRTY to PAGE_HYPERVISOR_RO, and build PAGE_HYPERVISOR_RW and PAGE_HYPERVISOR_RX on top of it. PTE_DIRTY is set in all cases for consistency with pt_update_entry() which sets it at runtime. This fixes arch_pmap_map() for free, since it already builds its PTE from PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these macros for its default, text and rodata permissions, and check_pgtbl_mode_support() to use PAGE_HYPERVISOR_RX for its temporary root entry. The latter drops PTE_WRITABLE, going from RWX to RX, but this is harmless, as that entry only has to make the current instruction stream fetchable between the two CSR_SATP writes, and nothing writes through it. Drop the now-redundant PTE_LEAF_DEFAULT, since converting the last open-coded site above leaves it with no user outside page.h itself. pte_is_table() and pte_is_mapping() both assert that a PTE doesn't use one of the two reserved encodings, (V=1, W=1, R=0) and (V=1, X=1, W=1, R=0), by masking it with PAGE_HYPERVISOR_RW. Now that PAGE_HYPERVISOR_RW also carries A and D, a reserved PTE with A or D set would no longer be caught. Mask with the V, R and W bits explicitly instead, and factor the check out into pte_has_reserved_rwx(). Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages") Fixes: f89e1a170299 ("xen/riscv: introduce asm/pmap.h header") Assisted-by: Claude:claude-opus-5 Signed-off-by: Baptiste Le Duc Reviewed-by: Oleksii Kurochko Tested-by: Zhang Zheng Acked-by: Jan Beulich --- xen/arch/riscv/include/asm/page.h | 33 ++++++++++++++++--------------- xen/arch/riscv/mm.c | 11 +++++------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h index c8bb334b85..f9e59e61de 100644 --- a/xen/arch/riscv/include/asm/page.h +++ b/xen/arch/riscv/include/asm/page.h @@ -46,12 +46,12 @@ #define PTE_PBMT_NOCACHE BIT(61, UL) #define PTE_PBMT_IO BIT(62, UL) -#define PTE_LEAF_DEFAULT (PTE_VALID | PTE_READABLE | PTE_WRITABLE) #define PTE_TABLE (PTE_VALID) -#define PAGE_HYPERVISOR_RO (PTE_VALID | PTE_READABLE) -#define PAGE_HYPERVISOR_RW (PTE_VALID | PTE_READABLE | PTE_WRITABLE) -#define PAGE_HYPERVISOR_RX (PTE_VALID | PTE_READABLE | PTE_EXECUTABLE) +#define PAGE_HYPERVISOR_RO (PTE_VALID | PTE_READABLE | \ + PTE_ACCESSED | PTE_DIRTY) +#define PAGE_HYPERVISOR_RW (PAGE_HYPERVISOR_RO | PTE_WRITABLE) +#define PAGE_HYPERVISOR_RX (PAGE_HYPERVISOR_RO | PTE_EXECUTABLE) #define PAGE_HYPERVISOR PAGE_HYPERVISOR_RW /* @@ -161,31 +161,32 @@ static inline bool pte_is_valid(pte_t p) * X W R Meaning * 0 0 0 Pointer to next level of page table. * 0 0 1 Read-only page. - * 0 1 0 Reserved for future use. + * 0 1 0 Reserved for future use. [1] * 0 1 1 Read-write page. * 1 0 0 Execute-only page. * 1 0 1 Read-execute page. - * 1 1 0 Reserved for future use. + * 1 1 0 Reserved for future use. [2] * 1 1 1 Read-write-execute page. + * + * So if V=1 and W=1 then R also needs to be 1 as R = 0 is reserved for + * future use ([1], [2]). */ +static inline bool pte_has_reserved_rwx(pte_t p) +{ + return (p.pte & (PTE_VALID | PTE_READABLE | PTE_WRITABLE)) == + (PTE_VALID | PTE_WRITABLE); +} + static inline bool pte_is_table(pte_t p) { - /* - * According to the spec if V=1 and W=1 then R also needs to be 1 as - * R = 0 is reserved for future use ( look at the Table 4.5 ) so check - * in ASSERT that if (V==1 && W==1) then R isn't 0. - * - * PAGE_HYPERVISOR_RW contains PTE_VALID too. - */ - ASSERT(((p.pte & PAGE_HYPERVISOR_RW) != (PTE_VALID | PTE_WRITABLE))); + ASSERT(!pte_has_reserved_rwx(p)); return ((p.pte & (PTE_VALID | PTE_ACCESS_MASK)) == PTE_VALID); } static inline bool pte_is_mapping(pte_t p) { - /* See pte_is_table() */ - ASSERT(((p.pte & PAGE_HYPERVISOR_RW) != (PTE_VALID | PTE_WRITABLE))); + ASSERT(!pte_has_reserved_rwx(p)); return pte_is_valid(p) && (p.pte & PTE_ACCESS_MASK); } diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c index 06f0406781..6f7dcd10b2 100644 --- a/xen/arch/riscv/mm.c +++ b/xen/arch/riscv/mm.c @@ -140,19 +140,18 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc, case 1: /* Level 0 */ { unsigned long paddr = (page_addr - map_start) + pa_start; - unsigned int pte_flags = PTE_LEAF_DEFAULT; + unsigned int pte_flags = PAGE_HYPERVISOR_RW; unsigned long addr = is_identity_mapping ? page_addr : virt_to_maddr(page_addr); pte_t pte_to_be_written; index = pt_index(0, page_addr); - if ( is_kernel_text(addr) || - is_kernel_inittext(addr) ) - pte_flags = PTE_EXECUTABLE | PTE_READABLE | PTE_VALID; + if ( is_kernel_text(addr) || is_kernel_inittext(addr) ) + pte_flags = PAGE_HYPERVISOR_RX; if ( is_kernel_rodata(addr) ) - pte_flags = PTE_READABLE | PTE_VALID; + pte_flags = PAGE_HYPERVISOR_RO; pte_to_be_written = paddr_to_pte(paddr, pte_flags); @@ -197,7 +196,7 @@ static bool __init check_pgtbl_mode_support(struct mmu_desc *mmu_desc, index = pt_index(page_table_level, aligned_load_start); stage1_pgtbl_root[index] = paddr_to_pte(aligned_load_start, - PTE_LEAF_DEFAULT | PTE_EXECUTABLE); + PAGE_HYPERVISOR_RX); sfence_vma(); csr_write(CSR_SATP,