linux-kernel - Re: [PATCH v6 2/9] x86/bhi: Make clear_bhb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251215180136.sjtvt57autnrassg@desk>
Date: Mon, 15 Dec 2025 10:01:36 -0800
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Nikolay Borisov <nik.borisov@...e.com>, x86@...nel.org,
	David Kaplan <david.kaplan@....com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Sean Christopherson <seanjc@...gle.com>,
	Paolo Bonzini <pbonzini@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	Asit Mallick <asit.k.mallick@...el.com>,
	Tao Zhang <tao1.zhang@...el.com>
Subject: Re: [PATCH v6 2/9] x86/bhi: Make clear_bhb_loop() effective on newer
 CPUs

On Sun, Dec 14, 2025 at 07:02:33PM +0000, David Laight wrote:
> On Sun, 14 Dec 2025 10:38:27 -0800
> Pawan Gupta <pawan.kumar.gupta@...ux.intel.com> wrote:
> 
> > On Wed, Dec 10, 2025 at 01:35:42PM +0000, David Laight wrote:
> > > On Wed, 10 Dec 2025 14:31:31 +0200
> > > Nikolay Borisov <nik.borisov@...e.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@...ux.intel.com>
> > > > > Reviewed-by: Nikolay Borisov <nik.borisov@...e.com>
> > > > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@...ux.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.  
> > 
> > Using globals was much more involved, requiring changes in atleast 3 files.
> > The current ALTERNATIVE approach is much simpler and avoids additional
> > handling to make sure that globals are set correctly for all mitigation
> > modes of BHI and VMSCAPE.
> > 
> > [ BTW, I am travelling on a vacation and will be intermittently checking my
> >   emails. ]
> > 
> > > 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.  
> > 
> > Thanks for catching this.
> 
> I was trying to find where it was called from.
> Failed to find the one on system call entry...

The macro CLEAR_BRANCH_HISTORY calls clear_bhb_loop() at system call entry.

> > > There are probably some schemes that only need one register.
> > > eg two separate ALTERNATIVE blocks.  
> > 
> > Also, I think it is better to use a callee-saved register like rbx to avoid
> > callers having to save/restore registers. Something like below:
> 
> I'm not sure.
> %ax is the return value so can be 'trashed' by a normal function call.
> But if the bpf code is saving %ax then it isn't expecting a normal call.

BHB clear sequence is executed at the end of the BPF JITted code, and %rax
is likely the return value of the BPF program. So, saving/restoring %rax
around the sequence makes sense to me.

> OTOH if you are going to save the register in clear_bhb_loop you might
> as well use %ax to get the slightly shorter instructions for %al.
> (I think 'movb' comes out shorter - as if it really matters.)

%rbx is a callee-saved register so it felt more intuitive to save/restore
it in clear_bhb_loop(). But, I can use %ax if you feel strongly.

> Definitely worth a comment that it must save all resisters.

Yes, will add a comment.

> I also wonder if it needs to setup a stack frame?

I don't know if thats necessary, objtool doesn't complain because
clear_bhb_loop() is marked STACK_FRAME_NON_STANDARD.

> Again, the code is so slow it won't matter.
> 
> 	David