diff mbox series

[v4,13/26] x86/boot: Split trampoline and pt init code

Message ID 9f951d6332eea6e46ebd46ca919ed5b1b85c0ba3.1671098103.git.baskov@ispras.ru (mailing list archive)
State Handled Elsewhere
Headers show
Series x86_64: Improvements at compressed kernel stage | expand

Commit Message

Evgeniy Baskov Dec. 15, 2022, 12:38 p.m. UTC
When allocating trampoline from libstub trampoline allocation is
performed separately, so it needs to be skipped.

Split trampoline initialization and allocation code into two
functions to make them invokable separately.

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/pgtable_64.c | 73 +++++++++++++++++----------
 1 file changed, 46 insertions(+), 27 deletions(-)

Comments

Ard Biesheuvel March 10, 2023, 2:56 p.m. UTC | #1
On Thu, 15 Dec 2022 at 13:40, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> When allocating trampoline from libstub trampoline allocation is
> performed separately, so it needs to be skipped.
>
> Split trampoline initialization and allocation code into two
> functions to make them invokable separately.
>
> 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/pgtable_64.c | 73 +++++++++++++++++----------
>  1 file changed, 46 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> index c7cf5a1059a8..1f7169248612 100644
> --- a/arch/x86/boot/compressed/pgtable_64.c
> +++ b/arch/x86/boot/compressed/pgtable_64.c
> @@ -106,12 +106,8 @@ static unsigned long find_trampoline_placement(void)
>         return bios_start - TRAMPOLINE_32BIT_SIZE;
>  }
>
> -struct paging_config paging_prepare(void *rmode)
> +bool trampoline_pgtable_init(struct boot_params *boot_params)
>  {
> -       struct paging_config paging_config = {};
> -
> -       /* Initialize boot_params. Required for cmdline_find_option_bool(). */
> -       boot_params = rmode;
>
>         /*
>          * Check if LA57 is desired and supported.
> @@ -125,26 +121,10 @@ struct paging_config paging_prepare(void *rmode)
>          *
>          * That's substitute for boot_cpu_has() in early boot code.
>          */
> -       if (IS_ENABLED(CONFIG_X86_5LEVEL) &&
> -                       !cmdline_find_option_bool("no5lvl") &&
> -                       native_cpuid_eax(0) >= 7 &&
> -                       (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)))) {
> -               paging_config.l5_required = 1;
> -       }
> -
> -       paging_config.trampoline_start = find_trampoline_placement();
> -
> -       trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
> -
> -       /* Preserve trampoline memory */
> -       memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
> -
> -       /* Clear trampoline memory first */
> -       memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
> -
> -       /* Copy trampoline code in place */
> -       memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
> -                       &trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
> +       bool l5_required = IS_ENABLED(CONFIG_X86_5LEVEL) &&
> +                          !cmdline_find_option_bool("no5lvl") &&
> +                          native_cpuid_eax(0) >= 7 &&
> +                          (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)));
>
>         /*
>          * The code below prepares page table in trampoline memory.
> @@ -160,10 +140,10 @@ struct paging_config paging_prepare(void *rmode)
>          * We are not going to use the page table in trampoline memory if we
>          * are already in the desired paging mode.
>          */
> -       if (paging_config.l5_required == !!(native_read_cr4() & X86_CR4_LA57))
> +       if (l5_required == !!(native_read_cr4() & X86_CR4_LA57))
>                 goto out;
>
> -       if (paging_config.l5_required) {
> +       if (l5_required) {
>                 /*
>                  * For 4- to 5-level paging transition, set up current CR3 as
>                  * the first and the only entry in a new top-level page table.
> @@ -185,6 +165,45 @@ struct paging_config paging_prepare(void *rmode)
>                        (void *)src, PAGE_SIZE);
>         }
>
> +out:
> +       return l5_required;
> +}
> +
> +struct paging_config paging_prepare(void *rmode)
> +{
> +       struct paging_config paging_config = {};
> +       bool early_trampoline_alloc = 0;

false

> +
> +       /* Initialize boot_params. Required for cmdline_find_option_bool(). */
> +       boot_params = rmode;
> +
> +       /*
> +        * We only need to find trampoline placement, if we have
> +        * not already done it from libstub.
> +        */
> +
> +       paging_config.trampoline_start = find_trampoline_placement();
> +       trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
> +       early_trampoline_alloc = 0;
> +

false again

And it never becomes true, nor is it used anywhere else. Can we get rid of it?

> +       /*
> +        * Preserve trampoline memory.
> +        * When trampoline is located in memory
> +        * owned by us, i.e. allocated in EFISTUB,
> +        * we don't care about previous contents
> +        * of this memory so copying can also be skipped.

Can you please reflow comments so they takes up fewer lines?

> +        */
> +       memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
> +
> +       /* Clear trampoline memory first */
> +       memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
> +
> +       /* Copy trampoline code in place */
> +       memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
> +                       &trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
> +
> +       paging_config.l5_required = trampoline_pgtable_init(boot_params);
> +
>  out:
>         return paging_config;
>  }
> --
> 2.37.4
>
Evgeniy Baskov March 11, 2023, 2:37 p.m. UTC | #2
On 2023-03-10 17:56, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 13:40, Evgeniy Baskov <baskov@ispras.ru> wrote:
>> 
>> When allocating trampoline from libstub trampoline allocation is
>> performed separately, so it needs to be skipped.
>> 
>> Split trampoline initialization and allocation code into two
>> functions to make them invokable separately.
>> 
>> 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/pgtable_64.c | 73 
>> +++++++++++++++++----------
>>  1 file changed, 46 insertions(+), 27 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/pgtable_64.c 
>> b/arch/x86/boot/compressed/pgtable_64.c
>> index c7cf5a1059a8..1f7169248612 100644
>> --- a/arch/x86/boot/compressed/pgtable_64.c
>> +++ b/arch/x86/boot/compressed/pgtable_64.c
>> @@ -106,12 +106,8 @@ static unsigned long 
>> find_trampoline_placement(void)
>>         return bios_start - TRAMPOLINE_32BIT_SIZE;
>>  }
>> 
>> -struct paging_config paging_prepare(void *rmode)
>> +bool trampoline_pgtable_init(struct boot_params *boot_params)
>>  {
>> -       struct paging_config paging_config = {};
>> -
>> -       /* Initialize boot_params. Required for 
>> cmdline_find_option_bool(). */
>> -       boot_params = rmode;
>> 
>>         /*
>>          * Check if LA57 is desired and supported.
>> @@ -125,26 +121,10 @@ struct paging_config paging_prepare(void *rmode)
>>          *
>>          * That's substitute for boot_cpu_has() in early boot code.
>>          */
>> -       if (IS_ENABLED(CONFIG_X86_5LEVEL) &&
>> -                       !cmdline_find_option_bool("no5lvl") &&
>> -                       native_cpuid_eax(0) >= 7 &&
>> -                       (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 
>> & 31)))) {
>> -               paging_config.l5_required = 1;
>> -       }
>> -
>> -       paging_config.trampoline_start = find_trampoline_placement();
>> -
>> -       trampoline_32bit = (unsigned long 
>> *)paging_config.trampoline_start;
>> -
>> -       /* Preserve trampoline memory */
>> -       memcpy(trampoline_save, trampoline_32bit, 
>> TRAMPOLINE_32BIT_SIZE);
>> -
>> -       /* Clear trampoline memory first */
>> -       memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
>> -
>> -       /* Copy trampoline code in place */
>> -       memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / 
>> sizeof(unsigned long),
>> -                       &trampoline_32bit_src, 
>> TRAMPOLINE_32BIT_CODE_SIZE);
>> +       bool l5_required = IS_ENABLED(CONFIG_X86_5LEVEL) &&
>> +                          !cmdline_find_option_bool("no5lvl") &&
>> +                          native_cpuid_eax(0) >= 7 &&
>> +                          (native_cpuid_ecx(7) & (1 << 
>> (X86_FEATURE_LA57 & 31)));
>> 
>>         /*
>>          * The code below prepares page table in trampoline memory.
>> @@ -160,10 +140,10 @@ struct paging_config paging_prepare(void *rmode)
>>          * We are not going to use the page table in trampoline memory 
>> if we
>>          * are already in the desired paging mode.
>>          */
>> -       if (paging_config.l5_required == !!(native_read_cr4() & 
>> X86_CR4_LA57))
>> +       if (l5_required == !!(native_read_cr4() & X86_CR4_LA57))
>>                 goto out;
>> 
>> -       if (paging_config.l5_required) {
>> +       if (l5_required) {
>>                 /*
>>                  * For 4- to 5-level paging transition, set up current 
>> CR3 as
>>                  * the first and the only entry in a new top-level 
>> page table.
>> @@ -185,6 +165,45 @@ struct paging_config paging_prepare(void *rmode)
>>                        (void *)src, PAGE_SIZE);
>>         }
>> 
>> +out:
>> +       return l5_required;
>> +}
>> +
>> +struct paging_config paging_prepare(void *rmode)
>> +{
>> +       struct paging_config paging_config = {};
>> +       bool early_trampoline_alloc = 0;
> 
> false
> 
>> +
>> +       /* Initialize boot_params. Required for 
>> cmdline_find_option_bool(). */
>> +       boot_params = rmode;
>> +
>> +       /*
>> +        * We only need to find trampoline placement, if we have
>> +        * not already done it from libstub.
>> +        */
>> +
>> +       paging_config.trampoline_start = find_trampoline_placement();
>> +       trampoline_32bit = (unsigned long 
>> *)paging_config.trampoline_start;
>> +       early_trampoline_alloc = 0;
>> +
> 
> false again
> 
> And it never becomes true, nor is it used anywhere else. Can we get rid 
> of it?

Yes, probably it is just a leftover of the approach I used
before. I'll remove that.
> 
>> +       /*
>> +        * Preserve trampoline memory.
>> +        * When trampoline is located in memory
>> +        * owned by us, i.e. allocated in EFISTUB,
>> +        * we don't care about previous contents
>> +        * of this memory so copying can also be skipped.
> 
> Can you please reflow comments so they takes up fewer lines?
> 

Will fix.

>> +        */
>> +       memcpy(trampoline_save, trampoline_32bit, 
>> TRAMPOLINE_32BIT_SIZE);
>> +
>> +       /* Clear trampoline memory first */
>> +       memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
>> +
>> +       /* Copy trampoline code in place */
>> +       memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / 
>> sizeof(unsigned long),
>> +                       &trampoline_32bit_src, 
>> TRAMPOLINE_32BIT_CODE_SIZE);
>> +
>> +       paging_config.l5_required = 
>> trampoline_pgtable_init(boot_params);
>> +
>>  out:
>>         return paging_config;
>>  }
>> --
>> 2.37.4
>>
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index c7cf5a1059a8..1f7169248612 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -106,12 +106,8 @@  static unsigned long find_trampoline_placement(void)
 	return bios_start - TRAMPOLINE_32BIT_SIZE;
 }
 
