Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <87jzvm12q0.ffs@tglx>
In-Reply-To: <87jzvm12q0.ffs@tglx>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Thu, 29 Jun 2023 23:42:49 +0200
Message-ID: <CAMj1kXG0GZ8K4kVux3zQ6TXGoeQ8bwHq7JXbx-YXFbLSbD-6Gg@mail.gmail.com>
Subject: Re: x86/efi: Make efi_set_virtual_address_map IBT safe
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        =?UTF-8?B?TmlrbMSBdnMgS2/EvGVzxYZpa292cw==?= 
        <pinkflames.linux@gmail.com>, linux-efi@vger.kernel.org,
        x86@kernel.org, regressions@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, 29 Jun 2023 at 21:35, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Nikl=C4=81vs reported a boot regression on an Alderlake machine and bisec=
ted it
> to commit 9df9d2f0471b ("init: Invoke arch_cpu_finalize_init() earlier").
>
> By moving the invocation of arch_cpu_finalize_init() further down he
> identified that efi_enter_virtual_mode() is the function which causes
> the boot hang.
>
> The main difference of the earlier invocation is that the boot CPU is
> already fully initialized and mitigations and alternatives are applied.
>
> But the only really interesting change turned out to be IBT, which is
> now enabled before efi_enter_virtual_mode(). "ibt=3Doff" on the kernel
> command line cured the problem.
>
> Inspection of the involved calls in efi_enter_virtual_mode() unearthed th=
at
> efi_set_virtual_address_map() is the only place in the kernel which invok=
es
> an EFI call without the IBT safe wrapper. This went obviously unnoticed s=
o
> far as IBT was enabled later.
>
> Use arch_efi_call_virt() instead of efi_call() to cure that.
>
> Fixes: fe379fa4d199 ("x86/ibt: Disable IBT around firmware")
> Fixes: 9df9d2f0471b ("init: Invoke arch_cpu_finalize_init() earlier")
> Reported-by: Nikl=C4=81vs Ko=C4=BCes=C5=86ikovs <pinkflames.linux@gmail.c=
om>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=3D217602

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

I take it you'll send this straight to Linus?


> ---
> I put two fixes tags in as the IBT one missed that and the earlier
> invocation unearthed it and made it observable.
> ---
>  arch/x86/platform/efi/efi_64.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -853,9 +853,9 @@ efi_set_virtual_address_map(unsigned lon
>
>         /* Disable interrupts around EFI calls: */
>         local_irq_save(flags);
> -       status =3D efi_call(efi.runtime->set_virtual_address_map,
> -                         memory_map_size, descriptor_size,
> -                         descriptor_version, virtual_map);
> +       status =3D arch_efi_call_virt(efi.runtime, set_virtual_address_ma=
p,
> +                                   memory_map_size, descriptor_size,
> +                                   descriptor_version, virtual_map);
>         local_irq_restore(flags);
>
>         efi_fpu_end();