Re: [edk2-platforms][PATCH v2 1/1] MinPlatformPkg/Acpi/AcpiSmm: Add Standalone MM support

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Pushed: https://github.com/tianocore/edk2-platforms/commit/3250973

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Monday, March 8, 2021 9:18 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 v2 1/1] MinPlatformPkg/Acpi/AcpiSmm:
> Add Standalone MM support
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> 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 <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>
> ---
> 
> 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.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> -#include "AcpiSmm.h"
> +#include <PiMm.h>
> +#include <Library/BoardAcpiEnableLib.h> #include <Library/DebugLib.h>
> +#include <Library/MmServicesTableLib.h> #include <Library/PcdLib.h>
> +#include <Protocol/SmmSwDispatch2.h>
> +
> +#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.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiMm.h>
> +
> +#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.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiMm.h>
> +
> +#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.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> +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.<BR>
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef _ACPI_SMM_H_
> -#define _ACPI_SMM_H_
> -
> -#include <PiSmm.h>
> -#include <Library/DebugLib.h>
> -#include <Library/HobLib.h>
> -#include <Library/IoLib.h>
> -#include <Library/PcdLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> -#include <Library/UefiLib.h>
> -#include <Library/SmmServicesTableLib.h> -#include
> <Library/BoardAcpiEnableLib.h> -#include <Protocol/SmmSwDispatch2.h>
> -
> -#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.<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
>  #
>  # 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.<BR>
> +# Copyright (c) Microsoft Corporation.<BR>
>  #
>  # 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/StandaloneMmMemoryAll
> ocationLib/StandaloneMmMemoryAllocationLib.inf
> 
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stan
> daloneMmServicesTableLib.inf
> +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> 
> SpiFlashCommonLib|MinPlatformPkg/Flash/Library/SpiFlashCommonLibNull/
> SpiFlashCommonLibNull.inf
> 
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntry
> Point/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
> --
> 2.28.0.windows.1