From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (NAM12-DM6-obe.outbound.protection.outlook.com [40.92.22.84]) by mx.groups.io with SMTP id smtpd.web12.1142.1608667571771300282 for ; Tue, 22 Dec 2020 12:06:12 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@outlook.com header.s=selector1 header.b=RPFxbJi9; spf=pass (domain: outlook.com, ip: 40.92.22.84, mailfrom: kun.q@outlook.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nHhkeYXKHfMH9YtuFivMeoRSqBWUzkrsTlGXrgpviUODvhKmhHD7s0gn50Jw+7axH+BsQKAIrGW7VZPwyOK7bi1nV+2CLkfVVrJP+GUKX/4RqleV8exOYwnN3UOeAhPlwAdI8dbRRbqI0ZpQy7FNPPfGnTgrUxr+edFe/8kAsfyxq6bn46gnC3D4Hp5X6apDPr7PsYscYN2hkwJXggbNM9WhQnVshjJduVEnIG/mn/Ssb47dYMvREbo2vQgDOW72p3IiWfI7p5vwO2CpLfUXbHhm9qY0wXGFkyHG/D1IBCVzKtwYkSqHxkPJBM5FLVpFHWAUswJW1XAQ+faC8ncbAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=m+nhYI7x3zcSEKjVfxD3+3/jT6SrPWerQpfYpbTndD4=; b=KOJHyjkdU7x/igL5ZvOvZv0QjnxiDBhrEFOywb9mAVBXUjbbVpQCLvgW5JzgWau87LdMYqYhFS/WHTYkGMP19asaa2jgyJApM7oMAP8HXPCg9RKQ+s9O22TMnVCIl6BZTxykfX7tF1oHB3LqonLkmUrRtaxQpml5l7+0R4dOhmiDFG2KaPZ5hWN16ZBi28vUGuzQ8FdvL2ltz6ZTFcDAcjjAlwJuvqxb/19+rQ/BBUzKcrPuUoGSOskeuhaDvWso5f5Hy3L5AMi2ZNrRDZ8up7b0wDi+SNx0YuczsW1d/CFuspLEkATGZFb4kJzKxGHJj7wjs8heGli+Wg4h1OejNA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=m+nhYI7x3zcSEKjVfxD3+3/jT6SrPWerQpfYpbTndD4=; b=RPFxbJi9Evf+1VSfO+1WjDiUmVTgXm/ysBmqbtPL33SvAvMmKQXLjRaKCKyeKt1emrloWpX8UEsIl46CgQz+7LmysdnP70sU6dubBEUSwEbofn9fTKf9/1oyMfnA53vzxPgO7P57SgrUvEXeEu6NdQ+Lvfy/4xc03AaD2DYSo17eRT9zwL/2NB6ee+Se8IpriqDOdJX/Um1/HLE4iwOnlEgAAtBY9v1dkG5jCOmm6Cnh/Ouf0nm7yFc7442M317hsrmNYGSuoRyavGcpjupbhxY4nCo3o2OT8uCvGN5LP2IGRTlB3ektbhUd7UwEPn2bBGH6xppEK+5JB58dVRKF/Q== Received: from DM6NAM12FT039.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc64::50) by DM6NAM12HT155.eop-nam12.prod.protection.outlook.com (2a01:111:e400:fc64::235) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3676.10; Tue, 22 Dec 2020 20:06:10 +0000 Received: from DM5PR06MB3098.namprd06.prod.outlook.com (2a01:111:e400:fc64::49) by DM6NAM12FT039.mail.protection.outlook.com (2a01:111:e400:fc64::315) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3676.10 via Frontend Transport; Tue, 22 Dec 2020 20:06:10 +0000 Received: from DM5PR06MB3098.namprd06.prod.outlook.com ([fe80::4830:27de:1fd4:e993]) by DM5PR06MB3098.namprd06.prod.outlook.com ([fe80::4830:27de:1fd4:e993%6]) with mapi id 15.20.3676.033; Tue, 22 Dec 2020 20:06:10 +0000 From: "Kun Qin" To: "devel@edk2.groups.io" , "hao.a.wu@intel.com" CC: "Wang, Jian J" , "Bi, Dandan" , Liming Gao , "Yao, Jiewen" Subject: Re: [edk2-devel] [PATCH v1 07/15] MdeModulePkg: FirmwarePerformanceDataTable: Added StandaloneMm support Thread-Topic: [edk2-devel] [PATCH v1 07/15] MdeModulePkg: FirmwarePerformanceDataTable: Added StandaloneMm support Thread-Index: AQHW1W6oyfrVZONY+kaoYxmfYqYrSaoC0BmAgACypKs= Date: Tue, 22 Dec 2020 20:06:10 +0000 Message-ID: References: <20201218185011.1366-1-kun.q@outlook.com> , In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-incomingtopheadermarker: OriginalChecksum:AA160B9544211869D6BC3D2252AFC3DBBA6C2EDC6646AD183E669BD77B5DEFC4;UpperCasedChecksum:D62715D3FAFC5A8056978DF4AEF81048ACCA91BD836F812489C188EA702C794D;SizeAsReceived:7285;Count:44 x-tmn: [E6CRD+zCcPe45Zhw/7T4/koMDhNocdIm] x-ms-publictraffictype: Email x-incomingheadercount: 44 x-eopattributedmessage: 0 x-ms-office365-filtering-correlation-id: bfd21938-9ebe-4dea-c5bf-08d8a6b50699 x-ms-traffictypediagnostic: DM6NAM12HT155: x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: +WnyLiarNhXcmRG7CXp5ZikiXobxPhmzDzjcfJOe694/bes4CGmb7ySXFcHfkXmG8Aq1OM4qOl4j+o/eENxvQSYYHR76XZmCEK2rRdsWL+qpJFbWtyYPgKBeepe59uWYgKne0KpA5pqFeKq4L3NsOjNEewRepDJOQhA0/O7bGzIQJiPD7xBn6h9PIMXUENG7TgB756VKH1KrPsNb8CuGuOOtK/iQdQB1yfVd7/lgBmsUdivyOgnYF1rYJQBABgvszp87AXKHFxpRkJTDaRt1e9rRJHDLW5FiV8YpTpywdFs= x-ms-exchange-antispam-messagedata: bAZCxOg+j8BE5km1bDSX80WMaThK9ecFtS1Nfec0QOBE+GkC/D5UHRnXRsmm40NPRJ1oitWbQRjoKHqRxFhF7qW/5wI5fvZvJHhko8aLlE28HHLUOxQgueaZkAeaLW71SYg1NUmWYxl5xmT99c0nFA== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-AuthSource: DM6NAM12FT039.eop-nam12.prod.protection.outlook.com X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: bfd21938-9ebe-4dea-c5bf-08d8a6b50699 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Dec 2020 20:06:10.1718 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6NAM12HT155 X-Groupsio-MsgNum: 69387 Content-Language: en-US Content-Type: multipart/related; boundary="_004_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_"; type="multipart/alternative" --_004_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_ Content-Type: multipart/alternative; boundary="_000_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_" --_000_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable Hi Hao, Thanks for the feedback, I will remove the EFIAPI for this patch (as well = as others you noted in other patches). As per listing the driver under [Component.X64], this is because listing t= he driver under other section will trigger IA32 build, which needs a IA32 v= ersion of `MemLib`. I will modify =93[PATCH v1 04/15] StandaloneMmPkg: Stan= daloneMmMemLib: Extends support for X64 architecture=94 accordingly to make= MemLib cover IA32 architecture as well. Thanks, Kun From: devel@edk2.groups.io on behalf of Wu, Hao A <= hao.a.wu@intel.com> Sent: Tuesday, December 22, 2020 12:36:03 AM To: Kun Qin ; devel@edk2.groups.io Cc: Wang, Jian J ; Bi, Dandan = ; Liming Gao ; Yao, Jiewen Subject: Re: [edk2-devel] [PATCH v1 07/15] MdeModulePkg: FirmwarePerforman= ceDataTable: Added StandaloneMm support > -----Original Message----- > From: Kun Qin > Sent: Saturday, December 19, 2020 2:50 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A = ; > Bi, Dandan ; Liming Gao > Subject: [PATCH v1 07/15] MdeModulePkg: FirmwarePerformanceDataTable: > Added StandaloneMm support > > This change added support of FPDT driver under StandaloneMm. It replaces > SMM version ReportStatusCode protocol with MM version. This patch also > abstracts standalone and traditional MM interfaces into separate files t= o > support each corresponding function prototypes and implementations. Could you help to remove the 'EFIAPI' for functions: FirmwarePerformanceCommonEntryPoint() IsBufferOutsideMmValid() I found that they are internal functions. Another question I have is that is there any special consideration to list= FirmwarePerformanceStandaloneMm.inf under the [Components.X64] section in the DSC file? Best Regards, Hao Wu > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Dandan Bi > Cc: Liming Gao > > Signed-off-by: Kun Qin > --- > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{Firm > warePerformanceSmm.c =3D> FirmwarePerformanceCommon.c} | 72 > ++++++++++---------- > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceStandaloneMm.c | 62 > +++++++++++++++++ > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceTraditional.c | 62 +++= ++++++++++++++ > MdeModulePkg/MdeModulePkg.dsc > | 2 + > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceCommon.h | 55 +++= ++++++++++++ > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceSmm.inf | 11 +-- > > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{Firm > warePerformanceSmm.inf =3D> FirmwarePerformanceStandaloneMm.inf} | 31 > ++++----- > 7 files changed, 237 insertions(+), 58 deletions(-) > > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.c > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceCommon.c > similarity index 77% > rename from > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceSmm.c > rename to > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceCommon.c > index d6c6e7693e4d..c2aa9455f9c4 100644 > --- > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.c > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceCommon.c > @@ -1,11 +1,11 @@ > /** @file > - This module collects performance data for SMM driver boot records and= S3 > Suspend Performance Record. > + This module collects performance data for MM driver boot records and = S3 > Suspend Performance Record. > > This module registers report status code listener to collect performa= nce > data > - for SMM driver boot records and S3 Suspend Performance Record. > + for MM driver boot records and S3 Suspend Performance Record. > > Caution: This module requires additional review when modified. > - This driver will have external input - communicate buffer in SMM mode= . > + This driver will have external input - communicate buffer in MM mode. > This external input must be validated carefully to avoid security iss= ue like > buffer overflow, integer overflow. > > @@ -16,13 +16,13 @@ > > **/ > > -#include > +#include > > -#include > +#include > > #include > > -#include > +#include > #include > #include > #include > @@ -30,23 +30,22 @@ > #include > #include > #include -#include > > #include -#include > +#include "FirmwarePerformanceCommon.h" > > -SMM_BOOT_PERFORMANCE_TABLE *mSmmBootPerformanceTable =3D > NULL; > +SMM_BOOT_PERFORMANCE_TABLE *mMmBootPerformanceTable =3D > NULL; > > -EFI_SMM_RSC_HANDLER_PROTOCOL *mRscHandlerProtocol =3D NULL; > +EFI_MM_RSC_HANDLER_PROTOCOL *mRscHandlerProtocol =3D NULL; > UINT64 mSuspendStartTime =3D 0; > BOOLEAN mS3SuspendLockBoxSaved =3D FALSE; > UINT32 mBootRecordSize =3D 0; > UINT8 *mBootRecordBuffer =3D NULL; > > -SPIN_LOCK mSmmFpdtLock; > -BOOLEAN mSmramIsOutOfResource =3D FALSE; > +SPIN_LOCK mMmFpdtLock; > +BOOLEAN mMmramIsOutOfResource =3D FALSE; > > /** > - Report status code listener for SMM. This is used to record the > performance > + Report status code listener for MM. This is used to record the > + performance > data for S3 Suspend Start and S3 Suspend End in FPDT. > > @param[in] CodeType Indicates the type of status code bei= ng > reported. > @@ -66,7 +65,7 @@ BOOLEAN mSmramIsOutOfResource = =3D FALSE; > **/ > EFI_STATUS > EFIAPI > -FpdtStatusCodeListenerSmm ( > +FpdtStatusCodeListenerMm ( > IN EFI_STATUS_CODE_TYPE CodeType, > IN EFI_STATUS_CODE_VALUE Value, > IN UINT32 Instance, > @@ -89,19 +88,19 @@ FpdtStatusCodeListenerSmm ( > // Collect one or more Boot records in boot time > // > if (Data !=3D NULL && CompareGuid (&Data->Type, > &gEdkiiFpdtExtendedFirmwarePerformanceGuid)) { > - AcquireSpinLock (&mSmmFpdtLock); > + AcquireSpinLock (&mMmFpdtLock); > // > // Get the boot performance data. > // > - CopyMem (&mSmmBootPerformanceTable, Data + 1, Data->Size); > - mBootRecordBuffer =3D ((UINT8 *) (mSmmBootPerformanceTable)) + > sizeof (SMM_BOOT_PERFORMANCE_TABLE); > + CopyMem (&mMmBootPerformanceTable, Data + 1, Data->Size); > + mBootRecordBuffer =3D ((UINT8 *) (mMmBootPerformanceTable)) + sizeo= f > + (SMM_BOOT_PERFORMANCE_TABLE); > > - ReleaseSpinLock (&mSmmFpdtLock); > + ReleaseSpinLock (&mMmFpdtLock); > return EFI_SUCCESS; > } > > if (Data !=3D NULL && CompareGuid (&Data->Type, > &gEfiFirmwarePerformanceGuid)) { > - DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListenerSmm: Performance > data reported through gEfiFirmwarePerformanceGuid will not be collected > by FirmwarePerformanceDataTableSmm\n")); > + DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListenerMm: Performance > data > + reported through gEfiFirmwarePerformanceGuid will not be collected by > + FirmwarePerformanceDataTableMm\n")); > return EFI_UNSUPPORTED; > } > > @@ -157,7 +156,7 @@ FpdtStatusCodeListenerSmm ( > /** > Communication service SMI Handler entry. > > - This SMI handler provides services for report SMM boot records. > + This SMI handler provides services for report MM boot records. > > Caution: This function may receive untrusted input. > Communicate buffer and buffer size are external input, so this functi= on will > do basic validation. > @@ -166,7 +165,7 @@ FpdtStatusCodeListenerSmm ( > @param[in] RegisterContext Points to an optional handler context = which > was specified when the > handler was registered. > @param[in, out] CommBuffer A pointer to a collection of data in m= emory > that will > - be conveyed from a non-SMM environment= into an SMM > environment. > + be conveyed from a non-MM environment = into an MM > environment. > @param[in, out] CommBufferSize The size of the CommBuffer. > > @retval EFI_SUCCESS The interrupt was handled= and quiesced. > No other handlers > @@ -207,8 +206,8 @@ FpdtSmiHandler ( > return EFI_SUCCESS; > } > > - if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, > TempCommBufferSize)) { > - DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM communication data > buffer in SMRAM or overflow!\n")); > + if (!IsBufferOutsideMmValid ((UINTN)CommBuffer, > TempCommBufferSize)) { > + DEBUG ((DEBUG_ERROR, "FpdtSmiHandler: MM communication data > buffer > + in MMRAM or overflow!\n")); > return EFI_SUCCESS; > } > > @@ -218,8 +217,8 @@ FpdtSmiHandler ( > > switch (SmmCommData->Function) { > case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE : > - if (mSmmBootPerformanceTable !=3D NULL) { > - mBootRecordSize =3D mSmmBootPerformanceTable->Header.Length - > sizeof (SMM_BOOT_PERFORMANCE_TABLE); > + if (mMmBootPerformanceTable !=3D NULL) { > + mBootRecordSize =3D mMmBootPerformanceTable->Header.Length - > + sizeof (SMM_BOOT_PERFORMANCE_TABLE); > } > SmmCommData->BootRecordSize =3D mBootRecordSize; > break; > @@ -244,8 +243,8 @@ FpdtSmiHandler ( > BootRecordSize =3D mBootRecordSize - BootRecordOffset; > } > SmmCommData->BootRecordSize =3D BootRecordSize; > - if (!SmmIsBufferOutsideSmmValid ((UINTN)BootRecordData, > BootRecordSize)) { > - DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM Data buffer in SMRAM > or overflow!\n")); > + if (!IsBufferOutsideMmValid ((UINTN)BootRecordData, BootRecordSiz= e)) > { > + DEBUG ((DEBUG_ERROR, "FpdtSmiHandler: MM Data buffer in > MMRAM > + or overflow!\n")); > Status =3D EFI_ACCESS_DENIED; > break; > } > @@ -267,7 +266,7 @@ FpdtSmiHandler ( > } > > /** > - The module Entry Point of the Firmware Performance Data Table SMM > driver. > + The module Entry Point of the Firmware Performance Data Table MM > driver. > > @param[in] ImageHandle The firmware allocated handle for the EFI > image. > @param[in] SystemTable A pointer to the EFI System Table. > @@ -278,9 +277,8 @@ FpdtSmiHandler ( > **/ > EFI_STATUS > EFIAPI > -FirmwarePerformanceSmmEntryPoint ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +FirmwarePerformanceCommonEntryPoint ( > + VOID > ) > { > EFI_STATUS Status; > @@ -289,13 +287,13 @@ FirmwarePerformanceSmmEntryPoint ( > // > // Initialize spin lock > // > - InitializeSpinLock (&mSmmFpdtLock); > + InitializeSpinLock (&mMmFpdtLock); > > // > - // Get SMM Report Status Code Handler Protocol. > + // Get MM Report Status Code Handler Protocol. > // > - Status =3D gSmst->SmmLocateProtocol ( > - &gEfiSmmRscHandlerProtocolGuid, > + Status =3D gMmst->MmLocateProtocol ( > + &gEfiMmRscHandlerProtocolGuid, > NULL, > (VOID **) &mRscHandlerProtocol > ); > @@ -304,14 +302,14 @@ FirmwarePerformanceSmmEntryPoint ( > // > // Register report status code listener for BootRecords and S3 Suspen= d > Start and End. > // > - Status =3D mRscHandlerProtocol->Register (FpdtStatusCodeListenerSmm); > + Status =3D mRscHandlerProtocol->Register (FpdtStatusCodeListenerMm); > ASSERT_EFI_ERROR (Status); > > // > // Register SMI handler. > // > Handle =3D NULL; > - Status =3D gSmst->SmiHandlerRegister (FpdtSmiHandler, > &gEfiFirmwarePerformanceGuid, &Handle); > + Status =3D gMmst->MmiHandlerRegister (FpdtSmiHandler, > + &gEfiFirmwarePerformanceGuid, &Handle); > ASSERT_EFI_ERROR (Status); > > return Status; > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceStandaloneMm.c > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceStandaloneMm.c > new file mode 100644 > index 000000000000..2c4f40eceaea > --- /dev/null > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceStandaloneMm.c > @@ -0,0 +1,62 @@ > +/** @file > + This module collects performance data for MM driver boot records and = S3 > Suspend Performance Record. > + > + This module registers report status code listener to collect > + performance data for MM driver boot records and S3 Suspend > Performance Record. > + > + Caution: This module requires additional review when modified. > + This driver will have external input - communicate buffer in MM mode. > + This external input must be validated carefully to avoid security > + issue like buffer overflow, integer overflow. > + > + FpdtSmiHandler() will receive untrusted input and do basic validation= . > + > + Copyright (c) 2011 - 2018, Intel Corporation. All rights > + reserved.
Copyright (c), Microsoft Corporation. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include #include > +"FirmwarePerformanceCommon.h" > + > +/** > + This function is an abstraction layer for implementation specific Mm = buffer > validation routine. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and not > overlap with SMRAM. > + @retval FALSE This buffer is not valid per processor architecture or = overlap > with SMRAM. > +**/ > +BOOLEAN > +EFIAPI > +IsBufferOutsideMmValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ) > +{ > + return MmIsBufferOutsideMmValid (Buffer, Length); } > + > +/** > + The module Entry Point of the Firmware Performance Data Table MM > driver. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI > image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The entry point is executed successfully. > + @retval Other Some error occurs when executing this entry po= int. > + > +**/ > +EFI_STATUS > +EFIAPI > +FirmwarePerformanceStandaloneMmEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *SystemTable > + ) > +{ > + return FirmwarePerformanceCommonEntryPoint (); } > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceTraditional.c > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceTraditional.c > new file mode 100644 > index 000000000000..507aabcb4e88 > --- /dev/null > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceTraditional.c > @@ -0,0 +1,62 @@ > +/** @file > + This module collects performance data for MM driver boot records and = S3 > Suspend Performance Record. > + > + This module registers report status code listener to collect > + performance data for MM driver boot records and S3 Suspend > Performance Record. > + > + Caution: This module requires additional review when modified. > + This driver will have external input - communicate buffer in MM mode. > + This external input must be validated carefully to avoid security > + issue like buffer overflow, integer overflow. > + > + FpdtSmiHandler() will receive untrusted input and do basic validation= . > + > + Copyright (c) 2011 - 2018, Intel Corporation. All rights > + reserved.
Copyright (c), Microsoft Corporation. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include > +#include "FirmwarePerformanceCommon.h" > + > +/** > + This function is an abstraction layer for implementation specific Mm = buffer > validation routine. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and not > overlap with SMRAM. > + @retval FALSE This buffer is not valid per processor architecture or = overlap > with SMRAM. > +**/ > +BOOLEAN > +EFIAPI > +IsBufferOutsideMmValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ) > +{ > + return SmmIsBufferOutsideSmmValid (Buffer, Length); } > + > +/** > + The module Entry Point of the Firmware Performance Data Table MM > driver. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI > image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The entry point is executed successfully. > + @retval Other Some error occurs when executing this entry po= int. > + > +**/ > +EFI_STATUS > +EFIAPI > +FirmwarePerformanceSmmEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return FirmwarePerformanceCommonEntryPoint (); } > diff --git a/MdeModulePkg/MdeModulePkg.dsc > b/MdeModulePkg/MdeModulePkg.dsc index 05bf5fe08094..dc85d108d958 > 100644 > --- a/MdeModulePkg/MdeModulePkg.dsc > +++ b/MdeModulePkg/MdeModulePkg.dsc > @@ -168,6 +168,7 @@ [LibraryClasses.common.MM_STANDALONE] > > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntry > Point/StandaloneMmDriverEntryPoint.inf > > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stan > daloneMmServicesTableLib.inf > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalo > neMmLib.inf > + > + > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm > MemLib.i > + nf > > [LibraryClasses.ARM, LibraryClasses.AARCH64] > ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf > @@ -501,6 +502,7 @@ [Components.IA32, Components.X64] > > [Components.X64] > MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf > + > + > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePe > + rformanceStandaloneMm.inf > > [BuildOptions] > > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceCommon.h > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceCommon.h > new file mode 100644 > index 000000000000..ba01e28f2493 > --- /dev/null > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceCommon.h > @@ -0,0 +1,55 @@ > +/** @file > + This module collects performance data for SMM driver boot records and= S3 > Suspend Performance Record. > + > + This module registers report status code listener to collect > + performance data for SMM driver boot records and S3 Suspend > Performance Record. > + > + Caution: This module requires additional review when modified. > + This driver will have external input - communicate buffer in SMM mode= . > + This external input must be validated carefully to avoid security > + issue like buffer overflow, integer overflow. > + > + FpdtSmiHandler() will receive untrusted input and do basic validation= . > + > + Copyright (c) 2011 - 2018, Intel Corporation. All rights > + reserved.
Copyright (c), Microsoft Corporation. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef _FW_PERF_COMMON_H_ > +#define _FW_PERF_COMMON_H_ > + > +/** > + This function is an abstraction layer for implementation specific Mm = buffer > validation routine. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and not > overlap with SMRAM. > + @retval FALSE This buffer is not valid per processor architecture or = overlap > with SMRAM. > +**/ > +BOOLEAN > +EFIAPI > +IsBufferOutsideMmValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ); > + > +/** > + The module Entry Point of the Firmware Performance Data Table MM > driver. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI > image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The entry point is executed successfully. > + @retval Other Some error occurs when executing this entry po= int. > + > +**/ > +EFI_STATUS > +EFIAPI > +FirmwarePerformanceCommonEntryPoint ( > + VOID > + ); > + > +#endif // _FW_PERF_COMMON_H_ > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.inf > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.inf > index 618cbd56ca59..b7194bd899dd 100644 > --- > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.inf > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceSmm.inf > @@ -26,7 +26,9 @@ [Defines] > # > > [Sources] > - FirmwarePerformanceSmm.c > + FirmwarePerformanceCommon.c > + FirmwarePerformanceCommon.h > + FirmwarePerformanceTraditional.c > > [Packages] > MdePkg/MdePkg.dec > @@ -34,7 +36,7 @@ [Packages] > > [LibraryClasses] > UefiDriverEntryPoint > - SmmServicesTableLib > + MmServicesTableLib > BaseLib > DebugLib > TimerLib > @@ -42,12 +44,11 @@ [LibraryClasses] > PcdLib > BaseMemoryLib > MemoryAllocationLib > - UefiBootServicesTableLib > SynchronizationLib > SmmMemLib > > [Protocols] > - gEfiSmmRscHandlerProtocolGuid ## CONSUMES > + gEfiMmRscHandlerProtocolGuid ## CONSUMES > > [Guids] > ## SOMETIMES_PRODUCES ## UNDEFINED # SaveLockBox > @@ -61,7 +62,7 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeS3SuspendEnd ## > CONSUMES > > [Depex] > - gEfiSmmRscHandlerProtocolGuid > + gEfiMmRscHandlerProtocolGuid > > [UserExtensions.TianoCore."ExtraFiles"] > FirmwarePerformanceSmmExtra.uni > diff --git > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.inf > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceStandaloneMm.inf > similarity index 65% > copy from > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceSmm.inf > copy to > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm > warePerformanceStandaloneMm.inf > index 618cbd56ca59..e6aad88be0ef 100644 > --- > a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwarePerformanceSmm.inf > +++ > b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir > mwa > +++ rePerformanceStandaloneMm.inf > @@ -5,19 +5,19 @@ > # for SMM boot performance records and S3 Suspend Performance Record. > # > # Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) Microsoft Corporation. > # SPDX-License-Identifier: BSD-2-Clause-Patent # ## > > [Defines] > INF_VERSION =3D 0x00010005 > - BASE_NAME =3D FirmwarePerformanceSmm > - MODULE_UNI_FILE =3D FirmwarePerformanceSmm.uni > - FILE_GUID =3D 044310AB-77FD-402a-AF1A-87D4120E73= 29 > - MODULE_TYPE =3D DXE_SMM_DRIVER > + BASE_NAME =3D FirmwarePerformanceStandaloneMm > + FILE_GUID =3D 827AC29D-E52D-4B1A-874A-C6577E0699= CF > + MODULE_TYPE =3D MM_STANDALONE > VERSION_STRING =3D 1.0 > - PI_SPECIFICATION_VERSION =3D 0x0001000A > - ENTRY_POINT =3D FirmwarePerformanceSmmEntryPoint > + PI_SPECIFICATION_VERSION =3D 0x00010032 > + ENTRY_POINT =3D FirmwarePerformanceStandaloneMmEnt= ryPoint > > # > # The following information is for reference only and not required by t= he > build tools. > @@ -26,15 +26,18 @@ [Defines] > # > > [Sources] > - FirmwarePerformanceSmm.c > + FirmwarePerformanceCommon.c > + FirmwarePerformanceCommon.h > + FirmwarePerformanceStandaloneMm.c > > [Packages] > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > + StandaloneMmPkg/StandaloneMmPkg.dec > > [LibraryClasses] > - UefiDriverEntryPoint > - SmmServicesTableLib > + StandaloneMmDriverEntryPoint > + MmServicesTableLib > BaseLib > DebugLib > TimerLib > @@ -42,12 +45,11 @@ [LibraryClasses] > PcdLib > BaseMemoryLib > MemoryAllocationLib > - UefiBootServicesTableLib > SynchronizationLib > - SmmMemLib > + MemLib > > [Protocols] > - gEfiSmmRscHandlerProtocolGuid ## CONSUMES > + gEfiMmRscHandlerProtocolGuid ## CONSUMES > > [Guids] > ## SOMETIMES_PRODUCES ## UNDEFINED # SaveLockBox > @@ -61,7 +63,4 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeS3SuspendEnd ## > CONSUMES > > [Depex] > - gEfiSmmRscHandlerProtocolGuid > - > -[UserExtensions.TianoCore."ExtraFiles"] > - FirmwarePerformanceSmmExtra.uni > + gEfiMmRscHandlerProtocolGuid > -- > 2.28.0.windows.1 --_000_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

