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


Min Xu
 

On June 4, 2021 12:12 AM, Laszlo wrote:
On 06/03/21 15:51, Yao, Jiewen wrote:
Hi, All
We plan to do a design review for TDVF in OVMF package.


The TDVF Design slides for TinaoCore Design Review Meeting (Jun 11) is
now available in blow link:
https://edk2.groups.io/g/devel/files/Designs/2021/0611.

The Bugzilla is https://bugzilla.tianocore.org/show_bug.cgi?id=3429



You can have an offline review first. You comments will be warmly
welcomed and we will continuously update the slides based on the
feedbacks.
Resending my earlier comments in this mailing list thread, with the feedback
inserted at the proper spots that has been given in the off-list thread since
then:
Continue my comments from here.

*** Slide 11 -- TDVF Image (1)

(9) CFV -- Configuration Firmware Volume (VarStore.fdf.inc), containing SB
keys -- how is this firmware volume populated (at build time)? Is this a
hexdump?
CFV is populated in post build. We can provide such python scripts to do the
SB keys enrollment.

... Back from slide 16: it seems like CFV is a raw hexdump indeed; how is that
managed when keys change (at build time)?
As I mentioned above, SB keys are enrolled in post build phase. We can provide
a python scripts to add/delete/append the keys.

(10) This slide (slide 11) basically describes an intrusive reorganization of
"OvmfPkgX64.fdf". I don't think I can agree to that.
While confidential computing is important, it is not relevant for many users.
Even if we don't cause outright regressions for existent setups, the
maintenance cost of the traditional OVMF platform will skyrocket.

The big bunch of areas that SEV-ES introduced to MEMFD is already a big
complication. I'd feel much better if we could isolate all that to a dedicated
"remote attested boot" firmware platform, and not risk the functionality and
maintenance of the traditional platform. I think this ties in with my comment
(1).
Actually in our first version of TDVF, it is a separated dsc/fdf. But when I try to
implement the *one binary*, I have to figure out some way to put our mailbox/tdhob.
I checked the OvmfPkgX64.fdf and mimics what SEV-ES does in MEMFD.
I would wait for a conclusion of the *one binary* and then figure out how to
handle the mailbox/tdhob.

For example, seeing a configuration firmware volume (CFV) with secure boot
keys embedded, in the "usual" FDF, will confuse lots of people, me included.
In the traditional OVMF use case, we use a different method:
namely OvmfPkg/EnrollDefaultKeys, for "priming" a variable store template,
in the build environment.
As I mentioned above, the SB keys are enrolled in post-build. The standard build
script:
build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t GCC5
Its output is a standard OVMF image (with a clean CFV/VarStore)
If the customers want the SB feature configured, it's up to them to enroll the SB
keys.
CFV is just a concept in TDVF. From the perspective of Standard OVMF, it is still
the VarStore.


Edk2 (and PI and UEFI) are definitely flexible enough for accommodating TDX,
but the existent, traditional OVMF platforms are a bad fit. In my opinion.


*** Slide 12: TDVF Image (2)

(11) "Page table should support both 4-level and 5-level page table"

As a general development strategy, I would suggest building TDX support in
small, well-isolated layers. 5-level paging is not enabled (has never been
tested, to my knowledge) with OVMF on QEMU/KVM, regardless of
confidential computing, for starters. If 5-level paging is a strict requirement
for TDX, then it arguably needs to be implemented independently of TDX, at
first. So that the common edk2 architecture be at least testable on
QEMU/KVM with 5-level paging enabled.
Yes, 5-level paging is a strict requirement for TDX.
I would wait for the conclusion of the *one binary*.

*** Slide 13:

(12) My comment is that the GUID-ed structure chain already starts at a fixed
GPA (the "AMD SEV option"). Ordering between GUID-ed structures is
irrelevant, so any TDX-specific structures should be eligible for appending to,
prepending to, or intermixing with, other (possibly
SEV-specific) structures. There need not be a separate entry point, just
different GUIDS.
Yes, we prefer a TDX GUID in ResetVector. In that GUID there is a offset which
points to the actual TDX Metadata blob.

(13) Regarding "4G-0x20[0x10] is OVMF AP reset vector (used in OVMF
implementation)" -- I think this is a typo: this "AP reset vector" is
*not* used in OVMF. To my knowledge, it is a vestige from the UefiCpuPkg
reset vector. In OVMF, APs are booted via MpInitLib (in multiple firmware
phases), using INIT-SIPI-SIPI, and the reset vector for the APs, posited
through those IPIs, is prepared in low RAM.
Thanks Laszlo for explanation.

*** Slides 14 through 16:

I consider these TDVF firmware image internals, implementation details
-- do whatever you need to do, just don't interfere with existing platforms /
use cases. See my comment (10) above.
Sure. All the TDVF changes will not interfere with existing platfomrs/use cases.

*** Slides 17-21:

(14) Again, a very big difference from traditional OVMF: APs never enter SEC
in traditional OVMF. I assume this new functionality is part of TdxStartupLib
(from slide 18) -- will there be a Null instance of that?
Yes, there is a NULL instance of TdxStartupLib.


Last week I posted a 43-part patch series to edk2-devel, for separating out
the dynamic Xen enlightenments from the IA32, IA32X64, X64 platforms, in
favor of the dedicated OvmfXen platform. TDX seems to bring in
incomparably more complications than Xen, and the OvmfPkg maintainers
have found even the Xen complications troublesome in the long term.

If I had had access to all this information when we first discussed "one
binary" on the mailing list, I'd have never agreed to "one binary". I'm OK with
attempting one firmware binary for "confidential computing", but that "one
platform" cannot be "OvmfPkgX64.dsc".

Even if I make a comparison with just the "technology" (not the remotely-
attested deployment) of SEV and SEV-ES, as it is included in
"OvmfPkgX64.dsc", TDX is hugely more complicated and intrusive than that.
SEV proved possible to integrate into existing modules, into the existing boot
flow, maybe through the addition of some new drivers (such as a new
IOMMU protocol implementation, and some "clever" depexes). But we never
had to restructure the FD layout, eliminate whole firmware phases, or think
about multiprocessing in the reset vector or the SEC phase.

In order to bring an example from the ARM world, please note that platforms
that use a PEI phase, and platforms that don't, are distinct platforms. In
ArmVirtPkg, two examples are ArmVirtQemu and ArmVirtQemuKernel. The
latter does not include the PEI Core.
Thanks Laszlo. I will check the example from the ARM world.


*** Slides 22 through 34:

(15) All these extra tasks and complications are perfectly fine, as long as they
exist peacefully *separately* from the traditional ("legacy") OVMF platforms.

Honestly, in the virtual world, picking your firmware binary is easy.
The approach here reminds me of a physical firmware binary that includes
everything possible from "edk2-platforms/Features/Intel", just so it can be
deployed to any physical board imaginable. That's not how Intel builds
physical firmware, right? We have "edk2-platforms/Platform/Intel"
and "edk2-platforms/Silicon/Intel" with many-many separate DSC files.
I will continue my comments in my next mail.

Thanks!
Min

Join devel@edk2.groups.io to automatically receive all group messages.