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

[edk2-platforms][PATCH v1 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.

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       |  1 +
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c         | 15 +++++-
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c         |  4 +-
 Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c                      | 10 ++--
 Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf       |  1 +
 8 files changed, 70 insertions(+), 53 deletions(-)

-- 
2.28.0.windows.1

[edk2-platforms][PATCH v1 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>
---
 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 v1 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
index 2a04f86fedac..7a8e3fed22f9 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckTcgTrustedBoot.c
@@ -41,6 +41,7 @@ TestPointCheckTcgTrustedBoot (
     goto Done;
   }
 
+  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
   Status = Tcg2->GetCapability (Tcg2, &ProtocolCapability);
   if (EFI_ERROR(Status)) {
     DEBUG ((DEBUG_ERROR, "Tcg2->GetCapability - %r\n", Status));
-- 
2.28.0.windows.1

[edk2-platforms][PATCH v1 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.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c
index 4b4f874c7bbc..3e8b2621cccd 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[6] & 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,
+      6,
       TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL
       );
   }
-- 
2.28.0.windows.1

[edk2-platforms][PATCH v1 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>
---
 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 v1 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>
---
 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-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements

[Nate DeSimone]

Hi Michael,

Here is a summary of my feedback:

  1. Patch 2/5: DxeCheckTcgTrustedBoot.c: line 44 - I think we should also have a ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability)) before setting the size.
  2. Patch 3/5: SmmTestPointCheckLib.c: line 112 - I think we should have a #define that describes whatever "6" means in this context.

Everything else looks good!

Thanks,
Nate

> -----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 0/5] MinPlatformPkg:
> TestPointCheckLib bug fixes and improvements
> 
> From: Michael Kubacki <mailto: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.
> 
> Cc: Chasel Chiu <mailto:chasel.chiu@intel.com>
> Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <mailto:gaoliming@byosoft.com.cn>
> Cc: Eric Dong <mailto:eric.dong@intel.com>
> Signed-off-by: Michael Kubacki <mailto: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       |  1 +
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.c         | 15 +++++-
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
> intCheckLib.c         |  4 +-
>  Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
> | 10 ++--
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.inf       |  1 +
>  8 files changed, 70 insertions(+), 53 deletions(-)
> 
> --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78710): https://edk2.groups.io/g/devel/message/78710
> Mute This Topic: https://groups.io/mt/84686301/1767664
> Group Owner: mailto:devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [nathaniel.l.desimone@intel.com]
> -=-=-=-=-=-=
> 

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

[Nate DeSimone]

Hi Michael,

Comments are inline.

Thanks,
Nate

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> 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-platforms][PATCH v1 3/5]
> MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
> 
> 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.
> 
> 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/SmmTestPo
> intCheckLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTest
> PointCheckLib.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTest
> PointCheckLib.c
> index 4b4f874c7bbc..3e8b2621cccd 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTest
> PointCheckLib.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTe
> +++ stPointCheckLib.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[6] &
> + TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL) == 0) {

I think we should have a #define that describes whatever "6" means in this context.

>      return EFI_SUCCESS;
>    }
> 
> @@ -125,7 +125,7 @@ TestPointSmmEndOfDxeSmrrFunctional (
>      TestPointLibSetFeaturesVerified (
>        PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
>        NULL,
> -      5,
> +      6,
>        TEST_POINT_BYTE6_SMM_END_OF_DXE_SMRR_FUNCTIONAL
>        );
>    }
> --
> 2.28.0.windows.1

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

[Nate DeSimone]

Hi Michael,

Comments are inline.

Thanks,
Nate

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> 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-platforms][PATCH v1 2/5]
> MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
> 
> 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/DxeCheckTc
> gTrustedBoot.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> TcgTrustedBoot.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> TcgTrustedBoot.c
> index 2a04f86fedac..7a8e3fed22f9 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> TcgTrustedBoot.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckTcgTrustedBoot.c
> @@ -41,6 +41,7 @@ TestPointCheckTcgTrustedBoot (
>      goto Done;
>    }
> 
> +  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);

I think we should also have a ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability)) before setting the size.

