On Mon, May 05, 2025, Pratik R. Sampat wrote:
On 5/5/2025 6:15 PM, Sean Christopherson wrote:
On Mon, May 05, 2025, Pratik R. Sampat wrote: 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;elsesev_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) --
I agree with this approach. One thing maybe to consider further is to also call into SEV_platform_status() to check for init so that SEV/SEV-ES is not penalized and disabled for SNP's failures. Another approach could be to break up the SEV and SNP init setup so that we can spare a couple of platform calls in the process?
Nah, SNP initialization failure should be a rare occurence, I don't want to make the "normal" flow more complex just to handle something that should never happen in practice.