[edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

[edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

[Michael Kubacki]

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/DxeCheckAcpi.c                 | 32 +++++------
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmiHandlerInstrument.c |  4 +-
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckSmmInfo.c              | 56 ++++++++++----------
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c       |  3 ++
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c         | 15 +++++-
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c         | 26 +++++----
 Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c                      | 10 ++--
 Platform/Intel/MinPlatformPkg/Include/Library/TestPointCheckLib.h                           |  1 +
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf       |  2 +
 9 files changed, 87 insertions(+), 62 deletions(-)

-- 
2.28.0.windows.1

[edk2-platforms][PATCH v2 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue

[Michael Kubacki]

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

[edk2-platforms][PATCH v2 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol

[Michael Kubacki]

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

[edk2-platforms][PATCH v2 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index

[Michael Kubacki]

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

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

[Michael Kubacki]

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

[edk2-platforms][PATCH v2 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional

[Michael Kubacki]

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

Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

[Nate DeSimone]

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

Re: [edk2-platforms][PATCH v2 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

[Nate DeSimone]

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