[PATCH RESEND] xor: add missing vzeroupper to AVX code

[PATCH RESEND] xor: add missing vzeroupper to AVX code

[Eric Biggers]

Since the AVX optimized XOR code uses YMM registers, execute vzeroupper
before returning from it.  This is needed to avoid degrading the
performance of any later SSE code that may happen to be executed.

Fixes: ea4d26ae24e5 ("raid5: add AVX optimized RAID5 checksumming")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---

This didn't get taken through the x86 tree.  Andrew, it seems you're
taking patches to lib/raid/.  Can you apply this one?

 lib/raid/xor/x86/xor-avx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/raid/xor/x86/xor-avx.c b/lib/raid/xor/x86/xor-avx.c
index f7777d7aa269..95b21e7225e8 100644
--- a/lib/raid/xor/x86/xor-avx.c
+++ b/lib/raid/xor/x86/xor-avx.c
@@ -147,6 +147,7 @@ static void xor_gen_avx(void *dest, void **srcs, unsigned int src_cnt,
 {
 	kernel_fpu_begin();
 	xor_gen_avx_inner(dest, srcs, src_cnt, bytes);
+	asm volatile("vzeroupper");
 	kernel_fpu_end();
 }
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.55.0

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[Christoph Hellwig]

On Mon, Aug 31, 2026 at 02:22:48PM -0700, Eric Biggers wrote:
> Since the AVX optimized XOR code uses YMM registers, execute vzeroupper
> before returning from it.  This is needed to avoid degrading the
> performance of any later SSE code that may happen to be executed.
> 
> Fixes: ea4d26ae24e5 ("raid5: add AVX optimized RAID5 checksumming")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
> 
> This didn't get taken through the x86 tree.  Andrew, it seems you're
> taking patches to lib/raid/.  Can you apply this one?

Can we do kernel_avx_{begin,end} instead of having to open code
and document this everywhere, please?

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[David Laight]

On Wed, 2 Sep 2026 15:37:06 +0200
Christoph Hellwig <hch@lst.de> wrote:

> On Mon, Aug 31, 2026 at 02:22:48PM -0700, Eric Biggers wrote:
> > Since the AVX optimized XOR code uses YMM registers, execute vzeroupper
> > before returning from it.  This is needed to avoid degrading the
> > performance of any later SSE code that may happen to be executed.
> > 
> > Fixes: ea4d26ae24e5 ("raid5: add AVX optimized RAID5 checksumming")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> > 
> > This didn't get taken through the x86 tree.  Andrew, it seems you're
> > taking patches to lib/raid/.  Can you apply this one?  
> 
> Can we do kernel_avx_{begin,end} instead of having to open code
> and document this everywhere, please?

In which case I think you want the vzeroupper in kernel_avx_begin().

Actually, for some cpu at least, you need vzeroupper in kernel_fpu_begin()
even if the code only uses the SSE registers.

See: https://stackoverflow.com/questions/41303780/why-is-this-sse-code-6-times-slower-without-vzeroupper-on-skylake
Basically, on Skylake, all SSE hit a penalty if any ymm high bits might be non-zero.
Don't know what has changed since...

I don't have the Intel optimisation manual downloaded (ok, I might have
it but have NFI where), and the link on that page is broken.

David

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[Eric Biggers]

On Wed, Sep 02, 2026 at 03:37:06PM +0200, Christoph Hellwig wrote:
> On Mon, Aug 31, 2026 at 02:22:48PM -0700, Eric Biggers wrote:
> > Since the AVX optimized XOR code uses YMM registers, execute vzeroupper
> > before returning from it.  This is needed to avoid degrading the
> > performance of any later SSE code that may happen to be executed.
> > 
> > Fixes: ea4d26ae24e5 ("raid5: add AVX optimized RAID5 checksumming")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> > 
> > This didn't get taken through the x86 tree.  Andrew, it seems you're
> > taking patches to lib/raid/.  Can you apply this one?
> 
> Can we do kernel_avx_{begin,end} instead of having to open code
> and document this everywhere, please?

