On 7/20/23 5:24 PM, Dave Hansen wrote:
On 7/20/23 12:11, Kim Phillips wrote:
Hopefully the commit text in this version will help answer all your
questions?:
To be honest, it didn't really. I kinda feel like I was having the APM
contents tossed casually in my direction rather than being provided a
fully considered explanation.
Sorry to hear that, it wasn't the intention.
Here's what I came up with instead:
Host-side Automatic IBRS has different behavior based on whether SEV-SNP
is enabled.
Without SEV-SNP, Automatic IBRS protects only the kernel. But when
SEV-SNP is enabled, the Automatic IBRS protection umbrella widens to all
host-side code, including userspace. This protection comes at a cost:
reduced userspace indirect branch performance.
To avoid this performance loss, nix using Automatic IBRS on SEV-SNP
hosts. Fall back to retpolines instead.
=====
Is that about right?
Sure, see new version below.
I don't think any chit-chat about the guest side is even relevant.
This also absolutely needs a comment. Perhaps just pull the code up to
the top level of the function and do this:
/*
* Automatic IBRS imposes unacceptable overhead on host
* userspace for SEV-SNP systems. Zap it instead.
*/
if (cpu_feature_enabled(X86_FEATURE_SEV_SNP))
setup_clear_cpu_cap(X86_FEATURE_AUTOIBRS);
Clearing X86_FEATURE_AUTOIBRS won't work because it'll unnecessarily
prohibit KVM from subsequently advertising the feature to guests.
I've tried to address the comment comment, see below.
BTW, I assume you've grumbled to folks about this. It's an awful shame
the hardware (or ucode) was built this was. It's just throwing
Automatic IBRS out the window because it's not architected in a nice way.
Is there any plan to improve this?
Sure. Until then:
From fb55df544ed066a3c8fdb1581932a23c03ce6d2c 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: Don't enable Automatic IBRS if SEV SNP is
enabled
Host-side Automatic IBRS has a different behaviour depending on whether
SEV-SNP is enabled.
Without SEV-SNP, Automatic IBRS protects only the kernel. But when
SEV-SNP is enabled, the Automatic IBRS protection umbrella widens to all
host-side code, including userspace. This protection comes at a cost:
reduced userspace indirect branch performance.
To avoid this performance loss, don't use Automatic IBRS on SEV-SNP
hosts. Fall back to retpolines instead.
Signed-off-by: Kim Phillips <kim.phillips@xxxxxxx>
---
arch/x86/kernel/cpu/common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8cd4126d8253..a93e6204d847 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1347,8 +1347,11 @@ 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.
+ * Don't use AutoIBRS when SNP is enabled because it degrades host
+ * userspace indirect branch performance.
*/
- 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
