Re: [edk2-devel] [PATCH v3 03/24] UefiCpuPkg/MpInitLib: Always use AP Create if GhcbApicIds HOB is present

["Ni, Ray" <ray.ni@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 12311 bytes --]

Acked-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io>
Sent: Friday, March 8, 2024 23:30
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas, Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Xu, Min M <min.m.xu@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Michael Roth <michael.roth@amd.com>
Subject: [edk2-devel] [PATCH v3 03/24] UefiCpuPkg/MpInitLib: Always use AP Create if GhcbApicIds HOB is present

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

Currently, the first time an AP is started for an SEV-SNP guest, it relies
on the VMSA as set by the hypervisor. If the list of APIC IDs has been
retrieved, this is not necessary. The list of APIC IDs will be identified
by a GUIDed HOB. If the GUIDed HOB is present, use the SEV-SNP AP Create
protocol to start the AP for the first time and each time thereafter.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 UefiCpuPkg/UefiCpuPkg.dec                     |  5 +-
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  1 +
 UefiCpuPkg/Include/Guid/GhcbApicIds.h         | 17 +++++
 UefiCpuPkg/Library/MpInitLib/MpLib.h          | 15 +++-
 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c    | 21 +++++-
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  9 ++-
 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c     | 78 ++++++++++++++++++--
 8 files changed, 133 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 571b59b36f0a..c31d8b6736cf 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -2,7 +2,7 @@
 # This Package provides UEFI compatible CPU modules and libraries.
 #
 # Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.<BR>
-# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
+# Copyright (C) 2023 - 2024, Advanced Micro Devices, Inc. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -91,6 +91,9 @@ [Guids]
   ## Include/Guid/MpInformation2.h
   gMpInformation2HobGuid         = { 0x417a7f64, 0xf4e9, 0x4b32, {0x84, 0x6a, 0x5c, 0xc4, 0xd8, 0x62, 0x18, 0x79 }}

+  ## Include/Guid/GhcbApicIds.h
+  gGhcbApicIdsGuid               = { 0xbc964338, 0xee39, 0x4fc8, { 0xa2, 0x24, 0x10, 0x10, 0x8b, 0x17, 0x80, 0x1b }}
+
 [Protocols]
   ## Include/Protocol/SmmCpuService.h
   gEfiSmmCpuServiceProtocolGuid   = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 55e46d4a1fad..69950fcd1289 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -68,6 +68,7 @@ [Guids]
   gEfiEventExitBootServicesGuid                 ## CONSUMES  ## Event
   gEfiEventLegacyBootGuid                       ## SOMETIMES_CONSUMES  ## Event
   gEdkiiMicrocodePatchHobGuid                   ## SOMETIMES_CONSUMES  ## HOB
+  gGhcbApicIdsGuid                              ## SOMETIMES_CONSUMES  ## HOB

 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber            ## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index bc3d716aa951..22f74a814534 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -76,3 +76,4 @@ [Ppis]
 [Guids]
   gEdkiiS3SmmInitDoneGuid
   gEdkiiMicrocodePatchHobGuid
