Re: [PATCH 1/2] EmulatorPkg: Add persistent memory in EmuThunkPpi

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, December 6, 2022 1:41 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 1/2] EmulatorPkg: Add persistent memory in EmuThunkPpi
> 
> The persistent memory is for PEIM to use, and won't lose during cold
> or warm reset. PcdPersistentMemorySize is only used by WinHost.c,
> other modules can check the persistent memory size using the field
> PersistentMemorySize.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  EmulatorPkg/EmulatorPkg.dec        |  3 ++-
>  EmulatorPkg/Include/Ppi/EmuThunk.h |  4 +++-
>  EmulatorPkg/Unix/Host/Host.c       | 13 +++++++++++--
>  EmulatorPkg/Unix/Host/Host.inf     |  3 ++-
>  EmulatorPkg/Win/Host/WinHost.c     | 11 ++++++++++-
>  EmulatorPkg/Win/Host/WinHost.inf   |  2 +-
>  6 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/EmulatorPkg/EmulatorPkg.dec b/EmulatorPkg/EmulatorPkg.dec
> index b9c70b63b3..fe81652b04 100644
> --- a/EmulatorPkg/EmulatorPkg.dec
> +++ b/EmulatorPkg/EmulatorPkg.dec
> @@ -2,7 +2,7 @@
>  #
>  # This is the Emu Emulation Environment Platform
>  #
> -# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2008 - 2022, Intel Corporation. All rights reserved.<BR>
>  # Portions copyright (c) 2011, Apple Inc. All rights reserved.
>  # (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
>  #
> @@ -86,6 +86,7 @@
>    # The user ID and password are fixed as below.
> 
> gEmulatorPkgTokenSpaceGuid.PcdRedfishServieUserId|"admin"|VOID*|0x0
> 0001022
> 
> gEmulatorPkgTokenSpaceGuid.PcdRedfishServiePassword|"pwd123456"|VO
> ID*|0x00001023
> +
> gEmulatorPkgTokenSpaceGuid.PcdPersistentMemorySize|0x4000000|UINT3
> 2|0x00001024
> 
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>    gEmulatorPkgTokenSpaceGuid.PcdEmuBootMode|1|UINT32|0x00001006
> diff --git a/EmulatorPkg/Include/Ppi/EmuThunk.h
> b/EmulatorPkg/Include/Ppi/EmuThunk.h
> index c0e2bc6b98..cf29cf824c 100644
> --- a/EmulatorPkg/Include/Ppi/EmuThunk.h
> +++ b/EmulatorPkg/Include/Ppi/EmuThunk.h
> @@ -2,7 +2,7 @@
>    Emulator Thunk to abstract OS services from pure EFI code
> 
>    Copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
> -
> +  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -107,6 +107,8 @@ typedef struct {
>    EMU_PEI_AUTOSCAN           MemoryAutoScan;
>    EMU_PEI_FD_INFORMATION     FirmwareDevices;
>    EMU_PEI_THUNK_INTERFACE    Thunk;
> +  UINTN                      PersistentMemorySize;
> +  UINT8                      PersistentMemory[0];
>  } EMU_THUNK_PPI;
> 
>  extern EFI_GUID  gEmuThunkPpiGuid;
> diff --git a/EmulatorPkg/Unix/Host/Host.c b/EmulatorPkg/Unix/Host/Host.c
> index 38c01c84af..8d0be5b54b 100644
> --- a/EmulatorPkg/Unix/Host/Host.c
> +++ b/EmulatorPkg/Unix/Host/Host.c
> @@ -1,6 +1,6 @@
>  /*++ @file
> 
> -Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -102,6 +102,7 @@ main (
>    CHAR16                *FirmwareVolumesStr;
>    UINTN                 *StackPointer;
>    FILE                  *GdbTempFile;
> +  EMU_THUNK_PPI         *SecEmuThunkPpi;
> 
>    //
>    // Xcode does not support sourcing gdb scripts directly, so the Xcode XML
> @@ -137,7 +138,15 @@ main (
>    //
>    // PPIs pased into PEI_CORE
>    //
> -  AddThunkPpi (EFI_PEI_PPI_DESCRIPTOR_PPI, &gEmuThunkPpiGuid,
> &mSecEmuThunkPpi);
> +  SecEmuThunkPpi = AllocateZeroPool (sizeof (EMU_THUNK_PPI) +
> FixedPcdGet32 (PcdPersistentMemorySize));
> +  if (SecEmuThunkPpi == NULL) {
> +    printf ("ERROR : Can not allocate memory for SecEmuThunkPpi.
> Exiting.\n");
> +    exit (1);
> +  }
> +
> +  CopyMem (SecEmuThunkPpi, &mSecEmuThunkPpi, sizeof
> (EMU_THUNK_PPI));
> +  SecEmuThunkPpi->PersistentMemorySize = FixedPcdGet32
> (PcdPersistentMemorySize);
> +  AddThunkPpi (EFI_PEI_PPI_DESCRIPTOR_PPI, &gEmuThunkPpiGuid,
> SecEmuThunkPpi);
> 
>    SecInitThunkProtocol ();
> 
> diff --git a/EmulatorPkg/Unix/Host/Host.inf
> b/EmulatorPkg/Unix/Host/Host.inf
> index c479d2b7d0..f5ebbed683 100644
> --- a/EmulatorPkg/Unix/Host/Host.inf
> +++ b/EmulatorPkg/Unix/Host/Host.inf
> @@ -2,7 +2,7 @@
>  # Entry Point of Emu Emulator
>  #
>  # Main executable file of Unix Emulator that loads PEI core after initialization
> finished.
> -# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2008 - 2022, Intel Corporation. All rights reserved.<BR>
>  # Portions copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -106,6 +106,7 @@
>    gEmulatorPkgTokenSpaceGuid.PcdEmuFlashNvStorageFtwSpareBase
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>    gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage
> +  gEmulatorPkgTokenSpaceGuid.PcdPersistentMemorySize
> 
>  [FeaturePcd]
>    gEmulatorPkgTokenSpaceGuid.PcdEmulatorLazyLoadSymbols
> diff --git a/EmulatorPkg/Win/Host/WinHost.c
> b/EmulatorPkg/Win/Host/WinHost.c
> index 096292f95a..3b2fde297d 100644
> --- a/EmulatorPkg/Win/Host/WinHost.c
> +++ b/EmulatorPkg/Win/Host/WinHost.c
> @@ -436,6 +436,7 @@ Returns:
>    UINTN                SystemAffinityMask;
>    INT32                LowBit;
>    UINTN                ResetJumpCode;
> +  EMU_THUNK_PPI        *SecEmuThunkPpi;
> 
>    //
>    // Enable the privilege so that RTC driver can successfully run SetTime()
> @@ -477,7 +478,15 @@ Returns:
>    //
>    // PPIs pased into PEI_CORE
>    //
> -  AddThunkPpi (EFI_PEI_PPI_DESCRIPTOR_PPI, &gEmuThunkPpiGuid,
> &mSecEmuThunkPpi);
> +  SecEmuThunkPpi = AllocateZeroPool (sizeof (EMU_THUNK_PPI) +
> FixedPcdGet32 (PcdPersistentMemorySize));
> +  if (SecEmuThunkPpi == NULL) {
> +    SecPrint ("ERROR : Can not allocate memory for SecEmuThunkPpi.
> Exiting.\n");
> +    exit (1);
> +  }
> +
> +  CopyMem (SecEmuThunkPpi, &mSecEmuThunkPpi, sizeof
> (EMU_THUNK_PPI));
> +  SecEmuThunkPpi->PersistentMemorySize = FixedPcdGet32
> (PcdPersistentMemorySize);
> +  AddThunkPpi (EFI_PEI_PPI_DESCRIPTOR_PPI, &gEmuThunkPpiGuid,
> SecEmuThunkPpi);
>    AddThunkPpi (EFI_PEI_PPI_DESCRIPTOR_PPI, &gEfiPeiReset2PpiGuid,
> &mEmuReset2Ppi);
> 
>    //
> diff --git a/EmulatorPkg/Win/Host/WinHost.inf
> b/EmulatorPkg/Win/Host/WinHost.inf
> index b61901fae2..4dac6e033e 100644
> --- a/EmulatorPkg/Win/Host/WinHost.inf
> +++ b/EmulatorPkg/Win/Host/WinHost.inf
> @@ -74,7 +74,7 @@
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
> -
> +  gEmulatorPkgTokenSpaceGuid.PcdPersistentMemorySize
>    gEmulatorPkgTokenSpaceGuid.PcdEmuBootMode
>    gEmulatorPkgTokenSpaceGuid.PcdEmuFirmwareVolume
>    gEmulatorPkgTokenSpaceGuid.PcdEmuMemorySize
> --
> 2.31.1.windows.1