[PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array

[PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array

[Jason Lou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3508

Sort the CpuCacheInfo array by CPU package ID, core type, cache level
and cache type.

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
 UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c         | 47 +++++++++++++++++++-
 UefiCpuPkg/Include/Library/CpuCacheInfoLib.h                 |  2 +-
 UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf    |  4 +-
 UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h | 27 +++++++++++
 UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf    |  4 +-
 5 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
index 126ee0da86..7474fe0f50 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
@@ -37,6 +37,47 @@ CpuCacheInfoPrintCpuCacheInfoTable (
   DEBUG ((DEBUG_INFO, "+-------+--------------------------------------------------------------------------------------+\n"));
 }
 
+/**
+  Function to compare CPU package ID, core type, cache level and cache type for use in QuickSort.
+
+  @param[in]  Buffer1             pointer to CPU_CACHE_INFO poiner to compare
+  @param[in]  Buffer2             pointer to second CPU_CACHE_INFO pointer to compare
+
+  @retval  0                      Buffer1 equal to Buffer2
+  @retval  1                      Buffer1 is greater than Buffer2
+  @retval  -1                     Buffer1 is less than Buffer2
+**/
+INTN
+EFIAPI
+CpuCacheInfoCompare (
+  IN CONST VOID             *Buffer1,
+  IN CONST VOID             *Buffer2
+  )
+{
+  CPU_CACHE_INFO_COMPARATOR Comparator1, Comparator2;
+
+  ZeroMem (&Comparator1, sizeof (Comparator1));
+  ZeroMem (&Comparator2, sizeof (Comparator2));
+
+  Comparator1.Fields.Package    = ((CPU_CACHE_INFO*)Buffer1)->Package;
+  Comparator1.Fields.CoreType   = ((CPU_CACHE_INFO*)Buffer1)->CoreType;
+  Comparator1.Fields.CacheLevel = ((CPU_CACHE_INFO*)Buffer1)->CacheLevel;
+  Comparator1.Fields.CacheType  = ((CPU_CACHE_INFO*)Buffer1)->CacheType;
+
+  Comparator2.Fields.Package    = ((CPU_CACHE_INFO*)Buffer2)->Package;
+  Comparator2.Fields.CoreType   = ((CPU_CACHE_INFO*)Buffer2)->CoreType;
+  Comparator2.Fields.CacheLevel = ((CPU_CACHE_INFO*)Buffer2)->CacheLevel;
+  Comparator2.Fields.CacheType  = ((CPU_CACHE_INFO*)Buffer2)->CacheType;
+
+  if (Comparator1.Data64 == Comparator2.Data64) {
+    return 0;
+  } else if (Comparator1.Data64 > Comparator2.Data64) {
+    return 1;
+  } else {
+    return -1;
+  }
+}
+
 /**
   Get the total number of package and package ID in the platform.
 
@@ -325,6 +366,10 @@ CpuCacheInfoCollectCpuCacheInfoData (
   if (*CacheInfoCount < LocalCacheInfoCount) {
     Status = EFI_BUFFER_TOO_SMALL;
   } else {
+    //
+    // Sort LocalCacheInfo array by CPU package ID, core type, cache level and cache type.
+    //
+    PerformQuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInfo), (SORT_COMPARE) CpuCacheInfoCompare);
     CopyMem (CacheInfo, LocalCacheInfo, sizeof (*CacheInfo) * LocalCacheInfoCount);
     DEBUG_CODE (
       CpuCacheInfoPrintCpuCacheInfoTable (CacheInfo, LocalCacheInfoCount);
@@ -340,7 +385,7 @@ CpuCacheInfoCollectCpuCacheInfoData (
 }
 
 /**
-  Get CpuCacheInfo data array.
+  Get CpuCacheInfo data array. The array is sorted by CPU package ID, core type, cache level and cache type.
 
   @param[in, out] CpuCacheInfo        Pointer to the CpuCacheInfo array.
   @param[in, out] CpuCacheInfoCount   As input, point to the length of response CpuCacheInfo array.
diff --git a/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h b/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
index a66152bce0..3422997f54 100644
--- a/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
+++ b/UefiCpuPkg/Include/Library/CpuCacheInfoLib.h
@@ -59,7 +59,7 @@ typedef struct {
 } CPU_CACHE_INFO;
 
 /**
-  Get CpuCacheInfo data array.
+  Get CpuCacheInfo data array. The array is sorted by CPU package ID, core type, cache level and cache type.
 
   @param[in, out] CpuCacheInfo        Pointer to the CpuCacheInfo array.
   @param[in, out] CpuCacheInfoCount   As input, point to the length of response CpuCacheInfo array.
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
index c481080e49..c3d3f1e799 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
@@ -3,7 +3,7 @@
 #
 #  Provides cache info for each package, core type, cache level and cache type.
 #
-#  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,6 +25,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [LibraryClasses]
@@ -33,6 +34,7 @@
   BaseMemoryLib
   MemoryAllocationLib
   UefiBootServicesTableLib
+  SortLib
 
 [Protocols]
   gEfiMpServiceProtocolGuid
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
index b6e6ae5bc5..6135215e5f 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
@@ -17,8 +17,35 @@
 #include <Library/DebugLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/SortLib.h>
 #include <Library/CpuCacheInfoLib.h>
 
+typedef union {
+  struct {
+    //
+    // Type of the cache that this package's this type of logical processor corresponds to.
+    // Value = CPUID.04h:EAX[04:00]
+    //
+    UINT32        CacheType : 5;
+    //
+    // Level of the cache that this package's this type of logical processor corresponds to.
+    // Value = CPUID.04h:EAX[07:05]
+    //
+    UINT32        CacheLevel : 3;
+    //
+    // Core type of logical processor.
+    // Value = CPUID.1Ah:EAX[31:24]
+    //
+    UINT32        CoreType : 8;
+    UINT32        Reserved : 16;
+    //
+    // Package number.
+    //
+    UINT32        Package;
+  } Fields;
+  UINT64        Data64;
+} CPU_CACHE_INFO_COMPARATOR;
+
 typedef struct {
   //
   // Package ID, the information comes from
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
index 0c73015cac..0864497849 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
@@ -3,7 +3,7 @@
 #
 #  Provides cache info for each package, core type, cache level and cache type.
 #
-#  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,6 +25,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [LibraryClasses]
@@ -33,6 +34,7 @@
   BaseMemoryLib
   MemoryAllocationLib
   PeiServicesTablePointerLib
+  SortLib
 
 [Ppis]
   gEdkiiPeiMpServices2PpiGuid
-- 
2.28.0.windows.1

Re: [PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array

[Ni, Ray]

The patch looks good to me. 2 minor comments:
> +  } Fields;
1. Can you rename to "Bits" to align to existing naming convention?

> 
> +  UINT64        Data64;

2. Can you rename to "Uint64"?

Re: [PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array

[Ni, Ray]

With the renaming, Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Ni, Ray
> Sent: Friday, August 6, 2021 9:55 PM
> To: Lou, Yun <Yun.Lou@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <Rahul1.Kumar@intel.com>
> Subject: RE: [PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array
> 
> The patch looks good to me. 2 minor comments:
> > +  } Fields;
> 1. Can you rename to "Bits" to align to existing naming convention?
> 
> >
> > +  UINT64        Data64;
> 
> 2. Can you rename to "Uint64"?
> 

Re: [edk2-devel] [PATCH v2] UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array

[Jason Lou]

[-- Attachment #1: Type: text/plain, Size: 56 bytes --]

Sure, I will rename "Fields" and "Data64" in v3 patch.

[-- Attachment #2: Type: text/html, Size: 56 bytes --]