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