Re: [patch 2/3] MdeModulePkg/BdsDxe: Remove useless Pref Code

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 1/24/2018 4:01 PM, Dandan Bi wrote:
> Our new performance infrastructure can support to dump performance
> date form ACPI table in OS. So we can remove the old pref code to
> write performance data to OS.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>   MdeModulePkg/Universal/BdsDxe/Bds.h      |  3 +-
>   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  3 +-
>   MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 57 +-------------------------------
>   3 files changed, 3 insertions(+), 60 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/BdsDxe/Bds.h b/MdeModulePkg/Universal/BdsDxe/Bds.h
> index 5658e61..63d961f 100644
> --- a/MdeModulePkg/Universal/BdsDxe/Bds.h
> +++ b/MdeModulePkg/Universal/BdsDxe/Bds.h
> @@ -1,9 +1,9 @@
>   /** @file
>     Head file for BDS Architectural Protocol implementation
>   
> -Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD License
>   which accompanies this distribution.  The full text of the license may be found at
>   http://opensource.org/licenses/bsd-license.php
>   
> @@ -16,11 +16,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>   #define _BDS_MODULE_H_
>   
>   #include <Uefi.h>
>   #include <Guid/GlobalVariable.h>
>   #include <Guid/ConnectConInEvent.h>
> -#include <Guid/Performance.h>
>   #include <Guid/StatusCodeDataTypeVariable.h>
>   #include <Guid/EventGroup.h>
>   
>   #include <Protocol/Bds.h>
>   #include <Protocol/LoadedImage.h>
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> index a00b442..b8c5aa9 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> @@ -3,11 +3,11 @@
>   #
>   #  When DxeCore dispatching all DXE driver, this module will produce architecture protocol
>   #  gEfiBdsArchProtocolGuid. After DxeCore finish dispatching, DxeCore will invoke Entry
>   #  interface of protocol gEfiBdsArchProtocolGuid, then BDS phase is entered.
>   #
> -#  Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
>   #  This program and the accompanying materials
>   #  are licensed and made available under the terms and conditions of the BSD License
>   #  which accompanies this distribution.  The full text of the license may be found at
>   #  http://opensource.org/licenses/bsd-license.php
>   #
> @@ -76,11 +76,10 @@
>                                                   ## SOMETIMES_CONSUMES ## Variable:L"ConIn" (The device path of console in device)
>                                                   ## SOMETIMES_CONSUMES ## Variable:L"ConOut" (The device path of console out device)
>                                                   ## SOMETIMES_CONSUMES ## Variable:L"ErrOut" (The device path of error out device)
>     gConnectConInEventGuid                        ## SOMETIMES_CONSUMES ## Event
>     gEdkiiStatusCodeDataTypeVariableGuid          ## SOMETIMES_CONSUMES ## GUID
> -  gPerformanceProtocolGuid                      ## SOMETIMES_PRODUCES ## Variable:L"PerfDataMemAddr" (The ACPI address of performance data)
>     gEfiEventReadyToBootGuid                      ## CONSUMES           ## Event
>   
>   [Protocols]
>     gEfiBdsArchProtocolGuid                       ## PRODUCES
>     gEfiSimpleTextInputExProtocolGuid             ## CONSUMES
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 2b24755..3191a98 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -3,11 +3,11 @@
>     When this module was dispatched by DxeCore, gEfiBdsArchProtocolGuid will be installed
>     which contains interface of BdsEntry.
>     After DxeCore finish DXE phase, gEfiBdsArchProtocolGuid->BdsEntry will be invoked
>     to enter BDS phase.
>   
> -Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
>   (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD License
>   which accompanies this distribution.  The full text of the license may be found at
> @@ -633,61 +633,10 @@ BdsFormalizeEfiGlobalVariable (
>     //
>     BdsFormalizeOSIndicationVariable ();
>   }
>   
>   /**
> -
> -  Allocate a block of memory that will contain performance data to OS.
> -
> -**/
> -VOID
> -BdsAllocateMemoryForPerformanceData (
> -  VOID
> -  )
> -{
> -  EFI_STATUS                    Status;
> -  EFI_PHYSICAL_ADDRESS          AcpiLowMemoryBase;
> -  EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock;
> -
> -  AcpiLowMemoryBase = 0x0FFFFFFFFULL;
> -
> -  //
> -  // Allocate a block of memory that will contain performance data to OS.
> -  //
> -  Status = gBS->AllocatePages (
> -                  AllocateMaxAddress,
> -                  EfiReservedMemoryType,
> -                  EFI_SIZE_TO_PAGES (PERF_DATA_MAX_LENGTH),
> -                  &AcpiLowMemoryBase
> -                  );
> -  if (!EFI_ERROR (Status)) {
> -    //
> -    // Save the pointer to variable for use in S3 resume.
> -    //
> -    Status = BdsDxeSetVariableAndReportStatusCodeOnError (
> -               L"PerfDataMemAddr",
> -               &gPerformanceProtocolGuid,
> -               EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> -               sizeof (EFI_PHYSICAL_ADDRESS),
> -               &AcpiLowMemoryBase
> -               );
> -    if (EFI_ERROR (Status)) {
> -      DEBUG ((EFI_D_ERROR, "[Bds] PerfDataMemAddr (%08x) cannot be saved to NV storage.\n", AcpiLowMemoryBase));
> -    }
> -    //
> -    // Mark L"PerfDataMemAddr" variable to read-only if the Variable Lock protocol exists
> -    // Still lock it even the variable cannot be saved to prevent it's set by 3rd party code.
> -    //
> -    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **) &VariableLock);
> -    if (!EFI_ERROR (Status)) {
> -      Status = VariableLock->RequestToLock (VariableLock, L"PerfDataMemAddr", &gPerformanceProtocolGuid);
> -      ASSERT_EFI_ERROR (Status);
> -    }
> -  }
> -}
> -
> -/**
>     Enter an infinite loop of calling the Boot Manager Menu.
>   
>     This is a last resort alternative to BdsEntry() giving up for good. This
>     function never returns.
>   
> @@ -778,14 +727,10 @@ BdsEntry (
>     //
>     PERF_END (NULL, "DXE", NULL, 0);
>     PERF_START (NULL, "BDS", NULL, 0);
>     DEBUG ((EFI_D_INFO, "[Bds] Entry...\n"));
>   
> -  PERF_CODE (
> -    BdsAllocateMemoryForPerformanceData ();
> -  );
> -
>     //
>     // Fill in FirmwareVendor and FirmwareRevision from PCDs
>     //
>     FirmwareVendor = (CHAR16 *) PcdGetPtr (PcdFirmwareVendor);
>     gST->FirmwareVendor = AllocateRuntimeCopyPool (StrSize (FirmwareVendor), FirmwareVendor);
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray