Re: [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking


Nate DeSimone
 

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
Kubacki
Sent: Thursday, August 5, 2021 7:57 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2-devel] [edk2-platforms][PATCH v1 4/5]
MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

The current logic depends on a particular order in which the descriptors for
three or more regions are placed in the array to perform proper adjacency
checking. When three or more regions are all adjacent, but neighboring
descriptors are not adjacent, the logic can improperly report a failure. Adjust
the logic so that all descriptors are checked for adjacency.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---

Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
mmInfo.c | 56 ++++++++++----------
1 file changed, 29 insertions(+), 27 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
SmmInfo.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
SmmInfo.c
index c493750a27e6..f15f76eab574 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
SmmInfo.c
+++
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckSmmInfo.c
@@ -59,34 +59,36 @@ CheckSmramDescriptor (
)
{
UINTN Index;
- UINT64 Base;
+ UINTN Index2;
UINT64 Length;
+ BOOLEAN AdjacentRegion;

- Base = 0;
Length = 0;
for (Index = 0; Index < NumberOfSmmReservedRegions; Index++) {
- if (Base == 0) {
- Base = Descriptor[Index].PhysicalStart;
- Length = Descriptor[Index].PhysicalSize;
+ AdjacentRegion = FALSE;
+ for (Index2 = 0; Index2 < NumberOfSmmReservedRegions; Index2++) {
+ if ((NumberOfSmmReservedRegions == 1)
+ || (Descriptor[Index].PhysicalStart + Descriptor[Index].PhysicalSize ==
Descriptor[Index2].PhysicalStart)
+ || (Descriptor[Index2].PhysicalStart + Descriptor[Index2].PhysicalSize
== Descriptor[Index].PhysicalStart)) {
+ AdjacentRegion = TRUE;
+ break;
+ }
+ }
+
+ if (AdjacentRegion == TRUE) {
+ Length += Descriptor[Index].PhysicalSize;
} else {
- if (Base + Length == Descriptor[Index].PhysicalStart) {
- Length = Length + Descriptor[Index].PhysicalSize;
- } else if (Descriptor[Index].PhysicalStart + Descriptor[Index].PhysicalSize
== Base) {
- Base = Descriptor[Index].PhysicalStart;
- Length = Length + Descriptor[Index].PhysicalSize;
- } else {
- DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
- TestPointLibAppendErrorString (
- PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
- NULL,
-
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_CODE \
- TEST_POINT_DXE_SMM_READY_TO_LOCK
-
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_STRING
- );
- return EFI_INVALID_PARAMETER;
- }
+ DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
+ TestPointLibAppendErrorString (
+ PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
+ NULL,
+
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_CODE \
+ TEST_POINT_DXE_SMM_READY_TO_LOCK
+
TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_STRING
+ );
+ return EFI_INVALID_PARAMETER;
}
- }
+ }

if (Length != GetPowerOfTwo64 (Length)) {
DEBUG ((DEBUG_ERROR, "Smram is not aligned\n")); @@ -94,7 +96,7 @@
CheckSmramDescriptor (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,

TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_CODE \
- TEST_POINT_DXE_SMM_READY_TO_LOCK
+ TEST_POINT_DXE_SMM_READY_TO_LOCK

TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
R_STRING
);
return EFI_INVALID_PARAMETER;
@@ -111,14 +113,14 @@ TestPointCheckSmmInfo (
EFI_SMM_ACCESS2_PROTOCOL *SmmAccess;
UINTN Size;
EFI_SMRAM_DESCRIPTOR *SmramRanges;
-
+
DEBUG ((DEBUG_INFO, "==== TestPointCheckSmmInfo - Enter\n"));
-
+
Status = gBS->LocateProtocol (&gEfiSmmAccess2ProtocolGuid, NULL, (VOID
**)&SmmAccess);
if (EFI_ERROR (Status)) {
goto Done ;
}
-
+
Size = 0;
Status = SmmAccess->GetCapabilities (SmmAccess, &Size, NULL);
ASSERT (Status == EFI_BUFFER_TOO_SMALL); @@ -128,7 +130,7 @@
TestPointCheckSmmInfo (

Status = SmmAccess->GetCapabilities (SmmAccess, &Size, SmramRanges);
ASSERT_EFI_ERROR (Status);
-
+
DEBUG ((DEBUG_INFO, "SMRAM Info\n"));
DumpSmramDescriptor (Size / sizeof (EFI_SMRAM_DESCRIPTOR),
SmramRanges);

--
2.28.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78714): https://edk2.groups.io/g/devel/message/78714
Mute This Topic: https://groups.io/mt/84686308/1767664
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[nathaniel.l.desimone@intel.com]
-=-=-=-=-=-=

Join devel@edk2.groups.io to automatically receive all group messages.