public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg.
@ 2023-12-07  7:32 duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create " duntan
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel

In the V2 patch set, modify some function protype and some variables naming.

Original message:
Create and consume gMpInformationHobGuid2 in UefiCpuPkg in this patch series.

Currently, there is a gMpInformationHobGuid defined, created and consumed only in StandaloneMmPkg.
The HOB contains the EFI_PROCESSOR_INFORMATION structure for each CPU and the number of processors.
This is the same as the information that PiSmmCpuDxeSmm uses MpService Protocol to get.

This new gMpInformationHobGuid2 also contains the NumberOfProcessors and the EFI_PROCESSOR_INFORMATION for each CPU. 
lso the HOB is extended to support the case that the maximum HOB length is not enough for all CPU.
So there might be more than one HOB instance in the HOB list. Each HOB describes the corresponding CPU index range.

The plan is to create gMpInformationHob2Guid in CpuMpPei module. Then PiSmmCpuDxeSmm and other MM_STANDALONE modules can consume the hob.
This can avoid calling MpService Protocol in PiSmmCpuDxeSmm. Also the issue that one gMpInformationHobGuid might be not enough when CPU number is 1~2000 or bigger can be solved.

Also the code to extracting information from gSmmBaseHobGuid is modified to remove the assumption that there is only one HOB instance of gSmmBaseHobGuid.

Dun Tan (6):
  UefiCpuPkg: Create gMpInformationHobGuid2 in UefiCpuPkg
  UefiCpuPkg: Build MpInfo2HOB in CpuMpPei
  UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe
  UefiCpuPkg: Add a new field in MpInfo2 HOB
  UefiCpuPkg: Cache core type in MpInfo2 HOB
  UefiCpuPkg: Avoid assuming only one smmbasehob

 UefiCpuPkg/CpuMpPei/CpuMpPei.c               | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/CpuMpPei/CpuMpPei.h               |   6 +++++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.inf             |   3 ++-
 UefiCpuPkg/Include/Guid/MpInformation2.h     |  58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 353 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   8 ++++----
 UefiCpuPkg/UefiCpuPkg.dec                    |   3 +++
 8 files changed, 512 insertions(+), 67 deletions(-)
 create mode 100644 UefiCpuPkg/Include/Guid/MpInformation2.h

-- 
2.31.1.windows.1



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



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

* [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create gMpInformationHobGuid2 in UefiCpuPkg
  2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
@ 2023-12-07  7:32 ` duntan
  2023-12-08  8:24   ` Ni, Ray
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 2/6] UefiCpuPkg: Build MpInfo2HOB in CpuMpPei duntan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann

Create gMpInformationHobGuid2 in UefiCpuPkg.

Currently, there is a gMpInformationHobGuid defined,
created and consumed only in StandaloneMmPkg. The HOB
contains the EFI_PROCESSOR_INFORMATION structure for
each CPU and the number of processors. This is the same
as the information that PiSmmCpuDxeSmm uses MpService
Protocol to get.

This new gMpInformationHobGuid2 also contains the
NumberOfProcessors and the EFI_PROCESSOR_INFORMATION
for each CPU. Also the HOB is extended to support the
case that the maximum HOB length is not enough for all
CPU. So there might be more than one HOB instance in the
HOB list. Each HOB describes the corresponding CPU index
range.

The plan is to create gMpInformationHob2Guid in CpuMpPei
module(implemented in next commit). Then PiSmmCpuDxeSmm
and other MM_STANDALONE modules can consume the hob. This
can avoid calling MpService Protocol in PiSmmCpuDxeSmm.
Also the issue that one gMpInformationHobGuid might be not
enough when CPU number is 1~2000 or bigger can be solved.

Signed-off-by: Dun Tan <dun.tan@intel.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>
---
 UefiCpuPkg/Include/Guid/MpInformation2.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/UefiCpuPkg.dec                |  3 +++
 2 files changed, 59 insertions(+)