Hi Hao,

 

Thanks for the feedback, I will remove the EFIAPI f= or this patch (as well as others you noted in other patches).

 

As per listing the driver under [Component.X64], th= is is because listing the driver under other section will trigger IA32 buil= d, which needs a IA32 version of `MemLib`. I will modify =93[PATCH v1 04/15= ] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture=94 accordingly to make MemLib cover = IA32 architecture as well.

 

Thanks,

Kun

 

From: devel@edk2.groups.io <devel@edk2.groups.io> = on behalf of Wu, Hao A <hao.a.wu@intel.com>
Sent: Tuesday, December 22, 2020 12:36:03 AM
To: Kun Qin <kun.q@outlook.com>; devel@edk2.groups.io <dev= el@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Bi, Dandan <dand= an.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Yao, Jiew= en <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 07/15] MdeModulePkg: FirmwarePe= rformanceDataTable: Added StandaloneMm support

 

> -----Original M= essage-----
> From: Kun Qin <kun.q@outlook.com>
> Sent: Saturday, December 19, 2020 2:50 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.w= u@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>; Liming Gao <gaoliming@byos= oft.com.cn>
> Subject: [PATCH v1 07/15] MdeModulePkg: FirmwarePerformanceDataTable:=
> Added StandaloneMm support
>
> This change added support of FPDT driver under StandaloneMm. It repla= ces
> SMM version ReportStatusCode protocol with MM version. This patch als= o
> abstracts standalone and traditional MM interfaces into separate file= s to
> support each corresponding function prototypes and implementations.

