diff mbox series

[RFC] arch: arm64: have memblocks out of kernel text use section map

Message ID 1636708842-25787-1-git-send-email-huangzhaoyang@gmail.com (mailing list archive)
State New
Headers show
Series [RFC] arch: arm64: have memblocks out of kernel text use section map | expand

Commit Message

Zhaoyang Huang Nov. 12, 2021, 9:20 a.m. UTC 
From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>

By comparing the swapper_pg_dir with k54 and previous versions,we find
that the linear mappings within which the addr is out of kernel text section
will use the smallest pte. It should arise for the sake of rodata_full, which
set all memblock use NO_CONT_MAPPINGS.

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
---
 arch/arm64/mm/mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Ard Biesheuvel Nov. 12, 2021, 9:31 a.m. UTC | #1 
On Fri, 12 Nov 2021 at 10:21, Huangzhaoyang <huangzhaoyang@gmail.com> wrote:
>
> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
>
> By comparing the swapper_pg_dir with k54

I take it this means Linux v5.4 ?

> and previous versions,we find
> that the linear mappings within which the addr is out of kernel text section
> will use the smallest pte. It should arise for the sake of rodata_full, which
> set all memblock use NO_CONT_MAPPINGS.
>

OK so your intent seems to be to use block mappings for the linear
alias of the kernel text and rodata, right?

What does that achieve? It should make no difference in TLB pressure,
as the linear alias is rarely referenced (we only kept it around for
hibernate). So I guess we save a handful of pages for page tables.

> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> ---
>  arch/arm64/mm/mmu.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index cfd9deb..14e1bea 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -252,6 +252,8 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
>         pmd_clear_fixmap();
>  }
>
> +static bool crash_mem_map __initdata;
> +
>  static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
>                                 unsigned long end, phys_addr_t phys,
>                                 pgprot_t prot,
> @@ -259,7 +261,15 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
>  {
>         unsigned long next;
>         pud_t pud = READ_ONCE(*pudp);
> +       unsigned long len = end - addr;
> +       phys_addr_t kernel_start = __pa_symbol(_stext);
> +       phys_addr_t kernel_end = __pa_symbol(__init_begin);
>
> +       if (debug_pagealloc_enabled() || crash_mem_map || IS_ENABLED(CONFIG_KFENCE))
> +               ;
> +       else if (phys > kernel_end || phys + len < kernel_start) {
> +               flags &= ~(NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
> +       }

Please don't use empty statements like this, and I'm not sure we even
need this check: I wouldn't expect debug_pagealloc or KFENCE to ever
require page granular mappings of this region either.

Also, please add a comment to explain what the condition is meant to
check (i..e, that the PMD entry covers a part of the linear alias of
the kernel image, and so there is no need to map it down to pages or
to avoid contiguous mappings)

>         /*
>          * Check for initial section mappings in the pgd/pud.
>          */
> @@ -483,7 +493,6 @@ void __init mark_linear_text_alias_ro(void)
>                             PAGE_KERNEL_RO);
>  }
>
> -static bool crash_mem_map __initdata;
>
>  static int __init enable_crash_mem_map(char *arg)
>  {
> --
> 1.9.1
>
Zhaoyang Huang Nov. 12, 2021, 9:45 a.m. UTC | #2 
On Fri, Nov 12, 2021 at 5:31 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 12 Nov 2021 at 10:21, Huangzhaoyang <huangzhaoyang@gmail.com> wrote:
> >
> > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> >
> > By comparing the swapper_pg_dir with k54
>
> I take it this means Linux v5.4 ?
>
> > and previous versions,we find
> > that the linear mappings within which the addr is out of kernel text section
> > will use the smallest pte. It should arise for the sake of rodata_full, which
> > set all memblock use NO_CONT_MAPPINGS.
> >
>
> OK so your intent seems to be to use block mappings for the linear
> alias of the kernel text and rodata, right?
>
> What does that achieve? It should make no difference in TLB pressure,
> as the linear alias is rarely referenced (we only kept it around for
> hibernate). So I guess we save a handful of pages for page tables.
Thanks for the quick response and sorry for causing confusion with my
poor comments. What I want to do is on the contrary, that is using
block mapping on the addr OUT OF the kernel text, which could be
affected by setting rodata_full(can_set_direct_map) so far.


>
> > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> > ---
> >  arch/arm64/mm/mmu.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index cfd9deb..14e1bea 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -252,6 +252,8 @@ static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
> >         pmd_clear_fixmap();
> >  }
> >
> > +static bool crash_mem_map __initdata;
> > +
> >  static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> >                                 unsigned long end, phys_addr_t phys,
> >                                 pgprot_t prot,
> > @@ -259,7 +261,15 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
> >  {
> >         unsigned long next;
> >         pud_t pud = READ_ONCE(*pudp);
> > +       unsigned long len = end - addr;
> > +       phys_addr_t kernel_start = __pa_symbol(_stext);
> > +       phys_addr_t kernel_end = __pa_symbol(__init_begin);
> >
> > +       if (debug_pagealloc_enabled() || crash_mem_map || IS_ENABLED(CONFIG_KFENCE))
> > +               ;
> > +       else if (phys > kernel_end || phys + len < kernel_start) {
> > +               flags &= ~(NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
> > +       }
>
> Please don't use empty statements like this, and I'm not sure we even
> need this check: I wouldn't expect debug_pagealloc or KFENCE to ever
> require page granular mappings of this region either.
>
> Also, please add a comment to explain what the condition is meant to
> check (i..e, that the PMD entry covers a part of the linear alias of
> the kernel image, and so there is no need to map it down to pages or
> to avoid contiguous mappings)
>
> >         /*
> >          * Check for initial section mappings in the pgd/pud.
> >          */
> > @@ -483,7 +493,6 @@ void __init mark_linear_text_alias_ro(void)
> >                             PAGE_KERNEL_RO);
> >  }
> >
> > -static bool crash_mem_map __initdata;
> >
> >  static int __init enable_crash_mem_map(char *arg)
> >  {
> > --
> > 1.9.1
> >
diff mbox series

Patch

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index cfd9deb..14e1bea 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -252,6 +252,8 @@  static void init_pmd(pud_t *pudp, unsigned long addr, unsigned long end,
 	pmd_clear_fixmap();
 }
 
+static bool crash_mem_map __initdata;
+
 static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 				unsigned long end, phys_addr_t phys,
 				pgprot_t prot,
@@ -259,7 +261,15 @@  static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 {
 	unsigned long next;
 	pud_t pud = READ_ONCE(*pudp);
+	unsigned long len = end - addr;
+	phys_addr_t kernel_start = __pa_symbol(_stext);
+	phys_addr_t kernel_end = __pa_symbol(__init_begin);
 
+	if (debug_pagealloc_enabled() || crash_mem_map || IS_ENABLED(CONFIG_KFENCE))
+		;
+	else if (phys > kernel_end || phys + len < kernel_start) {
+		flags &= ~(NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS);
+	}
 	/*
 	 * Check for initial section mappings in the pgd/pud.
 	 */
@@ -483,7 +493,6 @@  void __init mark_linear_text_alias_ro(void)
 			    PAGE_KERNEL_RO);
 }
 
-static bool crash_mem_map __initdata;
 
 static int __init enable_crash_mem_map(char *arg)
 {