On Mon, May 05, 2025, Pratik R. Sampat wrote:
Hi Sean,
On 5/2/25 4:50 PM, Sean Christopherson wrote:
On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
[...]
Applied 2-9 to kvm-x86 selftests. AIUI, the KVM side of things should already be fixed. If KVM isn't fixed, I want to take that discussion/patch to a separate thread.
Thanks for pulling these patches in.
For 1 - Ashish's commit now returns failure for this case [1]. Although, it appears that the return code isn't checked within sev_platform_init()[2], so it shouldn't change existing behavior. In the kselftest case, if platform init fails, the selftest will also fail — just as it does currently too.
Argh, now I remember the issue. But _sev_platform_init_locked() returns '0' if psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked() will magically make it succeed the second time around.
So shouldn't the KVM code be this?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index e0f446922a6e..dd04f979357d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void) sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
out: + if (sev_enabled) { + init_args.probe = true; + if (sev_platform_init(&init_args)) + sev_supported = sev_es_supported = sev_snp_supported = false; + else + sev_snp_supported &= sev_is_snp_initialized(); + } + if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" : @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
if (!sev_enabled) return; - - /* - * Do both SNP and SEV initialization at KVM module load. - */ - init_args.probe = true; - sev_platform_init(&init_args); }
void sev_hardware_unsetup(void) --
Ashish, what am I missing?
Regardless of what we decide on what the right behavior is, fail vs skip (I don't mind the former) we can certainly do that over new patches rebased over the new series.
FAIL, for sure. Unless someone else pipes up with a good reason why they need to defer INIT_EX, that's Google's problem to solve.