| Message ID | df47d38d252b5825bc86afaf0d021b016286bf06.1743617897.git.jpoimboe@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/bugs: RSB mitigation fixes and documentation | expand |
On 4/2/25 13:19, Josh Poimboeuf wrote: > __write_ibpb() does IBPB, which (among other things) flushes branch type > predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation > has been disabled, branch type flushing isn't needed, in which case the > lighter-weight SBPB can be used. Maybe add something here that indicates the x86_pred_cmd variable tracks this optimization so switch to using that variable vs the hardcoded IBPB? Thanks, Tom > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > --- > arch/x86/entry/entry.S | 2 +- > arch/x86/kernel/cpu/bugs.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S > index 3a53319988b9..a5b421ec19c0 100644 > --- a/arch/x86/entry/entry.S > +++ b/arch/x86/entry/entry.S > @@ -21,7 +21,7 @@ > SYM_FUNC_START(__write_ibpb) > ANNOTATE_NOENDBR > movl $MSR_IA32_PRED_CMD, %ecx > - movl $PRED_CMD_IBPB, %eax > + movl _ASM_RIP(x86_pred_cmd), %eax > xorl %edx, %edx > wrmsr > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 310cb3f7139c..c8b8dc829046 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -58,7 +58,7 @@ EXPORT_SYMBOL_GPL(x86_spec_ctrl_base); > DEFINE_PER_CPU(u64, x86_spec_ctrl_current); > EXPORT_PER_CPU_SYMBOL_GPL(x86_spec_ctrl_current); > > -u64 x86_pred_cmd __ro_after_init = PRED_CMD_IBPB; > +u32 x86_pred_cmd __ro_after_init = PRED_CMD_IBPB; > EXPORT_SYMBOL_GPL(x86_pred_cmd); > > static u64 __ro_after_init x86_arch_cap_msr;
On Wed, Apr 2, 2025 at 11:20 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > __write_ibpb() does IBPB, which (among other things) flushes branch type > predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation > has been disabled, branch type flushing isn't needed, in which case the > lighter-weight SBPB can be used. When nested SVM is not supported, should KVM "promote" SRSO_USER_KERNEL_NO on the host to SRSO_NO in KVM_GET_SUPPORTED_CPUID? Or is a Linux guest clever enough to do the promotion itself if CPUID.80000001H:ECX.SVM[bit 2] is clear?
On Wed, Apr 02, 2025 at 03:41:25PM -0500, Tom Lendacky wrote: > On 4/2/25 13:19, Josh Poimboeuf wrote: > > __write_ibpb() does IBPB, which (among other things) flushes branch type > > predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation > > has been disabled, branch type flushing isn't needed, in which case the > > lighter-weight SBPB can be used. > > Maybe add something here that indicates the x86_pred_cmd variable tracks > this optimization so switch to using that variable vs the hardcoded IBPB? Indeed, adding a second paragraph to clarify that: x86/bugs: Use SBPB in write_ibpb() if applicable write_ibpb() does IBPB, which (among other things) flushes branch type predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation has been disabled, branch type flushing isn't needed, in which case the lighter-weight SBPB can be used. The 'x86_pred_cmd' variable already keeps track of whether IBPB or SBPB should be used. Use that instead of hardcoding IBPB.
On Wed, Apr 02, 2025 at 02:04:04PM -0700, Jim Mattson wrote: > On Wed, Apr 2, 2025 at 11:20 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > __write_ibpb() does IBPB, which (among other things) flushes branch type > > predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation > > has been disabled, branch type flushing isn't needed, in which case the > > lighter-weight SBPB can be used. > > When nested SVM is not supported, should KVM "promote" > SRSO_USER_KERNEL_NO on the host to SRSO_NO in KVM_GET_SUPPORTED_CPUID? > Or is a Linux guest clever enough to do the promotion itself if > CPUID.80000001H:ECX.SVM[bit 2] is clear? I'm afraid that question is beyond my pay grade, maybe some AMD or virt folks can chime in.
On Wed, Apr 2, 2025 at 7:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > On Wed, Apr 02, 2025 at 02:04:04PM -0700, Jim Mattson wrote: > > On Wed, Apr 2, 2025 at 11:20 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > > > __write_ibpb() does IBPB, which (among other things) flushes branch type > > > predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation > > > has been disabled, branch type flushing isn't needed, in which case the > > > lighter-weight SBPB can be used. > > > > When nested SVM is not supported, should KVM "promote" > > SRSO_USER_KERNEL_NO on the host to SRSO_NO in KVM_GET_SUPPORTED_CPUID? > > Or is a Linux guest clever enough to do the promotion itself if > > CPUID.80000001H:ECX.SVM[bit 2] is clear? > > I'm afraid that question is beyond my pay grade, maybe some AMD or virt > folks can chime in. That question aside, I'm not sure that this series is safe with respect to nested virtualization. If the CPU has SRSO_NO, then KVM will report SRSO_NO in KVM_GET_SUPPORTED_CPUID. However, in nested virtualization, the L1 guest and the L2 guest share a prediction domain. KVM currently ensures isolation between L1 and L2 with a call to indirect_branch_prediction_barrier() in svm_vcpu_load(). I think that particular barrier should *always* be a full IBPB--even if the host has SRSO_NO.
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Jim Mattson <jmattson@google.com> > Sent: Wednesday, April 9, 2025 11:07 AM > To: Josh Poimboeuf <jpoimboe@kernel.org> > Cc: x86@kernel.org; linux-kernel@vger.kernel.org; amit@kernel.org; > kvm@vger.kernel.org; Shah, Amit <Amit.Shah@amd.com>; Lendacky, Thomas > <Thomas.Lendacky@amd.com>; bp@alien8.de; tglx@linutronix.de; > peterz@infradead.org; pawan.kumar.gupta@linux.intel.com; corbet@lwn.net; > mingo@redhat.com; dave.hansen@linux.intel.com; hpa@zytor.com; > seanjc@google.com; pbonzini@redhat.com; daniel.sneddon@linux.intel.com; > kai.huang@intel.com; Das1, Sandipan <Sandipan.Das@amd.com>; > boris.ostrovsky@oracle.com; Moger, Babu <Babu.Moger@amd.com>; Kaplan, > David <David.Kaplan@amd.com>; dwmw@amazon.co.uk; > andrew.cooper3@citrix.com > Subject: Re: [PATCH v3 2/6] x86/bugs: Use SBPB in __write_ibpb() if applicable > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On Wed, Apr 2, 2025 at 7:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > On Wed, Apr 02, 2025 at 02:04:04PM -0700, Jim Mattson wrote: > > > On Wed, Apr 2, 2025 at 11:20 AM Josh Poimboeuf <jpoimboe@kernel.org> > wrote: > > > > > > > > __write_ibpb() does IBPB, which (among other things) flushes > > > > branch type predictions on AMD. If the CPU has SRSO_NO, or if the > > > > SRSO mitigation has been disabled, branch type flushing isn't > > > > needed, in which case the lighter-weight SBPB can be used. > > > > > > When nested SVM is not supported, should KVM "promote" > > > SRSO_USER_KERNEL_NO on the host to SRSO_NO in > KVM_GET_SUPPORTED_CPUID? > > > Or is a Linux guest clever enough to do the promotion itself if > > > CPUID.80000001H:ECX.SVM[bit 2] is clear? > > > > I'm afraid that question is beyond my pay grade, maybe some AMD or > > virt folks can chime in. > > That question aside, I'm not sure that this series is safe with respect to nested > virtualization. > > If the CPU has SRSO_NO, then KVM will report SRSO_NO in > KVM_GET_SUPPORTED_CPUID. However, in nested virtualization, the L1 guest > and the L2 guest share a prediction domain. KVM currently ensures isolation > between L1 and L2 with a call to > indirect_branch_prediction_barrier() in svm_vcpu_load(). I think that particular > barrier should *always* be a full IBPB--even if the host has SRSO_NO. I don't think that's true. If SRSO_NO=1, the indirect_branch_prediction_barrier() in svm_vcpu_load() I believe only needs to prevent indirect predictions from leaking from one VM to another, which is what SBPB provides. Keep in mind that before SRSO came out, IBPB on these parts was only flushing indirect predictions. SBPB become the 'legacy' IBPB functionality while IBPB turned into a full flush (on certain parts). If the CPU is immune to SRSO, you don't need the full flush. I also don't think promoting SRSO_USER_KERNEL_NO to SRSO_NO should ever be done. When SRSO_NO=1, it tells the OS that it can use SBPB on context switches because the only process->process BTB concern is with indirect predictions. The OS has to use IBPB if SRSO_NO=0 (regardless of SRSO_USER_KERNEL_NO) to prevent SRSO attacks from process->process. --David Kaplan
On Wed, Apr 9, 2025 at 11:29 AM Kaplan, David <David.Kaplan@amd.com> wrote: > > [AMD Official Use Only - AMD Internal Distribution Only] > > > -----Original Message----- > > From: Jim Mattson <jmattson@google.com> > > Sent: Wednesday, April 9, 2025 11:07 AM > > To: Josh Poimboeuf <jpoimboe@kernel.org> > > Cc: x86@kernel.org; linux-kernel@vger.kernel.org; amit@kernel.org; > > kvm@vger.kernel.org; Shah, Amit <Amit.Shah@amd.com>; Lendacky, Thomas > > <Thomas.Lendacky@amd.com>; bp@alien8.de; tglx@linutronix.de; > > peterz@infradead.org; pawan.kumar.gupta@linux.intel.com; corbet@lwn.net; > > mingo@redhat.com; dave.hansen@linux.intel.com; hpa@zytor.com; > > seanjc@google.com; pbonzini@redhat.com; daniel.sneddon@linux.intel.com; > > kai.huang@intel.com; Das1, Sandipan <Sandipan.Das@amd.com>; > > boris.ostrovsky@oracle.com; Moger, Babu <Babu.Moger@amd.com>; Kaplan, > > David <David.Kaplan@amd.com>; dwmw@amazon.co.uk; > > andrew.cooper3@citrix.com > > Subject: Re: [PATCH v3 2/6] x86/bugs: Use SBPB in __write_ibpb() if applicable > > > > Caution: This message originated from an External Source. Use proper caution > > when opening attachments, clicking links, or responding. > > > > > > On Wed, Apr 2, 2025 at 7:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > > > On Wed, Apr 02, 2025 at 02:04:04PM -0700, Jim Mattson wrote: > > > > On Wed, Apr 2, 2025 at 11:20 AM Josh Poimboeuf <jpoimboe@kernel.org> > > wrote: > > > > > > > > > > __write_ibpb() does IBPB, which (among other things) flushes > > > > > branch type predictions on AMD. If the CPU has SRSO_NO, or if the > > > > > SRSO mitigation has been disabled, branch type flushing isn't > > > > > needed, in which case the lighter-weight SBPB can be used. > > > > > > > > When nested SVM is not supported, should KVM "promote" > > > > SRSO_USER_KERNEL_NO on the host to SRSO_NO in > > KVM_GET_SUPPORTED_CPUID? > > > > Or is a Linux guest clever enough to do the promotion itself if > > > > CPUID.80000001H:ECX.SVM[bit 2] is clear? > > > > > > I'm afraid that question is beyond my pay grade, maybe some AMD or > > > virt folks can chime in. > > > > That question aside, I'm not sure that this series is safe with respect to nested > > virtualization. > > > > If the CPU has SRSO_NO, then KVM will report SRSO_NO in > > KVM_GET_SUPPORTED_CPUID. However, in nested virtualization, the L1 guest > > and the L2 guest share a prediction domain. KVM currently ensures isolation > > between L1 and L2 with a call to > > indirect_branch_prediction_barrier() in svm_vcpu_load(). I think that particular > > barrier should *always* be a full IBPB--even if the host has SRSO_NO. > > I don't think that's true. > > If SRSO_NO=1, the indirect_branch_prediction_barrier() in svm_vcpu_load() I believe only needs to prevent indirect predictions from leaking from one VM to another, which is what SBPB provides. Keep in mind that before SRSO came out, IBPB on these parts was only flushing indirect predictions. SBPB become the 'legacy' IBPB functionality while IBPB turned into a full flush (on certain parts). If the CPU is immune to SRSO, you don't need the full flush. > > I also don't think promoting SRSO_USER_KERNEL_NO to SRSO_NO should ever be done. When SRSO_NO=1, it tells the OS that it can use SBPB on context switches because the only process->process BTB concern is with indirect predictions. The OS has to use IBPB if SRSO_NO=0 (regardless of SRSO_USER_KERNEL_NO) to prevent SRSO attacks from process->process. Thanks, David! Conceptually, it sounds like SRSO_NO = (SRSO_USER_KERNEL_NO + SRSO_SAME_MODE_NO). You don't need an IBPB between L1 and L2 on SRSO_NO parts, because SRSO_SAME_MODE_NO is implied. Similarly, you can't "promote" SRSO_USER_KERNEL_NO to SRSO_NO, even absent SVM, because SRSO_SAME_MODE_NO is missing.
diff --git a/arch/x86/entry/entry.S b/arch/x86/entry/entry.S index 3a53319988b9..a5b421ec19c0 100644 --- a/arch/x86/entry/entry.S +++ b/arch/x86/entry/entry.S @@ -21,7 +21,7 @@ SYM_FUNC_START(__write_ibpb) ANNOTATE_NOENDBR movl $MSR_IA32_PRED_CMD, %ecx - movl $PRED_CMD_IBPB, %eax + movl _ASM_RIP(x86_pred_cmd), %eax xorl %edx, %edx wrmsr diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 310cb3f7139c..c8b8dc829046 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -58,7 +58,7 @@ EXPORT_SYMBOL_GPL(x86_spec_ctrl_base); DEFINE_PER_CPU(u64, x86_spec_ctrl_current); EXPORT_PER_CPU_SYMBOL_GPL(x86_spec_ctrl_current); -u64 x86_pred_cmd __ro_after_init = PRED_CMD_IBPB; +u32 x86_pred_cmd __ro_after_init = PRED_CMD_IBPB; EXPORT_SYMBOL_GPL(x86_pred_cmd); static u64 __ro_after_init x86_arch_cap_msr;
__write_ibpb() does IBPB, which (among other things) flushes branch type predictions on AMD. If the CPU has SRSO_NO, or if the SRSO mitigation has been disabled, branch type flushing isn't needed, in which case the lighter-weight SBPB can be used. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/x86/entry/entry.S | 2 +- arch/x86/kernel/cpu/bugs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)