Re: [PATCH 2/2] OvmfPkg/AmdSev/SecretDxe: make secret location naming generic

["James Bottomley" <jejb@linux.ibm.com>]

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@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