Hi Laszlo. To clarify your "one binary" feedback below, do you mean you suggest A) create a separate DSC (for example OvmfPkg/ConfidentialComputing.dsc) for a full solution including AMD SEC + Intel TDX + NonConfidentialComputing? Or B) to create a standalone DSC for Intel TDX (for example, create a OvmfPkg/IntelTdx/IntelTdxXS64.dsc) ?
To me, A) does not change too much, we just create another DSC file - that is it. Then the original OvmfPkgX64.dsc will only be used for POC/Testing purpose. It does not provide any security guarantee. (The threat model is: we don't trust VMM. Without attestation, you don't know if VMM modified the OVMF.) I don't know how "simply" it means. To enable TDX to make it work is not a simple work. Some architecture changes are mandatory, such as reset vector, IO/MMIO access, #VE handler, IOMMU based shared memory access, etc. I think AMD SEV already did those. =================== My point is that the "one binary" that the slide deck refers to (i.e., OvmfPkgX64.dsc) might prove OK for utilizing the Intel TDX *technology* in itself. Simply enabling OVMF + a guest OS to boot in a TDX domain. But "OvmfPkgX64.dsc" is *not* the "one binary" that is suitable for remote attestation, which has a much broader scope, involving multiple computers, networking, deployment, guest-owner/host-owner separation, whatever. For the latter, we needed a separate platform anyway, even with only SEV in mind; that's why "OvmfPkg/AmdSev/AmdSevX64.dsc" exists. =================== Thank you Yao Jiewen > -----Original Message----- > From: r...@edk2.groups.io <r...@edk2.groups.io> On Behalf Of Laszlo Ersek > Sent: Friday, June 4, 2021 12:12 AM > To: Yao, Jiewen <jiewen....@intel.com>; r...@edk2.groups.io; > devel@edk2.groups.io > Cc: j...@linux.ibm.com; Brijesh Singh <brijesh.si...@amd.com>; Tom > Lendacky <thomas.lenda...@amd.com>; erdemak...@google.com; > c...@microsoft.com; bret.barke...@microsoft.com; Jon Lange > <jla...@microsoft.com>; Karen Noel <kn...@redhat.com>; Paolo Bonzini > <pbonz...@redhat.com>; Nathaniel McCallum <npmccal...@redhat.com>; > Dr. David Alan Gilbert <dgilb...@redhat.com>; Ademar de Souza Reis Jr. > <ar...@redhat.com> > Subject: Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF > > 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: > > > *** Slides 4, 6, 7: the "one binary requirement". > > (1) The presentation refers to "OvmfPkgX64.dsc" as "the one" on slide#4, > but then the explanation for the requirement, given on slide 7, speaks > about "common attestation interface". > > I think we have a misunderstanding here. The "OvmfPkgX64.dsc" platform > indeed contains SEV, SEV-ES, and (in the future, will contain) SEV-SNP > support. In that sense, adding TDX support to the same platform should > be (hopefully) possible, at the cost of ugly gymnastics in the reset > vector. > > But "OvmfPkgX64.dsc" is *already* different from the remotely attested > OVMF platform, namely "OvmfPkg/AmdSev/AmdSevX64.dsc". > > The latter has some additional modules (secret PEIM and DXE driver), it > has the Grub binary built in, and -- probably most importantly -- it > trusts host-originated information less than "OvmfPkgX64.dsc". > > For example, "OvmfPkg/AmdSev/AmdSevX64.dsc" has a dedicated > PlatformBootManagerLib instance, one that ignores the non-volatile UEFI > variables Boot#### and BootOrder, and ignores (thus far) the fw_cfg > originated kernel/initrd/cmdline as well. > > It remains an "area of research" to see what else should be removed from > the traditional host-guest integration (which integration is usually > desirable for management and convenience), in the remotely-attested boot > scenario. See e.g. > <https://bugzilla.tianocore.org/show_bug.cgi?id=3180>. > > My point is that the "one binary" that the slide deck refers to (i.e., > OvmfPkgX64.dsc) might prove OK for utilizing the Intel TDX *technology* > in itself. Simply enabling OVMF + a guest OS to boot in a TDX domain. > > But "OvmfPkgX64.dsc" is *not* the "one binary" that is suitable for > remote attestation, which has a much broader scope, involving multiple > computers, networking, deployment, guest-owner/host-owner separation, > whatever. For the latter, we needed a separate platform anyway, even > with only SEV in mind; that's why "OvmfPkg/AmdSev/AmdSevX64.dsc" > exists. > > > *** Slides 8-9: general boot flow -- TDVF; TDVF Flow > > I'm likely missing a big bunch of background here, so just a few > questions: > > (2) Not sure what RTMR is, but it's associated with "Enable TrustedBoot" > -- so is a virtual TPM a hard requirement? > > ... Back from slide 10: "TCG measurement and event log framework w/o > TPM" -- that's curious. > > [response from Dave Gilbert:] > > My reading of this is that the RTMR (and another set of similar > > registers) are a TDX thing that is like the PCRs from a TPM but > > without the rest of the TPM features; so you can do the one-way > > measurement into the RTMRs just like you do into a TPM PCR, and the > > measurements pop out somewhere in the TDX quote. Just like a TPM you > > need the event log to make any sense of how the final hashed value > > supposedly got to where it did. > > [response from Erdem Aktas:] > > +1 to David on this. TDX provides 2 kinds of measurement registers: > > MRTDs and RTMRs > > > (https://software.intel.com/content/dam/develop/external/us/en/docume > nts/tdx-module-1eas-v0.85.039.pdf > > section 10.1.2) . MRTDs are build-time measurement registers which are > > updated when TDX is being created. Once TDX is finalized (before the > > first run), the MRTDs are finalized and cannot be updated anymore. On > > the other hand, while the TD is running, TD can extend RTMRs through > > TDCALLs which will provide TPM PCR kind of capabilities. > > ... Back from slide 43: feel free to skip this now; I will comment in > more detail below. > > (3) Prepare AcpiTable -- OVMF fetches ACPI tables from QEMU; is this a > new (firmware originated) table that is installed in addition, or does > it replace QEMU's tables? > > ... Ignore for now, will comment on the MADT stuff later. > > (4) Does DMA management mean a different IOMMU protocol? That is > going > to conflict with the SEV IOMMU protocol. Depexes in OVMF expect one or > zero IOMMU protocols to be present. > > ... Back from slide 40: feel free to skip this now; I'll comment on this > separately, below. > > (5) Enumerate VirtIO -- virtio enumeration is PCI based on x86. But I > see no mention of PCI. If you mean VirtioMmioDeviceLib, that's no good, > because it does not support virtio-1.0, only virtio-0.9.5. > > ... Back from slide 42: I got my answer to this on slide 42, so don't > worry about this point. > > (6) The PEI phase is skipped as a whole. I don't see how that can be > reasonably brought together with either "OvmfPkgX64.dsc" or > "OvmfPkg/AmdSev/AmdSevX64.dsc". I guess you can always modify SEC to > jump into DXE directly, but then why keep the PEI core and a bunch of > PEIMs in the firmware binary? > > Also touched on by slide 9, TDVF Flow -- PEI is eliminated but SEC > becomes more heavy-weight. > > Wouldn't this deserve a dedicated, separate platform DSC? The > 8-bit/32-bit branching at the front of the reset vector is a smaller > complication in comparison. > > Slide 6 references the mailing list archive: > > https://edk2.groups.io/g/devel/topic/81969494#74319 > > and in that message, I wrote: > > I'm doubtful that this is a unique problem ("just fix the reset > vector") the likes of which will supposedly never return during the > integration of SEV and TDX > > See also: > > https://listman.redhat.com/archives/edk2-devel-archive/2021- > April/msg00784.html > > where I said: > > It's not lost on me that we're talking about ~3 instructions. > > Let's keep a close eye on further "polymorphisms" that would require > hacks. > > The first 9 slides in the presentation introduce much-much more > intrusive problems than the "polymorphism" of the reset vector. Would I > be correct to say that my concern in the above messages was right? I > think I was only given a fraction of the necessary information when I > was asked "do you agree 'one binary' is viable". > > [response from Erdem Aktas:] > > Let's not worry about this for now. We want the one binary solution > > for practical reasons and also for simplicity. In the end, we want to > > do what is right and good for everyone. > > Those are legit concerns and I think what Intel is trying to do (sorry > > for mind reading) is to discuss all those concerns and questions to > > make the right decision. I really appreciate their effort on preparing > > those slides and bringing it to the community to review. > > > > I will also read your comments more carefully and provide my thoughts > > on them. > > Sorry for being a little slow on this. > > > *** Slide 10 -- Key impact to firmware > > (7) "CPUs are left running in long mode after exiting firmware" -- what > kind of "pen" are we talking about? Does a HLT loop still work? > > (8) "I/O, MMIO, MSR accesses are different" -- those are implemented by > low-level libraries in edk2; how can they be configured dynamically? > > ... Back from slide 53: I'll comment on slide 53 separately; ignore > this. > > > *** 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? > > ... Back from slide 16: it seems like CFV is a raw hexdump indeed; how > is that managed when keys change (at build time)? > > (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). > > 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. > > 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. > > > *** 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. > > (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. > > > *** 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. > > > *** 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? > > 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. > > > *** 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. > > > *** Slide 35-36: DXE phase > > (16) "Some DXE Drivers not allowed to load/start in Td guest -- Network > stack, RNG, ..." > > Same comment as (several times) above. The Linuxboot project is a good > example for eliminating cruft from DXEFV (in fact, for eliminating most > of the DXE phase). In a TDX environment, why include drivers in the > firmware binary that are never used? Meanwhile, DXEFV in OVMF grows by > a > MB every 1.5 years or so. Again, remove these drivers from the DSC/FDF > then, and it needs to be a separate platform. > > (17) "Other DXE Phase drivers -- [...] AcpiPlatformDxe" > > I'm not sure what this section is supposed to mean. Other DXE phase > drivers included, or excluded? Without AcpiPlatformDxe, the guest OS > will not see QEMU's ACPI content, and will almost certainly malfunction. > > ... Back from slide 48: ignore this for now, I'll comment in more detail > later. > > > *** Slide 37: DXE Core > > (18) says "SMM is not supported in Td guest" -- how is the variable > store protected from direct hardware (pflash) access from the guest OS? > Without SMM, the guest OS need not go through gRT->SetVariable() to > update authenticated non-volatile UEFI variables, and that undermines > Secure Boot. > > Note that, while SEV-ES has the same limitation wrt. SMM, the > "OvmfPkg/AmdSev/AmdSevX64.dsc" platform doesn't even include the > Secure > Boot firmware feature. For another example, the OVMF firmware binary in > RHEL that's going to support SEV-ES is such a build of "OvmfPkgX64.dsc" > that does not include the Secure Boot feature either. > > But in this TDX slide deck, Secure Boot certificates are embedded in the > CFV (configuration firmware volume) -- see slide 11 and slide 16 --, > which suggests that this platform does want secure boot. > > ... Back from slide 48: I'm going to make *additional* comments on this, > when I'm at slide 48, too. > > The rest of this slide (slide 37) looks reasonable (generic DXE Core > changes -- possibly PI spec changes too). > > > *** Slides 38 through 39: > > These seem reasonable (TdxDxe assumes some responsibilities of > OvmfPkg/PlatformPei) > > > *** Slides 40 through 42: > > *If* you really can implement TDX support for the IOMMU driver *this* > surgically, then I'm OK with it. The general logic in the IOMMU driver > was truly difficult to write, and I'd be seriously concerned if those > parts would have to be modified. Customizing just the page > encryption/decryption primitives for TDX vs. SEV is OK. > > > *** Slides 43 through 47: > > (19) Slide 46 and slide 47 are almost identical. Please consolidate them > into a single slide. > > (20) the TPM2 infrastructure in edk2 is baroque (over-engineered), in my > opinion. It has so many layers that I can never keep them in mind. When > we added TPM support to OVMF, I required commit messages that would > help > us recall the layering. In particular, please refer to commit > 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module", 2018-03-09). Here's an > excerpt: > > TPM 2 consumer driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance > | > v > TCG2 protocol interface > | > v > TCG2 protocol provider: Tcg2Dxe.inf driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibRouter instance > | > v > NULL class: Tpm2InstanceLibDTpm instance > (via earlier registration) > | > v > TPM2 chip (actual hardware) > > The slide deck says that EFI_TD_PROTOCOL is supposed to reuse the > EFI_TCG2_PROTOCOL definition. If that's the case, then why don't we push > the TDX specifics (more or less: the replacement of PCRs with RTMR) down > to the lowest possible level? > > Can we have "Tpm2InstanceLibTdxRtmr", plugged into the same Tcg2Dxe.inf > driver? > > If not, can we have a new TdTcg2Dxe.inf driver, but make it so that it > install the same protocol as before (EFI_TCG2_PROTOCOL -- same old > protocol GUID)? Then DxeTpmMeasurementLib doesn't have to change. > > As long as there is *at most* one EFI_TCG2_PROTOCOL instance published > in the protocol database, DxeTpmMeasurementLib should be fine. In SEV* > guests, the standard Tcg2Dxe driver provides that protocol. In TDX > guests, TdTcg2Dxe.inf should provide the protocol. Arbitration between > the two can be implemented with the pattern seen in the following > commits: > > 1 05db0948cc60 EmbeddedPkg: introduce EDKII Platform Has ACPI GUID > 2 786f476323a6 EmbeddedPkg: introduce PlatformHasAcpiLib > 3 65a69b214840 EmbeddedPkg: introduce EDKII Platform Has Device Tree > GUID > 4 2558bfe3e907 ArmVirtPkg: add PlatformHasAcpiDtDxe > > The basic idea is that Tcg2Dxe can have a depex on a new "running in > SEV" protocol GUID, and TdTcg2Dxe can have a depex on a new "running in > TDX" protocol GUID. A separate platform driver can install the proper > GUID -- possibly *neither* of those GUIDs. > > And, we don't have to change the depex section of > "SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf" for this; we can implement a > library instance with an empty constructor, but a non-empty depex, and > then hook this lib instance into Tcg2Dxe.inf via module scope NULL lib > class override in the DSC file. Basically we could forcibly restrict > Tcg2Dxe's DEPEX by making it inherit the new DEPEX from the library. > > > *** Slide 48: DXE Phase -- Other Modules > > Regarding IncompatiblePciDeviceSupportDxe: the proposed update sounds > plausible and simple enough. > > (21) "AcpiPlatformDxe: Support for MADT/ACPI addition to report Td > Mailbox entry" > > Firmware-owned tables must not be installed from this driver. > > Please refer to my "Xen removal" patch set again, for > <https://bugzilla.tianocore.org/show_bug.cgi?id=2122>, which I mention > above in point (14). As part of the Xen removal, the AcpiPlatformDxe > driver in OvmfPkg is significantly trimmed: all unused (dead) cruft is > removed, including any ACPI table templates that are built into the > firmware. > > OvmfPkg/AcpiPlatformDxe is responsible *solely* for implementing the > client side of the QEMU ACPI linker/loader. > > If you need to prepare & install different ACPI tables, please do it > elsewhere, in another DXE driver. A bunch of other firmware modules do > that (NFIT, IBFT, BGRT, ...). > > For example, the OvmfPkg/TdxDxe DXE_DRIVER is supposed to be launched > early in the DXE phase, via APRIORI section -- please consider > registering a protocol notify in that driver, for > EFI_ACPI_TABLE_PROTOCOL, and when it becomes available, install > whatever > *extra* tables you need. > > Note that if you need to *customize* an ACPI table that QEMU already > provides, then you will have to modify the ACPI generator on the QEMU > side. It is a design tenet between QEMU and OVMF that OVMF include no > business logic for either parsing or fixing up ACPI tables that QEMU > provides. AcpiPlatformDxe contains the minimum (which is already a whole > lot, unfortunately) that's necessary for implementing the QEMU ACPI > linker/loader client in the UEFI environment. > > The slide deck mentions MADT, which is also known as the "APIC" table -- > and indeed, QEMU does provide that. (See acpi_build_madt() > [hw/i386/acpi-common.c].) So if TDX needs MADT customizations, that > should go into QEMU. > > (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. > > 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. > > 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. > > 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. > > (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.) > > 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? > > > *** Slide 50: Library > > (23) Should we introduce Null instances for all (or most) new lib > classes here? Code size is a concern (static linking). If we extend a > common OvmfPkg module with new hook points, it's one thing to return > from that hook point early *dynamically*, but it's even better (given > separate platforms) to allow the traditional platform firmware to use a > Null lib instance, to cut out all the dead code statically. > > > *** Slides 51 through 52 > > Seems OK. > > > *** Slide 53: > > (24) It might be worth noting that BaseIoLibIntrinsic already has some > SEV enlightenment, as the FIFO IO port operations (which normally use > the REP prefix) are not handled on SEV. I don't have an immediate idea > why this might matter, we should just minimize code duplication if > possible. > > > *** Slides 54-56: > > No comments, this stuff seems reasonable. > > > *** Slide 57: MpInitLib > > I don't know enough to give a summary judgement. > > > All in all, I see the controversial / messy parts in the platform > bringup, and how all that differs from the traditional ("legacy") OVMF > platforms. I admit I *may* be biased in favor of SEV, possibly because > SEV landed first -- if you find signs of such a bias in my comments, > please don't hesitate to debunk those points. Yet my general impression > is that the early bringup stuff is significantly different from > everything before, and because of this, a separate platform is > justified. > > Definitely separate from the traditional OVMF IA32, IA32X64, and X64 > platforms, and *possibly* separate from the "remote attestation" > AmdSevX64.dsc platform. I would approach the TDX feature-set in complete > isolation (exactly how Intel commenced the work, if I understand > correctly), modulo obviously shareable / reusable parts, and then slowly > & gradually work on extracting / refactoring commonalities. > > (But, given my stance on Xen for example, I could disagree even with the > latter, retroactive kind of unification -- it all boils down to shared > developer and user base. Component sharing should reflect the community > structure, otherwise maintenance will be a nightmare.) > > Thanks > Laszlo > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76024): https://edk2.groups.io/g/devel/message/76024 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] -=-=-=-=-=-=-=-=-=-=-=-
