From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web12.1555.1616809513297242186 for ; Fri, 26 Mar 2021 18:45:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=slZAqHcf; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [10.124.238.202] (unknown [167.220.2.74]) by linux.microsoft.com (Postfix) with ESMTPSA id BB5AC2099AD4; Fri, 26 Mar 2021 18:45:12 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com BB5AC2099AD4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1616809512; bh=1GFkg45vZnJx1WxKi6RFbsdn1iEogd0fHqUwa7H8hlY=; h=Subject:From:To:Reply-To:References:Date:In-Reply-To:From; b=slZAqHcfCkSXX9JT8fH2wiisWuLniPu5Y8PI9Z4mryG8rnKmbwJO/d3eJYzGKka5M 738dZtgMkDuXgzwesLtU+IScvm5pYVFp2LsR9c6Tnc00F3/HxnD7emCiquw5adRutd 7g9icX62J5LRANLmF8xwBJrdVvNUj+sDp6ZSZJ3A= Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] MinPlatformPkg/Acpi/AcpiSmm: Add Standalone MM support From: "Michael Kubacki" To: devel@edk2.groups.io, Chasel Chiu , Nate DeSimone , Liming Gao , Eric Dong Reply-To: devel@edk2.groups.io, mikuback@linux.microsoft.com References: <166A6D94DDC3B65B.3944@groups.io> Message-ID: <8f967a59-c8de-506e-bf32-a1e0a4813c0a@linux.microsoft.com> Date: Fri, 26 Mar 2021 18:45:12 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <166A6D94DDC3B65B.3944@groups.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sending a review reminder since it's been a few weeks. Thanks, Michael On 3/8/2021 9:17 AM, Michael Kubacki wrote: > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3248 > > Adds a new module called AcpiStandaloneMm that serves the same role > as AcpiSmm but in a Standalone MM environment. > > This change follows a similar pattern to other changes that have > added Standalone MM support to a SMM module. The SMM INF name and > file path remain unaltered to allow backward compatibility and much > of the code is shared between the driver instances with unique entry > points for each respective module type. > > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Liming Gao > Cc: Eric Dong > Signed-off-by: Michael Kubacki > --- > > Notes: > V2 change: Add BZ link > > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/{AcpiSmm.c => AcpiMm.c} | 33 +++++++++---------- > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c | 34 ++++++++++++++++++++ > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c | 34 ++++++++++++++++++++ > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.h | 23 +++++++++++++ > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.h | 24 -------------- > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf | 21 ++++++------ > Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/{AcpiSmm.inf => AcpiStandaloneMm.inf} | 32 +++++++++--------- > Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 2 ++ > 8 files changed, 133 insertions(+), 70 deletions(-) > > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c > similarity index 81% > rename from Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c > rename to Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c > index 809f75d3c588..2cf559f3fe09 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.c > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.c > @@ -1,12 +1,20 @@ > /** @file > - Acpi Smm driver. > + Functions shared between driver instances. > > Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > -#include "AcpiSmm.h" > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "AcpiMm.h" > > /** > Enable SCI > @@ -53,20 +61,13 @@ DisableAcpiCallback ( > } > > /** > - Initializes the Acpi Smm Driver > - > - @param[in] ImageHandle - Pointer to the loaded image protocol for this driver > - @param[in] SystemTable - Pointer to the EFI System Table > - > - @retval Status - EFI_SUCCESS > - @retval Assert, otherwise. > + ACPI initialization logic shared between the Traditional MM and > + Standalone MM driver instances. > > **/ > -EFI_STATUS > -EFIAPI > -InitializeAcpiSmm ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +VOID > +InitializeAcpiMm ( > + VOID > ) > { > EFI_STATUS Status; > @@ -77,7 +78,7 @@ InitializeAcpiSmm ( > // > // Locate the ICH SMM SW dispatch protocol > // > - Status = gSmst->SmmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, (VOID**)&SwDispatch); > + Status = gMmst->MmLocateProtocol (&gEfiSmmSwDispatch2ProtocolGuid, NULL, (VOID**) &SwDispatch); > ASSERT_EFI_ERROR (Status); > > // > @@ -103,6 +104,4 @@ InitializeAcpiSmm ( > &SwHandle > ); > ASSERT_EFI_ERROR (Status); > - > - return EFI_SUCCESS; > } > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c > new file mode 100644 > index 000000000000..f378942fdc07 > --- /dev/null > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.c > @@ -0,0 +1,34 @@ > +/** @file > + Standalone MM driver for ACPI initialization. > + > +Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include "AcpiMm.h" > + > +/** > + The Standalone MM driver entry point. > + > + @param[in] ImageHandle - Pointer to the loaded image protocol for this driver > + @param[in] SystemTable - Pointer to the EFI MM System Table > + > + @retval Status - EFI_SUCCESS > + @retval Assert, otherwise. > + > +**/ > +EFI_STATUS > +EFIAPI > +AcpiStandaloneMmEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *MmSystemTable > + ) > +{ > + InitializeAcpiMm (); > + > + return EFI_SUCCESS; > +} > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c > new file mode 100644 > index 000000000000..9512926b9e2e > --- /dev/null > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiTraditionalMm.c > @@ -0,0 +1,34 @@ > +/** @file > + Traditional MM driver for ACPI initialization. > + > +Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include "AcpiMm.h" > + > +/** > + The Traditional MM driver entry point. > + > + @param[in] ImageHandle - Pointer to the loaded image protocol for this driver > + @param[in] SystemTable - Pointer to the EFI System Table > + > + @retval Status - EFI_SUCCESS > + @retval Assert, otherwise. > + > +**/ > +EFI_STATUS > +EFIAPI > +AcpiTraditionalMmEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + InitializeAcpiMm (); > + > + return EFI_SUCCESS; > +} > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.h b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.h > new file mode 100644 > index 000000000000..051474b0e833 > --- /dev/null > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiMm.h > @@ -0,0 +1,23 @@ > +/** @file > + Internal header file for the ACPI MM driver. > + > +Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef _ACPI_MM_H_ > +#define _ACPI_MM_H_ > + > +/** > + ACPI initialization logic shared between the Traditional MM and > + Standalone MM driver instances. > + > +**/ > +VOID > +InitializeAcpiMm ( > + VOID > + ); > + > +#endif > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.h b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.h > deleted file mode 100644 > index e34ffb1b755b..000000000000 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.h > +++ /dev/null > @@ -1,24 +0,0 @@ > -/** @file > - Header file for the Smm platform driver. > - > -Copyright (c) 2017, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#ifndef _ACPI_SMM_H_ > -#define _ACPI_SMM_H_ > - > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > -#include > - > -#endif > - > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > index fbaf46752563..651d4a293e9b 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > @@ -1,7 +1,8 @@ > ### @file > -# Component information file for ACPI SMM module. > +# Component information file for ACPI Traditional MM module. > # > # Copyright (c) 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) Microsoft Corporation.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -14,18 +15,15 @@ [Defines] > VERSION_STRING = 1.0 > MODULE_TYPE = DXE_SMM_DRIVER > PI_SPECIFICATION_VERSION = 1.20 > - ENTRY_POINT = InitializeAcpiSmm > + ENTRY_POINT = AcpiTraditionalMmEntryPoint > > [LibraryClasses] > - UefiDriverEntryPoint > - UefiBootServicesTableLib > + BoardAcpiEnableLib > DebugLib > - HobLib > - IoLib > + MmServicesTableLib > PcdLib > + UefiDriverEntryPoint > UefiLib > - SmmServicesTableLib > - BoardAcpiEnableLib > > [Packages] > MdePkg/MdePkg.dec > @@ -36,13 +34,12 @@ [Pcd] > gMinPlatformPkgTokenSpaceGuid.PcdAcpiDisableSwSmi ## CONSUMES > > [Sources] > - AcpiSmm.h > - AcpiSmm.c > + AcpiMm.h > + AcpiMm.c > + AcpiTraditionalMm.c > > [Protocols] > gEfiSmmSwDispatch2ProtocolGuid ## CONSUMES > > -[Guids] > - > [Depex] > gEfiSmmSwDispatch2ProtocolGuid > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > similarity index 50% > copy from Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > copy to Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > index fbaf46752563..f7d0861b512d 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > @@ -1,7 +1,8 @@ > ### @file > -# Component information file for ACPI SMM module. > +# Component information file for ACPI Standalone MM module. > # > # Copyright (c) 2017, Intel Corporation. All rights reserved.
> +# Copyright (c) Microsoft Corporation.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -9,40 +10,37 @@ > > [Defines] > INF_VERSION = 0x00010017 > - BASE_NAME = AcpiSmm > - FILE_GUID = DF9A9FFC-A075-4867-A0B2-5E7540BB023E > + BASE_NAME = AcpiStandaloneMm > + FILE_GUID = F113611F-DEE7-4137-8623-0168675E9F6D > VERSION_STRING = 1.0 > - MODULE_TYPE = DXE_SMM_DRIVER > - PI_SPECIFICATION_VERSION = 1.20 > - ENTRY_POINT = InitializeAcpiSmm > + MODULE_TYPE = MM_STANDALONE > + PI_SPECIFICATION_VERSION = 0x00010032 > + ENTRY_POINT = AcpiStandaloneMmEntryPoint > > [LibraryClasses] > - UefiDriverEntryPoint > - UefiBootServicesTableLib > + BoardAcpiEnableLib > DebugLib > - HobLib > - IoLib > + MmServicesTableLib > PcdLib > - UefiLib > - SmmServicesTableLib > - BoardAcpiEnableLib > + StandaloneMmDriverEntryPoint > > [Packages] > MdePkg/MdePkg.dec > MinPlatformPkg/MinPlatformPkg.dec > > +# Note: All PCDs consumed in the Standalone MM instance must be either FixedAtBuild > +# or PatchableInModule > [Pcd] > gMinPlatformPkgTokenSpaceGuid.PcdAcpiEnableSwSmi ## CONSUMES > gMinPlatformPkgTokenSpaceGuid.PcdAcpiDisableSwSmi ## CONSUMES > > [Sources] > - AcpiSmm.h > - AcpiSmm.c > + AcpiMm.h > + AcpiMm.c > + AcpiStandaloneMm.c > > [Protocols] > gEfiSmmSwDispatch2ProtocolGuid ## CONSUMES > > -[Guids] > - > [Depex] > gEfiSmmSwDispatch2ProtocolGuid > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > index 0460fd5a3206..998ee7909568 100644 > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc > @@ -117,6 +117,7 @@ [LibraryClasses.common.MM_STANDALONE] > DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf > + PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > SpiFlashCommonLib|MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf > StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf > > @@ -147,6 +148,7 @@ [Components] > > MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf > MinPlatformPkg/Acpi/AcpiSmm/AcpiSmm.inf > + MinPlatformPkg/Acpi/AcpiSmm/AcpiStandaloneMm.inf > MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateLib.inf > MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiEnableLibNull.inf > MinPlatformPkg/Acpi/Library/BoardAcpiLibNull/BoardAcpiTableLibNull.inf >