MIME-Version: 1.0
References: <cover.1671098103.git.baskov@ispras.ru> <2ebefd395ecadb2b2605bc52a8ac223392c0c153.1671098103.git.baskov@ispras.ru>
In-Reply-To: <2ebefd395ecadb2b2605bc52a8ac223392c0c153.1671098103.git.baskov@ispras.ru>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 10 Mar 2023 15:52:52 +0100
Message-ID: <CAMj1kXHY045xxhbC_+JRhwy-vObfKGFA8YnZr7anh6Khuk-HAA@mail.gmail.com>
Subject: Re: [PATCH v4 12/26] x86/boot: Make kernel_add_identity_map() a pointer
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:
>
> Convert kernel_add_identity_map() into a function pointer to be able
> to provide alternative implementations of this function. Required
> to enable calling the code using this function from EFI environment.
>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/boot/compressed/ident_map_64.c |  7 ++++---
>  arch/x86/boot/compressed/misc.c         | 24 ++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.h         | 15 +++------------
>  3 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
> index ba5108c58a4e..1aee524d3c2b 100644
> --- a/arch/x86/boot/compressed/ident_map_64.c
> +++ b/arch/x86/boot/compressed/ident_map_64.c
> @@ -92,9 +92,9 @@ bool has_nx; /* set in head_64.S */
>  /*
>   * Adds the specified range to the identity mappings.
>   */
> -unsigned long kernel_add_identity_map(unsigned long start,
> -                                     unsigned long end,
> -                                     unsigned int flags)
> +unsigned long kernel_add_identity_map_(unsigned long start,

Please use a more discriminating name here - the trailing _ is rather
hard to spot.

> +                                      unsigned long end,
> +                                      unsigned int flags)
>  {
>         int ret;
>
> @@ -142,6 +142,7 @@ void initialize_identity_maps(void *rmode)
>         struct setup_data *sd;
>
>         boot_params = rmode;
> +       kernel_add_identity_map = kernel_add_identity_map_;
>
>         /* Exclude the encryption mask from __PHYSICAL_MASK */
>         physical_mask &= ~sme_me_mask;
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index aa4a22bc9cf9..c9c235d65d16 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -275,6 +275,22 @@ static void parse_elf(void *output, unsigned long output_len,
>         free(phdrs);
>  }
>
> +/*
> + * This points to actual implementation of mapping function
> + * for current environment: either EFI API wrapper,
> + * own implementation or dummy implementation below.
> + */
> +unsigned long (*kernel_add_identity_map)(unsigned long start,
> +                                        unsigned long end,
> +                                        unsigned int flags);
> +
> +static inline unsigned long kernel_add_identity_map_dummy(unsigned long start,

This function is never called, it only has its address taken, so the
'inline' makes no sense here.

> +                                                         unsigned long end,
> +                                                         unsigned int flags)
> +{
> +       return start;
> +}
> +
>  /*
>   * The compressed kernel image (ZO), has been moved so that its position
>   * is against the end of the buffer used to hold the uncompressed kernel
> @@ -312,6 +328,14 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
>
>         init_default_io_ops();
>
> +       /*
> +        * On 64-bit this pointer is set during page table uninitialization,

initialization

> +        * but on 32-bit it remains uninitialized, since paging is disabled.
> +        */
> +       if (IS_ENABLED(CONFIG_X86_32))
> +               kernel_add_identity_map = kernel_add_identity_map_dummy;
> +
> +
>         /*
>          * Detect TDX guest environment.
>          *
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index 38d31bec062d..0076b2845b4b 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -180,18 +180,9 @@ static inline int count_immovable_mem_regions(void) { return 0; }
>  #ifdef CONFIG_X86_5LEVEL
>  extern unsigned int __pgtable_l5_enabled, pgdir_shift, ptrs_per_p4d;
>  #endif
> -#ifdef CONFIG_X86_64
> -extern unsigned long kernel_add_identity_map(unsigned long start,
> -                                            unsigned long end,
> -                                            unsigned int flags);
> -#else
> -static inline unsigned long kernel_add_identity_map(unsigned long start,
> -                                                   unsigned long end,
> -                                                   unsigned int flags)
> -{
> -       return start;
> -}
> -#endif
> +extern unsigned long (*kernel_add_identity_map)(unsigned long start,
> +                                               unsigned long end,
> +                                               unsigned int flags);
>  /* Used by PAGE_KERN* macros: */
>  extern pteval_t __default_kernel_pte_mask;
>
> --
> 2.37.4
>