linux-kernel - Re: [PATCH] arm64: add early fixmap initialization flag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZdPyWkOlUan5AI9r@vm3>
Date: Tue, 20 Feb 2024 09:29:14 +0900
From: Itaru Kitayama <itaru.kitayama@...ux.dev>
To: Mark Rutland <mark.rutland@....com>
Cc: skseofh@...il.com, catalin.marinas@....com, will@...nel.org,
	ryan.roberts@....com, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: add early fixmap initialization flag

On Mon, Feb 19, 2024 at 10:48:26AM +0000, Mark Rutland wrote:
> On Sat, Feb 17, 2024 at 11:03:26PM +0900, skseofh@...il.com wrote:
> > From: Daero Lee <skseofh@...il.com>
> > 
> > early_fixmap_init may be called multiple times. Since there is no
> > change in the page table after early fixmap initialization, an
> > initialization flag was added.
> 
> Why is that better?
> 
> We call early_fixmap_init() in two places:
> 
> * early_fdt_map()
> * setup_arch()
> 
> ... and to get to setup_arch() we *must* have gone through early_fdt_map(),
> since __primary_switched() calls that before going to setup_arch().
> 
> So AFAICT we can remove the second call to early_fixmap_init() in setup_arch(),
> and rely on the earlier one in early_fdt_map().

Removing the second call makes the code base a bit harder to understand
as the functions related to DT and ACPI setup are not separated cleanly.
I prefer calling the early_fixmap_init() in setup_arch() as well.

Itaru.

> 
> Mark.
> 
> > 
> > Signed-off-by: Daero Lee <skseofh@...il.com>
> > ---
> >  arch/arm64/mm/fixmap.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
> > index c0a3301203bd..fbdd5f30f3a1 100644
> > --- a/arch/arm64/mm/fixmap.c
> > +++ b/arch/arm64/mm/fixmap.c
> > @@ -32,6 +32,8 @@ static pte_t bm_pte[NR_BM_PTE_TABLES][PTRS_PER_PTE] __page_aligned_bss;
> >  static pmd_t bm_pmd[PTRS_PER_PMD] __page_aligned_bss __maybe_unused;
> >  static pud_t bm_pud[PTRS_PER_PUD] __page_aligned_bss __maybe_unused;
> >  
> > +static int early_fixmap_initialized __initdata;
> > +
> >  static inline pte_t *fixmap_pte(unsigned long addr)
> >  {
> >  	return &bm_pte[BM_PTE_TABLE_IDX(addr)][pte_index(addr)];
> > @@ -100,10 +102,15 @@ void __init early_fixmap_init(void)
> >  	unsigned long addr = FIXADDR_TOT_START;
> >  	unsigned long end = FIXADDR_TOP;
> >  
> > +	if (early_fixmap_initialized)
> > +		return;
> > +
> >  	pgd_t *pgdp = pgd_offset_k(addr);
> >  	p4d_t *p4dp = p4d_offset(pgdp, addr);
> >  
> >  	early_fixmap_init_pud(p4dp, addr, end);
> > +
> > +	early_fixmap_initialized = 1;
> >  }
> >  
> >  /*
> > -- 
> > 2.25.1
> >