MP procedures are those procedures that run in every CPU thread.
The EDKII perf infra is not MP safe so it doesn't support to be called
from those MP procedures.

The patch adds SMM MP perf-logging support in SmmMpPerf.c.
The following procedures are perf-logged:
* SmmInitHandler
* SmmCpuFeaturesRendezvousEntry
* PlatformValidSmi
* SmmCpuFeaturesRendezvousExit

Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c        | 34 ++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 11 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |  1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  2 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c        | 91 ++++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h        | 77 +++++++++++++++++
 6 files changed, 216 insertions(+)
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index fa666bd118..bcd90f0671 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -778,6 +778,15 @@ BSPHandler (
   //
   WaitForAllAPs (ApCount);
 
+  //
+  // At this point, all APs should have exited from APHandler().
+  // Migrate the SMM MP performance logging to standard SMM performance logging.
+  // Any SMM MP performance logging after this point will be migrated in next SMI.
+  //
+  PERF_CODE (
+    MigrateMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
+    );
+
   //
   // Reset the tokens buffer.
   //
@@ -1769,12 +1778,24 @@ SmiRendezvous (
   //
   // Perform CPU specific entry hooks
   //
+  PERF_CODE (
+    MpPerfBegin (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (SmmRendezvousEntry));
+    );
   SmmCpuFeaturesRendezvousEntry (CpuIndex);
+  PERF_CODE (
+    MpPerfEnd (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (SmmRendezvousEntry));
+    );
 
   //
   // Determine if this is a valid SMI
   //
+  PERF_CODE (
+    MpPerfBegin (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (PlatformValidSmi));
+    );
   ValidSmi = PlatformValidSmi ();
+  PERF_CODE (
+    MpPerfEnd (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (PlatformValidSmi));
+    );
 
   //
   // Determine if BSP has been already in progress. Note this must be checked after
@@ -1904,7 +1925,20 @@ SmiRendezvous (
   }
 
 Exit:
+  //
+  // Note: SmmRendezvousExit perf-logging entry is the only one that will be
+  //       migrated to standard perf-logging database in next SMI by BSPHandler().
+  //       Hence, the number of SmmRendezvousEntry entries will be larger than
+  //       the number of SmmRendezvousExit entries. Delta equals to the number
+  //       of CPU threads.
+  //
+  PERF_CODE (
+    MpPerfBegin (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (SmmRendezvousExit));
+    );
   SmmCpuFeaturesRendezvousExit (CpuIndex);
+  PERF_CODE (
+    MpPerfEnd (CpuIndex, SMM_MP_PERF_PROCEDURE_ID (SmmRendezvousExit));
+    );
 
   //
   // Restore Cr2
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2fc7dda682..5afab1e040 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -362,6 +362,9 @@ SmmInitHandler (
 
   for (Index = 0; Index < mNumberOfCpus; Index++) {
     if (ApicId == (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
+      PERF_CODE (
+        MpPerfBegin (Index, SMM_MP_PERF_PROCEDURE_ID (SmmInitHandler));
+        );
       //
       // Initialize SMM specific features on the currently executing CPU
       //
@@ -392,6 +395,10 @@ SmmInitHandler (
         SemaphoreHook (Index, &mRebased[Index]);
       }
 
+      PERF_CODE (
+        MpPerfEnd (Index, SMM_MP_PERF_PROCEDURE_ID (SmmInitHandler));
+        );
+
       return;
     }
   }
@@ -699,6 +706,10 @@ PiCpuSmmEntry (
 
   gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus = mMaxNumberOfCpus;
 
+  PERF_CODE (
+    InitializeMpPerf (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus);
+    );
+
   //
   // The CPU save state and code for the SMI entry point are tiled within an SMRAM
   // allocated buffer.  The minimum size of this buffer for a uniprocessor system
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index b03f2ef882..1876a27cae 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -60,6 +60,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "CpuService.h"
 #include "SmmProfile.h"
+#include "SmmMpPerf.h"
 
 //
 // CET definition
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index af66a1941c..4864532c35 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -42,6 +42,8 @@
   SmmCpuMemoryManagement.c
   SmmMp.h
   SmmMp.c
+  SmmMpPerf.h
+  SmmMpPerf.c
 
 [Sources.Ia32]
   Ia32/Semaphore.c
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
new file mode 100644
index 0000000000..c13556af46
--- /dev/null
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
@@ -0,0 +1,91 @@
+/** @file
+SMM MP perf-logging implementation
+
+Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "PiSmmCpuDxeSmm.h"
+
+
+#define  SMM_MP_PERF_PROCEDURE_NAME(procedure)  # procedure
+GLOBAL_REMOVE_IF_UNREFERENCED
+CHAR8  *gSmmMpPerfProcedureName[] = {
+  SMM_MP_PERF_PROCEDURE_LIST (SMM_MP_PERF_PROCEDURE_NAME)
+};
+//
+// Each element holds the performance data for one processor.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+SMM_PERF_AP_PROCEDURE_PERFORMANCE  *mSmmMpProcedurePerformance = NULL;
+
+/**
+  Initialize the perf-logging feature for APs.
+
+  @param NumberofCpus    Number of processors in the platform.
+**/
+VOID
+InitializeMpPerf (
+  UINTN  NumberofCpus
+  )
+{
+  mSmmMpProcedurePerformance = AllocateZeroPool (NumberofCpus * sizeof (*mSmmMpProcedurePerformance));
+  ASSERT (mSmmMpProcedurePerformance != NULL);
+}
+
+/**
+  Migrate MP performance data to standardized performance database.
+
+  @param NumberofCpus    Number of processors in the platform.
+**/
+VOID
+MigrateMpPerf (
+  UINTN  NumberofCpus
+  )
+{
+  UINTN  CpuIndex;
+  UINTN  MpProcecureId;
+
+  for (CpuIndex = 0; CpuIndex < NumberofCpus; CpuIndex++) {
+    for (MpProcecureId = 0; MpProcecureId < SMM_MP_PERF_PROCEDURE_ID (SmmMpProcedureMax); MpProcecureId++) {
+      if (mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId] != 0) {
+        PERF_START (NULL, gSmmMpPerfProcedureName[MpProcecureId], NULL, mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcecureId]);
+        PERF_END (NULL, gSmmMpPerfProcedureName[MpProcecureId], NULL, mSmmMpProcedurePerformance[CpuIndex].End[MpProcecureId]);
+      }
+    }
+  }
+
+  ZeroMem (mSmmMpProcedurePerformance, NumberofCpus * sizeof (*mSmmMpProcedurePerformance));
+}
+
+/**
+  Save the performance counter value before running the MP procedure.
+
+  @param CpuIndex        The index of the CPU.
+  @param MpProcedureId   The ID of the MP procedure.
+**/
+VOID
+MpPerfBegin (
+  IN UINTN  CpuIndex,
+  IN UINTN  MpProcedureId
+  )
+{
+  mSmmMpProcedurePerformance[CpuIndex].Begin[MpProcedureId] = GetPerformanceCounter ();
+}
+
+/**
+  Save the performance counter value after running the MP procedure.
+
+  @param CpuIndex        The index of the CPU.
+  @param MpProcedureId   The ID of the MP procedure.
+**/
+VOID
+MpPerfEnd (
+  IN UINTN  CpuIndex,
+  IN UINTN  MpProcedureId
+  )
+{
+  mSmmMpProcedurePerformance[CpuIndex].End[MpProcedureId] = GetPerformanceCounter ();
+}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
new file mode 100644
index 0000000000..b148a99e86
--- /dev/null
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h
@@ -0,0 +1,77 @@
+/** @file
+SMM MP perf-logging implementation
+
+Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MP_PERF_H_
+#define _MP_PERF_H_
+
+//
+// The list of all MP procedures that need to be perf-logged.
+//
+#define  SMM_MP_PERF_PROCEDURE_LIST(_) \
+  _(SmmInitHandler), \
+  _(SmmRendezvousEntry), \
+  _(PlatformValidSmi), \
+  _(SmmRendezvousExit), \
+  _(SmmMpProcedureMax) // Add new entries above this line
+
+#define  SMM_MP_PERF_PROCEDURE_ID(procedure)  SmmMpProcedureId ## procedure
+enum {
+  SMM_MP_PERF_PROCEDURE_LIST (SMM_MP_PERF_PROCEDURE_ID)
+};
+
+typedef struct {
+  UINT64    Begin[SMM_MP_PERF_PROCEDURE_ID (SmmMpProcedureMax)];
+  UINT64    End[SMM_MP_PERF_PROCEDURE_ID (SmmMpProcedureMax)];
+} SMM_PERF_AP_PROCEDURE_PERFORMANCE;
+
+/**
+  Initialize the perf-logging feature for APs.
+
+  @param NumberofCpus    Number of processors in the platform.
+**/
+VOID
+InitializeMpPerf (
+  UINTN  NumberofCpus
+  );
+
+/**
+  Migrate MP performance data to standardized performance database.
+
+  @param NumberofCpus    Number of processors in the platform.
+**/
+VOID
+MigrateMpPerf (
+  UINTN  NumberofCpus
+  );
+
+/**
+  Save the performance counter value before running the MP procedure.
+
+  @param CpuIndex        The index of the CPU.
+  @param MpProcedureId   The ID of the MP procedure.
+**/
+VOID
+MpPerfBegin (
+  IN UINTN  CpuIndex,
+  IN UINTN  MpProcedureId
+  );
+
+/**
+  Save the performance counter value after running the MP procedure.
+
+  @param CpuIndex        The index of the CPU.
+  @param MpProcedureId   The ID of the MP procedure.
+**/
+VOID
+MpPerfEnd (
+  IN UINTN  CpuIndex,
+  IN UINTN  MpProcedureId
+  );
+
+#endif
-- 
2.39.1.windows.1



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

> +//
> 
> +// The list of all MP procedures that need to be perf-logged.
> 
> +//
> 
> +#define  SMM_MP_PERF_PROCEDURE_LIST(_) \
> 
> +  _(SmmInitHandler), \
> 
> +  _(SmmRendezvousEntry), \
> 
> +  _(PlatformValidSmi), \
> 
> +  _(SmmRendezvousExit), \
> 
> +  _(SmmMpProcedureMax) // Add new entries above this line
> 
> +

SmmRendezvousEntry is for the SmmCpuFeaturesRendezvousEntry
SmmRendezvousExit is for the SmmCpuFeaturesRendezvousExit

Since the name might not be same as function, could we add the function name as the comment after the entries? For example:

_(SmmRendezvousEntry), \     /// Map to SmmCpuFeaturesRendezvousEntry
_(SmmRendezvousExit), \       /// Map to SmmCpuFeaturesRendezvousExit

> 
> +#define  SMM_MP_PERF_PROCEDURE_ID(procedure)
> SmmMpProcedureId ## procedure
> 
> +enum {
> 
> +  SMM_MP_PERF_PROCEDURE_LIST (SMM_MP_PERF_PROCEDURE_ID)
> 
> +};


Could we add the comments to this enum? It defines the ID of the MP procedure. For MP procedure,  it needs call SMM_MP_PERF_PROCEDURE_ID with entries name defined in the SMM_MP_PERF_PROCEDURE_LIST.



Others good to me.

Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>





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

> 
> Could we add the comments to this enum? It defines the ID of the MP procedure.
> For MP procedure,  it needs call SMM_MP_PERF_PROCEDURE_ID with entries
> name defined in the SMM_MP_PERF_PROCEDURE_LIST.

Thanks. Will add the comments.

> 
> 
> 
> Others good to me.
> 
> Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> 
> 



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