Re: [PATCH RFC v9 08/51] x86/speculation: Do not enable Automatic IBRS if SEV SNP is enabled

[Kim Phillips <kim.phillips@amd.com>]

On 7/18/23 6:17 PM, Dave Hansen wrote:
> On 7/18/23 15:34, Kim Phillips wrote:
> ...
>> Automatic IBRS provides protection to [1]:
>>
>>   - Processes running at CPL=0
>>   - Processes running as host when Secure Nested Paging (SEV-SNP) is enabled
>>
>> i.e.,
>>
>>      (CPL < 3) || ((ASID == 0) && SNP)
>>
>> Because of this limitation, do not enable Automatic IBRS when SNP is
>> enabled.
> 
> Gah, I found that hard to parse.  I think it's because you're talking
> about an SEV-SNP host in one part and "SNP" in the other but _meaning_
> SNP host and SNP guest.
> 
> Could I maybe suggest that you folks follow the TDX convention and
> actually add _GUEST and _HOST to the feature name be explicit about
> which side is which?
> 
>> Instead, fall back to retpolines.
> 
> Now I'm totally lost.
> 
> This is talking about falling back to retpolines ... in the kernel.  But
> "Automatic IBRS provides protection to ... CPL < 3", aka. the kernel.
> 
>> Note that the AutoIBRS feature may continue to be used within the
>> guest.
> 
> What is this trying to say?
> 
> "AutoIBRS can still be used in a guest since it protects CPL < 3"
> 
> or
> 
> "The AutoIBRS bits can still be twiddled within the guest even though it
> doesn't do any good"
> 
> ?

Hopefully the commit text in this version will help answer all your
questions?:

 From 96dbd72d018287bc5b72f6083884e2125c9d09bc Mon Sep 17 00:00:00 2001
From: Kim Phillips <kim.phillips@amd.com>
Date: Mon, 17 Jul 2023 14:08:15 -0500
Subject: [PATCH] x86/speculation: Do not enable Automatic IBRS if SEV SNP is
  enabled

Automatic IBRS provides protection to [1]:

  - Processes running at CPL=0
  - Processes running as host when Secure Nested Paging (SEV-SNP) is enabled

I.e., from the host side (ASID=0, based on host EFER.AutoIBRS)
If SYSCFG[SNPEn]=0 then:
      IBRS is enabled for supervisor mode (CPL < 3) only

If SYSCFG[SNPEn]=1 then:
      IBRS is enabled at all CPLs

 From the guest side (ASID!=0, based on guest EFER.AutoIBRS)
      IBRS is enabled for supervisor mode (CPL < 3)

Therefore, don't enable Automatic IBRS in host mode if SNP is enabled,
because it will penalize user-mode indirect branch performance.  Have
the kernel fall back to retpolines instead.

Note that the AutoIBRS feature may continue to be used within guests,
where ASID != 0.

[1] "AMD64 Architecture Programmer's Manual Volume 2: System Programming",
     Pub. 24593, rev. 3.41, June 2023, Part 1, Section 3.1.7 "Extended
     Feature Enable Register (EFER)" - accessible via Link.

Link: https://bugzilla.kernel.org/attachment.cgi?id=304652
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
  arch/x86/kernel/cpu/common.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8cd4126d8253..311c0a6422b5 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1348,7 +1348,8 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
  	 * AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
  	 * flag and protect from vendor-specific bugs via the whitelist.
  	 */
-	if ((ia32_cap & ARCH_CAP_IBRS_ALL) || cpu_has(c, X86_FEATURE_AUTOIBRS)) {
+	if ((ia32_cap & ARCH_CAP_IBRS_ALL) || (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
+	    !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
  		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
  		if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
  		    !(ia32_cap & ARCH_CAP_PBRSB_NO))
-- 
2.34.1