| Message ID | 20230613224545.902376621@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | init, treewide, x86: Cleanup check_bugs() and start sanitizing the x86 boot process | expand |
On 6/13/2023 4:39 PM, Thomas Gleixner wrote: > > @@ -2396,6 +2393,13 @@ void __init arch_cpu_finalize_init(void) > '0' + (boot_cpu_data.x86 > 6 ? 6 : boot_cpu_data.x86); > } > > + /* > + * Must be before alternatives because it might set or clear > + * feature bits. > + */ > + fpu__init_system(); > + fpu__init_cpu(); fpu__init_cpu() appears to be duplicated here because fpu__init_system() invoked this already: void __init fpu__init_system(void) { ... /* * The FPU has to be operational for some of the * later FPU init activities: */ fpu__init_cpu(); ... } https://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git/tree/arch/x86/kernel/fpu/init.c?h=init#n223 Thanks, Chang
On Tue, Jun 13 2023 at 22:03, Chang S. Bae wrote: > On 6/13/2023 4:39 PM, Thomas Gleixner wrote: >> >> @@ -2396,6 +2393,13 @@ void __init arch_cpu_finalize_init(void) >> '0' + (boot_cpu_data.x86 > 6 ? 6 : boot_cpu_data.x86); >> } >> >> + /* >> + * Must be before alternatives because it might set or clear >> + * feature bits. >> + */ >> + fpu__init_system(); >> + fpu__init_cpu(); > > fpu__init_cpu() appears to be duplicated here because fpu__init_system() > invoked this already: > > void __init fpu__init_system(void) > { > ... > /* > * The FPU has to be operational for some of the > * later FPU init activities: > */ > fpu__init_cpu(); Well, that's _before_ xstate initialization and I couldn't convince myself that it's good enough. All of this is such a horrible mess...
Hi, On Wed, Jun 14, 2023 at 01:39:46AM +0200, Thomas Gleixner wrote: > Initializing the FPU during the early boot process is a pointless > exercise. Early boot is convoluted and fragile enough. > > Nothing requires that the FPU is set up early. It has to be initialized > before fork_init() because the task_struct size depends on the FPU register > buffer size. > > Move the initialization to arch_cpu_finalize_init() which is the perfect > place to do so. > > No functional change. > > This allows to remove quite some of the custom early command line parsing, > but that's subject to the next installment. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> The backport of this patch into chromeos-5.10 and chromeos-5.15 via stable relase merges is causing various Chromebooks (not all of them) to crash early during boot. Subsequent fixes have not addressed the problem for us, so we already reverted the patch from chromeos-5.15 and will revert it from chromeos-5.10 as well. I don't know if this is a Chromebook specific problem, or if it affects mainline, so this is just a heads-up in case others experience similar problems. Thanks, Guenter
On 1.09.23 г. 20:30 ч., Guenter Roeck wrote: > Hi, > > On Wed, Jun 14, 2023 at 01:39:46AM +0200, Thomas Gleixner wrote: >> Initializing the FPU during the early boot process is a pointless >> exercise. Early boot is convoluted and fragile enough. >> >> Nothing requires that the FPU is set up early. It has to be initialized >> before fork_init() because the task_struct size depends on the FPU register >> buffer size. >> >> Move the initialization to arch_cpu_finalize_init() which is the perfect >> place to do so. >> >> No functional change. >> >> This allows to remove quite some of the custom early command line parsing, >> but that's subject to the next installment. >> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > The backport of this patch into chromeos-5.10 and chromeos-5.15 via stable > relase merges is causing various Chromebooks (not all of them) to crash > early during boot. Subsequent fixes have not addressed the problem for us, > so we already reverted the patch from chromeos-5.15 and will revert it > from chromeos-5.10 as well. > > I don't know if this is a Chromebook specific problem, or if it affects > mainline, so this is just a heads-up in case others experience similar > problems. Looking at 5.15 the following prerequisites are likely missing: af8060279968 ("mm: Move mm_cachep initialization to mm_init()") 5b93a83649c7 ("x86/mm: Initialize text poking earlier") Can you test with those patches backported to 5.15.y and see if it works? > > Thanks, > Guenter
On 1.09.23 г. 20:30 ч., Guenter Roeck wrote: > Hi, > > On Wed, Jun 14, 2023 at 01:39:46AM +0200, Thomas Gleixner wrote: >> Initializing the FPU during the early boot process is a pointless >> exercise. Early boot is convoluted and fragile enough. >> >> Nothing requires that the FPU is set up early. It has to be initialized >> before fork_init() because the task_struct size depends on the FPU register >> buffer size. >> >> Move the initialization to arch_cpu_finalize_init() which is the perfect >> place to do so. >> >> No functional change. >> >> This allows to remove quite some of the custom early command line parsing, >> but that's subject to the next installment. >> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > The backport of this patch into chromeos-5.10 and chromeos-5.15 via stable > relase merges is causing various Chromebooks (not all of them) to crash > early during boot. Subsequent fixes have not addressed the problem for us, > so we already reverted the patch from chromeos-5.15 and will revert it > from chromeos-5.10 as well. > > I don't know if this is a Chromebook specific problem, or if it affects > mainline, so this is just a heads-up in case others experience similar > problems. Another thing - if you choose to revert the arch_finalize patch then bear in mind that the GDS' 'force' option is rendered inoperable as the FPU can't be disabled due to the way things are sequenced. > > Thanks, > Guenter
On 9/1/23 11:00, Nikolay Borisov wrote: > > > On 1.09.23 г. 20:30 ч., Guenter Roeck wrote: >> Hi, >> >> On Wed, Jun 14, 2023 at 01:39:46AM +0200, Thomas Gleixner wrote: >>> Initializing the FPU during the early boot process is a pointless >>> exercise. Early boot is convoluted and fragile enough. >>> >>> Nothing requires that the FPU is set up early. It has to be initialized >>> before fork_init() because the task_struct size depends on the FPU register >>> buffer size. >>> >>> Move the initialization to arch_cpu_finalize_init() which is the perfect >>> place to do so. >>> >>> No functional change. >>> >>> This allows to remove quite some of the custom early command line parsing, >>> but that's subject to the next installment. >>> >>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >> >> The backport of this patch into chromeos-5.10 and chromeos-5.15 via stable >> relase merges is causing various Chromebooks (not all of them) to crash >> early during boot. Subsequent fixes have not addressed the problem for us, >> so we already reverted the patch from chromeos-5.15 and will revert it >> from chromeos-5.10 as well. >> >> I don't know if this is a Chromebook specific problem, or if it affects >> mainline, so this is just a heads-up in case others experience similar >> problems. > > > Looking at 5.15 the following prerequisites are likely missing: > > af8060279968 ("mm: Move mm_cachep initialization to mm_init()") > 5b93a83649c7 ("x86/mm: Initialize text poking earlier") > > > Can you test with those patches backported to 5.15.y and see if it works? Both patches are already in v5.10.y and 5.15.y, so unfortunately that doesn't help. Guenter
On Fri, Sep 01, 2023 at 09:02:19PM +0300, Nikolay Borisov wrote: > > > On 1.09.23 г. 20:30 ч., Guenter Roeck wrote: > > Hi, > > > > On Wed, Jun 14, 2023 at 01:39:46AM +0200, Thomas Gleixner wrote: > > > Initializing the FPU during the early boot process is a pointless > > > exercise. Early boot is convoluted and fragile enough. > > > > > > Nothing requires that the FPU is set up early. It has to be initialized > > > before fork_init() because the task_struct size depends on the FPU register > > > buffer size. > > > > > > Move the initialization to arch_cpu_finalize_init() which is the perfect > > > place to do so. > > > > > > No functional change. > > > > > > This allows to remove quite some of the custom early command line parsing, > > > but that's subject to the next installment. > > > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > > > The backport of this patch into chromeos-5.10 and chromeos-5.15 via stable > > relase merges is causing various Chromebooks (not all of them) to crash > > early during boot. Subsequent fixes have not addressed the problem for us, > > so we already reverted the patch from chromeos-5.15 and will revert it > > from chromeos-5.10 as well. > > > > I don't know if this is a Chromebook specific problem, or if it affects > > mainline, so this is just a heads-up in case others experience similar > > problems. > > > Another thing - if you choose to revert the arch_finalize patch then bear in > mind that the GDS' 'force' option is rendered inoperable as the FPU can't be > disabled due to the way things are sequenced. I understand, but that is still better than not being able to boot in the first place. Thanks, Guenter
--- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1604,8 +1604,6 @@ static void __init early_identify_cpu(st sld_setup(c); - fpu__init_system(); - #ifdef CONFIG_X86_32 /* * Regardless of whether PCID is enumerated, the SDM says @@ -2287,8 +2285,6 @@ void cpu_init(void) doublefault_init_cpu_tss(); - fpu__init_cpu(); - if (is_uv_system()) uv_cpu_init(); @@ -2304,6 +2300,7 @@ void cpu_init_secondary(void) */ cpu_init_exception_handling(); cpu_init(); + fpu__init_cpu(); } #endif @@ -2396,6 +2393,13 @@ void __init arch_cpu_finalize_init(void) '0' + (boot_cpu_data.x86 > 6 ? 6 : boot_cpu_data.x86); } + /* + * Must be before alternatives because it might set or clear + * feature bits. + */ + fpu__init_system(); + fpu__init_cpu(); + alternative_instructions(); if (IS_ENABLED(CONFIG_X86_64)) {
Initializing the FPU during the early boot process is a pointless exercise. Early boot is convoluted and fragile enough. Nothing requires that the FPU is set up early. It has to be initialized before fork_init() because the task_struct size depends on the FPU register buffer size. Move the initialization to arch_cpu_finalize_init() which is the perfect place to do so. No functional change. This allows to remove quite some of the custom early command line parsing, but that's subject to the next installment. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/cpu/common.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)