Re: [PATCH v4 12/26] x86/boot: Make kernel_add_identity_map() a pointer

[Evgeniy Baskov]

On 2023-03-10 17:52, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 13:40, Evgeniy Baskov <baskov@xxxxxxxxx> 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@xxxxxxx>
> > Tested-by: Peter Jones <pjones@xxxxxxxxxx>
> > Signed-off-by: Evgeniy Baskov <baskov@xxxxxxxxx>
> > ---
> >  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.

Got it. The kernel_add_identity_map_impl() will fit better, I think.

> > +                                      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.

Indeed. I'll remove the inline.

> > +                                                         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

Thanks!

> > +        * 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