[PATCH V2 0/6] Enable perf-logging in SMM environment

[PATCH V2 0/6] Enable perf-logging in SMM environment

[Ni, Ray]

Ray Ni (6):
  UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures
  UefiCpuPkg/CpuSmm: Add perf-logging for MP procedures
  MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures
  MdeModulePkg/SmmCore: Add perf-logging for SmmDriverDispatchHandler
  MdeModulePkg/SmmPerformanceLib: Disable perf-logging after ExitBS
  MdeModulePkg/SmmCorePerformanceLib: Disable perf-logging at runtime

 MdeModulePkg/Core/PiSmmCore/Dispatcher.c      |  5 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c       | 14 ++-
 MdeModulePkg/Core/PiSmmCore/Smi.c             |  6 ++
 .../SmmCorePerformanceLib.c                   | 48 +++++++++-
 .../SmmCorePerformanceLib.inf                 |  3 +-
 .../SmmPerformanceLib/SmmPerformanceLib.c     | 63 ++++++++++++-
 .../SmmPerformanceLib/SmmPerformanceLib.inf   |  4 +
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c         | 42 ++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c    | 38 ++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  2 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  3 +
 .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 13 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c         | 91 +++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h         | 77 ++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c        |  4 +-
 15 files changed, 402 insertions(+), 11 deletions(-)
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h

-- 
2.39.1.windows.1

[PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

[Ni, Ray]

The patch adds perf-logging for the following potential
time-consuming BSP procedures:
* PiCpuSmmEntry
  - SmmRelocateBases
    * ExecuteFirstSmiInit

* BSPHandler
  - SmmWaitForApArrival
  - PerformRemainingTasks
    * InitPaging
    * SetMemMapAttributes
    * SetUefiMemMapAttributes
    * SetPageTableAttributes
    * ConfigSmmCodeAccessCheck
    * SmmCpuFeaturesCompleteSmmReadyToLock

Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c         |  8 +++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c    | 27 +++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  1 +
 .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 13 ++++++---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c        |  4 ++-
 6 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index baf827cf9d..fa666bd118 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -351,6 +351,8 @@ SmmWaitForApArrival (
   UINT32   DelayedCount;
   UINT32   BlockedCount;
 
+  PERF_FUNCTION_BEGIN ();
+
   DelayedCount = 0;
   BlockedCount = 0;
 
@@ -439,7 +441,7 @@ SmmWaitForApArrival (
     DEBUG ((DEBUG_INFO, "SmmWaitForApArrival: Delayed AP Count = %d, Blocked AP Count = %d\n", DelayedCount, BlockedCount));
   }
 
-  return;
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -577,6 +579,8 @@ BSPHandler (
   ASSERT (CpuIndex == mSmmMpSyncData->BspIndex);
   ApCount = 0;
 
+  PERF_FUNCTION_BEGIN ();
+
   //
   // Flag BSP's presence
   //
@@ -792,6 +796,8 @@ BSPHandler (
   *mSmmMpSyncData->Counter                  = 0;
   *mSmmMpSyncData->AllCpusInSync            = FALSE;
   mSmmMpSyncData->AllApArrivedWithException = FALSE;
+
+  PERF_FUNCTION_END ();
 }
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index c0e368ea94..2fc7dda682 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -410,12 +410,15 @@ ExecuteFirstSmiInit (
 {
   UINTN  Index;
 
+  PERF_FUNCTION_BEGIN ();
+
   if (mSmmInitialized == NULL) {
     mSmmInitialized = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * mMaxNumberOfCpus);
   }
 
   ASSERT (mSmmInitialized != NULL);
   if (mSmmInitialized == NULL) {
+    PERF_FUNCTION_END ();
     return;
   }
 
@@ -442,6 +445,8 @@ ExecuteFirstSmiInit (
     while (!(BOOLEAN)mSmmInitialized[Index]) {
     }
   }
+
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -463,6 +468,8 @@ SmmRelocateBases (
   UINTN                 Index;
   UINTN                 BspIndex;
 
+  PERF_FUNCTION_BEGIN ();
+
   //
   // Make sure the reserved size is large enough for procedure SmmInitTemplate.
   //
@@ -540,6 +547,7 @@ SmmRelocateBases (
   //
   CopyMem (CpuStatePtr, &BakBuf2, sizeof (BakBuf2));
   CopyMem (U8Ptr, BakBuf, sizeof (BakBuf));
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -617,6 +625,8 @@ PiCpuSmmEntry (
   GuidHob        = NULL;
   SmmBaseHobData = NULL;
 
+  PERF_FUNCTION_BEGIN ();
+
   //
   // Initialize address fixup
   //
@@ -1194,6 +1204,7 @@ PiCpuSmmEntry (
 
   DEBUG ((DEBUG_INFO, "SMM CPU Module exit from SMRAM with EFI_SUCCESS\n"));
 
+  PERF_FUNCTION_END ();
   return EFI_SUCCESS;
 }
 
@@ -1348,12 +1359,15 @@ ConfigSmmCodeAccessCheck (
   UINTN       Index;
   EFI_STATUS  Status;
 
+  PERF_FUNCTION_BEGIN ();
+
   //
   // Check to see if the Feature Control MSR is supported on this CPU
   //
   Index = gSmmCpuPrivate->SmmCoreEntryContext.CurrentlyExecutingCpu;
   if (!SmmCpuFeaturesIsSmmRegisterSupported (Index, SmmRegFeatureControl)) {
     mSmmCodeAccessCheckEnable = FALSE;
+    PERF_FUNCTION_END ();
     return;
   }
 
@@ -1363,6 +1377,7 @@ ConfigSmmCodeAccessCheck (
   //
   if ((AsmReadMsr64 (EFI_MSR_SMM_MCA_CAP) & SMM_CODE_ACCESS_CHK_BIT) == 0) {
     mSmmCodeAccessCheckEnable = FALSE;
+    PERF_FUNCTION_END ();
     return;
   }
 
@@ -1419,6 +1434,8 @@ ConfigSmmCodeAccessCheck (
       ReleaseSpinLock (mConfigSmmCodeAccessCheckLock);
     }
   }
+
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -1540,6 +1557,8 @@ PerformRemainingTasks (
   )
 {
   if (mSmmReadyToLock) {
+    PERF_FUNCTION_BEGIN ();
+
     //
     // Start SMM Profile feature
     //
@@ -1574,12 +1593,20 @@ PerformRemainingTasks (
     //
     ConfigSmmCodeAccessCheck ();
 
+    //
+    // Measure performance of SmmCpuFeaturesCompleteSmmReadyToLock() from caller side
+    // as the implementation is provided by platform.
+    //
+    PERF_START (NULL, "SmmCompleteReadyToLock", NULL, 0);
     SmmCpuFeaturesCompleteSmmReadyToLock ();
+    PERF_END (NULL, "SmmCompleteReadyToLock", NULL, 0);
 
     //
     // Clean SMM ready to lock flag
     //
     mSmmReadyToLock = FALSE;
+
+    PERF_FUNCTION_END ();
   }
 }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a5c2bdd971..b03f2ef882 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -50,6 +50,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/SmmCpuFeaturesLib.h>
 #include <Library/PeCoffGetEntryPointLib.h>
 #include <Library/RegisterCpuFeaturesLib.h>
+#include <Library/PerformanceLib.h>
 
 #include <AcpiCpuData.h>
 #include <CpuHotPlugData.h>
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 158e05e264..af66a1941c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -97,6 +97,7 @@
   ReportStatusCodeLib
   SmmCpuFeaturesLib
   PeCoffGetEntryPointLib
+  PerformanceLib
 
 [Protocols]
   gEfiSmmAccess2ProtocolGuid               ## CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 834a756061..8b21e16f1c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -1100,6 +1100,8 @@ SetMemMapAttributes (
     return;
   }
 
+  PERF_FUNCTION_BEGIN ();
+
   DEBUG ((DEBUG_INFO, "MemoryAttributesTable:\n"));
   DEBUG ((DEBUG_INFO, "  Version                   - 0x%08x\n", MemoryAttributesTable->Version));
   DEBUG ((DEBUG_INFO, "  NumberOfEntries           - 0x%08x\n", MemoryAttributesTable->NumberOfEntries));
@@ -1152,7 +1154,7 @@ SetMemMapAttributes (
   PatchSmmSaveStateMap ();
   PatchGdtIdtMap ();
 
-  return;
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -1454,6 +1456,8 @@ SetUefiMemMapAttributes (
   UINTN                  Index;
   EFI_MEMORY_DESCRIPTOR  *Entry;
 
+  PERF_FUNCTION_BEGIN ();
+
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
 
   if (mUefiMemoryMap != NULL) {
@@ -1537,6 +1541,8 @@ SetUefiMemMapAttributes (
   //
   // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().
   //
+
+  PERF_FUNCTION_END ();
 }
 
 /**
@@ -1862,6 +1868,7 @@ SetPageTableAttributes (
     return;
   }
 
+  PERF_FUNCTION_BEGIN ();
   DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));
 
   //
@@ -1900,5 +1907,5 @@ SetPageTableAttributes (
     EnableCet ();
   }
 
-  return;
+  PERF_FUNCTION_END ();
 }
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 1b0b6673e1..ed6e58065f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -575,6 +575,8 @@ InitPaging (
   IA32_CR4  Cr4;
   BOOLEAN   Enable5LevelPaging;
 
+  PERF_FUNCTION_BEGIN ();
+
   Cr4.UintN          = AsmReadCr4 ();
   Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
 
@@ -810,7 +812,7 @@ InitPaging (
   //
   mXdEnabled = TRUE;
 
-  return;
+  PERF_FUNCTION_END ();
 }
 
 /**
-- 
2.39.1.windows.1

[PATCH V2 2/6] UefiCpuPkg/CpuSmm: Add perf-logging for MP procedures

[Ni, Ray]

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

[PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures

[Ni, Ray]

Following procedures are perf-logged:
* SmmReadyToBootHandler
* SmmReadyToLockHandler
* SmmEndOfDxeHandler
* SmmEntryPoint
  (It's the main routine run in BSP when SMI happens.)
* SmiManage

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 14 +++++++++++++-
 MdeModulePkg/Core/PiSmmCore/Smi.c       |  6 ++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 875c7c0258..a15afa8dd6 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -1,7 +1,7 @@
 /** @file
   SMM Core Main Entry Point
 
-  Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -304,6 +304,7 @@ SmmReadyToBootHandler (
 {
   EFI_STATUS  Status;
   EFI_HANDLE  SmmHandle;
+  PERF_CALLBACK_BEGIN (&gEfiEventReadyToBootGuid);
 
   //
   // Install SMM Ready To Boot protocol.
@@ -318,6 +319,7 @@ SmmReadyToBootHandler (
 
   SmiHandlerUnRegister (DispatchHandle);
 
+  PERF_CALLBACK_END (&gEfiEventReadyToBootGuid);
   return Status;
 }
 
@@ -352,6 +354,8 @@ SmmReadyToLockHandler (
   EFI_HANDLE  SmmHandle;
   VOID        *Interface;
 
+  PERF_CALLBACK_BEGIN (&gEfiDxeSmmReadyToLockProtocolGuid);
+
   //
   // Unregister SMI Handlers that are no required after the SMM driver dispatch is stopped
   //
@@ -408,6 +412,7 @@ SmmReadyToLockHandler (
 
   SmramProfileReadyToLock ();
 
+  PERF_CALLBACK_END (&gEfiDxeSmmReadyToLockProtocolGuid);
   return Status;
 }
 
@@ -442,6 +447,8 @@ SmmEndOfDxeHandler (
 
   DEBUG ((DEBUG_INFO, "SmmEndOfDxeHandler\n"));
 
+  PERF_CALLBACK_BEGIN (&gEfiEndOfDxeEventGroupGuid);
+
   //
   // Install SMM EndOfDxe protocol
   //
@@ -479,6 +486,7 @@ SmmEndOfDxeHandler (
     }
   }
 
+  PERF_CALLBACK_END (&gEfiEndOfDxeEventGroupGuid);
   return EFI_SUCCESS;
 }
 
@@ -669,6 +677,8 @@ SmmEntryPoint (
   VOID                        *CommunicationBuffer;
   UINTN                       BufferSize;
 
+  PERF_FUNCTION_BEGIN ();
+
   //
   // Update SMST with contents of the SmmEntryContext structure
   //
@@ -769,6 +779,8 @@ SmmEntryPoint (
     //
     gSmmCorePrivate->InSmm = FALSE;
   }
+
+  PERF_FUNCTION_END ();
 }
 
 /**
diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 6d13969979..2985f989c3 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -109,6 +109,8 @@ SmiManage (
   BOOLEAN      SuccessReturn;
   EFI_STATUS   Status;
 
+  PERF_FUNCTION_BEGIN ();
+
   Status        = EFI_NOT_FOUND;
   SuccessReturn = FALSE;
   if (HandlerType == NULL) {
@@ -125,6 +127,7 @@ SmiManage (
       //
       // There is no handler registered for this interrupt source
       //
+      PERF_FUNCTION_END ();
       return Status;
     }
   }
@@ -148,6 +151,7 @@ SmiManage (
         // no additional handlers will be processed and EFI_INTERRUPT_PENDING will be returned.
         //
         if (HandlerType != NULL) {
+          PERF_FUNCTION_END ();
           return EFI_INTERRUPT_PENDING;
         }
 
@@ -160,6 +164,7 @@ SmiManage (
         // additional handlers will be processed.
         //
         if (HandlerType != NULL) {
+          PERF_FUNCTION_END ();
           return EFI_SUCCESS;
         }
 
@@ -194,6 +199,7 @@ SmiManage (
     Status = EFI_SUCCESS;
   }
 
+  PERF_FUNCTION_END ();
   return Status;
 }
 
-- 
2.39.1.windows.1

[PATCH V2 4/6] MdeModulePkg/SmmCore: Add perf-logging for SmmDriverDispatchHandler

[Ni, Ray]

SmmDriverDispatchHandler is the routine that dispatches SMM drivers
from FV. It's a time-consuming routine.
Add perf-logging for this routine.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
index f635565dd1..bb789e5890 100644
--- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
+++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
@@ -28,7 +28,7 @@
   Depex - Dependency Expression.
 
   Copyright (c) 2014, Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -1322,6 +1322,8 @@ SmmDriverDispatchHandler (
     return EFI_NOT_FOUND;
   }
 
+  PERF_CALLBACK_BEGIN (&gEfiEventDxeDispatchGuid);
+
   for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
     FvHandle = HandleBuffer[HandleIndex];
 
@@ -1511,6 +1513,7 @@ SmmDriverDispatchHandler (
     }
   }
 
+  PERF_CALLBACK_END (&gEfiEventDxeDispatchGuid);
   return EFI_SUCCESS;
 }
 
-- 
2.39.1.windows.1

[PATCH V2 5/6] MdeModulePkg/SmmPerformanceLib: Disable perf-logging after ExitBS

[Ni, Ray]

Because SMM perf-logging is migrated to non-SMRAM at ReadyToBoot
by DxeCorePerformanceLib, the perf-logging after ExitBS is useless and
impact the SMI latency at runtime.
Hence the SmmPerformanceLib is updated to disable perf-logging
after ExitBS.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
 .../SmmPerformanceLib/SmmPerformanceLib.c     | 63 ++++++++++++++++++-
 .../SmmPerformanceLib/SmmPerformanceLib.inf   |  4 ++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
index 623f8a978c..b9c33c0f64 100644
--- a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
+++ b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
@@ -23,6 +23,36 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // The cached SMM Performance Protocol and SMM PerformanceEx Protocol interface.
 EDKII_PERFORMANCE_MEASUREMENT_PROTOCOL  *mPerformanceMeasurement = NULL;
 BOOLEAN                                 mPerformanceMeasurementEnabled;
+VOID                                    *mPerformanceLibExitBootServicesRegistration;
+
+/**
+  This is the Event call back function is triggered in SMM to notify the Library
+  the system is entering runtime phase.
+
+  @param[in] Protocol   Points to the protocol's unique identifier
+  @param[in] Interface  Points to the interface instance
+  @param[in] Handle     The handle on which the interface was installed
+
+  @retval EFI_SUCCESS SmmAtRuntimeCallBack runs successfully
+ **/
+EFI_STATUS
+EFIAPI
+SmmPerformanceLibExitBootServicesCallback (
+  IN CONST EFI_GUID  *Protocol,
+  IN VOID            *Interface,
+  IN EFI_HANDLE      Handle
+  )
+{
+  //
+  // Disable performance measurement after ExitBootServices because
+  // 1. Performance measurement might impact SMI latency at runtime;
+  // 2. Performance log is copied to non SMRAM at ReadyToBoot so runtime performance
+  //    log is not useful.
+  //
+  mPerformanceMeasurementEnabled = FALSE;
+
+  return EFI_SUCCESS;
+}
 
 /**
   The constructor function initializes the Performance Measurement Enable flag
@@ -40,9 +70,40 @@ SmmPerformanceLibConstructor (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  EFI_STATUS  Status;
+
   mPerformanceMeasurementEnabled =  (BOOLEAN)((PcdGet8 (PcdPerformanceLibraryPropertyMask) & PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
 
-  return EFI_SUCCESS;
+  Status = gSmst->SmmRegisterProtocolNotify (
+                    &gEdkiiSmmExitBootServicesProtocolGuid,
+                    SmmPerformanceLibExitBootServicesCallback,
+                    &mPerformanceLibExitBootServicesRegistration
+                    );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
+EFI_STATUS
+EFIAPI
+SmmPerformanceLibDestructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+
+  //
+  // Unregister SmmExitBootServices notification.
+  //
+  Status = gSmst->SmmRegisterProtocolNotify (
+                    &gEdkiiSmmExitBootServicesProtocolGuid,
+                    NULL,
+                    &mPerformanceLibExitBootServicesRegistration
+                    );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
 }
 
 /**
diff --git a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
index d79cd5c8da..002462f5ca 100644
--- a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
+++ b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
@@ -21,6 +21,7 @@
   LIBRARY_CLASS                  = PerformanceLib|DXE_SMM_DRIVER
 
   CONSTRUCTOR                    = SmmPerformanceLibConstructor
+  DESTRUCTOR                     = SmmPerformanceLibDestructor
 
 #
 # The following information is for reference only and not required by the build tools.
@@ -46,5 +47,8 @@
 [Guids]
   gEdkiiSmmPerformanceMeasurementProtocolGuid          ## SOMETIMES_CONSUMES   ## UNDEFINED # Locate protocol
 
+[Protocols]
+  gEdkiiSmmExitBootServicesProtocolGuid                ## CONSUMES
+
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask    ## CONSUMES
-- 
2.39.1.windows.1

[PATCH V2 6/6] MdeModulePkg/SmmCorePerformanceLib: Disable perf-logging at runtime

[Ni, Ray]

Because SMM perf-logging is migrated to non-SMRAM at ReadyToBoot
by DxeCorePerformanceLib, the perf-logging after ExitBS is useless and
impact the SMI latency at runtime.
Hence the SmmCorePerformanceLib is updated to disable perf-logging
after ExitBS.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
 .../SmmCorePerformanceLib.c                   | 48 ++++++++++++++++++-
 .../SmmCorePerformanceLib.inf                 |  3 +-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
index 3efe56e056..c566a298dd 100644
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c
@@ -16,7 +16,7 @@
 
  SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive untrusted input and do basic validation.
 
-Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -48,6 +48,7 @@ SPIN_LOCK             mSmmFpdtLock;
 PERFORMANCE_PROPERTY  mPerformanceProperty;
 UINT32                mCachedLength   = 0;
 UINT32                mBootRecordSize = 0;
+BOOLEAN               mPerformanceMeasurementEnabled;
 
 //
 // Interfaces for SMM PerformanceMeasurement Protocol.
@@ -929,6 +930,36 @@ FpdtSmiHandler (
   return EFI_SUCCESS;
 }
 
+/**
+  This is the Event call back function is triggered in SMM to notify the Library
+  the system is entering runtime phase.
+
+  @param[in] Protocol   Points to the protocol's unique identifier
+  @param[in] Interface  Points to the interface instance
+  @param[in] Handle     The handle on which the interface was installed
+
+  @retval EFI_SUCCESS SmmAtRuntimeCallBack runs successfully
+ **/
+EFI_STATUS
+EFIAPI
+SmmCorePerformanceLibExitBootServicesCallback (
+  IN CONST EFI_GUID  *Protocol,
+  IN VOID            *Interface,
+  IN EFI_HANDLE      Handle
+  )
+{
+  //
+  // Disable performance measurement after ExitBootServices because
+  // 1. Performance measurement might impact SMI latency at runtime;
+  // 2. Performance log is copied to non SMRAM at ReadyToBoot so runtime performance
+  //    log is not useful.
+  //
+  mPerformanceMeasurementEnabled = FALSE;
+
+  return EFI_SUCCESS;
+}
+
+
 /**
   SmmBase2 protocol notify callback function, when SMST and SMM memory service get initialized
   this function is callbacked to initialize the Smm Performance Lib
@@ -948,6 +979,7 @@ InitializeSmmCorePerformanceLib (
   EFI_HANDLE            SmiHandle;
   EFI_STATUS            Status;
   PERFORMANCE_PROPERTY  *PerformanceProperty;
+  VOID                  *Registration;
 
   //
   // Initialize spin lock
@@ -987,6 +1019,16 @@ InitializeSmmCorePerformanceLib (
     Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid, &mPerformanceProperty);
     ASSERT_EFI_ERROR (Status);
   }
+
+  //
+  // Register callback function for ExitBootServices event.
+  //
+  Status = gSmst->SmmRegisterProtocolNotify (
+                    &gEdkiiSmmExitBootServicesProtocolGuid,
+                    SmmCorePerformanceLibExitBootServicesCallback,
+                    &Registration
+                    );
+  ASSERT_EFI_ERROR (Status);
 }
 
 /**
@@ -1011,6 +1053,8 @@ SmmCorePerformanceLibConstructor (
   EFI_EVENT   Event;
   VOID        *Registration;
 
+  mPerformanceMeasurementEnabled =  (BOOLEAN)((PcdGet8 (PcdPerformanceLibraryPropertyMask) & PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
+
   if (!PerformanceMeasurementEnabled ()) {
     //
     // Do not initialize performance infrastructure if not required.
@@ -1383,7 +1427,7 @@ PerformanceMeasurementEnabled (
   VOID
   )
 {
-  return (BOOLEAN)((PcdGet8 (PcdPerformanceLibraryPropertyMask) & PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
+  return mPerformanceMeasurementEnabled;
 }
 
 /**
diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
index 9eecc4b58c..9a7e14e80c 100644
--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
+++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf
@@ -8,7 +8,7 @@
 #  This library is mainly used by SMM Core to start performance logging to ensure that
 #  SMM Performance and PerformanceEx Protocol are installed at the very beginning of SMM phase.
 #
-#  Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -58,6 +58,7 @@
 
 [Protocols]
   gEfiSmmBase2ProtocolGuid                  ## CONSUMES
+  gEdkiiSmmExitBootServicesProtocolGuid     ## CONSUMES
 
 [Guids]
   ## PRODUCES ## SystemTable
-- 
2.39.1.windows.1

Re: [edk2-devel] [PATCH V2 0/6] Enable perf-logging in SMM environment

[Dong, Eric]

This patch serial looks good to me.

Thanks,
Eric

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Wednesday, May 31, 2023 7:35 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH V2 0/6] Enable perf-logging in SMM environment


Ray Ni (6):
  UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures
  UefiCpuPkg/CpuSmm: Add perf-logging for MP procedures
  MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures
  MdeModulePkg/SmmCore: Add perf-logging for SmmDriverDispatchHandler
  MdeModulePkg/SmmPerformanceLib: Disable perf-logging after ExitBS
  MdeModulePkg/SmmCorePerformanceLib: Disable perf-logging at runtime

 MdeModulePkg/Core/PiSmmCore/Dispatcher.c      |  5 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c       | 14 ++-
 MdeModulePkg/Core/PiSmmCore/Smi.c             |  6 ++
 .../SmmCorePerformanceLib.c                   | 48 +++++++++-
 .../SmmCorePerformanceLib.inf                 |  3 +-
 .../SmmPerformanceLib/SmmPerformanceLib.c     | 63 ++++++++++++-
 .../SmmPerformanceLib/SmmPerformanceLib.inf   |  4 +
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c         | 42 ++++++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c    | 38 ++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    |  2 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  3 +
 .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 13 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c         | 91 +++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h         | 77 ++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c        |  4 +-
 15 files changed, 402 insertions(+), 11 deletions(-)  create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.c
 create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMpPerf.h

--
2.39.1.windows.1

Re: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

[Wu, Jiaxin]

> 
> The patch adds perf-logging for the following potential
> time-consuming BSP procedures:
> * PiCpuSmmEntry
>   - SmmRelocateBases
>     * ExecuteFirstSmiInit
> 

ExecuteFirstSmiInit is not belong to the SmmRelocateBases. Others good to me.

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


> * BSPHandler
>   - SmmWaitForApArrival
>   - PerformRemainingTasks
>     * InitPaging
>     * SetMemMapAttributes
>     * SetUefiMemMapAttributes
>     * SetPageTableAttributes
>     * ConfigSmmCodeAccessCheck
>     * SmmCpuFeaturesCompleteSmmReadyToLock
> 

Re: [PATCH V2 2/6] UefiCpuPkg/CpuSmm: Add perf-logging for MP procedures

[Wu, Jiaxin]

> +//
> 
> +// 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>

Re: [PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures

[Wu, Jiaxin]

Could we also make below perf-logged? 

PlatformHookBeforeSmmDispatch & PlatformHookAfterSmmDispatch 

Others good to me.

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


Thanks,
Jiaxin 

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, May 31, 2023 7:35 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for
> time-consuming procedures
> 
> Following procedures are perf-logged:
> * SmmReadyToBootHandler
> * SmmReadyToLockHandler
> * SmmEndOfDxeHandler
> * SmmEntryPoint
>   (It's the main routine run in BSP when SMI happens.)
> * SmiManage
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 14 +++++++++++++-
>  MdeModulePkg/Core/PiSmmCore/Smi.c       |  6 ++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> index 875c7c0258..a15afa8dd6 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    SMM Core Main Entry Point
> 
> 
> 
> -  Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -304,6 +304,7 @@ SmmReadyToBootHandler (
>  {
> 
>    EFI_STATUS  Status;
> 
>    EFI_HANDLE  SmmHandle;
> 
> +  PERF_CALLBACK_BEGIN (&gEfiEventReadyToBootGuid);
> 
> 
> 
>    //
> 
>    // Install SMM Ready To Boot protocol.
> 
> @@ -318,6 +319,7 @@ SmmReadyToBootHandler (
> 
> 
>    SmiHandlerUnRegister (DispatchHandle);
> 
> 
> 
> +  PERF_CALLBACK_END (&gEfiEventReadyToBootGuid);
> 
>    return Status;
> 
>  }
> 
> 
> 
> @@ -352,6 +354,8 @@ SmmReadyToLockHandler (
>    EFI_HANDLE  SmmHandle;
> 
>    VOID        *Interface;
> 
> 
> 
> +  PERF_CALLBACK_BEGIN (&gEfiDxeSmmReadyToLockProtocolGuid);
> 
> +
> 
>    //
> 
>    // Unregister SMI Handlers that are no required after the SMM driver
> dispatch is stopped
> 
>    //
> 
> @@ -408,6 +412,7 @@ SmmReadyToLockHandler (
> 
> 
>    SmramProfileReadyToLock ();
> 
> 
> 
> +  PERF_CALLBACK_END (&gEfiDxeSmmReadyToLockProtocolGuid);
> 
>    return Status;
> 
>  }
> 
> 
> 
> @@ -442,6 +447,8 @@ SmmEndOfDxeHandler (
> 
> 
>    DEBUG ((DEBUG_INFO, "SmmEndOfDxeHandler\n"));
> 
> 
> 
> +  PERF_CALLBACK_BEGIN (&gEfiEndOfDxeEventGroupGuid);
> 
> +
> 
>    //
> 
>    // Install SMM EndOfDxe protocol
> 
>    //
> 
> @@ -479,6 +486,7 @@ SmmEndOfDxeHandler (
>      }
> 
>    }
> 
> 
> 
> +  PERF_CALLBACK_END (&gEfiEndOfDxeEventGroupGuid);
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> 
> 
> @@ -669,6 +677,8 @@ SmmEntryPoint (
>    VOID                        *CommunicationBuffer;
> 
>    UINTN                       BufferSize;
> 
> 
> 
> +  PERF_FUNCTION_BEGIN ();
> 
> +
> 
>    //
> 
>    // Update SMST with contents of the SmmEntryContext structure
> 
>    //
> 
> @@ -769,6 +779,8 @@ SmmEntryPoint (
>      //
> 
>      gSmmCorePrivate->InSmm = FALSE;
> 
>    }
> 
> +
> 
> +  PERF_FUNCTION_END ();
> 
>  }
> 
> 
> 
>  /**
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
> b/MdeModulePkg/Core/PiSmmCore/Smi.c
> index 6d13969979..2985f989c3 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> @@ -109,6 +109,8 @@ SmiManage (
>    BOOLEAN      SuccessReturn;
> 
>    EFI_STATUS   Status;
> 
> 
> 
> +  PERF_FUNCTION_BEGIN ();
> 
> +
> 
>    Status        = EFI_NOT_FOUND;
> 
>    SuccessReturn = FALSE;
> 
>    if (HandlerType == NULL) {
> 
> @@ -125,6 +127,7 @@ SmiManage (
>        //
> 
>        // There is no handler registered for this interrupt source
> 
>        //
> 
> +      PERF_FUNCTION_END ();
> 
>        return Status;
> 
>      }
> 
>    }
> 
> @@ -148,6 +151,7 @@ SmiManage (
>          // no additional handlers will be processed and
> EFI_INTERRUPT_PENDING will be returned.
> 
>          //
> 
>          if (HandlerType != NULL) {
> 
> +          PERF_FUNCTION_END ();
> 
>            return EFI_INTERRUPT_PENDING;
> 
>          }
> 
> 
> 
> @@ -160,6 +164,7 @@ SmiManage (
>          // additional handlers will be processed.
> 
>          //
> 
>          if (HandlerType != NULL) {
> 
> +          PERF_FUNCTION_END ();
> 
>            return EFI_SUCCESS;
> 
>          }
> 
> 
> 
> @@ -194,6 +199,7 @@ SmiManage (
>      Status = EFI_SUCCESS;
> 
>    }
> 
> 
> 
> +  PERF_FUNCTION_END ();
> 
>    return Status;
> 
>  }
> 
> 
> 
> --
> 2.39.1.windows.1

Re: [PATCH V2 4/6] MdeModulePkg/SmmCore: Add perf-logging for SmmDriverDispatchHandler

[Wu, Jiaxin]

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



> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, May 31, 2023 7:35 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH V2 4/6] MdeModulePkg/SmmCore: Add perf-logging for
> SmmDriverDispatchHandler
> 
> SmmDriverDispatchHandler is the routine that dispatches SMM drivers
> from FV. It's a time-consuming routine.
> Add perf-logging for this routine.
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> index f635565dd1..bb789e5890 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
> @@ -28,7 +28,7 @@
>    Depex - Dependency Expression.
> 
> 
> 
>    Copyright (c) 2014, Hewlett-Packard Development Company, L.P.
> 
> -  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -1322,6 +1322,8 @@ SmmDriverDispatchHandler (
>      return EFI_NOT_FOUND;
> 
>    }
> 
> 
> 
> +  PERF_CALLBACK_BEGIN (&gEfiEventDxeDispatchGuid);
> 
> +
> 
>    for (HandleIndex = 0; HandleIndex < HandleCount; HandleIndex++) {
> 
>      FvHandle = HandleBuffer[HandleIndex];
> 
> 
> 
> @@ -1511,6 +1513,7 @@ SmmDriverDispatchHandler (
>      }
> 
>    }
> 
> 
> 
> +  PERF_CALLBACK_END (&gEfiEventDxeDispatchGuid);
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> 
> 
> --
> 2.39.1.windows.1

Re: [PATCH V2 5/6] MdeModulePkg/SmmPerformanceLib: Disable perf-logging after ExitBS

[Wu, Jiaxin]

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



> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, May 31, 2023 7:35 PM
> To: devel@edk2.groups.io
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Wang, Jian J
> <jian.j.wang@intel.com>
> Subject: [PATCH V2 5/6] MdeModulePkg/SmmPerformanceLib: Disable perf-
> logging after ExitBS
> 
> Because SMM perf-logging is migrated to non-SMRAM at ReadyToBoot
> by DxeCorePerformanceLib, the perf-logging after ExitBS is useless and
> impact the SMI latency at runtime.
> Hence the SmmPerformanceLib is updated to disable perf-logging
> after ExitBS.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>  .../SmmPerformanceLib/SmmPerformanceLib.c     | 63
> ++++++++++++++++++-
>  .../SmmPerformanceLib/SmmPerformanceLib.inf   |  4 ++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git
> a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
> b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
> index 623f8a978c..b9c33c0f64 100644
> --- a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
> +++ b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.c
> @@ -23,6 +23,36 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  // The cached SMM Performance Protocol and SMM PerformanceEx
> Protocol interface.
> 
>  EDKII_PERFORMANCE_MEASUREMENT_PROTOCOL
> *mPerformanceMeasurement = NULL;
> 
>  BOOLEAN                                 mPerformanceMeasurementEnabled;
> 
> +VOID                                    *mPerformanceLibExitBootServicesRegistration;
> 
> +
> 
> +/**
> 
> +  This is the Event call back function is triggered in SMM to notify the Library
> 
> +  the system is entering runtime phase.
> 
> +
> 
> +  @param[in] Protocol   Points to the protocol's unique identifier
> 
> +  @param[in] Interface  Points to the interface instance
> 
> +  @param[in] Handle     The handle on which the interface was installed
> 
> +
> 
> +  @retval EFI_SUCCESS SmmAtRuntimeCallBack runs successfully
> 
> + **/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +SmmPerformanceLibExitBootServicesCallback (
> 
> +  IN CONST EFI_GUID  *Protocol,
> 
> +  IN VOID            *Interface,
> 
> +  IN EFI_HANDLE      Handle
> 
> +  )
> 
> +{
> 
> +  //
> 
> +  // Disable performance measurement after ExitBootServices because
> 
> +  // 1. Performance measurement might impact SMI latency at runtime;
> 
> +  // 2. Performance log is copied to non SMRAM at ReadyToBoot so runtime
> performance
> 
> +  //    log is not useful.
> 
> +  //
> 
> +  mPerformanceMeasurementEnabled = FALSE;
> 
> +
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> 
> 
>  /**
> 
>    The constructor function initializes the Performance Measurement Enable
> flag
> 
> @@ -40,9 +70,40 @@ SmmPerformanceLibConstructor (
>    IN EFI_SYSTEM_TABLE  *SystemTable
> 
>    )
> 
>  {
> 
> +  EFI_STATUS  Status;
> 
> +
> 
>    mPerformanceMeasurementEnabled =  (BOOLEAN)((PcdGet8
> (PcdPerformanceLibraryPropertyMask) &
> PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
> 
> 
> 
> -  return EFI_SUCCESS;
> 
> +  Status = gSmst->SmmRegisterProtocolNotify (
> 
> +                    &gEdkiiSmmExitBootServicesProtocolGuid,
> 
> +                    SmmPerformanceLibExitBootServicesCallback,
> 
> +                    &mPerformanceLibExitBootServicesRegistration
> 
> +                    );
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +SmmPerformanceLibDestructor (
> 
> +  IN EFI_HANDLE        ImageHandle,
> 
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS  Status;
> 
> +
> 
> +  //
> 
> +  // Unregister SmmExitBootServices notification.
> 
> +  //
> 
> +  Status = gSmst->SmmRegisterProtocolNotify (
> 
> +                    &gEdkiiSmmExitBootServicesProtocolGuid,
> 
> +                    NULL,
> 
> +                    &mPerformanceLibExitBootServicesRegistration
> 
> +                    );
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +
> 
> +  return Status;
> 
>  }
> 
> 
> 
>  /**
> 
> diff --git
> a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
> b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
> index d79cd5c8da..002462f5ca 100644
> --- a/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
> +++
> b/MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanceLib.inf
> @@ -21,6 +21,7 @@
>    LIBRARY_CLASS                  = PerformanceLib|DXE_SMM_DRIVER
> 
> 
> 
>    CONSTRUCTOR                    = SmmPerformanceLibConstructor
> 
> +  DESTRUCTOR                     = SmmPerformanceLibDestructor
> 
> 
> 
>  #
> 
>  # The following information is for reference only and not required by the
> build tools.
> 
> @@ -46,5 +47,8 @@
>  [Guids]
> 
>    gEdkiiSmmPerformanceMeasurementProtocolGuid          ##
> SOMETIMES_CONSUMES   ## UNDEFINED # Locate protocol
> 
> 
> 
> +[Protocols]
> 
> +  gEdkiiSmmExitBootServicesProtocolGuid                ## CONSUMES
> 
> +
> 
>  [Pcd]
> 
>    gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask    ##
> CONSUMES
> 
> --
> 2.39.1.windows.1

Re: [PATCH V2 6/6] MdeModulePkg/SmmCorePerformanceLib: Disable perf-logging at runtime

[Wu, Jiaxin]

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



> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, May 31, 2023 7:35 PM
> To: devel@edk2.groups.io
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Wang, Jian J
> <jian.j.wang@intel.com>
> Subject: [PATCH V2 6/6] MdeModulePkg/SmmCorePerformanceLib: Disable
> perf-logging at runtime
> 
> Because SMM perf-logging is migrated to non-SMRAM at ReadyToBoot
> by DxeCorePerformanceLib, the perf-logging after ExitBS is useless and
> impact the SMI latency at runtime.
> Hence the SmmCorePerformanceLib is updated to disable perf-logging
> after ExitBS.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>  .../SmmCorePerformanceLib.c                   | 48 ++++++++++++++++++-
>  .../SmmCorePerformanceLib.inf                 |  3 +-
>  2 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.c
> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.c
> index 3efe56e056..c566a298dd 100644
> ---
> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.c
> +++
> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.c
> @@ -16,7 +16,7 @@
> 
> 
>   SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive
> untrusted input and do basic validation.
> 
> 
> 
> -Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -48,6 +48,7 @@ SPIN_LOCK             mSmmFpdtLock;
>  PERFORMANCE_PROPERTY  mPerformanceProperty;
> 
>  UINT32                mCachedLength   = 0;
> 
>  UINT32                mBootRecordSize = 0;
> 
> +BOOLEAN               mPerformanceMeasurementEnabled;
> 
> 
> 
>  //
> 
>  // Interfaces for SMM PerformanceMeasurement Protocol.
> 
> @@ -929,6 +930,36 @@ FpdtSmiHandler (
>    return EFI_SUCCESS;
> 
>  }
> 
> 
> 
> +/**
> 
> +  This is the Event call back function is triggered in SMM to notify the Library
> 
> +  the system is entering runtime phase.
> 
> +
> 
> +  @param[in] Protocol   Points to the protocol's unique identifier
> 
> +  @param[in] Interface  Points to the interface instance
> 
> +  @param[in] Handle     The handle on which the interface was installed
> 
> +
> 
> +  @retval EFI_SUCCESS SmmAtRuntimeCallBack runs successfully
> 
> + **/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +SmmCorePerformanceLibExitBootServicesCallback (
> 
> +  IN CONST EFI_GUID  *Protocol,
> 
> +  IN VOID            *Interface,
> 
> +  IN EFI_HANDLE      Handle
> 
> +  )
> 
> +{
> 
> +  //
> 
> +  // Disable performance measurement after ExitBootServices because
> 
> +  // 1. Performance measurement might impact SMI latency at runtime;
> 
> +  // 2. Performance log is copied to non SMRAM at ReadyToBoot so runtime
> performance
> 
> +  //    log is not useful.
> 
> +  //
> 
> +  mPerformanceMeasurementEnabled = FALSE;
> 
> +
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> +
> 
> +
> 
>  /**
> 
>    SmmBase2 protocol notify callback function, when SMST and SMM memory
> service get initialized
> 
>    this function is callbacked to initialize the Smm Performance Lib
> 
> @@ -948,6 +979,7 @@ InitializeSmmCorePerformanceLib (
>    EFI_HANDLE            SmiHandle;
> 
>    EFI_STATUS            Status;
> 
>    PERFORMANCE_PROPERTY  *PerformanceProperty;
> 
> +  VOID                  *Registration;
> 
> 
> 
>    //
> 
>    // Initialize spin lock
> 
> @@ -987,6 +1019,16 @@ InitializeSmmCorePerformanceLib (
>      Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid,
> &mPerformanceProperty);
> 
>      ASSERT_EFI_ERROR (Status);
> 
>    }
> 
> +
> 
> +  //
> 
> +  // Register callback function for ExitBootServices event.
> 
> +  //
> 
> +  Status = gSmst->SmmRegisterProtocolNotify (
> 
> +                    &gEdkiiSmmExitBootServicesProtocolGuid,
> 
> +                    SmmCorePerformanceLibExitBootServicesCallback,
> 
> +                    &Registration
> 
> +                    );
> 
> +  ASSERT_EFI_ERROR (Status);
> 
>  }
> 
> 
> 
>  /**
> 
> @@ -1011,6 +1053,8 @@ SmmCorePerformanceLibConstructor (
>    EFI_EVENT   Event;
> 
>    VOID        *Registration;
> 
> 
> 
> +  mPerformanceMeasurementEnabled =  (BOOLEAN)((PcdGet8
> (PcdPerformanceLibraryPropertyMask) &
> PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
> 
> +
> 
>    if (!PerformanceMeasurementEnabled ()) {
> 
>      //
> 
>      // Do not initialize performance infrastructure if not required.
> 
> @@ -1383,7 +1427,7 @@ PerformanceMeasurementEnabled (
>    VOID
> 
>    )
> 
>  {
> 
> -  return (BOOLEAN)((PcdGet8 (PcdPerformanceLibraryPropertyMask) &
> PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0);
> 
> +  return mPerformanceMeasurementEnabled;
> 
>  }
> 
> 
> 
>  /**
> 
> diff --git
> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.inf
> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.inf
> index 9eecc4b58c..9a7e14e80c 100644
> ---
> a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.inf
> +++
> b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformance
> Lib.inf
> @@ -8,7 +8,7 @@
>  #  This library is mainly used by SMM Core to start performance logging to
> ensure that
> 
>  #  SMM Performance and PerformanceEx Protocol are installed at the very
> beginning of SMM phase.
> 
>  #
> 
> -#  Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.<BR>
> 
> +#  Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -58,6 +58,7 @@
> 
> 
>  [Protocols]
> 
>    gEfiSmmBase2ProtocolGuid                  ## CONSUMES
> 
> +  gEdkiiSmmExitBootServicesProtocolGuid     ## CONSUMES
> 
> 
> 
>  [Guids]
> 
>    ## PRODUCES ## SystemTable
> 
> --
> 2.39.1.windows.1

Re: [PATCH V2 2/6] UefiCpuPkg/CpuSmm: Add perf-logging for MP procedures

[Ni, Ray]

> 
> 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>
> 
> 

Re: [PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures

[Ni, Ray]

We can add perf-logging for the two functions in a separate patch.

________________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Friday, June 9, 2023 22:36
To: Ni, Ray; devel@edk2.groups.io
Cc: Wang, Jian J; Gao, Liming
Subject: RE: [PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for time-consuming procedures

Could we also make below perf-logged?

PlatformHookBeforeSmmDispatch & PlatformHookAfterSmmDispatch

Others good to me.

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


Thanks,
Jiaxin

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, May 31, 2023 7:35 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH V2 3/6] MdeModulePkg/SmmCore: Add perf-logging for
> time-consuming procedures
>
> Following procedures are perf-logged:
> * SmmReadyToBootHandler
> * SmmReadyToLockHandler
> * SmmEndOfDxeHandler
> * SmmEntryPoint
>   (It's the main routine run in BSP when SMI happens.)
> * SmiManage
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 14 +++++++++++++-
>  MdeModulePkg/Core/PiSmmCore/Smi.c       |  6 ++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> index 875c7c0258..a15afa8dd6 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
> @@ -1,7 +1,7 @@
>  /** @file
>
>    SMM Core Main Entry Point
>
>
>
> -  Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
>
> +  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
>  **/
>
> @@ -304,6 +304,7 @@ SmmReadyToBootHandler (
>  {
>
>    EFI_STATUS  Status;
>
>    EFI_HANDLE  SmmHandle;
>
> +  PERF_CALLBACK_BEGIN (&gEfiEventReadyToBootGuid);
>
>
>
>    //
>
>    // Install SMM Ready To Boot protocol.
>
> @@ -318,6 +319,7 @@ SmmReadyToBootHandler (
>
>
>    SmiHandlerUnRegister (DispatchHandle);
>
>
>
> +  PERF_CALLBACK_END (&gEfiEventReadyToBootGuid);
>
>    return Status;
>
>  }
>
>
>
> @@ -352,6 +354,8 @@ SmmReadyToLockHandler (
>    EFI_HANDLE  SmmHandle;
>
>    VOID        *Interface;
>
>
>
> +  PERF_CALLBACK_BEGIN (&gEfiDxeSmmReadyToLockProtocolGuid);
>
> +
>
>    //
>
>    // Unregister SMI Handlers that are no required after the SMM driver
> dispatch is stopped
>
>    //
>
> @@ -408,6 +412,7 @@ SmmReadyToLockHandler (
>
>
>    SmramProfileReadyToLock ();
>
>
>
> +  PERF_CALLBACK_END (&gEfiDxeSmmReadyToLockProtocolGuid);
>
>    return Status;
>
>  }
>
>
>
> @@ -442,6 +447,8 @@ SmmEndOfDxeHandler (
>
>
>    DEBUG ((DEBUG_INFO, "SmmEndOfDxeHandler\n"));
>
>
>
> +  PERF_CALLBACK_BEGIN (&gEfiEndOfDxeEventGroupGuid);
>
> +
>
>    //
>
>    // Install SMM EndOfDxe protocol
>
>    //
>
> @@ -479,6 +486,7 @@ SmmEndOfDxeHandler (
>      }
>
>    }
>
>
>
> +  PERF_CALLBACK_END (&gEfiEndOfDxeEventGroupGuid);
>
>    return EFI_SUCCESS;
>
>  }
>
>
>
> @@ -669,6 +677,8 @@ SmmEntryPoint (
>    VOID                        *CommunicationBuffer;
>
>    UINTN                       BufferSize;
>
>
>
> +  PERF_FUNCTION_BEGIN ();
>
> +
>
>    //
>
>    // Update SMST with contents of the SmmEntryContext structure
>
>    //
>
> @@ -769,6 +779,8 @@ SmmEntryPoint (
>      //
>
>      gSmmCorePrivate->InSmm = FALSE;
>
>    }
>
> +
>
> +  PERF_FUNCTION_END ();
>
>  }
>
>
>
>  /**
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
> b/MdeModulePkg/Core/PiSmmCore/Smi.c
> index 6d13969979..2985f989c3 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> @@ -109,6 +109,8 @@ SmiManage (
>    BOOLEAN      SuccessReturn;
>
>    EFI_STATUS   Status;
>
>
>
> +  PERF_FUNCTION_BEGIN ();
>
> +
>
>    Status        = EFI_NOT_FOUND;
>
>    SuccessReturn = FALSE;
>
>    if (HandlerType == NULL) {
>
> @@ -125,6 +127,7 @@ SmiManage (
>        //
>
>        // There is no handler registered for this interrupt source
>
>        //
>
> +      PERF_FUNCTION_END ();
>
>        return Status;
>
>      }
>
>    }
>
> @@ -148,6 +151,7 @@ SmiManage (
>          // no additional handlers will be processed and
> EFI_INTERRUPT_PENDING will be returned.
>
>          //
>
>          if (HandlerType != NULL) {
>
> +          PERF_FUNCTION_END ();
>
>            return EFI_INTERRUPT_PENDING;
>
>          }
>
>
>
> @@ -160,6 +164,7 @@ SmiManage (
>          // additional handlers will be processed.
>
>          //
>
>          if (HandlerType != NULL) {
>
> +          PERF_FUNCTION_END ();
>
>            return EFI_SUCCESS;
>
>          }
>
>
>
> @@ -194,6 +199,7 @@ SmiManage (
>      Status = EFI_SUCCESS;
>
>    }
>
>
>
> +  PERF_FUNCTION_END ();
>
>    return Status;
>
>  }
>
>
>
> --
> 2.39.1.windows.1

Re: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

[Ni, Ray]

________________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Friday, June 9, 2023 21:42
To: devel@edk2.groups.io; Ni, Ray
Cc: Dong, Eric; Kumar, Rahul R; Gerd Hoffmann
Subject: RE: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

>
> The patch adds perf-logging for the following potential
> time-consuming BSP procedures:
> * PiCpuSmmEntry
>   - SmmRelocateBases
>     * ExecuteFirstSmiInit
>

> ExecuteFirstSmiInit is not belong to the SmmRelocateBases. Others good to me.
ExecuteFirstSmiInit contributes to the overall running time when the CpuSmm driver relocates the SMM Base.


> * BSPHandler
>   - SmmWaitForApArrival
>   - PerformRemainingTasks
>     * InitPaging
>     * SetMemMapAttributes
>     * SetUefiMemMapAttributes
>     * SetPageTableAttributes
>     * ConfigSmmCodeAccessCheck
>     * SmmCpuFeaturesCompleteSmmReadyToLock
>

Re: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

[Ni, Ray]

Jiaxin,
you are right!

ExecuteFirstSmiInit doesn't belong to SmmRelocateBases.
I will update commit message.

________________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Monday, June 12, 2023 13:16
To: Wu, Jiaxin; devel@edk2.groups.io
Cc: Dong, Eric; Kumar, Rahul R; Gerd Hoffmann
Subject: Re: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures



________________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Friday, June 9, 2023 21:42
To: devel@edk2.groups.io; Ni, Ray
Cc: Dong, Eric; Kumar, Rahul R; Gerd Hoffmann
Subject: RE: [edk2-devel] [PATCH V2 1/6] UefiCpuPkg/CpuSmm: Add perf-logging for time-consuming BSP procedures

>
> The patch adds perf-logging for the following potential
> time-consuming BSP procedures:
> * PiCpuSmmEntry
>   - SmmRelocateBases
>     * ExecuteFirstSmiInit
>

> ExecuteFirstSmiInit is not belong to the SmmRelocateBases. Others good to me.
ExecuteFirstSmiInit contributes to the overall running time when the CpuSmm driver relocates the SMM Base.


> * BSPHandler
>   - SmmWaitForApArrival
>   - PerformRemainingTasks
>     * InitPaging
>     * SetMemMapAttributes
>     * SetUefiMemMapAttributes
>     * SetPageTableAttributes
>     * ConfigSmmCodeAccessCheck
>     * SmmCpuFeaturesCompleteSmmReadyToLock
>