diff --git a/UefiCpuPkg/Include/Guid/MpInformation2.h b/UefiCpuPkg/Include/Guid/MpInformation2.h
new file mode 100644
index 0000000000..7a25fcc2f3
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/MpInformation2.h
@@ -0,0 +1,56 @@
+/** @file
+  EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
+
+  MP information protocol only provides static information of MP processor.
+
+  If SwitchBSP or Enable/DisableAP in MP service is called between the HOB
+  production and HOB consumption, EFI_PROCESSOR_INFORMATION.StatusFlag field
+  in this HOB may be invalidated.
+
+  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MP_INFORMATION2_H_
+#define MP_INFORMATION2_H_
+
+#include <Protocol/MpService.h>
+#include <PiPei.h>
+#include <Ppi/SecPlatformInformation.h>
+
+#define MP_INFORMATION2_HOB_REVISION  1
+
+#define MP_INFORMATION2_GUID \
+  { \
+    0x417a7f64, 0xf4e9, 0x4b32, {0x84, 0x6a, 0x5c, 0xc4, 0xd8, 0x62, 0x18, 0x79}  \
+  }
+
+typedef struct {
+  EFI_PROCESSOR_INFORMATION    ProcessorInfo;
+  //
+  // Add more fields in future
+  //
+} MP_INFORMATION2_ENTRY;
+
+typedef struct {
+  UINT16                   NumberOfProcessors;
+  UINT16                   EntrySize;
+  UINT8                    Version;
+  UINT8                    Reserved[3];
+  UINT64                   ProcessorIndex;
+  MP_INFORMATION2_ENTRY    Entry[0];
+} MP_INFORMATION2_HOB_DATA;
+
+//
+// Producer of MP_INFORMATION2_HOB_DATA should assign sizeof (MP_INFORMATION2_ENTRY) to MP_INFORMATION2_HOB_DATA.EntrySize
+// Consumer of MP_INFORMATION2_HOB_DATA should use below macro or similar logic to get the individual entry
+// as the entry structure might be updated to include more information.
+//
+#define GET_MP_INFORMATION_ENTRY(MpInfoHobData, Index) \
+    (MP_INFORMATION2_ENTRY *)((UINTN)&((MP_INFORMATION2_HOB_DATA *)(MpInfoHobData))->Entry + (MpInfoHobData)->EntrySize * Index)
+
+extern EFI_GUID  gMpInformationHobGuid2;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 0b5431dbf7..61bd34ef17 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -85,6 +85,9 @@
   ## Include/Guid/SmmBaseHob.h
   gSmmBaseHobGuid      = { 0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47, 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 0x73 }}
 
+  ## Include/Guid/MpInformation2.h
+  gMpInformationHobGuid2         = { 0x417a7f64, 0xf4e9, 0x4b32, {0x84, 0x6a, 0x5c, 0xc4, 0xd8, 0x62, 0x18, 0x79 }}
+
 [Protocols]
   ## Include/Protocol/SmmCpuService.h
   gEfiSmmCpuServiceProtocolGuid   = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
-- 
2.31.1.windows.1



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



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

* [edk2-devel] [Patch V2 2/6] UefiCpuPkg: Build MpInfo2HOB in CpuMpPei
  2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create " duntan
@ 2023-12-07  7:32 ` duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe duntan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann

Build MpInfo2HOB in CpuMpPei module so that later
PiSmmCpuDxe or other StandaloneMm module can consume
the HOB.
Since there might be more one gMpInformationHobGuid2
in HOB list, CpuMpPei create a gMpInformationHobGuid2
with 0 value NumberOfProcessors field in the end of the
process to indicate it's the last MP_INFORMATION2_HOB.

Signed-off-by: Dun Tan <dun.tan@intel.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>
---
 UefiCpuPkg/CpuMpPei/CpuMpPei.c   | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 UefiCpuPkg/CpuMpPei/CpuMpPei.h   |  4 +++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.inf |  3 ++-
 3 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index b504bea3cf..8cacf4ddf5 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -541,6 +541,92 @@ InitializeMpExceptionStackSwitchHandlers (
   FreePages (SwitchStackData, EFI_SIZE_TO_PAGES (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)));
 }
 
+/**
+  Create gMpInformationHobGuid2.
+**/
+VOID
+BuildMpInformationHob (
+  VOID
+  )
+{
+  EFI_STATUS                Status;
+  UINTN                     ProcessorIndex;
+  UINTN                     NumberOfProcessors;
+  UINTN                     NumberOfEnabledProcessors;
+  UINTN                     NumberOfProcessorsInHob;
+  UINTN                     MaxProcessorsPerHob;
+  MP_INFORMATION2_HOB_DATA  *MpInformation2HobData;
+  MP_INFORMATION2_ENTRY     *MpInformation2Entry;
+  UINTN                     Index;
+
+  ProcessorIndex        = 0;
+  MpInformation2HobData = NULL;
+  MpInformation2Entry   = NULL;
+
+  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
+  ASSERT_EFI_ERROR (Status);
+  MaxProcessorsPerHob     = ((MAX_UINT16 & ~7) - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_INFORMATION2_HOB_DATA)) / sizeof (MP_INFORMATION2_ENTRY);
+  NumberOfProcessorsInHob = MaxProcessorsPerHob;
+
+  //
+  // Create MP_INFORMATION2_HOB. when the max HobLength 0xFFF8 is not enough, there
+  // will be a MP_INFORMATION2_HOB series in the HOB list.
+  // In the HOB list, there is a gMpInformationHobGuid2 with 0 value NumberOfProcessors
+  // fields to indicate it's the last MP_INFORMATION2_HOB.
+  //
+  while (NumberOfProcessorsInHob != 0) {
+    NumberOfProcessorsInHob = MIN (NumberOfProcessors - ProcessorIndex, MaxProcessorsPerHob);
+    MpInformation2HobData   = BuildGuidHob (
+                                &gMpInformationHobGuid2,
+                                sizeof (MP_INFORMATION2_HOB_DATA) + sizeof (MP_INFORMATION2_ENTRY) * NumberOfProcessorsInHob
+                                );
+    ASSERT (MpInformation2HobData != NULL);
+
+    MpInformation2HobData->Version            = MP_INFORMATION2_HOB_REVISION;
+    MpInformation2HobData->ProcessorIndex     = ProcessorIndex;
+    MpInformation2HobData->NumberOfProcessors = (UINT16)NumberOfProcessorsInHob;
+    MpInformation2HobData->EntrySize          = sizeof (MP_INFORMATION2_ENTRY);
+
+    DEBUG ((DEBUG_INFO, "Creating MpInformation2 HOB...\n"));
+
+    for (Index = 0; Index < NumberOfProcessorsInHob; Index++) {
+      MpInformation2Entry = &MpInformation2HobData->Entry[Index];
+      Status              = MpInitLibGetProcessorInfo (
+                              (Index + ProcessorIndex) | CPU_V2_EXTENDED_TOPOLOGY,
+                              &MpInformation2Entry->ProcessorInfo,
+                              NULL
+                              );
+      ASSERT_EFI_ERROR (Status);
+      DEBUG ((
+        DEBUG_INFO,
+        "  Processor[%04d]: ProcessorId = 0x%lx, StatusFlag = 0x%x\n",
+        Index + ProcessorIndex,
+        MpInformation2Entry->ProcessorInfo.ProcessorId,
+        MpInformation2Entry->ProcessorInfo.StatusFlag
+        ));
+      DEBUG ((
+        DEBUG_INFO,
+        "    Location (Package/Core/Thread) = %d/%d/%d\n",
+        MpInformation2Entry->ProcessorInfo.Location.Package,
+        MpInformation2Entry->ProcessorInfo.Location.Core,
+        MpInformation2Entry->ProcessorInfo.Location.Thread
+        ));
+      DEBUG ((
+        DEBUG_INFO,
+        "    Location2 (Package/Die/Tile/Module/Core/Thread) = %d/%d/%d/%d/%d/%d\n",
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Package,
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Die,
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Tile,
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Module,
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Core,
+        MpInformation2Entry->ProcessorInfo.ExtendedInformation.Location2.Thread
+        ));
+    }
+
+    ProcessorIndex += NumberOfProcessorsInHob;
+  }
+}
+
 /**
   Initializes MP and exceptions handlers.
 
@@ -602,6 +688,11 @@ InitializeCpuMpWorker (
   Status = PeiServicesInstallPpi (mPeiCpuMpPpiList);
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // Create gMpInformationHobGuid2
+  //
+  BuildMpInformationHob ();
+
   return Status;
 }
 
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index 1b9a94e18f..a40fd2c077 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -1,7 +1,7 @@
 /** @file
   Definitions to install Multiple Processor PPI.
 
-  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -30,6 +30,8 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/CpuPageTableLib.h>
 
+#include <Guid/MpInformation2.h>
+
 extern EFI_PEI_PPI_DESCRIPTOR  mPeiCpuMpPpiDesc;
 
 /**
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index 865be5627e..812fa179bd 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -1,7 +1,7 @@
 ## @file
 #  CPU driver installs CPU PI Multi-processor PPI.
 #
-#  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2015 - 2023, Intel Corporation. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -50,6 +50,7 @@
 
 [Guids]
   gEdkiiMigratedFvInfoGuid                                             ## SOMETIMES_CONSUMES     ## HOB
+  gMpInformationHobGuid2                        ## PRODUCES
 
 [Ppis]
   gEfiPeiMpServicesPpiGuid                      ## PRODUCES
-- 
2.31.1.windows.1



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



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

* [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe
  2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create " duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 2/6] UefiCpuPkg: Build MpInfo2HOB in CpuMpPei duntan
@ 2023-12-07  7:32 ` duntan
  2023-12-08  8:17   ` Ni, Ray
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB duntan
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob duntan
  4 siblings, 1 reply; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann

Consume MpInfo2Hob in PiSmmCpuDxe driver to get
NumberOfProcessors, MaxNumberOfCpus and
EFI_PROCESSOR_INFORMATION for all CPU from the
MpInformation2 HOB.
This can avoid calling MP service.

Signed-off-by: Dun Tan <dun.tan@intel.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>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |   2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   8 ++++----
 3 files changed, 169 insertions(+), 51 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 1d022a7051..b729f8ee63 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -99,8 +99,8 @@ UINTN  mSmmStackSize;
 UINTN    mSmmShadowStackSize;
 BOOLEAN  mCetSupported = TRUE;
 
-UINTN  mMaxNumberOfCpus = 1;
-UINTN  mNumberOfCpus    = 1;
+UINTN  mMaxNumberOfCpus = 0;
+UINTN  mNumberOfCpus    = 0;
 
 //
 // SMM ready to lock flag
@@ -586,6 +586,147 @@ SmmReadyToLockEventNotify (
   return EFI_SUCCESS;
 }
 
+/**
+  Function to compare 2 MP_INFORMATION2_HOB_DATA pointer based on ProcessorIndex.
+
+  @param[in] Buffer1            pointer to MP_INFORMATION2_HOB_DATA poiner to compare
+  @param[in] Buffer2            pointer to second MP_INFORMATION2_HOB_DATA pointer to compare
+
+  @retval 0                     Buffer1 equal to Buffer2
+  @retval <0                    Buffer1 is less than Buffer2
+  @retval >0                    Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+MpInformation2HobCompare (
+  IN  CONST VOID  *Buffer1,
+  IN  CONST VOID  *Buffer2
+  )
+{
+  if ((*(MP_INFORMATION2_HOB_DATA **)Buffer1)->ProcessorIndex > (*(MP_INFORMATION2_HOB_DATA **)Buffer2)->ProcessorIndex) {
+    return 1;
+  } else if ((*(MP_INFORMATION2_HOB_DATA **)Buffer1)->ProcessorIndex < (*(MP_INFORMATION2_HOB_DATA **)Buffer2)->ProcessorIndex) {
+    return -1;
+  }
+
+  return 0;
+}
+
+/**
+  Extract NumberOfCpus, MaxNumberOfCpus and EFI_PROCESSOR_INFORMATION for all CPU from MpInformation2 HOB.
+
+  @param[out] NumberOfCpus           Pointer to NumberOfCpus.
+  @param[out] MaxNumberOfCpus        Pointer to MaxNumberOfCpus.
+
+  @retval ProcessorInfo              Pointer to EFI_PROCESSOR_INFORMATION buffer.
+**/
+EFI_PROCESSOR_INFORMATION *
+GetMpInformation (
+  OUT UINTN  *NumberOfCpus,
+  OUT UINTN  *MaxNumberOfCpus
+  )
+{
+  EFI_HOB_GUID_TYPE          *GuidHob;
+  EFI_HOB_GUID_TYPE          *FirstMpInfo2Hob;
+  MP_INFORMATION2_HOB_DATA   *MpInformation2HobData;
+  UINTN                      HobCount;
+  UINTN                      HobIndex;
+  MP_INFORMATION2_HOB_DATA   **MpInfo2Hobs;
+  UINTN                      SortBuffer;
+  UINTN                      ProcessorIndex;
+  UINT64                     PrevProcessorIndex;
+  MP_INFORMATION2_ENTRY      *MpInformation2Entry;
+  EFI_PROCESSOR_INFORMATION  *ProcessorInfo;
+
+  GuidHob               = NULL;
+  MpInformation2HobData = NULL;
+  FirstMpInfo2Hob       = NULL;
+  MpInfo2Hobs           = NULL;
+  HobIndex              = 0;
+  HobCount              = 0;
+
+  FirstMpInfo2Hob = GetFirstGuidHob (&gMpInformationHobGuid2);
+  ASSERT (FirstMpInfo2Hob != NULL);
+  GuidHob = FirstMpInfo2Hob;
+  while (GuidHob != NULL) {
+    MpInformation2HobData = GET_GUID_HOB_DATA (GuidHob);
+
+    //
+    // This is the last MpInformationHob in the HOB list.
+    //
+    if (MpInformation2HobData->NumberOfProcessors == 0) {
+      ASSERT (HobCount != 0);
+      break;
+    }
+
+    HobCount++;
+    *NumberOfCpus += MpInformation2HobData->NumberOfProcessors;
+    GuidHob        = GetNextGuidHob (&gMpInformationHobGuid2, GET_NEXT_HOB (GuidHob));
+  }
+
+  ASSERT (*NumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
+
+  //
+  // If support CPU hot plug, we need to allocate resources for possibly hot-added processors
+  //
+  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
+    *MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+  } else {
+    *MaxNumberOfCpus = *NumberOfCpus;
+  }
+
+  MpInfo2Hobs = AllocatePool (sizeof (MP_INFORMATION2_HOB_DATA *) * HobCount);
+  ASSERT (MpInfo2Hobs != NULL);
+  if (MpInfo2Hobs == NULL) {
+    return NULL;
+  }
+
+  //
+  // Record each MpInformation2Hob pointer in the MpInfo2Hobs.
+  // The FirstMpInfo2Hob is to speed up this while-loop without
+  // needing to look for MpInfo2Hob from beginning.
+  //
+  GuidHob = FirstMpInfo2Hob;
+  ASSERT (GuidHob != NULL);
+  while (HobIndex < HobCount) {
+    MpInfo2Hobs[HobIndex++] = GET_GUID_HOB_DATA (GuidHob);
+    GuidHob                 = GetNextGuidHob (&gMpInformationHobGuid2, GET_NEXT_HOB (GuidHob));
+  }
+
+  ProcessorInfo = (EFI_PROCESSOR_INFORMATION *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) * (*MaxNumberOfCpus));
+  ASSERT (ProcessorInfo != NULL);
+  if (ProcessorInfo == NULL) {
+    FreePool (MpInfo2Hobs);
+    return NULL;
+  }
+
+  QuickSort (MpInfo2Hobs, HobCount, sizeof (MP_INFORMATION2_HOB_DATA *), (BASE_SORT_COMPARE)MpInformation2HobCompare, &SortBuffer);
+  PrevProcessorIndex = 0;
+  for (HobIndex = 0; HobIndex < HobCount; HobIndex++) {
+    //
+    // Make sure no overlap and no gap in the CPU range covered by each HOB
+    //
+    ASSERT (MpInfo2Hobs[HobIndex]->ProcessorIndex == PrevProcessorIndex);
+
+    //
+    // Cache each EFI_PROCESSOR_INFORMATION in order.
+    //
+    for (ProcessorIndex = 0; ProcessorIndex < MpInfo2Hobs[HobIndex]->NumberOfProcessors; ProcessorIndex++) {
+      MpInformation2Entry = GET_MP_INFORMATION_ENTRY (MpInfo2Hobs[HobIndex], ProcessorIndex);
+      CopyMem (
+        &ProcessorInfo[PrevProcessorIndex + ProcessorIndex],
+        &MpInformation2Entry->ProcessorInfo,
+        sizeof (EFI_PROCESSOR_INFORMATION)
+        );
+    }
+
+    PrevProcessorIndex += MpInfo2Hobs[HobIndex]->NumberOfProcessors;
+  }
+
+  FreePool (MpInfo2Hobs);
+  return ProcessorInfo;
+}
+
 /**
   The module Entry Point of the CPU SMM driver.
 
@@ -603,26 +744,24 @@ PiCpuSmmEntry (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUS                Status;
-  EFI_MP_SERVICES_PROTOCOL  *MpServices;
-  UINTN                     NumberOfEnabledProcessors;
-  UINTN                     Index;
-  VOID                      *Buffer;
-  UINTN                     BufferPages;
-  UINTN                     TileCodeSize;
-  UINTN                     TileDataSize;
-  UINTN                     TileSize;
-  UINT8                     *Stacks;
-  VOID                      *Registration;
-  UINT32                    RegEax;
-  UINT32                    RegEbx;
-  UINT32                    RegEcx;
-  UINT32                    RegEdx;
-  UINTN                     FamilyId;
-  UINTN                     ModelId;
-  UINT32                    Cr3;
-  EFI_HOB_GUID_TYPE         *GuidHob;
-  SMM_BASE_HOB_DATA         *SmmBaseHobData;
+  EFI_STATUS         Status;
+  UINTN              Index;
+  VOID               *Buffer;
+  UINTN              BufferPages;
+  UINTN              TileCodeSize;
+  UINTN              TileDataSize;
+  UINTN              TileSize;
+  UINT8              *Stacks;
+  VOID               *Registration;
+  UINT32             RegEax;
+  UINT32             RegEbx;
+  UINT32             RegEcx;
+  UINT32             RegEdx;
+  UINTN              FamilyId;
+  UINTN              ModelId;
+  UINT32             Cr3;
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  SMM_BASE_HOB_DATA  *SmmBaseHobData;
 
   GuidHob        = NULL;
   SmmBaseHobData = NULL;
@@ -654,17 +793,10 @@ PiCpuSmmEntry (
   FindSmramInfo (&mCpuHotPlugData.SmrrBase, &mCpuHotPlugData.SmrrSize);
 
   //
-  // Get MP Services Protocol
-  //
-  Status = SystemTable->BootServices->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&MpServices);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Use MP Services Protocol to retrieve the number of processors and number of enabled processors
+  // Retrive NumberOfProcessors, MaxNumberOfCpus and EFI_PROCESSOR_INFORMATION for all CPU from MpInformation2 HOB.
   //
-  Status = MpServices->GetNumberOfProcessors (MpServices, &mNumberOfCpus, &NumberOfEnabledProcessors);
-  ASSERT_EFI_ERROR (Status);
-  ASSERT (mNumberOfCpus <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
+  gSmmCpuPrivate->ProcessorInfo = GetMpInformation (&mNumberOfCpus, &mMaxNumberOfCpus);
+  ASSERT (gSmmCpuPrivate->ProcessorInfo != NULL);
 
   //
   // If support CPU hot plug, PcdCpuSmmEnableBspElection should be set to TRUE.
@@ -690,15 +822,6 @@ PiCpuSmmEntry (
   mAddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) & PAGING_1G_ADDRESS_MASK_64;
   DEBUG ((DEBUG_INFO, "mAddressEncMask = 0x%lx\n", mAddressEncMask));
 
-  //
-  // If support CPU hot plug, we need to allocate resources for possibly hot-added processors
-  //
-  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
-    mMaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
-  } else {
-    mMaxNumberOfCpus = mNumberOfCpus;
-  }
-
   gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus = mMaxNumberOfCpus;
 
   PERF_CODE (
@@ -908,9 +1031,6 @@ PiCpuSmmEntry (
   //
   // Allocate buffer for pointers to array in  SMM_CPU_PRIVATE_DATA.
   //
-  gSmmCpuPrivate->ProcessorInfo = (EFI_PROCESSOR_INFORMATION *)AllocatePool (sizeof (EFI_PROCESSOR_INFORMATION) * mMaxNumberOfCpus);
-  ASSERT (gSmmCpuPrivate->ProcessorInfo != NULL);
-
   gSmmCpuPrivate->Operation = (SMM_CPU_OPERATION *)AllocatePool (sizeof (SMM_CPU_OPERATION) * mMaxNumberOfCpus);
   ASSERT (gSmmCpuPrivate->Operation != NULL);
 
@@ -945,8 +1065,6 @@ PiCpuSmmEntry (
     gSmmCpuPrivate->Operation[Index]        = SmmCpuNone;
 
     if (Index < mNumberOfCpus) {
-      Status = MpServices->GetProcessorInfo (MpServices, Index | CPU_V2_EXTENDED_TOPOLOGY, &gSmmCpuPrivate->ProcessorInfo[Index]);
-      ASSERT_EFI_ERROR (Status);
       mCpuHotPlugData.ApicId[Index] = gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId;
 
       DEBUG ((
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 20ada465c2..f18345881b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -14,7 +14,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <PiSmm.h>
 
-#include <Protocol/MpService.h>
 #include <Protocol/SmmConfiguration.h>
 #include <Protocol/SmmCpu.h>
 #include <Protocol/SmmAccess2.h>
@@ -27,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/MemoryAttributesTable.h>
 #include <Guid/PiSmmMemoryAttributesTable.h>
 #include <Guid/SmmBaseHob.h>
+#include <Guid/MpInformation2.h>
 
 #include <Library/BaseLib.h>
 #include <Library/IoLib.h>
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 5d52ed7d13..372596f24c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -106,7 +106,6 @@
 
 [Protocols]
   gEfiSmmAccess2ProtocolGuid               ## CONSUMES
-  gEfiMpServiceProtocolGuid                ## CONSUMES
   gEfiSmmConfigurationProtocolGuid         ## PRODUCES
   gEfiSmmCpuProtocolGuid                   ## PRODUCES
   gEfiSmmReadyToLockProtocolGuid           ## NOTIFY
@@ -120,6 +119,7 @@
   gEdkiiPiSmmMemoryAttributesTableGuid     ## CONSUMES ## SystemTable
   gEfiMemoryAttributesTableGuid            ## CONSUMES ## SystemTable
   gSmmBaseHobGuid                          ## CONSUMES
+  gMpInformationHobGuid2                   ## CONSUMES # Assume the HOB must has been created
 
 [FeaturePcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug                         ## CONSUMES
@@ -153,11 +153,11 @@
 [FixedPcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmMpTokenCountPerChunk               ## CONSUMES
 
+[Depex]
+  TRUE
+
 [Pcd.X64]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess        ## CONSUMES
 
-[Depex]
-  gEfiMpServiceProtocolGuid
-
 [UserExtensions.TianoCore."ExtraFiles"]
   PiSmmCpuDxeSmmExtra.uni
-- 
2.31.1.windows.1



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



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

* [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB
  2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
                   ` (2 preceding siblings ...)
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe duntan
@ 2023-12-07  7:32 ` duntan
  2023-12-08  8:10   ` Ni, Ray
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob duntan
  4 siblings, 1 reply; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann

Cache core type in MpInfo2 HOB by CpuMpPei module.

Signed-off-by: Dun Tan <dun.tan@intel.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>
---
 UefiCpuPkg/CpuMpPei/CpuMpPei.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 UefiCpuPkg/CpuMpPei/CpuMpPei.h |  2 ++
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index 8cacf4ddf5..7e2d7efb6b 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -541,6 +541,30 @@ InitializeMpExceptionStackSwitchHandlers (
   FreePages (SwitchStackData, EFI_SIZE_TO_PAGES (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)));
 }
 
+/**
+  Get CPU core type.
+
+  @param[in, out] Buffer  Argument of the procedure.
+**/
+VOID
+EFIAPI
+GetProcessorCoreType (
+  IN OUT VOID  *Buffer
+  )
+{
+  EFI_STATUS                               Status;
+  UINT8                                    *CoreTypes;
+  CPUID_NATIVE_MODEL_ID_AND_CORE_TYPE_EAX  NativeModelIdAndCoreTypeEax;
+  UINTN                                    ProcessorIndex;
+
+  Status = MpInitLibWhoAmI (&ProcessorIndex);
+  ASSERT_EFI_ERROR (Status);
+
+  CoreTypes = (UINT8 *)Buffer;
+  AsmCpuidEx (CPUID_HYBRID_INFORMATION, CPUID_HYBRID_INFORMATION_MAIN_LEAF, &NativeModelIdAndCoreTypeEax.Uint32, NULL, NULL, NULL);
+  CoreTypes[ProcessorIndex] = (UINT8)NativeModelIdAndCoreTypeEax.Bits.CoreType;
+}
+
 /**
   Create gMpInformationHobGuid2.
 **/
