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(-)
--- 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)) {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. 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 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
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 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 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...
The following commit has been merged into the x86/boot branch of tip:
Commit-ID: b81fac906a8f9e682e513ddd95697ec7a20878d4
Gitweb: https://git.kernel.org/tip/b81fac906a8f9e682e513ddd95697ec7a20878d4
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 14 Jun 2023 01:39:46 +02:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 16 Jun 2023 10:16:01 +02:00
x86/fpu: Move FPU initialization into arch_cpu_finalize_init()
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>
Link: https://lore.kernel.org/r/20230613224545.902376621@linutronix.de
---
arch/x86/kernel/cpu/common.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2807e5b..46ebdd6 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1604,8 +1604,6 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
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)) {© 2016 - 2026 Red Hat, Inc.