Again, there are cases in the kernel where both AVX and SSE are used
within a single kernel-mode FPU section, or where a CPU feature check
occurs within the section and one or the other is used.  So that
abstraction will not work as-is, and it would be different from all
userspace code as well.  If you'd like to try to refactor everything you
can try to do so, but let's not block fixing these bugs first.

Also, AVX != "vzeroupper is needed".  The relevant thing is the width of
the registers used.  There is 128-bit AVX code.

- Eric

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[David Laight]

On Wed, 2 Sep 2026 09:23:59 -0700
Eric Biggers <ebiggers@kernel.org> wrote:

> On Wed, Sep 02, 2026 at 03:37:06PM +0200, Christoph Hellwig wrote:
> > On Mon, Aug 31, 2026 at 02:22:48PM -0700, Eric Biggers wrote:  
> > > Since the AVX optimized XOR code uses YMM registers, execute vzeroupper
> > > before returning from it.  This is needed to avoid degrading the
> > > performance of any later SSE code that may happen to be executed.
> > > 
> > > Fixes: ea4d26ae24e5 ("raid5: add AVX optimized RAID5 checksumming")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > > ---
> > > 
> > > This didn't get taken through the x86 tree.  Andrew, it seems you're
> > > taking patches to lib/raid/.  Can you apply this one?  
> > 
> > Can we do kernel_avx_{begin,end} instead of having to open code
> > and document this everywhere, please?  
> 
> Again, there are cases in the kernel where both AVX and SSE are used
> within a single kernel-mode FPU section, or where a CPU feature check
> occurs within the section and one or the other is used.  So that
> abstraction will not work as-is, and it would be different from all
> userspace code as well.  If you'd like to try to refactor everything you
> can try to do so, but let's not block fixing these bugs first.
> 
> Also, AVX != "vzeroupper is needed".  The relevant thing is the width of
> the registers used.  There is 128-bit AVX code.

It also depends on the instruction encoding used for 128-bit AVX code.
If the VEX encoding is used the high bits of the ymm registers get cleared
(rather than preserved) and you get different delays.
Flipping to/from VEX encoded 128bit instructions adds delays on some cpu.

Yes, it is all a mess....

David

> 
> - Eric
> 

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[Eric Biggers]

On Wed, Sep 02, 2026 at 07:19:56PM +0100, David Laight wrote:
> It also depends on the instruction encoding used for 128-bit AVX code.
> If the VEX encoding is used the high bits of the ymm registers get cleared
> (rather than preserved) and you get different delays.
> Flipping to/from VEX encoded 128bit instructions adds delays on some cpu.

All instructions operating on XMM registers that aren't VEX or EVEX
coded are typically called "SSE instructions", not AVX.  (Even if they
require something that wasn't in the original SSE.)

The point is that "128-bit AVX" exists, typically because the AES and
carryless multiplication instructions have typically been 128-bit only.
256 and 512-bit support for those came much later than the rest of AVX.

Just to give a random example, aes_xts_encrypt_aesni_avx() and
aes_xts_decrypt_aesni_avx() are "128-bit AVX".  And they indeed don't do
vzeroupper, because they don't need to.

Some cases such as lib/crc/x86/crc-pclmul-template.S skip providing
128-bit AVX code and just have the CPUs that could run it instead run
the 128-bit SSE code, but that is a tradeoff made in those cases.

- Eric

Re: [PATCH RESEND] xor: add missing vzeroupper to AVX code

[Christoph Hellwig]

On Wed, Sep 02, 2026 at 09:23:59AM -0700, Eric Biggers wrote:
> > 
> > Can we do kernel_avx_{begin,end} instead of having to open code
> > and document this everywhere, please?
> 
> Again, there are cases in the kernel where both AVX and SSE are used
> within a single kernel-mode FPU section, or where a CPU feature check
> occurs within the section and one or the other is used.  So that
> abstraction will not work as-is,

So don't use it for that one case (which I've seen, but make it two or
three and the same still applies), and everyone else gets to use the
easy case.

> and it would be different from all
> userspace code as well.

I don't know how you defined "all".  Almost no userspace code has
kernel_fpu_{begin,end} to start with.  The one code base I've recently
ported to the kernel that has it, also has kernel_avx_{begin,end}
already.