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

Message ID 2ebefd395ecadb2b2605bc52a8ac223392c0c153.1671098103.git.baskov@ispras.ru
State New
Headers
Series x86_64: Improvements at compressed kernel stage |

Commit Message

Evgeniy Baskov Dec. 15, 2022, 12:38 p.m. UTC
  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(-)
  

Comments

Ard Biesheuvel March 10, 2023, 2:52 p.m. UTC | #1
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
>
  
Evgeniy Baskov March 11, 2023, 2:34 p.m. UTC | #2
On 2023-03-10 17:52, Ard Biesheuvel wrote:
> 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.

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

Patch

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