@@ -558,13 +582,36 @@ BuildMpInformationHob (
   MP_INFORMATION2_HOB_DATA  *MpInformation2HobData;
   MP_INFORMATION2_ENTRY     *MpInformation2Entry;
   UINTN                     Index;
+  UINT8                     *CoreTypes;
+  UINT32                    CpuidMaxInput;
+  UINTN                     CoreTypePages;
 
   ProcessorIndex        = 0;
   MpInformation2HobData = NULL;
   MpInformation2Entry   = NULL;
+  CoreTypes             = NULL;
+  CoreTypePages         = 0;
 
   Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, &NumberOfEnabledProcessors);
   ASSERT_EFI_ERROR (Status);
+
+  //
+  // Get Processors CoreType
+  //
+  AsmCpuid (CPUID_SIGNATURE, &CpuidMaxInput, NULL, NULL, NULL);
+  if (CpuidMaxInput >= CPUID_HYBRID_INFORMATION) {
+    CoreTypePages = EFI_SIZE_TO_PAGES (sizeof (UINT8) * NumberOfProcessors);
+    CoreTypes     = AllocatePages (CoreTypePages);
+    ASSERT (CoreTypes != NULL);
+
+    Status = MpInitLibStartupAllCPUs (
+               GetProcessorCoreType,
+               0,
+               (VOID *)CoreTypes
+               );
+    ASSERT_EFI_ERROR (Status);
+  }
+
   MaxProcessorsPerHob     = ((MAX_UINT16 & ~7) - sizeof (EFI_HOB_GUID_TYPE) - sizeof (MP_INFORMATION2_HOB_DATA)) / sizeof (MP_INFORMATION2_ENTRY);
   NumberOfProcessorsInHob = MaxProcessorsPerHob;
 
