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


IanX Kuo
 

From: IanX Kuo <ianx.kuo@intel.com>

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

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/UefiCpu=
Pkg/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;=0D
UINTN Index;=0D
UINTN NextIndex;=0D
+ VOID *QuickSortBuffer;=0D
=0D
//=0D
// Get number of Packages and Package ID.=0D
@@ -369,7 +370,13 @@ CpuCacheInfoCollectCpuCacheInfoData (
//=0D
// Sort LocalCacheInfo array by CPU package ID, core type, cache level=
and cache type.=0D
//=0D
- PerformQuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalC=
acheInfo), (SORT_COMPARE) CpuCacheInfoCompare);=0D
+ QuickSortBuffer =3D AllocateZeroPool (sizeof (*LocalCacheInfo));=0D
+ ASSERT (QuickSortBuffer !=3D NULL);=0D
+ if (QuickSortBuffer =3D=3D NULL) {=0D
+ return EFI_OUT_OF_RESOURCES;=0D
+ }=0D
+=0D
+ QuickSort (LocalCacheInfo, LocalCacheInfoCount, sizeof (*LocalCacheInf=
o), CpuCacheInfoCompare, QuickSortBuffer);=0D
CopyMem (CacheInfo, LocalCacheInfo, sizeof (*CacheInfo) * LocalCacheIn=
foCount);=0D
DEBUG_CODE (=0D
CpuCacheInfoPrintCpuCacheInfoTable (CacheInfo, LocalCacheInfoCount);=
=0D
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf b/Ue=
fiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
index c3d3f1e799..fdd79970f9 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf
@@ -25,7 +25,6 @@
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
- MdeModulePkg/MdeModulePkg.dec=0D
UefiCpuPkg/UefiCpuPkg.dec=0D
=0D
[LibraryClasses]=0D
@@ -34,7 +33,6 @@
BaseMemoryLib=0D
MemoryAllocationLib=0D
UefiBootServicesTableLib=0D
- SortLib=0D
=0D
[Protocols]=0D
gEfiMpServiceProtocolGuid=0D
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>=0D
#include <Library/BaseMemoryLib.h>=0D
#include <Library/MemoryAllocationLib.h>=0D
-#include <Library/SortLib.h>=0D
#include <Library/CpuCacheInfoLib.h>=0D
=0D
typedef union {=0D
diff --git a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf b/Ue=
fiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
index 0864497849..c643fc89be 100644
--- a/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
+++ b/UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf
@@ -25,7 +25,6 @@
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
- MdeModulePkg/MdeModulePkg.dec=0D
UefiCpuPkg/UefiCpuPkg.dec=0D
=0D
[LibraryClasses]=0D
@@ -34,7 +33,6 @@
BaseMemoryLib=0D
MemoryAllocationLib=0D
PeiServicesTablePointerLib=0D
- SortLib=0D
=0D
[Ppis]=0D
gEdkiiPeiMpServices2PpiGuid=0D
--=20
2.30.0.windows.1


Marvin Häuser
 

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