Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

["Yao, Jiewen" <jiewen.yao@intel.com>]

OK. Got it.
Let me explain it in more detail.

Let's assume PEI phase include 3 major classes {PEI Core, PEI Arch PEM*, Feature X*}. * means 0~multiple.
To all of us what really matter is Feature X, the existence of PEI Core + PEI Arch PEIM* is to support Feature X*.

From architecture perspective, if a platform is complex (e.g. there are lots of Feature X*) and feature X* have lots of inter-dependency, then PEI is a good place to coordinate the Feature X*. (Example, Feature X* are memory init and silicon init)
But if a platform simple (e.g. there is only few Feature X*) and feature X* have no much dependency, the including PEI does not bring too much value. That is why you see multiple platforms in EDKII does not include PEI.

From security perspective, Feature X* shall always perform check, no matter where the feature X sits in SEC, PEI or DXE. The risk of Feature X always exists, no matter where the feature X sits in SEC, PEI or DXE. I completely agree.
At same time, the PEI Core + PEI Arch PEIM* also bring unknown security risk. That was TRUE in history. It did happen. So my motivation to remove PEI phase is to reduce the risk introduced by PEI Core + PEI Arch PEIM*. Again, I do not mean to reduce the risk introduced by Feature X.

Now it seems we are really debating two things: (please correct me if I am wrong)
1) What is risk introduced by PEI Core + PEI Arch PEIM* ?
2) What is the delta of risk by moving Feature X from PEI to other place (SEC or DXE) ?

For 1), my answer is that the risk is definitely bigger than zero, based upon history data. (This is an objective answer.) That is the main of my motivation to make it become zero by removing PEI.
For 2), my answer is that the delta is almost 0, based upon my experience. (I admit this is a subjective answer, because I cannot prove.). We are trying our best to reduce the risk of the Feature A* as well. Assuming delta of risk <= risk, then it will become very smaller.

So, my judgement is by removing PEI, we can reduce the risk introduce by PEI Core + PEI Arch PEIM*. Reducing code == Reducing Security Risk.
Also, this gives us a chance to focus on reviewing Feature X itself, instead of the complex interaction with PEI Core + PEI Arch PEIM*. Reducing complexity == Reducing Security Risk.
(In history, we got lots of complain on the complexity of the non-deterministic flow by CALLBACK and NOTIFY function in Core. A feature developer might not have idea on when the code will be called, and what the system status is at that moment.)


Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of James
> Bottomley
> Sent: Wednesday, November 24, 2021 10:07 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Gerd
> Hoffmann <kraxel@redhat.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Erdem Aktas
> <erdemaktas@google.com>; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
> support Tdx
> 
> On Wed, 2021-11-24 at 14:03 +0000, Yao, Jiewen wrote:
> > James
> > I am sorry that it is hard for me to understand your point.
> >
> > To be honest, I am not sure what is objective on the discussion.
> > Are you question the general threat model analysis on UEFI PI
> > architecture?
> 
> The object is for me to understand why you think eliminating PEI
> improves security because I think it moves it in the opposite
> direction.
> 
> > Or are you trying to persuade me we should include PEI in TDVF,
> > because you think it is safer to add code in PEI ?
> > Or something else?
> >
> > Please enlighten me that.
> 
> Somewhere a decision was taken to remove PEI from the OVMF that is used
> to bring up TDX on the grounds of "improving security".  I'm struggling
> to understand the rationale for this.
> 
> James
> 
> 
> 
> 
> 
>