public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Min Xu" <min.m.xu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	"rfc@edk2.groups.io" <rfc@edk2.groups.io>
Cc: "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"erdemaktas@google.com" <erdemaktas@google.com>,
	"cho@microsoft.com" <cho@microsoft.com>,
	"bret.barkelew@microsoft.com" <bret.barkelew@microsoft.com>,
	Jon Lange <jlange@microsoft.com>, Karen Noel <knoel@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Nathaniel McCallum <npmccallum@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Ademar de Souza Reis Jr. <areis@redhat.com>
Subject: Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF
Date: Tue, 8 Jun 2021 12:27:27 +0000	[thread overview]
Message-ID: <PH0PR11MB50649131AF6826AC8823D731C5379@PH0PR11MB5064.namprd11.prod.outlook.com> (raw)
In-Reply-To: <e23a6f0e-2d2a-7471-9696-6996f664fd4d@redhat.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

  parent reply	other threads:[~2021-06-08 12:27 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03 13:51 [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF Yao, Jiewen
2021-06-03 16:11 ` Laszlo Ersek
2021-06-03 23:19   ` Yao, Jiewen
2021-06-04 10:11     ` Laszlo Ersek
2021-06-04 10:24       ` Yao, Jiewen
2021-06-04 10:43       ` Michael Brown
2021-06-04 14:52         ` Michael Brown
2021-06-04 15:04           ` James Bottomley
2021-06-04  7:33   ` Min Xu
2021-06-06  2:03   ` Min Xu
2021-06-06 11:29     ` Michael Brown
2021-06-06 12:49       ` Min Xu
2021-06-07 13:52         ` Laszlo Ersek
2021-06-06  8:52   ` Min Xu
2021-06-06 11:39     ` Michael Brown
2021-06-08 12:27   ` Min Xu [this message]
2021-06-08 15:36     ` Laszlo Ersek
2021-06-08 16:01 ` James Bottomley
2021-06-08 19:33   ` Laszlo Ersek
2021-06-09  0:58     ` Min Xu
2021-06-09 11:00       ` Laszlo Ersek
2021-06-09 14:36         ` James Bottomley
2021-06-09  2:01   ` Min Xu
2021-06-09 14:28     ` James Bottomley
2021-06-09 15:47       ` Paolo Bonzini
2021-06-09 15:59         ` James Bottomley
2021-06-10 21:01           ` Erdem Aktas
2021-06-10 22:30 ` Min Xu
2021-06-11  1:33   ` James Bottomley
2021-06-11  1:36     ` Yao, Jiewen
2021-06-11  1:38       ` James Bottomley
2021-06-11  1:55         ` James Bottomley
     [not found] ` <168759329436FBCF.5845@groups.io>
2021-06-11  6:37   ` Min Xu
2021-06-22 13:34     ` Laszlo Ersek
2021-06-22 13:38       ` Laszlo Ersek
2021-06-24  0:24         ` Min Xu
2021-06-24  0:35           ` James Bottomley
2021-06-24  0:55             ` Min Xu
     [not found]             ` <168B5EA81BA66FAC.7570@groups.io>
2021-07-01  5:00               ` Min Xu
2021-06-23  2:44       ` Min Xu
2021-06-23 17:47         ` Laszlo Ersek
2021-06-23 11:56       ` Min Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR11MB50649131AF6826AC8823D731C5379@PH0PR11MB5064.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox