On 3/4/2025 3:58 PM, Sean Christopherson wrote: > 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? Yes, SEV INIT_EX is mainly used/required by Google. > > 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. So you are proposing that we drop all deferred initialization support for SEV, i.e, we drop the psp_init_on_probe module parameter for CCP driver, remove the probe field from sev_platform_init_args and correspondingly drop any support to skip/defer SEV INIT in _sev_platform_init_locked() and then also drop all existing support in KVM for SEV deferred initialization, i.e, remove the call to sev_platform_init() from __sev_guest_init(). Thanks, Ashish
