[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: Missing `vzeroupper` in parallel AVX2/AVX-512 RAID6 and crypto implementations causes severe AVX-to-SSE transition penalties.
--

commit 0dc8e5aa3bec5867a6cc446b05765378e42b49c6
Author: Eric Biggers <ebiggers@kernel.org>

xor: add missing vzeroupper to AVX code

This commit adds a missing vzeroupper instruction to the AVX optimized XOR
code. This clears the upper bits of the YMM registers before returning,
preventing severe performance degradation when later SSE code is executed.

> diff --git a/lib/raid/xor/x86/xor-avx.c b/lib/raid/xor/x86/xor-avx.c
> index f7777d7aa269b..95b21e7225e8d 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();

[Severity: High]
This is a pre-existing issue, but this fix appears incomplete. While this
solves the missing vzeroupper in xor_gen_avx(), are there other parallel
AVX2/AVX-512 RAID6 and crypto implementations that suffer from this same
AVX-to-SSE transition penalty?

For instance, raid6_avx21_gen_syndrome() in lib/raid/raid6/x86/avx2.c and
raid6_avx5121_gen_syndrome() in lib/raid/raid6/x86/avx512.c both heavily use
YMM/ZMM registers but call kernel_fpu_end() without clearing the upper
halves:

lib/raid/raid6/x86/avx2.c:raid6_avx21_gen_syndrome() {
	...
	asm volatile("sfence" : : : "memory");
	kernel_fpu_end();
}

Similarly, raid6_2data_recov_avx2() in lib/raid/raid6/x86/recov_avx2.c
returns without executing vzeroupper:

lib/raid/raid6/x86/recov_avx2.c:raid6_2data_recov_avx2() {
	...
	kernel_fpu_end();
}

Also outside the RAID subsystem, routines like __aria_aesni_avx2_crypt_32way()
in arch/x86/crypto/aria-aesni-avx2-asm_64.S seem to have the exact same
omission:

arch/x86/crypto/aria-aesni-avx2-asm_64.S:__aria_aesni_avx2_crypt_32way() {
	...
	FRAME_END
	RET;
}

Could these functions also leave the CPU in a dirty AVX state and cause
severe performance regressions for subsequent legacy SSE instructions?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831212248.213805-1-ebiggers@kernel.org?part=1

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

[Eric Biggers]

On Mon, Aug 31, 2026 at 10:06:58PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix: Missing `vzeroupper` in parallel AVX2/AVX-512 RAID6 and crypto implementations causes severe AVX-to-SSE transition penalties.

Already addressed by other patches I sent.

- Eric

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.