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

[Dave Hansen]

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