Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF

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

On 06/04/2021 12:12 AM, Laszlo wrote:

> (22) EmuVariableFvbRuntimeDxe
> 
> Ouch, this is an unpleasant surprise.
> 
> First, if you know for a fact that pflash is not part of the *board* in any TDX
> setup, then pulling
> 
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> 
> into the firmware platform is useless, as it is mutually exclusive with
> 
>   OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
> 
> (via dynamic means -- a dynamic PCD).
> 
> Note that the FDF file places QemuFlashFvbServicesRuntimeDxe in APRIORI
> DXE when SMM_REQUIRE is FALSE. This driver checks for pflash presence,
> and lets EmuVariableFvbRuntimeDxe perform its in-RAM flash emulation
> only in case pflash is not found.
> 
Right, pflash is not part of the *board* in any TDX setup.
This is a limitation from the Qemu (the tdx-qemu) side. TDVF is copied into
RAM. Generic loader is the one for it.
In this case (pflash is not found), FvbServiceRuntimeDxe.inf will return
EFI_WRITE_PROTECTED in its FvbInitialize().
EmuVariableFvbRuntimeDxe will then perform its in-RAM flash emulation.

> So this is again in favor of a separate platform -- if we know pflash is never
> part of the board, then QemuFlashFvbServicesRuntimeDxe is never needed,
> but you cannot remove it from the traditional DSC/FDF files.
> 
Yes, if TDVF is a separate DSD/FDF, QemuFlashFvbServicesRuntimeDxe will be
removed from the image.

> Second, EmuVariableFvbRuntimeDxe consumes the PlatformFvbLib class, for
> the PlatformFvbDataWritten() API (among other things). This lib class is
> implemented by two instances in OvmfPkg, PlatformFvbLibNull and
> EmuVariableFvbLib. The latter instance allows Platform BDS to hook an event
> (for signaling) via "PcdEmuVariableEvent" into the
> EmuVariableFvbRuntimeDxe driver.
> 
> In old (very old) QEMU board configurations, namely those without pflash,
> this (mis)feature is used by OVMF's PlatformBootManagerLib to write out all
> variables to the EFI system partition in a regular file called \NvVars, with the
> help of NvVarsFileLib, whenever EmuVariableFvbRuntimeDxe writes out an
> emulated "flash" block. For this purpose, the traditional OVMF DSC files link
> EmuVariableFvbLib into EmuVariableFvbRuntimeDxe.
> 
Thanks for the explanation. It's very helpful for me to understand the code.

> But it counts as an absolute disaster nowadays, and should not be revived in
> any platform. If you don't have pflash in TDX guests, just accept that you
> won't have non-volatile variables. And link PlatformFvbLibNull into
> EmuVariableFvbRuntimeDxe. You're going to need a separate
> PlatformBootManagerLib instance anyway.
>
I have a question here, that if PlatformFvbLibNull is linked into EmuVaiableFvRuntimeDxe,
Does it mean it cannot write to the in-RAM variable store?

> 
> (We should have removed EmuVariableFvbRuntimeDxe a long time ago from
> the traditional OVMF platforms, i.e. made pflash a hard requirement, even
> when SMM is not built into the platform -- but whenever I tried that, Jordan
> always shot me down.)
>
I am afraid in TDVF we have to use EmuVariableFvRuntimeDxe to emulate the
in-RAM, as I explained pflash is not part of the *board* in TDX setup.
In the traditional EmuVariableFvRuntimeDxe, the FV contents will be initialized.
But TDVF has its own requirement, that the SB keys in CFV need to be copied
into the FV contents. That's why EmuVariableFvRuntimeDxe is updated in
TDVF project.

> 
> My point is: using EmuVariableFvbRuntimeDxe in the TDX platform may be
> defensible per se, but we must be very clear that it will never provide a
> standards-conformant service for non-volatile UEFI variables, and we must
> keep as much of the \NvVars mess out of EmuVariableFvbRuntimeDxe as
> possible. This will require a separate PlatformBootManagerLib instance for
> TDX anyway (or maybe share PlatformBootManagerLibGrub with
> "OvmfPkg/AmdSev/AmdSevX64.dsc").
> 
> Apart from the volatility aspect, let's assume we have this in-RAM emulated
> "flash device", storing authenticated UEFI variables for Secure Boot purposes.
> And we don't have SMM.
> 
> What protects this in-RAM variable store from tampering by the guest OS?
> It's all guest RAM only, after all. What provides the privilege barrier between
> the guest firmware and the guest OS?
A good question and we will answer it a bit later. Thanks for your patience.

> Thanks
> Laszlo