[PATCH v4 0/3] Distinguish between variants of IBPB

[PATCH v4 0/3] Distinguish between variants of IBPB

[Jim Mattson]

Prior to Zen4, AMD's IBPB did not flush the RAS (or, in Intel
terminology, the RSB). Hence, the older version of AMD's IBPB was not
equivalent to Intel's IBPB. However, KVM has been treating them as
equivalent, synthesizing Intel's CPUID.(EAX=7,ECX=0):EDX[bit 26] on any
platform that supports the synthetic features X86_FEATURE_IBPB and
X86_FEATURE_IBRS.

Equivalence also requires a previously ignored feature on the AMD side,
CPUID Fn8000_0008_EBX[IBPB_RET], which is enumerated on Zen4.

v4: Added "guaranteed" to X86_FEATURE_IBPB comment [Pawan]
    Changed logic for deducing AMD IBPB features from Intel IBPB features
    in kvm_set_cpu_caps [Tom]
    Intel CPUs that suffer from PBRSB can't claim AMD_IBPB_RET [myself]

v3: Pass through IBPB_RET from hardware to userspace. [Tom]
    Derive AMD_IBPB from X86_FEATURE_SPEC_CTRL rather than
    X86_FEATURE_IBPB. [Tom]
    Clarify semantics of X86_FEATURE_IBPB.

v2: Use IBPB_RET to identify semantic equality. [Venkatesh]

Jim Mattson (3):
  x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET
  KVM: x86: Advertise AMD_IBPB_RET to userspace
  KVM: x86: AMD's IBPB is not equivalent to Intel's IBPB

 arch/x86/include/asm/cpufeatures.h |  3 ++-
 arch/x86/kvm/cpuid.c               | 12 +++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.46.0.662.g92d0881bb0-goog

[PATCH v4 1/3] x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET

[Jim Mattson]

AMD's initial implementation of IBPB did not clear the return address
predictor. Beginning with Zen4, AMD's IBPB *does* clear the return
address predictor. This behavior is enumerated by
CPUID.80000008H:EBX.IBPB_RET[bit 30].

Define X86_FEATURE_AMD_IBPB_RET for use in KVM_GET_SUPPORTED_CPUID,
when determining cross-vendor capabilities.

Suggested-by: Venkatesh Srinivas <venkateshs@chromium.org>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/cpufeatures.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index cabd6b58e8ec..a222a24677d7 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -215,7 +215,7 @@
 #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* Disable Speculative Store Bypass. */
 #define X86_FEATURE_LS_CFG_SSBD		( 7*32+24)  /* AMD SSBD implementation via LS_CFG MSR */
 #define X86_FEATURE_IBRS		( 7*32+25) /* "ibrs" Indirect Branch Restricted Speculation */
-#define X86_FEATURE_IBPB		( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier without RSB flush */
+#define X86_FEATURE_IBPB		( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier without a guaranteed RSB flush */
 #define X86_FEATURE_STIBP		( 7*32+27) /* "stibp" Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_ZEN			( 7*32+28) /* Generic flag for all Zen and newer */
 #define X86_FEATURE_L1TF_PTEINV		( 7*32+29) /* L1TF workaround PTE inversion */
@@ -348,6 +348,7 @@
 #define X86_FEATURE_CPPC		(13*32+27) /* "cppc" Collaborative Processor Performance Control */
 #define X86_FEATURE_AMD_PSFD            (13*32+28) /* Predictive Store Forwarding Disable */
 #define X86_FEATURE_BTC_NO		(13*32+29) /* Not vulnerable to Branch Type Confusion */
+#define X86_FEATURE_AMD_IBPB_RET	(13*32+30) /* IBPB clears return address predictor */
 #define X86_FEATURE_BRS			(13*32+31) /* "brs" Branch Sampling available */
 
 /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
-- 
2.46.0.662.g92d0881bb0-goog

Re: [PATCH v4 1/3] x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET

[Borislav Petkov]

On Fri, Sep 13, 2024 at 10:32:27AM -0700, Jim Mattson wrote:
> AMD's initial implementation of IBPB did not clear the return address
> predictor. Beginning with Zen4, AMD's IBPB *does* clear the return
> address predictor. This behavior is enumerated by
> CPUID.80000008H:EBX.IBPB_RET[bit 30].
> 
> Define X86_FEATURE_AMD_IBPB_RET for use in KVM_GET_SUPPORTED_CPUID,
> when determining cross-vendor capabilities.
> 
> Suggested-by: Venkatesh Srinivas <venkateshs@chromium.org>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index cabd6b58e8ec..a222a24677d7 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -215,7 +215,7 @@
>  #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* Disable Speculative Store Bypass. */
>  #define X86_FEATURE_LS_CFG_SSBD		( 7*32+24)  /* AMD SSBD implementation via LS_CFG MSR */
>  #define X86_FEATURE_IBRS		( 7*32+25) /* "ibrs" Indirect Branch Restricted Speculation */
> -#define X86_FEATURE_IBPB		( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier without RSB flush */

