All of lore.kernel.org
 help / color / mirror / Atom feed
From: ard.biesheuvel@linaro.org (Ard Biesheuvel)
To: linux-arm-kernel@lists.infradead.org
Subject: [linux-next PATCH] arm64: fix kernel crash with 48-bit VA and 64KB granule
Date: Wed, 6 Jan 2016 08:13:24 +0100	[thread overview]
Message-ID: <CAKv+Gu8t56rfdN1ot08BgsteaaincFek0uzoDAroefMWQ+kKEA@mail.gmail.com> (raw)
In-Reply-To: <20160106061407.GA7082@arm.org>

On 6 January 2016 at 07:14, Dennis Chen <dennis.chen@arm.com> wrote:
> On Tue, Jan 05, 2016 at 09:56:03AM +0000, Catalin Marinas wrote:
>> On Tue, Jan 05, 2016 at 04:40:44PM +0800, Dennis Chen wrote:
>> > On Tue, Jan 05, 2016 at 09:38:11AM +0100, Ard Biesheuvel wrote:
>> > > >> On 5 January 2016 at 03:18, Dennis Chen <dennis.chen@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@arm.com>
>> > > >> > Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>> > > >> >
>> > > >> > Cc: Will Deacon <will.deacon@arm.com>
>> > > >> > Cc: Catalin Marinas <catalin.marinas@arm.com>
>> > > >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > > >> > Cc: Sudeep Holla <sudeep.holla@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);
>> [...]
>> > > Well, since arm_enable_runtime_services() is an early_initcall()
>> > > itself, how are you guaranteeing the ordering between the two? Link
>> > > order?
>> >
>> > Link order.
>>
>> And can you explain how this works, what guarantees it gives?
>>
> You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro,
> for the init call section, early_initcall is the first chuck in the section,
> followed by LEVEL[0-7]. For the same level, the layout order is determined
> by the link order, ARCH is always precedence over the drivers. Catalin, are
> you giving me a kernel examination? :)
>

We all know how initcalls are implemented. The question is how you are
going to ensure that the early_initcall() that initializes the PGD
cache is always invoked before the early_initcall() that creates the
UEFI runtime page tables.

And saying that the currently observed link order happens to be
correct is not good enough. We need to be sure that, even if we change
the link order, or switch to LTO at some point, things don't suddenly
break again.

-- 
Ard.

  reply	other threads:[~2016-01-06  7:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-05  2:18 [linux-next PATCH] arm64: fix kernel crash with 48-bit VA and 64KB granule Dennis Chen
2016-01-05  7:36 ` Ard Biesheuvel
2016-01-05  8:35   ` Dennis Chen
2016-01-05  8:38     ` Ard Biesheuvel
2016-01-05  8:40       ` Dennis Chen
2016-01-05  9:56         ` Catalin Marinas
2016-01-06  6:14           ` Dennis Chen
2016-01-06  7:13             ` Ard Biesheuvel [this message]
2016-01-06  7:38               ` Dennis Chen
2016-01-06  7:42                 ` Ard Biesheuvel
2016-01-06  8:52                   ` Dennis Chen
2016-01-06  8:54                     ` Ard Biesheuvel
2016-01-06  8:59                       ` Dennis Chen
2016-01-06  9:51                         ` Will Deacon
2016-01-05 12:31   ` Will Deacon
2016-01-05 12:47     ` Ard Biesheuvel
2016-01-05 15:44       ` Will Deacon
2016-01-06  2:55         ` Dennis Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKv+Gu8t56rfdN1ot08BgsteaaincFek0uzoDAroefMWQ+kKEA@mail.gmail.com \
    --to=ard.biesheuvel@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.