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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a11ba4c9-8f6f-c231-c480-e2f25b8132b8@amd.com>
Date:   Thu, 20 Jul 2023 14:11:16 -0500
From:   Kim Phillips <kim.phillips@....com>
To:     Dave Hansen <dave.hansen@...el.com>,
        Michael Roth <michael.roth@....com>, <kvm@...r.kernel.org>
CC:     <linux-coco@...ts.linux.dev>, <linux-mm@...ck.org>,
        <linux-crypto@...r.kernel.org>, <x86@...nel.org>,
        <linux-kernel@...r.kernel.org>, <tglx@...utronix.de>,
        <mingo@...hat.com>, <jroedel@...e.de>, <thomas.lendacky@....com>,
        <hpa@...or.com>, <ardb@...nel.org>, <pbonzini@...hat.com>,
        <seanjc@...gle.com>, <vkuznets@...hat.com>, <jmattson@...gle.com>,
        <luto@...nel.org>, <dave.hansen@...ux.intel.com>, <slp@...hat.com>,
        <pgonda@...gle.com>, <peterz@...radead.org>,
        <srinivas.pandruvada@...ux.intel.com>, <rientjes@...gle.com>,
        <dovmurik@...ux.ibm.com>, <tobin@....com>, <bp@...en8.de>,
        <vbabka@...e.cz>, <kirill@...temov.name>, <ak@...ux.intel.com>,
        <tony.luck@...el.com>, <marcorr@...gle.com>,
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        <alpergun@...gle.com>, <dgilbert@...hat.com>, <jarkko@...nel.org>,
        <ashish.kalra@....com>, <nikunj.dadhania@....com>,
        <liam.merwick@...cle.com>, <zhi.a.wang@...el.com>
Subject: Re: [PATCH RFC v9 08/51] x86/speculation: Do not enable Automatic
 IBRS if SEV SNP is enabled

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