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 <[email protected]>
---
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)) {
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 <[email protected]>
AuthorDate: Wed, 14 Jun 2023 01:39:46 +02:00
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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)) {
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 <[email protected]>
> >
> > 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
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 <[email protected]>
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 <[email protected]>
>
> 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