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

[Paolo Bonzini]

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