I do not see any conflict here.
You can use below definition
enum {
CC_ATTR_AMD_SEV = 0x0001,
CC_ATTR_AMD_SEV_ES = 0x0101,
CC_ATTR_AMD_SEV_SNP = 0x0201,
CC_ATTR_INTEL_TDX = 0x0002,
} ConfidentialComputingAttr;
BTW: Please remove SGX, we don’t need it here.
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Thursday, September 9, 2021 6:51 PM
> To: Xu, Min M <min.m...@intel.com>; Yao, Jiewen <jiewen....@intel.com>;
> devel@edk2.groups.io
> Cc: James Bottomley <j...@linux.ibm.com>; Tom Lendacky
> <thomas.lenda...@amd.com>; Justen, Jordan L <jordan.l.jus...@intel.com>;
> Ard Biesheuvel <ardb+tianoc...@kernel.org>; Erdem Aktas
> <erdemak...@google.com>; Michael Roth <michael.r...@amd.com>; Gerd
> Hoffmann <kra...@redhat.com>
> Subject: Re: [edk2-devel] [PATCH v6 00/29] Add AMD Secure Nested Paging
> (SEV-SNP) support
>
> Hi Min,
>
> On 9/8/21 7:31 PM, Xu, Min M wrote:
> > On September 9, 2021 3:46 AM, Brijesh Singh wrote:
> >> Thank you so much Yao for reviewing the patches. Based on some comments
> >> from Gerd I may update code around the reset vector area (mainly use the
> >> metadata format etc). For your comments regarding the introducing a new
> >> PcdConfidentialComputingCategory I will look to see what I can come up
> with
> >> and in UefiCpuPkg I will try to move all the SEV specific functions in new
> >> files
> >> (where applicable).
> >>
> > Hi, Brijesh
> > if you are considering to introduce a new PcdConfidentialComputingCategory
> > as Jiewen suggested below:
> >>> 0008-UefiCpuPkg-Define-the-SEV-SNP-specific-dynamic-PCDs
> >>> I really don't like the idea to use BOOL PcdSevEsIsEnabled and
> >> PcdSevSnpIsEnabled.
> >>> Can we define *one* PCD - such as PcdConfidentialComputingCategory?
> >>> We can assign range 0x0000~0xFFFF to AMD SEV, 0x10000~0x1FFFF to Intel
> >> TDX.
> >>> Then SEV=0x0000, SEV-ES=0x0001, SEV-SNP=0x0002, and TDX=0x10000
> >> later.
> >>> I really don't want to keep adding PCD endlessly in the future, like
> >> PcdSevXXXIsEnabled, PcdSevYYYIsEnabled, PcdTdxIsEnabled,
> >> PcdTdx20Enabled, PcdTdx30Enabled, ......
> > I also have some suggestions.
> >
> > As we have below definition in WorkArea.h
> > typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
> > UINT8 GuestType;
> > UINT8 Reserved1[3];
> > } CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;
> >
> > Can we update above CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to
> below:
> > typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
> > UINT8 GuestType;
> > UINT8 SubType; // subtype which
> > indicates SEV-ES, SEV-NP,
> or TDX 1.0, TDX 2.0 etc.
> > UINT8 Reserved1[2];
> > } CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;
> >
> > The PcdConfidentialComputingCategory can be defined as UINT32, like below:
> > ## This dynamic PCD indicates the Confidential Computing Category
> > # [7:0] Confidential Computing Category (0 - Non-Cc, 1 - AmdSev, 2 -
> IntelTdx)
> > # [15:8] Sub-Category (defined by each vendor, SEV-ES, SEV-SNP, or
> > TDX-1.0,
> TDX-2.0, etc)
> > # [31:16] Reserved
> > # @Prompt Confidential Computing Category
> >
> gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingCategory|0|UINT32|0x
> 60000018
> >
> > So that we simply copy the
> CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER to
> PcdConfidentialComputingCategory.
> > What's your thought?
>
> I am not sure if its a good idea to pack a header like above in a 32-bit
> PCD. The caller need to unpack the 32-bit number and perform a bitshit
> etc. Additionally we also need to check for reserved bits being set to
> zero etc. I am more inclined toward something like this:
>
> enum {
>
> /* The guest is running with memory encryption disabled. */
>
> CC_ATTR_UNDEF,
>
> /* The guest is running with active AMD SEV memory encryption. */
>
> CC_ATTR_AMD_SEV,
>
> /* The guest is running with active AMD SEV-ES memory encryption. */
>
> CC_ATTR_AMD_SEV_ES,
>
> /* The guest is running with active AMD SEV-SNP memory encryption. */
>
> CC_ATTR_AMD_SEV_SNP,
>
> /* The guest is running with active Intel TDX memory encryption. */
>
> CC_ATTR_INTEL_TDX,
>
> /* The guest is running with active Intel SGX memory encryption. */
>
> CC_ATTR_INTEL_SGX,
>
> } ConfidentialComputingAttr;
>
> The PcdConfidentialComputingAttr will be set to zero. The OVMF will set
> this dynamic PCD in PEI phase. The UefiCpuPkg will provide a new
> function that can be used by anyone to check the CC guest type.
>
> BOOLEAN
>
> CcPlatformHas(enum ConfidentialComputingAttr attr)
>
> {
>
> UINT32 Val;
>
> Val = PcdGet32 (PcdConfidentialComputingAttr);
>
> return val == attr;
>
> }
>
>
> > Thanks!
> > Min
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80433): https://edk2.groups.io/g/devel/message/80433
Mute This Topic: https://groups.io/mt/85306653/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
