[PATCH V3 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h


Min Xu
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902

The confidential computing guest type (GUEST_TYPE) was defined in
OvmfPkg/Include/WorkArea.h. Now it is to be moved to
MdePkg/Include/ConfidentialComputingGuestAttr.h and renamed as
CC_GUEST_TYPE.

There are 2 reasons for this change.
1. CC_GUEST_TYPE is a generic definition and will be used in CcProbeLib
which is defined in MdePkg.
2. Based on the latest edk2 coding style:
- First character should be upper case
- Must contain lower case characters
- No white space characters
- Global variable name must start with a 'g'

As the first step CC_GUEST_TYPE is defined in this patch. In the
next patch GUEST_TYPE will be deleted. This is to make sure the
bisect work correctly.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
MdePkg/Include/ConfidentialComputingGuestAttr.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h b/MdePkg/Include/ConfidentialComputingGuestAttr.h
index dd2541c6dcdf..9e9424a01559 100644
--- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
+++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
@@ -1,5 +1,5 @@
/** @file
-Definitions for Confidential Computing Attribute
+Definitions for Confidential Computing Guest Attributes

Copyright (c) 2021 AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -9,6 +9,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_
#define CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_

+//
+// Confidential computing guest type
+//
+typedef enum {
+ CCGuestTypeNonEncrypted = 0,
+ CCGuestTypeAmdSev,
+ CCGuestTypeIntelTdx,
+} CC_GUEST_TYPE;
+
typedef enum {
/* The guest is running with memory encryption disabled. */
CCAttrNotEncrypted = 0,
--
2.29.2.windows.2


Lendacky, Thomas
 

On 4/16/22 22:01, Min Xu wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
The confidential computing guest type (GUEST_TYPE) was defined in
OvmfPkg/Include/WorkArea.h. Now it is to be moved to
MdePkg/Include/ConfidentialComputingGuestAttr.h and renamed as
CC_GUEST_TYPE.
There are 2 reasons for this change.
1. CC_GUEST_TYPE is a generic definition and will be used in CcProbeLib
which is defined in MdePkg.
2. Based on the latest edk2 coding style:
- First character should be upper case
- Must contain lower case characters
- No white space characters
- Global variable name must start with a 'g'
As the first step CC_GUEST_TYPE is defined in this patch. In the
next patch GUEST_TYPE will be deleted. This is to make sure the
bisect work correctly.
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: James Bottomley <jejb@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Gerd Hoffmann <kraxel@...>
Cc: Brijesh Singh <brijesh.singh@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Min Xu <min.m.xu@...>
---
MdePkg/Include/ConfidentialComputingGuestAttr.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h b/MdePkg/Include/ConfidentialComputingGuestAttr.h
index dd2541c6dcdf..9e9424a01559 100644
--- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
+++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
@@ -1,5 +1,5 @@
/** @file
-Definitions for Confidential Computing Attribute
+Definitions for Confidential Computing Guest Attributes
Copyright (c) 2021 AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -9,6 +9,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_
#define CONFIDENTIAL_COMPUTING_GUEST_ATTR_H_
+//
+// Confidential computing guest type
+//
+typedef enum {
+ CCGuestTypeNonEncrypted = 0,
+ CCGuestTypeAmdSev,
+ CCGuestTypeIntelTdx,
+} CC_GUEST_TYPE;
Should these be CcGuest... ? The precedent seems to be use lowercase even when the the acronym is uppercase, e.g. PCI => Pci, GHCB => Ghcb, SMBIOS => SmBios, NVME => Nvme, etc.

Thanks,
Tom

+
typedef enum {
/* The guest is running with memory encryption disabled. */
CCAttrNotEncrypted = 0,


Min Xu
 

On April 18, 2022 9:48 PM, Tom Lendacky wrote:
+//
+// Confidential computing guest type
+//
+typedef enum {
+ CCGuestTypeNonEncrypted = 0,
+ CCGuestTypeAmdSev,
+ CCGuestTypeIntelTdx,
+} CC_GUEST_TYPE;
Should these be CcGuest... ? The precedent seems to be use lowercase even
when the the acronym is uppercase, e.g. PCI => Pci, GHCB => Ghcb, SMBIOS =>
SmBios, NVME => Nvme, etc.
Thanks for reminder. It will be updated in the next version.

Thanks
Min


Gerd Hoffmann
 

Hi,

As the first step CC_GUEST_TYPE is defined in this patch. In the
next patch GUEST_TYPE will be deleted. This is to make sure the
bisect work correctly.
Hmm, what exactly went wrong? Splitting into two patches looks
pointless. If the change is too big for an all-in-one patch typical
bisectable splitting would be:

(1) add CC_GUEST_TYPE
(2) multiple patches switching code from GUEST_TYPE to CC_GUEST_TYPE
(for example one patch per Pkg).
(3) remove GUEST_TYPE

+//
+// Confidential computing guest type
+//
+typedef enum {
+ CCGuestTypeNonEncrypted = 0,
+ CCGuestTypeAmdSev,
+ CCGuestTypeIntelTdx,
+} CC_GUEST_TYPE;
Acked-by: Gerd Hoffmann <kraxel@...>

take care,
Gerd