I see upstream

#define X86_FEATURE_IBPB		( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier */

Where does "without RSB flush" come from?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4 1/3] x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET

[Jim Mattson]

On Mon, Oct 7, 2024 at 7:30 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Sep 13, 2024 at 10:32:27AM -0700, Jim Mattson wrote:
> > AMD's initial implementation of IBPB did not clear the return address
> > predictor. Beginning with Zen4, AMD's IBPB *does* clear the return
> > address predictor. This behavior is enumerated by
> > CPUID.80000008H:EBX.IBPB_RET[bit 30].
> >
> > Define X86_FEATURE_AMD_IBPB_RET for use in KVM_GET_SUPPORTED_CPUID,
> > when determining cross-vendor capabilities.
> >
> > Suggested-by: Venkatesh Srinivas <venkateshs@chromium.org>
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > ---
> >  arch/x86/include/asm/cpufeatures.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index cabd6b58e8ec..a222a24677d7 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -215,7 +215,7 @@
> >  #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE        ( 7*32+23) /* Disable Speculative Store Bypass. */
> >  #define X86_FEATURE_LS_CFG_SSBD              ( 7*32+24)  /* AMD SSBD implementation via LS_CFG MSR */
> >  #define X86_FEATURE_IBRS             ( 7*32+25) /* "ibrs" Indirect Branch Restricted Speculation */
> > -#define X86_FEATURE_IBPB             ( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier without RSB flush */
>
> I see upstream
>
> #define X86_FEATURE_IBPB                ( 7*32+26) /* "ibpb" Indirect Branch Prediction Barrier */
>
> Where does "without RSB flush" come from?

Bad git hygiene. This should have been a 4 patch set, not a 3 patch
set. Sigh. I'll send out v5.

[PATCH v4 2/3] KVM: x86: Advertise AMD_IBPB_RET to userspace

[Jim Mattson]

This is an inherent feature of IA32_PRED_CMD[0], so it is trivially
virtualizable (as long as IA32_PRED_CMD[0] is virtualized).

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2617be544480..ec7b2ca3b4d3 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -751,7 +751,7 @@ void kvm_set_cpu_caps(void)
 		F(CLZERO) | F(XSAVEERPTR) |
 		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
 		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
-		F(AMD_PSFD)
+		F(AMD_PSFD) | F(AMD_IBPB_RET)
 	);
 
 	/*
-- 
2.46.0.662.g92d0881bb0-goog

[PATCH v4 3/3] KVM: x86: AMD's IBPB is not equivalent to Intel's IBPB

[Jim Mattson]

From Intel's documention [1], "CPUID.(EAX=07H,ECX=0):EDX[26]
enumerates support for indirect branch restricted speculation (IBRS)
and the indirect branch predictor barrier (IBPB)." Further, from [2],
"Software that executed before the IBPB command cannot control the
predicted targets of indirect branches (4) executed after the command
on the same logical processor," where footnote 4 reads, "Note that
indirect branches include near call indirect, near jump indirect and
near return instructions. Because it includes near returns, it follows
that **RSB entries created before an IBPB command cannot control the
predicted targets of returns executed after the command on the same
logical processor.**" [emphasis mine]

On the other hand, AMD's IBPB "may not prevent return branch
predictions from being specified by pre-IBPB branch targets" [3].

However, some AMD processors have an "enhanced IBPB" [terminology
mine] which does clear the return address predictor. This feature is
enumerated by CPUID.80000008:EDX.IBPB_RET[bit 30] [4].

Adjust the cross-vendor features enumerated by KVM_GET_SUPPORTED_CPUID
accordingly.

[1] https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/cpuid-enumeration-and-architectural-msrs.html
[2] https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/speculative-execution-side-channel-mitigations.html#Footnotes
[3] https://www.amd.com/en/resources/product-security/bulletin/amd-sb-1040.html
[4] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24594.pdf

Fixes: 0c54914d0c52 ("KVM: x86: use Intel speculation bugs and features as derived in generic x86 code")
Suggested-by: Venkatesh Srinivas <venkateshs@chromium.org>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/cpuid.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index ec7b2ca3b4d3..600d79ea22be 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -690,7 +690,9 @@ void kvm_set_cpu_caps(void)
 	kvm_cpu_cap_set(X86_FEATURE_TSC_ADJUST);
 	kvm_cpu_cap_set(X86_FEATURE_ARCH_CAPABILITIES);
 
-	if (boot_cpu_has(X86_FEATURE_IBPB) && boot_cpu_has(X86_FEATURE_IBRS))
+	if (boot_cpu_has(X86_FEATURE_AMD_IBPB_RET) &&
+	    boot_cpu_has(X86_FEATURE_AMD_IBPB) &&
+	    boot_cpu_has(X86_FEATURE_AMD_IBRS))
 		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL);
 	if (boot_cpu_has(X86_FEATURE_STIBP))
 		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP);
@@ -759,8 +761,12 @@ void kvm_set_cpu_caps(void)
 	 * arch/x86/kernel/cpu/bugs.c is kind enough to
 	 * record that in cpufeatures so use them.
 	 */
