Re: [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong  <eric.dong@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 6:27 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new
> EfiLocateFirstAcpiTable()
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get ACPI table
> using signature as input.
> 
> After evaluation, we found there are many duplicated code to find ACPI
> table by signature in different modules.
> 
> This patch updates PiSmmCpuDxeSmm to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
> 
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |  4 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c         | 84 ++----------------------
> --
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +-
>  3 files changed, 6 insertions(+), 85 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index a7fb7b0b1482..0fdc1b134ea3 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -4,7 +4,7 @@
>  # This SMM driver performs SMM initialization, deploy SMM Entry Vector,  #
> provides CPU specific services in SMM.
>  #
> -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2018, Intel Corporation. All rights
> +reserved.<BR>
>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>  #  # This
> program and the accompanying materials @@ -114,8 +114,6 @@ [Protocols]
> [Guids]
>    gEfiAcpiVariableGuid                     ## SOMETIMES_CONSUMES ## HOB # it is
> used for S3 boot.
>    gEfiGlobalVariableGuid                   ## SOMETIMES_PRODUCES ##
> Variable:L"SmmProfileData"
> -  gEfiAcpi20TableGuid                      ## SOMETIMES_CONSUMES ##
> SystemTable
> -  gEfiAcpi10TableGuid                      ## SOMETIMES_CONSUMES ##
> SystemTable
>    gEdkiiPiSmmMemoryAttributesTableGuid     ## CONSUMES ## SystemTable
>    gEfiMemoryAttributesTableGuid            ## CONSUMES ## SystemTable
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index a743cf64f907..91b8e7ddb991 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Enable SMM profile.
> 
> -Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> 
>  This program and the accompanying materials @@ -726,84 +726,6 @@
> InitPaging (  }
> 
>  /**
> -  To find FADT in ACPI tables.
> -
> -  @param AcpiTableGuid   The GUID used to find ACPI table in UEFI
> ConfigurationTable.
> -
> -  @return  FADT table pointer.
> -**/
> -EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  * -
> FindAcpiFadtTableByAcpiGuid (
> -  IN EFI_GUID  *AcpiTableGuid
> -  )
> -{
> -  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> -  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> -  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> -  UINTN                                         Index;
> -  UINT32                                        Data32;
> -  Rsdp  = NULL;
> -  Rsdt  = NULL;
> -  Fadt  = NULL;
> -  //
> -  // found ACPI table RSD_PTR from system table
> -  //
> -  for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
> -    if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid),
> AcpiTableGuid)) {
> -      //
> -      // A match was found.
> -      //
> -      Rsdp = gST->ConfigurationTable[Index].VendorTable;
> -      break;
> -    }
> -  }
> -
> -  if (Rsdp == NULL) {
> -    return NULL;
> -  }
> -
> -  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> -  if (Rsdt == NULL || Rsdt->Signature !=
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> -    return NULL;
> -  }
> -
> -  for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt-
> >Length; Index = Index + sizeof (UINT32)) {
> -
> -    Data32  = *(UINT32 *) ((UINT8 *) Rsdt + Index);
> -    Fadt    = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *)
> (UINTN) Data32;
> -    if (Fadt->Header.Signature ==
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> -      break;
> -    }
> -  }
> -
> -  if (Fadt == NULL || Fadt->Header.Signature !=
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> -    return NULL;
> -  }
> -
> -  return Fadt;
> -}
> -
> -/**
> -  To find FADT in ACPI tables.
> -
> -  @return  FADT table pointer.
> -**/
> -EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  * -FindAcpiFadtTable (
> -  VOID
> -  )
> -{
> -  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> -
> -  Fadt = FindAcpiFadtTableByAcpiGuid (&gEfiAcpi20TableGuid);
> -  if (Fadt != NULL) {
> -    return Fadt;
> -  }
> -
> -  return FindAcpiFadtTableByAcpiGuid (&gEfiAcpi10TableGuid); -}
> -
> -/**
>    To get system port address of the SMI Command Port in FADT table.
> 
>  **/
> @@ -814,7 +736,9 @@ GetSmiCommandPort (
>  {
>    EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> 
> -  Fadt = FindAcpiFadtTable ();
> +  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *)
> EfiLocateFirstAcpiTable (
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
> +                                                         );
>    ASSERT (Fadt != NULL);
> 
>    mSmiCommandPort = Fadt->SmiCmd;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> index bacb2f8ad32c..f93e90c143d2 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> @@ -1,7 +1,7 @@
>  /** @file
>  SMM profile internal header file.
> 
> -Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 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
> @@ -16,7 +16,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define _SMM_PROFILE_INTERNAL_H_
> 
>  #include <Guid/GlobalVariable.h>
> -#include <Guid/Acpi.h>
>  #include <Protocol/SmmReadyToLock.h>
>  #include <Library/UefiRuntimeServicesTableLib.h>
>  #include <Library/DxeServicesTableLib.h>
> --
> 2.7.0.windows.1