Re: [PATCH 3/6] raid6: add missing vzeroupper to AVX-512 code

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

Pre-existing issues:
- [High] Undefined behavior in inline assembly due to writing to input operands.
--

--- Patch [3]: [PATCH 3/6] raid6: add missing vzeroupper to AVX-512 code ---
commit c5b5d02d83bf2abe1380d3036a95bcf38db79ff8
Author: Eric Biggers <[email protected]>

raid6: add missing vzeroupper to AVX-512 code

Since the AVX-512 optimized RAID6 code uses ZMM 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: e0a491c12968 ("lib/raid6: Add AVX512 optimized gen_syndrome functions")
Fixes: 13c520b2993c ("lib/raid6: Add AVX512 optimized recovery functions")
Cc: [email protected]

> diff --git a/lib/raid/raid6/x86/recov_avx512.c b/lib/raid/raid6/x86/recov_avx512.c
> index ec72d5a30c01e..299a3f044d616 100644
> --- a/lib/raid/raid6/x86/recov_avx512.c
> +++ b/lib/raid/raid6/x86/recov_avx512.c
> @@ -211,6 +211,7 @@ static void raid6_2data_recov_avx512(int disks, size_t bytes, int faila,

[Severity: High]
This is a pre-existing issue, but while reviewing this code, I noticed that
earlier in this function and in raid6_datap_recov_avx512(), the inline
assembly writes to memory operands that are listed only in the input
constraints list.

For example, in raid6_2data_recov_avx512():

		asm volatile("vmovdqa64 %%zmm1, %0\n\t"
			     "vmovdqa64 %%zmm13,%1\n\t"
			     "vpxorq %%zmm1, %%zmm0, %%zmm0\n\t"
			     "vpxorq %%zmm13, %%zmm8, %%zmm8"
			     :
			     : "m" (dq[0]), "m" (dq[64]));

And in raid6_datap_recov_avx512():

		asm volatile("vmovdqa64 %%zmm1, %0\n\t"
			     "vmovdqa64 %%zmm14, %1\n\t"
			     "vmovdqa64 %%zmm2, %2\n\t"
			     "vmovdqa64 %%zmm12,%3"
			     :
			     : "m" (dq[0]), "m" (dq[64]), "m" (p[0]),
			       "m" (p[64]));

Could this cause data corruption or miscompilation?

GCC documentation forbids writing to input operands. If GCC assumes input
operands are unmodified, might it optimize away the writes, cache stale
values, or reorder accesses during RAID6 recovery?

>  #endif
>  	}
>  
> +	asm volatile("vzeroupper");
>  	kernel_fpu_end();
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3