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|0x60000018
>
> 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 (#80426): https://edk2.groups.io/g/devel/message/80426
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]
-=-=-=-=-=-=-=-=-=-=-=-
