On 7/16/21 2:31 PM, Sean Christopherson wrote:

That's not what I was asking.  My question is if KVM will break/fail if someone
runs a KVM build with SNP enabled halfway through the series.  E.g. if I make a
KVM build at patch 22, "KVM: SVM: Add KVM_SNP_INIT command", what will happen if
I attempt to launch an SNP guest?  Obviously it won't fully succeed, but will KVM
fail gracefully and do all the proper cleanup?  Repeat the question for all patches
between this one and the final patch of the series.

SNP simply not working is ok, but if KVM explodes or does weird things without
"full" SNP support, then at minimum the module param should be off by default
until it's safe to enable.  E.g. for the TDP MMU, I believe the approach was to
put all the machinery in place but not actually let userspace flip on the module
param until the full implementation was ready.  Bisecting and testing the
individual commits is a bit painful because it requires modifying KVM code, but
on the plus side unrelated bisects won't stumble into a half-baked state.

There is one to two patches where I can think of that we may break the
KVM if SNP guest is created before applying the full series. In one
patch we add LAUNCH_UPDATE but reclaim is done in next patch. I like
your idea to push the module init  later in the series.

Ya, got that, but again not what I was asking :-)  Why use cpu_feature_enabled()
instead of boot_cpu_has()?  As a random developer, I would fully expect that
boot_cpu_has(X86_FEATURE_SEV_SNP) is true iff SNP is fully enabled by the kernel.

I have to check but I think boot_cpu_has(X64_FEATURE_SEV_SNP) will
return true even when the CONFIG_MEM_ENCRYPT is disabled.

quoted

The approach here is similar to SEV/ES. IIRC, it was done mainly to
avoid adding dead code when CONFIG_KVM_AMD_SEV is disabled.

But this is already in an #ifdef, checking sev_es_guest() is pointless.

Ah Good point.