linux-kernel - Re: [PATCH v10 04/50] x86/cpufeatures: Add SEV-SNP CPU feature

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]

Date:   Wed, 13 Dec 2023 18:35:35 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Borislav Petkov <bp@...en8.de>, Michael Roth <michael.roth@....com>
Cc:     kvm@...r.kernel.org, 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, 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, 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, jarkko@...nel.org, ashish.kalra@....com,
        nikunj.dadhania@....com, pankaj.gupta@....com,
        liam.merwick@...cle.com, zhi.a.wang@...el.com,
        Brijesh Singh <brijesh.singh@....com>,
        Jarkko Sakkinen <jarkko@...fian.com>
Subject: Re: [PATCH v10 04/50] x86/cpufeatures: Add SEV-SNP CPU feature

On 12/13/23 16:41, Borislav Petkov wrote:
> On Wed, Dec 13, 2023 at 03:18:17PM +0100, Paolo Bonzini wrote:
>> Surely we can agree that cpu_feature_enabled(X86_FEATURE_SEV_SNP) has nothing
>> to do with SEV-SNP host patches being present?
> 
> It does - we're sanitizing the meaning of a CPUID flag present in
> /proc/cpuinfo, see here:
> 
> https://git.kernel.org/tip/79c603ee43b2674fba0257803bab265147821955
>
>> And that therefore retpolines are preferred even without any SEV-SNP
>> support in KVM?
> 
> No, automatic IBRS should be disabled when SNP is enabled. Not CPUID
> present - enabled.

Ok, so the root cause of the problem is commit message/patch ordering:

1) patch 4 should have unconditionally cleared the feature (until the 
initialization code comes around in patch 6); and it should have 
mentioned in the commit message that we don't want X86_FEATURE_SEV_SNP 
to be set, unless SNP can be enabled via MSR_AMD64_SYSCFG.

2) possibly, the commit message of patch 5 could have said something 
like "at this point in the kernel SNP is never enabled".

3) Patch 23 should have been placed before the SNP initialization, 
because as things stand the patches (mildly) break bisectability.

> We clear that bit on a couple of occasions in the SNP
> host patchset if we determine that SNP host support is not possible so
> 4/50 needs to go together with the rest to mean something.

Understood now.  With the patch ordering and commit message edits I 
suggested above, indeed I would not have picked up patch 4.

But with your explanation, I would even say that "4/50 needs to go 
together with the rest" *for correctness*, not just to mean something.

Paolo