public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V4 0/2] Re-design CcProbeLib
@ 2022-08-26 23:07 Min Xu
  2022-08-26 23:07 ` [PATCH V4 1/2] OvmfPkg: Add SecPeiCcProbeLib Min Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Min Xu @ 2022-08-26 23:07 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Gerd Hoffmann, Erdem Aktas, James Bottomley, Jiewen Yao,
	Tom Lendacky, Yuan Yu

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

CcProbeLib once was designed to probe the Confidential Computing guest
type by checking the PcdOvmfWorkArea. But this memory is allocated with
either EfiACPIMemoryNVS or EfiBootServicesData. It cannot be accessed
after ExitBootService. Please see the detailed analysis in BZ#3974.

To fix this issue, CcProbeLib is re-designed as 2 implementation:
 - SecPeiCcProbeLib
 - DxeCcProbeLib

In SecPeiCcProbeLib we check the CC guest type by reading the
PcdOvmfWorkArea. Because it is used in SEC / PEI and we don't worry about
the issues in BZ#3974.

In DxeCcProbeLib we cache the GuestType in Ovmf work area in a global
variable. After that the Guest type is returned with the cached value.
So that we don't need to worry about the access to Ovmf work area after
ExitBootService.

The reason why we probe CC guest type in 2 different ways is the global
varialbe. Global variable cannot be used in SEC/PEI and CcProbe is called
very frequently.

Code: https://github.com/mxu9/edk2/tree/CcProbeLib.BZ3974.v4

v4 changes:
 - Read Cc guest type in both DxeCcProbeLib's constructor and CcProbe. So
   that we guarantee the Cc guest type is read early enough.

v3 changes:
 - Re-design CcProbeLib to 2 implementation: SecPeiCcProbeLib and
   DxeCcProbeLib. The difference between the 2 implementation is the
   cache of the CcGuestType.

v2 changes:
 - Reserve Ovmf work-area as RT_DATA. See
   https://edk2.groups.io/g/devel/message/92599

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Yuan Yu <yuanyu@google.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min M Xu (2):
  OvmfPkg: Add SecPeiCcProbeLib
  OvmfPkg: Update CcProbeLib to DxeCcProbeLib

 OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  3 +-
 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c    | 68 +++++++++++++++++++
 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf  | 26 +++++++
 .../{CcProbeLib.c => SecPeiCcProbeLib.c}      |  0
 .../{CcProbeLib.inf => SecPeiCcProbeLib.inf}  |  8 +--
 OvmfPkg/OvmfPkgX64.dsc                        |  5 +-
 6 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100644 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c
 create mode 100644 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
 rename OvmfPkg/Library/CcProbeLib/{CcProbeLib.c => SecPeiCcProbeLib.c} (100%)
 rename OvmfPkg/Library/CcProbeLib/{CcProbeLib.inf => SecPeiCcProbeLib.inf} (65%)

-- 
2.29.2.windows.2


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

* [PATCH V4 1/2] OvmfPkg: Add SecPeiCcProbeLib
  2022-08-26 23:07 [PATCH V4 0/2] Re-design CcProbeLib Min Xu
@ 2022-08-26 23:07 ` Min Xu
  2022-08-26 23:07 ` [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib Min Xu
  2022-08-29  9:34 ` [PATCH V4 0/2] Re-design CcProbeLib Gerd Hoffmann
  2 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2022-08-26 23:07 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
	Tom Lendacky

From: Min M Xu <min.m.xu@intel.com>

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

SecPeiCcProbeLib is designed to probe the Confidential Computing guest
type in SEC/PEI phase. The CC guest type was set by each CC guest at
the beginning of boot up and saved in PcdOvmfWorkArea.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.c | 31 +++++++++++++++++++
 .../Library/CcProbeLib/SecPeiCcProbeLib.inf   | 25 +++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.c
 create mode 100644 OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf

diff --git a/OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.c b/OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.c
new file mode 100644
index 000000000000..d698e5c8d7f8
--- /dev/null
+++ b/OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.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/SecPeiCcProbeLib.inf b/OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
new file mode 100644
index 000000000000..f63ed71e7c28
--- /dev/null
+++ b/OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.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                      = SecPeiCcProbeLib
+  FILE_GUID                      = 78eb7f2a-a42a-4b01-b160-5a05a0a52bac
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = CcProbeLib|SEC PEIM PEI_CORE
+
+[Sources]
+  SecPeiCcProbeLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
-- 
2.29.2.windows.2


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

