[edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuCacheInfoLib: Add QuickSort function on BaseLib

IanX Kuo posted 3 patches 4 years, 3 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuCacheInfoLib: Add QuickSort function on BaseLib
Posted by IanX Kuo 4 years, 3 months ago
From: IanX Kuo <ianx.kuo@intel.com>

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

Remove MdeModulePkg dependency

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: IanX Kuo <ianx.kuo@intel.com>
---
 UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c     | 9 ++++++++-
 .../Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf       | 2 --
 .../Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h    | 1 -
 .../Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf       | 2 --
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
index c0077d6770..45c2ca13dc 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
@@ -282,6 +282,7 @@ CpuCacheInfoCollectCpuCacheInfoData (
   UINTN                     LocalCacheInfoCount;
   UINTN                     Index;
   UINTN                     NextIndex;
+  VOID                      *QuickSortBuffer;
 
   //
   // Get number of Packages and Package ID.
@@ -369,7 +370,13 @@ CpuCacheInfoCollectCpuCacheInfoData (
     //
     // Sort LocalCacheInfo array by CPU package ID, core type, cache level and cache type.
     //
-    PerformQuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInfo), (SORT_COMPARE) CpuCacheInfoCompare);
+    QuickSortBuffer = AllocateZeroPool (sizeof (*LocalCacheInfo));
+    ASSERT (QuickSortBuffer != NULL);
+    if (QuickSortBuffer == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    QuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInfo), CpuCacheInfoCompare, QuickSortBuffer);
     CopyMem (CacheInfo, LocalCacheInfo, sizeof (*CacheInfo) * LocalCacheInfoCount);
     DEBUG_CODE (
       CpuCacheInfoPrintCpuCacheInfoTable (CacheInfo, LocalCacheInfoCount);
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
index c3d3f1e799..fdd79970f9 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
@@ -25,7 +25,6 @@
 
 [Packages]
   MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [LibraryClasses]
@@ -34,7 +33,6 @@
   BaseMemoryLib
   MemoryAllocationLib
   UefiBootServicesTableLib
-  SortLib
 
 [Protocols]
   gEfiMpServiceProtocolGuid
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
index 26e1f46516..829a9f43ce 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
@@ -17,7 +17,6 @@
 #include <Library/DebugLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
-#include <Library/SortLib.h>
 #include <Library/CpuCacheInfoLib.h>
 
 typedef union {
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
index 0864497849..c643fc89be 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
@@ -25,7 +25,6 @@
 
 [Packages]
   MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [LibraryClasses]
@@ -34,7 +33,6 @@
   BaseMemoryLib
   MemoryAllocationLib
   PeiServicesTablePointerLib
-  SortLib
 
 [Ppis]
   gEdkiiPeiMpServices2PpiGuid
-- 
2.30.0.windows.1



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


Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/CpuCacheInfoLib: Add QuickSort function on BaseLib
Posted by Marvin Häuser 4 years, 3 months ago
Hey IanX,

On 16.10.21 01:25, IanX Kuo wrote:
> From: IanX Kuo <ianx.kuo@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3675
>
> Remove MdeModulePkg dependency
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: IanX Kuo <ianx.kuo@intel.com>
> ---
>   UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c     | 9 ++++++++-
>   .../Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf       | 2 --
>   .../Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h    | 1 -
>   .../Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf       | 2 --
>   4 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
> index c0077d6770..45c2ca13dc 100644
> --- a/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
> +++ b/UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c
> @@ -282,6 +282,7 @@ CpuCacheInfoCollectCpuCacheInfoData (
>     UINTN                     LocalCacheInfoCount;
>     UINTN                     Index;
>     UINTN                     NextIndex;
> +  VOID                      *QuickSortBuffer;
>   
>     //
>     // Get number of Packages and Package ID.
> @@ -369,7 +370,13 @@ CpuCacheInfoCollectCpuCacheInfoData (
>       //
>       // Sort LocalCacheInfo array by CPU package ID, core type, cache level and cache type.
>       //
> -    PerformQuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInfo), (SORT_COMPARE) CpuCacheInfoCompare);
> +    QuickSortBuffer = AllocateZeroPool (sizeof (*LocalCacheInfo));
> +    ASSERT (QuickSortBuffer != NULL);

Sorry, maybe I should have been more specific. Asserting on dynamic 
memory allocations is a bad idea (independently of whether it's 
otherwise handled or not) as this makes all kinds of analyses harder, 
for example fuzz-testing would just die with ASSERTs enabled, and static 
analysis may draw incorrect conclusions from this incorrect assertion. 
Disabling ASSERTs for analyses can work, but it may decrease their 
efficacy (e.g. fuzz-testing can no longer verify invariants this way, 
and static analysis may start emitting more False Positives). This is an 
edk2-wide problem that I hope to address with a new macro in the near 
future. For now, if it does not interrupt your testing much, I'd prefer 
dropping it.

I still believe this data could just live on the stack, avoiding this 
issue entirely.

> +    if (QuickSortBuffer == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }

Thanks!

Best regards,
Marvin

> +
> +    QuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInfo), CpuCacheInfoCompare, QuickSortBuffer);
>       CopyMem (CacheInfo, LocalCacheInfo, sizeof (*CacheInfo) * LocalCacheInfoCount);
>       DEBUG_CODE (
>         CpuCacheInfoPrintCpuCacheInfoTable (CacheInfo, LocalCacheInfoCount);
> diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
> index c3d3f1e799..fdd79970f9 100644
> --- a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
> +++ b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
> @@ -25,7 +25,6 @@
>   
>   [Packages]
>     MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
>     UefiCpuPkg/UefiCpuPkg.dec
>   
>   [LibraryClasses]
> @@ -34,7 +33,6 @@
>     BaseMemoryLib
>     MemoryAllocationLib
>     UefiBootServicesTableLib
> -  SortLib
>   
>   [Protocols]
>     gEfiMpServiceProtocolGuid
> diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
> index 26e1f46516..829a9f43ce 100644
> --- a/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
> +++ b/UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h
> @@ -17,7 +17,6 @@
>   #include <Library/DebugLib.h>
>   #include <Library/BaseMemoryLib.h>
>   #include <Library/MemoryAllocationLib.h>
> -#include <Library/SortLib.h>
>   #include <Library/CpuCacheInfoLib.h>
>   
>   typedef union {
> diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
> index 0864497849..c643fc89be 100644
> --- a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
> +++ b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
> @@ -25,7 +25,6 @@
>   
>   [Packages]
>     MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
>     UefiCpuPkg/UefiCpuPkg.dec
>   
>   [LibraryClasses]
> @@ -34,7 +33,6 @@
>     BaseMemoryLib
>     MemoryAllocationLib
>     PeiServicesTablePointerLib
> -  SortLib
>   
>   [Ppis]
>     gEdkiiPeiMpServices2PpiGuid



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