Re: [PATCH v11 09/12] x86/vmscape: Use static_call() for predictor flush

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Performance regressions in highly sensitive fast paths due to replaced zero-overhead checks.
- [Low] Missing `<linux/static_call.h>` header inclusion in `arch/x86/include/asm/entry-common.h`.
- [Low] Out-of-sync tools ABI header.
--

commit e665a60c1a2370dfaa4dba80cd6f14bfe1c7e97b
Author: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

x86/vmscape: Use static_call() for predictor flush

This commit replaces the flag-based mitigation selector for VMSCAPE with a
static_call to the mitigation function. This avoids adding multiple checks
for different mitigation options at exit-to-userspace.

> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
[ ... ]
> @@ -503,7 +503,7 @@
>  #define X86_FEATURE_TSA_SQ_NO		(21*32+11) /* AMD CPU not vulnerable to TSA-SQ */
>  #define X86_FEATURE_TSA_L1_NO		(21*32+12) /* AMD CPU not vulnerable to TSA-L1 */
>  #define X86_FEATURE_CLEAR_CPU_BUF_VM	(21*32+13) /* Clear CPU buffers using VERW before VMRUN */
> -#define X86_FEATURE_IBPB_EXIT_TO_USER	(21*32+14) /* Use IBPB on exit-to-userspace, see VMSCAPE bug */
> +/* Free */
>  #define X86_FEATURE_ABMC		(21*32+15) /* Assignable Bandwidth Monitoring Counters */

This isn't a bug, but freeing the feature flag here without updating
tools/arch/x86/include/asm/cpufeatures.h might trigger ABI header sync
warnings during tools builds. Is a follow-up planned for the tools header?

> diff --git a/arch/x86/include/asm/entry-common.h b/arch/x86/include/asm/entry-common.h
> --- a/arch/x86/include/asm/entry-common.h
> +++ b/arch/x86/include/asm/entry-common.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/randomize_kstack.h>
>  #include <linux/user-return-notifier.h>
> +#include <linux/static_call_types.h>

Does this code need to include linux/static_call.h directly?

Using static_call_cond below relies on the macro definition from
linux/static_call.h. Depending on transitive includes might lead to
implicit function declaration or macro expansion errors.

[ ... ]
> @@ -94,10 +95,8 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>  	 */
>  	choose_random_kstack_offset(rdtsc());
>  
> -	/* Avoid unnecessary reads of 'x86_predictor_flush_exit_to_user' */
> -	if (cpu_feature_enabled(X86_FEATURE_IBPB_EXIT_TO_USER) &&
> -	    this_cpu_read(x86_predictor_flush_exit_to_user)) {
> -		write_ibpb();
> +	if (unlikely(this_cpu_read(x86_predictor_flush_exit_to_user))) {
> +		static_call_cond(vmscape_predictor_flush)();
>  		this_cpu_write(x86_predictor_flush_exit_to_user, false);
>  	}
>  }

Does removing the alternative-patched cpu feature check introduce a
performance regression here?

By dropping the zero-overhead check, it appears an unconditional per-cpu
memory read is now evaluated on every exit to userspace. This seems to
reverse the explicitly documented optimization to avoid unnecessary reads.

Could a static key or static branch be used to maintain the zero-overhead
path when the mitigation is disabled?

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -11463,7 +11463,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  	 * set for the CPU that actually ran the guest, and not the CPU that it
>  	 * may migrate to.
>  	 */
> -	if (cpu_feature_enabled(X86_FEATURE_IBPB_EXIT_TO_USER))
> +	if (static_call_query(vmscape_predictor_flush))
>  		this_cpu_write(x86_predictor_flush_exit_to_user, true);
>  
>  	/*

Will replacing the cpu feature check with static_call_query add overhead on
the KVM VM-exit fast path?

Looking at linux/static_call.h, static_call_query reads a global function
pointer.

Would it be better to use a static key here to avoid the memory read when
the mitigation is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422-vmscape-bhb-v11-0-b18e0cf32af4@linux.intel.com?part=9