[linux-next PATCH] arm64: fix kernel crash with 48-bit VA and 64KB granule

[Will Deacon]

On Tue, Jan 05, 2016 at 08:36:39AM +0100, Ard Biesheuvel wrote:
> On 5 January 2016 at 03:18, Dennis Chen <dennis.chen at arm.com> wrote:
> > The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime
> > code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for
> > efi_mm.pgd while not the static efi_pgd[], since this function will be
> > called with early_initcall, which results in the pgd_cache used by
> > pgd_alloc() has not been initialized yet, kernel will hang in this
> > case. This patch is trying to make the pgd_cache_init() called before
> > arm_enable_runtime_services() by changing its core_initcall to
> > early_initcall.
> >
> > Signed-off-by: Dennis Chen <dennis.chen at arm.com>
> > Tested-by: Sudeep Holla <sudeep.holla at arm.com>
> >
> > Cc: Will Deacon <will.deacon at arm.com>
> > Cc: Catalin Marinas <catalin.marinas at arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > Cc: Sudeep Holla <sudeep.holla at arm.com>
> > ---
> >  arch/arm64/mm/pgd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
> > index cb3ba1b..859a788 100644
> > --- a/arch/arm64/mm/pgd.c
> > +++ b/arch/arm64/mm/pgd.c
> > @@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
> >                                               SLAB_PANIC, NULL);
> >         return 0;
> >  }
> > -core_initcall(pgd_cache_init);
> > +early_initcall(pgd_cache_init);
> 
> 
> Hello all,
> 
> Apologies for not spotting this before sending it out.
> 
> The issue only occurs when PGD_SIZE != PAGE_SIZE, which is why I did
> not see it myself. I only tested with 4k/39-bits and 64k/42-bits IIRC,
> since the series this is part of primarily concerns ARM not arm64.
> 
> Anyway, shuffling init ordering is my least favorite way of fixing
> bugs. Since only ARM requires pgd_alloc() (for the kernel mappings),
> we could also simply factor out pgd_alloc() into efi_pgd_alloc(), and
> define it to mean '__get_free_page(PGALLOC_GFP)' on arm64.

Or we could put the pgd_cache initialisation in pgtable_cache_init, which
sounds like the right place for it anyway.

Curious, but why are our EFI runtime services initialised off the back of
initcalls, whereas on x86 the initialisation is driven directly from
init/main.c? I'm not saying it should be changed, it just looks odd having
the two paths.

Will