public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V4 0/7] Introduce CcProbe in MdePkg
@ 2022-04-19  0:26 Min Xu
  2022-04-19  0:26 ` [PATCH V4 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h Min Xu
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas, Tom Lendacky

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

Bad IO performance in SEC phase is observed after TDX features was
introduced. (after commit b6b2de884864 - "MdePkg: Support mmio for
Tdx guest in BaseIoLibIntrinsic").

This is because IsTdxGuest() will be called in each MMIO operation.
It is trying to cache the result of the probe in the efi data segment.
However, that doesn't work in SEC, because the data segment is read only
(so the write seems to succeed but a read will always return the
original value), leading to us calling TdIsEnabled() check for every
mmio we do, which is causing the slowdown because it's very expensive.

CcProbe is introduced in this patch-set. It is called in
BaseIoLibIntrinsicSev instead of IsTdxGuest. There are 2 versions of
the CcProbeLib. Null instance of CcProbe always returns
CCGuestTypeNonEncrypted. Its OvmfPkg version checks the Ovmf work area
and returns the CC guest type.

In this patch-set another issue is fixed with CcProbe as well. If the
working guest is SEV and in the beginning of SecMain.c TdIsEnabled()
was called. At this point, exception handling is not established and
a CPUID instruction will generate a #VC and cause the booting SEV guest
to crash. Patch #7 is to fix this broken.

Code is at: https://github.com/mxu9/edk2/tree/cc_probe.v4

v4 changes:
 - Rename the field name in CC_GUEST_TYPE from CCXxx to CcXxx based
   on the community feedback.

v3 changes:
 - Fix the broken issue in SEV guest at SecMain.c. Please refer to
   Patch #7.

v2 changes:
 - Rename TdProbe to CcProbe to make the lib work for Confidential
   Computing guests.
 - Rename the GUEST_TYPE to CC_GUEST_TYPE and move it from
   WorkArea.h@OvmfPkg to ConfidentialComputingGuestAttr.h@MdePkg.
   This is because CcProbeLib is designed to return the CC Guest
   type and the lib is located at MdePkg.
 - Rename the CC_GUEST_TYPE's fields name to Camel style. See the
   commit message in patch #1.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min Xu (7):
  MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h
  OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE
  MdePkg: Add CcProbeLibNull
  OvmfPkg: Add CcProbeLib
  OvmfPkg: Add CcProbeLib in *.dsc
  MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev
  OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled

 .../Include/ConfidentialComputingGuestAttr.h  | 11 ++++++-
 MdePkg/Include/Library/CcProbeLib.h           | 26 ++++++++++++++++
 .../BaseIoLibIntrinsicSev.inf                 |  1 +
 .../BaseIoLibIntrinsic/IoLibInternalTdx.c     | 13 ++------
 .../Library/CcProbeLibNull/CcProbeLibNull.c   | 26 ++++++++++++++++
 .../Library/CcProbeLibNull/CcProbeLibNull.inf | 21 +++++++++++++
 MdePkg/MdePkg.dec                             |  5 +++
 MdePkg/MdePkg.dsc                             |  1 +
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
 OvmfPkg/Bhyve/BhyveX64.dsc                    |  1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc                |  1 +
 OvmfPkg/Include/WorkArea.h                    |  9 +-----
 OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  1 +
 OvmfPkg/IntelTdx/Sec/SecMain.c                |  6 ++--
 OvmfPkg/IntelTdx/Sec/SecMain.inf              |  1 +
 .../PeiMemEncryptSevLibInternal.c             |  2 +-
 .../SecMemEncryptSevLibInternal.c             |  2 +-
 OvmfPkg/Library/CcProbeLib/CcProbeLib.c       | 31 +++++++++++++++++++
 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf     | 25 +++++++++++++++
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c   |  2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                |  1 +
 OvmfPkg/OvmfPkgIa32.dsc                       |  1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
 OvmfPkg/OvmfPkgX64.dsc                        |  1 +
 OvmfPkg/OvmfXen.dsc                           |  1 +
 OvmfPkg/Sec/AmdSev.c                          |  2 +-
 OvmfPkg/Sec/SecMain.c                         |  5 +--
 OvmfPkg/Sec/SecMain.inf                       |  1 +
 28 files changed, 170 insertions(+), 29 deletions(-)
 create mode 100644 MdePkg/Include/Library/CcProbeLib.h
 create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
 create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
 create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.c
 create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf

-- 
2.29.2.windows.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V4 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  0:26 ` [PATCH V4 2/7] OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE Min Xu
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas, Tom Lendacky

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@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 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..44e6df800207 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 2/7] OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
  2022-04-19  0:26 ` [PATCH V4 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  0:26 ` [PATCH V4 3/7] MdePkg: Add CcProbeLibNull Min Xu
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas, Tom Lendacky

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

