[edk2-devel] [PATCH 2/2] OvmfPkg/AmdSev/SecretDxe: make secret location naming generic

Thu Dec 17 00:25:31 UTC 2020

Thank you James.
I think you are on the right way. We will think about the feature in TDVF and do development in edk2-staging tree at first.

The API change looks good to me. I don’t think you need change OvmfPkg/AmdSev/SecretDxe at this moment.
We are also thinking how to support both SEV and TDX at same tree. But it is not fully finalized yet.
Not only Secret handling, but also MemEncryptionLib, IoMmu, UnacceptedMemory, etc. We will do that step by step in the near future.

I believe we can cross the bridge when we come to it.

Thank you
Yao Jiewen

> -----Original Message-----
> From: James Bottomley <jejb at linux.ibm.com>
> Sent: Thursday, December 17, 2020 12:54 AM
> To: Dov Murik <dovmurik at linux.vnet.ibm.com>
> Cc: devel at edk2.groups.io; Dov.Murik1 at il.ibm.com; ashish.kalra at amd.com;
> brijesh.singh at amd.com; tobin at ibm.com; david.kaplan at amd.com;
> jon.grimm at amd.com; thomas.lendacky at amd.com; frankeh at us.ibm.com;
> Dr . David Alan Gilbert <dgilbert at redhat.com>; Laszlo Ersek
> <lersek at redhat.com>; Justen, Jordan L <jordan.l.justen at intel.com>; Ard
> Biesheuvel <ard.biesheuvel at arm.com>; Yao, Jiewen <jiewen.yao at intel.com>
> Subject: Re: [PATCH 2/2] OvmfPkg/AmdSev/SecretDxe: make secret location
> naming generic
> 
> On Wed, 2020-12-16 at 03:27 -0500, Dov Murik wrote:
> > On Tue, Dec 15, 2020 at 05:41:46PM -0800, James Bottomley wrote:
> > > It is anticipated that this part of the code will work for both
> > > Intel TDX and AMD SEV, so remove the SEV specific naming and change
> > > to ConfidentialComputing as a more architecture neutral prefix.
> > > Apart from the symbol rename, there are no code changes.
> > >
> > > Signed-off-by: James Bottomley <
> > > James.Bottomley at HansenPartnership.com>
> > > ---
> > >  OvmfPkg/OvmfPkg.dec                                    |  2 +-
> > >  OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf                 |  2 +-
> > >  ...SevLaunchSecret.h => ConfidentialComputingSecret.h} | 10 +++++-
> > > ----
> > >  OvmfPkg/AmdSev/SecretDxe/SecretDxe.c                   |  6 +++---
> > >  4 files changed, 10 insertions(+), 10 deletions(-)
> > >  rename OvmfPkg/Include/Guid/{SevLaunchSecret.h =>
> > > ConfidentialComputingSecret.h} (69%)
> > >
> > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> > > index 8a294116efaa..50d7b27d941c 100644
> > > --- a/OvmfPkg/OvmfPkg.dec
> > > +++ b/OvmfPkg/OvmfPkg.dec
> > > @@ -117,7 +117,7 @@ [Guids]
> > >    gLinuxEfiInitrdMediaGuid              = {0x5568e427, 0x68fc,
> > > 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
> > >    gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a,
> > > 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
> > >    gGrubFileGuid                         = {0xb5ae312c, 0xbc8a,
> > > 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
> > > -  gSevLaunchSecretGuid                  = {0xadf956ad, 0xe98c,
> > > 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> > > +  gConfidentialComputingSecretGuid      = {0xadf956ad, 0xe98c,
> > > 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
> > >
> > >  [Ppis]
> > >    # PPI whose presence in the PPI database signals that the TPM
> > > base address
> > > diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> > > b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> > > index 62ab00a3d382..40bda7ff846c 100644
> > > --- a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> > > +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
> > > @@ -27,7 +27,7 @@ [LibraryClasses]
> > >    UefiDriverEntryPoint
> > >
> > >  [Guids]
> > > -  gSevLaunchSecretGuid
> > > +  gConfidentialComputingSecretGuid
> > >
> > >  [FixedPcd]
> > >    gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
> > > diff --git a/OvmfPkg/Include/Guid/SevLaunchSecret.h
> > > b/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
> > > similarity index 69%
> > > rename from OvmfPkg/Include/Guid/SevLaunchSecret.h
> > > rename to OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
> > > index dfd89646651b..7026fc5b089f 100644
> > > --- a/OvmfPkg/Include/Guid/SevLaunchSecret.h
> > > +++ b/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
> > > @@ -6,12 +6,12 @@
> > >     SPDX-License-Identifier: BSD-2-Clause-Patent
> > >   **/
> >
> > The comment at the top of this file (not visible in this diff) also
> > mentions "SEV Launch Secret" which should be renamed to "Confidential
> > Computing Secret".
> 
> Yes, I can update that.  The other thing I didn't change is the tree
> location ... it's still OvmfPkg/AmdSev/SecretDxe.  That's because I
> wasn't sure what the TDX implementation would look like.  It's possible
> they might have their own SecretDxe simply using the header for the
> structure and GUID (which means everything is correct) or whether both
> SEV and TDX should use the same .c file.
> 
> I think this raises the broader question of how much collaboration
> should there be between the two systems.  I did a small amount of .dsc
> file stripping in the previous patch, but it sounds like Intel has done
> a whole lot more for TDVF ... removing the entire PEI phase was what I
> heard in the webinar yesterday ... so I think we could get a lot of
> cross fertilization doing combinations at that level.  We might need to
> think about what features are general to a OVMF supporting a
> confidential VM, like stripping, and what are technology specific, like
> the exact mechanism of secret injection.
> 
> James
> 

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