From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Josh Poimboeuf <jpoimboe@kernel.org>,
David Kaplan <david.kaplan@amd.com>,
Sean Christopherson <seanjc@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
Asit Mallick <asit.k.mallick@intel.com>,
Tao Zhang <tao1.zhang@intel.com>
Subject: Re: [PATCH v3 3/3] x86/vmscape: Remove LFENCE from BHB clearing long loop
Date: Tue, 4 Nov 2025 14:01:00 -0800 [thread overview]
Message-ID: <20251104220100.wrorcuok5slqy74u@desk> (raw)
In-Reply-To: <c98e68f0-e5e2-482d-9a64-ad8164e4bae8@intel.com>
On Mon, Nov 03, 2025 at 12:45:35PM -0800, Dave Hansen wrote:
> On 10/27/25 16:43, Pawan Gupta wrote:
> > Long loop is used to clear the branch history when switching from a guest
> > to host userspace. The LFENCE barrier is not required in this case as ring
> > transition itself acts as a barrier.
> >
> > Move the prologue, LFENCE and epilogue out of __CLEAR_BHB_LOOP macro to
> > allow skipping the LFENCE in the long loop variant. Rename the long loop
> > function to clear_bhb_long_loop_no_barrier() to reflect the change.
>
> Too. Much. Assembly.
>
> Is there a reason we can't do more of this in C?
Apart from VMSCAPE, BHB clearing is also required when entering kernel from
system calls. And one of the safety requirement is to absolutely not
execute any indirect call/jmp unless we have cleared the BHB. In a C
implementation we cannot guarantee that the compiler won't generate
indirect branches before the BHB clearing can be done.
> Can we have _one_ assembly function, please? One that takes the loop
> counts? No macros, no duplication functions. Just one:
This seems possible for all the C callers. ASM callers should stick to asm
versions of BHB clearing to guarantee the compiler did not do anything
funky that would break the mitigation.
> void __clear_bhb_loop(int inner, int outer);
>
> Then we have sensible code that looks like this:
>
> void clear_bhb_loop()
> {
> __clear_bhb_loop(inner, outer);
> lfence();
> }
>
> void clear_bhb_loop_nofence()
> {
> __clear_bhb_loop(inner, outer);
> }
>
> We don't need a short and a long *version*. We just have one function
> (or pair of functions) that gets called that works everywhere.
>
> Actually, if you just used global variables and called the assembly one:
>
> extern void clear_bhb_loop_nofence();
>
> then the other implementation would just be:
>
> void clear_bhb_loop()
> {
> __clear_bhb_loop(inner, outer);
> lfence();
> }
>
> Then we have *ONE* assembly function instead of four.
>
> Right? What am I missing?
Overall, these look to be good improvements to me. The only concern is
making sure that we don't inadvertently call the C version from places that
strictly require no indirect branches before BHB clearing.
> Does the LFENCE *need* to be before that last pop and RET?
At syscall entry, VMexit and BPF (for native BHI mitigation), it does not
matter whether the LFENCE is before or after the last RET, if we can
guarantee that there will be no indirect call/jmp before LFENCE. C version
may not be able to provide this guarantee.
For exit-to-userspace (for VMSCAPE), C implementation is perfectly fine
since the goal is to protect userspace.
To summarize, only 1 of the BHB clear callsite can safely use the C
version, while others need to continue to use the assembly version. I do
not anticipate more such callsites that would be okay with indirect
branches before BHB clearing.
I am open to suggestions on making the code more readable while ensuring
the safety.
next prev parent reply other threads:[~2025-11-04 22:01 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-27 23:43 [PATCH v3 0/3] VMSCAPE optimization for BHI variant Pawan Gupta
2025-10-27 23:43 ` [PATCH v3 1/3] x86/bhi: Add BHB clearing for CPUs with larger branch history Pawan Gupta
2025-11-03 20:04 ` Dave Hansen
2025-11-03 22:45 ` Pawan Gupta
2025-10-27 23:43 ` [PATCH v3 2/3] x86/vmscape: Replace IBPB with branch history clear on exit to userspace Pawan Gupta
2025-10-29 22:47 ` Sean Christopherson
2025-10-30 0:08 ` Pawan Gupta
2025-11-03 20:31 ` Dave Hansen
2025-11-06 23:40 ` Pawan Gupta
2025-11-19 10:33 ` Nikolay Borisov
2025-11-19 18:26 ` Pawan Gupta
2025-10-27 23:43 ` [PATCH v3 3/3] x86/vmscape: Remove LFENCE from BHB clearing long loop Pawan Gupta
2025-11-03 20:45 ` Dave Hansen
2025-11-04 22:01 ` Pawan Gupta [this message]
2025-11-04 22:35 ` Dave Hansen
2025-11-04 23:36 ` Pawan Gupta
2025-11-03 20:07 ` [PATCH v3 0/3] VMSCAPE optimization for BHI variant Dave Hansen
2025-11-03 23:03 ` Pawan Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251104220100.wrorcuok5slqy74u@desk \
--to=pawan.kumar.gupta@linux.intel.com \
--cc=asit.k.mallick@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=david.kaplan@amd.com \
--cc=hpa@zytor.com \
--cc=jpoimboe@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tao1.zhang@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®