RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

In TDVF BSP and APs are simplified. BSP is the vCPU-0, while the others
are treated as APs.

So MP intialization is rather simple. The processor info is retrieved by
TDCALL, ApWorker is not supported, BSP is always the working processor,
while the APs are just in a wait-for-precedure state.

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: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   3 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  15 +-
 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h     |  71 ++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  27 ++++
 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c       | 128 ++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   |  73 ++++++++++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   3 +
 7 files changed, 319 insertions(+), 1 deletion(-)
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index e1cd0b350008..159b4d16ed0e 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -24,10 +24,12 @@
 [Sources.IA32]
   Ia32/AmdSev.c
   Ia32/MpFuncs.nasm
+  MpLibTdxNull.c
 
 [Sources.X64]
   X64/AmdSev.c
   X64/MpFuncs.nasm
+  MpLibTdx.c
 
 [Sources.common]
   AmdSev.c
@@ -36,6 +38,7 @@
   MpLib.c
   MpLib.h
   Microcode.c
+  MpIntelTdx.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 60d14a5a0e10..237cad8c5fd6 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -7,6 +7,7 @@
 **/
 
 #include "MpLib.h"
+#include "MpIntelTdx.h"
 
 #include <Library/UefiLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -15,8 +16,8 @@
 #include <Library/VmgExitLib.h>
 #include <Register/Amd/Fam17Msr.h>
 #include <Register/Amd/Ghcb.h>
-
 #include <Protocol/Timer.h>
+#include <ConfidentialComputingGuestAttr.h>
 
 #define  AP_SAFE_STACK_SIZE  128
 
@@ -824,6 +825,10 @@ MpInitLibStartupThisAP (
 {
   EFI_STATUS  Status;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_UNSUPPORTED;
+  }
+
   //
   // temporarily stop checkAllApsStatus for avoid resource dead-lock.
   //
@@ -880,6 +885,10 @@ MpInitLibSwitchBSP (
   EFI_TIMER_ARCH_PROTOCOL  *Timer;
   UINT64                   TimerPeriod;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_UNSUPPORTED;
+  }
+
   TimerPeriod = 0;
   //
   // Locate Timer Arch Protocol
@@ -953,6 +962,10 @@ MpInitLibEnableDisableAP (
   EFI_STATUS  Status;
   BOOLEAN     TempStopCheckState;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_UNSUPPORTED;
+  }
+
   TempStopCheckState = FALSE;
   //
   // temporarily stop checkAllAPsStatus for initialize parameters.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
new file mode 100644
index 000000000000..b2136f466ce6
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
@@ -0,0 +1,71 @@
+/** @file
+  Intel Tdx header file.
+
+  Copyright (c) 2021, 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
+EFIAPI
+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
+EFIAPI
+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 4a73787ee43a..de2ed8e6c294 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -9,9 +9,11 @@
 **/
 
 #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;
 
@@ -1803,6 +1805,10 @@ MpInitLibInitialize (
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_SUCCESS;
+  }
+
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
     MaxLogicalProcessorNumber = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
@@ -2073,6 +2079,10 @@ 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;
 
@@ -2307,6 +2317,10 @@ EnableDisableApWorker (
   CPU_MP_DATA  *CpuMpData;
   UINTN        CallerNumber;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_UNSUPPORTED;
+  }
+
   CpuMpData = GetCpuMpData ();
 
   //
@@ -2367,6 +2381,11 @@ MpInitLibWhoAmI (
     return EFI_INVALID_PARAMETER;
   }
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    *ProcessorNumber = 0;
+    return EFI_SUCCESS;
+  }
+
   CpuMpData = GetCpuMpData ();
 
   return GetProcessorNumber (CpuMpData, ProcessorNumber);
