Re: [PATCH] x86/lib: don't use MMX before FPU initialization

[Borislav Petkov <bp@alien8.de>]

On Thu, Jan 14, 2021 at 01:36:57PM +0100, Krzysztof Mazur wrote:
> The OSFXSR must be set only on CPUs with SSE. There
> are some CPUs with 3DNow!, but without SSE and FXSR (like AMD
> Geode LX, which is still used in many embedded systems).
> So, I've changed that to:
> 
> if (unlikely(in_interrupt()) || (boot_cpu_has(X86_FEATURE_XMM) &&
> 		unlikely(!(cr4_read_shadow() & X86_CR4_OSFXSR))))

Why?

X86_CR4_OSFXSR won't ever be set on those CPUs but the test will be
performed anyway. So there's no need for boot_cpu_has().

> However, except for some embedded systems, probably almost nobody uses
> that code today, and the most imporant is avoiding future breakage.

Yes, exactly. K7 is a don't-care performance-wise nowadays.

> And I'm not sure which approach is better. Because that CR4 test tests
> for a feature that is not used in mmx_memcpy(), but it's used in
> kernel_fpu_begin(). And in future kernel_fpu_begin() may change and
> require also other stuff. So I think that the best approach would be
> delay any FPU optimized memcpy() after fpu__init_system() is executed.

I'd prefer the simplest approach as this code is almost never used. So
no need to complicate things unnecessarily.

> Subject: [PATCH] x86/lib: don't use mmx_memcpy() to early

s/to/too/

> The MMX 3DNow! optimized memcpy() is used very early,
> even before FPU is initialized in the kernel. It worked fine, but commit
> 7ad816762f9bf89e940e618ea40c43138b479e10 ("x86/fpu: Reset MXCSR
> to default in kernel_fpu_begin()") broke that. After that
> commit the kernel_fpu_begin() assumes that FXSR is enabled in
> the CR4 register on all processors with SSE. Because memcpy() is used
> before FXSR is enabled, the kernel crashes just after "Booting the kernel."
> message. It affects all kernels with CONFIG_X86_USE_3DNOW (enabled when
> some AMD/Cyrix processors are selected) on processors with SSE
> (like AMD K7, which supports both MMX 3DNow! and SSE).
> 

Fixes: 7ad816762f9b ("x86/fpu: Reset MXCSR to default in kernel_fpu_begin()")

...

Thx. 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette