[PATCH] x86/kerrnel/FPU: clear MP bit of cr0

[PATCH] x86/kerrnel/FPU: clear MP bit of cr0

[Khalid Ali]

From: Khalid Ali <khaliidcaliy@gmail.com>

Clear MP bit when initializing x87 FPU, since what it does
is making WAIT/FWAIT instructions to react to setting of TS flag.
Right now TS bit is cleared so MP should be cleared, as it is not
needed. This should set the bit in defined state.

Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
---
 arch/x86/kernel/fpu/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 998a08f17e33..2a2b45610b74 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -30,7 +30,7 @@ static void fpu__init_cpu_generic(void)
 		cr4_set_bits(cr4_mask);
 
 	cr0 = read_cr0();
-	cr0 &= ~(X86_CR0_TS|X86_CR0_EM); /* clear TS and EM */
+	cr0 &= ~(X86_CR0_TS|X86_CR0_EM|X86_CR0_MP); /* clear TS, EM and MP*/
 	if (!boot_cpu_has(X86_FEATURE_FPU))
 		cr0 |= X86_CR0_EM;
 	write_cr0(cr0);
-- 
2.49.0

Re: [PATCH] x86/kerrnel/FPU: clear MP bit of cr0

[Dave Hansen]

On 5/26/25 01:22, Khalid Ali wrote:
> Clear MP bit when initializing x87 FPU, since what it does
> is making WAIT/FWAIT instructions to react to setting of TS flag.
> Right now TS bit is cleared so MP should be cleared, as it is not
> needed. This should set the bit in defined state.

Hi Khalid,

What is the user visible result of this change?

Could you also give us a little more background about how you came upon
this problem? Was a program misbehaving and you narrowed it down to
this? Did you find it by inspection?

Re: [PATCH] x86/kerrnel/FPU: clear MP bit of cr0

[H. Peter Anvin]

On 2025-05-26 01:22, Khalid Ali wrote:
> From: Khalid Ali <khaliidcaliy@gmail.com>
> 
> Clear MP bit when initializing x87 FPU, since what it does
> is making WAIT/FWAIT instructions to react to setting of TS flag.
> Right now TS bit is cleared so MP should be cleared, as it is not
> needed. This should set the bit in defined state.

Setting it to a defined value is probably good, but the bit should 
definitely be SET, not clear.

If we end up relying on TS being set anywhere then we want WAIT/FWAIT to 
match, and that is the meaning of the MP bit.

	-hpa


> 
> Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
> ---
>   arch/x86/kernel/fpu/init.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
> index 998a08f17e33..2a2b45610b74 100644
> --- a/arch/x86/kernel/fpu/init.c
> +++ b/arch/x86/kernel/fpu/init.c
> @@ -30,7 +30,7 @@ static void fpu__init_cpu_generic(void)
>   		cr4_set_bits(cr4_mask);
>   
>   	cr0 = read_cr0();
> -	cr0 &= ~(X86_CR0_TS|X86_CR0_EM); /* clear TS and EM */
> +	cr0 &= ~(X86_CR0_TS|X86_CR0_EM|X86_CR0_MP); /* clear TS, EM and MP*/
>   	if (!boot_cpu_has(X86_FEATURE_FPU))
>   		cr0 |= X86_CR0_EM;
>   	write_cr0(cr0);

Re: [PATCH] x86/kerrnel/FPU: clear MP bit of cr0

[H. Peter Anvin]

On 2025-05-26 01:22, Khalid Ali wrote:
> From: Khalid Ali <khaliidcaliy@gmail.com>
> 
> Clear MP bit when initializing x87 FPU, since what it does
> is making WAIT/FWAIT instructions to react to setting of TS flag.
> Right now TS bit is cleared so MP should be cleared, as it is not
> needed. This should set the bit in defined state.
> 
> Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>

Note also that we DO initialize all bits of CR0 elsewhere in the code 
(head_32.S and head_64.S) to:

#define CR0_STATE       (X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | \
                          X86_CR0_NE | X86_CR0_WP | X86_CR0_AM | \
                          X86_CR0_PG)

So it is currently defined; this patch would actually *introduce* a bug.

It is a bit confusing to have the bits initialized in different places, 
so I would agree with setting it explicitly, even if it is redundant.

	-hpa