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

Wed Nov 24 15:30:37 UTC 2021

One more clarification: My comment below is only applicable for the TDVF platform, but not applicable to a general platform including OVMF.

In TDVF, Feature X* is a very small set, but in OVMF or general platform, Feature X* is a large set.

For example, if a platform need support S3 resume, Recovery, Capsule Update, then I won't recommend to remove PEI.
The reason is 2) the delta of risk becomes high then. Current PEI already provides a mature (and complex) infrastructure for them.
Moving those feature to somewhere else means to carry the burden to reinvent the infrastructure for S3, recovery, capsule update.

Here, I only recommend to remove for TDVF config-B, because the Feature X* is so simple that it could be moved to SEC without extra risk.

Removing PEI for general OVMF is a different topic. I don’t want to discuss in this thread.

Thank you
Yao Jiewen

> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Wednesday, November 24, 2021 11:00 PM
> To: devel at edk2.groups.io; jejb at linux.ibm.com; Gerd Hoffmann
> <kraxel at redhat.com>
> Cc: Xu, Min M <min.m.xu at intel.com>; Ard Biesheuvel
> <ardb+tianocore at kernel.org>; Justen, Jordan L <jordan.l.justen at intel.com>;
> Brijesh Singh <brijesh.singh at amd.com>; Erdem Aktas
> <erdemaktas at google.com>; Tom Lendacky <thomas.lendacky at amd.com>
> Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
> support Tdx
> 
> 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of James
> > Bottomley
> > Sent: Wednesday, November 24, 2021 10:07 PM
> > To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io; Gerd
> > Hoffmann <kraxel at redhat.com>
> > Cc: Xu, Min M <min.m.xu at intel.com>; Ard Biesheuvel
> > <ardb+tianocore at kernel.org>; Justen, Jordan L <jordan.l.justen at intel.com>;
> > Brijesh Singh <brijesh.singh at amd.com>; Erdem Aktas
> > <erdemaktas at google.com>; Tom Lendacky <thomas.lendacky at 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
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84043): https://edk2.groups.io/g/devel/message/84043
Mute This Topic: https://groups.io/mt/86739864/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-