* [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib
  2022-08-26 23:07 [PATCH V4 0/2] Re-design CcProbeLib Min Xu
  2022-08-26 23:07 ` [PATCH V4 1/2] OvmfPkg: Add SecPeiCcProbeLib Min Xu
@ 2022-08-26 23:07 ` Min Xu
  2022-08-29 13:36   ` Lendacky, Thomas
  2022-08-29  9:34 ` [PATCH V4 0/2] Re-design CcProbeLib Gerd Hoffmann
  2 siblings, 1 reply; 6+ messages in thread
From: Min Xu @ 2022-08-26 23:07 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Gerd Hoffmann, Erdem Aktas, James Bottomley, Jiewen Yao,
	Tom Lendacky

From: Min M Xu <min.m.xu@intel.com>

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

CcProbeLib once was designed to probe the Confidential Computing guest
type by checking the PcdOvmfWorkArea. But this memory is allocated with
either EfiACPIMemoryNVS or EfiBootServicesData. It cannot be accessed
after ExitBootService. Please see the detailed analysis in BZ#3974.

To fix this issue, CcProbeLib is redesigned as 2 implementation:
 - SecPeiCcProbeLib
 - DxeCcProbeLib

In SecPeiCcProbeLib we check the CC guest type by reading the
PcdOvmfWorkArea. Because it is used in SEC / PEI and we don't worry about
the issues in BZ#3974.

In DxeCcProbeLib we cache the GuestType in Ovmf work area in a variable.
After that the Guest type is returned with the cached value. So that we
don't need to worry about the access to Ovmf work area after
ExitBootService.

To gurantee the GuestType is cached, we read the value in both
constructor and CcProbe. Because in some corner case, the constructor
may be called after CcProbe. For example in MdeModulePkg/Core/Dxe/DxeMain,
BaseDebugLibSerialPortConstructor is called before
DxeCcProbeLibConstructor. While CcProbe () is called in
BaseDebugLibSerialPortConstructor.

The reason why we probe CC guest type in 2 different ways is the global
varialbe. Global variable cannot be used in SEC/PEI and CcProbe is called
very frequently.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  3 +-
 OvmfPkg/Library/CcProbeLib/CcProbeLib.c       | 31 ---------
 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c    | 68 +++++++++++++++++++
 .../{CcProbeLib.inf => DxeCcProbeLib.inf}     |  7 +-
 OvmfPkg/OvmfPkgX64.dsc                        |  5 +-
 5 files changed, 78 insertions(+), 36 deletions(-)
 delete mode 100644 OvmfPkg/Library/CcProbeLib/CcProbeLib.c
 create mode 100644 OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c
 rename OvmfPkg/Library/CcProbeLib/{CcProbeLib.inf => DxeCcProbeLib.inf} (65%)

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 71b1cf8e7090..1a7ecd503e50 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -140,7 +140,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
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
@@ -234,6 +234,7 @@
   HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
   PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
   PeilessStartupLib|OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
diff --git a/OvmfPkg/Library/CcProbeLib/CcProbeLib.c b/OvmfPkg/Library/CcProbeLib/CcProbeLib.c
deleted file mode 100644
index d698e5c8d7f8..000000000000
--- a/OvmfPkg/Library/CcProbeLib/CcProbeLib.c
+++ /dev/null
@@ -1,31 +0,0 @@
-/** @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/DxeCcProbeLib.c b/OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c
new file mode 100644
index 000000000000..868e32bf7f60
--- /dev/null
+++ b/OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.c
@@ -0,0 +1,68 @@
+/** @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 <Uefi/UefiBaseType.h>
+#include <Library/CcProbeLib.h>
+#include <WorkArea.h>
+
+STATIC UINT8    mCcProbeGuestType = 0;
+STATIC BOOLEAN  mCcProbed         = FALSE;
+
+/**
+ * Read the the ConfidentialComputing Guest type from Ovmf work-area.
+ *
+ * @return The ConfidentialComputing Guest type
+ */
+STATIC
+UINT8
+ReadCcGuestType (
+  VOID
+  )
+{
+  OVMF_WORK_AREA  *WorkArea;
+
+  if (!mCcProbed) {
+    WorkArea          = (OVMF_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+    mCcProbeGuestType = WorkArea != NULL ? WorkArea->Header.GuestType : CcGuestTypeNonEncrypted;
+    mCcProbed         = TRUE;
+  }
+
+  return mCcProbeGuestType;
+}
+
+/**
+  Probe the ConfidentialComputing Guest type. See defition of
+  CC_GUEST_TYPE in <ConfidentialComputingGuestAttr.h>.
+
+  @return The guest type
+
+**/
+UINT8
+EFIAPI
+CcProbe (
+  VOID
+  )
+{
+  return ReadCcGuestType ();
+}
+
+/**
+ * Constructor of DxeCcProbeLib
+ *
+ * @return EFI_SUCCESS Successfully called of constructor
+ */
+EFI_STATUS
+EFIAPI
+DxeCcProbeLibConstructor (
+  VOID
+  )
+{
+  ReadCcGuestType ();
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/CcProbeLib/CcProbeLib.inf b/OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
similarity index 65%
rename from OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
rename to OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
index 5300c9ba2644..2fd0b4d46d10 100644
--- a/OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
+++ b/OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
@@ -8,14 +8,15 @@
 
 [Defines]
   INF_VERSION                    = 0x00010005
-  BASE_NAME                      = CcProbeLib
+  BASE_NAME                      = DxeCcProbeLib
   FILE_GUID                      = 05184ec9-abb0-4491-8584-e388639a7c48
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = CcProbeLib
+  LIBRARY_CLASS                  = CcProbeLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
+  CONSTRUCTOR                    = DxeCcProbeLibConstructor
 
 [Sources]
-  CcProbeLib.c
+  DxeCcProbeLib.c
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 6e68f60dc90f..2741f4d988b7 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -199,7 +199,7 @@
 
 !if $(SMM_REQUIRE) == FALSE
   LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
-  CcProbeLib|OvmfPkg/Library/CcProbeLib/CcProbeLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/DxeCcProbeLib.inf
 !else
   CcProbeLib|MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
 !endif
@@ -287,6 +287,7 @@
 !endif
   VmgExitLib|OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
   MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
 
 [LibraryClasses.common.PEI_CORE]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
@@ -303,6 +304,7 @@
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 !endif
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
 
 [LibraryClasses.common.PEIM]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
@@ -332,6 +334,7 @@
   PlatformInitLib|OvmfPkg/Library/PlatformInitLib/PlatformInitLib.inf
 
   MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+  CcProbeLib|OvmfPkg/Library/CcProbeLib/SecPeiCcProbeLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
-- 
2.29.2.windows.2


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

* Re: [PATCH V4 0/2] Re-design CcProbeLib
  2022-08-26 23:07 [PATCH V4 0/2] Re-design CcProbeLib Min Xu
  2022-08-26 23:07 ` [PATCH V4 1/2] OvmfPkg: Add SecPeiCcProbeLib Min Xu
  2022-08-26 23:07 ` [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib Min Xu
@ 2022-08-29  9:34 ` Gerd Hoffmann
  2 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2022-08-29  9:34 UTC (permalink / raw)
  To: Min Xu
  Cc: devel, Erdem Aktas, James Bottomley, Jiewen Yao, Tom Lendacky,
	Yuan Yu

On Sat, Aug 27, 2022 at 07:07:06AM +0800, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3974
> 
> CcProbeLib once was designed to probe the Confidential Computing guest
> type by checking the PcdOvmfWorkArea. But this memory is allocated with
> either EfiACPIMemoryNVS or EfiBootServicesData. It cannot be accessed
> after ExitBootService. Please see the detailed analysis in BZ#3974.
> 
> To fix this issue, CcProbeLib is re-designed as 2 implementation:
>  - SecPeiCcProbeLib
>  - DxeCcProbeLib
> 
> In SecPeiCcProbeLib we check the CC guest type by reading the
> PcdOvmfWorkArea. Because it is used in SEC / PEI and we don't worry about
> the issues in BZ#3974.
> 
> In DxeCcProbeLib we cache the GuestType in Ovmf work area in a global
> variable. After that the Guest type is returned with the cached value.
> So that we don't need to worry about the access to Ovmf work area after
> ExitBootService.
> 
> The reason why we probe CC guest type in 2 different ways is the global
> varialbe. Global variable cannot be used in SEC/PEI and CcProbe is called
> very frequently.
> 
> Code: https://github.com/mxu9/edk2/tree/CcProbeLib.BZ3974.v4
> 
> v4 changes:
>  - Read Cc guest type in both DxeCcProbeLib's constructor and CcProbe. So
>    that we guarantee the Cc guest type is read early enough.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd


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

* Re: [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib
  2022-08-26 23:07 ` [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib Min Xu
@ 2022-08-29 13:36   ` Lendacky, Thomas
  2022-08-29 23:57     ` Min Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Lendacky, Thomas @ 2022-08-29 13:36 UTC (permalink / raw)
  To: Min Xu, devel; +Cc: Gerd Hoffmann, Erdem Aktas, James Bottomley, Jiewen Yao

On 8/26/22 18:07, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3974
> 
> CcProbeLib once was designed to probe the Confidential Computing guest
> type by checking the PcdOvmfWorkArea. But this memory is allocated with
> either EfiACPIMemoryNVS or EfiBootServicesData. It cannot be accessed
> after ExitBootService. Please see the detailed analysis in BZ#3974.
> 
> To fix this issue, CcProbeLib is redesigned as 2 implementation:
>   - SecPeiCcProbeLib
>   - DxeCcProbeLib
> 
> In SecPeiCcProbeLib we check the CC guest type by reading the
> PcdOvmfWorkArea. Because it is used in SEC / PEI and we don't worry about
> the issues in BZ#3974.
> 
> In DxeCcProbeLib we cache the GuestType in Ovmf work area in a variable.
> After that the Guest type is returned with the cached value. So that we
> don't need to worry about the access to Ovmf work area after
> ExitBootService.
> 
> To gurantee the GuestType is cached, we read the value in both

s/gurantee/guarantee/

> constructor and CcProbe. Because in some corner case, the constructor
> may be called after CcProbe. For example in MdeModulePkg/Core/Dxe/DxeMain,
> BaseDebugLibSerialPortConstructor is called before
> DxeCcProbeLibConstructor. While CcProbe () is called in
> BaseDebugLibSerialPortConstructor.

Is there a way to put some kind of ordering in place so that CcProbe's 
constructor is called before BaseDebugLibSerialPortConstructor?

> 
> The reason why we probe CC guest type in 2 different ways is the global
> varialbe. Global variable cannot be used in SEC/PEI and CcProbe is called

s/varialbe/variable/

Thanks,
Tom

> very frequently.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---

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

* Re: [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib
  2022-08-29 13:36   ` Lendacky, Thomas
@ 2022-08-29 23:57     ` Min Xu
  0 siblings, 0 replies; 6+ messages in thread
From: Min Xu @ 2022-08-29 23:57 UTC (permalink / raw)
  To: Tom Lendacky, devel@edk2.groups.io
  Cc: Gerd Hoffmann, Aktas, Erdem, James Bottomley, Yao, Jiewen

On August 29, 2022 9:37 PM, Tom Lendacky wrote:
> On 8/26/22 18:07, Min Xu wrote:
> > From: Min M Xu <min.m.xu@intel.com>
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3974
> >
> > To gurantee the GuestType is cached, we read the value in both
> 
> s/gurantee/guarantee/
Thanks for reminder. It will be fixed in the next version.
> 
> > constructor and CcProbe. Because in some corner case, the constructor
> > may be called after CcProbe. For example in
> > MdeModulePkg/Core/Dxe/DxeMain, BaseDebugLibSerialPortConstructor is
> > called before DxeCcProbeLibConstructor. While CcProbe () is called in
> > BaseDebugLibSerialPortConstructor.
> 
> Is there a way to put some kind of ordering in place so that CcProbe's
> constructor is called before BaseDebugLibSerialPortConstructor?
Even there is a way to make the CcProbe's constructor be called before BaseDebugLibSerialPortConstructor, we cannot guarantee it will not be broken in the future. So I think it's safer to read the ovmf work area in both constructor and CcProbe.
> 
> >
> > The reason why we probe CC guest type in 2 different ways is the
> > global varialbe. Global variable cannot be used in SEC/PEI and CcProbe
> > is called
> 
> s/varialbe/variable/
Thanks for reminder. It will be fixed in the next version.

Thanks
Min

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

end of thread, other threads:[~2022-08-29 23:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-26 23:07 [PATCH V4 0/2] Re-design CcProbeLib Min Xu
2022-08-26 23:07 ` [PATCH V4 1/2] OvmfPkg: Add SecPeiCcProbeLib Min Xu
2022-08-26 23:07 ` [PATCH V4 2/2] OvmfPkg: Update CcProbeLib to DxeCcProbeLib Min Xu
2022-08-29 13:36   ` Lendacky, Thomas
2022-08-29 23:57     ` Min Xu
2022-08-29  9:34 ` [PATCH V4 0/2] Re-design CcProbeLib Gerd Hoffmann

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