public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
@ 2022-05-07  1:36 Min Xu
  2022-05-07  1:36 ` [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib" Min Xu
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
scenario. This patch-set is to fix this issue.

As commit 88da06ca describes TDVF BSP and APs are simplied and it can
simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This
is done by setting different FILE_GUID to these drivers (of the same
name). In the other hand, we import a set of MpInitLibDepLib. These
libs simply depend on the PPI/Protocols. While these PPI/Protocols are
installed according to the guest type.

This patch-set is a replacement of
https://edk2.groups.io/g/devel/message/89381. Please see the dicussion in
 - https://edk2.groups.io/g/devel/message/89382
 - https://edk2.groups.io/g/devel/message/89455
 - https://edk2.groups.io/g/devel/message/89522
 - https://edk2.groups.io/g/devel/message/89535

The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2

v2 changes:
 - Remove the un-used FILE_GUID definitions.
 - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in DispatchTable.
 - Add more comments.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min M Xu (4):
  UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
  OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
  OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
  OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers

Min Xu (2):
  OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
  OvmfPkg: Add MpInitLibDepLib

 OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
 .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
 OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
 OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
 .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27 +++++
 .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27 +++++
 .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23 ++++
 .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27 +++++
 .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
 OvmfPkg/OvmfPkg.dec                           |   5 +
 OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
 OvmfPkg/OvmfPkgX64.fdf                        |   4 +
 OvmfPkg/Sec/SecMain.c                         |  34 +++++-
 OvmfPkg/Sec/SecMain.inf                       |   2 +
 OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
 OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
 22 files changed, 343 insertions(+), 314 deletions(-)
 create mode 100644 OvmfPkg/Include/Ppi/MpInitLibDep.h
 create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c

-- 
2.29.2.windows.2


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

* [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  5:30   ` Ni, Ray
  2022-05-07  1:36 ` [PATCH V2 2/6] OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions Min Xu
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

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

This reverts commit 88da06ca763eb6514565c1867a801a427c1f3447.
This commit triggers the ASSERT in Non-Td guest.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
 6 files changed, 5 insertions(+), 308 deletions(-)
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
 delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 159b4d16ed0e..e1cd0b350008 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -24,12 +24,10 @@
 [Sources.IA32]
   Ia32/AmdSev.c
   Ia32/MpFuncs.nasm
-  MpLibTdxNull.c
 
 [Sources.X64]
   X64/AmdSev.c
   X64/MpFuncs.nasm
-  MpLibTdx.c
 
 [Sources.common]
   AmdSev.c
@@ -38,7 +36,6 @@
   MpLib.c
   MpLib.h
   Microcode.c
-  MpIntelTdx.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
deleted file mode 100644
index 8a26f6c19fc4..000000000000
--- a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
+++ /dev/null
@@ -1,69 +0,0 @@
-/** @file
-  CPU MP Initialize Library header file for Td guest.
-
-  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef MP_INTEL_TDX_H_
-#define MP_INTEL_TDX_H_
-
-#include <PiPei.h>
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-#include <Uefi/UefiBaseType.h>
-#include <Protocol/MpService.h>
-
-/**
-  Gets detailed MP-related information on the requested processor at the
-  instant this call is made. This service may only be called from the BSP.
-
-  @param[in]  ProcessorNumber       The handle number of processor.
-  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
-                                    the requested processor is deposited.
-  @param[out]  HealthData            Return processor health data.
-
-  @retval EFI_SUCCESS             Processor information was returned.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
-  @retval EFI_NOT_FOUND           The processor with the handle specified by
-                                  ProcessorNumber does not exist in the platform.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetProcessorInfo (
-  IN  UINTN                      ProcessorNumber,
-  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
-  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
-  );
-
-/**
-  Retrieves the number of logical processor in the platform and the number of
-  those logical processors that are enabled on this boot. This service may only
-  be called from the BSP.
-
-  @param[out] NumberOfProcessors          Pointer to the total number of logical
-                                          processors in the system, including the BSP
-                                          and disabled APs.
-  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
-                                          processors that exist in system, including
-                                          the BSP.
-
-  @retval EFI_SUCCESS             The number of logical processors and enabled
-                                  logical processors was retrieved.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
-                                  is NULL.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetNumberOfProcessors (
-  OUT UINTN *NumberOfProcessors, OPTIONAL
-  OUT UINTN *NumberOfEnabledProcessors  OPTIONAL
-  );
-
-#endif
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 91c7afaeb2ad..4a73787ee43a 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -9,11 +9,9 @@
 **/
 
 #include "MpLib.h"
-#include "MpIntelTdx.h"
 #include <Library/VmgExitLib.h>
 #include <Register/Amd/Fam17Msr.h>
 #include <Register/Amd/Ghcb.h>
-#include <ConfidentialComputingGuestAttr.h>
 
 EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
 
@@ -1805,10 +1803,6 @@ MpInitLibInitialize (
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return EFI_SUCCESS;
-  }
-
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
     MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
@@ -2079,10 +2073,6 @@ MpInitLibGetProcessorInfo (
   CPU_INFO_IN_HOB  *CpuInfoInHob;
   UINTN            OriginalProcessorNumber;
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return TdxMpInitLibGetProcessorInfo (ProcessorNumber, ProcessorInfoBuffer, HealthData);
-  }
-
   CpuMpData    = GetCpuMpData ();
   CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
 
@@ -2177,10 +2167,6 @@ SwitchBSPWorker (
   BOOLEAN                      OldInterruptState;
   BOOLEAN                      OldTimerInterruptState;
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return EFI_UNSUPPORTED;
-  }
-
   //
   // Save and Disable Local APIC timer interrupt
   //
@@ -2321,10 +2307,6 @@ EnableDisableApWorker (
   CPU_MP_DATA  *CpuMpData;
   UINTN        CallerNumber;
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return EFI_UNSUPPORTED;
-  }
-
   CpuMpData = GetCpuMpData ();
 
   //
@@ -2385,11 +2367,6 @@ MpInitLibWhoAmI (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    *ProcessorNumber = 0;
-    return EFI_SUCCESS;
-  }
-
   CpuMpData = GetCpuMpData ();
 
   return GetProcessorNumber (CpuMpData, ProcessorNumber);
@@ -2428,16 +2405,12 @@ MpInitLibGetNumberOfProcessors (
   UINTN        EnabledProcessorNumber;
   UINTN        Index;
 
+  CpuMpData = GetCpuMpData ();
+
   if ((NumberOfProcessors == NULL) && (NumberOfEnabledProcessors == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return TdxMpInitLibGetNumberOfProcessors (NumberOfProcessors, NumberOfEnabledProcessors);
-  }
-
-  CpuMpData = GetCpuMpData ();
-
   //
   // Check whether caller processor is BSP
   //
@@ -2517,16 +2490,13 @@ StartupAllCPUsWorker (
   BOOLEAN      HasEnabledAp;
   CPU_STATE    ApState;
 
+  CpuMpData = GetCpuMpData ();
+
   if (FailedCpuList != NULL) {
     *FailedCpuList = NULL;
   }
 
-  Status = MpInitLibGetNumberOfProcessors (&ProcessorCount, NULL);
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  if ((ProcessorCount == 1) && ExcludeBsp) {
+  if ((CpuMpData->CpuCount == 1) && ExcludeBsp) {
     return EFI_NOT_STARTED;
   }
 
@@ -2534,22 +2504,6 @@ StartupAllCPUsWorker (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    //
-    // For Td guest ExcludeBsp must be FALSE. Otherwise it will return in above checks.
-    //
-    ASSERT (!ExcludeBsp);
-
-    //
-    // Start BSP.
-    //
-    Procedure (ProcedureArgument);
-
-    return EFI_SUCCESS;
-  }
-
-  CpuMpData = GetCpuMpData ();
-
   //
   // Check whether caller processor is BSP
   //
@@ -2689,13 +2643,6 @@ StartupThisAPWorker (
   CPU_AP_DATA  *CpuData;
   UINTN        CallerNumber;
 
-  //
-  // In Td guest, startup of AP is not supported in current stage.
-  //
-  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
-    return EFI_UNSUPPORTED;
-  }
-
   CpuMpData = GetCpuMpData ();
 
   if (Finished != NULL) {
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
deleted file mode 100644
index fdb58fba9323..000000000000
--- a/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/** @file
-  CPU MP Initialize Library common functions for Td guest.
-
-  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "MpLib.h"
-#include "MpIntelTdx.h"
-
-/**
-  Gets detailed MP-related information on the requested processor at the
-  instant this call is made. This service may only be called from the BSP.
-
-  In current stage only the BSP is workable. So ProcessorNumber should be 0.
-
-  @param[in]  ProcessorNumber       The handle number of processor.
-  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
-                                    the requested processor is deposited.
-  @param[out]  HealthData            Return processor health data.
-
-  @retval EFI_SUCCESS             Processor information was returned.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL or ProcessorNumber is not 0.
-  @retval EFI_NOT_FOUND           The processor with the handle specified by
-                                  ProcessorNumber does not exist in the platform.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetProcessorInfo (
-  IN  UINTN                      ProcessorNumber,
-  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
-  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
-  )
-{
-  UINTN  OriginalProcessorNumber;
-
-  //
-  // Lower 24 bits contains the actual processor number.
-  //
-  OriginalProcessorNumber = ProcessorNumber;
-  ProcessorNumber        &= BIT24 - 1;
-
-  if ((ProcessorInfoBuffer == NULL) || (ProcessorNumber != 0)) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  ProcessorInfoBuffer->ProcessorId = 0;
-  ProcessorInfoBuffer->StatusFlag  = PROCESSOR_AS_BSP_BIT | PROCESSOR_ENABLED_BIT;
-  ZeroMem (&ProcessorInfoBuffer->Location, sizeof (EFI_CPU_PHYSICAL_LOCATION));
-
-  if ((OriginalProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) {
-    ZeroMem (&ProcessorInfoBuffer->ExtendedInformation.Location2, sizeof (EFI_CPU_PHYSICAL_LOCATION2));
-  }
-
-  if (HealthData != NULL) {
-    HealthData->Uint32 = 0;
-  }
-
-  return EFI_SUCCESS;
-}
-
-/**
-  Retrieves the number of logical processor in the platform and the number of
-  those logical processors that are enabled on this boot. This service may only
-  be called from the BSP.
-
-  @param[out] NumberOfProcessors          Pointer to the total number of logical
-                                          processors in the system, including the BSP
-                                          and disabled APs.
-  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
-                                          processors that exist in system, including
-                                          the BSP.
-
-  @retval EFI_SUCCESS             The number of logical processors and enabled
-                                  logical processors was retrieved.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
-                                  is NULL.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetNumberOfProcessors (
-  OUT UINTN *NumberOfProcessors, OPTIONAL
-  OUT UINTN *NumberOfEnabledProcessors OPTIONAL
-  )
-{
-  ASSERT (NumberOfProcessors != NULL || NumberOfEnabledProcessors != NULL);
-  //
-  // In current stage only the BSP is workable. So NumberOfProcessors
-  // & NumberOfEnableddProcessors are both 1.
-  //
-  if (NumberOfProcessors != NULL) {
-    *NumberOfProcessors = 1;
-  }
-
-  if (NumberOfEnabledProcessors != NULL) {
-    *NumberOfEnabledProcessors = 1;
-  }
-
-  return EFI_SUCCESS;
-}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
deleted file mode 100644
index b5aaf6df283f..000000000000
--- a/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
+++ /dev/null
@@ -1,69 +0,0 @@
-/** @file
-  CPU MP Initialize Library common functions (NULL instance) for Td guest.
-
-  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#include "MpLib.h"
-#include "MpIntelTdx.h"
-
-/**
-  Gets detailed MP-related information on the requested processor at the
-  instant this call is made. This service may only be called from the BSP.
-
-  @param[in]  ProcessorNumber       The handle number of processor.
-  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
-                                    the requested processor is deposited.
-  @param[out]  HealthData            Return processor health data.
-
-  @retval EFI_SUCCESS             Processor information was returned.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
-  @retval EFI_NOT_FOUND           The processor with the handle specified by
-                                  ProcessorNumber does not exist in the platform.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetProcessorInfo (
-  IN  UINTN                      ProcessorNumber,
-  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
-  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
-  )
-{
-  ASSERT (FALSE);
-  return EFI_UNSUPPORTED;
-}
-
-/**
-  Retrieves the number of logical processor in the platform and the number of
-  those logical processors that are enabled on this boot. This service may only
-  be called from the BSP.
-
-  @param[out] NumberOfProcessors          Pointer to the total number of logical
-                                          processors in the system, including the BSP
-                                          and disabled APs.
-  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
-                                          processors that exist in system, including
-                                          the BSP.
-
-  @retval EFI_SUCCESS             The number of logical processors and enabled
-                                  logical processors was retrieved.
-  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
-  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
-                                  is NULL.
-  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
-
-**/
-EFI_STATUS
-TdxMpInitLibGetNumberOfProcessors (
-  OUT UINTN *NumberOfProcessors, OPTIONAL
-  OUT UINTN                     *NumberOfEnabledProcessors OPTIONAL
-  )
-{
-  ASSERT (FALSE);
-  return EFI_UNSUPPORTED;
-}
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 894be0f8daab..5facf4db9499 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -24,12 +24,10 @@
 [Sources.IA32]
   Ia32/AmdSev.c
   Ia32/MpFuncs.nasm
-  MpLibTdxNull.c
 
 [Sources.X64]
   X64/AmdSev.c
   X64/MpFuncs.nasm
-  MpLibTdx.c
 
 [Sources.common]
   AmdSev.c
@@ -38,7 +36,6 @@
   MpLib.c
   MpLib.h
   Microcode.c
-  MpIntelTdx.h
 
 [Packages]
   MdePkg/MdePkg.dec
-- 
2.29.2.windows.2


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

* [PATCH V2 2/6] OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
  2022-05-07  1:36 ` [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib" Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  1:36 ` [PATCH V2 3/6] OvmfPkg: Add MpInitLibDepLib Min Xu
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

MpInitLibDepLib is a set of libraries which depend on PPI/Protocol.
This patch defines the related PPI/Protocols in OvmfPkg.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Include/Ppi/MpInitLibDep.h            | 28 +++++++++++++++++++
 .../Include/Protocol/MpInitLibDepProtocols.h  | 28 +++++++++++++++++++
 OvmfPkg/OvmfPkg.dec                           |  5 ++++
 3 files changed, 61 insertions(+)
 create mode 100644 OvmfPkg/Include/Ppi/MpInitLibDep.h
 create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h

diff --git a/OvmfPkg/Include/Ppi/MpInitLibDep.h b/OvmfPkg/Include/Ppi/MpInitLibDep.h
new file mode 100644
index 000000000000..232ff52e19fe
--- /dev/null
+++ b/OvmfPkg/Include/Ppi/MpInitLibDep.h
@@ -0,0 +1,28 @@
+/** @file
+  MpInitLibDepLib PPI definitions
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MPINITLIB_DEP_H_
+#define MPINITLIB_DEP_H_
+
+// {138F9CF4-F0E7-4721-8F49-F5FFECF42D40}
+#define EFI_PEI_MPINITLIB_MP_DEP_PPI_GUID \
+{ \
+  0x138f9cf4, 0xf0e7, 0x4721, { 0x8f, 0x49, 0xf5, 0xff, 0xec, 0xf4, 0x2d, 0x40 } \
+};
+
+extern EFI_GUID  gEfiPeiMpInitLibMpDepPpiGuid;
+
+// {0B590774-BC67-49F4-A7DB-E82E89E6B5D6}
+#define EFI_PEI_MPINITLIB_UP_DEP_PPI_GUID \
+{ \
+  0xb590774, 0xbc67, 0x49f4, { 0xa7, 0xdb, 0xe8, 0x2e, 0x89, 0xe6, 0xb5, 0xd6 } \
+};
+
+extern EFI_GUID  gEfiPeiMpInitLibUpDepPpiGuid;
+
+#endif
diff --git a/OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h b/OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
new file mode 100644
index 000000000000..449c8fedb3c6
--- /dev/null
+++ b/OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
@@ -0,0 +1,28 @@
+/** @file
+  MpInitLibDep Protocol Guid definitions
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MPINITLIB_DEP_PROTOCOLS_H_
+#define MPINITLIB_DEP_PROTOCOLS_H_
+
+// {BB00A5CA-08CE-462F-A537-43C74A825CA4}
+#define EFI_MPINITLIB_MP_DEP_PROTOCOL_GUID \
+{ \
+  0xbb00a5ca, 0x8ce, 0x462f, { 0xa5, 0x37, 0x43, 0xc7, 0x4a, 0x82, 0x5c, 0xa4 } \
+};
+
+extern EFI_GUID  gEfiMpInitLibMpDepProtocolGuid;
+
+// {A9E7CEF1-5682-42CC-B123-9930973F4A9F}
+#define EFI_PEI_MPINITLIB_UP_DEP_PPI_GUID \
+{ \
+  0xa9e7cef1, 0x5682, 0x42cc, { 0xb1, 0x23, 0x99, 0x30, 0x97, 0x3f, 0x4a, 0x9f } \
+};
+
+extern EFI_GUID  gEfiMpInitLibUpDepProtocolGuid;
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index b9ca44120289..8c2048051bea 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -152,6 +152,9 @@
   # the PEI phase, regardless of memory encryption
   gOvmfTpmMmioAccessiblePpiGuid         = {0x35c84ff2, 0x7bfe, 0x453d, {0x84, 0x5f, 0x68, 0x3a, 0x49, 0x2c, 0xf7, 0xb7}}
 
+  gEfiPeiMpInitLibMpDepPpiGuid          = {0x138f9cf4, 0xf0e7, 0x4721, { 0x8f, 0x49, 0xf5, 0xff, 0xec, 0xf4, 0x2d, 0x40}}
+  gEfiPeiMpInitLibUpDepPpiGuid          = {0xb590774, 0xbc67, 0x49f4, { 0xa7, 0xdb, 0xe8, 0x2e, 0x89, 0xe6, 0xb5, 0xd6}}
+
 [Protocols]
   gVirtioDeviceProtocolGuid             = {0xfa920010, 0x6785, 0x4941, {0xb6, 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
   gXenBusProtocolGuid                   = {0x3d3ca290, 0xb9a5, 0x11e3, {0xb7, 0x5d, 0xb8, 0xac, 0x6f, 0x7d, 0x65, 0xe6}}
@@ -167,6 +170,8 @@
   gEfiVgaMiniPortProtocolGuid           = {0xc7735a2f, 0x88f5, 0x4882, {0xae, 0x63, 0xfa, 0xac, 0x8c, 0x8b, 0x86, 0xb3}}
   gOvmfLoadedX86LinuxKernelProtocolGuid = {0xa3edc05d, 0xb618, 0x4ff6, {0x95, 0x52, 0x76, 0xd7, 0x88, 0x63, 0x43, 0xc8}}
   gQemuAcpiTableNotifyProtocolGuid      = {0x928939b2, 0x4235, 0x462f, {0x95, 0x80, 0xf6, 0xa2, 0xb2, 0xc2, 0x1a, 0x4f}}
+  gEfiMpInitLibMpDepProtocolGuid        = {0xbb00a5ca, 0x8ce,  0x462f, {0xa5, 0x37, 0x43, 0xc7, 0x4a, 0x82, 0x5c, 0xa4}}
+  gEfiMpInitLibUpDepProtocolGuid        = {0xa9e7cef1, 0x5682, 0x42cc, {0xb1, 0x23, 0x99, 0x30, 0x97, 0x3f, 0x4a, 0x9f}}
 
 [PcdsFixedAtBuild]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
-- 
2.29.2.windows.2


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

* [PATCH V2 3/6] OvmfPkg: Add MpInitLibDepLib
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
  2022-05-07  1:36 ` [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib" Min Xu
  2022-05-07  1:36 ` [PATCH V2 2/6] OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  1:36 ` [PATCH V2 4/6] OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c Min Xu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

There are 4 MpInitLibDepLib:
 - PeiMpInitLibMpDepLib:
   MpInitLib multi-processor dependency
 - PeiMpInitLibUpDepLib:
   MpInitLib unique-processor dependency
 - DxeMpInitLibMpDepLib:
   MpInitLib multi-processor dependency
 - DxeMpInitLibUpDepLib
   MpInitLib unique-processor dependency

The Pei libs depend on the corresponding PPI. The Dxe libs depend on the
corresponding Protocol.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  | 27 +++++++++++++++++++
 .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  | 27 +++++++++++++++++++
 .../Library/MpInitLibDepLib/MpInitLibDepLib.c | 23 ++++++++++++++++
 .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  | 27 +++++++++++++++++++
 .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  | 27 +++++++++++++++++++
 5 files changed, 131 insertions(+)
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
 create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf

diff --git a/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf b/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
new file mode 100644
index 000000000000..97a8a52d4c29
--- /dev/null
+++ b/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
@@ -0,0 +1,27 @@
+## @file
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeMpInitLibMpDepLib
+  FILE_GUID                      = 57461928-290D-4FEC-A439-377420A829BE
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL
+
+[LibraryClasses]
+  BaseLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Sources]
+  MpInitLibDepLib.c
+
+[Depex]
+  gEfiMpInitLibMpDepProtocolGuid
\ No newline at end of file
diff --git a/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf b/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
new file mode 100644
index 000000000000..1241fa5de2fa
--- /dev/null
+++ b/OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
@@ -0,0 +1,27 @@
+## @file
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeMpInitLibUpDepLib
+  FILE_GUID                      = 95FA4B7B-930E-4755-A9B7-10F0716DA374
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL
+
+[LibraryClasses]
+  BaseLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Sources]
+  MpInitLibDepLib.c
+
+[Depex]
+  gEfiMpInitLibUpDepProtocolGuid
\ No newline at end of file
diff --git a/OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c b/OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
new file mode 100644
index 000000000000..a7501bd9d960
--- /dev/null
+++ b/OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
@@ -0,0 +1,23 @@
+/** @file
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+
+/**
+This is null constructor which always return EFI_SUCCESS.
+@param  ImageHandle   The firmware allocated handle for the EFI image.
+@param  SystemTable   A pointer to the EFI System Table.
+@retval EFI_SUCCESS   Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+MpInitLibDepContructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf b/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
new file mode 100644
index 000000000000..3a3c24ecd142
--- /dev/null
+++ b/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
@@ -0,0 +1,27 @@
+## @file
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = PeiMpInitLibMpDepLib
+  FILE_GUID                      = D14271DE-FBEA-4AAC-9633-7143DCD7C1C8
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL
+
+[LibraryClasses]
+  BaseLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Sources]
+  MpInitLibDepLib.c
+
+[Depex]
+  gEfiPeiMpInitLibMpDepPpiGuid
\ No newline at end of file
diff --git a/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf b/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
new file mode 100644
index 000000000000..4a55a242a6f1
--- /dev/null
+++ b/OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
@@ -0,0 +1,27 @@
+## @file
+#
+#  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = MpInitLibUpDepLib
+  FILE_GUID                      = C64B5035-FA3D-4215-ADBF-9C9F3F458E30
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL
+
+[LibraryClasses]
+  BaseLib
+
+[Packages]
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[Sources]
+  MpInitLibDepLib.c
+
+[Depex]
+  gEfiPeiMpInitLibUpDepPpiGuid
\ No newline at end of file
-- 
2.29.2.windows.2


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

* [PATCH V2 4/6] OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
                   ` (2 preceding siblings ...)
  2022-05-07  1:36 ` [PATCH V2 3/6] OvmfPkg: Add MpInitLibDepLib Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  1:36 ` [PATCH V2 5/6] OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols Min Xu
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

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

Td guest should use MpInitLibUp, other guest use the MpInitLib. So
in SecMain.c different PPI is installed according to the working
guest type.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Sec/SecMain.c   | 34 ++++++++++++++++++++++++++++++++--
 OvmfPkg/Sec/SecMain.inf |  2 ++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 89371361cd05..1167d22a68cc 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -28,6 +28,7 @@
 #include <Library/LocalApicLib.h>
 #include <Library/CpuExceptionHandlerLib.h>
 #include <Ppi/TemporaryRamSupport.h>
+#include <Ppi/MpInitLibDep.h>
 #include <Library/PlatformInitLib.h>
 #include <Library/CcProbeLib.h>
 #include "AmdSev.h"
@@ -61,12 +62,30 @@ EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI  mTemporaryRamSupportPpi = {
   TemporaryRamMigration
 };
 
-EFI_PEI_PPI_DESCRIPTOR  mPrivateDispatchTable[] = {
+EFI_PEI_PPI_DESCRIPTOR  mPrivateDispatchTableMp[] = {
+  {
+    (EFI_PEI_PPI_DESCRIPTOR_PPI),
+    &gEfiTemporaryRamSupportPpiGuid,
+    &mTemporaryRamSupportPpi
+  },
   {
     (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+    &gEfiPeiMpInitLibMpDepPpiGuid,
+    NULL
+  },
+};
+
+EFI_PEI_PPI_DESCRIPTOR  mPrivateDispatchTableUp[] = {
+  {
+    (EFI_PEI_PPI_DESCRIPTOR_PPI),
     &gEfiTemporaryRamSupportPpiGuid,
     &mTemporaryRamSupportPpi
   },
+  {
+    (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+    &gEfiPeiMpInitLibUpDepPpiGuid,
+    NULL
+  },
 };
 
 //
@@ -936,6 +955,7 @@ SecStartupPhase2 (
   EFI_SEC_PEI_HAND_OFF        *SecCoreData;
   EFI_FIRMWARE_VOLUME_HEADER  *BootFv;
   EFI_PEI_CORE_ENTRY_POINT    PeiCoreEntryPoint;
+  EFI_PEI_PPI_DESCRIPTOR      *EfiPeiPpiDescriptor;
 
   SecCoreData = (EFI_SEC_PEI_HAND_OFF *)Context;
 
@@ -948,10 +968,20 @@ SecStartupPhase2 (
   SecCoreData->BootFirmwareVolumeBase = BootFv;
   SecCoreData->BootFirmwareVolumeSize = (UINTN)BootFv->FvLength;
 
+  //
+  // Td guest is required to use the MpInitLibUp (unique-processor version).
+  // Other guests use the MpInitLib (multi-processor version).
+  //
+  if (CcProbe () == CcGuestTypeIntelTdx) {
+    EfiPeiPpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)&mPrivateDispatchTableUp;
+  } else {
+    EfiPeiPpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)&mPrivateDispatchTableMp;
+  }
+
   //
   // Transfer the control to the PEI core
   //
-  (*PeiCoreEntryPoint)(SecCoreData, (EFI_PEI_PPI_DESCRIPTOR *)&mPrivateDispatchTable);
+  (*PeiCoreEntryPoint)(SecCoreData, EfiPeiPpiDescriptor);
 
   //
   // If we get here then the PEI Core returned, which is not recoverable.
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 1557b5f4a84e..561a840f29c5 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -59,6 +59,8 @@
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
+  gEfiPeiMpInitLibMpDepPpiGuid
+  gEfiPeiMpInitLibUpDepPpiGuid
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
-- 
2.29.2.windows.2


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

* [PATCH V2 5/6] OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
                   ` (3 preceding siblings ...)
  2022-05-07  1:36 ` [PATCH V2 4/6] OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  1:36 ` [PATCH V2 6/6] OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers Min Xu
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

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

In Td guest CpuDxe driver uses the MpInitLibUp, the other guest type
use the MpInitLib. So we install different Protocols according to
the current working guest type.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/TdxDxe/TdxDxe.c   | 22 +++++++++++++++++++++-
 OvmfPkg/TdxDxe/TdxDxe.inf |  2 ++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/TdxDxe/TdxDxe.c b/OvmfPkg/TdxDxe/TdxDxe.c
index f0929998233c..2318db989792 100644
--- a/OvmfPkg/TdxDxe/TdxDxe.c
+++ b/OvmfPkg/TdxDxe/TdxDxe.c
@@ -23,6 +23,7 @@
 #include <Library/UefiLib.h>
 #include <Library/HobLib.h>
 #include <Protocol/Cpu.h>
+#include <Protocol/MpInitLibDepProtocols.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <ConfidentialComputingGuestAttr.h>
 #include <IndustryStandard/Tdx.h>
@@ -250,13 +251,32 @@ TdxDxeEntryPoint (
 
   if (!TdIsEnabled ()) {
     //
-    // If it is Non-Td guest, we're done.
+    // If it is Non-Td guest, we install gEfiMpInitLibMpDepProtocolGuid so that
+    // MpInitLib will be used in CpuDxe driver.
     //
+    gBS->InstallProtocolInterface (
+           &ImageHandle,
+           &gEfiMpInitLibMpDepProtocolGuid,
+           EFI_NATIVE_INTERFACE,
+           NULL
+           );
+
     return EFI_SUCCESS;
   }
 
   SetMmioSharedBit ();
 
+  //
+  // It is Td guest, we install gEfiMpInitLibUpDepProtocolGuid so that
+  // MpInitLibUp will be used in CpuDxe driver.
+  //
+  gBS->InstallProtocolInterface (
+         &ImageHandle,
+         &gEfiMpInitLibUpDepProtocolGuid,
+         EFI_NATIVE_INTERFACE,
+         NULL
+         );
+
   //
   // Call TDINFO to get actual number of cpus in domain
   //
diff --git a/OvmfPkg/TdxDxe/TdxDxe.inf b/OvmfPkg/TdxDxe/TdxDxe.inf
index 2ec2ef2ed5f2..a7e0abda1522 100644
--- a/OvmfPkg/TdxDxe/TdxDxe.inf
+++ b/OvmfPkg/TdxDxe/TdxDxe.inf
@@ -50,6 +50,8 @@
   gQemuAcpiTableNotifyProtocolGuid                 ## CONSUMES
   gEfiAcpiSdtProtocolGuid                          ## CONSUMES
   gEfiAcpiTableProtocolGuid                        ## CONSUMES
+  gEfiMpInitLibMpDepProtocolGuid
+  gEfiMpInitLibUpDepProtocolGuid
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
-- 
2.29.2.windows.2


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

* [PATCH V2 6/6] OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
                   ` (4 preceding siblings ...)
  2022-05-07  1:36 ` [PATCH V2 5/6] OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols Min Xu
@ 2022-05-07  1:36 ` Min Xu
  2022-05-07  5:30 ` [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Ni, Ray
  2022-05-09 12:44 ` Min Xu
  7 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-07  1:36 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Eric Dong, Ray Ni, Brijesh Singh, Erdem Aktas,
	James Bottomley, Jiewen Yao, Tom Lendacky, Gerd Hoffmann

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

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

In OvmfPkgX64 we enable 2 different CpuMpPei and CpuDxe drivers. The
difference between the drivers is the MpInitLib or MpInitLibUp. This is
acomplished by adding a MpInitLibDepLib.

In IntelTdxX64 we enable 2 versions of CpuDxe drivers. It is because PEI
is skipped in IntelTdxX64.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 30 ++++++++++++++++-
 OvmfPkg/IntelTdx/IntelTdxX64.fdf |  3 ++
 OvmfPkg/OvmfPkgX64.dsc           | 55 ++++++++++++++++++++++++++++++--
 OvmfPkg/OvmfPkgX64.fdf           |  4 +++
 4 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 73a6c30096a8..80c331ea233a 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -57,6 +57,11 @@
 !endif
 !endif
 
+  #
+  # Define the FILE_GUID of CpuDxe for unique-processor version.
+  #
+  DEFINE UP_CPU_DXE_GUID  = 6490f1c5-ebcc-4665-8892-0075b9bb49b7
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
@@ -550,7 +555,30 @@
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
-  UefiCpuPkg/CpuDxe/CpuDxe.inf
+
+  UefiCpuPkg/CpuDxe/CpuDxe.inf {
+    <LibraryClasses>
+      #
+      # Directly use DxeMpInitLib. It depends on DxeMpInitLibMpDepLib which
+      # checks the Protocol of gEfiMpInitLibMpDepProtocolGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
+  }
+
+  UefiCpuPkg/CpuDxe/CpuDxe.inf {
+    <Defines>
+      FILE_GUID = $(UP_CPU_DXE_GUID)
+
+    <LibraryClasses>
+      #
+      # Directly use MpInitLibUp. It depends on DxeMpInitLibUpDepLib which
+      # checks the Protocol of gEfiMpInitLibUpDepProtocolGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
+  }
+
   OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
index 9e290ea78f61..1029916c3484 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
@@ -185,7 +185,10 @@ INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
+
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
+INF  FILE_GUID = $(UP_CPU_DXE_GUID) UefiCpuPkg/CpuDxe/CpuDxe.inf
+
 INF  OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 45ffa2dbe35f..71526bba3183 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -72,6 +72,12 @@
 !endif
 !endif
 
+  #
+  # Define the FILE_GUID of CpuMpPei/CpuDxe for unique-processor version.
+  #
+  DEFINE UP_CPU_PEI_GUID  = 280251c4-1d09-4035-9062-839acb5f18c1
+  DEFINE UP_CPU_DXE_GUID  = 6490f1c5-ebcc-4665-8892-0075b9bb49b7
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
@@ -728,7 +734,29 @@
   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
   OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
-  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+
+  UefiCpuPkg/CpuMpPei/CpuMpPei.inf {
+    <LibraryClasses>
+      #
+      # Directly use PeiMpInitLib. It depends on PeiMpInitLibMpDepLib which
+      # checks the PPI of gEfiPeiMpInitLibMpDepPpiGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
+  }
+
+  UefiCpuPkg/CpuMpPei/CpuMpPei.inf {
+    <Defines>
+      FILE_GUID = $(UP_CPU_PEI_GUID)
+
+    <LibraryClasses>
+      #
+      # Directly use MpInitLibUp. It depends on PeiMpInitLibUpDepLib which
+      # checks the PPI of gEfiPeiMpInitLibUpDepPpiGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
+  }
 
 !include OvmfPkg/OvmfTpmComponentsPei.dsc.inc
 
@@ -760,7 +788,30 @@
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
-  UefiCpuPkg/CpuDxe/CpuDxe.inf
+
+  UefiCpuPkg/CpuDxe/CpuDxe.inf {
+    <LibraryClasses>
+      #
+      # Directly use DxeMpInitLib. It depends on DxeMpInitLibMpDepLib which
+      # checks the Protocol of gEfiMpInitLibMpDepProtocolGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
+  }
+
+  UefiCpuPkg/CpuDxe/CpuDxe.inf {
+    <Defines>
+      FILE_GUID = $(UP_CPU_DXE_GUID)
+
+    <LibraryClasses>
+      #
+      # Directly use MpInitLibUp. It depends on DxeMpInitLibUpDepLib which
+      # checks the Protocol of gEfiMpInitLibUpDepProtocolGuid.
+      #
+      MpInitLib|UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
+      NULL|OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
+  }
+
 !ifdef $(CSM_ENABLE)
   OvmfPkg/8259InterruptControllerDxe/8259.inf
   OvmfPkg/8254TimerDxe/8254Timer.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 6e72cdf3453e..aa9a83032d9b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -185,6 +185,7 @@ INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+INF  FILE_GUID = $(UP_CPU_PEI_GUID) UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 
 !include OvmfPkg/OvmfTpmPei.fdf.inc
 
@@ -239,7 +240,10 @@ INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
+
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
+INF  FILE_GUID = $(UP_CPU_DXE_GUID) UefiCpuPkg/CpuDxe/CpuDxe.inf
+
 !ifdef $(CSM_ENABLE)
   INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
   INF  OvmfPkg/8254TimerDxe/8254Timer.inf
-- 
2.29.2.windows.2


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

* Re: [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
  2022-05-07  1:36 ` [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib" Min Xu
@ 2022-05-07  5:30   ` Ni, Ray
  0 siblings, 0 replies; 18+ messages in thread
From: Ni, Ray @ 2022-05-07  5:30 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Tom Lendacky, Gerd Hoffmann

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

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Saturday, May 7, 2022 9:36 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Aktas, Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
> 
> From: Min M Xu <min.m.xu@intel.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> 
> This reverts commit 88da06ca763eb6514565c1867a801a427c1f3447.
> This commit triggers the ASSERT in Non-Td guest.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
>  UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
>  6 files changed, 5 insertions(+), 308 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index 159b4d16ed0e..e1cd0b350008 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -24,12 +24,10 @@
>  [Sources.IA32]
>    Ia32/AmdSev.c
>    Ia32/MpFuncs.nasm
> -  MpLibTdxNull.c
> 
>  [Sources.X64]
>    X64/AmdSev.c
>    X64/MpFuncs.nasm
> -  MpLibTdx.c
> 
>  [Sources.common]
>    AmdSev.c
> @@ -38,7 +36,6 @@
>    MpLib.c
>    MpLib.h
>    Microcode.c
> -  MpIntelTdx.h
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
> deleted file mode 100644
> index 8a26f6c19fc4..000000000000
> --- a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/** @file
> -  CPU MP Initialize Library header file for Td guest.
> -
> -  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef MP_INTEL_TDX_H_
> -#define MP_INTEL_TDX_H_
> -
> -#include <PiPei.h>
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -#include <Uefi/UefiBaseType.h>
> -#include <Protocol/MpService.h>
> -
> -/**
> -  Gets detailed MP-related information on the requested processor at the
> -  instant this call is made. This service may only be called from the BSP.
> -
> -  @param[in]  ProcessorNumber       The handle number of processor.
> -  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
> -                                    the requested processor is deposited.
> -  @param[out]  HealthData            Return processor health data.
> -
> -  @retval EFI_SUCCESS             Processor information was returned.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
> -  @retval EFI_NOT_FOUND           The processor with the handle specified by
> -                                  ProcessorNumber does not exist in the platform.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetProcessorInfo (
> -  IN  UINTN                      ProcessorNumber,
> -  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
> -  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
> -  );
> -
> -/**
> -  Retrieves the number of logical processor in the platform and the number of
> -  those logical processors that are enabled on this boot. This service may only
> -  be called from the BSP.
> -
> -  @param[out] NumberOfProcessors          Pointer to the total number of logical
> -                                          processors in the system, including the BSP
> -                                          and disabled APs.
> -  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
> -                                          processors that exist in system, including
> -                                          the BSP.
> -
> -  @retval EFI_SUCCESS             The number of logical processors and enabled
> -                                  logical processors was retrieved.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
> -                                  is NULL.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetNumberOfProcessors (
> -  OUT UINTN *NumberOfProcessors, OPTIONAL
> -  OUT UINTN *NumberOfEnabledProcessors  OPTIONAL
> -  );
> -
> -#endif
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 91c7afaeb2ad..4a73787ee43a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -9,11 +9,9 @@
>  **/
> 
>  #include "MpLib.h"
> -#include "MpIntelTdx.h"
>  #include <Library/VmgExitLib.h>
>  #include <Register/Amd/Fam17Msr.h>
>  #include <Register/Amd/Ghcb.h>
> -#include <ConfidentialComputingGuestAttr.h>
> 
>  EFI_GUID  mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
> 
> @@ -1805,10 +1803,6 @@ MpInitLibInitialize (
>    UINTN                    BackupBufferAddr;
>    UINTN                    ApIdtBase;
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return EFI_SUCCESS;
> -  }
> -
>    OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>    if (OldCpuMpData == NULL) {
>      MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
> @@ -2079,10 +2073,6 @@ MpInitLibGetProcessorInfo (
>    CPU_INFO_IN_HOB  *CpuInfoInHob;
>    UINTN            OriginalProcessorNumber;
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return TdxMpInitLibGetProcessorInfo (ProcessorNumber, ProcessorInfoBuffer, HealthData);
> -  }
> -
>    CpuMpData    = GetCpuMpData ();
>    CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
> 
> @@ -2177,10 +2167,6 @@ SwitchBSPWorker (
>    BOOLEAN                      OldInterruptState;
>    BOOLEAN                      OldTimerInterruptState;
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return EFI_UNSUPPORTED;
> -  }
> -
>    //
>    // Save and Disable Local APIC timer interrupt
>    //
> @@ -2321,10 +2307,6 @@ EnableDisableApWorker (
>    CPU_MP_DATA  *CpuMpData;
>    UINTN        CallerNumber;
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return EFI_UNSUPPORTED;
> -  }
> -
>    CpuMpData = GetCpuMpData ();
> 
>    //
> @@ -2385,11 +2367,6 @@ MpInitLibWhoAmI (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    *ProcessorNumber = 0;
> -    return EFI_SUCCESS;
> -  }
> -
>    CpuMpData = GetCpuMpData ();
> 
>    return GetProcessorNumber (CpuMpData, ProcessorNumber);
> @@ -2428,16 +2405,12 @@ MpInitLibGetNumberOfProcessors (
>    UINTN        EnabledProcessorNumber;
>    UINTN        Index;
> 
> +  CpuMpData = GetCpuMpData ();
> +
>    if ((NumberOfProcessors == NULL) && (NumberOfEnabledProcessors == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return TdxMpInitLibGetNumberOfProcessors (NumberOfProcessors, NumberOfEnabledProcessors);
> -  }
> -
> -  CpuMpData = GetCpuMpData ();
> -
>    //
>    // Check whether caller processor is BSP
>    //
> @@ -2517,16 +2490,13 @@ StartupAllCPUsWorker (
>    BOOLEAN      HasEnabledAp;
>    CPU_STATE    ApState;
> 
> +  CpuMpData = GetCpuMpData ();
> +
>    if (FailedCpuList != NULL) {
>      *FailedCpuList = NULL;
>    }
> 
> -  Status = MpInitLibGetNumberOfProcessors (&ProcessorCount, NULL);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  if ((ProcessorCount == 1) && ExcludeBsp) {
> +  if ((CpuMpData->CpuCount == 1) && ExcludeBsp) {
>      return EFI_NOT_STARTED;
>    }
> 
> @@ -2534,22 +2504,6 @@ StartupAllCPUsWorker (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    //
> -    // For Td guest ExcludeBsp must be FALSE. Otherwise it will return in above checks.
> -    //
> -    ASSERT (!ExcludeBsp);
> -
> -    //
> -    // Start BSP.
> -    //
> -    Procedure (ProcedureArgument);
> -
> -    return EFI_SUCCESS;
> -  }
> -
> -  CpuMpData = GetCpuMpData ();
> -
>    //
>    // Check whether caller processor is BSP
>    //
> @@ -2689,13 +2643,6 @@ StartupThisAPWorker (
>    CPU_AP_DATA  *CpuData;
>    UINTN        CallerNumber;
> 
> -  //
> -  // In Td guest, startup of AP is not supported in current stage.
> -  //
> -  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
> -    return EFI_UNSUPPORTED;
> -  }
> -
>    CpuMpData = GetCpuMpData ();
> 
>    if (Finished != NULL) {
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
> deleted file mode 100644
> index fdb58fba9323..000000000000
> --- a/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
> +++ /dev/null
> @@ -1,106 +0,0 @@
> -/** @file
> -  CPU MP Initialize Library common functions for Td guest.
> -
> -  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "MpLib.h"
> -#include "MpIntelTdx.h"
> -
> -/**
> -  Gets detailed MP-related information on the requested processor at the
> -  instant this call is made. This service may only be called from the BSP.
> -
> -  In current stage only the BSP is workable. So ProcessorNumber should be 0.
> -
> -  @param[in]  ProcessorNumber       The handle number of processor.
> -  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
> -                                    the requested processor is deposited.
> -  @param[out]  HealthData            Return processor health data.
> -
> -  @retval EFI_SUCCESS             Processor information was returned.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL or ProcessorNumber is not 0.
> -  @retval EFI_NOT_FOUND           The processor with the handle specified by
> -                                  ProcessorNumber does not exist in the platform.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetProcessorInfo (
> -  IN  UINTN                      ProcessorNumber,
> -  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
> -  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
> -  )
> -{
> -  UINTN  OriginalProcessorNumber;
> -
> -  //
> -  // Lower 24 bits contains the actual processor number.
> -  //
> -  OriginalProcessorNumber = ProcessorNumber;
> -  ProcessorNumber        &= BIT24 - 1;
> -
> -  if ((ProcessorInfoBuffer == NULL) || (ProcessorNumber != 0)) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  ProcessorInfoBuffer->ProcessorId = 0;
> -  ProcessorInfoBuffer->StatusFlag  = PROCESSOR_AS_BSP_BIT | PROCESSOR_ENABLED_BIT;
> -  ZeroMem (&ProcessorInfoBuffer->Location, sizeof (EFI_CPU_PHYSICAL_LOCATION));
> -
> -  if ((OriginalProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) {
> -    ZeroMem (&ProcessorInfoBuffer->ExtendedInformation.Location2, sizeof (EFI_CPU_PHYSICAL_LOCATION2));
> -  }
> -
> -  if (HealthData != NULL) {
> -    HealthData->Uint32 = 0;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Retrieves the number of logical processor in the platform and the number of
> -  those logical processors that are enabled on this boot. This service may only
> -  be called from the BSP.
> -
> -  @param[out] NumberOfProcessors          Pointer to the total number of logical
> -                                          processors in the system, including the BSP
> -                                          and disabled APs.
> -  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
> -                                          processors that exist in system, including
> -                                          the BSP.
> -
> -  @retval EFI_SUCCESS             The number of logical processors and enabled
> -                                  logical processors was retrieved.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
> -                                  is NULL.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetNumberOfProcessors (
> -  OUT UINTN *NumberOfProcessors, OPTIONAL
> -  OUT UINTN *NumberOfEnabledProcessors OPTIONAL
> -  )
> -{
> -  ASSERT (NumberOfProcessors != NULL || NumberOfEnabledProcessors != NULL);
> -  //
> -  // In current stage only the BSP is workable. So NumberOfProcessors
> -  // & NumberOfEnableddProcessors are both 1.
> -  //
> -  if (NumberOfProcessors != NULL) {
> -    *NumberOfProcessors = 1;
> -  }
> -
> -  if (NumberOfEnabledProcessors != NULL) {
> -    *NumberOfEnabledProcessors = 1;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> deleted file mode 100644
> index b5aaf6df283f..000000000000
> --- a/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/** @file
> -  CPU MP Initialize Library common functions (NULL instance) for Td guest.
> -
> -  Copyright (c) 2020 - 2022, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#include "MpLib.h"
> -#include "MpIntelTdx.h"
> -
> -/**
> -  Gets detailed MP-related information on the requested processor at the
> -  instant this call is made. This service may only be called from the BSP.
> -
> -  @param[in]  ProcessorNumber       The handle number of processor.
> -  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information for
> -                                    the requested processor is deposited.
> -  @param[out]  HealthData            Return processor health data.
> -
> -  @retval EFI_SUCCESS             Processor information was returned.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
> -  @retval EFI_NOT_FOUND           The processor with the handle specified by
> -                                  ProcessorNumber does not exist in the platform.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetProcessorInfo (
> -  IN  UINTN                      ProcessorNumber,
> -  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
> -  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> -
> -/**
> -  Retrieves the number of logical processor in the platform and the number of
> -  those logical processors that are enabled on this boot. This service may only
> -  be called from the BSP.
> -
> -  @param[out] NumberOfProcessors          Pointer to the total number of logical
> -                                          processors in the system, including the BSP
> -                                          and disabled APs.
> -  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled logical
> -                                          processors that exist in system, including
> -                                          the BSP.
> -
> -  @retval EFI_SUCCESS             The number of logical processors and enabled
> -                                  logical processors was retrieved.
> -  @retval EFI_DEVICE_ERROR        The calling processor is an AP.
> -  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and NumberOfEnabledProcessors
> -                                  is NULL.
> -  @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> -
> -**/
> -EFI_STATUS
> -TdxMpInitLibGetNumberOfProcessors (
> -  OUT UINTN *NumberOfProcessors, OPTIONAL
> -  OUT UINTN                     *NumberOfEnabledProcessors OPTIONAL
> -  )
> -{
> -  ASSERT (FALSE);
> -  return EFI_UNSUPPORTED;
> -}
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index 894be0f8daab..5facf4db9499 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> @@ -24,12 +24,10 @@
>  [Sources.IA32]
>    Ia32/AmdSev.c
>    Ia32/MpFuncs.nasm
> -  MpLibTdxNull.c
> 
>  [Sources.X64]
>    X64/AmdSev.c
>    X64/MpFuncs.nasm
> -  MpLibTdx.c
> 
>  [Sources.common]
>    AmdSev.c
> @@ -38,7 +36,6 @@
>    MpLib.c
>    MpLib.h
>    Microcode.c
> -  MpIntelTdx.h
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> --
> 2.29.2.windows.2


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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
                   ` (5 preceding siblings ...)
  2022-05-07  1:36 ` [PATCH V2 6/6] OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers Min Xu
@ 2022-05-07  5:30 ` Ni, Ray
  2022-05-09 12:44 ` Min Xu
  7 siblings, 0 replies; 18+ messages in thread
From: Ni, Ray @ 2022-05-07  5:30 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Tom Lendacky, Gerd Hoffmann

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

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Saturday, May 7, 2022 9:36 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Aktas, Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> 
> Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
> scenario. This patch-set is to fix this issue.
> 
> As commit 88da06ca describes TDVF BSP and APs are simplied and it can
> simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
> include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This
> is done by setting different FILE_GUID to these drivers (of the same
> name). In the other hand, we import a set of MpInitLibDepLib. These
> libs simply depend on the PPI/Protocols. While these PPI/Protocols are
> installed according to the guest type.
> 
> This patch-set is a replacement of
> https://edk2.groups.io/g/devel/message/89381. Please see the dicussion in
>  - https://edk2.groups.io/g/devel/message/89382
>  - https://edk2.groups.io/g/devel/message/89455
>  - https://edk2.groups.io/g/devel/message/89522
>  - https://edk2.groups.io/g/devel/message/89535
> 
> The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2
> 
> v2 changes:
>  - Remove the un-used FILE_GUID definitions.
>  - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in DispatchTable.
>  - Add more comments.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min M Xu (4):
>   UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
>   OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
>   OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
>   OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
> 
> Min Xu (2):
>   OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
>   OvmfPkg: Add MpInitLibDepLib
> 
>  OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
>  .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
>  .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27 +++++
>  .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27 +++++
>  .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23 ++++
>  .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27 +++++
>  .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
>  OvmfPkg/OvmfPkg.dec                           |   5 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
>  OvmfPkg/OvmfPkgX64.fdf                        |   4 +
>  OvmfPkg/Sec/SecMain.c                         |  34 +++++-
>  OvmfPkg/Sec/SecMain.inf                       |   2 +
>  OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
>  OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
>  UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
>  22 files changed, 343 insertions(+), 314 deletions(-)
>  create mode 100644 OvmfPkg/Include/Ppi/MpInitLibDep.h
>  create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> 
> --
> 2.29.2.windows.2


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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
                   ` (6 preceding siblings ...)
  2022-05-07  5:30 ` [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Ni, Ray
@ 2022-05-09 12:44 ` Min Xu
  2022-05-09 17:29   ` Lendacky, Thomas
                     ` (3 more replies)
  7 siblings, 4 replies; 18+ messages in thread
From: Min Xu @ 2022-05-09 12:44 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Tom Lendacky, Gerd Hoffmann

Gerd & Tom
What are your comments about this patch-set?

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Saturday, May 7, 2022 9:36 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
> Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> 
> Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
> scenario. This patch-set is to fix this issue.
> 
> As commit 88da06ca describes TDVF BSP and APs are simplied and it can
> simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
> include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This is
> done by setting different FILE_GUID to these drivers (of the same name). In
> the other hand, we import a set of MpInitLibDepLib. These libs simply
> depend on the PPI/Protocols. While these PPI/Protocols are installed
> according to the guest type.
> 
> This patch-set is a replacement of
> https://edk2.groups.io/g/devel/message/89381. Please see the dicussion in
>  - https://edk2.groups.io/g/devel/message/89382
>  - https://edk2.groups.io/g/devel/message/89455
>  - https://edk2.groups.io/g/devel/message/89522
>  - https://edk2.groups.io/g/devel/message/89535
> 
> The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2
> 
> v2 changes:
>  - Remove the un-used FILE_GUID definitions.
>  - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in
> DispatchTable.
>  - Add more comments.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min M Xu (4):
>   UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
>   OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
>   OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
>   OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
> 
> Min Xu (2):
>   OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
>   OvmfPkg: Add MpInitLibDepLib
> 
>  OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
>  .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
>  .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27
> +++++  .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27
> +++++  .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23
> ++++  .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27
> +++++  .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
>  OvmfPkg/OvmfPkg.dec                           |   5 +
>  OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
>  OvmfPkg/OvmfPkgX64.fdf                        |   4 +
>  OvmfPkg/Sec/SecMain.c                         |  34 +++++-
>  OvmfPkg/Sec/SecMain.inf                       |   2 +
>  OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
>  OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
>  UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
>  UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
>  22 files changed, 343 insertions(+), 314 deletions(-)  create mode 100644
> OvmfPkg/Include/Ppi/MpInitLibDep.h
>  create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
>  create mode 100644
> OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
>  create mode 100644
> OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
>  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
>  create mode 100644
> OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
>  create mode 100644
> OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
>  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> 
> --
> 2.29.2.windows.2


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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 12:44 ` Min Xu
@ 2022-05-09 17:29   ` Lendacky, Thomas
  2022-05-09 23:37     ` Min Xu
  2022-05-10  9:26   ` Gerd Hoffmann
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Lendacky, Thomas @ 2022-05-09 17:29 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Gerd Hoffmann

On 5/9/22 07:44, Xu, Min M wrote:
> Gerd & Tom
> What are your comments about this patch-set?

Hi Min,

This appears to resolve the issue. I was able to boot a 64 vCPU guest in 
legacy, SEV, SEV-ES and SEV-SNP modes without any asserts.

I'm assuming that you were able to see the ASSERTs on your end and 
validate, too?

Thanks,
Tom

> 
>> -----Original Message-----
>> From: Xu, Min M <min.m.xu@intel.com>
>> Sent: Saturday, May 7, 2022 9:36 AM
>> To: devel@edk2.groups.io
>> Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
>> Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
>> Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
>> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
>> Subject: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
>>
>> Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
>> scenario. This patch-set is to fix this issue.
>>
>> As commit 88da06ca describes TDVF BSP and APs are simplied and it can
>> simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
>> include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This is
>> done by setting different FILE_GUID to these drivers (of the same name). In
>> the other hand, we import a set of MpInitLibDepLib. These libs simply
>> depend on the PPI/Protocols. While these PPI/Protocols are installed
>> according to the guest type.
>>
>> This patch-set is a replacement of
>> https://edk2.groups.io/g/devel/message/89381
>>   - https://edk2.groups.io/g/devel/message/89382
>>   - https://edk2.groups.io/g/devel/message/89455
>>   - https://edk2.groups.io/g/devel/message/89522
>>   - https://edk2.groups.io/g/devel/message/89535
>>
>> The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2
>>
>> v2 changes:
>>   - Remove the un-used FILE_GUID definitions.
>>   - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in
>> DispatchTable.
>>   - Add more comments.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Min Xu <min.m.xu@intel.com>
>>
>> Min M Xu (4):
>>    UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
>>    OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
>>    OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
>>    OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
>>
>> Min Xu (2):
>>    OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
>>    OvmfPkg: Add MpInitLibDepLib
>>
>>   OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
>>   .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
>>   OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
>>   OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
>>   .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27
>> +++++  .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27
>> +++++  .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23
>> ++++  .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27
>> +++++  .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
>>   OvmfPkg/OvmfPkg.dec                           |   5 +
>>   OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
>>   OvmfPkg/OvmfPkgX64.fdf                        |   4 +
>>   OvmfPkg/Sec/SecMain.c                         |  34 +++++-
>>   OvmfPkg/Sec/SecMain.inf                       |   2 +
>>   OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
>>   OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
>>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
>>   UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
>>   UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
>>   UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
>>   UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
>>   UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
>>   22 files changed, 343 insertions(+), 314 deletions(-)  create mode 100644
>> OvmfPkg/Include/Ppi/MpInitLibDep.h
>>   create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
>>   create mode 100644
>> OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
>>   create mode 100644
>> OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
>>   create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
>>   create mode 100644
>> OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
>>   create mode 100644
>> OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
>>   delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
>>   delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
>>   delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
>>
>> --
>> 2.29.2.windows.2
> 

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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 17:29   ` Lendacky, Thomas
@ 2022-05-09 23:37     ` Min Xu
  2022-05-10 13:22       ` Lendacky, Thomas
  2022-05-10 15:16       ` Lendacky, Thomas
  0 siblings, 2 replies; 18+ messages in thread
From: Min Xu @ 2022-05-09 23:37 UTC (permalink / raw)
  To: Tom Lendacky, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Gerd Hoffmann

On May 10, 2022 1:30 AM, Tom Lendacky wrote:
> 
> On 5/9/22 07:44, Xu, Min M wrote:
> > Gerd & Tom
> > What are your comments about this patch-set?
> 
> Hi Min,
> 
> This appears to resolve the issue. I was able to boot a 64 vCPU guest in
> legacy, SEV, SEV-ES and SEV-SNP modes without any asserts.
> 
> I'm assuming that you were able to see the ASSERTs on your end and
> validate, too?
> 
Yes. I enable a 4 vCPU legacy guest and can see the ASSERTs. But it appears in a random rate so it missed in the CI.

Thanks
Min

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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 12:44 ` Min Xu
  2022-05-09 17:29   ` Lendacky, Thomas
@ 2022-05-10  9:26   ` Gerd Hoffmann
  2022-05-11  0:13     ` Min Xu
  2022-05-11  8:47   ` Yao, Jiewen
  2022-05-11  8:47   ` Yao, Jiewen
  3 siblings, 1 reply; 18+ messages in thread
From: Gerd Hoffmann @ 2022-05-10  9:26 UTC (permalink / raw)
  To: Xu, Min M
  Cc: devel@edk2.groups.io, Dong, Eric, Ni, Ray, Brijesh Singh,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Tom Lendacky

On Mon, May 09, 2022 at 12:44:58PM +0000, Xu, Min M wrote:
> Gerd & Tom
> What are your comments about this patch-set?
> 
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> > 
> > Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
> > scenario. This patch-set is to fix this issue.
> > 
> > As commit 88da06ca describes TDVF BSP and APs are simplied and it can
> > simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
> > include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This is
> > done by setting different FILE_GUID to these drivers (of the same name). In
> > the other hand, we import a set of MpInitLibDepLib. These libs simply
> > depend on the PPI/Protocols. While these PPI/Protocols are installed
> > according to the guest type.

So the idea is to pick the one or the other implementations via guid
and depex dependencies?  The approach looks sane to me.

Assuming the above is correct:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd


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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 23:37     ` Min Xu
@ 2022-05-10 13:22       ` Lendacky, Thomas
  2022-05-10 15:16       ` Lendacky, Thomas
  1 sibling, 0 replies; 18+ messages in thread
From: Lendacky, Thomas @ 2022-05-10 13:22 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Gerd Hoffmann

On 5/9/22 18:37, Xu, Min M wrote:
> On May 10, 2022 1:30 AM, Tom Lendacky wrote:
>>
>> On 5/9/22 07:44, Xu, Min M wrote:
>>> Gerd & Tom
>>> What are your comments about this patch-set?
>>
>> Hi Min,
>>
>> This appears to resolve the issue. I was able to boot a 64 vCPU guest in
>> legacy, SEV, SEV-ES and SEV-SNP modes without any asserts.
>>
>> I'm assuming that you were able to see the ASSERTs on your end and
>> validate, too?
>>
> Yes. I enable a 4 vCPU legacy guest and can see the ASSERTs. But it appears in a random rate so it missed in the CI.

Yeah, with a low number of vCPUs, the crashes were random. When I upped 
the count to 64 vCPUs that could all run in parallel (running on an EPYC 
server box) it happened on (almost) every boot.

But glad that you were able to observe it and create this fix.

Thanks, Min!

Tom

> 
> Thanks
> Min

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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 23:37     ` Min Xu
  2022-05-10 13:22       ` Lendacky, Thomas
@ 2022-05-10 15:16       ` Lendacky, Thomas
  1 sibling, 0 replies; 18+ messages in thread
From: Lendacky, Thomas @ 2022-05-10 15:16 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Yao, Jiewen, Gerd Hoffmann



On 5/9/22 18:37, Xu, Min M wrote:
> On May 10, 2022 1:30 AM, Tom Lendacky wrote:
>>
>> On 5/9/22 07:44, Xu, Min M wrote:
>>> Gerd & Tom
>>> What are your comments about this patch-set?
>>
>> Hi Min,
>>
>> This appears to resolve the issue. I was able to boot a 64 vCPU guest in
>> legacy, SEV, SEV-ES and SEV-SNP modes without any asserts.
>>
>> I'm assuming that you were able to see the ASSERTs on your end and
>> validate, too?
>>
> Yes. I enable a 4 vCPU legacy guest and can see the ASSERTs. But it appears in a random rate so it missed in the CI.

Hmmm... I hadn't noticed it before, but I'm seeing the following message
from the Linux kernel for each AP being brought online:

APIC: Stale IRR: 00000000,00000000,00000000,00000000,00000000,00000000,00000001,00000000 ISR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000

Let me investigate this further to see where this regression was
introduced.

Thanks,
Tom

> 
> Thanks
> Min

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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-10  9:26   ` Gerd Hoffmann
@ 2022-05-11  0:13     ` Min Xu
  0 siblings, 0 replies; 18+ messages in thread
From: Min Xu @ 2022-05-11  0:13 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel@edk2.groups.io, Dong, Eric, Ni, Ray, Brijesh Singh,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Tom Lendacky

On May 10, 2022 5:27 PM, Gerd Hoffmann wrote:
> On Mon, May 09, 2022 at 12:44:58PM +0000, Xu, Min M wrote:
> > Gerd & Tom
> > What are your comments about this patch-set?
> >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> > >
> > > Above BZ reports an issue that commit 88da06ca triggers ASSERT in
> > > some scenario. This patch-set is to fix this issue.
> > >
> > > As commit 88da06ca describes TDVF BSP and APs are simplied and it
> > > can simply use MpInitLibUp instead of MpInitLib. To achieve this
> > > goal, we include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and
> > > IntelTdxX64. This is done by setting different FILE_GUID to these
> > > drivers (of the same name). In the other hand, we import a set of
> > > MpInitLibDepLib. These libs simply depend on the PPI/Protocols.
> > > While these PPI/Protocols are installed according to the guest type.
> 
> So the idea is to pick the one or the other implementations via guid and
> depex dependencies?  The approach looks sane to me.
Yes, it is the idea. In this way we can decouple the Tdx guest from MpInitLib (multi-processor version) in current stage.
 
Thanks
Min

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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 12:44 ` Min Xu
  2022-05-09 17:29   ` Lendacky, Thomas
  2022-05-10  9:26   ` Gerd Hoffmann
@ 2022-05-11  8:47   ` Yao, Jiewen
  2022-05-11  8:47   ` Yao, Jiewen
  3 siblings, 0 replies; 18+ messages in thread
From: Yao, Jiewen @ 2022-05-11  8:47 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Tom Lendacky, Gerd Hoffmann

Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Monday, May 9, 2022 8:45 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Aktas, Erdem <erdemaktas@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: RE: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> 
> Gerd & Tom
> What are your comments about this patch-set?
> 
> > -----Original Message-----
> > From: Xu, Min M <min.m.xu@intel.com>
> > Sent: Saturday, May 7, 2022 9:36 AM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
> > Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> > Subject: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> >
> > Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
> > scenario. This patch-set is to fix this issue.
> >
> > As commit 88da06ca describes TDVF BSP and APs are simplied and it can
> > simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
> > include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This is
> > done by setting different FILE_GUID to these drivers (of the same name). In
> > the other hand, we import a set of MpInitLibDepLib. These libs simply
> > depend on the PPI/Protocols. While these PPI/Protocols are installed
> > according to the guest type.
> >
> > This patch-set is a replacement of
> > https://edk2.groups.io/g/devel/message/89381. Please see the dicussion in
> >  - https://edk2.groups.io/g/devel/message/89382
> >  - https://edk2.groups.io/g/devel/message/89455
> >  - https://edk2.groups.io/g/devel/message/89522
> >  - https://edk2.groups.io/g/devel/message/89535
> >
> > The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2
> >
> > v2 changes:
> >  - Remove the un-used FILE_GUID definitions.
> >  - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in
> > DispatchTable.
> >  - Add more comments.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> >
> > Min M Xu (4):
> >   UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
> >   OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
> >   OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
> >   OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
> >
> > Min Xu (2):
> >   OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
> >   OvmfPkg: Add MpInitLibDepLib
> >
> >  OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
> >  .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
> >  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
> >  OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
> >  .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27
> > +++++  .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27
> > +++++  .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23
> > ++++  .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27
> > +++++  .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
> >  OvmfPkg/OvmfPkg.dec                           |   5 +
> >  OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
> >  OvmfPkg/OvmfPkgX64.fdf                        |   4 +
> >  OvmfPkg/Sec/SecMain.c                         |  34 +++++-
> >  OvmfPkg/Sec/SecMain.inf                       |   2 +
> >  OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
> >  OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
> >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
> >  UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
> >  UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
> >  UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
> >  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
> >  22 files changed, 343 insertions(+), 314 deletions(-)  create mode 100644
> > OvmfPkg/Include/Ppi/MpInitLibDep.h
> >  create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
> >  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> >
> > --
> > 2.29.2.windows.2


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

* Re: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
  2022-05-09 12:44 ` Min Xu
                     ` (2 preceding siblings ...)
  2022-05-11  8:47   ` Yao, Jiewen
@ 2022-05-11  8:47   ` Yao, Jiewen
  3 siblings, 0 replies; 18+ messages in thread
From: Yao, Jiewen @ 2022-05-11  8:47 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Dong, Eric, Ni, Ray, Brijesh Singh, Aktas, Erdem, James Bottomley,
	Tom Lendacky, Gerd Hoffmann

Merged https://github.com/tianocore/edk2/pull/2877



> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Monday, May 9, 2022 8:45 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh
> <brijesh.singh@amd.com>; Aktas, Erdem <erdemaktas@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: RE: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> 
> Gerd & Tom
> What are your comments about this patch-set?
> 
> > -----Original Message-----
> > From: Xu, Min M <min.m.xu@intel.com>
> > Sent: Saturday, May 7, 2022 9:36 AM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M <min.m.xu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Aktas,
> > Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> > <thomas.lendacky@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> > Subject: [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
> >
> > Above BZ reports an issue that commit 88da06ca triggers ASSERT in some
> > scenario. This patch-set is to fix this issue.
> >
> > As commit 88da06ca describes TDVF BSP and APs are simplied and it can
> > simply use MpInitLibUp instead of MpInitLib. To achieve this goal, we
> > include 2 CpuMpPei/CpuDxe drivers in OvmfPkgX64 and IntelTdxX64. This is
> > done by setting different FILE_GUID to these drivers (of the same name). In
> > the other hand, we import a set of MpInitLibDepLib. These libs simply
> > depend on the PPI/Protocols. While these PPI/Protocols are installed
> > according to the guest type.
> >
> > This patch-set is a replacement of
> > https://edk2.groups.io/g/devel/message/89381. Please see the dicussion in
> >  - https://edk2.groups.io/g/devel/message/89382
> >  - https://edk2.groups.io/g/devel/message/89455
> >  - https://edk2.groups.io/g/devel/message/89522
> >  - https://edk2.groups.io/g/devel/message/89535
> >
> > The code is at: https://github.com/mxu9/edk2/tree/Rework-MpInitLib.v2
> >
> > v2 changes:
> >  - Remove the un-used FILE_GUID definitions.
> >  - Delete un-used EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST in
> > DispatchTable.
> >  - Add more comments.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.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: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> >
> > Min M Xu (4):
> >   UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib"
> >   OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c
> >   OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols
> >   OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers
> >
> > Min Xu (2):
> >   OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions
> >   OvmfPkg: Add MpInitLibDepLib
> >
> >  OvmfPkg/Include/Ppi/MpInitLibDep.h            |  28 +++++
> >  .../Include/Protocol/MpInitLibDepProtocols.h  |  28 +++++
> >  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  30 ++++-
> >  OvmfPkg/IntelTdx/IntelTdxX64.fdf              |   3 +
> >  .../MpInitLibDepLib/DxeMpInitLibMpDepLib.inf  |  27
> > +++++  .../MpInitLibDepLib/DxeMpInitLibUpDepLib.inf  |  27
> > +++++  .../Library/MpInitLibDepLib/MpInitLibDepLib.c |  23
> > ++++  .../MpInitLibDepLib/PeiMpInitLibMpDepLib.inf  |  27
> > +++++  .../MpInitLibDepLib/PeiMpInitLibUpDepLib.inf  |  27 +++++
> >  OvmfPkg/OvmfPkg.dec                           |   5 +
> >  OvmfPkg/OvmfPkgX64.dsc                        |  55 ++++++++-
> >  OvmfPkg/OvmfPkgX64.fdf                        |   4 +
> >  OvmfPkg/Sec/SecMain.c                         |  34 +++++-
> >  OvmfPkg/Sec/SecMain.inf                       |   2 +
> >  OvmfPkg/TdxDxe/TdxDxe.c                       |  22 +++-
> >  OvmfPkg/TdxDxe/TdxDxe.inf                     |   2 +
> >  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 -
> >  UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  69 ------------
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c          |  63 +----------
> >  UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 106 ------------------
> >  UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  69 ------------
> >  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 -
> >  22 files changed, 343 insertions(+), 314 deletions(-)  create mode 100644
> > OvmfPkg/Include/Ppi/MpInitLibDep.h
> >  create mode 100644 OvmfPkg/Include/Protocol/MpInitLibDepProtocols.h
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibMpDepLib.inf
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/DxeMpInitLibUpDepLib.inf
> >  create mode 100644 OvmfPkg/Library/MpInitLibDepLib/MpInitLibDepLib.c
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibMpDepLib.inf
> >  create mode 100644
> > OvmfPkg/Library/MpInitLibDepLib/PeiMpInitLibUpDepLib.inf
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
> >  delete mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
> >
> > --
> > 2.29.2.windows.2


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

end of thread, other threads:[~2022-05-11  8:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-07  1:36 [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Min Xu
2022-05-07  1:36 ` [PATCH V2 1/6] UefiCpuPkg: Revert "UefiCpuPkg: Enable Tdx support in MpInitLib" Min Xu
2022-05-07  5:30   ` Ni, Ray
2022-05-07  1:36 ` [PATCH V2 2/6] OvmfPkg: Add MpInitLibDepLib related PPI/Protocol definitions Min Xu
2022-05-07  1:36 ` [PATCH V2 3/6] OvmfPkg: Add MpInitLibDepLib Min Xu
2022-05-07  1:36 ` [PATCH V2 4/6] OvmfPkg/Sec: Install MpInitLibDepLib PPIs in SecMain.c Min Xu
2022-05-07  1:36 ` [PATCH V2 5/6] OvmfPkg/TdxDxe: Install MpInitLibDepLib protocols Min Xu
2022-05-07  1:36 ` [PATCH V2 6/6] OvmfPkg: Enable 2 different CpuMpPei and CpuDxe drivers Min Xu
2022-05-07  5:30 ` [PATCH V2 0/6] Support 2 CpuMpPei/CpuDxe in One image Ni, Ray
2022-05-09 12:44 ` Min Xu
2022-05-09 17:29   ` Lendacky, Thomas
2022-05-09 23:37     ` Min Xu
2022-05-10 13:22       ` Lendacky, Thomas
2022-05-10 15:16       ` Lendacky, Thomas
2022-05-10  9:26   ` Gerd Hoffmann
2022-05-11  0:13     ` Min Xu
2022-05-11  8:47   ` Yao, Jiewen
2022-05-11  8:47   ` Yao, Jiewen

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