@@ -597,12 +644,16 @@ BuildMpInformationHob (
                               NULL
                               );
       ASSERT_EFI_ERROR (Status);
+
+      MpInformation2Entry->CoreType = (CoreTypes != NULL) ? CoreTypes[Index + ProcessorIndex] : 0;
+
       DEBUG ((
         DEBUG_INFO,
-        "  Processor[%04d]: ProcessorId = 0x%lx, StatusFlag = 0x%x\n",
+        "  Processor[%04d]: ProcessorId = 0x%lx, StatusFlag = 0x%x, CoreType = 0x%x\n",
         Index + ProcessorIndex,
         MpInformation2Entry->ProcessorInfo.ProcessorId,
-        MpInformation2Entry->ProcessorInfo.StatusFlag
+        MpInformation2Entry->ProcessorInfo.StatusFlag,
+        MpInformation2Entry->CoreType
         ));
       DEBUG ((
         DEBUG_INFO,
@@ -625,6 +676,10 @@ BuildMpInformationHob (
 
     ProcessorIndex += NumberOfProcessorsInHob;
   }
+
+  if (CoreTypes != NULL) {
+    FreePages (CoreTypes, CoreTypePages);
+  }
 }
 
 /**
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index a40fd2c077..e7d07ffd64 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -32,6 +32,8 @@
 
 #include <Guid/MpInformation2.h>
 
+#include <Register/Cpuid.h>
+
 extern EFI_PEI_PPI_DESCRIPTOR  mPeiCpuMpPpiDesc;
 
 /**
-- 
2.31.1.windows.1



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



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

* [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob
  2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
                   ` (3 preceding siblings ...)
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB duntan
@ 2023-12-07  7:32 ` duntan
  2023-12-08  8:26   ` Ni, Ray
  4 siblings, 1 reply; 10+ messages in thread
From: duntan @ 2023-12-07  7:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann

Modify the gSmmBaseHobGuid consumption code to
remove the asuumption that there is only one
gSmmBaseHobGuid. If the CPU number is big enough,
there will be more than one SmmBaseHob in the
HOB list.

Signed-off-by: Dun Tan <dun.tan@intel.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>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 147 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index b729f8ee63..fab2fed286 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -586,6 +586,132 @@ SmmReadyToLockEventNotify (
   return EFI_SUCCESS;
 }
 
+/**
+  Function to compare 2 SMM_BASE_HOB_DATA pointer based on ProcessorIndex.
+
+  @param[in] Buffer1            pointer to SMM_BASE_HOB_DATA poiner to compare
+  @param[in] Buffer2            pointer to second SMM_BASE_HOB_DATA pointer to compare
+
+  @retval 0                     Buffer1 equal to Buffer2
+  @retval <0                    Buffer1 is less than Buffer2
+  @retval >0                    Buffer1 is greater than Buffer2
+**/
+INTN
+EFIAPI
+SmBaseHobCompare (
+  IN  CONST VOID  *Buffer1,
+  IN  CONST VOID  *Buffer2
+  )
+{
+  if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex > (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
+    return 1;
+  } else if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex < (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
+    return -1;
+  }
+
+  return 0;
+}
+
+/**
+  Extract SmBase for all CPU from SmmBase HOB.
+
+  @param[in]  MaxNumberOfCpus   Max NumberOfCpus.
+
+  @retval SmBaseBuffer          Pointer to SmBase Buffer.
+  @retval NULL                  gSmmBaseHobGuid was not been created.
+**/
+UINTN *
+GetSmBase (
+  IN  UINTN  MaxNumberOfCpus
+  )
+{
+  UINTN              HobCount;
+  EFI_HOB_GUID_TYPE  *GuidHob;
+  SMM_BASE_HOB_DATA  *SmmBaseHobData;
+  UINTN              NumberOfProcessors;
+  SMM_BASE_HOB_DATA  **SmBaseHobs;
+  UINTN              *SmBaseBuffer;
+  UINTN              HobIndex;
+  UINTN              SortBuffer;
+  UINTN              ProcessorIndex;
+  UINT64             PrevProcessorIndex;
+  EFI_HOB_GUID_TYPE  *FirstSmmBaseGuidHob;
+
+  SmmBaseHobData     = NULL;
+  HobIndex           = 0;
+  ProcessorIndex     = 0;
+  HobCount           = 0;
+  NumberOfProcessors = 0;
+
+  FirstSmmBaseGuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
+  if (FirstSmmBaseGuidHob == NULL) {
+    return NULL;
+  }
+
+  GuidHob = FirstSmmBaseGuidHob;
+  while (GuidHob != NULL) {
+    HobCount++;
+    SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
+
+    if (NumberOfProcessors >= MaxNumberOfCpus) {
+      break;
+    }
+
+    NumberOfProcessors += SmmBaseHobData->NumberOfProcessors;
+    GuidHob             = GetNextGuidHob (&gSmmBaseHobGuid, GET_NEXT_HOB (GuidHob));
+  }
+
+  ASSERT (NumberOfProcessors == MaxNumberOfCpus);
+  if (NumberOfProcessors != MaxNumberOfCpus) {
+    CpuDeadLoop ();
+  }
+
+  SmBaseHobs = AllocatePool (sizeof (SMM_BASE_HOB_DATA *) * HobCount);
+  ASSERT (SmBaseHobs != NULL);
+  if (SmBaseHobs == NULL) {
+    return NULL;
+  }
+
+  //
+  // Record each SmmBaseHob pointer in the SmBaseHobs.
+  // The FirstSmmBaseGuidHob is to speed up this while-loop
+  // without needing to look for SmBaseHob from beginning.
+  //
+  GuidHob = FirstSmmBaseGuidHob;
+  while (HobIndex < HobCount) {
+    SmBaseHobs[HobIndex++] = GET_GUID_HOB_DATA (GuidHob);
+    GuidHob                = GetNextGuidHob (&gSmmBaseHobGuid, GET_NEXT_HOB (GuidHob));
+  }
+
+  SmBaseBuffer = (UINTN *)AllocatePool (sizeof (UINTN) * (MaxNumberOfCpus));
+  ASSERT (SmBaseBuffer != NULL);
+  if (SmBaseBuffer == NULL) {
+    FreePool (SmBaseHobs);
+    return NULL;
+  }
+
+  QuickSort (SmBaseHobs, HobCount, sizeof (SMM_BASE_HOB_DATA *), (BASE_SORT_COMPARE)SmBaseHobCompare, &SortBuffer);
+  PrevProcessorIndex = 0;
+  for (HobIndex = 0; HobIndex < HobCount; HobIndex++) {
+    //
+    // Make sure no overlap and no gap in the CPU range covered by each HOB
+    //
+    ASSERT (SmBaseHobs[HobIndex]->ProcessorIndex == PrevProcessorIndex);
+
+    //
+    // Cache each SmBase in order.
+    //
+    for (ProcessorIndex = 0; ProcessorIndex < SmBaseHobs[HobIndex]->NumberOfProcessors; ProcessorIndex++) {
+      SmBaseBuffer[PrevProcessorIndex + ProcessorIndex] = (UINTN)SmBaseHobs[HobIndex]->SmBase[ProcessorIndex];
+    }
+
+    PrevProcessorIndex += SmBaseHobs[HobIndex]->NumberOfProcessors;
+  }
+
+  FreePool (SmBaseHobs);
+  return SmBaseBuffer;
+}
+
 /**
   Function to compare 2 MP_INFORMATION2_HOB_DATA pointer based on ProcessorIndex.
 
@@ -744,27 +870,22 @@ PiCpuSmmEntry (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUS         Status;
-  UINTN              Index;
-  VOID               *Buffer;
-  UINTN              BufferPages;
-  UINTN              TileCodeSize;
-  UINTN              TileDataSize;
-  UINTN              TileSize;
-  UINT8              *Stacks;
-  VOID               *Registration;
-  UINT32             RegEax;
-  UINT32             RegEbx;
-  UINT32             RegEcx;
-  UINT32             RegEdx;
-  UINTN              FamilyId;
-  UINTN              ModelId;
-  UINT32             Cr3;
-  EFI_HOB_GUID_TYPE  *GuidHob;
-  SMM_BASE_HOB_DATA  *SmmBaseHobData;
-
-  GuidHob        = NULL;
-  SmmBaseHobData = NULL;
+  EFI_STATUS  Status;
+  UINTN       Index;
+  VOID        *Buffer;
+  UINTN       BufferPages;
+  UINTN       TileCodeSize;
+  UINTN       TileDataSize;
+  UINTN       TileSize;
+  UINT8       *Stacks;
+  VOID        *Registration;
+  UINT32      RegEax;
+  UINT32      RegEbx;
+  UINT32      RegEcx;
+  UINT32      RegEdx;
+  UINTN       FamilyId;
+  UINTN       ModelId;
+  UINT32      Cr3;
 
   PERF_FUNCTION_BEGIN ();
 
@@ -986,8 +1107,8 @@ PiCpuSmmEntry (
   // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found,
   // means the SmBase relocation has been done.
   //
-  GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
-  if (GuidHob != NULL) {
+  mCpuHotPlugData.SmBase = GetSmBase (mMaxNumberOfCpus);
+  if (mCpuHotPlugData.SmBase != NULL) {
     //
     // Check whether the Required TileSize is enough.
     //
@@ -997,12 +1118,6 @@ PiCpuSmmEntry (
       return RETURN_BUFFER_TOO_SMALL;
     }
 
-    SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
-
-    //
-    // Assume single instance of HOB produced, expect the HOB.NumberOfProcessors equals to the mMaxNumberOfCpus.
-    //
-    ASSERT (SmmBaseHobData->NumberOfProcessors == (UINT32)mMaxNumberOfCpus && SmmBaseHobData->ProcessorIndex == 0);
     mSmmRelocated = TRUE;
   } else {
     //
@@ -1048,8 +1163,6 @@ PiCpuSmmEntry (
   //
   mCpuHotPlugData.ApicId = (UINT64 *)AllocatePool (sizeof (UINT64) * mMaxNumberOfCpus);
   ASSERT (mCpuHotPlugData.ApicId != NULL);
-  mCpuHotPlugData.SmBase = (UINTN *)AllocatePool (sizeof (UINTN) * mMaxNumberOfCpus);
-  ASSERT (mCpuHotPlugData.SmBase != NULL);
   mCpuHotPlugData.ArrayLength = (UINT32)mMaxNumberOfCpus;
 
   //
@@ -1058,7 +1171,9 @@ PiCpuSmmEntry (
   // size for each CPU in the platform
   //
   for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
-    mCpuHotPlugData.SmBase[Index] = mSmmRelocated ? (UINTN)SmmBaseHobData->SmBase[Index] : (UINTN)Buffer + Index * TileSize - SMM_HANDLER_OFFSET;
+    if (!mSmmRelocated) {
+      mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * TileSize - SMM_HANDLER_OFFSET;
+    }
 
     gSmmCpuPrivate->CpuSaveStateSize[Index] = sizeof (SMRAM_SAVE_STATE_MAP);
     gSmmCpuPrivate->CpuSaveState[Index]     = (VOID *)(mCpuHotPlugData.SmBase[Index] + SMRAM_SAVE_STATE_MAP_OFFSET);
-- 
2.31.1.windows.1



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



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

* Re: [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB duntan
@ 2023-12-08  8:10   ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-12-08  8:10 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann

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

Thanks,
Ray
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, December 7, 2023 3:32 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB
> 
> Cache core type in MpInfo2 HOB by CpuMpPei module.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.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>
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.c | 59
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  UefiCpuPkg/CpuMpPei/CpuMpPei.h |  2 ++
>  2 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> index 8cacf4ddf5..7e2d7efb6b 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> @@ -541,6 +541,30 @@ InitializeMpExceptionStackSwitchHandlers (
>    FreePages (SwitchStackData, EFI_SIZE_TO_PAGES (NumberOfProcessors *
> sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)));
>  }
> 
> +/**
> +  Get CPU core type.
> +
> +  @param[in, out] Buffer  Argument of the procedure.
> +**/
> +VOID
> +EFIAPI
> +GetProcessorCoreType (
> +  IN OUT VOID  *Buffer
> +  )
> +{
> +  EFI_STATUS                               Status;
> +  UINT8                                    *CoreTypes;
> +  CPUID_NATIVE_MODEL_ID_AND_CORE_TYPE_EAX
> NativeModelIdAndCoreTypeEax;
> +  UINTN                                    ProcessorIndex;
> +
> +  Status = MpInitLibWhoAmI (&ProcessorIndex);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  CoreTypes = (UINT8 *)Buffer;
> +  AsmCpuidEx (CPUID_HYBRID_INFORMATION,
> CPUID_HYBRID_INFORMATION_MAIN_LEAF,
> &NativeModelIdAndCoreTypeEax.Uint32, NULL, NULL, NULL);
> +  CoreTypes[ProcessorIndex] =
> (UINT8)NativeModelIdAndCoreTypeEax.Bits.CoreType;
> +}
> +
>  /**
>    Create gMpInformationHobGuid2.
>  **/
> @@ -558,13 +582,36 @@ BuildMpInformationHob (
>    MP_INFORMATION2_HOB_DATA  *MpInformation2HobData;
>    MP_INFORMATION2_ENTRY     *MpInformation2Entry;
>    UINTN                     Index;
> +  UINT8                     *CoreTypes;
> +  UINT32                    CpuidMaxInput;
> +  UINTN                     CoreTypePages;
> 
>    ProcessorIndex        = 0;
>    MpInformation2HobData = NULL;
>    MpInformation2Entry   = NULL;
> +  CoreTypes             = NULL;
> +  CoreTypePages         = 0;
> 
>    Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors,
> &NumberOfEnabledProcessors);
>    ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Get Processors CoreType
> +  //
> +  AsmCpuid (CPUID_SIGNATURE, &CpuidMaxInput, NULL, NULL, NULL);
> +  if (CpuidMaxInput >= CPUID_HYBRID_INFORMATION) {
> +    CoreTypePages = EFI_SIZE_TO_PAGES (sizeof (UINT8) *
> NumberOfProcessors);
> +    CoreTypes     = AllocatePages (CoreTypePages);
> +    ASSERT (CoreTypes != NULL);
> +
> +    Status = MpInitLibStartupAllCPUs (
> +               GetProcessorCoreType,
> +               0,
> +               (VOID *)CoreTypes
> +               );
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
>    MaxProcessorsPerHob     = ((MAX_UINT16 & ~7) - sizeof
> (EFI_HOB_GUID_TYPE) - sizeof (MP_INFORMATION2_HOB_DATA)) / sizeof
> (MP_INFORMATION2_ENTRY);
>    NumberOfProcessorsInHob = MaxProcessorsPerHob;
> 
> @@ -597,12 +644,16 @@ BuildMpInformationHob (
>                                NULL
>                                );
>        ASSERT_EFI_ERROR (Status);
> +
> +      MpInformation2Entry->CoreType = (CoreTypes != NULL) ?
> CoreTypes[Index + ProcessorIndex] : 0;
> +
>        DEBUG ((
>          DEBUG_INFO,
> -        "  Processor[%04d]: ProcessorId = 0x%lx, StatusFlag = 0x%x\n",
> +        "  Processor[%04d]: ProcessorId = 0x%lx, StatusFlag = 0x%x,
> CoreType = 0x%x\n",
>          Index + ProcessorIndex,
>          MpInformation2Entry->ProcessorInfo.ProcessorId,
> -        MpInformation2Entry->ProcessorInfo.StatusFlag
> +        MpInformation2Entry->ProcessorInfo.StatusFlag,
> +        MpInformation2Entry->CoreType
>          ));
>        DEBUG ((
>          DEBUG_INFO,
> @@ -625,6 +676,10 @@ BuildMpInformationHob (
> 
>      ProcessorIndex += NumberOfProcessorsInHob;
>    }
> +
> +  if (CoreTypes != NULL) {
> +    FreePages (CoreTypes, CoreTypePages);
> +  }
>  }
> 
>  /**
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index a40fd2c077..e7d07ffd64 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -32,6 +32,8 @@
> 
>  #include <Guid/MpInformation2.h>
> 
> +#include <Register/Cpuid.h>
> +
>  extern EFI_PEI_PPI_DESCRIPTOR  mPeiCpuMpPpiDesc;
> 
>  /**
> --
> 2.31.1.windows.1



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



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

* Re: [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe duntan
@ 2023-12-08  8:17   ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-12-08  8:17 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann

> +  GuidHob = FirstMpInfo2Hob;
> +  ASSERT (GuidHob != NULL);

1. this is not needed.

Reviewed-by: Ray Ni <ray.ni@intel.com> with above assertion removal.


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



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

* Re: [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create gMpInformationHobGuid2 in UefiCpuPkg
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create " duntan
@ 2023-12-08  8:24   ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-12-08  8:24 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann

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

Thanks,
Ray
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, December 7, 2023 3:32 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V2 1/6] UefiCpuPkg: Create gMpInformationHobGuid2 in
> UefiCpuPkg
> 
> Create gMpInformationHobGuid2 in UefiCpuPkg.
> 
> Currently, there is a gMpInformationHobGuid defined,
> created and consumed only in StandaloneMmPkg. The HOB
> contains the EFI_PROCESSOR_INFORMATION structure for
> each CPU and the number of processors. This is the same
> as the information that PiSmmCpuDxeSmm uses MpService
> Protocol to get.
> 
> This new gMpInformationHobGuid2 also contains the
> NumberOfProcessors and the EFI_PROCESSOR_INFORMATION
> for each CPU. Also the HOB is extended to support the
> case that the maximum HOB length is not enough for all
> CPU. So there might be more than one HOB instance in the
> HOB list. Each HOB describes the corresponding CPU index
> range.
> 
> The plan is to create gMpInformationHob2Guid in CpuMpPei
> module(implemented in next commit). Then PiSmmCpuDxeSmm
> and other MM_STANDALONE modules can consume the hob. This
> can avoid calling MpService Protocol in PiSmmCpuDxeSmm.
> Also the issue that one gMpInformationHobGuid might be not
> enough when CPU number is 1~2000 or bigger can be solved.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.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>
> ---
>  UefiCpuPkg/Include/Guid/MpInformation2.h | 56
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  UefiCpuPkg/UefiCpuPkg.dec                |  3 +++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/UefiCpuPkg/Include/Guid/MpInformation2.h
> b/UefiCpuPkg/Include/Guid/MpInformation2.h
> new file mode 100644
> index 0000000000..7a25fcc2f3
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Guid/MpInformation2.h
> @@ -0,0 +1,56 @@
> +/** @file
> +  EFI MP information protocol provides a lightweight
> MP_SERVICES_PROTOCOL.
> +
> +  MP information protocol only provides static information of MP processor.
> +
> +  If SwitchBSP or Enable/DisableAP in MP service is called between the HOB
> +  production and HOB consumption,
> EFI_PROCESSOR_INFORMATION.StatusFlag field
> +  in this HOB may be invalidated.
> +
> +  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef MP_INFORMATION2_H_
> +#define MP_INFORMATION2_H_
> +
> +#include <Protocol/MpService.h>
> +#include <PiPei.h>
> +#include <Ppi/SecPlatformInformation.h>
> +
> +#define MP_INFORMATION2_HOB_REVISION  1
> +
> +#define MP_INFORMATION2_GUID \
> +  { \
> +    0x417a7f64, 0xf4e9, 0x4b32, {0x84, 0x6a, 0x5c, 0xc4, 0xd8, 0x62, 0x18,
> 0x79}  \
> +  }
> +
> +typedef struct {
> +  EFI_PROCESSOR_INFORMATION    ProcessorInfo;
> +  //
> +  // Add more fields in future
> +  //
> +} MP_INFORMATION2_ENTRY;
> +
> +typedef struct {
> +  UINT16                   NumberOfProcessors;
> +  UINT16                   EntrySize;
> +  UINT8                    Version;
> +  UINT8                    Reserved[3];
> +  UINT64                   ProcessorIndex;
> +  MP_INFORMATION2_ENTRY    Entry[0];
> +} MP_INFORMATION2_HOB_DATA;
> +
> +//
> +// Producer of MP_INFORMATION2_HOB_DATA should assign sizeof
> (MP_INFORMATION2_ENTRY) to MP_INFORMATION2_HOB_DATA.EntrySize
> +// Consumer of MP_INFORMATION2_HOB_DATA should use below macro or
> similar logic to get the individual entry
> +// as the entry structure might be updated to include more information.
> +//
> +#define GET_MP_INFORMATION_ENTRY(MpInfoHobData, Index) \
> +    (MP_INFORMATION2_ENTRY
> *)((UINTN)&((MP_INFORMATION2_HOB_DATA *)(MpInfoHobData))->Entry +
> (MpInfoHobData)->EntrySize * Index)
> +
> +extern EFI_GUID  gMpInformationHobGuid2;
> +
> +#endif
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 0b5431dbf7..61bd34ef17 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -85,6 +85,9 @@
>    ## Include/Guid/SmmBaseHob.h
>    gSmmBaseHobGuid      = { 0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47,
> 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 0x73 }}
> 
> +  ## Include/Guid/MpInformation2.h
> +  gMpInformationHobGuid2         = { 0x417a7f64, 0xf4e9, 0x4b32,
> {0x84, 0x6a, 0x5c, 0xc4, 0xd8, 0x62, 0x18, 0x79 }}
> +
>  [Protocols]
>    ## Include/Protocol/SmmCpuService.h
>    gEfiSmmCpuServiceProtocolGuid   = { 0x1d202cab, 0xc8ab, 0x4d5c,
> { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
> --
> 2.31.1.windows.1



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



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

* Re: [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob
  2023-12-07  7:32 ` [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob duntan
@ 2023-12-08  8:26   ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-12-08  8:26 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R, Gerd Hoffmann

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

Thanks,
Ray
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Thursday, December 7, 2023 3:33 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob
> 
> Modify the gSmmBaseHobGuid consumption code to
> remove the asuumption that there is only one
> gSmmBaseHobGuid. If the CPU number is big enough,
> there will be more than one SmmBaseHob in the
> HOB list.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.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>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 179
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++--------------------------------
>  1 file changed, 147 insertions(+), 32 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index b729f8ee63..fab2fed286 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -586,6 +586,132 @@ SmmReadyToLockEventNotify (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  Function to compare 2 SMM_BASE_HOB_DATA pointer based on
> ProcessorIndex.
> +
> +  @param[in] Buffer1            pointer to SMM_BASE_HOB_DATA
> poiner to compare
> +  @param[in] Buffer2            pointer to second
> SMM_BASE_HOB_DATA pointer to compare
> +
> +  @retval 0                     Buffer1 equal to Buffer2
> +  @retval <0                    Buffer1 is less than Buffer2
> +  @retval >0                    Buffer1 is greater than Buffer2
> +**/
> +INTN
> +EFIAPI
> +SmBaseHobCompare (
> +  IN  CONST VOID  *Buffer1,
> +  IN  CONST VOID  *Buffer2
> +  )
> +{
> +  if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex >
> (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
> +    return 1;
> +  } else if ((*(SMM_BASE_HOB_DATA **)Buffer1)->ProcessorIndex <
> (*(SMM_BASE_HOB_DATA **)Buffer2)->ProcessorIndex) {
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +/**
> +  Extract SmBase for all CPU from SmmBase HOB.
> +
> +  @param[in]  MaxNumberOfCpus   Max NumberOfCpus.
> +
> +  @retval SmBaseBuffer          Pointer to SmBase Buffer.
> +  @retval NULL                  gSmmBaseHobGuid was not been
> created.
> +**/
> +UINTN *
> +GetSmBase (
> +  IN  UINTN  MaxNumberOfCpus
> +  )
> +{
> +  UINTN              HobCount;
> +  EFI_HOB_GUID_TYPE  *GuidHob;
> +  SMM_BASE_HOB_DATA  *SmmBaseHobData;
> +  UINTN              NumberOfProcessors;
> +  SMM_BASE_HOB_DATA  **SmBaseHobs;
> +  UINTN              *SmBaseBuffer;
> +  UINTN              HobIndex;
> +  UINTN              SortBuffer;
> +  UINTN              ProcessorIndex;
> +  UINT64             PrevProcessorIndex;
> +  EFI_HOB_GUID_TYPE  *FirstSmmBaseGuidHob;
> +
> +  SmmBaseHobData     = NULL;
> +  HobIndex           = 0;
> +  ProcessorIndex     = 0;
> +  HobCount           = 0;
> +  NumberOfProcessors = 0;
> +
> +  FirstSmmBaseGuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
> +  if (FirstSmmBaseGuidHob == NULL) {
> +    return NULL;
> +  }
> +
> +  GuidHob = FirstSmmBaseGuidHob;
> +  while (GuidHob != NULL) {
> +    HobCount++;
> +    SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
> +
> +    if (NumberOfProcessors >= MaxNumberOfCpus) {
> +      break;
> +    }
> +
> +    NumberOfProcessors += SmmBaseHobData->NumberOfProcessors;
> +    GuidHob             = GetNextGuidHob (&gSmmBaseHobGuid,
> GET_NEXT_HOB (GuidHob));
> +  }
> +
> +  ASSERT (NumberOfProcessors == MaxNumberOfCpus);
> +  if (NumberOfProcessors != MaxNumberOfCpus) {
> +    CpuDeadLoop ();
> +  }
> +
> +  SmBaseHobs = AllocatePool (sizeof (SMM_BASE_HOB_DATA *) *
> HobCount);
> +  ASSERT (SmBaseHobs != NULL);
> +  if (SmBaseHobs == NULL) {
> +    return NULL;
> +  }
> +
> +  //
> +  // Record each SmmBaseHob pointer in the SmBaseHobs.
> +  // The FirstSmmBaseGuidHob is to speed up this while-loop
> +  // without needing to look for SmBaseHob from beginning.
> +  //
> +  GuidHob = FirstSmmBaseGuidHob;
> +  while (HobIndex < HobCount) {
> +    SmBaseHobs[HobIndex++] = GET_GUID_HOB_DATA (GuidHob);
> +    GuidHob                = GetNextGuidHob (&gSmmBaseHobGuid,
> GET_NEXT_HOB (GuidHob));
> +  }
> +
> +  SmBaseBuffer = (UINTN *)AllocatePool (sizeof (UINTN) *
> (MaxNumberOfCpus));
> +  ASSERT (SmBaseBuffer != NULL);
> +  if (SmBaseBuffer == NULL) {
> +    FreePool (SmBaseHobs);
> +    return NULL;
> +  }
> +
> +  QuickSort (SmBaseHobs, HobCount, sizeof (SMM_BASE_HOB_DATA *),
> (BASE_SORT_COMPARE)SmBaseHobCompare, &SortBuffer);
> +  PrevProcessorIndex = 0;
> +  for (HobIndex = 0; HobIndex < HobCount; HobIndex++) {
> +    //
> +    // Make sure no overlap and no gap in the CPU range covered by each
> HOB
> +    //
> +    ASSERT (SmBaseHobs[HobIndex]->ProcessorIndex ==
> PrevProcessorIndex);
> +
> +    //
> +    // Cache each SmBase in order.
> +    //
> +    for (ProcessorIndex = 0; ProcessorIndex <
> SmBaseHobs[HobIndex]->NumberOfProcessors; ProcessorIndex++) {
> +      SmBaseBuffer[PrevProcessorIndex + ProcessorIndex] =
> (UINTN)SmBaseHobs[HobIndex]->SmBase[ProcessorIndex];
> +    }
> +
> +    PrevProcessorIndex += SmBaseHobs[HobIndex]->NumberOfProcessors;
> +  }
> +
> +  FreePool (SmBaseHobs);
> +  return SmBaseBuffer;
> +}
> +
>  /**
>    Function to compare 2 MP_INFORMATION2_HOB_DATA pointer based on
> ProcessorIndex.
> 
> @@ -744,27 +870,22 @@ PiCpuSmmEntry (
>    IN EFI_SYSTEM_TABLE  *SystemTable
>    )
>  {
> -  EFI_STATUS         Status;
> -  UINTN              Index;
> -  VOID               *Buffer;
> -  UINTN              BufferPages;
> -  UINTN              TileCodeSize;
> -  UINTN              TileDataSize;
> -  UINTN              TileSize;
> -  UINT8              *Stacks;
> -  VOID               *Registration;
> -  UINT32             RegEax;
> -  UINT32             RegEbx;
> -  UINT32             RegEcx;
> -  UINT32             RegEdx;
> -  UINTN              FamilyId;
> -  UINTN              ModelId;
> -  UINT32             Cr3;
> -  EFI_HOB_GUID_TYPE  *GuidHob;
> -  SMM_BASE_HOB_DATA  *SmmBaseHobData;
> -
> -  GuidHob        = NULL;
> -  SmmBaseHobData = NULL;
> +  EFI_STATUS  Status;
> +  UINTN       Index;
> +  VOID        *Buffer;
> +  UINTN       BufferPages;
> +  UINTN       TileCodeSize;
> +  UINTN       TileDataSize;
> +  UINTN       TileSize;
> +  UINT8       *Stacks;
> +  VOID        *Registration;
> +  UINT32      RegEax;
> +  UINT32      RegEbx;
> +  UINT32      RegEcx;
> +  UINT32      RegEdx;
> +  UINTN       FamilyId;
> +  UINTN       ModelId;
> +  UINT32      Cr3;
> 
>    PERF_FUNCTION_BEGIN ();
> 
> @@ -986,8 +1107,8 @@ PiCpuSmmEntry (
>    // Retrive the allocated SmmBase from gSmmBaseHobGuid. If found,
>    // means the SmBase relocation has been done.
>    //
> -  GuidHob = GetFirstGuidHob (&gSmmBaseHobGuid);
> -  if (GuidHob != NULL) {
> +  mCpuHotPlugData.SmBase = GetSmBase (mMaxNumberOfCpus);
> +  if (mCpuHotPlugData.SmBase != NULL) {
>      //
>      // Check whether the Required TileSize is enough.
>      //
> @@ -997,12 +1118,6 @@ PiCpuSmmEntry (
>        return RETURN_BUFFER_TOO_SMALL;
>      }
> 
> -    SmmBaseHobData = GET_GUID_HOB_DATA (GuidHob);
> -
> -    //
> -    // Assume single instance of HOB produced, expect the
> HOB.NumberOfProcessors equals to the mMaxNumberOfCpus.
> -    //
> -    ASSERT (SmmBaseHobData->NumberOfProcessors ==
> (UINT32)mMaxNumberOfCpus && SmmBaseHobData->ProcessorIndex == 0);
>      mSmmRelocated = TRUE;
>    } else {
>      //
> @@ -1048,8 +1163,6 @@ PiCpuSmmEntry (
>    //
>    mCpuHotPlugData.ApicId = (UINT64 *)AllocatePool (sizeof (UINT64) *
> mMaxNumberOfCpus);
>    ASSERT (mCpuHotPlugData.ApicId != NULL);
> -  mCpuHotPlugData.SmBase = (UINTN *)AllocatePool (sizeof (UINTN) *
> mMaxNumberOfCpus);
> -  ASSERT (mCpuHotPlugData.SmBase != NULL);
>    mCpuHotPlugData.ArrayLength = (UINT32)mMaxNumberOfCpus;
> 
>    //
> @@ -1058,7 +1171,9 @@ PiCpuSmmEntry (
>    // size for each CPU in the platform
>    //
>    for (Index = 0; Index < mMaxNumberOfCpus; Index++) {
> -    mCpuHotPlugData.SmBase[Index] = mSmmRelocated ?
> (UINTN)SmmBaseHobData->SmBase[Index] : (UINTN)Buffer + Index *
> TileSize - SMM_HANDLER_OFFSET;
> +    if (!mSmmRelocated) {
> +      mCpuHotPlugData.SmBase[Index] = (UINTN)Buffer + Index * TileSize
> - SMM_HANDLER_OFFSET;
> +    }
> 
>      gSmmCpuPrivate->CpuSaveStateSize[Index] = sizeof
> (SMRAM_SAVE_STATE_MAP);
>      gSmmCpuPrivate->CpuSaveState[Index]     = (VOID
> *)(mCpuHotPlugData.SmBase[Index] + SMRAM_SAVE_STATE_MAP_OFFSET);
> --
> 2.31.1.windows.1



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



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

end of thread, other threads:[~2023-12-08  8:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-07  7:32 [edk2-devel] [Patch V2 0/6] Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg duntan
2023-12-07  7:32 ` [edk2-devel] [Patch V2 1/6] UefiCpuPkg: Create " duntan
2023-12-08  8:24   ` Ni, Ray
2023-12-07  7:32 ` [edk2-devel] [Patch V2 2/6] UefiCpuPkg: Build MpInfo2HOB in CpuMpPei duntan
2023-12-07  7:32 ` [edk2-devel] [Patch V2 3/6] UefiCpuPkg: Consume MpInfo2Hob in PiSmmCpuDxe duntan
2023-12-08  8:17   ` Ni, Ray
2023-12-07  7:32 ` [edk2-devel] [Patch V2 5/6] UefiCpuPkg: Cache core type in MpInfo2 HOB duntan
2023-12-08  8:10   ` Ni, Ray
2023-12-07  7:32 ` [edk2-devel] [Patch V2 6/6] UefiCpuPkg: Avoid assuming only one smmbasehob duntan
2023-12-08  8:26   ` Ni, Ray

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