-struct paging_config paging_prepare(void *rmode)
+bool trampoline_pgtable_init(struct boot_params *boot_params)
 {
-	struct paging_config paging_config = {};
-
-	/* Initialize boot_params. Required for cmdline_find_option_bool(). */
-	boot_params = rmode;
 
 	/*
 	 * Check if LA57 is desired and supported.
@@ -125,26 +121,10 @@  struct paging_config paging_prepare(void *rmode)
 	 *
 	 * That's substitute for boot_cpu_has() in early boot code.
 	 */
-	if (IS_ENABLED(CONFIG_X86_5LEVEL) &&
-			!cmdline_find_option_bool("no5lvl") &&
-			native_cpuid_eax(0) >= 7 &&
-			(native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)))) {
-		paging_config.l5_required = 1;
-	}
-
-	paging_config.trampoline_start = find_trampoline_placement();
-
-	trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
-
-	/* Preserve trampoline memory */
-	memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
-
-	/* Clear trampoline memory first */
-	memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
-
-	/* Copy trampoline code in place */
-	memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
-			&trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
+	bool l5_required = IS_ENABLED(CONFIG_X86_5LEVEL) &&
+			   !cmdline_find_option_bool("no5lvl") &&
+			   native_cpuid_eax(0) >= 7 &&
+			   (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)));
 
 	/*
 	 * The code below prepares page table in trampoline memory.
@@ -160,10 +140,10 @@  struct paging_config paging_prepare(void *rmode)
 	 * We are not going to use the page table in trampoline memory if we
 	 * are already in the desired paging mode.
 	 */
