On Mon, Mar 03, 2025, Ashish Kalra wrote: > On 3/3/2025 2:49 PM, Sean Christopherson wrote: > > On Mon, Mar 03, 2025, Ashish Kalra wrote: > >> On 2/28/2025 4:32 PM, Sean Christopherson wrote: > >>> On Fri, Feb 28, 2025, Ashish Kalra wrote: > >>>> And the other consideration is that runtime setup of especially SEV-ES VMs will not > >>>> work if/when first SEV-ES VM is launched, if SEV INIT has not been issued at > >>>> KVM setup time. > >>>> > >>>> This is because qemu has a check for SEV INIT to have been done (via SEV platform > >>>> status command) prior to launching SEV-ES VMs via KVM_SEV_INIT2 ioctl. > >>>> > >>>> So effectively, __sev_guest_init() does not get invoked in case of launching > >>>> SEV_ES VMs, if sev_platform_init() has not been done to issue SEV INIT in > >>>> sev_hardware_setup(). > >>>> > >>>> In other words the deferred initialization only works for SEV VMs and not SEV-ES VMs. > >>> > >>> In that case, I vote to kill off deferred initialization entirely, and commit to > >>> enabling all of SEV+ when KVM loads (which we should have done from day one). > >>> Assuming we can do that in a way that's compatible with the /dev/sev ioctls. > >> > >> Yes, that's what seems to be the right approach to enabling all SEV+ when KVM loads. > >> > >> For SEV firmware hotloading we will do implicit SEV Shutdown prior to DLFW_EX > >> and SEV (re)INIT after that to ensure that SEV is in UNINIT state before > >> DLFW_EX. > >> > >> We still probably want to keep the deferred initialization for SEV in > >> __sev_guest_init() by calling sev_platform_init() to support the SEV INIT_EX > >> case. > > > > Refresh me, how does INIT_EX fit into all of this? I.e. why does it need special > > casing? > > For SEV INIT_EX, we need the filesystem to be up and running as the user-supplied > SEV related persistent data is read from a regular file and provided to the > INIT_EX command. > > Now, with the modified SEV/SNP init flow, when SEV/SNP initialization is > performed during KVM module load, then as i believe the filesystem will be > mounted before KVM module loads, so SEV INIT_EX can be supported without > any issues. > > Therefore, we don't need deferred initialization support for SEV INIT_EX > in case of KVM being loaded as a module. > > But if KVM module is built-in, then filesystem will not be mounted when > SEV/SNP initialization is done during KVM initialization and in that case > SEV INIT_EX cannot be supported. > > Therefore to support SEV INIT_EX when KVM module is built-in, the following > will need to be done: > > - Boot kernel with psp_init_on_probe=false command line. > - This ensures that during KVM initialization, only SNP INIT is done. > - Later at runtime, when filesystem has already been mounted, > SEV VM launch will trigger deferred SEV (INIT_EX) initialization > (via the __sev_guest_init() -> sev_platform_init() code path). > > NOTE: psp_init_on_probe module parameter and deferred SEV initialization > during SEV VM launch (__sev_guest_init()->sev_platform_init()) was added > specifically to support SEV INIT_EX case. Ugh. That's quite the unworkable mess. sev_hardware_setup() can't determine if SEV/SEV-ES is fully supported without initializing the platform, but userspace needs KVM to do initialization so that SEV platform status reads out correctly. Aha! Isn't that a Google problem? And one that resolves itself if initialization is done on kvm-amd.ko load? A system/kernel _could_ be configured to use a path during initcalls, with the approproate initramfs magic. So there's no hard requirement that makes init_ex_path incompatible with CRYPTO_DEV_CCP_DD=y or CONFIG_KVM_AMD=y. Google's environment simply doesn't jump through those hoops. But Google _does_ build kvm-amd.ko as a module. So rather than carry a bunch of hard-to-follow code (and potentially impossible constraints), always do initialization at kvm-amd.ko load, and require the platform owner to ensure init_ex_path can be resolved when sev_hardware_setup() runs, i.e. when kvm-amd.ko is loaded or its initcall runs.