+  gGhcbApicIdsGuid                       ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Include/Guid/GhcbApicIds.h b/UefiCpuPkg/Include/Guid/GhcbApicIds.h
new file mode 100644
index 000000000000..9d5bfcb0de22
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/GhcbApicIds.h
@@ -0,0 +1,17 @@
+/** @file
+  APIC ID list retrieved for an SEV-ES/SEV-SNP guest via the GHCB.
+
+  Copyright (C) 2024, Advanced Micro Devices, Inc. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef GHCB_APIC_IDS_H_
+#define GHCB_APIC_IDS_H_
+
+#define GHCB_APIC_IDS_GUID \
+  { 0xbc964338, 0xee39, 0x4fc8, { 0xa2, 0x24, 0x10, 0x10, 0x8b, 0x17, 0x80, 0x1b }}
+
+extern EFI_GUID  gGhcbApicIdsGuid;
+
+#endif
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index d26035559f22..65e05c4806f5 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -2,7 +2,7 @@
   Common header file for MP Initialize Library.

   Copyright (c) 2016 - 2023, Intel Corporation. All rights reserved.<BR>
-  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+  Copyright (c) 2020 - 2024, AMD Inc. All rights reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -924,6 +924,19 @@ SevSnpCreateAP (
   IN INTN         ProcessorNumber
   );

+/**
+  Determine if the SEV-SNP AP Create protocol should be used.
+
+  @param[in]  CpuMpData  Pointer to CPU MP Data
+
+  @retval     TRUE       Use SEV-SNP AP Create protocol
+  @retval     FALSE      Do not use SEV-SNP AP Create protocol
+**/
+BOOLEAN
+CanUseSevSnpCreateAP (
+  IN  CPU_MP_DATA  *CpuMpData
+  );
+
 /**
   Get pointer to CPU MP Data structure from GUIDed HOB.

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
index c83144285b68..0478e92317f1 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
@@ -2,7 +2,7 @@

   AMD SEV helper function.

-  Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
+  Copyright (c) 2021 - 2024, AMD Incorporated. All rights reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -68,3 +68,22 @@ SevSnpRmpAdjust (
   //
   return RETURN_UNSUPPORTED;
 }
+
+/**
+  Determine if the SEV-SNP AP Create protocol should be used.
+
+  @param[in]  CpuMpData  Pointer to CPU MP Data
+
+  @retval     TRUE       Use SEV-SNP AP Create protocol
+  @retval     FALSE      Do not use SEV-SNP AP Create protocol
+**/
+BOOLEAN
+CanUseSevSnpCreateAP (
+  IN  CPU_MP_DATA  *CpuMpData
+  )
+{
+  //
+  // SEV-SNP is not supported on 32-bit build.
+  //
+  return FALSE;
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9bac62f289e0..d7244565029d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2,7 +2,7 @@
   CPU MP Initialize Library common functions.

   Copyright (c) 2016 - 2022, Intel Corporation. All rights reserved.<BR>
-  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+  Copyright (c) 2020 - 2024, AMD Inc. All rights reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -1303,9 +1303,10 @@ WakeUpAP (
       //
       // Wakeup all APs
       //   Must use the INIT-SIPI-SIPI method for initial configuration in
-      //   order to obtain the APIC ID.
+      //   order to obtain the APIC ID if not an SEV-SNP guest and the
+      //   list of APIC IDs is not available.
       //
-      if (CpuMpData->SevSnpIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
+      if (CanUseSevSnpCreateAP (CpuMpData)) {
         SevSnpCreateAP (CpuMpData, -1);
       } else {
         if ((CpuMpData->InitFlag == ApInitConfig) && FixedPcdGetBool (PcdFirstTimeWakeUpAPsBySipi)) {
@@ -1415,7 +1416,7 @@ WakeUpAP (
         SetSevEsJumpTable (ExchangeInfo->BufferStart);
       }

-      if (CpuMpData->SevSnpIsEnabled && (CpuMpData->InitFlag != ApInitConfig)) {
+      if (CanUseSevSnpCreateAP (CpuMpData)) {
         SevSnpCreateAP (CpuMpData, (INTN)ProcessorNumber);
       } else {
         SendInitSipiSipi (
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
index c9f0984f41a2..bd12a5ee2fcb 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
@@ -2,7 +2,7 @@

   AMD SEV helper function.

-  Copyright (c) 2021, AMD Incorporated. All rights reserved.<BR>
+  Copyright (c) 2021 - 2024, AMD Incorporated. All rights reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -268,20 +268,55 @@ SevSnpCreateAP (
   IN INTN         ProcessorNumber
   )
 {
-  CPU_INFO_IN_HOB  *CpuInfoInHob;
-  CPU_AP_DATA      *CpuData;
-  UINTN            Index;
-  UINT32           ApicId;
+  CPU_INFO_IN_HOB    *CpuInfoInHob;
+  CPU_AP_DATA        *CpuData;
+  UINTN              Index;
+  UINTN              MaxIndex;
+  UINT32             ApicId;
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  GHCB_APIC_IDS      *GhcbApicIds;

   ASSERT (CpuMpData->MpCpuExchangeInfo->BufferStart < 0x100000);

   CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;

   if (ProcessorNumber < 0) {
-    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+    if (CpuMpData->InitFlag == ApInitConfig) {
+      //
+      // APs have not been started, so CpuCount is not "known" yet. Use the
+      // retrieved APIC IDs to start the APs and fill out the MpLib CPU
+      // information properly. CanUseSevSnpCreateAP() guarantees we have a
+      // HOB when InitFlag is ApInitConfig.
+      //
+      GuidHob     = GetFirstGuidHob (&gGhcbApicIdsGuid);
+      GhcbApicIds = (GHCB_APIC_IDS *)(*(UINTN *)GET_GUID_HOB_DATA (GuidHob));
+      MaxIndex    = MIN (GhcbApicIds->NumEntries, PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
+    } else {
+      //
+      // APs have been previously started.
+      //
+      MaxIndex = CpuMpData->CpuCount;
+    }
+
+    for (Index = 0; Index < MaxIndex; Index++) {
       if (Index != CpuMpData->BspNumber) {
         CpuData = &CpuMpData->CpuData[Index];
-        ApicId  = CpuInfoInHob[Index].ApicId,
+
+        if (CpuMpData->InitFlag == ApInitConfig) {
+          ApicId = GhcbApicIds->ApicIds[Index];
+
+          //
+          // For the first boot, use the BSP register information.
+          //
+          CopyMem (
+            &CpuData->VolatileRegisters,
+            &CpuMpData->CpuData[0].VolatileRegisters,
+            sizeof (CpuData->VolatileRegisters)
+            );
+        } else {
+          ApicId = CpuInfoInHob[Index].ApicId;
+        }
+
         SevSnpCreateSaveArea (CpuMpData, CpuData, ApicId);
       }
     }
@@ -325,3 +360,32 @@ SevSnpRmpAdjust (

   return AsmRmpAdjust ((UINT64)PageAddress, 0, Rdx);
 }
+
+/**
+  Determine if the SEV-SNP AP Create protocol should be used.
+
+  @param[in]  CpuMpData  Pointer to CPU MP Data
+
+  @retval     TRUE       Use SEV-SNP AP Create protocol
+  @retval     FALSE      Do not use SEV-SNP AP Create protocol
+**/
+BOOLEAN
+CanUseSevSnpCreateAP (
+  IN  CPU_MP_DATA  *CpuMpData
+  )
+{
+  //
+  // The AP Create protocol is used for an SEV-SNP guest if
+  //   - The initial configuration has been performed already or
+  //   - The APIC IDs GUIDed HOB is non-zero.
+  //
+  if (!CpuMpData->SevSnpIsEnabled) {
+    return FALSE;
+  }
+
+  if ((CpuMpData->InitFlag == ApInitConfig) && (GetFirstGuidHob (&gGhcbApicIdsGuid) == NULL)) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
--
2.43.2








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117345): https://edk2.groups.io/g/devel/message/117345
Mute This Topic: https://groups.io/mt/104810684/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 20290 bytes --]