-	if (paging_config.l5_required == !!(native_read_cr4() & X86_CR4_LA57))
+	if (l5_required == !!(native_read_cr4() & X86_CR4_LA57))
 		goto out;
 
-	if (paging_config.l5_required) {
+	if (l5_required) {
 		/*
 		 * For 4- to 5-level paging transition, set up current CR3 as
 		 * the first and the only entry in a new top-level page table.
@@ -185,6 +165,45 @@  struct paging_config paging_prepare(void *rmode)
 		       (void *)src, PAGE_SIZE);
 	}
 
+out:
+	return l5_required;
+}
+
+struct paging_config paging_prepare(void *rmode)
+{
+	struct paging_config paging_config = {};
+	bool early_trampoline_alloc = 0;
+
+	/* Initialize boot_params. Required for cmdline_find_option_bool(). */
+	boot_params = rmode;
+
+	/*
+	 * We only need to find trampoline placement, if we have
+	 * not already done it from libstub.
+	 */
+
+	paging_config.trampoline_start = find_trampoline_placement();
+	trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
+	early_trampoline_alloc = 0;
+
+	/*
+	 * Preserve trampoline memory.
+	 * When trampoline is located in memory
+	 * owned by us, i.e. allocated in EFISTUB,
+	 * we don't care about previous contents
+	 * of this memory so copying can also be skipped.
+	 */
+	memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
+
+	/* Clear trampoline memory first */
+	memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
+
+	/* Copy trampoline code in place */
+	memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
+			&trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
+
+	paging_config.l5_required = trampoline_pgtable_init(boot_params);
+
 out:
 	return paging_config;
 }