Replace GUEST_TYPE with CC_GUEST_TYPE which is defined in
MdePkg/Include/ConfidentialComputingGuestAttr.h.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Include/WorkArea.h                               | 9 +--------
 .../BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c   | 2 +-
 .../BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c   | 2 +-
 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c              | 2 +-
 OvmfPkg/Sec/AmdSev.c                                     | 2 +-
 5 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index b67edd14e79f..bf56fc4a6f65 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -10,14 +10,7 @@
 #ifndef __OVMF_WORK_AREA_H__
 #define __OVMF_WORK_AREA_H__
 
-//
-// Guest type for the work area
-//
-typedef enum {
-  GUEST_TYPE_NON_ENCRYPTED,
-  GUEST_TYPE_AMD_SEV,
-  GUEST_TYPE_INTEL_TDX,
-} GUEST_TYPE;
+#include <ConfidentialComputingGuestAttr.h>
 
 //
 // Confidential computing work area header definition. Any change
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index 3f8f91a5da12..41d1246a5b31 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -36,7 +36,7 @@ GetSevEsWorkArea (
   //
   // If its not SEV guest then SevEsWorkArea is not valid.
   //
-  if ((WorkArea == NULL) || (WorkArea->Header.GuestType != GUEST_TYPE_AMD_SEV)) {
+  if ((WorkArea == NULL) || (WorkArea->Header.GuestType != CcGuestTypeAmdSev)) {
     return NULL;
   }
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index 80aceba01bcf..27148c7e337a 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -36,7 +36,7 @@ GetSevEsWorkArea (
   //
   // If its not SEV guest then SevEsWorkArea is not valid.
   //
-  if ((WorkArea == NULL) || (WorkArea->Header.GuestType != GUEST_TYPE_AMD_SEV)) {
+  if ((WorkArea == NULL) || (WorkArea->Header.GuestType != CcGuestTypeAmdSev)) {
     return NULL;
   }
 
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
index b8230613dcea..7ab7027af168 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
@@ -35,7 +35,7 @@ QemuFwCfgIsTdxGuest (
   CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER  *CcWorkAreaHeader;
 
   CcWorkAreaHeader = (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
-  return (CcWorkAreaHeader != NULL && CcWorkAreaHeader->GuestType == GUEST_TYPE_INTEL_TDX);
+  return (CcWorkAreaHeader != NULL && CcWorkAreaHeader->GuestType == CcGuestTypeIntelTdx);
 }
 
 /**
diff --git a/OvmfPkg/Sec/AmdSev.c b/OvmfPkg/Sec/AmdSev.c
index d8fd35650d7d..6af38aaf46ee 100644
--- a/OvmfPkg/Sec/AmdSev.c
+++ b/OvmfPkg/Sec/AmdSev.c
@@ -251,7 +251,7 @@ IsSevGuest (
 
   WorkArea = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
 
-  return ((WorkArea != NULL) && (WorkArea->Header.GuestType == GUEST_TYPE_AMD_SEV));
+  return ((WorkArea != NULL) && (WorkArea->Header.GuestType == CcGuestTypeAmdSev));
 }
 
 /**
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 3/7] MdePkg: Add CcProbeLibNull
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
  2022-04-19  0:26 ` [PATCH V4 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h Min Xu
  2022-04-19  0:26 ` [PATCH V4 2/7] OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  0:26 ` [PATCH V4 4/7] OvmfPkg: Add CcProbeLib Min Xu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, Jiewen Yao, Gerd Hoffmann, Tom Lendacky

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

CcProbeLib is used to probe the Confidential Computing guest type.
This library is designed to run on SEC / PEI / DXE phases. A null
instance of the library always returns CCGuestTypeNonEncrypted.
A platform specific CcProbeLib will be implemented, for example,
in OvmfPkg.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 MdePkg/Include/Library/CcProbeLib.h           | 26 +++++++++++++++++++
 .../Library/CcProbeLibNull/CcProbeLibNull.c   | 26 +++++++++++++++++++
 .../Library/CcProbeLibNull/CcProbeLibNull.inf | 21 +++++++++++++++
 MdePkg/MdePkg.dec                             |  5 ++++
 MdePkg/MdePkg.dsc                             |  1 +
 5 files changed, 79 insertions(+)
 create mode 100644 MdePkg/Include/Library/CcProbeLib.h
 create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
 create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf

diff --git a/MdePkg/Include/Library/CcProbeLib.h b/MdePkg/Include/Library/CcProbeLib.h
new file mode 100644
index 000000000000..2857dddfb2d3
--- /dev/null
+++ b/MdePkg/Include/Library/CcProbeLib.h
@@ -0,0 +1,26 @@
+/** @file
+
+Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CC_PROBE_LIB_H_
+#define CC_PROBE_LIB_H_
+
+#include <ConfidentialComputingGuestAttr.h>
+
+/**
+  Probe the ConfidentialComputing Guest type. See defition of
+  CC_GUEST_TYPE in <ConfidentialComputingGuestAttr.h>.
+
+  @return The guest type
+
+**/
+UINT8
+EFIAPI
+CcProbe (
+  VOID
+  );
+
+#endif
diff --git a/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c b/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
new file mode 100644
index 000000000000..ca4111dc3077
--- /dev/null
+++ b/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
@@ -0,0 +1,26 @@
+/** @file
+
+  Null stub of CcProbeLib
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/CcProbeLib.h>
+
+/**
+  Probe the ConfidentialComputing Guest type. See defition of
+  CC_GUEST_TYPE in <ConfidentialComputingGuestAttr.h>.
+
+  @return The guest type
+
+**/
+UINT8
+EFIAPI
+CcProbe (
+  VOID
+  )
+{
+  return CcGuestTypeNonEncrypted;
+}
diff --git a/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf b/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
new file mode 100644
index 000000000000..f37c25f73439
--- /dev/null
+++ b/MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
@@ -0,0 +1,21 @@
+## @file
+# CcProbeLib null instance.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = CcProbeLibNull
+  FILE_GUID                      = B15D67FE-0DAC-4316-8E26-8A6b85E43782
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = CcProbeLib
+
+[Sources]
+  CcProbeLibNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 1934c9840423..faeb28c80cbd 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -267,6 +267,11 @@
   #
   RegisterFilterLib|Include/Library/RegisterFilterLib.h
 
+  ##  @libraryclass  This library provides interfances to probe ConfidentialComputing guest type.
+  #
+  #
+  CcProbeLib|Include/Library/CcProbeLib.h
+
 [LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64]
   ##  @libraryclass  Provides services to generate random number.
   #
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index d6a7af412be7..c8d282882ec1 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -130,6 +130,7 @@
   MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
 
   MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
+  MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
 
 [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
   #
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 4/7] OvmfPkg: Add CcProbeLib
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
                   ` (2 preceding siblings ...)
  2022-04-19  0:26 ` [PATCH V4 3/7] MdePkg: Add CcProbeLibNull Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  0:26 ` [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc Min Xu
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, James Bottomley, Jiewen Yao, Gerd Hoffmann,
	Brijesh Singh, Erdem Aktas, Tom Lendacky

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

This is the OvmfPkg specific CcProbeLib. It checks the Ovmf WorkArea
(PcdOvmfWorkAreaBase) to return the guest type.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Library/CcProbeLib/CcProbeLib.c   | 31 +++++++++++++++++++++++
 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf | 25 ++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.c
 create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf

diff --git a/OvmfPkg/Library/CcProbeLib/CcProbeLib.c b/OvmfPkg/Library/CcProbeLib/CcProbeLib.c
new file mode 100644
index 000000000000..d698e5c8d7f8
--- /dev/null
+++ b/OvmfPkg/Library/CcProbeLib/CcProbeLib.c
@@ -0,0 +1,31 @@
+/** @file
+
+  CcProbeLib is used to probe the Confidential computing guest type.
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/CcProbeLib.h>
+#include <WorkArea.h>
+
+/**
+  Probe the ConfidentialComputing Guest type. See defition of
+  CC_GUEST_TYPE in <ConfidentialComputingGuestAttr.h>.
+
+  @return The guest type
+
+**/
+UINT8
+EFIAPI
+CcProbe (
+  VOID
+  )
+{
+  OVMF_WORK_AREA  *WorkArea;
+
+  WorkArea = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+
+  return WorkArea != NULL ? WorkArea->Header.GuestType : CcGuestTypeNonEncrypted;
+}
diff --git a/OvmfPkg/Library/CcProbeLib/CcProbeLib.inf b/OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
new file mode 100644
index 000000000000..5300c9ba2644
--- /dev/null
+++ b/OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
@@ -0,0 +1,25 @@
+## @file
+# CcProbeLib is used to probe Confidential Computing guest type.
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = CcProbeLib
+  FILE_GUID                      = 05184ec9-abb0-4491-8584-e388639a7c48
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = CcProbeLib
+
+[Sources]
+  CcProbeLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
                   ` (3 preceding siblings ...)
  2022-04-19  0:26 ` [PATCH V4 4/7] OvmfPkg: Add CcProbeLib Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19 13:16   ` Lendacky, Thomas
  2022-04-19  0:26 ` [PATCH V4 6/7] MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev Min Xu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, James Bottomley, James Bottomley, Jiewen Yao,
	Gerd Hoffmann, Brijesh Singh, Erdem Aktas, Tom Lendacky

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

CcProbeLib is imported in BaseIoLibIntrinsicSev.
OvmfPkg/Library/CcProbeLib is the OvmfPkg version which checks
OvmfWorkArea to return the Cc guest type. It is included
in OvmfPkgX64.dsc and IntelTdx/IntelTdxX64.dsc.

Other .dsc include the MdePkg/Library/CcProbeLibNull because Cc guest
is not supported in those projects.

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc     | 1 +
 OvmfPkg/Bhyve/BhyveX64.dsc       | 1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc   | 1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
 OvmfPkg/Microvm/MicrovmX64.dsc   | 1 +
 OvmfPkg/OvmfPkgIa32.dsc          | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc       | 1 +
 OvmfPkg/OvmfPkgX64.dsc           | 1 +
 OvmfPkg/OvmfXen.dsc              | 1 +
 9 files changed, 9 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index fcdc3efab204..1c088f25fa4b 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -149,6 +149,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index e1b6b8e15f36..a8fa4d38ab60 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -146,6 +146,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 20f3bc340807..d1c85f60c768 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -158,6 +158,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 245155d41b30..73a6c30096a8 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -135,6 +135,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 59580ccd4691..c9c843e116a9 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -156,6 +156,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index e4218b01f0fc..9e4ecd21fb17 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -154,6 +154,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index a80cdaacb8bc..75fb8095eec2 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -158,6 +158,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index fb2899f8a1be..2e534d0d2478 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -165,6 +165,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 7bd594c6e263..8d420cf54371 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -147,6 +147,7 @@
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 6/7] MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
                   ` (4 preceding siblings ...)
  2022-04-19  0:26 ` [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  0:26 ` [PATCH V4 7/7] OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled Min Xu
  2022-04-19  1:28 ` [PATCH V4 0/7] Introduce CcProbe in MdePkg Yao, Jiewen
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu,
	James Bottomley, James Bottomley, Jiewen Yao, Gerd Hoffmann,
	Tom Lendacky

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

Bad IO performance in SEC phase is observed after TDX features was
introduced. (after commit b6b2de884864 - "MdePkg: Support mmio for
Tdx guest in BaseIoLibIntrinsic").

This is because IsTdxGuest() will be called in each MMIO operation.
It is trying to cache the result of the probe in the efi data segment.
However, that doesn't work in SEC, because the data segment is read only
(so the write seems to succeed but a read will always return the
original value), leading to us calling TdIsEnabled() check for every
mmio we do, which is causing the slowdown because it's very expensive.

This patch is to call CcProbe instead of TdIsEnabled in IsTdxGuest.
Null instance of CcProbe always returns CCGuestTypeNonEncrypted. Its
OvmfPkg version returns the guest type in Ovmf work area.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf    |  1 +
 .../Library/BaseIoLibIntrinsic/IoLibInternalTdx.c   | 13 ++-----------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
index 7fe1c60f046e..e1b8298ac451 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
@@ -55,6 +55,7 @@
   DebugLib
   BaseLib
   RegisterFilterLib
+  CcProbeLib
 
 [LibraryClasses.X64]
   TdxLib
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c
index 1e539dbfbbad..ec837f5eb03e 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c
@@ -10,6 +10,7 @@
 #include <Include/IndustryStandard/Tdx.h>
 #include <Library/TdxLib.h>
 #include <Register/Intel/Cpuid.h>
+#include <Library/CcProbeLib.h>
 #include "IoLibTdx.h"
 
 // Size of TDVMCALL Access, including IO and MMIO
@@ -22,9 +23,6 @@
 #define TDVMCALL_ACCESS_READ   0
 #define TDVMCALL_ACCESS_WRITE  1
 
-BOOLEAN  mTdxEnabled = FALSE;
-BOOLEAN  mTdxProbed  = FALSE;
-
 /**
   Check if it is Tdx guest.
 
@@ -38,14 +36,7 @@ IsTdxGuest (
   VOID
   )
 {
-  if (mTdxProbed) {
-    return mTdxEnabled;
-  }
-
-  mTdxEnabled = TdIsEnabled ();
-  mTdxProbed  = TRUE;
-
-  return mTdxEnabled;
+  return CcProbe () == CcGuestTypeIntelTdx;
 }
 
 /**
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V4 7/7] OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
                   ` (5 preceding siblings ...)
  2022-04-19  0:26 ` [PATCH V4 6/7] MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev Min Xu
@ 2022-04-19  0:26 ` Min Xu
  2022-04-19  1:28 ` [PATCH V4 0/7] Introduce CcProbe in MdePkg Yao, Jiewen
  7 siblings, 0 replies; 11+ messages in thread
From: Min Xu @ 2022-04-19  0:26 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas, Tom Lendacky

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

TdIsEnabled() uses the CPUID instruction. At this point, exception
handling is not established and a CPUID instruction will generate
a #VC and cause the booting guest to crash.

CcProbe() checks Ovmf work area to return the guest type. So call
of CcProbe() instead of TdIsEnabled() to fix the above issue.

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/IntelTdx/Sec/SecMain.c   | 6 +++---
 OvmfPkg/IntelTdx/Sec/SecMain.inf | 1 +
 OvmfPkg/Sec/SecMain.c            | 5 +++--
 OvmfPkg/Sec/SecMain.inf          | 1 +
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/IntelTdx/Sec/SecMain.c b/OvmfPkg/IntelTdx/Sec/SecMain.c
index 26d56be335e1..ab01ec9ab19c 100644
--- a/OvmfPkg/IntelTdx/Sec/SecMain.c
+++ b/OvmfPkg/IntelTdx/Sec/SecMain.c
@@ -25,7 +25,7 @@
 #include <Library/CpuExceptionHandlerLib.h>
 #include <IndustryStandard/Tdx.h>
 #include <Library/PlatformInitLib.h>
-
+#include <Library/CcProbeLib.h>
 #include <Library/PeilessStartupLib.h>
 
 #define SEC_IDT_ENTRY_COUNT  34
@@ -61,7 +61,7 @@ SecCoreStartupWithStack (
   UINT32                Index;
   volatile UINT8        *Table;
 
-  if (TdIsEnabled ()) {
+  if (CcProbe () == CcGuestTypeIntelTdx) {
     //
     // For Td guests, the memory map info is in TdHobLib. It should be processed
     // first so that the memory is accepted. Otherwise access to the unaccepted
@@ -119,7 +119,7 @@ SecCoreStartupWithStack (
   //
   AsmWriteIdtr (&IdtDescriptor);
 
-  if (TdIsEnabled ()) {
+  if (CcProbe () == CcGuestTypeIntelTdx) {
     //
     // InitializeCpuExceptionHandlers () should be called in Td guests so that
     // #VE exceptions can be handled correctly.
diff --git a/OvmfPkg/IntelTdx/Sec/SecMain.inf b/OvmfPkg/IntelTdx/Sec/SecMain.inf
index df2e749c3505..9cf1249d02e5 100644
--- a/OvmfPkg/IntelTdx/Sec/SecMain.inf
+++ b/OvmfPkg/IntelTdx/Sec/SecMain.inf
@@ -49,6 +49,7 @@
   CpuExceptionHandlerLib
   PeilessStartupLib
   PlatformInitLib
+  CcProbeLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index ca9717a7b526..8949d1015708 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -28,6 +28,7 @@
 #include <Library/CpuExceptionHandlerLib.h>
 #include <Ppi/TemporaryRamSupport.h>
 #include <Library/PlatformInitLib.h>
+#include <Library/CcProbeLib.h>
 #include "AmdSev.h"
 
 #define SEC_IDT_ENTRY_COUNT  34
@@ -738,7 +739,7 @@ SecCoreStartupWithStack (
   volatile UINT8        *Table;
 
  #if defined (TDX_GUEST_SUPPORTED)
-  if (TdIsEnabled ()) {
+  if (CcProbe () == CcGuestTypeIntelTdx) {
     //
     // For Td guests, the memory map info is in TdHobLib. It should be processed
     // first so that the memory is accepted. Otherwise access to the unaccepted
@@ -828,7 +829,7 @@ SecCoreStartupWithStack (
   }
 
  #if defined (TDX_GUEST_SUPPORTED)
-  if (TdIsEnabled ()) {
+  if (CcProbe () == CcGuestTypeIntelTdx) {
     //
     // InitializeCpuExceptionHandlers () should be called in Td guests so that
     // #VE exceptions can be handled correctly.
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 4b5b089ccd69..27100595aeca 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -54,6 +54,7 @@
   LocalApicLib
   MemEncryptSevLib
   CpuExceptionHandlerLib
+  CcProbeLib
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V4 0/7] Introduce CcProbe in MdePkg
  2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
                   ` (6 preceding siblings ...)
  2022-04-19  0:26 ` [PATCH V4 7/7] OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled Min Xu
@ 2022-04-19  1:28 ` Yao, Jiewen
  7 siblings, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2022-04-19  1:28 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Kinney, Michael D, Gao, Liming, Liu, Zhiguang, James Bottomley,
	Gerd Hoffmann, Brijesh Singh, Aktas, Erdem, Tom Lendacky

Thanks Min.

Merged - https://github.com/tianocore/edk2/pull/2790

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, April 19, 2022 8:26 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; James Bottomley <jejb@linux.ibm.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Tom Lendacky <thomas.lendacky@amd.com>
> Subject: [PATCH V4 0/7] Introduce CcProbe in MdePkg
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
> 
> Bad IO performance in SEC phase is observed after TDX features was
> introduced. (after commit b6b2de884864 - "MdePkg: Support mmio for
> Tdx guest in BaseIoLibIntrinsic").
> 
> This is because IsTdxGuest() will be called in each MMIO operation.
> It is trying to cache the result of the probe in the efi data segment.
> However, that doesn't work in SEC, because the data segment is read only
> (so the write seems to succeed but a read will always return the
> original value), leading to us calling TdIsEnabled() check for every
> mmio we do, which is causing the slowdown because it's very expensive.
> 
> CcProbe is introduced in this patch-set. It is called in
> BaseIoLibIntrinsicSev instead of IsTdxGuest. There are 2 versions of
> the CcProbeLib. Null instance of CcProbe always returns
> CCGuestTypeNonEncrypted. Its OvmfPkg version checks the Ovmf work area
> and returns the CC guest type.
> 
> In this patch-set another issue is fixed with CcProbe as well. If the
> working guest is SEV and in the beginning of SecMain.c TdIsEnabled()
> was called. At this point, exception handling is not established and
> a CPUID instruction will generate a #VC and cause the booting SEV guest
> to crash. Patch #7 is to fix this broken.
> 
> Code is at: https://github.com/mxu9/edk2/tree/cc_probe.v4
> 
> v4 changes:
>  - Rename the field name in CC_GUEST_TYPE from CCXxx to CcXxx based
>    on the community feedback.
> 
> v3 changes:
>  - Fix the broken issue in SEV guest at SecMain.c. Please refer to
>    Patch #7.
> 
> v2 changes:
>  - Rename TdProbe to CcProbe to make the lib work for Confidential
>    Computing guests.
>  - Rename the GUEST_TYPE to CC_GUEST_TYPE and move it from
>    WorkArea.h@OvmfPkg to ConfidentialComputingGuestAttr.h@MdePkg.
>    This is because CcProbeLib is designed to return the CC Guest
>    type and the lib is located at MdePkg.
>  - Rename the CC_GUEST_TYPE's fields name to Camel style. See the
>    commit message in patch #1.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min Xu (7):
>   MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h
>   OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE
>   MdePkg: Add CcProbeLibNull
>   OvmfPkg: Add CcProbeLib
>   OvmfPkg: Add CcProbeLib in *.dsc
>   MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev
>   OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled
> 
>  .../Include/ConfidentialComputingGuestAttr.h  | 11 ++++++-
>  MdePkg/Include/Library/CcProbeLib.h           | 26 ++++++++++++++++
>  .../BaseIoLibIntrinsicSev.inf                 |  1 +
>  .../BaseIoLibIntrinsic/IoLibInternalTdx.c     | 13 ++------
>  .../Library/CcProbeLibNull/CcProbeLibNull.c   | 26 ++++++++++++++++
>  .../Library/CcProbeLibNull/CcProbeLibNull.inf | 21 +++++++++++++
>  MdePkg/MdePkg.dec                             |  5 +++
>  MdePkg/MdePkg.dsc                             |  1 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |  1 +
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |  1 +
>  OvmfPkg/CloudHv/CloudHvX64.dsc                |  1 +
>  OvmfPkg/Include/WorkArea.h                    |  9 +-----
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  1 +
>  OvmfPkg/IntelTdx/Sec/SecMain.c                |  6 ++--
>  OvmfPkg/IntelTdx/Sec/SecMain.inf              |  1 +
>  .../PeiMemEncryptSevLibInternal.c             |  2 +-
>  .../SecMemEncryptSevLibInternal.c             |  2 +-
>  OvmfPkg/Library/CcProbeLib/CcProbeLib.c       | 31 +++++++++++++++++++
>  OvmfPkg/Library/CcProbeLib/CcProbeLib.inf     | 25 +++++++++++++++
>  OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c   |  2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc                |  1 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |  1 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  1 +
>  OvmfPkg/OvmfXen.dsc                           |  1 +
>  OvmfPkg/Sec/AmdSev.c                          |  2 +-
>  OvmfPkg/Sec/SecMain.c                         |  5 +--
>  OvmfPkg/Sec/SecMain.inf                       |  1 +
>  28 files changed, 170 insertions(+), 29 deletions(-)
>  create mode 100644 MdePkg/Include/Library/CcProbeLib.h
>  create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.c
>  create mode 100644 MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>  create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.c
>  create mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
> 
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc
  2022-04-19  0:26 ` [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc Min Xu
@ 2022-04-19 13:16   ` Lendacky, Thomas
  2022-04-19 13:19     ` Lendacky, Thomas
  0 siblings, 1 reply; 11+ messages in thread
From: Lendacky, Thomas @ 2022-04-19 13:16 UTC (permalink / raw)
  To: Min Xu, devel, James Bottomley
  Cc: James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas

On 4/18/22 19:26, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
> 
> CcProbeLib is imported in BaseIoLibIntrinsicSev.
> OvmfPkg/Library/CcProbeLib is the OvmfPkg version which checks
> OvmfWorkArea to return the Cc guest type. It is included
> in OvmfPkgX64.dsc and IntelTdx/IntelTdxX64.dsc.
> 
> Other .dsc include the MdePkg/Library/CcProbeLibNull because Cc guest
> is not supported in those projects.

Hmm... you missed my comment about OvmfPkgIa32X64.dsc requiring the 
non-NULL library.

Also, I'm not sure that the NULL library is what the AmdSevX64.dsc build 
needs. @James Bottomley, have you tested this series?

Thanks,
Tom

> 
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   OvmfPkg/AmdSev/AmdSevX64.dsc     | 1 +
>   OvmfPkg/Bhyve/BhyveX64.dsc       | 1 +
>   OvmfPkg/CloudHv/CloudHvX64.dsc   | 1 +
>   OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
>   OvmfPkg/Microvm/MicrovmX64.dsc   | 1 +
>   OvmfPkg/OvmfPkgIa32.dsc          | 1 +
>   OvmfPkg/OvmfPkgIa32X64.dsc       | 1 +
>   OvmfPkg/OvmfPkgX64.dsc           | 1 +
>   OvmfPkg/OvmfXen.dsc              | 1 +
>   9 files changed, 9 insertions(+)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index fcdc3efab204..1c088f25fa4b 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -149,6 +149,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index e1b6b8e15f36..a8fa4d38ab60 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -146,6 +146,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
> index 20f3bc340807..d1c85f60c768 100644
> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
> @@ -158,6 +158,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> index 245155d41b30..73a6c30096a8 100644
> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
> @@ -135,6 +135,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 59580ccd4691..c9c843e116a9 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -156,6 +156,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index e4218b01f0fc..9e4ecd21fb17 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -154,6 +154,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index a80cdaacb8bc..75fb8095eec2 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -158,6 +158,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index fb2899f8a1be..2e534d0d2478 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -165,6 +165,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 7bd594c6e263..8d420cf54371 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -147,6 +147,7 @@
>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>     PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>     PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc
  2022-04-19 13:16   ` Lendacky, Thomas
@ 2022-04-19 13:19     ` Lendacky, Thomas
  0 siblings, 0 replies; 11+ messages in thread
From: Lendacky, Thomas @ 2022-04-19 13:19 UTC (permalink / raw)
  To: Min Xu, devel, James Bottomley
  Cc: James Bottomley, Jiewen Yao, Gerd Hoffmann, Brijesh Singh,
	Erdem Aktas

On 4/19/22 08:16, Tom Lendacky wrote:
> On 4/18/22 19:26, Min Xu wrote:
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3902
>>
>> CcProbeLib is imported in BaseIoLibIntrinsicSev.
>> OvmfPkg/Library/CcProbeLib is the OvmfPkg version which checks
>> OvmfWorkArea to return the Cc guest type. It is included
>> in OvmfPkgX64.dsc and IntelTdx/IntelTdxX64.dsc.
>>
>> Other .dsc include the MdePkg/Library/CcProbeLibNull because Cc guest
>> is not supported in those projects.
> 
> Hmm... you missed my comment about OvmfPkgIa32X64.dsc requiring the 
> non-NULL library.
> 
> Also, I'm not sure that the NULL library is what the AmdSevX64.dsc build 
> needs. @James Bottomley, have you tested this series?
> 

Although I guess everything should work since you aren't replacing the 
work area checks in the SEV related files with CcProbe (), only the values 
that are checked against. Sorry for the noise.

Thanks,
Tom

> Thanks,
> Tom
> 
>>
>> Cc: James Bottomley <jejb@linux.ibm.com>
>> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Erdem Aktas <erdemaktas@google.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Min Xu <min.m.xu@intel.com>
>> ---
>>   OvmfPkg/AmdSev/AmdSevX64.dsc     | 1 +
>>   OvmfPkg/Bhyve/BhyveX64.dsc       | 1 +
>>   OvmfPkg/CloudHv/CloudHvX64.dsc   | 1 +
>>   OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
>>   OvmfPkg/Microvm/MicrovmX64.dsc   | 1 +
>>   OvmfPkg/OvmfPkgIa32.dsc          | 1 +
>>   OvmfPkg/OvmfPkgIa32X64.dsc       | 1 +
>>   OvmfPkg/OvmfPkgX64.dsc           | 1 +
>>   OvmfPkg/OvmfXen.dsc              | 1 +
>>   9 files changed, 9 insertions(+)
>>
>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> index fcdc3efab204..1c088f25fa4b 100644
>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
>> @@ -149,6 +149,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>> index e1b6b8e15f36..a8fa4d38ab60 100644
>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>> @@ -146,6 +146,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc 
>> b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> index 20f3bc340807..d1c85f60c768 100644
>> --- a/OvmfPkg/CloudHv/CloudHvX64.dsc
>> +++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
>> @@ -158,6 +158,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc 
>> b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> index 245155d41b30..73a6c30096a8 100644
>> --- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> +++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
>> @@ -135,6 +135,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc 
>> b/OvmfPkg/Microvm/MicrovmX64.dsc
>> index 59580ccd4691..c9c843e116a9 100644
>> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
>> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
>> @@ -156,6 +156,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index e4218b01f0fc..9e4ecd21fb17 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -154,6 +154,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index a80cdaacb8bc..75fb8095eec2 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -158,6 +158,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index fb2899f8a1be..2e534d0d2478 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -165,6 +165,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>> index 7bd594c6e263..8d420cf54371 100644
>> --- a/OvmfPkg/OvmfXen.dsc
>> +++ b/OvmfPkg/OvmfXen.dsc
>> @@ -147,6 +147,7 @@
>>     PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>     
>> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf 
>>
>>     
>> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
>>     IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>>     
>> OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf 
>>
>>     SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-04-19 13:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-19  0:26 [PATCH V4 0/7] Introduce CcProbe in MdePkg Min Xu
2022-04-19  0:26 ` [PATCH V4 1/7] MdePkg: Add CC_GUEST_TYPE in ConfidentialComputingGuestAttr.h Min Xu
2022-04-19  0:26 ` [PATCH V4 2/7] OvmfPkg: Replace GUEST_TYPE with CC_GUEST_TYPE Min Xu
2022-04-19  0:26 ` [PATCH V4 3/7] MdePkg: Add CcProbeLibNull Min Xu
2022-04-19  0:26 ` [PATCH V4 4/7] OvmfPkg: Add CcProbeLib Min Xu
2022-04-19  0:26 ` [PATCH V4 5/7] OvmfPkg: Add CcProbeLib in *.dsc Min Xu
2022-04-19 13:16   ` Lendacky, Thomas
2022-04-19 13:19     ` Lendacky, Thomas
2022-04-19  0:26 ` [PATCH V4 6/7] MdePkg: Probe Cc guest in BaseIoLibIntrinsicSev Min Xu
2022-04-19  0:26 ` [PATCH V4 7/7] OvmfPkg: Call CcProbe in SecMain.c instead of TsIsEnabled Min Xu
2022-04-19  1:28 ` [PATCH V4 0/7] Introduce CcProbe in MdePkg Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox