Re: [PATCH v3 2/2] zstd: use cpu_feature_enabled() for in-kernel BMI2 dispatch

[Dhruva G <goledhruva@gmail.com>]

Hi Usama,

On 01-09-2026 16:37, Usama Arif wrote:
> Zstd's dynamic BMI2 implementation probes CPUID when a compression or
> decompression context is initialized, stores the result in the context,
> and tests that value at every dispatch site. In normal kernel builds this
> bypasses the x86 feature policy and uses ordinary runtime branches instead
> of allowing x86 alternatives to resolve the feature check at boot.
> 
> Add ZSTD_USE_BMI2() and use it at every runtime BMI2/default selector. For
> normal x86 kernel objects, the predicate expands directly to
> cpu_feature_enabled(X86_FEATURE_BMI2). When dynamic BMI2 dispatch is not
> available it is false; other builds retain the caller-provided flag. Keep
> the existing HUF conditional layout because DYNAMIC_BMI2 also controls
> whether target-attributed variants are emitted.
> 
> Add the matching ZSTD_SET_BMI2() abstraction for context initialization.
> Normal x86 kernel objects and builds without dynamic BMI2 do not cache CPU
> state. Other builds with dynamic dispatch, including preboot, retain the
> existing behavior. Keep the BMI2 members in the context structures so
> their layouts do not change, and make the accessors return zero when
> cached state is unused.
> 
> Select the normal-kernel policy in zstd_deps.h. Builds which define
> __DISABLE_EXPORTS, including the x86 preboot decompressor, retain the existing
> CPUID-backed dispatch because the normal alternatives infrastructure is not
> available there.
> 
> A 4 KiB zstd-generic crypto_acomp benchmark in a one-vCPU KVM guest gave
> these median results:
> 
>                         Before      After     Change
>   Compression      16,634 ns/op  13,394 ns/op  -19.5%
>   Decompression     3,480 ns/op     963 ns/op  -72.3%
> 
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---

These comments still say BMI2 support is determined once per context:
- lib/zstd/compress/zstd_compress_internal.h:473
- lib/zstd/decompress/zstd_decompress_internal.h:159

>  lib/zstd/common/compiler.h                    | 12 ++++++++++++
>  lib/zstd/common/entropy_common.c              | 12 ++++++------
>  lib/zstd/common/fse_decompress.c              |  7 ++++---
>  lib/zstd/common/zstd_deps.h                   |  5 +++++
>  lib/zstd/compress/huf_compress.c              |  6 +++++-
>  lib/zstd/compress/zstd_compress.c             | 12 ++++++------
>  lib/zstd/compress/zstd_compress_internal.h    |  9 +++++++++
>  lib/zstd/compress/zstd_compress_sequences.c   |  8 +++++---
>  lib/zstd/compress/zstd_compress_superblock.c  |  2 +-
>  lib/zstd/decompress/huf_decompress.c          | 18 +++++++++---------
>  lib/zstd/decompress/zstd_decompress.c         |  4 +---
>  lib/zstd/decompress/zstd_decompress_block.c   | 19 +++++++------------
>  .../decompress/zstd_decompress_internal.h     |  2 +-
>  13 files changed, 71 insertions(+), 45 deletions(-)
> 
> diff --git a/lib/zstd/common/compiler.h b/lib/zstd/common/compiler.h
> index dc9bd15e174e9..47f6c0372c58c 100644
> --- a/lib/zstd/common/compiler.h
> +++ b/lib/zstd/common/compiler.h
> @@ -14,6 +14,7 @@
>  
>  #include <linux/types.h>
>  
> +#include "zstd_deps.h"
>  #include "portability_macros.h"
>  
>  /*-*******************************************************
> @@ -96,6 +97,17 @@
>   */
>  #define BMI2_TARGET_ATTRIBUTE TARGET_ATTRIBUTE("lzcnt,bmi,bmi2")
>  
> +#if !DYNAMIC_BMI2
> +#  define ZSTD_USE_BMI2(bmi2) 0
> +#  define ZSTD_SET_BMI2(state, value) do { } while (0)
> +#elif defined(ZSTD_USE_KERNEL_CPU_FEATURES)
> +#  define ZSTD_USE_BMI2(bmi2) cpu_feature_enabled(X86_FEATURE_BMI2)

Here, we do not include <asm/cpufeature.h>. Instead, every current .c user includes that header separately.
This works today, but it maybe fragile: the next user of ZSTD_USE_BMI2() can fail to compile unless they know
about this hidden requirement. 
Do you think perhaps we should provide that here in this header itself?

With that, feel free to add

Reviewed-by: Dhruva Gole <goledhruva@gmail.com>