On 9/21/26 12:55, Yeoreum Yun wrote:
> The behaviour of pXdp_get() will change with generic compile-time folded
> page tables, making it invalid to use set_pXd() with the result of
> pXdp_get(). Instead, the appropriate first-level set_pXd() must be paired
> with the corresponding pXdp_get().
>
> Therefore, call the appropriate set_pXd() for generic compile-time folded
> page tables so that vmalloc_fault() continues to handle folded entries
> correctly when checking pXd_present().
>
> There should be no functional change.
>
> Since most architectures use a similar pattern to handle vmalloc fault
> by syncing the faulted entry, it would be worth cleaning this up by
> introducing a common helper in the near future.
>
> Signed-off-by: Yeoreum Yun <[email protected]>
> ---
@ Riscv maintainers, is this good enough until we get some more generic cleanups
in this area?
> arch/riscv/mm/fault.c | 52
> ++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 04ed6f8acae4f..b2bcaf10d13f1 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -167,14 +167,16 @@ bad_area(struct pt_regs *regs, struct mm_struct *mm,
> int code,
>
> static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned
> long addr)
> {
> - pgd_t *pgd, *pgd_k;
> - pud_t *pud_k;
> - p4d_t *p4d_k;
> - pmd_t *pmd_k;
> - pte_t *pte_k;
> + pgd_t *pgdp_k, *pgdp, pgd_k;
> + p4d_t *p4dp_k, *p4dp;
> + pud_t *pudp_k, *pudp, pud_k;
> + pmd_t *pmdp_k, *pmdp, pmd_k;
> + pte_t *ptep_k;
> int index;
> unsigned long pfn;
>
> + BUILD_BUG_ON(CONFIG_PGTABLE_LEVELS != 5 && CONFIG_PGTABLE_LEVELS != 2);
> +
> /* User mode accesses just cause a SIGSEGV */
> if (user_mode(regs))
> return do_trap(regs, SIGSEGV, code, addr);
> @@ -189,39 +191,51 @@ static inline void vmalloc_fault(struct pt_regs *regs,
> int code, unsigned long a
> */
> index = pgd_index(addr);
> pfn = csr_read(CSR_SATP) & SATP_PPN;
> - pgd = (pgd_t *)pfn_to_virt(pfn) + index;
> - pgd_k = init_mm.pgd + index;
>
> - if (!pgd_present(pgdp_get(pgd_k))) {
> + pgdp = (pgd_t *)pfn_to_virt(pfn) + index;
> + pgdp_k = init_mm.pgd + index;
> +
> + pgd_k = pgdp_get(pgdp_k);
> + if (!pgd_present(pgd_k)) {
> no_context(regs, addr);
> return;
> }
> - set_pgd(pgd, pgdp_get(pgd_k));
> + if (CONFIG_PGTABLE_LEVELS == 5)
> + set_pgd(pgdp, pgd_k);
>
> - p4d_k = p4d_offset(pgd_k, addr);
> - if (!p4d_present(p4dp_get(p4d_k))) {
> + p4dp = p4d_offset(pgdp, addr);
> + p4dp_k = p4d_offset(pgdp_k, addr);
> + if (!p4d_present(p4dp_get(p4dp_k))) {
> no_context(regs, addr);
> return;
> }
>
> - pud_k = pud_offset(p4d_k, addr);
> - if (!pud_present(pudp_get(pud_k))) {
> + pudp = pud_offset(p4dp, addr);
> + pudp_k = pud_offset(p4dp_k, addr);
> +
> + pud_k = pudp_get(pudp_k);
> + if (!pud_present(pud_k)) {
> no_context(regs, addr);
> return;
> }
> - if (pud_leaf(pudp_get(pud_k)))
> + if (pud_leaf(pud_k))
> goto flush_tlb;
>
> /*
> * Since the vmalloc area is global, it is unnecessary
> * to copy individual PTEs
> */
> - pmd_k = pmd_offset(pud_k, addr);
> - if (!pmd_present(pmdp_get(pmd_k))) {
> + pmdp = pmd_offset(pudp, addr);
> + pmdp_k = pmd_offset(pudp_k, addr);
> +
> + pmd_k = pmdp_get(pmdp_k);
> + if (!pmd_present(pmd_k)) {
> no_context(regs, addr);
> return;
> }
> - if (pmd_leaf(pmdp_get(pmd_k)))
> + if (CONFIG_PGTABLE_LEVELS == 2)
> + set_pmd(pmdp, pmd_k);
> + if (pmd_leaf(pmd_k))
> goto flush_tlb;
>
> /*
> @@ -230,8 +244,8 @@ static inline void vmalloc_fault(struct pt_regs *regs,
> int code, unsigned long a
> * addresses. If we don't do this, this will just
> * silently loop forever.
> */
> - pte_k = pte_offset_kernel(pmd_k, addr);
> - if (!pte_present(ptep_get(pte_k))) {
> + ptep_k = pte_offset_kernel(pmdp_k, addr);
> + if (!pte_present(ptep_get(ptep_k))) {
> no_context(regs, addr);
> return;
> }
>
--
Cheers,
David
