MIME-Version: 1.0
References: <cover.1671098103.git.baskov@ispras.ru> <8945a9cd072754d8dda9961a5499fc7b0c2e8743.1671098103.git.baskov@ispras.ru>
In-Reply-To: <8945a9cd072754d8dda9961a5499fc7b0c2e8743.1671098103.git.baskov@ispras.ru>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Thu, 9 Mar 2023 17:00:35 +0100
Message-ID: <CAMj1kXH+vfZQMxfO+tZPnZrxaZX4ffxpzHrdCnTfP70DgO=YJQ@mail.gmail.com>
Subject: Re: [PATCH v4 15/26] efi/x86: Support extracting kernel from libstub
To:     Evgeniy Baskov <baskov@ispras.ru>
Cc:     Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        Peter Jones <pjones@redhat.com>,
        "Limonciello, Mario" <mario.limonciello@amd.com>,
        joeyli <jlee@suse.com>, lvc-project@linuxtesting.org,
        x86@kernel.org, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 15 Dec 2022 at 13:40, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> Doing it that way allows setting up stricter memory attributes,
> simplifies boot code path and removes potential relocation
> of kernel image.
>
> Wire up required interfaces and minimally initialize zero page
> fields needed for it to function correctly.
>
> Tested-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/boot/compressed/head_32.S            |  50 ++++-
>  arch/x86/boot/compressed/head_64.S            |  58 ++++-
>  drivers/firmware/efi/Kconfig                  |   2 +
>  drivers/firmware/efi/libstub/Makefile         |   2 +-
>  .../firmware/efi/libstub/x86-extract-direct.c | 208 ++++++++++++++++++
>  drivers/firmware/efi/libstub/x86-stub.c       | 119 +---------
>  drivers/firmware/efi/libstub/x86-stub.h       |  14 ++
>  7 files changed, 338 insertions(+), 115 deletions(-)
>  create mode 100644 drivers/firmware/efi/libstub/x86-extract-direct.c
>  create mode 100644 drivers/firmware/efi/libstub/x86-stub.h
>
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index ead6007df1e5..0be75e5072ae 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -152,11 +152,57 @@ SYM_FUNC_END(startup_32)
>
>  #ifdef CONFIG_EFI_STUB
>  SYM_FUNC_START(efi32_stub_entry)
> +/*
> + * Calculate the delta between where we were compiled to run
> + * at and where we were actually loaded at.  This can only be done
> + * with a short local call on x86.  Nothing  else will tell us what
> + * address we are running at.  The reserved chunk of the real-mode
> + * data at 0x1e4 (defined as a scratch field) are used as the stack
> + * for this calculation. Only 4 bytes are needed.
> + */

Please drop this comment

> +       call    1f
> +1:     popl    %ebx
> +       addl    $_GLOBAL_OFFSET_TABLE_+(.-1b), %ebx

Please drop this and ...

> +
> +       /* Clear BSS */
> +       xorl    %eax, %eax
> +       leal    _bss@GOTOFF(%ebx), %edi
> +       leal    _ebss@GOTOFF(%ebx), %ecx

just use (_bss - 1b) here (etc)

> +       subl    %edi, %ecx
> +       shrl    $2, %ecx
> +       rep     stosl
> +
>         add     $0x4, %esp
>         movl    8(%esp), %esi   /* save boot_params pointer */
> +       movl    %edx, %edi      /* save GOT address */

What does this do?

>         call    efi_main
> -       /* efi_main returns the possibly relocated address of startup_32 */
> -       jmp     *%eax
> +       movl    %eax, %ecx
> +
> +       /*
> +        * efi_main returns the possibly
> +        * relocated address of extracted kernel entry point.
> +        */
> +
> +       cli
> +
> +       /* Load new GDT */
> +       leal    gdt@GOTOFF(%ebx), %eax
> +       movl    %eax, 2(%eax)
> +       lgdt    (%eax)
> +
> +       /* Load segment registers with our descriptors */
> +       movl    $__BOOT_DS, %eax
> +       movl    %eax, %ds
> +       movl    %eax, %es
> +       movl    %eax, %fs
> +       movl    %eax, %gs
> +       movl    %eax, %ss
> +
> +       /* Zero EFLAGS */
> +       pushl   $0
> +       popfl
> +
> +       jmp     *%ecx
>  SYM_FUNC_END(efi32_stub_entry)
>  SYM_FUNC_ALIAS(efi_stub_entry, efi32_stub_entry)
>  #endif
...