Could you help to remove the 'EFIAPI' for functions:
  FirmwarePerformanceCommonEntryPoint()
  IsBufferOutsideMmValid()
I found that they are internal functions.

Another question I have is that is there any special consideration to list= FirmwarePerformanceStandaloneMm.inf
under the [Components.X64] section in the DSC file?

Best Regards,
Hao Wu


>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Signed-off-by: Kun Qin <kun.q@outlook.com>
> ---
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{Firm
> warePerformanceSmm.c =3D> FirmwarePerformanceCommon.c}  =          | 72
> ++++++++++----------
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceStandaloneMm.c      &nbs= p;            &= nbsp;           &nbs= p;   | 62
> +++++++++++++++++
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceTraditional.c       = ;            &n= bsp;            = ;    | 62 +++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dsc
> |  2 +
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceCommon.h       &nbs= p;            &= nbsp;           &nbs= p;        | 55 +++++++++++++++
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceSmm.inf        = ;            &n= bsp;            = ;         | 11 +--
>
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{Firm
> warePerformanceSmm.inf =3D> FirmwarePerformanceStandaloneMm.inf} |= 31
> ++++-----
>  7 files changed, 237 insertions(+), 58 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.c
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceCommon.c
> similarity index 77%
> rename from
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceSmm.c
> rename to
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceCommon.c
> index d6c6e7693e4d..c2aa9455f9c4 100644
> ---
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.c
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceCommon.c
> @@ -1,11 +1,11 @@
>  /** @file
> -  This module collects performance data for SMM driver boot rec= ords and S3
> Suspend Performance Record.
> +  This module collects performance data for MM driver boot reco= rds and S3
> Suspend Performance Record.
>
>    This module registers report status code listener t= o collect performance
> data
> -  for SMM driver boot records and S3 Suspend Performance Record= .
> +  for MM driver boot records and S3 Suspend Performance Record.=
>
>    Caution: This module requires additional review whe= n modified.
> -  This driver will have external input - communicate buffer in = SMM mode.
> +  This driver will have external input - communicate buffer in = MM mode.
>    This external input must be validated carefully to = avoid security issue like
>    buffer overflow, integer overflow.
>
> @@ -16,13 +16,13 @@
>
>  **/
>
> -#include <PiSmm.h>
> +#include <PiMm.h>
>
> -#include <Protocol/SmmReportStatusCodeHandler.h>
> +#include <Protocol/MmReportStatusCodeHandler.h>
>
>  #include <Guid/FirmwarePerformance.h>
>
> -#include <Library/SmmServicesTableLib.h>
> +#include <Library/MmServicesTableLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/TimerLib.h>
> @@ -30,23 +30,22 @@
>  #include <Library/PcdLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/MemoryAllocationLib.h> -#include
> <Library/UefiBootServicesTableLib.h>
>  #include <Library/SynchronizationLib.h> -#include <Lib= rary/SmmMemLib.h>
> +#include "FirmwarePerformanceCommon.h"
>
> -SMM_BOOT_PERFORMANCE_TABLE    *mSmmBootPerformanceTab= le =3D
> NULL;
> +SMM_BOOT_PERFORMANCE_TABLE    *mMmBootPerformanceTabl= e =3D
> NULL;
>
> -EFI_SMM_RSC_HANDLER_PROTOCOL  *mRscHandlerProtocol  &= nbsp; =3D NULL;
> +EFI_MM_RSC_HANDLER_PROTOCOL   *mRscHandlerProtocol &n= bsp;  =3D NULL;
>  UINT64         &nb= sp;            =   mSuspendStartTime       =3D 0;
>  BOOLEAN         &n= bsp;            = ; mS3SuspendLockBoxSaved  =3D FALSE;
>  UINT32         &nb= sp;            =   mBootRecordSize =3D 0;
>  UINT8         &nbs= p;            &= nbsp;  *mBootRecordBuffer =3D NULL;
>
> -SPIN_LOCK          = ;           mSmmFpdtLock;=
> -BOOLEAN          &= nbsp;            mSm= ramIsOutOfResource =3D FALSE;
> +SPIN_LOCK          = ;           mMmFpdtLock;<= br> > +BOOLEAN          &= nbsp;            mMm= ramIsOutOfResource =3D FALSE;
>
>  /**
> -  Report status code listener for SMM. This is used to record t= he
> performance
> +  Report status code listener for MM. This is used to record th= e
> + performance
>    data for S3 Suspend Start and S3 Suspend End in FPD= T.
>
>    @param[in]  CodeType    &n= bsp;       Indicates the type of status code = being
> reported.
> @@ -66,7 +65,7 @@ BOOLEAN       &n= bsp;            = ;   mSmramIsOutOfResource =3D FALSE;
>  **/
>  EFI_STATUS
>  EFIAPI
> -FpdtStatusCodeListenerSmm (
> +FpdtStatusCodeListenerMm (
>    IN EFI_STATUS_CODE_TYPE     Cod= eType,
>    IN EFI_STATUS_CODE_VALUE    Value, >    IN UINT32       =             Instance= ,
> @@ -89,19 +88,19 @@ FpdtStatusCodeListenerSmm (
>    // Collect one or more Boot records in boot time >    //
>    if (Data !=3D NULL && CompareGuid (&Dat= a->Type,
> &gEdkiiFpdtExtendedFirmwarePerformanceGuid)) {
> -    AcquireSpinLock (&mSmmFpdtLock);
> +    AcquireSpinLock (&mMmFpdtLock);
>      //
>      // Get the boot performance data.
>      //
> -    CopyMem (&mSmmBootPerformanceTable, Data + 1,= Data->Size);
> -    mBootRecordBuffer =3D ((UINT8 *) (mSmmBootPerform= anceTable)) +
> sizeof (SMM_BOOT_PERFORMANCE_TABLE);
> +    CopyMem (&mMmBootPerformanceTable, Data + 1, = Data->Size);
> +    mBootRecordBuffer =3D ((UINT8 *) (mMmBootPerforma= nceTable)) + sizeof
> + (SMM_BOOT_PERFORMANCE_TABLE);
>
> -    ReleaseSpinLock (&mSmmFpdtLock);
> +    ReleaseSpinLock (&mMmFpdtLock);
>      return EFI_SUCCESS;
>    }
>
>    if (Data !=3D NULL && CompareGuid (&Dat= a->Type,
> &gEfiFirmwarePerformanceGuid)) {
> -    DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListener= Smm: Performance
> data reported through gEfiFirmwarePerformanceGuid will not be collect= ed
> by FirmwarePerformanceDataTableSmm\n"));
> +    DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListener= Mm: Performance
> data
> + reported through gEfiFirmwarePerformanceGuid will not be collected = by
> + FirmwarePerformanceDataTableMm\n"));
>      return EFI_UNSUPPORTED;
>    }
>
> @@ -157,7 +156,7 @@ FpdtStatusCodeListenerSmm (
>  /**
>    Communication service SMI Handler entry.
>
> -  This SMI handler provides services for report SMM boot record= s.
> +  This SMI handler provides services for report MM boot records= .
>
>    Caution: This function may receive untrusted input.=
>    Communicate buffer and buffer size are external inp= ut, so this function will
> do basic validation.
> @@ -166,7 +165,7 @@ FpdtStatusCodeListenerSmm (
>    @param[in]     RegisterContext = Points to an optional handler context which
> was specified when the
>           &nbs= p;            &= nbsp;          handler was reg= istered.
>    @param[in, out] CommBuffer     = A pointer to a collection of data in memory
> that will
> -           &n= bsp;            = ;         be conveyed from a non-SM= M environment into an SMM
> environment.
> +           &n= bsp;            = ;         be conveyed from a non-MM= environment into an MM
> environment.
>    @param[in, out] CommBufferSize The size of the Comm= Buffer.
>
>    @retval EFI_SUCCESS     &n= bsp;            = ;       The interrupt was handled and quiesce= d.
> No other handlers
> @@ -207,8 +206,8 @@ FpdtSmiHandler (
>      return EFI_SUCCESS;
>    }
>
> -  if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer,
> TempCommBufferSize)) {
> -    DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM co= mmunication data
> buffer in SMRAM or overflow!\n"));
> +  if (!IsBufferOutsideMmValid ((UINTN)CommBuffer,
> TempCommBufferSize)) {
> +    DEBUG ((DEBUG_ERROR, "FpdtSmiHandler: MM com= munication data
> buffer
> + in MMRAM or overflow!\n"));
>      return EFI_SUCCESS;
>    }
>
> @@ -218,8 +217,8 @@ FpdtSmiHandler (
>
>    switch (SmmCommData->Function) {
>      case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_= SIZE :
> -      if (mSmmBootPerformanceTable !=3D NUL= L) {
> -        mBootRecordSize =3D mSmmB= ootPerformanceTable->Header.Length -
> sizeof (SMM_BOOT_PERFORMANCE_TABLE);
> +      if (mMmBootPerformanceTable !=3D NULL= ) {
> +        mBootRecordSize =3D mMmBo= otPerformanceTable->Header.Length -
> + sizeof (SMM_BOOT_PERFORMANCE_TABLE);
>        }
>        SmmCommData->BootRecordS= ize =3D mBootRecordSize;
>        break;
> @@ -244,8 +243,8 @@ FpdtSmiHandler (
>          BootRecordSize = = =3D mBootRecordSize - BootRecordOffset;
>        }
>        SmmCommData->BootRecordS= ize =3D BootRecordSize;
> -      if (!SmmIsBufferOutsideSmmValid ((UIN= TN)BootRecordData,
> BootRecordSize)) {
> -        DEBUG ((EFI_D_ERROR, &quo= t;FpdtSmiHandler: SMM Data buffer in SMRAM
> or overflow!\n"));
> +      if (!IsBufferOutsideMmValid ((UINTN)B= ootRecordData, BootRecordSize))
> {
> +        DEBUG ((DEBUG_ERROR, &quo= t;FpdtSmiHandler: MM Data buffer in
> MMRAM
> + or overflow!\n"));
>          Status =3D EFI_= ACCESS_DENIED;
>          break;
>        }
> @@ -267,7 +266,7 @@ FpdtSmiHandler (
>  }
>
>  /**
> -  The module Entry Point of the Firmware Performance Data Table= SMM
> driver.
> +  The module Entry Point of the Firmware Performance Data Table= MM
> driver.
>
>    @param[in]  ImageHandle    The = firmware allocated handle for the EFI
> image.
>    @param[in]  SystemTable    A po= inter to the EFI System Table.
> @@ -278,9 +277,8 @@ FpdtSmiHandler (
>  **/
>  EFI_STATUS
>  EFIAPI
> -FirmwarePerformanceSmmEntryPoint (
> -  IN EFI_HANDLE        =   ImageHandle,
> -  IN EFI_SYSTEM_TABLE    *SystemTable
> +FirmwarePerformanceCommonEntryPoint (
> +  VOID
>    )
>  {
>    EFI_STATUS       = ;         Status;
> @@ -289,13 +287,13 @@ FirmwarePerformanceSmmEntryPoint (
>    //
>    // Initialize spin lock
>    //
> -  InitializeSpinLock (&mSmmFpdtLock);
> +  InitializeSpinLock (&mMmFpdtLock);
>
>    //
> -  // Get SMM Report Status Code Handler Protocol.
> +  // Get MM Report Status Code Handler Protocol.
>    //
> -  Status =3D gSmst->SmmLocateProtocol (
> -           &n= bsp;        &gEfiSmmRscHandlerProtoc= olGuid,
> +  Status =3D gMmst->MmLocateProtocol (
> +           &n= bsp;        &gEfiMmRscHandlerProtoco= lGuid,
>           &nbs= p;          NULL,
>           &nbs= p;          (VOID **) &mRs= cHandlerProtocol
>           &nbs= p;          );
> @@ -304,14 +302,14 @@ FirmwarePerformanceSmmEntryPoint (
>    //
>    // Register report status code listener for BootRec= ords and S3 Suspend
> Start and End.
>    //
> -  Status =3D mRscHandlerProtocol->Register (FpdtStatusCodeLi= stenerSmm);
> +  Status =3D mRscHandlerProtocol->Register (FpdtStatusCodeLi= stenerMm);
>    ASSERT_EFI_ERROR (Status);
>
>    //
>    // Register SMI handler.
>    //
>    Handle =3D NULL;
> -  Status =3D gSmst->SmiHandlerRegister (FpdtSmiHandler,
> &gEfiFirmwarePerformanceGuid, &Handle);
> +  Status =3D gMmst->MmiHandlerRegister (FpdtSmiHandler,
> + &gEfiFirmwarePerformanceGuid, &Handle);
>    ASSERT_EFI_ERROR (Status);
>
>    return Status;
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceStandaloneMm.c
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceStandaloneMm.c
> new file mode 100644
> index 000000000000..2c4f40eceaea
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceStandaloneMm.c
> @@ -0,0 +1,62 @@
> +/** @file
> +  This module collects performance data for MM driver boot reco= rds and S3
> Suspend Performance Record.
> +
> +  This module registers report status code listener to collect<= br> > + performance data  for MM driver boot records and S3 Suspend > Performance Record.
> +
> +  Caution: This module requires additional review when modified= .
> +  This driver will have external input - communicate buffer in = MM mode.
> +  This external input must be validated carefully to avoid secu= rity
> + issue like  buffer overflow, integer overflow.
> +
> +  FpdtSmiHandler() will receive untrusted input and do basic va= lidation.
> +
> +  Copyright (c) 2011 - 2018, Intel Corporation. All rights
> + reserved.<BR>  Copyright (c), Microsoft Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiMm.h>
> +
> +#include <Library/StandaloneMmMemLib.h> #include
> +"FirmwarePerformanceCommon.h"
> +
> +/**
> +  This function is an abstraction layer for implementation spec= ific Mm buffer
> validation routine.
> +
> +  @param Buffer  The buffer start address to be checked. > +  @param Length  The buffer length to be checked.
> +
> +  @retval TRUE  This buffer is valid per processor archite= cture and not
> overlap with SMRAM.
> +  @retval FALSE This buffer is not valid per processor architec= ture or overlap
> with SMRAM.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsBufferOutsideMmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64        &nbs= p;       Length
> +  )
> +{
> +  return MmIsBufferOutsideMmValid (Buffer, Length); }
> +
> +/**
> +  The module Entry Point of the Firmware Performance Data Table= MM
> driver.
> +
> +  @param[in]  ImageHandle    The firmware a= llocated handle for the EFI
> image.
> +  @param[in]  SystemTable    A pointer to t= he EFI System Table.
> +
> +  @retval EFI_SUCCESS    The entry point is exec= uted successfully.
> +  @retval Other        =   Some error occurs when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +FirmwarePerformanceStandaloneMmEntryPoint (
> +  IN EFI_HANDLE        =      ImageHandle,
> +  IN EFI_MM_SYSTEM_TABLE    *SystemTable
> +  )
> +{
> +  return FirmwarePerformanceCommonEntryPoint (); }
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceTraditional.c
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceTraditional.c
> new file mode 100644
> index 000000000000..507aabcb4e88
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceTraditional.c
> @@ -0,0 +1,62 @@
> +/** @file
> +  This module collects performance data for MM driver boot reco= rds and S3
> Suspend Performance Record.
> +
> +  This module registers report status code listener to collect<= br> > + performance data  for MM driver boot records and S3 Suspend > Performance Record.
> +
> +  Caution: This module requires additional review when modified= .
> +  This driver will have external input - communicate buffer in = MM mode.
> +  This external input must be validated carefully to avoid secu= rity
> + issue like  buffer overflow, integer overflow.
> +
> +  FpdtSmiHandler() will receive untrusted input and do basic va= lidation.
> +
> +  Copyright (c) 2011 - 2018, Intel Corporation. All rights
> + reserved.<BR>  Copyright (c), Microsoft Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiSmm.h>
> +
> +#include <Library/SmmMemLib.h>
> +#include "FirmwarePerformanceCommon.h"
> +
> +/**
> +  This function is an abstraction layer for implementation spec= ific Mm buffer
> validation routine.
> +
> +  @param Buffer  The buffer start address to be checked. > +  @param Length  The buffer length to be checked.
> +
> +  @retval TRUE  This buffer is valid per processor archite= cture and not
> overlap with SMRAM.
> +  @retval FALSE This buffer is not valid per processor architec= ture or overlap
> with SMRAM.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsBufferOutsideMmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64        &nbs= p;       Length
> +  )
> +{
> +  return SmmIsBufferOutsideSmmValid (Buffer, Length); }
> +
> +/**
> +  The module Entry Point of the Firmware Performance Data Table= MM
> driver.
> +
> +  @param[in]  ImageHandle    The firmware a= llocated handle for the EFI
> image.
> +  @param[in]  SystemTable    A pointer to t= he EFI System Table.
> +
> +  @retval EFI_SUCCESS    The entry point is exec= uted successfully.
> +  @retval Other        =   Some error occurs when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +FirmwarePerformanceSmmEntryPoint (
> +  IN EFI_HANDLE        =   ImageHandle,
> +  IN EFI_SYSTEM_TABLE    *SystemTable
> +  )
> +{
> +  return FirmwarePerformanceCommonEntryPoint (); }
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc index 05bf5fe08094..dc85d108d958
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -168,6 +168,7 @@ [LibraryClasses.common.MM_STANDALONE]
>
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntry > Point/StandaloneMmDriverEntryPoint.inf
>
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stan > daloneMmServicesTableLib.inf
>
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalo
> neMmLib.inf
> +
> +
> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> MemLib.i
> + nf
>
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> @@ -501,6 +502,7 @@ [Components.IA32, Components.X64]
>
>  [Components.X64]
>    MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf > +
> +
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePe
> + rformanceStandaloneMm.inf
>
>  [BuildOptions]
>
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceCommon.h
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceCommon.h
> new file mode 100644
> index 000000000000..ba01e28f2493
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceCommon.h
> @@ -0,0 +1,55 @@
> +/** @file
> +  This module collects performance data for SMM driver boot rec= ords and S3
> Suspend Performance Record.
> +
> +  This module registers report status code listener to collect<= br> > + performance data  for SMM driver boot records and S3 Suspend > Performance Record.
> +
> +  Caution: This module requires additional review when modified= .
> +  This driver will have external input - communicate buffer in = SMM mode.
> +  This external input must be validated carefully to avoid secu= rity
> + issue like  buffer overflow, integer overflow.
> +
> +  FpdtSmiHandler() will receive untrusted input and do basic va= lidation.
> +
> +  Copyright (c) 2011 - 2018, Intel Corporation. All rights
> + reserved.<BR>  Copyright (c), Microsoft Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _FW_PERF_COMMON_H_
> +#define _FW_PERF_COMMON_H_
> +
> +/**
> +  This function is an abstraction layer for implementation spec= ific Mm buffer
> validation routine.
> +
> +  @param Buffer  The buffer start address to be checked. > +  @param Length  The buffer length to be checked.
> +
> +  @retval TRUE  This buffer is valid per processor archite= cture and not
> overlap with SMRAM.
> +  @retval FALSE This buffer is not valid per processor architec= ture or overlap
> with SMRAM.
> +**/
> +BOOLEAN
> +EFIAPI
> +IsBufferOutsideMmValid (
> +  IN EFI_PHYSICAL_ADDRESS  Buffer,
> +  IN UINT64        &nbs= p;       Length
> +  );
> +
> +/**
> +  The module Entry Point of the Firmware Performance Data Table= MM
> driver.
> +
> +  @param[in]  ImageHandle    The firmware a= llocated handle for the EFI
> image.
> +  @param[in]  SystemTable    A pointer to t= he EFI System Table.
> +
> +  @retval EFI_SUCCESS    The entry point is exec= uted successfully.
> +  @retval Other        =   Some error occurs when executing this entry point.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +FirmwarePerformanceCommonEntryPoint (
> +  VOID
> +  );
> +
> +#endif // _FW_PERF_COMMON_H_
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.inf
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.inf
> index 618cbd56ca59..b7194bd899dd 100644
> ---
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.inf
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceSmm.inf
> @@ -26,7 +26,9 @@ [Defines]
>  #
>
>  [Sources]
> -  FirmwarePerformanceSmm.c
> +  FirmwarePerformanceCommon.c
> +  FirmwarePerformanceCommon.h
> +  FirmwarePerformanceTraditional.c
>
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -34,7 +36,7 @@ [Packages]
>
>  [LibraryClasses]
>    UefiDriverEntryPoint
> -  SmmServicesTableLib
> +  MmServicesTableLib
>    BaseLib
>    DebugLib
>    TimerLib
> @@ -42,12 +44,11 @@ [LibraryClasses]
>    PcdLib
>    BaseMemoryLib
>    MemoryAllocationLib
> -  UefiBootServicesTableLib
>    SynchronizationLib
>    SmmMemLib
>
>  [Protocols]
> -  gEfiSmmRscHandlerProtocolGuid     &n= bsp;           ## CONSUME= S
> +  gEfiMmRscHandlerProtocolGuid     &nb= sp;           ## CONSUMES=
>
>  [Guids]
>    ## SOMETIMES_PRODUCES   ## UNDEFINED # Sa= veLockBox
> @@ -61,7 +62,7 @@ [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeS3Sus= pendEnd    ##
> CONSUMES
>
>  [Depex]
> -  gEfiSmmRscHandlerProtocolGuid
> +  gEfiMmRscHandlerProtocolGuid
>
>  [UserExtensions.TianoCore."ExtraFiles"]
>    FirmwarePerformanceSmmExtra.uni
> diff --git
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.inf
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceStandaloneMm.inf
> similarity index 65%
> copy from
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceSmm.inf
> copy to
> MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Firm
> warePerformanceStandaloneMm.inf
> index 618cbd56ca59..e6aad88be0ef 100644
> ---
> a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwarePerformanceSmm.inf
> +++
> b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/Fir
> mwa
> +++ rePerformanceStandaloneMm.inf
> @@ -5,19 +5,19 @@
>  #  for SMM boot performance records and S3 Suspend Perform= ance Record.
>  #
>  #  Copyright (c) 2011 - 2018, Intel Corporation. All right= s reserved.<BR>
> +#  Copyright (c) Microsoft Corporation.
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  #&nb= sp; ##
>
>  [Defines]
>    INF_VERSION      &nbs= p;             = = =3D 0x00010005
> -  BASE_NAME        &nbs= p;             = = =3D FirmwarePerformanceSmm
> -  MODULE_UNI_FILE       &nbs= p;        =3D FirmwarePerformanceSmm.uni=
> -  FILE_GUID        &nbs= p;             = = =3D 044310AB-77FD-402a-AF1A-87D4120E7329
> -  MODULE_TYPE        &n= bsp;           =3D DXE_SM= M_DRIVER
> +  BASE_NAME        &nbs= p;             = = =3D FirmwarePerformanceStandaloneMm
> +  FILE_GUID        &nbs= p;             = = =3D 827AC29D-E52D-4B1A-874A-C6577E0699CF
> +  MODULE_TYPE        &n= bsp;           =3D MM_STA= NDALONE
>    VERSION_STRING      &= nbsp;          =3D 1.0
> -  PI_SPECIFICATION_VERSION       = = =3D 0x0001000A
> -  ENTRY_POINT        &n= bsp;           =3D Firmwa= rePerformanceSmmEntryPoint
> +  PI_SPECIFICATION_VERSION       = = =3D 0x00010032
> +  ENTRY_POINT        &n= bsp;           =3D Firmwa= rePerformanceStandaloneMmEntryPoint
>
>  #
>  # The following information is for reference only and not requi= red by the
> build tools.
> @@ -26,15 +26,18 @@ [Defines]
>  #
>
>  [Sources]
> -  FirmwarePerformanceSmm.c
> +  FirmwarePerformanceCommon.c
> +  FirmwarePerformanceCommon.h
> +  FirmwarePerformanceStandaloneMm.c
>
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  StandaloneMmPkg/StandaloneMmPkg.dec
>
>  [LibraryClasses]
> -  UefiDriverEntryPoint
> -  SmmServicesTableLib
> +  StandaloneMmDriverEntryPoint
> +  MmServicesTableLib
>    BaseLib
>    DebugLib
>    TimerLib
> @@ -42,12 +45,11 @@ [LibraryClasses]
>    PcdLib
>    BaseMemoryLib
>    MemoryAllocationLib
> -  UefiBootServicesTableLib
>    SynchronizationLib
> -  SmmMemLib
> +  MemLib
>
>  [Protocols]
> -  gEfiSmmRscHandlerProtocolGuid     &n= bsp;           ## CONSUME= S
> +  gEfiMmRscHandlerProtocolGuid     &nb= sp;           ## CONSUMES=
>
>  [Guids]
>    ## SOMETIMES_PRODUCES   ## UNDEFINED # Sa= veLockBox
> @@ -61,7 +63,4 @@ [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeS3Sus= pendEnd    ##
> CONSUMES
>
>  [Depex]
> -  gEfiSmmRscHandlerProtocolGuid
> -
> -[UserExtensions.TianoCore."ExtraFiles"]
> -  FirmwarePerformanceSmmExtra.uni
> +  gEfiMmRscHandlerProtocolGuid
> --
> 2.28.0.windows.1





--_000_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_-- --_004_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_ Content-Type: image/png; name="E86BA3DA0C6844F8BBD99EC4C8136AF2.png" Content-Description: E86BA3DA0C6844F8BBD99EC4C8136AF2.png Content-Disposition: inline; filename="E86BA3DA0C6844F8BBD99EC4C8136AF2.png"; size=136; creation-date="Tue, 22 Dec 2020 20:06:09 GMT"; modification-date="Tue, 22 Dec 2020 20:06:09 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAArIAAAABCAYAAAAiuE1yAAAAAXNSR0IArs4c6QAAAARnQU1BAACx jwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAAdSURBVEhL7cMBCQAADAOg9S+9r8ZBwUxVVfXX 9AA07K9tyVMctgAAAABJRU5ErkJggg== --_004_DM5PR06MB30988F4E41CAD0EBDA0E5F82F3DF0DM5PR06MB3098namp_--