All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-05-27 14:48 ` Nanyong Sun
  0 siblings, 0 replies; 10+ messages in thread
From: Nanyong Sun @ 2021-05-27 14:48 UTC (permalink / raw)
  To: paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra,
	wangkefeng.wang, sunnanyong

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>
---
 arch/riscv/kernel/head.S | 13 +++++++++++--
 arch/riscv/mm/init.c     | 21 +++++++--------------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 89cc58ab52b4..1897b17c5fcc 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -93,12 +93,18 @@ relocate:
 	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 */
 	srl a2, a0, PAGE_SHIFT
 	li a1, SATP_MODE
 	or a2, a2, a1
-
+#ifdef CONFIG_64BIT
+	/* Load kernel page directory */
+	sfence.vma
+	csrw CSR_SATP, a2
+#else
 	/*
+	 * 32bit system need 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
@@ -110,6 +116,7 @@ relocate:
 	or a0, a0, a1
 	sfence.vma
 	csrw CSR_SATP, a0
+#endif /* CONFIG_64BIT */
 .align 2
 1:
 	/* Set trap vector to spin forever to help debug */
@@ -122,6 +129,7 @@ relocate:
 	la gp, __global_pointer$
 .option pop
 
+#ifdef CONFIG_32BIT
 	/*
 	 * Switch to kernel page tables.  A full fence is necessary in order to
 	 * avoid using the trampoline translations, which are only correct for
@@ -130,6 +138,7 @@ relocate:
 	 */
 	csrw CSR_SATP, a2
 	sfence.vma
+#endif /* CONFIG_32BIT */
 
 	ret
 #endif /* CONFIG_MMU */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 11b61bea0c4d..b7226ac2d04f 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -210,13 +210,17 @@ unsigned long pfn_base __ro_after_init;
 EXPORT_SYMBOL(pfn_base);
 
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
+#ifdef CONFIG_32BIT
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
+#endif /* CONFIG_32BIT */
 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
+#ifdef CONFIG_32BIT
 #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
+#endif /* CONFIG_32BIT */
 #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
 #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
 #endif /* CONFIG_XIP_KERNEL */
@@ -291,13 +295,11 @@ static void __init create_pte_mapping(pte_t *ptep,
 
 #ifndef __PAGETABLE_PMD_FOLDED
 
-pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
 pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
 pmd_t early_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
 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 */
@@ -543,21 +545,12 @@ 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_virt_addr,
-			   (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
-#ifdef CONFIG_XIP_KERNEL
-	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
-			   xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
-#else
-	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
-			   load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
-#endif
-#else
+#endif /* __PAGETABLE_PMD_FOLDED */
+#ifdef CONFIG_32BIT
 	/* Setup trampoline PGD */
 	create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
 			   load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
-#endif
+#endif /* CONFIG_32BIT */
 
 	/*
 	 * Setup early PGD covering entire kernel which will allow
-- 
2.18.0.huawei.25


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-05-27 14:48 ` Nanyong Sun
  0 siblings, 0 replies; 10+ messages in thread
From: Nanyong Sun @ 2021-05-27 14:48 UTC (permalink / raw)
  To: paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra,
	wangkefeng.wang, sunnanyong

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>
---
 arch/riscv/kernel/head.S | 13 +++++++++++--
 arch/riscv/mm/init.c     | 21 +++++++--------------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 89cc58ab52b4..1897b17c5fcc 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -93,12 +93,18 @@ relocate:
 	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 */
 	srl a2, a0, PAGE_SHIFT
 	li a1, SATP_MODE
 	or a2, a2, a1
-
+#ifdef CONFIG_64BIT
+	/* Load kernel page directory */
+	sfence.vma
+	csrw CSR_SATP, a2
+#else
 	/*
+	 * 32bit system need 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
@@ -110,6 +116,7 @@ relocate:
 	or a0, a0, a1
 	sfence.vma
 	csrw CSR_SATP, a0
+#endif /* CONFIG_64BIT */
 .align 2
 1:
 	/* Set trap vector to spin forever to help debug */
@@ -122,6 +129,7 @@ relocate:
 	la gp, __global_pointer$
 .option pop
 
+#ifdef CONFIG_32BIT
 	/*
 	 * Switch to kernel page tables.  A full fence is necessary in order to
 	 * avoid using the trampoline translations, which are only correct for
@@ -130,6 +138,7 @@ relocate:
 	 */
 	csrw CSR_SATP, a2
 	sfence.vma
+#endif /* CONFIG_32BIT */
 
 	ret
 #endif /* CONFIG_MMU */
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 11b61bea0c4d..b7226ac2d04f 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -210,13 +210,17 @@ unsigned long pfn_base __ro_after_init;
 EXPORT_SYMBOL(pfn_base);
 
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
+#ifdef CONFIG_32BIT
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
+#endif /* CONFIG_32BIT */
 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
+#ifdef CONFIG_32BIT
 #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
+#endif /* CONFIG_32BIT */
 #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
 #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
 #endif /* CONFIG_XIP_KERNEL */
@@ -291,13 +295,11 @@ static void __init create_pte_mapping(pte_t *ptep,
 
 #ifndef __PAGETABLE_PMD_FOLDED
 
-pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
 pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
 pmd_t early_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
 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 */
@@ -543,21 +545,12 @@ 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_virt_addr,
-			   (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
-#ifdef CONFIG_XIP_KERNEL
-	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
-			   xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
-#else
-	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
-			   load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
-#endif
-#else
+#endif /* __PAGETABLE_PMD_FOLDED */
+#ifdef CONFIG_32BIT
 	/* Setup trampoline PGD */
 	create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
 			   load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
-#endif
+#endif /* CONFIG_32BIT */
 
 	/*
 	 * Setup early PGD covering entire kernel which will allow
-- 
2.18.0.huawei.25


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
  2021-05-27 14:48 ` Nanyong Sun
@ 2021-06-17  9:38   ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
  -1 siblings, 0 replies; 10+ messages in thread
From: Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep) @ 2021-06-17  9:38 UTC (permalink / raw)
  To: paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

On 2021/5/27 22:48, Nanyong Sun wrote:

> 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

Long time no response.

Hi anup,

     do you agree this?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-06-17  9:38   ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
  0 siblings, 0 replies; 10+ messages in thread
From: Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep) @ 2021-06-17  9:38 UTC (permalink / raw)
  To: paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

On 2021/5/27 22:48, Nanyong Sun wrote:

> 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

Long time no response.

Hi anup,

     do you agree this?


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
  2021-05-27 14:48 ` Nanyong Sun
@ 2021-06-29 13:01   ` Alex Ghiti
  -1 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-06-29 13:01 UTC (permalink / raw)
  To: Nanyong Sun, paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

Hi Nanyong,

Le 27/05/2021 à 16:48, 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.

I do agree with your last point about code maintenance and this would be 
a welcome improvement before I respin my sv48 series.

Some comments below though.

> 
> 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>
> ---
>   arch/riscv/kernel/head.S | 13 +++++++++++--
>   arch/riscv/mm/init.c     | 21 +++++++--------------
>   2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 89cc58ab52b4..1897b17c5fcc 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -93,12 +93,18 @@ relocate:
>   	add a2, a2, a1
>   	csrw CSR_TVEC, a2

This is not needed in 64-bit then.

>   
> -	/* Compute satp for kernel page tables, but don't load it yet */
> +	/* Compute satp for kernel page tables */
>   	srl a2, a0, PAGE_SHIFT
>   	li a1, SATP_MODE
>   	or a2, a2, a1
> -
> +#ifdef CONFIG_64BIT
> +	/* Load kernel page directory */
> +	sfence.vma
> +	csrw CSR_SATP, a2
> +#else
>   	/*
> +	 * 32bit system need a trampoline to handle a corner case where

s/system/systems or s/need/needs.

> +	 * 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
> @@ -110,6 +116,7 @@ relocate:
>   	or a0, a0, a1
>   	sfence.vma
>   	csrw CSR_SATP, a0
> +#endif /* CONFIG_64BIT */
>   .align 2
>   1:
>   	/* Set trap vector to spin forever to help debug */
> @@ -122,6 +129,7 @@ relocate:
>   	la gp, __global_pointer$
>   .option pop
>   
> +#ifdef CONFIG_32BIT
>   	/*
>   	 * Switch to kernel page tables.  A full fence is necessary in order to
>   	 * avoid using the trampoline translations, which are only correct for
> @@ -130,6 +138,7 @@ relocate:
>   	 */
>   	csrw CSR_SATP, a2
>   	sfence.vma
> +#endif /* CONFIG_32BIT */
> 

I would have done something like that instead, in order to avoid to copy 
the code that loads the kernel page table:


diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 89cc58ab52b4..1251f4d201a8 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -98,18 +98,23 @@ relocate:
         li a1, SATP_MODE
         or a2, a2, a1

+       /*
+        * 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
         /*
          * 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


>   	ret
>   #endif /* CONFIG_MMU */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 11b61bea0c4d..b7226ac2d04f 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -210,13 +210,17 @@ unsigned long pfn_base __ro_after_init;
>   EXPORT_SYMBOL(pfn_base);
>   
>   pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#ifdef CONFIG_32BIT
>   pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#endif /* CONFIG_32BIT */
>   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
> +#ifdef CONFIG_32BIT
>   #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
> +#endif /* CONFIG_32BIT */
>   #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
>   #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
>   #endif /* CONFIG_XIP_KERNEL */
> @@ -291,13 +295,11 @@ static void __init create_pte_mapping(pte_t *ptep,
>   
>   #ifndef __PAGETABLE_PMD_FOLDED
>   
> -pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
>   pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
>   pmd_t early_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
>   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 */
> @@ -543,21 +545,12 @@ 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_virt_addr,
> -			   (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
> -#ifdef CONFIG_XIP_KERNEL
> -	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
> -			   xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#else
> -	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
> -			   load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#endif
> -#else
> +#endif /* __PAGETABLE_PMD_FOLDED */
> +#ifdef CONFIG_32BIT

Why did you change the #else for an #ifdef CONFIG_32BIT? 
__PAGETABLE_PMD_FOLDED <=> CONFIG_32BIT anyway.

>   	/* Setup trampoline PGD */
>   	create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
>   			   load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
> -#endif
> +#endif /* CONFIG_32BIT */

If you use IS_ENABLED(CONFIG_32BIT) instead of #ifdef (and maybe rather 
CONFIG_64BIT as this is the config that is widely used to distinguish 
between both), that would avoid the #ifdef around trampoline_pg_dir 
which would be even more readable.

>   
>   	/*
>   	 * Setup early PGD covering entire kernel which will allow
> 

Thanks,

Alex

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-06-29 13:01   ` Alex Ghiti
  0 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-06-29 13:01 UTC (permalink / raw)
  To: Nanyong Sun, paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

Hi Nanyong,

Le 27/05/2021 à 16:48, 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.

I do agree with your last point about code maintenance and this would be 
a welcome improvement before I respin my sv48 series.

Some comments below though.

> 
> 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>
> ---
>   arch/riscv/kernel/head.S | 13 +++++++++++--
>   arch/riscv/mm/init.c     | 21 +++++++--------------
>   2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 89cc58ab52b4..1897b17c5fcc 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -93,12 +93,18 @@ relocate:
>   	add a2, a2, a1
>   	csrw CSR_TVEC, a2

This is not needed in 64-bit then.

>   
> -	/* Compute satp for kernel page tables, but don't load it yet */
> +	/* Compute satp for kernel page tables */
>   	srl a2, a0, PAGE_SHIFT
>   	li a1, SATP_MODE
>   	or a2, a2, a1
> -
> +#ifdef CONFIG_64BIT
> +	/* Load kernel page directory */
> +	sfence.vma
> +	csrw CSR_SATP, a2
> +#else
>   	/*
> +	 * 32bit system need a trampoline to handle a corner case where

s/system/systems or s/need/needs.

> +	 * 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
> @@ -110,6 +116,7 @@ relocate:
>   	or a0, a0, a1
>   	sfence.vma
>   	csrw CSR_SATP, a0
> +#endif /* CONFIG_64BIT */
>   .align 2
>   1:
>   	/* Set trap vector to spin forever to help debug */
> @@ -122,6 +129,7 @@ relocate:
>   	la gp, __global_pointer$
>   .option pop
>   
> +#ifdef CONFIG_32BIT
>   	/*
>   	 * Switch to kernel page tables.  A full fence is necessary in order to
>   	 * avoid using the trampoline translations, which are only correct for
> @@ -130,6 +138,7 @@ relocate:
>   	 */
>   	csrw CSR_SATP, a2
>   	sfence.vma
> +#endif /* CONFIG_32BIT */
> 

I would have done something like that instead, in order to avoid to copy 
the code that loads the kernel page table:


diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 89cc58ab52b4..1251f4d201a8 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -98,18 +98,23 @@ relocate:
         li a1, SATP_MODE
         or a2, a2, a1

+       /*
+        * 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
         /*
          * 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


>   	ret
>   #endif /* CONFIG_MMU */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 11b61bea0c4d..b7226ac2d04f 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -210,13 +210,17 @@ unsigned long pfn_base __ro_after_init;
>   EXPORT_SYMBOL(pfn_base);
>   
>   pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#ifdef CONFIG_32BIT
>   pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
> +#endif /* CONFIG_32BIT */
>   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
> +#ifdef CONFIG_32BIT
>   #define trampoline_pg_dir      ((pgd_t *)XIP_FIXUP(trampoline_pg_dir))
> +#endif /* CONFIG_32BIT */
>   #define fixmap_pte             ((pte_t *)XIP_FIXUP(fixmap_pte))
>   #define early_pg_dir           ((pgd_t *)XIP_FIXUP(early_pg_dir))
>   #endif /* CONFIG_XIP_KERNEL */
> @@ -291,13 +295,11 @@ static void __init create_pte_mapping(pte_t *ptep,
>   
>   #ifndef __PAGETABLE_PMD_FOLDED
>   
> -pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
>   pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
>   pmd_t early_pmd[PTRS_PER_PMD] __initdata __aligned(PAGE_SIZE);
>   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 */
> @@ -543,21 +545,12 @@ 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_virt_addr,
> -			   (uintptr_t)trampoline_pmd, PGDIR_SIZE, PAGE_TABLE);
> -#ifdef CONFIG_XIP_KERNEL
> -	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
> -			   xiprom, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#else
> -	create_pmd_mapping(trampoline_pmd, kernel_virt_addr,
> -			   load_pa, PMD_SIZE, PAGE_KERNEL_EXEC);
> -#endif
> -#else
> +#endif /* __PAGETABLE_PMD_FOLDED */
> +#ifdef CONFIG_32BIT

Why did you change the #else for an #ifdef CONFIG_32BIT? 
__PAGETABLE_PMD_FOLDED <=> CONFIG_32BIT anyway.

>   	/* Setup trampoline PGD */
>   	create_pgd_mapping(trampoline_pg_dir, kernel_virt_addr,
>   			   load_pa, PGDIR_SIZE, PAGE_KERNEL_EXEC);
> -#endif
> +#endif /* CONFIG_32BIT */

If you use IS_ENABLED(CONFIG_32BIT) instead of #ifdef (and maybe rather 
CONFIG_64BIT as this is the config that is widely used to distinguish 
between both), that would avoid the #ifdef around trampoline_pg_dir 
which would be even more readable.

>   
>   	/*
>   	 * Setup early PGD covering entire kernel which will allow
> 

Thanks,

Alex

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
  2021-06-29 13:01   ` Alex Ghiti
@ 2021-07-28  3:09     ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
  -1 siblings, 0 replies; 10+ messages in thread
From: Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep) @ 2021-07-28  3:09 UTC (permalink / raw)
  To: Alex Ghiti, paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

在 2021/6/29 21:01, Alex Ghiti 写道:

> Hi Nanyong,
>
> Le 27/05/2021 à 16:48, 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.
>
> I do agree with your last point about code maintenance and this would 
> be a welcome improvement before I respin my sv48 series.
>
> Some comments below though.
>
>>
>> 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>
>> ---
>>   arch/riscv/kernel/head.S | 13 +++++++++++--
>>   arch/riscv/mm/init.c     | 21 +++++++--------------
>>   2 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> index 89cc58ab52b4..1897b17c5fcc 100644
>> --- a/arch/riscv/kernel/head.S
>> +++ b/arch/riscv/kernel/head.S
>> @@ -93,12 +93,18 @@ relocate:
>>       add a2, a2, a1
>>       csrw CSR_TVEC, a2
>
> This is not needed in 64-bit then.
>
>
I'm afraid this is still needed for 64-bit, which can convert the 
physical address world to the virtual address world.

I also have made a experiment, it could not boot up if this is deleted.

> Thanks,
>
> Alex
> .

Hi Alex,

     Very thanks for your carefully review, I have updated to version 
two just now : )


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-07-28  3:09     ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
  0 siblings, 0 replies; 10+ messages in thread
From: Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep) @ 2021-07-28  3:09 UTC (permalink / raw)
  To: Alex Ghiti, paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

在 2021/6/29 21:01, Alex Ghiti 写道:

> Hi Nanyong,
>
> Le 27/05/2021 à 16:48, 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.
>
> I do agree with your last point about code maintenance and this would 
> be a welcome improvement before I respin my sv48 series.
>
> Some comments below though.
>
>>
>> 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>
>> ---
>>   arch/riscv/kernel/head.S | 13 +++++++++++--
>>   arch/riscv/mm/init.c     | 21 +++++++--------------
>>   2 files changed, 18 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> index 89cc58ab52b4..1897b17c5fcc 100644
>> --- a/arch/riscv/kernel/head.S
>> +++ b/arch/riscv/kernel/head.S
>> @@ -93,12 +93,18 @@ relocate:
>>       add a2, a2, a1
>>       csrw CSR_TVEC, a2
>
> This is not needed in 64-bit then.
>
>
I'm afraid this is still needed for 64-bit, which can convert the 
physical address world to the virtual address world.

I also have made a experiment, it could not boot up if this is deleted.

> Thanks,
>
> Alex
> .

Hi Alex,

     Very thanks for your carefully review, I have updated to version 
two just now : )


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
  2021-07-28  3:09     ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
@ 2021-07-28  6:25       ` Alex Ghiti
  -1 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-07-28  6:25 UTC (permalink / raw)
  To: Sunnanyong (Nanyong Sun,
	Intelligent Computing Solution Development Dep),
	paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

Hi Nanyong,

Le 28/07/2021 à 05:09, Sunnanyong (Nanyong Sun, Intelligent Computing 
Solution Development Dep) a écrit :
> 在 2021/6/29 21:01, Alex Ghiti 写道:
> 
>> Hi Nanyong,
>>
>> Le 27/05/2021 à 16:48, 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.
>>
>> I do agree with your last point about code maintenance and this would 
>> be a welcome improvement before I respin my sv48 series.
>>
>> Some comments below though.
>>
>>>
>>> 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>
>>> ---
>>>   arch/riscv/kernel/head.S | 13 +++++++++++--
>>>   arch/riscv/mm/init.c     | 21 +++++++--------------
>>>   2 files changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>> index 89cc58ab52b4..1897b17c5fcc 100644
>>> --- a/arch/riscv/kernel/head.S
>>> +++ b/arch/riscv/kernel/head.S
>>> @@ -93,12 +93,18 @@ relocate:
>>>       add a2, a2, a1
>>>       csrw CSR_TVEC, a2
>>
>> This is not needed in 64-bit then.
>>
>>
> I'm afraid this is still needed for 64-bit, which can convert the 
> physical address world to the virtual address world.
> 
> I also have made a experiment, it could not boot up if this is deleted.
> 

Yes you're right, sorry about that.

>> Thanks,
>>
>> Alex
>> .
> 
> Hi Alex,
> 
>      Very thanks for your carefully review, I have updated to version 
> two just now : )
> 
> 

Thanks, I'll take a look.

Alex

> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit
@ 2021-07-28  6:25       ` Alex Ghiti
  0 siblings, 0 replies; 10+ messages in thread
From: Alex Ghiti @ 2021-07-28  6:25 UTC (permalink / raw)
  To: Sunnanyong (Nanyong Sun,
	Intelligent Computing Solution Development Dep),
	paul.walmsley, palmer, aou, anup.patel
  Cc: linux-riscv, linux-kernel, palmerdabbelt, atish.patra, wangkefeng.wang

Hi Nanyong,

Le 28/07/2021 à 05:09, Sunnanyong (Nanyong Sun, Intelligent Computing 
Solution Development Dep) a écrit :
> 在 2021/6/29 21:01, Alex Ghiti 写道:
> 
>> Hi Nanyong,
>>
>> Le 27/05/2021 à 16:48, 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.
>>
>> I do agree with your last point about code maintenance and this would 
>> be a welcome improvement before I respin my sv48 series.
>>
>> Some comments below though.
>>
>>>
>>> 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>
>>> ---
>>>   arch/riscv/kernel/head.S | 13 +++++++++++--
>>>   arch/riscv/mm/init.c     | 21 +++++++--------------
>>>   2 files changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>> index 89cc58ab52b4..1897b17c5fcc 100644
>>> --- a/arch/riscv/kernel/head.S
>>> +++ b/arch/riscv/kernel/head.S
>>> @@ -93,12 +93,18 @@ relocate:
>>>       add a2, a2, a1
>>>       csrw CSR_TVEC, a2
>>
>> This is not needed in 64-bit then.
>>
>>
> I'm afraid this is still needed for 64-bit, which can convert the 
> physical address world to the virtual address world.
> 
> I also have made a experiment, it could not boot up if this is deleted.
> 

Yes you're right, sorry about that.

>> Thanks,
>>
>> Alex
>> .
> 
> Hi Alex,
> 
>      Very thanks for your carefully review, I have updated to version 
> two just now : )
> 
> 

Thanks, I'll take a look.

Alex

> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-28  6:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 14:48 [PATCH -next] riscv: mm: remove redundant trampoline PGD for 64bit Nanyong Sun
2021-05-27 14:48 ` Nanyong Sun
2021-06-17  9:38 ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
2021-06-17  9:38   ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
2021-06-29 13:01 ` Alex Ghiti
2021-06-29 13:01   ` Alex Ghiti
2021-07-28  3:09   ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
2021-07-28  3:09     ` Sunnanyong (Nanyong Sun, Intelligent Computing Solution Development Dep)
2021-07-28  6:25     ` Alex Ghiti
2021-07-28  6:25       ` Alex Ghiti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.