* [edk2-platforms][PATCH v2 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
@ 2021-08-06 1:32 ` Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol Michael Kubacki
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:32 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
The MessageLength field of EFI_MM_COMMUNICATE_HEADER as defined in
MdePkg/Include/Protocol/MmCommunication.h was updated to a fixed
size as opposed to UINTN to avoid ambiguity between different
caller enviornments.
This change updates the MessageLength usage in MinPlatformPkg to
support the new field structure, in turn, fixing a build issue.
Original edk2 change:
https://bugzilla.tianocore.org/show_bug.cgi?id=3398
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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c | 4 ++--
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c | 15 ++++++++++++++-
Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c | 10 +++++-----
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf | 1 +
4 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
index 3ceeb821fb95..80e8d26f4e1d 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c
@@ -106,7 +106,7 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "SmiHandlerProfile: SmmCommunication - %r\n", Status));
@@ -139,7 +139,7 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
index c012e0afcbaa..e5efbd059954 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c
@@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/SafeIntLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <IndustryStandard/Acpi.h>
#include <IndustryStandard/DmaRemappingReportingTable.h>
@@ -520,6 +521,7 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
UINTN MemoryAttributesTableSize;
EFI_STATUS Status;
UINTN CommSize;
+ UINT64 LongCommSize;
UINT8 *CommBuffer;
EFI_SMM_COMMUNICATE_HEADER *CommHeader;
EFI_SMM_COMMUNICATION_PROTOCOL *SmmCommunication;
@@ -620,7 +622,18 @@ TestPointDxeSmmReadyToBootSmmPageProtection (
(UINTN)CommData->UefiMemoryAttributeTableSize
);
- CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + CommHeader->MessageLength;
+ Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommHeader->MessageLength, &LongCommSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: LongCommSize calculation - %r\n", Status));
+ return EFI_SUCCESS;
+ }
+
+ Status = SafeUint64ToUintn (LongCommSize, &CommSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: CommSize conversion - %r\n", Status));
+ return EFI_SUCCESS;
+ }
+
Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: SmmCommunication - %r\n", Status));
diff --git a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
index 3cc5ccfef6f4..8416b36f56ae 100644
--- a/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
+++ b/Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
@@ -122,7 +122,7 @@ GetTestPointDataSmm (
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_INFO, "SmiHandlerTestPoint: SmmCommunication - %r\n", Status));
@@ -155,7 +155,7 @@ GetTestPointDataSmm (
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;
- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength;
+ CommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;
@@ -233,7 +233,7 @@ PublishSmmTestPoint (
TestPoint = mSmmTestPointDatabase;
while ((UINTN)TestPoint < (UINTN)mSmmTestPointDatabase + mSmmTestPointDatabaseSize) {
TestPointSize = GetTestPointInfoSize (TestPoint, (UINTN)mSmmTestPointDatabase + mSmmTestPointDatabaseSize - (UINTN)TestPoint);
-
+
TestPointLibSetTable (TestPoint, TestPointSize);
TestPoint = (ADAPTER_INFO_PLATFORM_TEST_POINT *)((UINTN)TestPoint + TestPointSize);
@@ -286,7 +286,7 @@ OnReadyToBoot (
EFI_EVENT ReadyToBootLaterEvent;
gBS->CloseEvent (Event);
-
+
Status = gBS->CreateEvent (
EVT_NOTIFY_SIGNAL,
TPL_CALLBACK,
@@ -295,7 +295,7 @@ OnReadyToBoot (
&ReadyToBootLaterEvent
);
ASSERT_EFI_ERROR (Status);
-
+
gBS->SignalEvent (ReadyToBootLaterEvent);
}
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
index 2ae1db4ee483..54b4015d6767 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
@@ -32,6 +32,7 @@ [LibraryClasses]
TestPointLib
PciSegmentLib
PciSegmentInfoLib
+ SafeIntLib
[Packages]
MinPlatformPkg/MinPlatformPkg.dec
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [edk2-platforms][PATCH v2 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue Michael Kubacki
@ 2021-08-06 1:32 ` Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index Michael Kubacki
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:32 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
Per the protocol definition, the caller must allocate the input
structure and set the size field. TestPointCheckTcgTrustedBoot()
does not do this which can result in an EFI_BUFFER_TOO_SMALL error.
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/DxeCheckTcgTrustedBoot.c | 3 +++
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf | 1 +
2 files changed, 4 insertions(+)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
index 2a04f86fedac..5ec32fd2e875 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <PiDxe.h>
#include <Library/TestPointCheckLib.h>
#include <Library/TestPointLib.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Library/PrintLib.h>
@@ -41,6 +42,8 @@ TestPointCheckTcgTrustedBoot (
goto Done;
}
+ ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability));
+ ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
Status = Tcg2->GetCapability (Tcg2, &ProtocolCapability);
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "Tcg2->GetCapability - %r\n", Status));
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
index 54b4015d6767..195729e0ff5b 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
@@ -18,6 +18,7 @@ [Defines]
[LibraryClasses]
BaseLib
+ BaseMemoryLib
DebugLib
DxeServicesTableLib
UefiBootServicesTableLib
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [edk2-platforms][PATCH v2 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol Michael Kubacki
@ 2021-08-06 1:32 ` Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking Michael Kubacki
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:32 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
TestPointSmmEndOfDxeSmrrFunctional() uses the incorrect byte index
to skip the test. It should use byte 6 instead of byte 5.
Also defines a macro "TEST_POINT_INDEX_BYTE6_SMM" for the byte index
6 value.
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/SmmTestPointCheckLib.c | 26 +++++++++++---------
Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h | 1 +
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c
index 4b4f874c7bbc..75200969d3e7 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c
@@ -109,7 +109,7 @@ TestPointSmmEndOfDxeSmrrFunctional (
EFI_STATUS Status;
BOOLEAN Result;
- if ((mFeatureImplemented[5] & TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL) == 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] & TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL) == 0) {
return EFI_SUCCESS;
}
@@ -125,7 +125,7 @@ TestPointSmmEndOfDxeSmrrFunctional (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 5,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL
);
}
@@ -156,7 +156,8 @@ TestPointSmmReadyToLockSmmMemoryAttributeTableFunctional (
EFI_STATUS Status;
BOOLEAN Result;
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNCTIONAL) == 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] &
+ TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNCTIONAL) == 0) {
return EFI_SUCCESS;
}
@@ -173,7 +174,7 @@ TestPointSmmReadyToLockSmmMemoryAttributeTableFunctional (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNCTIONAL
);
}
@@ -202,7 +203,8 @@ TestPointSmmReadyToLockSecureSmmCommunicationBuffer (
EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable;
UINTN MemoryAttributesTableSize;
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER) == 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM] &
+ TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER) == 0) {
return EFI_SUCCESS;
}
@@ -248,7 +250,8 @@ TestPointSmmReadyToBootSmmPageProtection (
EFI_STATUS Status;
BOOLEAN Result;
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) == 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM]
+ & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) == 0) {
return EFI_SUCCESS;
}
@@ -264,7 +267,7 @@ TestPointSmmReadyToBootSmmPageProtection (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION
);
}
@@ -280,7 +283,7 @@ TestPointSmmReadyToBootSmmPageProtection (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER
);
}
@@ -312,7 +315,8 @@ TestPointSmmReadyToBootSmmPageProtectionHandler (
TEST_POINT_SMM_COMMUNICATION_UEFI_GCD_MAP_INFO *CommData;
UINTN TempCommBufferSize;
- if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) == 0) {
+ if ((mFeatureImplemented[TEST_POINT_INDEX_BYTE6_SMM]
+ & TEST_POINT_BYTE6_SMM_READY_TO_BOOT_SMM_PAGE_LEVEL_PROTECTION) == 0) {
return EFI_SUCCESS;
}
@@ -390,14 +394,14 @@ TestPointSmmReadyToBootSmmPageProtectionHandler (
TestPointLibSetFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER
);
} else {
TestPointLibClearFeaturesVerified (
PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
NULL,
- 6,
+ TEST_POINT_INDEX_BYTE6_SMM,
TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER
);
}
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
index 7265e2268961..73c30522179c 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
@@ -738,6 +738,7 @@ TestPointSmmExitBootServices (
#define TEST_POINT_SMM_READY_TO_BOOT L" - SMM Ready To Boot - "
#define TEST_POINT_SMM_EXIT_BOOT_SERVICES L" - SMM Exit Boot Services - "
+#define TEST_POINT_INDEX_BYTE6_SMM 6
#define TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL BIT0
#define TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SMM_MEMORY_ATTRIBUTE_TABLE_FUNCTIONAL BIT1
#define TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER BIT2
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [edk2-platforms][PATCH v2 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (2 preceding siblings ...)
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index Michael Kubacki
@ 2021-08-06 1:32 ` Michael Kubacki
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional Michael Kubacki
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:32 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.c | 56 ++++++++++----------
1 file changed, 29 insertions(+), 27 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.c
index c493750a27e6..f15f76eab574 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.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_ERROR_CODE \
- TEST_POINT_DXE_SMM_READY_TO_LOCK
- TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_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_ERROR_CODE \
+ TEST_POINT_DXE_SMM_READY_TO_LOCK
+ TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERROR_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_ERROR_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_ERROR_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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [edk2-platforms][PATCH v2 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (3 preceding siblings ...)
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking Michael Kubacki
@ 2021-08-06 1:32 ` Michael Kubacki
2021-08-09 19:18 ` [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Nate DeSimone
2021-08-09 19:26 ` Nate DeSimone
6 siblings, 0 replies; 8+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:32 UTC (permalink / raw)
To: devel; +Cc: Chasel Chiu, Nate DeSimone, Liming Gao, Eric Dong
From: Michael Kubacki <michael.kubacki@microsoft.com>
Makes the OutTable parameter in DumpAcpiRsdt() and DumpAcpiXsdt()
optional since the pointer passed can be NULL if the Signature
pointer is also NULL.
Can fix a potential failure in TestPointCheckAcpi().
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>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcpi.c | 32 ++++++++++----------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcpi.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcpi.c
index cd8f538f7f3f..3d75e5012a4c 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcpi.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckAcpi.c
@@ -477,7 +477,7 @@ DumpAcpiTable (
)
{
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-
+
if (Table == NULL) {
return ;
}
@@ -535,7 +535,7 @@ CheckAcpiTableResource (
)
{
EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-
+
if (Table == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -592,7 +592,7 @@ EFI_STATUS
DumpAcpiRsdt (
IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
IN UINT32 *Signature, OPTIONAL
- OUT VOID **OutTable,
+ OUT VOID **OutTable, OPTIONAL
IN BOOLEAN DumpPrint,
IN BOOLEAN CheckResource
)
@@ -610,7 +610,7 @@ DumpAcpiRsdt (
if (OutTable != NULL) {
*OutTable = NULL;
- } else {
+ } else if ((OutTable == NULL) && (Signature != NULL)) {
return EFI_INVALID_PARAMETER;
}
@@ -632,7 +632,7 @@ DumpAcpiRsdt (
*OutTable = Table;
}
}
-
+
if (OutTable != NULL) {
if (*OutTable == NULL) {
return EFI_NOT_FOUND;
@@ -646,7 +646,7 @@ EFI_STATUS
DumpAcpiXsdt (
IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
IN UINT32 *Signature, OPTIONAL
- OUT VOID **OutTable,
+ OUT VOID **OutTable, OPTIONAL
IN BOOLEAN DumpPrint,
IN BOOLEAN CheckResource
)
@@ -662,16 +662,16 @@ DumpAcpiXsdt (
if (Xsdt == NULL) {
return EFI_INVALID_PARAMETER;
}
-
+
if (OutTable != NULL) {
*OutTable = NULL;
- } else {
+ } else if ((OutTable == NULL) && (Signature != NULL)) {
return EFI_INVALID_PARAMETER;
}
ReturnStatus = EFI_SUCCESS;
EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
+
BasePtr = (UINTN)(Xsdt + 1);
for (Index = 0; Index < EntryCount; Index ++) {
CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
@@ -783,7 +783,7 @@ TestPointCheckAcpi (
if (Status == EFI_NOT_FOUND) {
Status = DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, TRUE, FALSE);
}
-
+
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "No ACPI table\n"));
TestPointLibAppendErrorString (
@@ -796,7 +796,7 @@ TestPointCheckAcpi (
}
DEBUG ((DEBUG_INFO, "==== TestPointCheckAcpi - Exit\n"));
-
+
return Status;
}
@@ -806,9 +806,9 @@ TestPointCheckAcpiGcdResource (
)
{
EFI_STATUS Status;
-
+
DEBUG ((DEBUG_INFO, "==== TestPointCheckAcpiGcdResource - Enter\n"));
-
+
//
// Check the ACPI existence
//
@@ -816,7 +816,7 @@ TestPointCheckAcpiGcdResource (
if (Status == EFI_NOT_FOUND) {
Status = DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, FALSE, FALSE);
}
-
+
if (!EFI_ERROR(Status)) {
//
// Then check resource in ACPI and GCD
@@ -828,7 +828,7 @@ TestPointCheckAcpiGcdResource (
Status = DumpAcpiWithGuid (&gEfiAcpi10TableGuid, NULL, NULL, FALSE, TRUE);
}
}
-
+
if (EFI_ERROR(Status)) {
DEBUG ((DEBUG_ERROR, "ACPI table resource not in GCD\n"));
TestPointLibAppendErrorString (
@@ -840,7 +840,7 @@ TestPointCheckAcpiGcdResource (
);
}
DEBUG ((DEBUG_INFO, "==== TestPointCheckAcpiGcdResource - Exit\n"));
-
+
return Status;
}
--
2.28.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (4 preceding siblings ...)
2021-08-06 1:32 ` [edk2-platforms][PATCH v2 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional Michael Kubacki
@ 2021-08-09 19:18 ` Nate DeSimone
2021-08-09 19:26 ` Nate DeSimone
6 siblings, 0 replies; 8+ messages in thread
From: Nate DeSimone @ 2021-08-09 19:18 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
For the series...
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Thursday, August 5, 2021 6:33 PM
> 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-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib
> bug fixes and improvements
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521
>
> This patch series groups together several bug fixes and improvements to
> TestPointCheckLib. The first patch is required for the others since it fixes a
> MinPlatformPkg build issue that occurs with the current edk2/master branch.
>
> V2 changes:
> 1. Added Reviewed-by replies received for v1 series 2. [v1 2/5]: Added a
> ZeroMem() for the ProtocolCapability buffer 3. [v1 3/5]: Added a #define for
> the byte index 6 parameter
>
> 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>
>
> Michael Kubacki (5):
> MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
> MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
> MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
> MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
> MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
>
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
> cpi.c | 32 +++++------
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> miHandlerInstrument.c | 4 +-
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> mmInfo.c | 56 ++++++++++----------
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTc
> gTrustedBoot.c | 3 ++
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.c | 15 +++++-
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
> intCheckLib.c | 26 +++++----
> Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
> | 10 ++--
> Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
> | 1 +
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.inf | 2 +
> 9 files changed, 87 insertions(+), 62 deletions(-)
>
> --
> 2.28.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements
2021-08-06 1:32 [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (5 preceding siblings ...)
2021-08-09 19:18 ` [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Nate DeSimone
@ 2021-08-09 19:26 ` Nate DeSimone
6 siblings, 0 replies; 8+ messages in thread
From: Nate DeSimone @ 2021-08-09 19:26 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
The series has been pushed as 5b257da~..89fb75a
Thanks,
Nate
> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Thursday, August 5, 2021 6:33 PM
> 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-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib
> bug fixes and improvements
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3531
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3518
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3520
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521
>
> This patch series groups together several bug fixes and improvements to
> TestPointCheckLib. The first patch is required for the others since it fixes a
> MinPlatformPkg build issue that occurs with the current edk2/master branch.
>
> V2 changes:
> 1. Added Reviewed-by replies received for v1 series 2. [v1 2/5]: Added a
> ZeroMem() for the ProtocolCapability buffer 3. [v1 3/5]: Added a #define for
> the byte index 6 parameter
>
> 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>
>
> Michael Kubacki (5):
> MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
> MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
> MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
> MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
> MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
>
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
> cpi.c | 32 +++++------
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> miHandlerInstrument.c | 4 +-
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> mmInfo.c | 56 ++++++++++----------
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTc
> gTrustedBoot.c | 3 ++
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.c | 15 +++++-
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
> intCheckLib.c | 26 +++++----
> Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
> | 10 ++--
> Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h
> | 1 +
>
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.inf | 2 +
> 9 files changed, 87 insertions(+), 62 deletions(-)
>
> --
> 2.28.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread