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;
> > + 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)
> > --
> >
>
> 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.
