Re: [PATCH v6 2/9] x86/bhi: Make clear_bhb_loop() effective on newer CPUs

[David Laight <david.laight.linux@gmail.com>]

On Wed, 10 Dec 2025 14:31:31 +0200
Nikolay Borisov <nik.borisov@suse.com> wrote:

> On 2.12.25 г. 8:19 ч., Pawan Gupta wrote:
> > As a mitigation for BHI, clear_bhb_loop() executes branches that overwrites
> > the Branch History Buffer (BHB). On Alder Lake and newer parts this
> > sequence is not sufficient because it doesn't clear enough entries. This
> > was not an issue because these CPUs have a hardware control (BHI_DIS_S)
> > that mitigates BHI in kernel.
> > 
> > BHI variant of VMSCAPE requires isolating branch history between guests and
> > userspace. Note that there is no equivalent hardware control for userspace.
> > To effectively isolate branch history on newer CPUs, clear_bhb_loop()
> > should execute sufficient number of branches to clear a larger BHB.
> > 
> > Dynamically set the loop count of clear_bhb_loop() such that it is
> > effective on newer CPUs too. Use the hardware control enumeration
> > X86_FEATURE_BHI_CTRL to select the appropriate loop count.
> > 
> > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>  
> 
> nit: My RB tag is incorrect, while I did agree with Dave's suggestion to 
> have global variables for the loop counts I haven't' really seen the 
> code so I couldn't have given my RB on something which I haven't seen 
> but did agree with in principle.

I thought the plan was to use global variables rather than ALTERNATIVE.
The performance of this code is dominated by the loop.

I also found this code in arch/x86/net/bpf_jit_comp.c:
	if (cpu_feature_enabled(X86_FEATURE_CLEAR_BHB_LOOP)) {
		/* The clearing sequence clobbers eax and ecx. */
		EMIT1(0x50); /* push rax */
		EMIT1(0x51); /* push rcx */
		ip += 2;

		func = (u8 *)clear_bhb_loop;
		ip += x86_call_depth_emit_accounting(&prog, func, ip);

		if (emit_call(&prog, func, ip))
			return -EINVAL;
		EMIT1(0x59); /* pop rcx */
		EMIT1(0x58); /* pop rax */
	}
which appears to assume that only rax and rcx are changed.
Since all the counts are small, there is nothing stopping the code
using the 8-bit registers %al, %ah, %cl and %ch.

There are probably some schemes that only need one register.
eg two separate ALTERNATIVE blocks.

	David

> 
> Now that I have seen the code I'm willing to give my :
> 
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
> > ---
> >   arch/x86/entry/entry_64.S | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> > index 886f86790b4467347031bc27d3d761d5cc286da1..9f6f4a7c5baf1fe4e3ab18b11e25e2fbcc77489d 100644
> > --- a/arch/x86/entry/entry_64.S
> > +++ b/arch/x86/entry/entry_64.S
> > @@ -1536,7 +1536,11 @@ SYM_FUNC_START(clear_bhb_loop)
> >   	ANNOTATE_NOENDBR
> >   	push	%rbp
> >   	mov	%rsp, %rbp
> > -	movl	$5, %ecx
> > +
> > +	/* loop count differs based on BHI_CTRL, see Intel's BHI guidance */
> > +	ALTERNATIVE "movl $5,  %ecx; movl $5, %edx",	\
> > +		    "movl $12, %ecx; movl $7, %edx", X86_FEATURE_BHI_CTRL  
> 
> nit: Just
> 
> > +
> >   	ANNOTATE_INTRA_FUNCTION_CALL
> >   	call	1f
> >   	jmp	5f
> > @@ -1557,7 +1561,7 @@ SYM_FUNC_START(clear_bhb_loop)
> >   	 * but some Clang versions (e.g. 18) don't like this.
> >   	 */
> >   	.skip 32 - 18, 0xcc
> > -2:	movl	$5, %eax
> > +2:	movl	%edx, %eax
> >   3:	jmp	4f
> >   	nop
> >   4:	sub	$1, %eax
> >   
> 
>