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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
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@xxxxxxx>
---
 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



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux