Re: [PATCH] arm64: add early fixmap initialization flag

[Mark Rutland]

On Tue, Feb 20, 2024 at 09:29:14AM +0900, Itaru Kitayama wrote:
> On Mon, Feb 19, 2024 at 10:48:26AM +0000, Mark Rutland wrote:
> > On Sat, Feb 17, 2024 at 11:03:26PM +0900, skseofh@xxxxxxxxx wrote:
> > > From: Daero Lee <skseofh@xxxxxxxxx>
> > > 
> > > 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.

I appreciate what you're saying here, but I don't think that it's better to
keep the second call in setup_arch().

We rely on having a (stub) DT in order to find UEFI and ACPI tables, so the DT
and ACPI setup can never be truly separated. We always need to map that DT in
order to find the UEFI+ACPI tables, and in order to do that we must initialize
the fixmap first.

I don't think there's any good reason to keep a redundant call in setup_arch();
that's just misleading and potentially problematic if we ever change
early_fixmap_init() so that it's not idempotent.

I agree it's somewhat a layering violation for  early_fdt_map() to be
responsible for initialising the fixmap, so how about we just pull that out,
e.g. as below?

Mark.

---->8----