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

[Zhaoyang Huang]

On Fri, Nov 12, 2021 at 5:31 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Fri, 12 Nov 2021 at 10:21, Huangzhaoyang <huangzhaoyang@xxxxxxxxx> wrote:
> >
> > From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> >  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
> >