Re: [PATCH 0/3] zstd: probe the CPU for BMI2 support once, not per context

[Usama Arif <usama.arif@linux.dev>]

On 28/08/2026 21:19, Linus Torvalds wrote:
> [ Sorry for breaking threading - I have turned off IMAP access to my
> mailbox, and so I have issues replying to lore messages sanely ]
> 
> On Thu, 27 Aug 2026, Usama Arif wrote:
>>
>> On 27/08/2026 03:39, Eric Biggers wrote:
>>>
>>> Why not just use cpu_feature_enabled(X86_FEATURE_BMI2), which compiles
>>> down to a static branch?  All these issues are caused by lib/zstd/ using
>>> its own custom CPU feature detection code, instead of the normal CPU
>>> feature detection code that the rest of the kernel uses.
> 
> Yes, please. The zlib code is just broken in how it makes up its own
> random inine asm that is actively worse than what the kernel already
> exposes.
> 
>> The only issue I saw with that was that zstd is a standalone library that
>> is imported
> 
> Let's ignore that part, and just make it work right. The Zstd people
> should think about this problem on their side.
> 
>> Keep the existing raw CPUID fallback for preboot and other builds which
>> cannot use the normal x86 feature infrastructure. Also retain the early
>> return when dynamic dispatch is disabled.
> 
> No, this is not great, that whole
> 
>         cctx->bmi2 = ZSTD_cpuSupportsBmi2();
>         ...
>         if (bmi2) ...
> 
> model in zstd needs to just die.
> 
> For the kernel, the whole dynamic "test a variable" model is simply wrong.
> It should expand to
> 
>         if (cpu_feature_enabled(X86_FEATURE_BMI2))
> 
> because for the kernel, that becomes a simple static branch.
> 
> When zstd goes through that variable, it loses that entirely and
> instead turns it in a static assignment and then a dynamic test (well,
> not "entirely" - with inlining it could still recover the right code).
> 
> So zstd really should be fixed to get rid of that bad
> 
>         #if DYNAMIC_BMI2
>             if (bmi2) {
>          ....
> 
> pattern entirely, and be taught to have a *helper* macro that just turns
> into 0 for when DYNAMIC_BMI2 is not set, and turns into using that stupid
> flag in user mode, and for the kernel it should just turn into that
> "cpu_feature_enabled(X86_FEATURE_BMI2)"
> 
> Why does it check for both BMI1 and BMI2 anyway? And Arif added an
> extra check for ABM. That all looks bogus. You can't have BMI2 without
> having BMI1, so all this code looks completely bogus to begin with.
> 

The three checks came from interpreting BMI2_TARGET_ATTRIBUTE("lzcnt,bmi,bmi2")
as requiring all three feature bits. Agreed that this is unnecessary here.
The kernel path should check only X86_FEATURE_BMI2.

I was also trying to preserve zstd’s standalone-library behavior.
I will instead make normal x86 kernel builds use a helper that expands directly to
cpu_feature_enabled(X86_FEATURE_BMI2) at each dispatch site. The stored BMI2
context flag and private CPUID probe will be removed from that path.
Preboot and standalone builds will retain their existing caller-provided 
lag and CPUID-based detection.

I will send a revised patch in a little bit.

Thanks!
Usama