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

2018-09-02 Thread Zeng, Star
Thanks Laszlo. :)

Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Saturday, September 1, 2018 4:34 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Dong, Eric ; Younas 
khan ; Gao, Liming ; Yao, 
Jiewen ; Kinney, Michael D 
Subject: Re: [edk2] [PATCH 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new 
EfiFindAcpiTableBySignature()

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 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  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.
> +# Copyright (c) 2009 - 2018, Intel Corporation. All rights 
> +reserved.
>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.  #  
> # 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.
> +Copyright (c) 2012 - 2018, Intel Corporation. All rights 
> +reserved.
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
>  
>  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;
> -  UINT32Data32;
> -  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 

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

2018-08-31 Thread Laszlo Ersek
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 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  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.
> +# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.
>  #
>  # 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.
> +Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.
>  
>  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;
> -  UINT32Data32;
> -  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 ();
> -  if (Fadt != NULL) {
> -return Fadt;
> -  }
> -
> -  return FindAcpiFadtTableByAcpiGuid ();
> -}
> -
> -/**
>   

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

2018-08-31 Thread Star Zeng
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 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 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.
+# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.
 #
 # 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.
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 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;
-  UINT32Data32;
-  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 ();
-  if (Fadt != NULL) {
-return Fadt;
-  }
-
-  return FindAcpiFadtTableByAcpiGuid ();
-}
-
-/**
   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 (
+