Re: [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest

["joeyli" <jlee@suse.com>]

On Fri, Mar 31, 2023 at 10:25:09AM +0200, Gerd Hoffmann wrote:
> On Fri, Mar 31, 2023 at 03:59:56PM +0800, joeyli wrote:
> > Hi Gerd,
> > 
> > On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote:
> > > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote:
> > > > From: Min M Xu <min.m.xu@intel.com>
> > > > 
> > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379
> > > > 
> > > > PlatformInitEmuVariableNvStore is called to initialize the
> > > > EmuVariableNvStore with the content pointed by
> > > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched
> > > > with -bios parameter, UEFI variables will be partially emulated, and
> > > > non-volatile variables may lose their contents after a reboot. This makes
> > > > the secure boot feature not working.
> > > > 
> > > > But in SEV guest, this design doesn't work. Because at this point the
> > > > variable store mapping is still private/encrypted, OVMF will see
> > > > ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in
> > > > SEV guest.
> > > 
> > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead.
> > > Without initializing the emu var store you will not get a functional
> > > secure boot setup anyway.
> > 
> > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a couple
> > of versions. Removing it will causes problem in VM live migration.
> 
> Hmm?  qemu live-migrates the rom image too.  Only after poweroff and
> reboot the guest will see an updated firmware image.
>

Thanks for your explanation. Understood.
 
> > I will prefer Min M's solution, until SEV experts found better
> > solution.
> 
> I'd prefer to not poke holes into secure boot.  Re-Initializing the emu
> var store from rom on each reset is also needed for security reasons in
> case the efi variable store is not in smm-protected flash memory.
>

I agree that the efi variable store is not secure without smm. But after
58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work with
SEV. System just hangs in "NvVarStore FV headers were invalid."

If secure boot can not work with SEV (even it is not really secure), why
not just block the building process when SEV with SECURE_BOOT_ENABLE? At
least the issue will not happen at runtime.

Thanks 
Joey Lee