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" ? > 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))