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

[Laszlo Ersek <lersek@redhat.com>]

On 08/31/18 13:29, Star Zeng wrote:
> 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
> EfiFindAcpiTableBySignature() 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..df643b72c517 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 *) EfiFindAcpiTableBySignature (
> +                                                         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>
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(I realize the new API might change still.)

Thanks
Laszlo