[edk2-devel] [PATCH v9 22/32] UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV status

Brijesh Singh via groups.io posted 32 patches 4 years, 4 months ago
There is a newer version of this series
[edk2-devel] [PATCH v9 22/32] UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV status
Posted by Brijesh Singh via groups.io 4 years, 4 months ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Previous commit introduced a generic confidential computing PCD that can
determine whether AMD SEV-ES is enabled. Update the MpInitLib to drop the
PcdSevEsIsEnabled in favor of PcdConfidentialComputingAttr.

Cc: Michael Roth <michael.roth@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  2 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h          | 13 ++++
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  6 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c          | 60 ++++++++++++++++++-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  4 +-
 6 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 6e510aa89120..28764418d7c1 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -73,7 +73,7 @@ [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode                           ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                       ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStatusCheckIntervalInMicroSeconds  ## CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled                          ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                       ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr          ## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 2cbd9b8b8acc..cbc3c1460423 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -63,9 +63,9 @@ [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize         ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode                       ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                   ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled                      ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase                   ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                       ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr      ## CONSUMES
 
 [Ppis]
   gEdkiiPeiShadowMicrocodePpiGuid        ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 3d4446df8ce6..2107f3f705a2 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -33,6 +33,7 @@
 #include <Library/HobLib.h>
 #include <Library/PcdLib.h>
 #include <Library/MicrocodeLib.h>
+#include <ConfidentialComputingGuestAttr.h>
 
 #include <Register/Amd/Fam17Msr.h>
 #include <Register/Amd/Ghcb.h>
@@ -774,5 +775,17 @@ SevEsPlaceApHlt (
   CPU_MP_DATA                *CpuMpData
   );
 
+/**
+ Check if the specified confidential computing attribute is active.
+
+ @retval TRUE   The specified Attr is active.
+ @retval FALSE  The specified Attr is not active.
+**/
+BOOLEAN
+EFIAPI
+ConfidentialComputingGuestHas (
+  CONFIDENTIAL_COMPUTING_GUEST_ATTR     Attr
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 93fc63bf93e3..657a73dca05e 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -93,7 +93,7 @@ GetWakeupBuffer (
   EFI_PHYSICAL_ADDRESS    StartAddress;
   EFI_MEMORY_TYPE         MemoryType;
 
-  if (PcdGetBool (PcdSevEsIsEnabled)) {
+  if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
     MemoryType = EfiReservedMemoryType;
   } else {
     MemoryType = EfiBootServicesData;
@@ -107,7 +107,7 @@ GetWakeupBuffer (
   // LagacyBios driver depends on CPU Arch protocol which guarantees below
   // allocation runs earlier than LegacyBios driver.
   //
-  if (PcdGetBool (PcdSevEsIsEnabled)) {
+  if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
     //
     // SEV-ES Wakeup buffer should be under 0x88000 and under any previous one
     //
@@ -124,7 +124,7 @@ GetWakeupBuffer (
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
-  } else if (PcdGetBool (PcdSevEsIsEnabled)) {
+  } else if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
     //
     // Next SEV-ES wakeup buffer allocation must be below this allocation
     //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 890945bc5994..16a1602c3079 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -295,7 +295,7 @@ GetApLoopMode (
       ApLoopMode = ApInHltLoop;
     }
 
-    if (PcdGetBool (PcdSevEsIsEnabled)) {
+    if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
       //
       // For SEV-ES, force AP in Hlt-loop mode in order to use the GHCB
       // protocol for starting APs
@@ -1046,7 +1046,7 @@ AllocateResetVector (
     // The AP reset stack is only used by SEV-ES guests. Do not allocate it
     // if SEV-ES is not enabled.
     //
-    if (PcdGetBool (PcdSevEsIsEnabled)) {
+    if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
       //
       // Stack location is based on ProcessorNumber, so use the total number
       // of processors for calculating the total stack area.
@@ -1816,7 +1816,7 @@ MpInitLibInitialize (
   CpuMpData->CpuData          = (CPU_AP_DATA *) (CpuMpData + 1);
   CpuMpData->CpuInfoInHob     = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber);
   InitializeSpinLock(&CpuMpData->MpLock);
-  CpuMpData->SevEsIsEnabled = PcdGetBool (PcdSevEsIsEnabled);
+  CpuMpData->SevEsIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
   CpuMpData->SevEsAPBuffer  = (UINTN) -1;
   CpuMpData->GhcbBase       = PcdGet64 (PcdGhcbBase);
 
@@ -2706,3 +2706,57 @@ MpInitLibStartupAllCPUs (
            NULL
            );
 }
+
+/**
+ The function check if the specified Attr is set in the CurrentAttr.
+
+ @retval  TRUE      The specified Attr is set.
+ @retval  FALSE     The specified Attr is not set.
+**/
+STATIC
+BOOLEAN
+AmdMemEncryptionAttrCheck (
+  UINT64                                CurrentAttr,
+  CONFIDENTIAL_COMPUTING_GUEST_ATTR     Attr
+  )
+{
+  switch (Attr) {
+    case CCAttrAmdSev:
+      return CurrentAttr >= CCAttrAmdSev;
+    case CCAttrAmdSevEs:
+      return CurrentAttr >= CCAttrAmdSevEs;
+    case CCAttrAmdSevSnp:
+      return CurrentAttr == CCAttrAmdSevSnp;
+    default:
+      return FALSE;
+  }
+}
+
+/**
+ Check if the specified confidential computing attribute is active.
+
+ @retval TRUE   The specified Attr is active.
+ @retval FALSE  The specified Attr is not active.
+**/
+BOOLEAN
+EFIAPI
+ConfidentialComputingGuestHas (
+  CONFIDENTIAL_COMPUTING_GUEST_ATTR     Attr
+  )
+{
+  UINT64    CurrentAttr;
+
+  //
+  // Get the current CC attribute.
+  //
+  CurrentAttr = PcdGet64 (PcdConfidentialComputingGuestAttr);
+
+  //
+  // If attr is for the AMD group then call AMD specific checks.
+  //
+  if (((CurrentAttr >> 8) & 0xff) == 1) {
+    return AmdMemEncryptionAttrCheck (CurrentAttr, Attr);
+  }
+
+  return (CurrentAttr == Attr);
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 90015c650c68..2f333a00460a 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -222,7 +222,7 @@ GetWakeupBuffer (
         // Need memory under 1MB to be collected here
         //
         WakeupBufferEnd = Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
-        if (PcdGetBool (PcdSevEsIsEnabled) &&
+        if (ConfidentialComputingGuestHas (CCAttrAmdSevEs) &&
             WakeupBufferEnd > mSevEsPeiWakeupBuffer) {
           //
           // SEV-ES Wakeup buffer should be under 1MB and under any previous one
@@ -253,7 +253,7 @@ GetWakeupBuffer (
           DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
                                WakeupBufferStart, WakeupBufferSize));
 
-          if (PcdGetBool (PcdSevEsIsEnabled)) {
+          if (ConfidentialComputingGuestHas (CCAttrAmdSevEs)) {
             //
             // Next SEV-ES wakeup buffer allocation must be below this
             // allocation
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81910): https://edk2.groups.io/g/devel/message/81910
Mute This Topic: https://groups.io/mt/86292908/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 22/32] UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV status
Posted by Gerd Hoffmann 4 years, 3 months ago
On Wed, Oct 13, 2021 at 11:57:03AM -0500, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> Previous commit introduced a generic confidential computing PCD that can
> determine whether AMD SEV-ES is enabled. Update the MpInitLib to drop the
> PcdSevEsIsEnabled in favor of PcdConfidentialComputingAttr.

Is there anything that guarantees MpLib initializes after PlatformPei so
it can depend on the PCD (see discussion in tdx thread) ?

thanks,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81995): https://edk2.groups.io/g/devel/message/81995
Mute This Topic: https://groups.io/mt/86292908/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v9 22/32] UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV status
Posted by Brijesh Singh via groups.io 4 years, 3 months ago
On 10/14/21 1:41 AM, Gerd Hoffmann via groups.io wrote:
> On Wed, Oct 13, 2021 at 11:57:03AM -0500, Brijesh Singh wrote:
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C45efbf71c7bc43fab38408d98eee7260%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637697977052800369%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2M1RTR8HQOAcpxA7tlWs%2B5uF%2Bn1JaLAFvagSn7v4SNw%3D&amp;reserved=0
>>
>> Previous commit introduced a generic confidential computing PCD that can
>> determine whether AMD SEV-ES is enabled. Update the MpInitLib to drop the
>> PcdSevEsIsEnabled in favor of PcdConfidentialComputingAttr.
> Is there anything that guarantees MpLib initializes after PlatformPei so
> it can depend on the PCD (see discussion in tdx thread) ?
At least in the current Ovmf implementation the PlatformPei must
initialize some of the PCDs used by the MpLibInit such as
PcdCpuBootLogicalProcessorNumber and other AP related PCD. PlatformPei
uses the Qemu FW_CFG to get the number of APs etc needed by the
MpLibInit, so, I believe its safe to say that MpLib initializes after
the PlatformPei.
>
> thanks,
>   Gerd
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82055): https://edk2.groups.io/g/devel/message/82055
Mute This Topic: https://groups.io/mt/86292908/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-