RE: [PATCH v2 1/5] perf/x86/intel/lbr: use setup_clear_cpu_cap instead of clear_cpu_cap

[H. Peter Anvin]

On November 2, 2022 9:23:00 AM PDT, "H. Peter Anvin" <hpa@xxxxxxxxx> wrote:
>On November 2, 2022 7:27:52 AM PDT, "Elliott, Robert (Servers)" <elliott@xxxxxxx> wrote:
>>
>>> From: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>>...
>>> (2) in particular holds even on bare metal.  The kernel bug here is that
>>> X86_FEATURE_AVX only tells you if the instructions are _present_, not if
>>> they are _usable_.   Indeed, the XCR0 check is present for all other
>>> files in arch/x86/crypto, either instead or in addition to
>>> boot_cpu_has(X86_FEATURE_AVX).
>>> 
>>> Maxim had sent a patch about a year ago to do it in aesni-intel-glue.c
>>> but Dave told him to fix the dependencies instead
>>> (https://lore.kernel.org/all/20211103124614.499580-1-
>>> mlevitsk@xxxxxxxxxx/).
>>>   What do you think of applying that patch instead?
>>
>>Most of the x86 crypto modules using X86_FEATURE_AVX do check
>>	cpu_has_xfeatures(XFEATURE_MASK_YMM, ...
>>
>>so it's probably prudent to add it to the rest (or remove it everywhere
>>if it is not needed).
>>
>>1. Currently checking XSAVE YMM:
>>  aria_aesni_avx_glue
>>  blake2s-glue
>>  camellia_aesni_avx2_glue	camellia_aesni_avx_glue
>>  cast5_avx_glue		cast6_avx_glue
>>  chacha_glue
>>  poly1305_glue
>>  serpent_avx2_glue		serpent_avx_glue
>>  sha1_ssse3_glue		sha256_ssse3_glue	sha512_ssse3_glue
>>  sm3_avx_glue
>>  sm4_aesni_avx2_glue	sm4_aesni_avx_glue
>>  twofish_avx_glue
>>
>>Currently not checking XSAVE YMM:
>>  aesni-intel_glue
>>  curve25519-x86_64
>>  nhpoly1305-avx2-glue
>>  polyval-clmulni_glue
>>
>>2. Similarly, modules using X86_FEATURE_AVX512F, X86_FEATURE_AVXX512VL
>>and/or X86_FEATURE_AVX512BW probably need to check XFEATURE_MASK_AVX512:
>>
>>Currently checking XSAVE AVX512:
>>  blake2s-glue
>>  poly1305_glue
>>
>>Currently not checking XSAVE AVX512:
>>  chacha_glue
>>
>>3. Similarly, modules using X86_FEATURE_XMM2 probably need to
>>check XFEATURE_MASK_SSE:
>>
>>Currently checking XSAVE SSE:
>>  aegis128-aesni-glue 
>>
>>Current not checking XSAVE SSE:
>>  nhpoly1305-sse2_glue
>>  serpent_sse2_glue
>>
>>
>>
>
>We have a dependency system for CPUID features. If you are going to do this (as opposed to "fixing" this in Qemu or just saying "don't do that, it isn't a valid hardware configuration."
One more thing: for obvious reasons, this doesn't fix user space if user space calls CPUID directly as opposed to reading /proc/cpuinfo or looking in sysfs. Unfortunately this is the rule rather than the exception, although for some features like AVX user space is also supposed to check XCR0, in which case it will work properly anyway.