-	if (boot_cpu_has(X86_FEATURE_IBPB))
+	if (boot_cpu_has(X86_FEATURE_IBPB)) {
 		kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB);
+		if (boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
+		    !boot_cpu_has_bug(X86_BUG_EIBRS_PBRSB))
+			kvm_cpu_cap_set(X86_FEATURE_AMD_IBPB_RET);
+	}
 	if (boot_cpu_has(X86_FEATURE_IBRS))
 		kvm_cpu_cap_set(X86_FEATURE_AMD_IBRS);
 	if (boot_cpu_has(X86_FEATURE_STIBP))
-- 
2.46.0.662.g92d0881bb0-goog

Re: [PATCH v4 0/3] Distinguish between variants of IBPB

[Jim Mattson]

On Fri, Sep 13, 2024 at 10:32 AM Jim Mattson <jmattson@google.com> wrote:
>
> Prior to Zen4, AMD's IBPB did not flush the RAS (or, in Intel
> terminology, the RSB). Hence, the older version of AMD's IBPB was not
> equivalent to Intel's IBPB. However, KVM has been treating them as
> equivalent, synthesizing Intel's CPUID.(EAX=7,ECX=0):EDX[bit 26] on any
> platform that supports the synthetic features X86_FEATURE_IBPB and
> X86_FEATURE_IBRS.
>
> Equivalence also requires a previously ignored feature on the AMD side,
> CPUID Fn8000_0008_EBX[IBPB_RET], which is enumerated on Zen4.
>
> v4: Added "guaranteed" to X86_FEATURE_IBPB comment [Pawan]
>     Changed logic for deducing AMD IBPB features from Intel IBPB features
>     in kvm_set_cpu_caps [Tom]
>     Intel CPUs that suffer from PBRSB can't claim AMD_IBPB_RET [myself]
>
> v3: Pass through IBPB_RET from hardware to userspace. [Tom]
>     Derive AMD_IBPB from X86_FEATURE_SPEC_CTRL rather than
>     X86_FEATURE_IBPB. [Tom]
>     Clarify semantics of X86_FEATURE_IBPB.
>
> v2: Use IBPB_RET to identify semantic equality. [Venkatesh]
>
> Jim Mattson (3):
>   x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET
>   KVM: x86: Advertise AMD_IBPB_RET to userspace
>   KVM: x86: AMD's IBPB is not equivalent to Intel's IBPB

Oops. I forgot to include the v3 responses:

> For the series:
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

and

> Assuming this goes through the KVM tree:
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

The only substantive change was to patch 3/3.

Sean: Are you willing to take this through KVM/x86?

Re: [PATCH v4 0/3] Distinguish between variants of IBPB

[Sean Christopherson]

On Fri, Sep 27, 2024, Jim Mattson wrote:
> On Fri, Sep 13, 2024 at 10:32 AM Jim Mattson <jmattson@google.com> wrote:
> >
> > Prior to Zen4, AMD's IBPB did not flush the RAS (or, in Intel
> > terminology, the RSB). Hence, the older version of AMD's IBPB was not
> > equivalent to Intel's IBPB. However, KVM has been treating them as
> > equivalent, synthesizing Intel's CPUID.(EAX=7,ECX=0):EDX[bit 26] on any
> > platform that supports the synthetic features X86_FEATURE_IBPB and
> > X86_FEATURE_IBRS.
> >
> > Equivalence also requires a previously ignored feature on the AMD side,
> > CPUID Fn8000_0008_EBX[IBPB_RET], which is enumerated on Zen4.
> >
> > v4: Added "guaranteed" to X86_FEATURE_IBPB comment [Pawan]
> >     Changed logic for deducing AMD IBPB features from Intel IBPB features
> >     in kvm_set_cpu_caps [Tom]
> >     Intel CPUs that suffer from PBRSB can't claim AMD_IBPB_RET [myself]
> >
> > v3: Pass through IBPB_RET from hardware to userspace. [Tom]
> >     Derive AMD_IBPB from X86_FEATURE_SPEC_CTRL rather than
> >     X86_FEATURE_IBPB. [Tom]
> >     Clarify semantics of X86_FEATURE_IBPB.
> >
> > v2: Use IBPB_RET to identify semantic equality. [Venkatesh]
> >
> > Jim Mattson (3):
> >   x86/cpufeatures: Define X86_FEATURE_AMD_IBPB_RET
> >   KVM: x86: Advertise AMD_IBPB_RET to userspace
> >   KVM: x86: AMD's IBPB is not equivalent to Intel's IBPB
> 
> Oops. I forgot to include the v3 responses:
> 
> > For the series:
> >
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
> and
> 
> > Assuming this goes through the KVM tree:
> >
> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> 
> The only substantive change was to patch 3/3.
> 
> Sean: Are you willing to take this through KVM/x86?

Yep, and I can fixup the reviews when applying.