lkml.org 
[lkml]   [2021]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 -next] riscv: mm: remove redundant trampoline PGD for 64bit
From
Date


Le 28/07/2021 à 04:49, Nanyong Sun a écrit :
> Remove redundant trampoline PGD for 64bit and add more comment
> for why 32bit systems need trampoline PGD.
>
> There was a patch and discussion similar to this,refer to
> the link [1][2].
>
> The trampoline PGD is redundant for 64bit systems because:
> 1. The early PGD covers the entire kernel mapping. Directly
> loading early PGD can achieve the result in boot stage.
> A more trampoline PGD makes code hard to understand.
> 2. Directly loading early PGD is safe in 64bit systems since
> the kernel virtual address starts as 0xFFFFxxxxxxxxxxxx,
> which has a very big gap with RAM address.It won't fall into
> the corner case that 32bit system worrys.
> 3. Remove redundant trampoline PGD can benefit to code maintaince,
> because 64bit systems have more page table levels.For example:
> If we want to support SV48 which has 4 page table levels, we have
> to add a trampoline_pud and insert it before trampoline_pmd.
>
> Reference link:
> [1]https://lore.kernel.org/linux-riscv/20190325092234.5451-4-anup.patel@wdc.com/
> [2]https://lkml.org/lkml/2019/3/28/147
>
> Signed-off-by: Nanyong Sun <sunnanyong@huawei.com>
> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>

I have not reviewed this patch yet! :)

> Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> Reviewed-by: weiyongjun <weiyongjun1@huawei.com>
> ---
> v2 changes:
> Adjust codes based on the review suggestions from Alex Ghiti,
> make codes more readable.
> ---
> arch/riscv/kernel/head.S | 46 ++++++++++++++++++++++++++--------------
> arch/riscv/mm/init.c | 16 ++++----------
> 2 files changed, 34 insertions(+), 28 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index fce5184b22c3..3816aa5edc69 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -89,29 +89,52 @@ relocate:
> add ra, ra, a1
>
> /* Point stvec to virtual address of intruction after satp write */
> - la a2, 1f
> +#ifdef CONFIG_64BIT
> + la a2, load_done
> +#else
> + la a2, load_kernel_pgd
> +#endif
> add a2, a2, a1
> csrw CSR_TVEC, a2
>
> - /* Compute satp for kernel page tables, but don't load it yet */
> + /* Compute satp for kernel page tables.
> + * For 64bit systems, load it and trap to stvec.
> + * For 32bit systems, don't load it yet.
> + */
> srl a2, a0, PAGE_SHIFT
> li a1, SATP_MODE
> or a2, a2, a1
>
> /*
> + * Before writing satp, we need a full fence here because setup_vm() just
> + * wrote these PTEs and we need to ensure the new translations are in use.
> + */
> + sfence.vma
> +#ifndef CONFIG_64BIT
> + /*
> + * 32bit systems need firstly loading a trampoline to handle a corner
> + * case where load address range overlaps kernel virtual address range.
> * Load trampoline page directory, which will cause us to trap to
> - * stvec if VA != PA, or simply fall through if VA == PA. We need a
> - * full fence here because setup_vm() just wrote these PTEs and we need
> - * to ensure the new translations are in use.
> + * stvec if VA != PA, or simply fall through if VA == PA.
> */
> la a0, trampoline_pg_dir
> XIP_FIXUP_OFFSET a0
> srl a0, a0, PAGE_SHIFT
> or a0, a0, a1
> - sfence.vma
> csrw CSR_SATP, a0
> +#endif
> .align 2
> -1:
> +load_kernel_pgd:
> + /*
> + * Switch to kernel page tables. A full fence is necessary in order to
> + * avoid using the trampoline translations, which are only correct for
> + * the first superpage. Fetching the fence is guarnteed to work
> + * because that first superpage is translated the same way.
> + */
> + csrw CSR_SATP, a2
> + sfence.vma
> +
> +load_done:
> /* Set trap vector to spin forever to help debug */
> la a0, .Lsecondary_park
> csrw CSR_TVEC, a0
> @@ -122,15 +145,6 @@ relocate:
> la gp, __global_pointer$
> .option pop
>
> - /*
> - * Switch to kernel page tables. A full fence is necessary in order to
> - * avoid using the trampoline translations, which are only correct for
> - * the first superpage. Fetching the fence is guarnteed to work
> - * because that first superpage is translated the same way.
> - */
> - csrw CSR_SATP, a2
> - sfence.vma
> -
> ret
> #endif /* CONFIG_MMU */
> #ifdef CONFIG_SMP
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index ac48742fa6fc..306fcb2334fa 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -219,13 +219,17 @@ unsigned long pfn_base __ro_after_init;
> EXPORT_SYMBOL(pfn_base);
>
> pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#ifndef CONFIG_64BIT
> pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#endif /* CONFIG_64BIT */
> static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
>
> pgd_t early_pg_dir[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>
> #ifdef CONFIG_XIP_KERNEL
> +#ifndef CONFIG_64BIT
> #define trampoline_pg_dir ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
> +#endif /* CONFIG_64BIT */
> #define fixmap_pte ((pte_t *)XIP_FIXUP(fixmap_pte))
> #define early_pg_dir ((pgd_t *)XIP_FIXUP(early_pg_dir))
> #endif /* CONFIG_XIP_KERNEL */
> @@ -300,13 +304,11 @@ static void __init create_pte_mapping(pte_t *ptep,
>
> #ifndef __PAGETABLE_PMD_FOLDED
>
> -static pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
> static pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
> static pmd_t early_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
> static pmd_t early_dtb_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
>
> #ifdef CONFIG_XIP_KERNEL
> -#define trampoline_pmd ((pmd_t *)XIP_FIXUP(trampoline_pmd))
> #define fixmap_pmd ((pmd_t *)XIP_FIXUP(fixmap_pmd))
> #define early_pmd ((pmd_t *)XIP_FIXUP(early_pmd))
> #endif /* CONFIG_XIP_KERNEL */
> @@ -585,16 +587,6 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> /* Setup fixmap PMD */
> create_pmd_mapping(fixmap_pmd, FIXADDR_START,
> (uintptr_t)fixmap_pte, PMD_SIZE, PAGE_TABLE);
> - /* Setup trampoline PGD and PMD */
> - create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
> - (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
> -#ifdef CONFIG_XIP_KERNEL
> - create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> - kernel_map.xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#else
> - create_pmd_mapping(trampoline_pmd, kernel_map.virt_addr,
> - kernel_map.phys_addr, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#endif
> #else
> /* Setup trampoline PGD */
> create_pgd_mapping(trampoline_pg_dir, kernel_map.virt_addr,
>

\
 
 \ /
  Last update: 2021-07-28 13:55    [W:1.625 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site