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 19:53:25 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Michael Roth <michael.roth@....com>, 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 Wed, Dec 13, 2023 at 06:35:35PM +0100, Paolo Bonzini wrote:
> 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.

I guess.

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

Sure.

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

Ok, I still haven't reached that one.

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

In the future, please simply refrain from picking up x86 patches if you
haven't gotten an explicit ACK.

We could have conflicting changes in tip, we could be reworking that
part of the code and the thing the patch touches could be completely
gone, and so on and so on...

Also, we want to have a full picture of what goes in. Exactly the same
as how you'd like to have a full picture of what goes into kvm and how
we don't apply kvm patches unless there's some extraordinary
circumstance or we have received an explicit ACK.

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

Yes, but for ease of integration it would be easier if they go in two
groups - kvm and x86 bits. Not: some x86 bits first, then kvm bits
through your tree and then some more x86 bits. That would be
a logistical nightmare.

And even if you bisect and land at 4/50 and you disable AIBRS even
without SNP being really enabled, that is not a big deal - you're only
bisecting and not really using that kernel and it's not like it breaks
builds so...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette