* [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
@ 2021-08-05 14:57 ` Michael Kubacki
2021-08-05 23:15 ` Nate DeSimone
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol Michael Kubacki
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-08-05 14:57 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>
---
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] 13+ messages in thread* Re: [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue Michael Kubacki
@ 2021-08-05 23:15 ` Nate DeSimone
0 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:15 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v1 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue Michael Kubacki
@ 2021-08-05 14:57 ` Michael Kubacki
2021-08-05 23:14 ` Nate DeSimone
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index Michael Kubacki
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-08-05 14:57 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 | 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [edk2-platforms][PATCH v1 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol Michael Kubacki
@ 2021-08-05 23:14 ` Nate DeSimone
0 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:14 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v1 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue Michael Kubacki
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 2/5] MinPlatformPkg/TestPointCheckLib: Set required size field in protocol Michael Kubacki
@ 2021-08-05 14:57 ` Michael Kubacki
2021-08-05 23:14 ` Nate DeSimone
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking Michael Kubacki
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-08-05 14:57 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.
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [edk2-platforms][PATCH v1 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index Michael Kubacki
@ 2021-08-05 23:14 ` Nate DeSimone
0 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:14 UTC (permalink / raw)
To: mikuback@linux.microsoft.com, devel@edk2.groups.io
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (2 preceding siblings ...)
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 3/5] MinPlatformPkg/TestPointCheckLib: Fix incorrect array index Michael Kubacki
@ 2021-08-05 14:57 ` Michael Kubacki
2021-08-05 23:15 ` [edk2-devel] " Nate DeSimone
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional Michael Kubacki
2021-08-05 23:14 ` [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Nate DeSimone
5 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-08-05 14:57 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>
---
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] 13+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking Michael Kubacki
@ 2021-08-05 23:15 ` Nate DeSimone
0 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:15 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (3 preceding siblings ...)
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking Michael Kubacki
@ 2021-08-05 14:57 ` Michael Kubacki
2021-08-05 23:15 ` [edk2-devel] " Nate DeSimone
2021-08-05 23:14 ` [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Nate DeSimone
5 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-08-05 14:57 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>
---
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] 13+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional Michael Kubacki
@ 2021-08-05 23:15 ` Nate DeSimone
0 siblings, 0 replies; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:15 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements
2021-08-05 14:57 [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Michael Kubacki
` (4 preceding siblings ...)
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 5/5] MinPlatformPkg/TestPointCheckLib: Make OutTable parameter optional Michael Kubacki
@ 2021-08-05 23:14 ` Nate DeSimone
2021-08-06 1:34 ` Michael Kubacki
5 siblings, 1 reply; 13+ messages in thread
From: Nate DeSimone @ 2021-08-05 23:14 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements
2021-08-05 23:14 ` [edk2-devel] [edk2-platforms][PATCH v1 0/5] MinPlatformPkg: TestPointCheckLib bug fixes and improvements Nate DeSimone
@ 2021-08-06 1:34 ` Michael Kubacki
0 siblings, 0 replies; 13+ messages in thread
From: Michael Kubacki @ 2021-08-06 1:34 UTC (permalink / raw)
To: devel, nathaniel.l.desimone; +Cc: Chiu, Chasel, Liming Gao, Dong, Eric
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]
>> -=-=-=-=-=-=
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread