From: "Nate DeSimone" <nathaniel.l.desimone@intel.com>
To: "mikuback@linux.microsoft.com" <mikuback@linux.microsoft.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Chiu, Chasel" <chasel.chiu@intel.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
"Dong, Eric" <eric.dong@intel.com>
Subject: Re: [edk2-platforms][PATCH v1 1/5] MinPlatformPkg/TestPointCheckLib: Fix MessageLength cast issue
Date: Thu, 5 Aug 2021 23:15:26 +0000 [thread overview]
Message-ID: <MWHPR1101MB21609173EE4C4A5F7DCB6883CDF29@MWHPR1101MB2160.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210805145706.2470-2-mikuback@linux.microsoft.com>
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
next prev parent reply other threads:[~2021-08-05 23:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
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 23:15 ` Nate DeSimone [this message]
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
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
2021-08-05 14:57 ` [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking 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: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
2021-08-06 1:34 ` Michael Kubacki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR1101MB21609173EE4C4A5F7DCB6883CDF29@MWHPR1101MB2160.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox