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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76105): https://edk2.groups.io/g/devel/message/76105 Mute This Topic: https://groups.io/mt/83283616/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-