linux-kernel - Re: [PATCH v5 6/7] KVM: SVM: Add support to initialize SEV/SNP functionality in KVM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8d3lDKfN1ffZbt5@google.com>
Date: Tue, 4 Mar 2025 13:58:44 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Ashish Kalra <ashish.kalra@....com>
Cc: pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	thomas.lendacky@....com, john.allen@....com, herbert@...dor.apana.org.au, 
	michael.roth@....com, dionnaglaze@...gle.com, nikunj@....com, ardb@...nel.org, 
	kevinloughlin@...gle.com, Neeraj.Upadhyay@....com, aik@....com, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-crypto@...r.kernel.org, linux-coco@...ts.linux.dev
Subject: Re: [PATCH v5 6/7] KVM: SVM: Add support to initialize SEV/SNP
 functionality in KVM

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.