Re: [PATCH v2 4/8] lib/crypto: x86: Stop using cpu_has_xfeatures()

On Thu, Jul 30, 2026 at 04:19:44PM -0700, Borislav Petkov wrote:
> On Mon, Jul 27, 2026 at 07:15:59PM -0700, Eric Biggers wrote:
> > Checking both boot_cpu_has() and cpu_has_xfeatures() has never really
> > been needed in practice, and it's never been universally done (e.g.,
> > lib/raid/ omits cpu_has_xfeatures()).  Nevertheless, both x86 and UML
> > now explicitly clear the AVX and AVX-512 flags if their xfeatures are
> > missing, which should remove any remaining doubts.
> > 
> > Thus, remove all the calls to cpu_has_xfeatures().
> > 
> > Signed-off-by: Eric Biggers <[email protected]>
> > ---
> >  lib/crypto/x86/blake2s.h  | 4 +---
> >  lib/crypto/x86/chacha.h   | 3 +--
> >  lib/crypto/x86/nh.h       | 4 +---
> >  lib/crypto/x86/poly1305.h | 7 ++-----
> >  lib/crypto/x86/sha1.h     | 4 +---
> >  lib/crypto/x86/sha256.h   | 4 +---
> >  lib/crypto/x86/sha512.h   | 3 +--
> >  lib/crypto/x86/sm3.h      | 3 +--
> >  8 files changed, 9 insertions(+), 23 deletions(-)
> > 
> > diff --git a/lib/crypto/x86/blake2s.h b/lib/crypto/x86/blake2s.h
> > index f8eed6cb042e4..0f7c51f055c8f 100644
> > --- a/lib/crypto/x86/blake2s.h
> > +++ b/lib/crypto/x86/blake2s.h
> > @@ -55,8 +55,6 @@ static void blake2s_mod_init_arch(void)
> >  	if (boot_cpu_has(X86_FEATURE_AVX) &&
> >  	    boot_cpu_has(X86_FEATURE_AVX2) &&
> >  	    boot_cpu_has(X86_FEATURE_AVX512F) &&
> > -	    boot_cpu_has(X86_FEATURE_AVX512VL) &&
> > -	    cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM |
> > -			      XFEATURE_MASK_AVX512, NULL))
> > +	    boot_cpu_has(X86_FEATURE_AVX512VL))
> >  		static_branch_enable(&blake2s_use_avx512);
> >  }
> 
> You could add a patch at the end or I can do it which does
> 
> 	s/boot_cpu_has/cpu_feature_enabled/
> 
> as latter is the interface we use now everywhere.
> 
> But yeah, it should be a separate patch.

cpu_feature_enabled() turns into an alternative (code patching), which
doesn't make much sense in any of the places you're asking for it to be
used, as they just check the features once at module init time to toggle
static keys.

I suppose that the static keys that are conditional on only one CPU
feature can be replaced with cpu_feature_enabled(), which should be
equally efficient.  However, many of the keys are conditional on two or
more features.

After that, there would be a mix of both versions.  So sure, it could
make sense to use cpu_feature_enabled() everywhere anyway, even when not
helpful.  I'm not sure it's clearly better than the status quo, though.

- Eric