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