@@ -2405,6 +2424,10 @@ MpInitLibGetNumberOfProcessors (
   UINTN        EnabledProcessorNumber;
   UINTN        Index;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return TdxMpInitLibGetNumberOfProcessors (NumberOfProcessors, NumberOfEnabledProcessors);
+  }
+
   CpuMpData = GetCpuMpData ();
 
   if ((NumberOfProcessors == NULL) && (NumberOfEnabledProcessors == NULL)) {
@@ -2490,6 +2513,10 @@ StartupAllCPUsWorker (
   BOOLEAN      HasEnabledAp;
   CPU_STATE    ApState;
 
+  if (CC_GUEST_IS_TDX (PcdGet64 (PcdConfidentialComputingGuestAttr))) {
+    return EFI_SUCCESS;
+  }
+
   CpuMpData = GetCpuMpData ();
 
   if (FailedCpuList != NULL) {
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
new file mode 100644
index 000000000000..d31d043b20b7
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
@@ -0,0 +1,128 @@
+/** @file
+  CPU MP Initialize Library common functions.
+
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MpLib.h"
+#include "MpIntelTdx.h"
+#include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
+#include <IndustryStandard/Tdx.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
+EFIAPI
+TdxMpInitLibGetProcessorInfo (
+  IN  UINTN                      ProcessorNumber,
+  OUT EFI_PROCESSOR_INFORMATION  *ProcessorInfoBuffer,
+  OUT EFI_HEALTH_FLAGS           *HealthData  OPTIONAL
+  )
+{
+  EFI_STATUS      Status;
+  TD_RETURN_DATA  TdReturnData;
+
+  if (ProcessorInfoBuffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+  ASSERT (Status == EFI_SUCCESS);
+
+  if (ProcessorNumber >= TdReturnData.TdInfo.NumVcpus) {
+    return EFI_NOT_FOUND;
+  }
+
+  ProcessorInfoBuffer->ProcessorId = ProcessorNumber;
+  ProcessorInfoBuffer->StatusFlag  = 0;
+  if (ProcessorNumber == 0) {
+    ProcessorInfoBuffer->StatusFlag |= PROCESSOR_AS_BSP_BIT;
+  }
+
+  ProcessorInfoBuffer->StatusFlag |= PROCESSOR_ENABLED_BIT;
+
+  //
+  // Get processor location information
+  //
+  GetProcessorLocationByApicId (
+    (UINT32)ProcessorNumber,
+    &ProcessorInfoBuffer->Location.Package,
+    &ProcessorInfoBuffer->Location.Core,
+    &ProcessorInfoBuffer->Location.Thread
+    );
+
+  if (HealthData != NULL) {
+    HealthData->Uint32 = 0;
+  }
+
+  return Status;
+}
+
+/**
+  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
+EFIAPI
+TdxMpInitLibGetNumberOfProcessors (
+  OUT UINTN *NumberOfProcessors, OPTIONAL
+  OUT UINTN                     *NumberOfEnabledProcessors OPTIONAL
+  )
+{
+  EFI_STATUS      Status;
+  TD_RETURN_DATA  TdReturnData;
+
+  if ((NumberOfProcessors == NULL) && (NumberOfEnabledProcessors == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+  ASSERT (Status == EFI_SUCCESS);
+
+  if (NumberOfProcessors != NULL) {
+    *NumberOfProcessors = TdReturnData.TdInfo.NumVcpus;
+  }
+
+  if (NumberOfEnabledProcessors != NULL) {
+    *NumberOfEnabledProcessors = TdReturnData.TdInfo.MaxVcpus;
+  }
+
+  return Status;
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c b/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
new file mode 100644
index 000000000000..2849e13f2aaa
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
@@ -0,0 +1,73 @@
+/** @file
+  CPU MP Initialize Library common functions.
+
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "MpLib.h"
+#include "MpIntelTdx.h"
+#include <Library/DebugLib.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
+EFIAPI
+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
+EFIAPI
+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 5facf4db9499..894be0f8daab 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -24,10 +24,12 @@
 [Sources.IA32]
   Ia32/AmdSev.c
   Ia32/MpFuncs.nasm
+  MpLibTdxNull.c
 
 [Sources.X64]
   X64/AmdSev.c
   X64/MpFuncs.nasm
+  MpLibTdx.c
 
 [Sources.common]
   AmdSev.c
@@ -36,6 +38,7 @@
   MpLib.c
   MpLib.h
   Microcode.c
+  MpIntelTdx.h
 
 [Packages]
   MdePkg/MdePkg.dec
-- 
2.29.2.windows.2



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

Min,
I don't quite understand.

Of all MP services, the TDX version only supports:
* GetNumberOfProcessors
* GetProcessorInfo

And even when TDX MP returns 3 or 4 threads from GetNumberOfProcessors(), StartupAllAPs() is an NOP.
Will that cause any issue?

Can you avoid running MP driver when TDX is enabled?

Thanks,
Ray


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

On March 15, 2022 3:58 PM, Ni Ray wrote:
> 
> Min,
> I don't quite understand.
> 
> Of all MP services, the TDX version only supports:
> * GetNumberOfProcessors
> * GetProcessorInfo
> 
> And even when TDX MP returns 3 or 4 threads from GetNumberOfProcessors(),
> StartupAllAPs() is an NOP.
> Will that cause any issue?
> 
> Can you avoid running MP driver when TDX is enabled?
MP Driver cannot be skipped in td guest. Because there are other drivers depend on the protocol (gEfiMpServiceProtocolGuid) which is produced by MP driver.

MP service of Td guest is different from the current MP service in EDK2. Td guest implement the MP service by the mechanism of mailbox. Please see [TDVF] 4.3 TDVF AP Handling & [GHCI] 4.1 ACPI MADT Multiprocessor Wakeup Table.
[TDVF] https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.01.pdf
[GHCI] https://www.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface-1.0-344426-002.pdf

So here is my suggestion to this problem.
In MpInitLib if it is Td guest, then GetNumberOfProcessors just return (NumberOfProcessors=1, NumberOfEnabledProcessors=1). Because only the BSP is workable at this moment.
GetProcessorInfo returns EFI_UNSUPPORTED. Because in current stage Td guest doesn't have the MP Service requirement.

What's your thought?

Thanks
Min


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

GetProcessorInfo() can return the information for index#0 CPU.
Other suggestions look good to me.

-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com> 
Sent: Wednesday, March 16, 2022 4:32 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Cc: 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>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
Subject: RE: [PATCH V8 13/47] UefiCpuPkg: Enable Tdx support in MpInitLib

On March 15, 2022 3:58 PM, Ni Ray wrote:
> 
> Min,
> I don't quite understand.
> 
> Of all MP services, the TDX version only supports:
> * GetNumberOfProcessors
> * GetProcessorInfo
> 
> And even when TDX MP returns 3 or 4 threads from GetNumberOfProcessors(),
> StartupAllAPs() is an NOP.
> Will that cause any issue?
> 
> Can you avoid running MP driver when TDX is enabled?
MP Driver cannot be skipped in td guest. Because there are other drivers depend on the protocol (gEfiMpServiceProtocolGuid) which is produced by MP driver.

MP service of Td guest is different from the current MP service in EDK2. Td guest implement the MP service by the mechanism of mailbox. Please see [TDVF] 4.3 TDVF AP Handling & [GHCI] 4.1 ACPI MADT Multiprocessor Wakeup Table.
[TDVF] https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.01.pdf
[GHCI] https://www.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface-1.0-344426-002.pdf

So here is my suggestion to this problem.
In MpInitLib if it is Td guest, then GetNumberOfProcessors just return (NumberOfProcessors=1, NumberOfEnabledProcessors=1). Because only the BSP is workable at this moment.
GetProcessorInfo returns EFI_UNSUPPORTED. Because in current stage Td guest doesn't have the MP Service requirement.

What's your thought?

Thanks
Min


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

On March 16, 2022 4:37 PM, Ni Ray wrote:
> 
> GetProcessorInfo() can return the information for index#0 CPU.
> Other suggestions look good to me.
> 
Agree. It will be updated in the next version.

Thanks
Min


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