Re: [PATCH 08/10] OvmfPkg: Update Sec to support Tdvf Config-B


Gerd Hoffmann
 

Hi,

PCDs cannot be set in SEC phase, so the values should be saved in a
Hob (for example, PLATFORM_INFO_HOB). In early DXE phase these values
are set to the PCDs. This is how TdxDxe does today.

Other tasks can be done in SEC phase. I think there should be a lib
(for example, PlatformPeiLib) to wrap these functions so that they can
be re-used by OvmfPkg/PlatformPei.
Yes, I think we need a PlatformLib for the platform initialization
code. With PEI we would simply link the lib into PlatformPei, without
PEI we would link parts of the lib into SEC and parts of the lib into
DXE.

PEI-less booting up legacy guest doesn't support TPM.

So to boot up legacy guest without PEI phase, there will be below changes.
1. OvmfStartupLib: (like TdxStartupLib)
- Decompress DxeFv, locate DxeCore, create IdentityMappingPageTables, then jump to DxeCore.
Yes. Basically rename TdxStartupLib to OvmfStartupLib and add some
IfTdx() checks.

2. PlatformPeiLib:
- Wrap the functions to do memory initialization, etc. (see tasks 1-5)
Yes. Move code from PlatformPei to PlatformLib. Might also need some
reorganization due to SEC restrictions.

3. OvmfLegacyDxe
- Set the PCDs (see task 6)
Well, in Tdx mode you have to set some PCDs too ...

Also not sure we actually need a new Dxe. Can't we just handle
that in PlatformDxe in case of a PEI-less boot?

I know there are many discussions in above options. Can we follow below road map so that we can discuss 3 (How to achieve ONE Binary) in more details?
1. Basic Config-B (PEI-less and only Tdx guest)
2. Advanced Config-B (RTMR based measurement)
3. One Binary Config-B (support legacy guest)
IMHO step #1 must be reorganizing the platform initialization code for
PEI-less boot (create PlatformLib as discussed above).

This patch series side-steps that by simply duplicating the code. PCI
initialization for example. Also setting the tdx PCDs. Having two (or
even more) copies of the same code in the tree is a bad idea though.
It makes long-term maintenance harder for various reasons.

... and given that TDX-capable
hardware is not yet production ready I find it rather important that testing
the PEI-less boot workflow does not require TDX.

It'll also make it much easier to add CI coverage.
I am thinking if SEV features are covered in CI?
Because I want to make sure our changes don't impact SEV.
AmdSevX64.dsc has build-test coverage. There is no qemu boot test
because FlashRomImage() (in OvmfPkg/PlatformCI/PlatformBuildLib.py)
is not flexible enough for that. Fixing that and adding a boot test
(in non-sev mode) shouldn't be that difficult though.

Same for IntelTdx.dsc: adding a CI boot test (in non-tdx mode) should be
easy, and it should help preventing regressions in PEI-less boot flow.

take care,
Gerd

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