>    Status = Tcg2->GetCapability (Tcg2, &ProtocolCapability);
>    if (EFI_ERROR(Status)) {
>      DEBUG ((DEBUG_ERROR, "Tcg2->GetCapability - %r\n", Status));
> --
> 2.28.0.windows.1

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

[Nate DeSimone]

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 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-platforms][PATCH v1 1/5]
> MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
> 
> 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>
> ---
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> miHandlerInstrument.c |  4 ++--
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.c         | 15 ++++++++++++++-
>  Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
> | 10 +++++-----
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
> ntCheckLib.inf       |  1 +
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> index 3ceeb821fb95..80e8d26f4e1d 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmiHandlerInstrument.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckSmiHandlerInstrument.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/DxeTestP
> ointCheckLib.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.c
> index c012e0afcbaa..e5efbd059954 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTe
> +++ stPointCheckLib.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/TestPointStubD
> +++ xe.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/DxeTestP
> ointCheckLib.inf
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.inf
> index 2ae1db4ee483..54b4015d6767 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestP
> ointCheckLib.inf
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTe
> +++ stPointCheckLib.inf
> @@ -32,6 +32,7 @@ [LibraryClasses]
>    TestPointLib
>    PciSegmentLib
>    PciSegmentInfoLib
> +  SafeIntLib
> 
>  [Packages]
>    MinPlatformPkg/MinPlatformPkg.dec
> --
> 2.28.0.windows.1

Re: [edk2-devel] [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]
> -=-=-=-=-=-=
> 

Re: [edk2-devel] [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional

[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 5/5]
> MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
> 
> 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>
> ---
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckA
> cpi.c | 32 ++++++++++----------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Acpi.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Acpi.c
> index cd8f538f7f3f..3d75e5012a4c 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> Acpi.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckAcpi.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
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78715): https://edk2.groups.io/g/devel/message/78715
> Mute This Topic: https://groups.io/mt/84686310/1767664
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [nathaniel.l.desimone@intel.com]
> -=-=-=-=-=-=
> 

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

[Michael Kubacki]

Thanks for the feedback, I addressed it in a v2 series:
   https://edk2.groups.io/g/devel/message/78767

Regards,
Michael

On 8/5/2021 7:14 PM, Nate DeSimone wrote:
> Hi Michael,
> 
> Here is a summary of my feedback:
> 
>    1. Patch 2/5: DxeCheckTcgTrustedBoot.c: line 44 - I think we should also have a ZeroMem ((VOID *) &ProtocolCapability, sizeof (ProtocolCapability)) before setting the size.
>    2. Patch 3/5: SmmTestPointCheckLib.c: line 112 - I think we should have a #define that describes whatever "6" means in this context.
> 
> Everything else looks good!
> 
> Thanks,
> Nate
> 
>> -----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 0/5] MinPlatformPkg:
>> TestPointCheckLib bug fixes and improvements
>>
>> From: Michael Kubacki <mailto: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.
>>
>> Cc: Chasel Chiu <mailto:chasel.chiu@intel.com>
>> Cc: Nate DeSimone <mailto:nathaniel.l.desimone@intel.com>
>> Cc: Liming Gao <mailto:gaoliming@byosoft.com.cn>
>> Cc: Eric Dong <mailto:eric.dong@intel.com>
>> Signed-off-by: Michael Kubacki <mailto: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       |  1 +
>>
>> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
>> ntCheckLib.c         | 15 +++++-
>>
>> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPo
>> intCheckLib.c         |  4 +-
>>   Platform/Intel/MinPlatformPkg/Test/TestPointStubDxe/TestPointStubDxe.c
>> | 10 ++--
>>
>> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPoi
>> ntCheckLib.inf       |  1 +
>>   8 files changed, 70 insertions(+), 53 deletions(-)
>>
>> --
>> 2.28.0.windows.1
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>> View/Reply Online (#78710): https://edk2.groups.io/g/devel/message/78710
>> Mute This Topic: https://groups.io/mt/84686301/1767664
>> Group Owner: mailto:devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
>> [nathaniel.l.desimone@intel.com]
>> -=-=-=-=-=-=
>>
> 
> 
> 
> 
>