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

["Min Xu" <min.m.xu@intel.com>]

On January 11, 2022 5:23 PM, Gerd Hoffmann wrote:
> 
> > > Well, if you want avoid the refactoring because of the risk there is
> > > still the option to have tdx config-b use the normal PEI boot flow.
> > > Then revisit refactoring and adding support for PEI-less boot later.
> > >
> > I think it still makes sense (Adding a basic PlatformInitLib which
> > brings up tdx guest and legacy guest in Pei-less boot, but not touch
> > PlatformPei).
> 
> > 1. The goal of TDVF-Config-B is to bring up tdx guest and legacy guest
> > without PEI. So that attack surface can be reduced.
> 
> Hmm?  Isn't the main goal of config-b to support the advanced tdx features
> (attestation etc)?
PEI-less boot is one of the main goal of Config-B. Actually PEI-less boot is in the original design of TDVF. RTMR-based measurement and measure boot are another important goals.
> 
> I don't see that PEI-less boot is required for that.  Sure, when stripping down
> the build and removing all the features which require PEIMs there isn't much
> left to do for the PEI phase.  So it makes sense to look into dropping PEI
> altogether.  But it's more a "nice to have"
> than a hard requirement, no?
No. I have to say PEI-less boot in Config-B is a hard requirement.
> 
> > 2. There are common functions when bring up tdx guest and legacy guest
> > in Config-B. So PlatformInitLib is necessary.
> 
> Sure.
> 
> > 3. As I explained there are many if-else checks in PlatformPei and the
> > logics are rather complicated (because PlatformPei serves
> > S3/SMM/SEV/TDX/Legacy/Microvm/CloudHypervisor, etc). To be honest I
> > have not so much confidence to abstract PlatformPei's common function
> > to PlatformInitLib.
> 
> What is the problem with moving code?  After some preparing steps (add
> platform info hob, move global variables to the hob) it should be possible to
> move the code needed by config-b (memory detection via fw_cfg or tdx hob,
> pci init, ...) from PlatformPei to PlatformInitLib and (also) use it in the SEC
> phase.  Likewise for code which runs in DXE in PEI-less mode (setting PCDs).
> 
> The code not needed by config-b (smm, s3, ...) can stay in PlatformPei.
Yes, PlatformPei can be refactored in this way.
> 
> > 4. But a basic version of PlatformInitLib is a good start.
> 
> Yes.  Having initially only the functions needed by config-b in PlatformInitLib
> is perfectly fine, but this should be a code *move* not a copy.
> 
> > During the development and community review, we can understand better
> > what functions should be wrapped into PlatformInitLib. After that
> > PlatformInitLib can be evolved for OvmfPkg/PlatformPei,
> > Bhyve/PlatformPei, XenPlatformPei.
> 
> Yes, most likely there are a number of opportunities to reduce code
> duplication in the three PlatformPei variants we have by moving code to the
> (shared) PlatformInitLib.
> 
> That can be looked at later.

So let me summarize the discussion about PlatformInitLib.
1. PlatformInitLib wraps the common functions in OvmfPkg/PlatformPei. These common functions covers the memory detection via fw_cfg, pci init, cmos, (MemDetect.c/Platform.c/Cmos.c). And PlatformInitLib will not handle the S3/SMM variants.
2. OvmfPkg/PlatformPei will be refactored with PlatformInitLib. The functions not needed by config-b stay in PlatformPei.
3. Config-B support PEI-less boot for both legacy guest and td guest.

If you agree, then I will update the patch-sets based on above discussions.

Thanks
Min