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

["Min Xu" <min.m.xu@intel.com>]

On Friday, March 31, 2023 10:49 PM, Joeyli wrote:
> 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."
Hi, Joeyli
ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped and an error code is returned. So system will not hang.
So another solution is simply remove the ASSERT. Then an error message is dumped out and system continues.

@Gerd Hoffmann @Tom Lendacky @joeyli What's your thought?

Thanks
Min