[PATCH 0/6] Add new EfiFindAcpiTableBySignature() API

[PATCH 0/6] Add new EfiFindAcpiTableBySignature() API

[Star Zeng]

The code for this patch series is also at
git@github.com:lzeng14/edk2.git branch FindAcpiTableBySignature_UefiLib

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 adds new EfiFindAcpiTableBySignature() API in UefiLib
for the request and removing 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: Jian J Wang <jian.j.wang@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Star Zeng (6):
  MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
  IntelSiliconPkg IntelVTdDxe: Use new EfiFindAcpiTableBySignature()
  MdeModulePkg S3SaveStateDxe: Use new EfiFindAcpiTableBySignature()
  PcAtChipsetPkg PcRtc: Use new EfiFindAcpiTableBySignature()
  ShellPkg DpDynamicCommand: Use new EfiFindAcpiTableBySignature()
  UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiFindAcpiTableBySignature()

 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        | 136 +------------
 .../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c        | 208 +------------------
 .../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf         |   3 +-
 MdePkg/Include/Library/UefiLib.h                   |  17 ++
 MdePkg/Library/UefiLib/Acpi.c                      | 226 +++++++++++++++++++++
 MdePkg/Library/UefiLib/UefiLib.inf                 |   3 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c |  80 +-------
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c      | 136 +------------
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h      |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni    |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf |   2 -
 .../DpDynamicCommand/DpDynamicCommand.inf          |   2 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf       |   4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c             |  84 +-------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h     |   3 +-
 15 files changed, 266 insertions(+), 640 deletions(-)
 create mode 100644 MdePkg/Library/UefiLib/Acpi.c

-- 
2.7.0.windows.1

[PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[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 adds new EfiFindAcpiTableBySignature() API in UefiLib
for the request and also the following patch to 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>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdePkg/Include/Library/UefiLib.h   |  17 +++
 MdePkg/Library/UefiLib/Acpi.c      | 226 +++++++++++++++++++++++++++++++++++++
 MdePkg/Library/UefiLib/UefiLib.inf |   3 +
 3 files changed, 246 insertions(+)
 create mode 100644 MdePkg/Library/UefiLib/Acpi.c

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index f80067f11103..8dd25f324fd2 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
   IN     UINT64                    OpenMode,
   IN     UINT64                    Attributes
   );
+
+/**
+  This function finds ACPI table by signature.
+  It will find the table in gEfiAcpi20TableGuid system configuration table first,
+  and then gEfiAcpi10TableGuid system configuration table.
+
+  @param Signature  ACPI table signature.
+
+  @return ACPI table or NULL if not found.
+
+**/
+VOID *
+EFIAPI
+EfiFindAcpiTableBySignature (
+  IN UINT32     Signature
+  );
+
 #endif
diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
new file mode 100644
index 000000000000..5cb93966b59f
--- /dev/null
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -0,0 +1,226 @@
+/** @file
+  This module provides help function for finding ACPI table.
+
+  Copyright (c) 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.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "UefiLibInternal.h"
+#include <IndustryStandard/Acpi.h>
+#include <Guid/Acpi.h>
+
+/**
+  This function scans ACPI table in RSDT.
+
+  @param Rsdt       ACPI RSDT
+  @param Signature  ACPI table signature
+
+  @return ACPI table or NULL if not found.
+
+**/
+VOID *
+ScanTableInRSDT (
+  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
+  IN UINT32                         Signature
+  )
+{
+  UINTN                              Index;
+  UINT32                             EntryCount;
+  UINT32                             *EntryPtr;
+  EFI_ACPI_DESCRIPTION_HEADER        *Table;
+
+  if (Rsdt == NULL) {
+    return NULL;
+  }
+
+  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
+
+  EntryPtr = (UINT32 *)(Rsdt + 1);
+  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
+    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
+    if (Table->Signature == Signature) {
+      return Table;
+    }
+  }
+
+  return NULL;
+}
+
+/**
+  This function scans ACPI table in XSDT.
+
+  @param Xsdt       ACPI XSDT
+  @param Signature  ACPI table signature
+
+  @return ACPI table or NULL if not found.
+
+**/
+VOID *
+ScanTableInXSDT (
+  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
+  IN UINT32                         Signature
+  )
+{
+  UINTN                          Index;
+  UINT32                         EntryCount;
+  UINT64                         EntryPtr;
+  UINTN                          BasePtr;
+  EFI_ACPI_DESCRIPTION_HEADER    *Table;
+
+  if (Xsdt == NULL) {
+    return NULL;
+  }
+
+  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
+
+  BasePtr = (UINTN)(Xsdt + 1);
+  for (Index = 0; Index < EntryCount; Index ++) {
+    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
+    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
+    if (Table->Signature == Signature) {
+      return Table;
+    }
+  }
+
+  return NULL;
+}
+
+/**
+  To find Facs in FADT.
+
+  @param Fadt   FADT table pointer
+
+  @return Facs table pointer or NULL if not found.
+
+**/
+EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
+FindAcpiFacsFromFadt (
+  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
+  )
+{
+  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
+  UINT64                                        Data64;
+
+  if (Fadt == NULL) {
+    return NULL;
+  }
+
+  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
+    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
+  } else {
+    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
+    if (Data64 != 0) {
+      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Data64;
+    } else {
+      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
+    }
+  }
+  return Facs;
+}
+
+/**
+  To find ACPI table in ACPI ConfigurationTable.
+
+  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
+  @param Signature      ACPI table signature.
+
+  @return ACPI table or NULL if not found.
+
+**/
+VOID  *
+FindAcpiTableInAcpiConfigurationTable (
+  IN EFI_GUID   *AcpiGuid,
+  IN UINT32     Signature
+
+  )
+{
+  EFI_STATUS                                    Status;
+  VOID                                          *Table;
+  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
+  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
+  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
+
+  Rsdp = NULL;
+  //
+  // Find ACPI ConfigurationTable (RSD_PTR)
+  //
+  Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
+  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+    return NULL;
+  }
+
+  Table = NULL;
+
+  //
+  // Search XSDT
+  //
+  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
+    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
+    if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+      //
+      // It is to find FACS ACPI table,
+      // need find FADT first.
+      //
+      Fadt = ScanTableInXSDT (Xsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
+      Table = FindAcpiFacsFromFadt (Fadt);
+    } else {
+      Table = ScanTableInXSDT (Xsdt, Signature);
+    }
+  }
+
+  if (Table != NULL) {
+    return Table;
+  }
+
+  //
+  // Search RSDT
+  //
+  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
+  if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+    //
+    // It is to find FACS ACPI table,
+    // need find FADT first.
+    //
+    Fadt = ScanTableInRSDT (Rsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
+    Table = FindAcpiFacsFromFadt (Fadt);
+  } else {
+    Table = ScanTableInRSDT (Rsdt, Signature);
+  }
+
+  return Table;
+}
+
+/**
+  This function finds ACPI table by signature.
+  It will find the table in gEfiAcpi20TableGuid system configuration table first,
+  and then gEfiAcpi10TableGuid system configuration table.
+
+  @param Signature  ACPI table signature.
+
+  @return ACPI table or NULL if not found.
+
+**/
+VOID *
+EFIAPI
+EfiFindAcpiTableBySignature (
+  IN UINT32     Signature
+  )
+{
+  VOID          *Table;
+
+  Table = FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi20TableGuid, Signature);
+  if (Table != NULL) {
+    return Table;
+  }
+
+  return FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi10TableGuid, Signature);
+}
+
diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
index a6c739ef3d6d..aea20fe67153 100644
--- a/MdePkg/Library/UefiLib/UefiLib.inf
+++ b/MdePkg/Library/UefiLib/UefiLib.inf
@@ -41,6 +41,7 @@ [Sources]
   Console.c
   UefiLib.c
   UefiLibInternal.h
+  Acpi.c
 
 
 [Packages]
@@ -62,6 +63,8 @@ [Guids]
   gEfiEventReadyToBootGuid                      ## SOMETIMES_CONSUMES  ## Event
   gEfiEventLegacyBootGuid                       ## SOMETIMES_CONSUMES  ## Event
   gEfiGlobalVariableGuid                        ## SOMETIMES_CONSUMES  ## Variable
+  gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
+  gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
 
 [Protocols]
   gEfiDriverBindingProtocolGuid                   ## SOMETIMES_PRODUCES
-- 
2.7.0.windows.1

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Yao, Jiewen]

Good enhancement.

I have 2 additional thought:

1) How to handle DSDT?
We have special code to handle FACS, but no DSDT.

2) How to handle SSDT or other multiple ACPI tables?
We may have multiple SSDT. Usually, it is identified as OEMID.
Do we want to provide similar function for them?

Anyway, just *additional* thought. :-)
Current implementation is good enough for check in.

Reviewed-by: Jiewen.yao@intel.com

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
> Zeng
> Sent: Friday, August 31, 2018 7:29 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
> for the request and also the following patch to 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>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdePkg/Include/Library/UefiLib.h   |  17 +++
>  MdePkg/Library/UefiLib/Acpi.c      | 226
> +++++++++++++++++++++++++++++++++++++
>  MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>  3 files changed, 246 insertions(+)
>  create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index f80067f11103..8dd25f324fd2 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
>    IN     UINT64                    OpenMode,
>    IN     UINT64                    Attributes
>    );
> +
> +/**
> +  This function finds ACPI table by signature.
> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
> +  and then gEfiAcpi10TableGuid system configuration table.
> +
> +  @param Signature  ACPI table signature.
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +VOID *
> +EFIAPI
> +EfiFindAcpiTableBySignature (
> +  IN UINT32     Signature
> +  );
> +
>  #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> new file mode 100644
> index 000000000000..5cb93966b59f
> --- /dev/null
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -0,0 +1,226 @@
> +/** @file
> +  This module provides help function for finding ACPI table.
> +
> +  Copyright (c) 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.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "UefiLibInternal.h"
> +#include <IndustryStandard/Acpi.h>
> +#include <Guid/Acpi.h>
> +
> +/**
> +  This function scans ACPI table in RSDT.
> +
> +  @param Rsdt       ACPI RSDT
> +  @param Signature  ACPI table signature
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +VOID *
> +ScanTableInRSDT (
> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> +  IN UINT32                         Signature
> +  )
> +{
> +  UINTN                              Index;
> +  UINT32                             EntryCount;
> +  UINT32                             *EntryPtr;
> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> +
> +  if (Rsdt == NULL) {
> +    return NULL;
> +  }
> +
> +  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> sizeof(UINT32);
> +
> +  EntryPtr = (UINT32 *)(Rsdt + 1);
> +  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> +    if (Table->Signature == Signature) {
> +      return Table;
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  This function scans ACPI table in XSDT.
> +
> +  @param Xsdt       ACPI XSDT
> +  @param Signature  ACPI table signature
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +VOID *
> +ScanTableInXSDT (
> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> +  IN UINT32                         Signature
> +  )
> +{
> +  UINTN                          Index;
> +  UINT32                         EntryCount;
> +  UINT64                         EntryPtr;
> +  UINTN                          BasePtr;
> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> +
> +  if (Xsdt == NULL) {
> +    return NULL;
> +  }
> +
> +  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> sizeof(UINT64);
> +
> +  BasePtr = (UINTN)(Xsdt + 1);
> +  for (Index = 0; Index < EntryCount; Index ++) {
> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
> sizeof(UINT64));
> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> +    if (Table->Signature == Signature) {
> +      return Table;
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  To find Facs in FADT.
> +
> +  @param Fadt   FADT table pointer
> +
> +  @return Facs table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> +FindAcpiFacsFromFadt (
> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> +  )
> +{
> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> +  UINT64                                        Data64;
> +
> +  if (Fadt == NULL) {
> +    return NULL;
> +  }
> +
> +  if (Fadt->Header.Revision <
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Fadt->FirmwareCtrl;
> +  } else {
> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> +    if (Data64 != 0) {
> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Data64;
> +    } else {
> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Fadt->FirmwareCtrl;
> +    }
> +  }
> +  return Facs;
> +}
> +
> +/**
> +  To find ACPI table in ACPI ConfigurationTable.
> +
> +  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
> +  @param Signature      ACPI table signature.
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +VOID  *
> +FindAcpiTableInAcpiConfigurationTable (
> +  IN EFI_GUID   *AcpiGuid,
> +  IN UINT32     Signature
> +
> +  )
> +{
> +  EFI_STATUS                                    Status;
> +  VOID                                          *Table;
> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> +
> +  Rsdp = NULL;
> +  //
> +  // Find ACPI ConfigurationTable (RSD_PTR)
> +  //
> +  Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
> +  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> +    return NULL;
> +  }
> +
> +  Table = NULL;
> +
> +  //
> +  // Search XSDT
> +  //
> +  if (Rsdp->Revision >=
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
> +    if (Signature ==
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +      //
> +      // It is to find FACS ACPI table,
> +      // need find FADT first.
> +      //
> +      Fadt = ScanTableInXSDT (Xsdt,
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> +      Table = FindAcpiFacsFromFadt (Fadt);
> +    } else {
> +      Table = ScanTableInXSDT (Xsdt, Signature);
> +    }
> +  }
> +
> +  if (Table != NULL) {
> +    return Table;
> +  }
> +
> +  //
> +  // Search RSDT
> +  //
> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> +  if (Signature ==
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +    //
> +    // It is to find FACS ACPI table,
> +    // need find FADT first.
> +    //
> +    Fadt = ScanTableInRSDT (Rsdt,
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> +    Table = FindAcpiFacsFromFadt (Fadt);
> +  } else {
> +    Table = ScanTableInRSDT (Rsdt, Signature);
> +  }
> +
> +  return Table;
> +}
> +
> +/**
> +  This function finds ACPI table by signature.
> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
> +  and then gEfiAcpi10TableGuid system configuration table.
> +
> +  @param Signature  ACPI table signature.
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +VOID *
> +EFIAPI
> +EfiFindAcpiTableBySignature (
> +  IN UINT32     Signature
> +  )
> +{
> +  VOID          *Table;
> +
> +  Table = FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi20TableGuid,
> Signature);
> +  if (Table != NULL) {
> +    return Table;
> +  }
> +
> +  return FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi10TableGuid,
> Signature);
> +}
> +
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> b/MdePkg/Library/UefiLib/UefiLib.inf
> index a6c739ef3d6d..aea20fe67153 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -41,6 +41,7 @@ [Sources]
>    Console.c
>    UefiLib.c
>    UefiLibInternal.h
> +  Acpi.c
> 
> 
>  [Packages]
> @@ -62,6 +63,8 @@ [Guids]
>    gEfiEventReadyToBootGuid                      ##
> SOMETIMES_CONSUMES  ## Event
>    gEfiEventLegacyBootGuid                       ##
> SOMETIMES_CONSUMES  ## Event
>    gEfiGlobalVariableGuid                        ##
> SOMETIMES_CONSUMES  ## Variable
> +  gEfiAcpi20TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> +  gEfiAcpi10TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> 
>  [Protocols]
>    gEfiDriverBindingProtocolGuid                   ##
> SOMETIMES_PRODUCES
> --
> 2.7.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Ni, Ruiyu]

I think LocateNextAcpiTable() is more proper to handle the multiple tables with same signature. It will carry three parameters, one is the table header stored in configuration table, one is the signature, another is the previous located table. Can we return a common table header other than void*?

Is there better place other than UefiLib?
Do we need to add a new library class like AcpiLib?

发自我的 iPhone

> 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com> 写道：
> 
> Good enhancement.
> 
> I have 2 additional thought:
> 
> 1) How to handle DSDT?
> We have special code to handle FACS, but no DSDT.
> 
> 2) How to handle SSDT or other multiple ACPI tables?
> We may have multiple SSDT. Usually, it is identified as OEMID.
> Do we want to provide similar function for them?
> 
> Anyway, just *additional* thought. :-)
> Current implementation is good enough for check in.
> 
> Reviewed-by: Jiewen.yao@intel.com
> 
> Thank you
> Yao Jiewen
> 
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
>> Zeng
>> Sent: Friday, August 31, 2018 7:29 PM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
>> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
>> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
>> EfiFindAcpiTableBySignature() API
>> 
>> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
>> for the request and also the following patch to 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>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>> MdePkg/Include/Library/UefiLib.h   |  17 +++
>> MdePkg/Library/UefiLib/Acpi.c      | 226
>> +++++++++++++++++++++++++++++++++++++
>> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>> 3 files changed, 246 insertions(+)
>> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
>> 
>> diff --git a/MdePkg/Include/Library/UefiLib.h
>> b/MdePkg/Include/Library/UefiLib.h
>> index f80067f11103..8dd25f324fd2 100644
>> --- a/MdePkg/Include/Library/UefiLib.h
>> +++ b/MdePkg/Include/Library/UefiLib.h
>> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
>>   IN     UINT64                    OpenMode,
>>   IN     UINT64                    Attributes
>>   );
>> +
>> +/**
>> +  This function finds ACPI table by signature.
>> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
>> +  and then gEfiAcpi10TableGuid system configuration table.
>> +
>> +  @param Signature  ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +EfiFindAcpiTableBySignature (
>> +  IN UINT32     Signature
>> +  );
>> +
>> #endif
>> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
>> new file mode 100644
>> index 000000000000..5cb93966b59f
>> --- /dev/null
>> +++ b/MdePkg/Library/UefiLib/Acpi.c
>> @@ -0,0 +1,226 @@
>> +/** @file
>> +  This module provides help function for finding ACPI table.
>> +
>> +  Copyright (c) 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.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include "UefiLibInternal.h"
>> +#include <IndustryStandard/Acpi.h>
>> +#include <Guid/Acpi.h>
>> +
>> +/**
>> +  This function scans ACPI table in RSDT.
>> +
>> +  @param Rsdt       ACPI RSDT
>> +  @param Signature  ACPI table signature
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +ScanTableInRSDT (
>> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
>> +  IN UINT32                         Signature
>> +  )
>> +{
>> +  UINTN                              Index;
>> +  UINT32                             EntryCount;
>> +  UINT32                             *EntryPtr;
>> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
>> +
>> +  if (Rsdt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>> sizeof(UINT32);
>> +
>> +  EntryPtr = (UINT32 *)(Rsdt + 1);
>> +  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
>> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
>> +    if (Table->Signature == Signature) {
>> +      return Table;
>> +    }
>> +  }
>> +
>> +  return NULL;
>> +}
>> +
>> +/**
>> +  This function scans ACPI table in XSDT.
>> +
>> +  @param Xsdt       ACPI XSDT
>> +  @param Signature  ACPI table signature
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +ScanTableInXSDT (
>> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
>> +  IN UINT32                         Signature
>> +  )
>> +{
>> +  UINTN                          Index;
>> +  UINT32                         EntryCount;
>> +  UINT64                         EntryPtr;
>> +  UINTN                          BasePtr;
>> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
>> +
>> +  if (Xsdt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>> sizeof(UINT64);
>> +
>> +  BasePtr = (UINTN)(Xsdt + 1);
>> +  for (Index = 0; Index < EntryCount; Index ++) {
>> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
>> sizeof(UINT64));
>> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
>> +    if (Table->Signature == Signature) {
>> +      return Table;
>> +    }
>> +  }
>> +
>> +  return NULL;
>> +}
>> +
>> +/**
>> +  To find Facs in FADT.
>> +
>> +  @param Fadt   FADT table pointer
>> +
>> +  @return Facs table pointer or NULL if not found.
>> +
>> +**/
>> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
>> +FindAcpiFacsFromFadt (
>> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
>> +  )
>> +{
>> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
>> +  UINT64                                        Data64;
>> +
>> +  if (Fadt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  if (Fadt->Header.Revision <
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
>> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Fadt->FirmwareCtrl;
>> +  } else {
>> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
>> +    if (Data64 != 0) {
>> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Data64;
>> +    } else {
>> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Fadt->FirmwareCtrl;
>> +    }
>> +  }
>> +  return Facs;
>> +}
>> +
>> +/**
>> +  To find ACPI table in ACPI ConfigurationTable.
>> +
>> +  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
>> +  @param Signature      ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID  *
>> +FindAcpiTableInAcpiConfigurationTable (
>> +  IN EFI_GUID   *AcpiGuid,
>> +  IN UINT32     Signature
>> +
>> +  )
>> +{
>> +  EFI_STATUS                                    Status;
>> +  VOID                                          *Table;
>> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
>> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
>> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
>> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
>> +
>> +  Rsdp = NULL;
>> +  //
>> +  // Find ACPI ConfigurationTable (RSD_PTR)
>> +  //
>> +  Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
>> +  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>> +    return NULL;
>> +  }
>> +
>> +  Table = NULL;
>> +
>> +  //
>> +  // Search XSDT
>> +  //
>> +  if (Rsdp->Revision >=
>> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
>> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
>> +    if (Signature ==
>> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>> +      //
>> +      // It is to find FACS ACPI table,
>> +      // need find FADT first.
>> +      //
>> +      Fadt = ScanTableInXSDT (Xsdt,
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
>> +      Table = FindAcpiFacsFromFadt (Fadt);
>> +    } else {
>> +      Table = ScanTableInXSDT (Xsdt, Signature);
>> +    }
>> +  }
>> +
>> +  if (Table != NULL) {
>> +    return Table;
>> +  }
>> +
>> +  //
>> +  // Search RSDT
>> +  //
>> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
>> +  if (Signature ==
>> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>> +    //
>> +    // It is to find FACS ACPI table,
>> +    // need find FADT first.
>> +    //
>> +    Fadt = ScanTableInRSDT (Rsdt,
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
>> +    Table = FindAcpiFacsFromFadt (Fadt);
>> +  } else {
>> +    Table = ScanTableInRSDT (Rsdt, Signature);
>> +  }
>> +
>> +  return Table;
>> +}
>> +
>> +/**
>> +  This function finds ACPI table by signature.
>> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
>> +  and then gEfiAcpi10TableGuid system configuration table.
>> +
>> +  @param Signature  ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +EfiFindAcpiTableBySignature (
>> +  IN UINT32     Signature
>> +  )
>> +{
>> +  VOID          *Table;
>> +
>> +  Table = FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi20TableGuid,
>> Signature);
>> +  if (Table != NULL) {
>> +    return Table;
>> +  }
>> +
>> +  return FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi10TableGuid,
>> Signature);
>> +}
>> +
>> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
>> b/MdePkg/Library/UefiLib/UefiLib.inf
>> index a6c739ef3d6d..aea20fe67153 100644
>> --- a/MdePkg/Library/UefiLib/UefiLib.inf
>> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
>> @@ -41,6 +41,7 @@ [Sources]
>>   Console.c
>>   UefiLib.c
>>   UefiLibInternal.h
>> +  Acpi.c
>> 
>> 
>> [Packages]
>> @@ -62,6 +63,8 @@ [Guids]
>>   gEfiEventReadyToBootGuid                      ##
>> SOMETIMES_CONSUMES  ## Event
>>   gEfiEventLegacyBootGuid                       ##
>> SOMETIMES_CONSUMES  ## Event
>>   gEfiGlobalVariableGuid                        ##
>> SOMETIMES_CONSUMES  ## Variable
>> +  gEfiAcpi20TableGuid                           ##
>> SOMETIMES_CONSUMES  ## SystemTable
>> +  gEfiAcpi10TableGuid                           ##
>> SOMETIMES_CONSUMES  ## SystemTable
>> 
>> [Protocols]
>>   gEfiDriverBindingProtocolGuid                   ##
>> SOMETIMES_PRODUCES
>> --
>> 2.7.0.windows.1
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Ni, Ruiyu]

I think LocateNextAcpiTable() is more proper to handle the multiple tables with same signature. It will carry three parameters, one is the table header stored in configuration table, one is the signature, another is the previous located table. Can we return a common table header other than void*?

Is there better place other than UefiLib?
Do we need to add a new library class like AcpiLib?

发自我的 iPhone

> 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com> 写道：
> 
> Good enhancement.
> 
> I have 2 additional thought:
> 
> 1) How to handle DSDT?
> We have special code to handle FACS, but no DSDT.
> 
> 2) How to handle SSDT or other multiple ACPI tables?
> We may have multiple SSDT. Usually, it is identified as OEMID.
> Do we want to provide similar function for them?
> 
> Anyway, just *additional* thought. :-)
> Current implementation is good enough for check in.
> 
> Reviewed-by: Jiewen.yao@intel.com
> 
> Thank you
> Yao Jiewen
> 
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
>> Zeng
>> Sent: Friday, August 31, 2018 7:29 PM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
>> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
>> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
>> EfiFindAcpiTableBySignature() API
>> 
>> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
>> for the request and also the following patch to 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>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>> MdePkg/Include/Library/UefiLib.h   |  17 +++
>> MdePkg/Library/UefiLib/Acpi.c      | 226
>> +++++++++++++++++++++++++++++++++++++
>> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>> 3 files changed, 246 insertions(+)
>> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
>> 
>> diff --git a/MdePkg/Include/Library/UefiLib.h
>> b/MdePkg/Include/Library/UefiLib.h
>> index f80067f11103..8dd25f324fd2 100644
>> --- a/MdePkg/Include/Library/UefiLib.h
>> +++ b/MdePkg/Include/Library/UefiLib.h
>> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
>>  IN     UINT64                    OpenMode,
>>  IN     UINT64                    Attributes
>>  );
>> +
>> +/**
>> +  This function finds ACPI table by signature.
>> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
>> +  and then gEfiAcpi10TableGuid system configuration table.
>> +
>> +  @param Signature  ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +EfiFindAcpiTableBySignature (
>> +  IN UINT32     Signature
>> +  );
>> +
>> #endif
>> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
>> new file mode 100644
>> index 000000000000..5cb93966b59f
>> --- /dev/null
>> +++ b/MdePkg/Library/UefiLib/Acpi.c
>> @@ -0,0 +1,226 @@
>> +/** @file
>> +  This module provides help function for finding ACPI table.
>> +
>> +  Copyright (c) 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.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include "UefiLibInternal.h"
>> +#include <IndustryStandard/Acpi.h>
>> +#include <Guid/Acpi.h>
>> +
>> +/**
>> +  This function scans ACPI table in RSDT.
>> +
>> +  @param Rsdt       ACPI RSDT
>> +  @param Signature  ACPI table signature
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +ScanTableInRSDT (
>> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
>> +  IN UINT32                         Signature
>> +  )
>> +{
>> +  UINTN                              Index;
>> +  UINT32                             EntryCount;
>> +  UINT32                             *EntryPtr;
>> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
>> +
>> +  if (Rsdt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>> sizeof(UINT32);
>> +
>> +  EntryPtr = (UINT32 *)(Rsdt + 1);
>> +  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
>> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
>> +    if (Table->Signature == Signature) {
>> +      return Table;
>> +    }
>> +  }
>> +
>> +  return NULL;
>> +}
>> +
>> +/**
>> +  This function scans ACPI table in XSDT.
>> +
>> +  @param Xsdt       ACPI XSDT
>> +  @param Signature  ACPI table signature
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +ScanTableInXSDT (
>> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
>> +  IN UINT32                         Signature
>> +  )
>> +{
>> +  UINTN                          Index;
>> +  UINT32                         EntryCount;
>> +  UINT64                         EntryPtr;
>> +  UINTN                          BasePtr;
>> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
>> +
>> +  if (Xsdt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>> sizeof(UINT64);
>> +
>> +  BasePtr = (UINTN)(Xsdt + 1);
>> +  for (Index = 0; Index < EntryCount; Index ++) {
>> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
>> sizeof(UINT64));
>> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
>> +    if (Table->Signature == Signature) {
>> +      return Table;
>> +    }
>> +  }
>> +
>> +  return NULL;
>> +}
>> +
>> +/**
>> +  To find Facs in FADT.
>> +
>> +  @param Fadt   FADT table pointer
>> +
>> +  @return Facs table pointer or NULL if not found.
>> +
>> +**/
>> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
>> +FindAcpiFacsFromFadt (
>> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
>> +  )
>> +{
>> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
>> +  UINT64                                        Data64;
>> +
>> +  if (Fadt == NULL) {
>> +    return NULL;
>> +  }
>> +
>> +  if (Fadt->Header.Revision <
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
>> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Fadt->FirmwareCtrl;
>> +  } else {
>> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
>> +    if (Data64 != 0) {
>> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Data64;
>> +    } else {
>> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
>> *)(UINTN)Fadt->FirmwareCtrl;
>> +    }
>> +  }
>> +  return Facs;
>> +}
>> +
>> +/**
>> +  To find ACPI table in ACPI ConfigurationTable.
>> +
>> +  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
>> +  @param Signature      ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID  *
>> +FindAcpiTableInAcpiConfigurationTable (
>> +  IN EFI_GUID   *AcpiGuid,
>> +  IN UINT32     Signature
>> +
>> +  )
>> +{
>> +  EFI_STATUS                                    Status;
>> +  VOID                                          *Table;
>> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
>> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
>> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
>> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
>> +
>> +  Rsdp = NULL;
>> +  //
>> +  // Find ACPI ConfigurationTable (RSD_PTR)
>> +  //
>> +  Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
>> +  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>> +    return NULL;
>> +  }
>> +
>> +  Table = NULL;
>> +
>> +  //
>> +  // Search XSDT
>> +  //
>> +  if (Rsdp->Revision >=
>> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
>> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
>> +    if (Signature ==
>> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>> +      //
>> +      // It is to find FACS ACPI table,
>> +      // need find FADT first.
>> +      //
>> +      Fadt = ScanTableInXSDT (Xsdt,
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
>> +      Table = FindAcpiFacsFromFadt (Fadt);
>> +    } else {
>> +      Table = ScanTableInXSDT (Xsdt, Signature);
>> +    }
>> +  }
>> +
>> +  if (Table != NULL) {
>> +    return Table;
>> +  }
>> +
>> +  //
>> +  // Search RSDT
>> +  //
>> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
>> +  if (Signature ==
>> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>> +    //
>> +    // It is to find FACS ACPI table,
>> +    // need find FADT first.
>> +    //
>> +    Fadt = ScanTableInRSDT (Rsdt,
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
>> +    Table = FindAcpiFacsFromFadt (Fadt);
>> +  } else {
>> +    Table = ScanTableInRSDT (Rsdt, Signature);
>> +  }
>> +
>> +  return Table;
>> +}
>> +
>> +/**
>> +  This function finds ACPI table by signature.
>> +  It will find the table in gEfiAcpi20TableGuid system configuration table first,
>> +  and then gEfiAcpi10TableGuid system configuration table.
>> +
>> +  @param Signature  ACPI table signature.
>> +
>> +  @return ACPI table or NULL if not found.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +EfiFindAcpiTableBySignature (
>> +  IN UINT32     Signature
>> +  )
>> +{
>> +  VOID          *Table;
>> +
>> +  Table = FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi20TableGuid,
>> Signature);
>> +  if (Table != NULL) {
>> +    return Table;
>> +  }
>> +
>> +  return FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi10TableGuid,
>> Signature);
>> +}
>> +
>> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
>> b/MdePkg/Library/UefiLib/UefiLib.inf
>> index a6c739ef3d6d..aea20fe67153 100644
>> --- a/MdePkg/Library/UefiLib/UefiLib.inf
>> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
>> @@ -41,6 +41,7 @@ [Sources]
>>  Console.c
>>  UefiLib.c
>>  UefiLibInternal.h
>> +  Acpi.c
>> 
>> 
>> [Packages]
>> @@ -62,6 +63,8 @@ [Guids]
>>  gEfiEventReadyToBootGuid                      ##
>> SOMETIMES_CONSUMES  ## Event
>>  gEfiEventLegacyBootGuid                       ##
>> SOMETIMES_CONSUMES  ## Event
>>  gEfiGlobalVariableGuid                        ##
>> SOMETIMES_CONSUMES  ## Variable
>> +  gEfiAcpi20TableGuid                           ##
>> SOMETIMES_CONSUMES  ## SystemTable
>> +  gEfiAcpi10TableGuid                           ##
>> SOMETIMES_CONSUMES  ## SystemTable
>> 
>> [Protocols]
>>  gEfiDriverBindingProtocolGuid                   ##
>> SOMETIMES_PRODUCES
>> --
>> 2.7.0.windows.1
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Yao, Jiewen]

Good idea on LocateNextAcpiTable().


> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Saturday, September 1, 2018 12:29 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael
> D <michael.d.kinney@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> I think LocateNextAcpiTable() is more proper to handle the multiple tables with
> same signature. It will carry three parameters, one is the table header stored in
> configuration table, one is the signature, another is the previous located table.
> Can we return a common table header other than void*?
> 
> Is there better place other than UefiLib?
> Do we need to add a new library class like AcpiLib?
> 
> 发自我的 iPhone
> 
> > 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com> 写
> 道：
> >
> > Good enhancement.
> >
> > I have 2 additional thought:
> >
> > 1) How to handle DSDT?
> > We have special code to handle FACS, but no DSDT.
> >
> > 2) How to handle SSDT or other multiple ACPI tables?
> > We may have multiple SSDT. Usually, it is identified as OEMID.
> > Do we want to provide similar function for them?
> >
> > Anyway, just *additional* thought. :-)
> > Current implementation is good enough for check in.
> >
> > Reviewed-by: Jiewen.yao@intel.com
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Star
> >> Zeng
> >> Sent: Friday, August 31, 2018 7:29 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao,
> >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> >> EfiFindAcpiTableBySignature() API
> >>
> >> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
> >> for the request and also the following patch to 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>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> >> ---
> >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> >> MdePkg/Library/UefiLib/Acpi.c      | 226
> >> +++++++++++++++++++++++++++++++++++++
> >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> >> 3 files changed, 246 insertions(+)
> >> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> >>
> >> diff --git a/MdePkg/Include/Library/UefiLib.h
> >> b/MdePkg/Include/Library/UefiLib.h
> >> index f80067f11103..8dd25f324fd2 100644
> >> --- a/MdePkg/Include/Library/UefiLib.h
> >> +++ b/MdePkg/Include/Library/UefiLib.h
> >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> >>  IN     UINT64                    OpenMode,
> >>  IN     UINT64                    Attributes
> >>  );
> >> +
> >> +/**
> >> +  This function finds ACPI table by signature.
> >> +  It will find the table in gEfiAcpi20TableGuid system configuration table
> first,
> >> +  and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> +  @param Signature  ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> +  IN UINT32     Signature
> >> +  );
> >> +
> >> #endif
> >> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> >> new file mode 100644
> >> index 000000000000..5cb93966b59f
> >> --- /dev/null
> >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> >> @@ -0,0 +1,226 @@
> >> +/** @file
> >> +  This module provides help function for finding ACPI table.
> >> +
> >> +  Copyright (c) 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.
> >> +
> >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> >> BASIS,
> >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include "UefiLibInternal.h"
> >> +#include <IndustryStandard/Acpi.h>
> >> +#include <Guid/Acpi.h>
> >> +
> >> +/**
> >> +  This function scans ACPI table in RSDT.
> >> +
> >> +  @param Rsdt       ACPI RSDT
> >> +  @param Signature  ACPI table signature
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInRSDT (
> >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> >> +  IN UINT32                         Signature
> >> +  )
> >> +{
> >> +  UINTN                              Index;
> >> +  UINT32                             EntryCount;
> >> +  UINT32                             *EntryPtr;
> >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> >> +
> >> +  if (Rsdt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT32);
> >> +
> >> +  EntryPtr = (UINT32 *)(Rsdt + 1);
> >> +  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> >> +    if (Table->Signature == Signature) {
> >> +      return Table;
> >> +    }
> >> +  }
> >> +
> >> +  return NULL;
> >> +}
> >> +
> >> +/**
> >> +  This function scans ACPI table in XSDT.
> >> +
> >> +  @param Xsdt       ACPI XSDT
> >> +  @param Signature  ACPI table signature
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInXSDT (
> >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> >> +  IN UINT32                         Signature
> >> +  )
> >> +{
> >> +  UINTN                          Index;
> >> +  UINT32                         EntryCount;
> >> +  UINT64                         EntryPtr;
> >> +  UINTN                          BasePtr;
> >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> >> +
> >> +  if (Xsdt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT64);
> >> +
> >> +  BasePtr = (UINTN)(Xsdt + 1);
> >> +  for (Index = 0; Index < EntryCount; Index ++) {
> >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
> >> sizeof(UINT64));
> >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> >> +    if (Table->Signature == Signature) {
> >> +      return Table;
> >> +    }
> >> +  }
> >> +
> >> +  return NULL;
> >> +}
> >> +
> >> +/**
> >> +  To find Facs in FADT.
> >> +
> >> +  @param Fadt   FADT table pointer
> >> +
> >> +  @return Facs table pointer or NULL if not found.
> >> +
> >> +**/
> >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> >> +FindAcpiFacsFromFadt (
> >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> >> +  )
> >> +{
> >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> >> +  UINT64                                        Data64;
> >> +
> >> +  if (Fadt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  if (Fadt->Header.Revision <
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> +  } else {
> >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> >> +    if (Data64 != 0) {
> >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Data64;
> >> +    } else {
> >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> +    }
> >> +  }
> >> +  return Facs;
> >> +}
> >> +
> >> +/**
> >> +  To find ACPI table in ACPI ConfigurationTable.
> >> +
> >> +  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
> >> +  @param Signature      ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID  *
> >> +FindAcpiTableInAcpiConfigurationTable (
> >> +  IN EFI_GUID   *AcpiGuid,
> >> +  IN UINT32     Signature
> >> +
> >> +  )
> >> +{
> >> +  EFI_STATUS                                    Status;
> >> +  VOID                                          *Table;
> >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> >> +
> >> +  Rsdp = NULL;
> >> +  //
> >> +  // Find ACPI ConfigurationTable (RSD_PTR)
> >> +  //
> >> +  Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
> >> +  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  Table = NULL;
> >> +
> >> +  //
> >> +  // Search XSDT
> >> +  //
> >> +  if (Rsdp->Revision >=
> >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> Rsdp->XsdtAddress;
> >> +    if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> +      //
> >> +      // It is to find FACS ACPI table,
> >> +      // need find FADT first.
> >> +      //
> >> +      Fadt = ScanTableInXSDT (Xsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> +      Table = FindAcpiFacsFromFadt (Fadt);
> >> +    } else {
> >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> >> +    }
> >> +  }
> >> +
> >> +  if (Table != NULL) {
> >> +    return Table;
> >> +  }
> >> +
> >> +  //
> >> +  // Search RSDT
> >> +  //
> >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> >> +  if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> +    //
> >> +    // It is to find FACS ACPI table,
> >> +    // need find FADT first.
> >> +    //
> >> +    Fadt = ScanTableInRSDT (Rsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> +    Table = FindAcpiFacsFromFadt (Fadt);
> >> +  } else {
> >> +    Table = ScanTableInRSDT (Rsdt, Signature);
> >> +  }
> >> +
> >> +  return Table;
> >> +}
> >> +
> >> +/**
> >> +  This function finds ACPI table by signature.
> >> +  It will find the table in gEfiAcpi20TableGuid system configuration table
> first,
> >> +  and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> +  @param Signature  ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> +  IN UINT32     Signature
> >> +  )
> >> +{
> >> +  VOID          *Table;
> >> +
> >> +  Table = FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi20TableGuid,
> >> Signature);
> >> +  if (Table != NULL) {
> >> +    return Table;
> >> +  }
> >> +
> >> +  return FindAcpiTableInAcpiConfigurationTable (&gEfiAcpi10TableGuid,
> >> Signature);
> >> +}
> >> +
> >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> >> b/MdePkg/Library/UefiLib/UefiLib.inf
> >> index a6c739ef3d6d..aea20fe67153 100644
> >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> >> @@ -41,6 +41,7 @@ [Sources]
> >>  Console.c
> >>  UefiLib.c
> >>  UefiLibInternal.h
> >> +  Acpi.c
> >>
> >>
> >> [Packages]
> >> @@ -62,6 +63,8 @@ [Guids]
> >>  gEfiEventReadyToBootGuid                      ##
> >> SOMETIMES_CONSUMES  ## Event
> >>  gEfiEventLegacyBootGuid                       ##
> >> SOMETIMES_CONSUMES  ## Event
> >>  gEfiGlobalVariableGuid                        ##
> >> SOMETIMES_CONSUMES  ## Variable
> >> +  gEfiAcpi20TableGuid                           ##
> >> SOMETIMES_CONSUMES  ## SystemTable
> >> +  gEfiAcpi10TableGuid                           ##
> >> SOMETIMES_CONSUMES  ## SystemTable
> >>
> >> [Protocols]
> >>  gEfiDriverBindingProtocolGuid                   ##
> >> SOMETIMES_PRODUCES
> >> --
> >> 2.7.0.windows.1
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Zeng, Star]

Good idea.

I did consider DSDT and multiple SSDTs cases. But I did not find any real case for them.
So I made the code simply for current cases, and the code can be easily enhanced later for DSDT, a new API can be added later for multiple SSDTs.

About adding the new API in UefiLib VS new library class, I did also consider it and even created code for new library class (git@github.com:lzeng14/edk2.git branch FindAcpiTableBySignature_UefiAcpiTableLib).
But I remember I did discuss it with Mike and Jiewen, we recommended UefiLib as it will do not require any platform change.



Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Saturday, September 1, 2018 7:04 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

Good idea on LocateNextAcpiTable().


> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Saturday, September 1, 2018 12:29 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, 
> Michael D <michael.d.kinney@intel.com>; Younas khan 
> <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> I think LocateNextAcpiTable() is more proper to handle the multiple 
> tables with same signature. It will carry three parameters, one is the 
> table header stored in configuration table, one is the signature, another is the previous located table.
> Can we return a common table header other than void*?
> 
> Is there better place other than UefiLib?
> Do we need to add a new library class like AcpiLib?
> 
> 发自我的 iPhone
> 
> > 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com> 写
> 道：
> >
> > Good enhancement.
> >
> > I have 2 additional thought:
> >
> > 1) How to handle DSDT?
> > We have special code to handle FACS, but no DSDT.
> >
> > 2) How to handle SSDT or other multiple ACPI tables?
> > We may have multiple SSDT. Usually, it is identified as OEMID.
> > Do we want to provide similar function for them?
> >
> > Anyway, just *additional* thought. :-) Current implementation is 
> > good enough for check in.
> >
> > Reviewed-by: Jiewen.yao@intel.com
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf 
> >> Of
> Star
> >> Zeng
> >> Sent: Friday, August 31, 2018 7:29 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan 
> >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao,
> >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> >> EfiFindAcpiTableBySignature() API
> >>
> >> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib 
> >> for the request and also the following patch to 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>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> >> ---
> >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> >> MdePkg/Library/UefiLib/Acpi.c      | 226
> >> +++++++++++++++++++++++++++++++++++++
> >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> >> 3 files changed, 246 insertions(+)
> >> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> >>
> >> diff --git a/MdePkg/Include/Library/UefiLib.h
> >> b/MdePkg/Include/Library/UefiLib.h
> >> index f80067f11103..8dd25f324fd2 100644
> >> --- a/MdePkg/Include/Library/UefiLib.h
> >> +++ b/MdePkg/Include/Library/UefiLib.h
> >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> >>  IN     UINT64                    OpenMode,
> >>  IN     UINT64                    Attributes
> >>  );
> >> +
> >> +/**
> >> +  This function finds ACPI table by signature.
> >> +  It will find the table in gEfiAcpi20TableGuid system 
> >> +configuration table
> first,
> >> +  and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> +  @param Signature  ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> +  IN UINT32     Signature
> >> +  );
> >> +
> >> #endif
> >> diff --git a/MdePkg/Library/UefiLib/Acpi.c 
> >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index 
> >> 000000000000..5cb93966b59f
> >> --- /dev/null
> >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> >> @@ -0,0 +1,226 @@
> >> +/** @file
> >> +  This module provides help function for finding ACPI table.
> >> +
> >> +  Copyright (c) 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.
> >> +
> >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> >> BASIS,
> >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include "UefiLibInternal.h"
> >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> >> +
> >> +/**
> >> +  This function scans ACPI table in RSDT.
> >> +
> >> +  @param Rsdt       ACPI RSDT
> >> +  @param Signature  ACPI table signature
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInRSDT (
> >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> >> +  IN UINT32                         Signature
> >> +  )
> >> +{
> >> +  UINTN                              Index;
> >> +  UINT32                             EntryCount;
> >> +  UINT32                             *EntryPtr;
> >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> >> +
> >> +  if (Rsdt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  EntryCount = (Rsdt->Length - sizeof 
> >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT32);
> >> +
> >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index < 
> >> + EntryCount; Index ++, EntryPtr ++) {
> >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> >> +    if (Table->Signature == Signature) {
> >> +      return Table;
> >> +    }
> >> +  }
> >> +
> >> +  return NULL;
> >> +}
> >> +
> >> +/**
> >> +  This function scans ACPI table in XSDT.
> >> +
> >> +  @param Xsdt       ACPI XSDT
> >> +  @param Signature  ACPI table signature
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInXSDT (
> >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> >> +  IN UINT32                         Signature
> >> +  )
> >> +{
> >> +  UINTN                          Index;
> >> +  UINT32                         EntryCount;
> >> +  UINT64                         EntryPtr;
> >> +  UINTN                          BasePtr;
> >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> >> +
> >> +  if (Xsdt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  EntryCount = (Xsdt->Length - sizeof 
> >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT64);
> >> +
> >> +  BasePtr = (UINTN)(Xsdt + 1);
> >> +  for (Index = 0; Index < EntryCount; Index ++) {
> >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * 
> >> + sizeof(UINT64)),
> >> sizeof(UINT64));
> >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> >> +    if (Table->Signature == Signature) {
> >> +      return Table;
> >> +    }
> >> +  }
> >> +
> >> +  return NULL;
> >> +}
> >> +
> >> +/**
> >> +  To find Facs in FADT.
> >> +
> >> +  @param Fadt   FADT table pointer
> >> +
> >> +  @return Facs table pointer or NULL if not found.
> >> +
> >> +**/
> >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE * 
> >> +FindAcpiFacsFromFadt (
> >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> >> +  )
> >> +{
> >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> >> +  UINT64                                        Data64;
> >> +
> >> +  if (Fadt == NULL) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  if (Fadt->Header.Revision <
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> +  } else {
> >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> >> +    if (Data64 != 0) {
> >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Data64;
> >> +    } else {
> >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> +    }
> >> +  }
> >> +  return Facs;
> >> +}
> >> +
> >> +/**
> >> +  To find ACPI table in ACPI ConfigurationTable.
> >> +
> >> +  @param AcpiTableGuid  The guid used to find ACPI ConfigurationTable.
> >> +  @param Signature      ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID  *
> >> +FindAcpiTableInAcpiConfigurationTable (
> >> +  IN EFI_GUID   *AcpiGuid,
> >> +  IN UINT32     Signature
> >> +
> >> +  )
> >> +{
> >> +  EFI_STATUS                                    Status;
> >> +  VOID                                          *Table;
> >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> >> +
> >> +  Rsdp = NULL;
> >> +  //
> >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status = 
> >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  if 
> >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> >> +    return NULL;
> >> +  }
> >> +
> >> +  Table = NULL;
> >> +
> >> +  //
> >> +  // Search XSDT
> >> +  //
> >> +  if (Rsdp->Revision >=
> >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> Rsdp->XsdtAddress;
> >> +    if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> +      //
> >> +      // It is to find FACS ACPI table,
> >> +      // need find FADT first.
> >> +      //
> >> +      Fadt = ScanTableInXSDT (Xsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> +      Table = FindAcpiFacsFromFadt (Fadt);
> >> +    } else {
> >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> >> +    }
> >> +  }
> >> +
> >> +  if (Table != NULL) {
> >> +    return Table;
> >> +  }
> >> +
> >> +  //
> >> +  // Search RSDT
> >> +  //
> >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;  
> >> + if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> +    //
> >> +    // It is to find FACS ACPI table,
> >> +    // need find FADT first.
> >> +    //
> >> +    Fadt = ScanTableInRSDT (Rsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> >> +
> >> +  return Table;
> >> +}
> >> +
> >> +/**
> >> +  This function finds ACPI table by signature.
> >> +  It will find the table in gEfiAcpi20TableGuid system 
> >> +configuration table
> first,
> >> +  and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> +  @param Signature  ACPI table signature.
> >> +
> >> +  @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> +  IN UINT32     Signature
> >> +  )
> >> +{
> >> +  VOID          *Table;
> >> +
> >> +  Table = FindAcpiTableInAcpiConfigurationTable 
> >> + (&gEfiAcpi20TableGuid,
> >> Signature);
> >> +  if (Table != NULL) {
> >> +    return Table;
> >> +  }
> >> +
> >> +  return FindAcpiTableInAcpiConfigurationTable 
> >> + (&gEfiAcpi10TableGuid,
> >> Signature);
> >> +}
> >> +
> >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> >> b/MdePkg/Library/UefiLib/UefiLib.inf
> >> index a6c739ef3d6d..aea20fe67153 100644
> >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> >> @@ -41,6 +41,7 @@ [Sources]
> >>  Console.c
> >>  UefiLib.c
> >>  UefiLibInternal.h
> >> +  Acpi.c
> >>
> >>
> >> [Packages]
> >> @@ -62,6 +63,8 @@ [Guids]
> >>  gEfiEventReadyToBootGuid                      ##
> >> SOMETIMES_CONSUMES  ## Event
> >>  gEfiEventLegacyBootGuid                       ##
> >> SOMETIMES_CONSUMES  ## Event
> >>  gEfiGlobalVariableGuid                        ##
> >> SOMETIMES_CONSUMES  ## Variable
> >> +  gEfiAcpi20TableGuid                           ##
> >> SOMETIMES_CONSUMES  ## SystemTable
> >> +  gEfiAcpi10TableGuid                           ##
> >> SOMETIMES_CONSUMES  ## SystemTable
> >>
> >> [Protocols]
> >>  gEfiDriverBindingProtocolGuid                   ##
> >> SOMETIMES_PRODUCES
> >> --
> >> 2.7.0.windows.1
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Ni, Ruiyu]

That's fine to be in UefiLib. It's already a combo library.

But I do recommend we think about how to handle multiple tables with same signature.
When we are adding new APIs, we not only need to evaluate the existing real case, but also
we need to generalize the real cases and try to think about the more flexible interface.


Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, September 3, 2018 11:26 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> Good idea.
> 
> I did consider DSDT and multiple SSDTs cases. But I did not find any real case
> for them.
> So I made the code simply for current cases, and the code can be easily
> enhanced later for DSDT, a new API can be added later for multiple SSDTs.
> 
> About adding the new API in UefiLib VS new library class, I did also consider it
> and even created code for new library class
> (git@github.com:lzeng14/edk2.git branch
> FindAcpiTableBySignature_UefiAcpiTableLib).
> But I remember I did discuss it with Mike and Jiewen, we recommended
> UefiLib as it will do not require any platform change.
> 
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Saturday, September 1, 2018 7:04 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney,
> Michael D <michael.d.kinney@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> Good idea on LocateNextAcpiTable().
> 
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Saturday, September 1, 2018 12:29 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Younas khan
> > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > I think LocateNextAcpiTable() is more proper to handle the multiple
> > tables with same signature. It will carry three parameters, one is the
> > table header stored in configuration table, one is the signature, another is
> the previous located table.
> > Can we return a common table header other than void*?
> >
> > Is there better place other than UefiLib?
> > Do we need to add a new library class like AcpiLib?
> >
> > 发自我的 iPhone
> >
> > > 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com>
> 写
> > 道：
> > >
> > > Good enhancement.
> > >
> > > I have 2 additional thought:
> > >
> > > 1) How to handle DSDT?
> > > We have special code to handle FACS, but no DSDT.
> > >
> > > 2) How to handle SSDT or other multiple ACPI tables?
> > > We may have multiple SSDT. Usually, it is identified as OEMID.
> > > Do we want to provide similar function for them?
> > >
> > > Anyway, just *additional* thought. :-) Current implementation is
> > > good enough for check in.
> > >
> > > Reviewed-by: Jiewen.yao@intel.com
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > >
> > >> -----Original Message-----
> > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > >> Of
> > Star
> > >> Zeng
> > >> Sent: Friday, August 31, 2018 7:29 PM
> > >> To: edk2-devel@lists.01.org
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> > Gao,
> > >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > >> EfiFindAcpiTableBySignature() API
> > >>
> > >> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
> > >> for the request and also the following patch to 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>
> > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> > >> ---
> > >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> > >> MdePkg/Library/UefiLib/Acpi.c      | 226
> > >> +++++++++++++++++++++++++++++++++++++
> > >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> > >> 3 files changed, 246 insertions(+)
> > >> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> > >>
> > >> diff --git a/MdePkg/Include/Library/UefiLib.h
> > >> b/MdePkg/Include/Library/UefiLib.h
> > >> index f80067f11103..8dd25f324fd2 100644
> > >> --- a/MdePkg/Include/Library/UefiLib.h
> > >> +++ b/MdePkg/Include/Library/UefiLib.h
> > >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> > >>  IN     UINT64                    OpenMode,
> > >>  IN     UINT64                    Attributes
> > >>  );
> > >> +
> > >> +/**
> > >> +  This function finds ACPI table by signature.
> > >> +  It will find the table in gEfiAcpi20TableGuid system
> > >> +configuration table
> > first,
> > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > >> +
> > >> +  @param Signature  ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +EFIAPI
> > >> +EfiFindAcpiTableBySignature (
> > >> +  IN UINT32     Signature
> > >> +  );
> > >> +
> > >> #endif
> > >> diff --git a/MdePkg/Library/UefiLib/Acpi.c
> > >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index
> > >> 000000000000..5cb93966b59f
> > >> --- /dev/null
> > >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> > >> @@ -0,0 +1,226 @@
> > >> +/** @file
> > >> +  This module provides help function for finding ACPI table.
> > >> +
> > >> +  Copyright (c) 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.
> > >> +
> > >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> > >> BASIS,
> > >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > >> EXPRESS OR IMPLIED.
> > >> +
> > >> +**/
> > >> +
> > >> +#include "UefiLibInternal.h"
> > >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> > >> +
> > >> +/**
> > >> +  This function scans ACPI table in RSDT.
> > >> +
> > >> +  @param Rsdt       ACPI RSDT
> > >> +  @param Signature  ACPI table signature
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +ScanTableInRSDT (
> > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> > >> +  IN UINT32                         Signature
> > >> +  )
> > >> +{
> > >> +  UINTN                              Index;
> > >> +  UINT32                             EntryCount;
> > >> +  UINT32                             *EntryPtr;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> > >> +
> > >> +  if (Rsdt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  EntryCount = (Rsdt->Length - sizeof
> > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > >> sizeof(UINT32);
> > >> +
> > >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index <
> > >> + EntryCount; Index ++, EntryPtr ++) {
> > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> > >> +    if (Table->Signature == Signature) {
> > >> +      return Table;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  return NULL;
> > >> +}
> > >> +
> > >> +/**
> > >> +  This function scans ACPI table in XSDT.
> > >> +
> > >> +  @param Xsdt       ACPI XSDT
> > >> +  @param Signature  ACPI table signature
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +ScanTableInXSDT (
> > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> > >> +  IN UINT32                         Signature
> > >> +  )
> > >> +{
> > >> +  UINTN                          Index;
> > >> +  UINT32                         EntryCount;
> > >> +  UINT64                         EntryPtr;
> > >> +  UINTN                          BasePtr;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> > >> +
> > >> +  if (Xsdt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  EntryCount = (Xsdt->Length - sizeof
> > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > >> sizeof(UINT64);
> > >> +
> > >> +  BasePtr = (UINTN)(Xsdt + 1);
> > >> +  for (Index = 0; Index < EntryCount; Index ++) {
> > >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index *
> > >> + sizeof(UINT64)),
> > >> sizeof(UINT64));
> > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> > >> +    if (Table->Signature == Signature) {
> > >> +      return Table;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  return NULL;
> > >> +}
> > >> +
> > >> +/**
> > >> +  To find Facs in FADT.
> > >> +
> > >> +  @param Fadt   FADT table pointer
> > >> +
> > >> +  @return Facs table pointer or NULL if not found.
> > >> +
> > >> +**/
> > >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> > >> +FindAcpiFacsFromFadt (
> > >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> > >> +  )
> > >> +{
> > >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> > >> +  UINT64                                        Data64;
> > >> +
> > >> +  if (Fadt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  if (Fadt->Header.Revision <
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> > >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Fadt->FirmwareCtrl;
> > >> +  } else {
> > >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> > >> +    if (Data64 != 0) {
> > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Data64;
> > >> +    } else {
> > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Fadt->FirmwareCtrl;
> > >> +    }
> > >> +  }
> > >> +  return Facs;
> > >> +}
> > >> +
> > >> +/**
> > >> +  To find ACPI table in ACPI ConfigurationTable.
> > >> +
> > >> +  @param AcpiTableGuid  The guid used to find ACPI
> ConfigurationTable.
> > >> +  @param Signature      ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID  *
> > >> +FindAcpiTableInAcpiConfigurationTable (
> > >> +  IN EFI_GUID   *AcpiGuid,
> > >> +  IN UINT32     Signature
> > >> +
> > >> +  )
> > >> +{
> > >> +  EFI_STATUS                                    Status;
> > >> +  VOID                                          *Table;
> > >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> > >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> > >> +
> > >> +  Rsdp = NULL;
> > >> +  //
> > >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status =
> > >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  if
> > >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  Table = NULL;
> > >> +
> > >> +  //
> > >> +  // Search XSDT
> > >> +  //
> > >> +  if (Rsdp->Revision >=
> > >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> > >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> > Rsdp->XsdtAddress;
> > >> +    if (Signature ==
> > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > >> +      //
> > >> +      // It is to find FACS ACPI table,
> > >> +      // need find FADT first.
> > >> +      //
> > >> +      Fadt = ScanTableInXSDT (Xsdt,
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > >> +      Table = FindAcpiFacsFromFadt (Fadt);
> > >> +    } else {
> > >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> > >> +    }
> > >> +  }
> > >> +
> > >> +  if (Table != NULL) {
> > >> +    return Table;
> > >> +  }
> > >> +
> > >> +  //
> > >> +  // Search RSDT
> > >> +  //
> > >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp-
> >RsdtAddress;
> > >> + if (Signature ==
> > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > >> +    //
> > >> +    // It is to find FACS ACPI table,
> > >> +    // need find FADT first.
> > >> +    //
> > >> +    Fadt = ScanTableInRSDT (Rsdt,
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> > >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> > >> +
> > >> +  return Table;
> > >> +}
> > >> +
> > >> +/**
> > >> +  This function finds ACPI table by signature.
> > >> +  It will find the table in gEfiAcpi20TableGuid system
> > >> +configuration table
> > first,
> > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > >> +
> > >> +  @param Signature  ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +EFIAPI
> > >> +EfiFindAcpiTableBySignature (
> > >> +  IN UINT32     Signature
> > >> +  )
> > >> +{
> > >> +  VOID          *Table;
> > >> +
> > >> +  Table = FindAcpiTableInAcpiConfigurationTable
> > >> + (&gEfiAcpi20TableGuid,
> > >> Signature);
> > >> +  if (Table != NULL) {
> > >> +    return Table;
> > >> +  }
> > >> +
> > >> +  return FindAcpiTableInAcpiConfigurationTable
> > >> + (&gEfiAcpi10TableGuid,
> > >> Signature);
> > >> +}
> > >> +
> > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > >> b/MdePkg/Library/UefiLib/UefiLib.inf
> > >> index a6c739ef3d6d..aea20fe67153 100644
> > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > >> @@ -41,6 +41,7 @@ [Sources]
> > >>  Console.c
> > >>  UefiLib.c
> > >>  UefiLibInternal.h
> > >> +  Acpi.c
> > >>
> > >>
> > >> [Packages]
> > >> @@ -62,6 +63,8 @@ [Guids]
> > >>  gEfiEventReadyToBootGuid                      ##
> > >> SOMETIMES_CONSUMES  ## Event
> > >>  gEfiEventLegacyBootGuid                       ##
> > >> SOMETIMES_CONSUMES  ## Event
> > >>  gEfiGlobalVariableGuid                        ##
> > >> SOMETIMES_CONSUMES  ## Variable
> > >> +  gEfiAcpi20TableGuid                           ##
> > >> SOMETIMES_CONSUMES  ## SystemTable
> > >> +  gEfiAcpi10TableGuid                           ##
> > >> SOMETIMES_CONSUMES  ## SystemTable
> > >>
> > >> [Protocols]
> > >>  gEfiDriverBindingProtocolGuid                   ##
> > >> SOMETIMES_PRODUCES
> > >> --
> > >> 2.7.0.windows.1
> > >>
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Zeng, Star]

Thanks.
Ok, please help and we can have good and flexible interface(s) for both producer and consumer.

First, there are two cases we need to consider.
1. Single table, like FADT, FACS, DSDT, etc.
2. Multiple tables, like SSDT, etc.

Then, we have two solutions.
S1:
One interface for single table case, consumer only needs input Signature, has no need input previous returned Table. Like GetFirstGuidHob?
One interface for multiple table case, consumer needs input Signature and previous returned Table. Like GetNextGuidHob? Could we add it later based on real request?
S2:
One interface for both single table and multiple table cases, consumer needs input Signature and previous returned Table. Does consumer need input the table header stored in configuration table by itself(getting configuration table)?


If we like S2, the interface should be like below?

/**
  This function locates next ACPI table based on Signature and previous returned Table.
  If the input previous returned Table is NULL, this function will locate next table
  in gEfiAcpi20TableGuid system configuration table first, and then gEfiAcpi10TableGuid
  system configuration table.
  If the input previous returned Table is not NULL and could be located in
  gEfiAcpi20TableGuid system configuration table, this function will just locate next
  table in gEfiAcpi20TableGuid system configuration table, otherwise gEfiAcpi10TableGuid
  system configuration table.

  @param Signature  ACPI table signature.
  @param Table      Pointer to previous returned Table, or NULL to locate first table.

  @return Next ACPI table or NULL if not found.

**/
EFI_ACPI_COMMON_HEADER *
EFIAPI
EfiLocateNextAcpiTable (
  IN UINT32                  Signature,
  IN EFI_ACPI_COMMON_HEADER  *Table
  )



Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, September 3, 2018 1:09 PM
To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

That's fine to be in UefiLib. It's already a combo library.

But I do recommend we think about how to handle multiple tables with same signature.
When we are adding new APIs, we not only need to evaluate the existing real case, but also we need to generalize the real cases and try to think about the more flexible interface.


Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, September 3, 2018 11:26 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu 
> <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>; 
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> Good idea.
> 
> I did consider DSDT and multiple SSDTs cases. But I did not find any 
> real case for them.
> So I made the code simply for current cases, and the code can be 
> easily enhanced later for DSDT, a new API can be added later for multiple SSDTs.
> 
> About adding the new API in UefiLib VS new library class, I did also 
> consider it and even created code for new library class 
> (git@github.com:lzeng14/edk2.git branch 
> FindAcpiTableBySignature_UefiAcpiTableLib).
> But I remember I did discuss it with Mike and Jiewen, we recommended 
> UefiLib as it will do not require any platform change.
> 
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Saturday, September 1, 2018 7:04 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, 
> Michael D <michael.d.kinney@intel.com>; Younas khan 
> <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> Good idea on LocateNextAcpiTable().
> 
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Saturday, September 1, 2018 12:29 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; 
> > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan 
> > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > I think LocateNextAcpiTable() is more proper to handle the multiple 
> > tables with same signature. It will carry three parameters, one is 
> > the table header stored in configuration table, one is the 
> > signature, another is
> the previous located table.
> > Can we return a common table header other than void*?
> >
> > Is there better place other than UefiLib?
> > Do we need to add a new library class like AcpiLib?
> >
> > 发自我的 iPhone
> >
> > > 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com>
> 写
> > 道：
> > >
> > > Good enhancement.
> > >
> > > I have 2 additional thought:
> > >
> > > 1) How to handle DSDT?
> > > We have special code to handle FACS, but no DSDT.
> > >
> > > 2) How to handle SSDT or other multiple ACPI tables?
> > > We may have multiple SSDT. Usually, it is identified as OEMID.
> > > Do we want to provide similar function for them?
> > >
> > > Anyway, just *additional* thought. :-) Current implementation is 
> > > good enough for check in.
> > >
> > > Reviewed-by: Jiewen.yao@intel.com
> > >
> > > Thank you
> > > Yao Jiewen
> > >
> > >
> > >> -----Original Message-----
> > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On 
> > >> Behalf Of
> > Star
> > >> Zeng
> > >> Sent: Friday, August 31, 2018 7:29 PM
> > >> To: edk2-devel@lists.01.org
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan 
> > >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> > Gao,
> > >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > >> EfiFindAcpiTableBySignature() API
> > >>
> > >> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib 
> > >> for the request and also the following patch to 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>
> > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> > >> ---
> > >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> > >> MdePkg/Library/UefiLib/Acpi.c      | 226
> > >> +++++++++++++++++++++++++++++++++++++
> > >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> > >> 3 files changed, 246 insertions(+) create mode 100644 
> > >> MdePkg/Library/UefiLib/Acpi.c
> > >>
> > >> diff --git a/MdePkg/Include/Library/UefiLib.h
> > >> b/MdePkg/Include/Library/UefiLib.h
> > >> index f80067f11103..8dd25f324fd2 100644
> > >> --- a/MdePkg/Include/Library/UefiLib.h
> > >> +++ b/MdePkg/Include/Library/UefiLib.h
> > >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> > >>  IN     UINT64                    OpenMode,
> > >>  IN     UINT64                    Attributes
> > >>  );
> > >> +
> > >> +/**
> > >> +  This function finds ACPI table by signature.
> > >> +  It will find the table in gEfiAcpi20TableGuid system 
> > >> +configuration table
> > first,
> > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > >> +
> > >> +  @param Signature  ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +EFIAPI
> > >> +EfiFindAcpiTableBySignature (
> > >> +  IN UINT32     Signature
> > >> +  );
> > >> +
> > >> #endif
> > >> diff --git a/MdePkg/Library/UefiLib/Acpi.c 
> > >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index 
> > >> 000000000000..5cb93966b59f
> > >> --- /dev/null
> > >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> > >> @@ -0,0 +1,226 @@
> > >> +/** @file
> > >> +  This module provides help function for finding ACPI table.
> > >> +
> > >> +  Copyright (c) 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.
> > >> +
> > >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> > >> BASIS,
> > >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > >> EXPRESS OR IMPLIED.
> > >> +
> > >> +**/
> > >> +
> > >> +#include "UefiLibInternal.h"
> > >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> > >> +
> > >> +/**
> > >> +  This function scans ACPI table in RSDT.
> > >> +
> > >> +  @param Rsdt       ACPI RSDT
> > >> +  @param Signature  ACPI table signature
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +ScanTableInRSDT (
> > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> > >> +  IN UINT32                         Signature
> > >> +  )
> > >> +{
> > >> +  UINTN                              Index;
> > >> +  UINT32                             EntryCount;
> > >> +  UINT32                             *EntryPtr;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> > >> +
> > >> +  if (Rsdt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  EntryCount = (Rsdt->Length - sizeof
> > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > >> sizeof(UINT32);
> > >> +
> > >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index < 
> > >> + EntryCount; Index ++, EntryPtr ++) {
> > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> > >> +    if (Table->Signature == Signature) {
> > >> +      return Table;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  return NULL;
> > >> +}
> > >> +
> > >> +/**
> > >> +  This function scans ACPI table in XSDT.
> > >> +
> > >> +  @param Xsdt       ACPI XSDT
> > >> +  @param Signature  ACPI table signature
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +ScanTableInXSDT (
> > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> > >> +  IN UINT32                         Signature
> > >> +  )
> > >> +{
> > >> +  UINTN                          Index;
> > >> +  UINT32                         EntryCount;
> > >> +  UINT64                         EntryPtr;
> > >> +  UINTN                          BasePtr;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> > >> +
> > >> +  if (Xsdt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  EntryCount = (Xsdt->Length - sizeof
> > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > >> sizeof(UINT64);
> > >> +
> > >> +  BasePtr = (UINTN)(Xsdt + 1);
> > >> +  for (Index = 0; Index < EntryCount; Index ++) {
> > >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * 
> > >> + sizeof(UINT64)),
> > >> sizeof(UINT64));
> > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> > >> +    if (Table->Signature == Signature) {
> > >> +      return Table;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  return NULL;
> > >> +}
> > >> +
> > >> +/**
> > >> +  To find Facs in FADT.
> > >> +
> > >> +  @param Fadt   FADT table pointer
> > >> +
> > >> +  @return Facs table pointer or NULL if not found.
> > >> +
> > >> +**/
> > >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE * 
> > >> +FindAcpiFacsFromFadt (
> > >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> > >> +  )
> > >> +{
> > >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> > >> +  UINT64                                        Data64;
> > >> +
> > >> +  if (Fadt == NULL) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  if (Fadt->Header.Revision <
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> > >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Fadt->FirmwareCtrl;
> > >> +  } else {
> > >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> > >> +    if (Data64 != 0) {
> > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Data64;
> > >> +    } else {
> > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > >> *)(UINTN)Fadt->FirmwareCtrl;
> > >> +    }
> > >> +  }
> > >> +  return Facs;
> > >> +}
> > >> +
> > >> +/**
> > >> +  To find ACPI table in ACPI ConfigurationTable.
> > >> +
> > >> +  @param AcpiTableGuid  The guid used to find ACPI
> ConfigurationTable.
> > >> +  @param Signature      ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID  *
> > >> +FindAcpiTableInAcpiConfigurationTable (
> > >> +  IN EFI_GUID   *AcpiGuid,
> > >> +  IN UINT32     Signature
> > >> +
> > >> +  )
> > >> +{
> > >> +  EFI_STATUS                                    Status;
> > >> +  VOID                                          *Table;
> > >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> > >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> > >> +
> > >> +  Rsdp = NULL;
> > >> +  //
> > >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status = 
> > >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  if 
> > >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> > >> +    return NULL;
> > >> +  }
> > >> +
> > >> +  Table = NULL;
> > >> +
> > >> +  //
> > >> +  // Search XSDT
> > >> +  //
> > >> +  if (Rsdp->Revision >=
> > >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> > >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> > Rsdp->XsdtAddress;
> > >> +    if (Signature ==
> > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > >> +      //
> > >> +      // It is to find FACS ACPI table,
> > >> +      // need find FADT first.
> > >> +      //
> > >> +      Fadt = ScanTableInXSDT (Xsdt,
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > >> +      Table = FindAcpiFacsFromFadt (Fadt);
> > >> +    } else {
> > >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> > >> +    }
> > >> +  }
> > >> +
> > >> +  if (Table != NULL) {
> > >> +    return Table;
> > >> +  }
> > >> +
> > >> +  //
> > >> +  // Search RSDT
> > >> +  //
> > >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp-
> >RsdtAddress;
> > >> + if (Signature ==
> > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > >> +    //
> > >> +    // It is to find FACS ACPI table,
> > >> +    // need find FADT first.
> > >> +    //
> > >> +    Fadt = ScanTableInRSDT (Rsdt,
> > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> > >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> > >> +
> > >> +  return Table;
> > >> +}
> > >> +
> > >> +/**
> > >> +  This function finds ACPI table by signature.
> > >> +  It will find the table in gEfiAcpi20TableGuid system 
> > >> +configuration table
> > first,
> > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > >> +
> > >> +  @param Signature  ACPI table signature.
> > >> +
> > >> +  @return ACPI table or NULL if not found.
> > >> +
> > >> +**/
> > >> +VOID *
> > >> +EFIAPI
> > >> +EfiFindAcpiTableBySignature (
> > >> +  IN UINT32     Signature
> > >> +  )
> > >> +{
> > >> +  VOID          *Table;
> > >> +
> > >> +  Table = FindAcpiTableInAcpiConfigurationTable
> > >> + (&gEfiAcpi20TableGuid,
> > >> Signature);
> > >> +  if (Table != NULL) {
> > >> +    return Table;
> > >> +  }
> > >> +
> > >> +  return FindAcpiTableInAcpiConfigurationTable
> > >> + (&gEfiAcpi10TableGuid,
> > >> Signature);
> > >> +}
> > >> +
> > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > >> b/MdePkg/Library/UefiLib/UefiLib.inf
> > >> index a6c739ef3d6d..aea20fe67153 100644
> > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > >> @@ -41,6 +41,7 @@ [Sources]
> > >>  Console.c
> > >>  UefiLib.c
> > >>  UefiLibInternal.h
> > >> +  Acpi.c
> > >>
> > >>
> > >> [Packages]
> > >> @@ -62,6 +63,8 @@ [Guids]
> > >>  gEfiEventReadyToBootGuid                      ##
> > >> SOMETIMES_CONSUMES  ## Event
> > >>  gEfiEventLegacyBootGuid                       ##
> > >> SOMETIMES_CONSUMES  ## Event
> > >>  gEfiGlobalVariableGuid                        ##
> > >> SOMETIMES_CONSUMES  ## Variable
> > >> +  gEfiAcpi20TableGuid                           ##
> > >> SOMETIMES_CONSUMES  ## SystemTable
> > >> +  gEfiAcpi10TableGuid                           ##
> > >> SOMETIMES_CONSUMES  ## SystemTable
> > >>
> > >> [Protocols]
> > >>  gEfiDriverBindingProtocolGuid                   ##
> > >> SOMETIMES_PRODUCES
> > >> --
> > >> 2.7.0.windows.1
> > >>
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Yao, Jiewen]

I prefer S1.
I believe that the EfiLocateNextAcpiTable() can also be used in S1.


> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, September 3, 2018 2:12 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> Thanks.
> Ok, please help and we can have good and flexible interface(s) for both producer
> and consumer.
> 
> First, there are two cases we need to consider.
> 1. Single table, like FADT, FACS, DSDT, etc.
> 2. Multiple tables, like SSDT, etc.
> 
> Then, we have two solutions.
> S1:
> One interface for single table case, consumer only needs input Signature, has no
> need input previous returned Table. Like GetFirstGuidHob?
> One interface for multiple table case, consumer needs input Signature and
> previous returned Table. Like GetNextGuidHob? Could we add it later based on
> real request?
> S2:
> One interface for both single table and multiple table cases, consumer needs
> input Signature and previous returned Table. Does consumer need input the
> table header stored in configuration table by itself(getting configuration table)?
> 
> 
> If we like S2, the interface should be like below?
> 
> /**
>   This function locates next ACPI table based on Signature and previous returned
> Table.
>   If the input previous returned Table is NULL, this function will locate next table
>   in gEfiAcpi20TableGuid system configuration table first, and then
> gEfiAcpi10TableGuid
>   system configuration table.
>   If the input previous returned Table is not NULL and could be located in
>   gEfiAcpi20TableGuid system configuration table, this function will just locate
> next
>   table in gEfiAcpi20TableGuid system configuration table, otherwise
> gEfiAcpi10TableGuid
>   system configuration table.
> 
>   @param Signature  ACPI table signature.
>   @param Table      Pointer to previous returned Table, or NULL to locate
> first table.
> 
>   @return Next ACPI table or NULL if not found.
> 
> **/
> EFI_ACPI_COMMON_HEADER *
> EFIAPI
> EfiLocateNextAcpiTable (
>   IN UINT32                  Signature,
>   IN EFI_ACPI_COMMON_HEADER  *Table
>   )
> 
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, September 3, 2018 1:09 PM
> To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> That's fine to be in UefiLib. It's already a combo library.
> 
> But I do recommend we think about how to handle multiple tables with same
> signature.
> When we are adding new APIs, we not only need to evaluate the existing real
> case, but also we need to generalize the real cases and try to think about the
> more flexible interface.
> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Monday, September 3, 2018 11:26 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu
> > <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>;
> > Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > Good idea.
> >
> > I did consider DSDT and multiple SSDTs cases. But I did not find any
> > real case for them.
> > So I made the code simply for current cases, and the code can be
> > easily enhanced later for DSDT, a new API can be added later for multiple
> SSDTs.
> >
> > About adding the new API in UefiLib VS new library class, I did also
> > consider it and even created code for new library class
> > (git@github.com:lzeng14/edk2.git branch
> > FindAcpiTableBySignature_UefiAcpiTableLib).
> > But I remember I did discuss it with Mike and Jiewen, we recommended
> > UefiLib as it will do not require any platform change.
> >
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Yao, Jiewen
> > Sent: Saturday, September 1, 2018 7:04 AM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Younas khan
> > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > Good idea on LocateNextAcpiTable().
> >
> >
> > > -----Original Message-----
> > > From: Ni, Ruiyu
> > > Sent: Saturday, September 1, 2018 12:29 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org;
> > > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > EfiFindAcpiTableBySignature() API
> > >
> > > I think LocateNextAcpiTable() is more proper to handle the multiple
> > > tables with same signature. It will carry three parameters, one is
> > > the table header stored in configuration table, one is the
> > > signature, another is
> > the previous located table.
> > > Can we return a common table header other than void*?
> > >
> > > Is there better place other than UefiLib?
> > > Do we need to add a new library class like AcpiLib?
> > >
> > > 发自我的 iPhone
> > >
> > > > 在 2018年8月31日，下午8:00，Yao, Jiewen <jiewen.yao@intel.com>
> > 写
> > > 道：
> > > >
> > > > Good enhancement.
> > > >
> > > > I have 2 additional thought:
> > > >
> > > > 1) How to handle DSDT?
> > > > We have special code to handle FACS, but no DSDT.
> > > >
> > > > 2) How to handle SSDT or other multiple ACPI tables?
> > > > We may have multiple SSDT. Usually, it is identified as OEMID.
> > > > Do we want to provide similar function for them?
> > > >
> > > > Anyway, just *additional* thought. :-) Current implementation is
> > > > good enough for check in.
> > > >
> > > > Reviewed-by: Jiewen.yao@intel.com
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > >> Behalf Of
> > > Star
> > > >> Zeng
> > > >> Sent: Friday, August 31, 2018 7:29 PM
> > > >> To: edk2-devel@lists.01.org
> > > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > > >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>;
> > > Gao,
> > > >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > >> EfiFindAcpiTableBySignature() API
> > > >>
> > > >> 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 adds new EfiFindAcpiTableBySignature() API in UefiLib
> > > >> for the request and also the following patch to 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>
> > > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > > >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > >> ---
> > > >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> > > >> MdePkg/Library/UefiLib/Acpi.c      | 226
> > > >> +++++++++++++++++++++++++++++++++++++
> > > >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> > > >> 3 files changed, 246 insertions(+) create mode 100644
> > > >> MdePkg/Library/UefiLib/Acpi.c
> > > >>
> > > >> diff --git a/MdePkg/Include/Library/UefiLib.h
> > > >> b/MdePkg/Include/Library/UefiLib.h
> > > >> index f80067f11103..8dd25f324fd2 100644
> > > >> --- a/MdePkg/Include/Library/UefiLib.h
> > > >> +++ b/MdePkg/Include/Library/UefiLib.h
> > > >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> > > >>  IN     UINT64                    OpenMode,
> > > >>  IN     UINT64                    Attributes
> > > >>  );
> > > >> +
> > > >> +/**
> > > >> +  This function finds ACPI table by signature.
> > > >> +  It will find the table in gEfiAcpi20TableGuid system
> > > >> +configuration table
> > > first,
> > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > >> +
> > > >> +  @param Signature  ACPI table signature.
> > > >> +
> > > >> +  @return ACPI table or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +VOID *
> > > >> +EFIAPI
> > > >> +EfiFindAcpiTableBySignature (
> > > >> +  IN UINT32     Signature
> > > >> +  );
> > > >> +
> > > >> #endif
> > > >> diff --git a/MdePkg/Library/UefiLib/Acpi.c
> > > >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index
> > > >> 000000000000..5cb93966b59f
> > > >> --- /dev/null
> > > >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> > > >> @@ -0,0 +1,226 @@
> > > >> +/** @file
> > > >> +  This module provides help function for finding ACPI table.
> > > >> +
> > > >> +  Copyright (c) 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.
> > > >> +
> > > >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> > IS"
> > > >> BASIS,
> > > >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > > >> EXPRESS OR IMPLIED.
> > > >> +
> > > >> +**/
> > > >> +
> > > >> +#include "UefiLibInternal.h"
> > > >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> > > >> +
> > > >> +/**
> > > >> +  This function scans ACPI table in RSDT.
> > > >> +
> > > >> +  @param Rsdt       ACPI RSDT
> > > >> +  @param Signature  ACPI table signature
> > > >> +
> > > >> +  @return ACPI table or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +VOID *
> > > >> +ScanTableInRSDT (
> > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> > > >> +  IN UINT32                         Signature
> > > >> +  )
> > > >> +{
> > > >> +  UINTN                              Index;
> > > >> +  UINT32                             EntryCount;
> > > >> +  UINT32                             *EntryPtr;
> > > >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> > > >> +
> > > >> +  if (Rsdt == NULL) {
> > > >> +    return NULL;
> > > >> +  }
> > > >> +
> > > >> +  EntryCount = (Rsdt->Length - sizeof
> > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > >> sizeof(UINT32);
> > > >> +
> > > >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index <
> > > >> + EntryCount; Index ++, EntryPtr ++) {
> > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> > > >> +    if (Table->Signature == Signature) {
> > > >> +      return Table;
> > > >> +    }
> > > >> +  }
> > > >> +
> > > >> +  return NULL;
> > > >> +}
> > > >> +
> > > >> +/**
> > > >> +  This function scans ACPI table in XSDT.
> > > >> +
> > > >> +  @param Xsdt       ACPI XSDT
> > > >> +  @param Signature  ACPI table signature
> > > >> +
> > > >> +  @return ACPI table or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +VOID *
> > > >> +ScanTableInXSDT (
> > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> > > >> +  IN UINT32                         Signature
> > > >> +  )
> > > >> +{
> > > >> +  UINTN                          Index;
> > > >> +  UINT32                         EntryCount;
> > > >> +  UINT64                         EntryPtr;
> > > >> +  UINTN                          BasePtr;
> > > >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> > > >> +
> > > >> +  if (Xsdt == NULL) {
> > > >> +    return NULL;
> > > >> +  }
> > > >> +
> > > >> +  EntryCount = (Xsdt->Length - sizeof
> > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > >> sizeof(UINT64);
> > > >> +
> > > >> +  BasePtr = (UINTN)(Xsdt + 1);
> > > >> +  for (Index = 0; Index < EntryCount; Index ++) {
> > > >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index *
> > > >> + sizeof(UINT64)),
> > > >> sizeof(UINT64));
> > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> > > >> +    if (Table->Signature == Signature) {
> > > >> +      return Table;
> > > >> +    }
> > > >> +  }
> > > >> +
> > > >> +  return NULL;
> > > >> +}
> > > >> +
> > > >> +/**
> > > >> +  To find Facs in FADT.
> > > >> +
> > > >> +  @param Fadt   FADT table pointer
> > > >> +
> > > >> +  @return Facs table pointer or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> > > >> +FindAcpiFacsFromFadt (
> > > >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> > > >> +  )
> > > >> +{
> > > >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> > > >> +  UINT64                                        Data64;
> > > >> +
> > > >> +  if (Fadt == NULL) {
> > > >> +    return NULL;
> > > >> +  }
> > > >> +
> > > >> +  if (Fadt->Header.Revision <
> > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> > > >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > >> +  } else {
> > > >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> > > >> +    if (Data64 != 0) {
> > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > >> *)(UINTN)Data64;
> > > >> +    } else {
> > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > >> +    }
> > > >> +  }
> > > >> +  return Facs;
> > > >> +}
> > > >> +
> > > >> +/**
> > > >> +  To find ACPI table in ACPI ConfigurationTable.
> > > >> +
> > > >> +  @param AcpiTableGuid  The guid used to find ACPI
> > ConfigurationTable.
> > > >> +  @param Signature      ACPI table signature.
> > > >> +
> > > >> +  @return ACPI table or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +VOID  *
> > > >> +FindAcpiTableInAcpiConfigurationTable (
> > > >> +  IN EFI_GUID   *AcpiGuid,
> > > >> +  IN UINT32     Signature
> > > >> +
> > > >> +  )
> > > >> +{
> > > >> +  EFI_STATUS                                    Status;
> > > >> +  VOID                                          *Table;
> > > >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> > > >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> > > >> +
> > > >> +  Rsdp = NULL;
> > > >> +  //
> > > >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status =
> > > >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  if
> > > >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> > > >> +    return NULL;
> > > >> +  }
> > > >> +
> > > >> +  Table = NULL;
> > > >> +
> > > >> +  //
> > > >> +  // Search XSDT
> > > >> +  //
> > > >> +  if (Rsdp->Revision >=
> > > >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> > > >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> > > Rsdp->XsdtAddress;
> > > >> +    if (Signature ==
> > > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > >> +      //
> > > >> +      // It is to find FACS ACPI table,
> > > >> +      // need find FADT first.
> > > >> +      //
> > > >> +      Fadt = ScanTableInXSDT (Xsdt,
> > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > >> +      Table = FindAcpiFacsFromFadt (Fadt);
> > > >> +    } else {
> > > >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> > > >> +    }
> > > >> +  }
> > > >> +
> > > >> +  if (Table != NULL) {
> > > >> +    return Table;
> > > >> +  }
> > > >> +
> > > >> +  //
> > > >> +  // Search RSDT
> > > >> +  //
> > > >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp-
> > >RsdtAddress;
> > > >> + if (Signature ==
> > > >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > >> +    //
> > > >> +    // It is to find FACS ACPI table,
> > > >> +    // need find FADT first.
> > > >> +    //
> > > >> +    Fadt = ScanTableInRSDT (Rsdt,
> > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> > > >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> > > >> +
> > > >> +  return Table;
> > > >> +}
> > > >> +
> > > >> +/**
> > > >> +  This function finds ACPI table by signature.
> > > >> +  It will find the table in gEfiAcpi20TableGuid system
> > > >> +configuration table
> > > first,
> > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > >> +
> > > >> +  @param Signature  ACPI table signature.
> > > >> +
> > > >> +  @return ACPI table or NULL if not found.
> > > >> +
> > > >> +**/
> > > >> +VOID *
> > > >> +EFIAPI
> > > >> +EfiFindAcpiTableBySignature (
> > > >> +  IN UINT32     Signature
> > > >> +  )
> > > >> +{
> > > >> +  VOID          *Table;
> > > >> +
> > > >> +  Table = FindAcpiTableInAcpiConfigurationTable
> > > >> + (&gEfiAcpi20TableGuid,
> > > >> Signature);
> > > >> +  if (Table != NULL) {
> > > >> +    return Table;
> > > >> +  }
> > > >> +
> > > >> +  return FindAcpiTableInAcpiConfigurationTable
> > > >> + (&gEfiAcpi10TableGuid,
> > > >> Signature);
> > > >> +}
> > > >> +
> > > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > > >> b/MdePkg/Library/UefiLib/UefiLib.inf
> > > >> index a6c739ef3d6d..aea20fe67153 100644
> > > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > > >> @@ -41,6 +41,7 @@ [Sources]
> > > >>  Console.c
> > > >>  UefiLib.c
> > > >>  UefiLibInternal.h
> > > >> +  Acpi.c
> > > >>
> > > >>
> > > >> [Packages]
> > > >> @@ -62,6 +63,8 @@ [Guids]
> > > >>  gEfiEventReadyToBootGuid                      ##
> > > >> SOMETIMES_CONSUMES  ## Event
> > > >>  gEfiEventLegacyBootGuid                       ##
> > > >> SOMETIMES_CONSUMES  ## Event
> > > >>  gEfiGlobalVariableGuid                        ##
> > > >> SOMETIMES_CONSUMES  ## Variable
> > > >> +  gEfiAcpi20TableGuid                           ##
> > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > >> +  gEfiAcpi10TableGuid                           ##
> > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > >>
> > > >> [Protocols]
> > > >>  gEfiDriverBindingProtocolGuid                   ##
> > > >> SOMETIMES_PRODUCES
> > > >> --
> > > >> 2.7.0.windows.1
> > > >>
> > > >> _______________________________________________
> > > >> edk2-devel mailing list
> > > >> edk2-devel@lists.01.org
> > > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Ni, Ruiyu]

I prefer S2.
Single interface is more easy for consumer to remember how to use.

Thanks/Ray

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Monday, September 3, 2018 2:15 PM
> To: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> I prefer S1.
> I believe that the EfiLocateNextAcpiTable() can also be used in S1.
> 
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Monday, September 3, 2018 2:12 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: edk2-devel@lists.01.org; Younas khan
> <pmdyounaskhan786@gmail.com>;
> > Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > Thanks.
> > Ok, please help and we can have good and flexible interface(s) for
> > both producer and consumer.
> >
> > First, there are two cases we need to consider.
> > 1. Single table, like FADT, FACS, DSDT, etc.
> > 2. Multiple tables, like SSDT, etc.
> >
> > Then, we have two solutions.
> > S1:
> > One interface for single table case, consumer only needs input
> > Signature, has no need input previous returned Table. Like
> GetFirstGuidHob?
> > One interface for multiple table case, consumer needs input Signature
> > and previous returned Table. Like GetNextGuidHob? Could we add it
> > later based on real request?
> > S2:
> > One interface for both single table and multiple table cases, consumer
> > needs input Signature and previous returned Table. Does consumer need
> > input the table header stored in configuration table by itself(getting
> configuration table)?
> >
> >
> > If we like S2, the interface should be like below?
> >
> > /**
> >   This function locates next ACPI table based on Signature and
> > previous returned Table.
> >   If the input previous returned Table is NULL, this function will locate next
> table
> >   in gEfiAcpi20TableGuid system configuration table first, and then
> > gEfiAcpi10TableGuid
> >   system configuration table.
> >   If the input previous returned Table is not NULL and could be located in
> >   gEfiAcpi20TableGuid system configuration table, this function will
> > just locate next
> >   table in gEfiAcpi20TableGuid system configuration table, otherwise
> > gEfiAcpi10TableGuid
> >   system configuration table.
> >
> >   @param Signature  ACPI table signature.
> >   @param Table      Pointer to previous returned Table, or NULL to locate
> > first table.
> >
> >   @return Next ACPI table or NULL if not found.
> >
> > **/
> > EFI_ACPI_COMMON_HEADER *
> > EFIAPI
> > EfiLocateNextAcpiTable (
> >   IN UINT32                  Signature,
> >   IN EFI_ACPI_COMMON_HEADER  *Table
> >   )
> >
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Monday, September 3, 2018 1:09 PM
> > To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: edk2-devel@lists.01.org; Younas khan
> <pmdyounaskhan786@gmail.com>;
> > Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > That's fine to be in UefiLib. It's already a combo library.
> >
> > But I do recommend we think about how to handle multiple tables with
> > same signature.
> > When we are adding new APIs, we not only need to evaluate the existing
> > real case, but also we need to generalize the real cases and try to
> > think about the more flexible interface.
> >
> >
> > Thanks/Ray
> >
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Monday, September 3, 2018 11:26 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu
> > > <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: edk2-devel@lists.01.org; Younas khan
> > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>;
> > > Zeng, Star <star.zeng@intel.com>
> > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > EfiFindAcpiTableBySignature() API
> > >
> > > Good idea.
> > >
> > > I did consider DSDT and multiple SSDTs cases. But I did not find any
> > > real case for them.
> > > So I made the code simply for current cases, and the code can be
> > > easily enhanced later for DSDT, a new API can be added later for
> > > multiple
> > SSDTs.
> > >
> > > About adding the new API in UefiLib VS new library class, I did also
> > > consider it and even created code for new library class
> > > (git@github.com:lzeng14/edk2.git branch
> > > FindAcpiTableBySignature_UefiAcpiTableLib).
> > > But I remember I did discuss it with Mike and Jiewen, we recommended
> > > UefiLib as it will do not require any platform change.
> > >
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: Yao, Jiewen
> > > Sent: Saturday, September 1, 2018 7:04 AM
> > > To: Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org;
> > > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > EfiFindAcpiTableBySignature() API
> > >
> > > Good idea on LocateNextAcpiTable().
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ruiyu
> > > > Sent: Saturday, September 1, 2018 12:29 AM
> > > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org;
> > > > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > > > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > > EfiFindAcpiTableBySignature() API
> > > >
> > > > I think LocateNextAcpiTable() is more proper to handle the
> > > > multiple tables with same signature. It will carry three
> > > > parameters, one is the table header stored in configuration table,
> > > > one is the signature, another is
> > > the previous located table.
> > > > Can we return a common table header other than void*?
> > > >
> > > > Is there better place other than UefiLib?
> > > > Do we need to add a new library class like AcpiLib?
> > > >
> > > > 发自我的 iPhone
> > > >
> > > > > 在 2018年8月31日，下午8:00，Yao, Jiewen
> <jiewen.yao@intel.com>
> > > 写
> > > > 道：
> > > > >
> > > > > Good enhancement.
> > > > >
> > > > > I have 2 additional thought:
> > > > >
> > > > > 1) How to handle DSDT?
> > > > > We have special code to handle FACS, but no DSDT.
> > > > >
> > > > > 2) How to handle SSDT or other multiple ACPI tables?
> > > > > We may have multiple SSDT. Usually, it is identified as OEMID.
> > > > > Do we want to provide similar function for them?
> > > > >
> > > > > Anyway, just *additional* thought. :-) Current implementation is
> > > > > good enough for check in.
> > > > >
> > > > > Reviewed-by: Jiewen.yao@intel.com
> > > > >
> > > > > Thank you
> > > > > Yao Jiewen
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > >> Behalf Of
> > > > Star
> > > > >> Zeng
> > > > >> Sent: Friday, August 31, 2018 7:29 PM
> > > > >> To: edk2-devel@lists.01.org
> > > > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> > > > >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen
> > > <jiewen.yao@intel.com>;
> > > > Gao,
> > > > >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > > >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > > >> EfiFindAcpiTableBySignature() API
> > > > >>
> > > > >> 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 adds new EfiFindAcpiTableBySignature() API in
> > > > >> UefiLib for the request and also the following patch to 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>
> > > > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > > > >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > > >> ---
> > > > >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> > > > >> MdePkg/Library/UefiLib/Acpi.c      | 226
> > > > >> +++++++++++++++++++++++++++++++++++++
> > > > >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> > > > >> 3 files changed, 246 insertions(+) create mode 100644
> > > > >> MdePkg/Library/UefiLib/Acpi.c
> > > > >>
> > > > >> diff --git a/MdePkg/Include/Library/UefiLib.h
> > > > >> b/MdePkg/Include/Library/UefiLib.h
> > > > >> index f80067f11103..8dd25f324fd2 100644
> > > > >> --- a/MdePkg/Include/Library/UefiLib.h
> > > > >> +++ b/MdePkg/Include/Library/UefiLib.h
> > > > >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> > > > >>  IN     UINT64                    OpenMode,
> > > > >>  IN     UINT64                    Attributes
> > > > >>  );
> > > > >> +
> > > > >> +/**
> > > > >> +  This function finds ACPI table by signature.
> > > > >> +  It will find the table in gEfiAcpi20TableGuid system
> > > > >> +configuration table
> > > > first,
> > > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > > >> +
> > > > >> +  @param Signature  ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +EFIAPI
> > > > >> +EfiFindAcpiTableBySignature (
> > > > >> +  IN UINT32     Signature
> > > > >> +  );
> > > > >> +
> > > > >> #endif
> > > > >> diff --git a/MdePkg/Library/UefiLib/Acpi.c
> > > > >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index
> > > > >> 000000000000..5cb93966b59f
> > > > >> --- /dev/null
> > > > >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> > > > >> @@ -0,0 +1,226 @@
> > > > >> +/** @file
> > > > >> +  This module provides help function for finding ACPI table.
> > > > >> +
> > > > >> +  Copyright (c) 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.
> > > > >> +
> > > > >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN
> "AS
> > > IS"
> > > > >> BASIS,
> > > > >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER
> > > > >> EXPRESS OR IMPLIED.
> > > > >> +
> > > > >> +**/
> > > > >> +
> > > > >> +#include "UefiLibInternal.h"
> > > > >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> > > > >> +
> > > > >> +/**
> > > > >> +  This function scans ACPI table in RSDT.
> > > > >> +
> > > > >> +  @param Rsdt       ACPI RSDT
> > > > >> +  @param Signature  ACPI table signature
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +ScanTableInRSDT (
> > > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> > > > >> +  IN UINT32                         Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  UINTN                              Index;
> > > > >> +  UINT32                             EntryCount;
> > > > >> +  UINT32                             *EntryPtr;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> > > > >> +
> > > > >> +  if (Rsdt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  EntryCount = (Rsdt->Length - sizeof
> > > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > > >> sizeof(UINT32);
> > > > >> +
> > > > >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index <
> > > > >> + EntryCount; Index ++, EntryPtr ++) {
> > > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER
> *)((UINTN)(*EntryPtr));
> > > > >> +    if (Table->Signature == Signature) {
> > > > >> +      return Table;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  return NULL;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  This function scans ACPI table in XSDT.
> > > > >> +
> > > > >> +  @param Xsdt       ACPI XSDT
> > > > >> +  @param Signature  ACPI table signature
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +ScanTableInXSDT (
> > > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> > > > >> +  IN UINT32                         Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  UINTN                          Index;
> > > > >> +  UINT32                         EntryCount;
> > > > >> +  UINT64                         EntryPtr;
> > > > >> +  UINTN                          BasePtr;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> > > > >> +
> > > > >> +  if (Xsdt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  EntryCount = (Xsdt->Length - sizeof
> > > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > > >> sizeof(UINT64);
> > > > >> +
> > > > >> +  BasePtr = (UINTN)(Xsdt + 1);  for (Index = 0; Index <
> > > > >> + EntryCount; Index ++) {
> > > > >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index *
> > > > >> + sizeof(UINT64)),
> > > > >> sizeof(UINT64));
> > > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> > > > >> +    if (Table->Signature == Signature) {
> > > > >> +      return Table;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  return NULL;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  To find Facs in FADT.
> > > > >> +
> > > > >> +  @param Fadt   FADT table pointer
> > > > >> +
> > > > >> +  @return Facs table pointer or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> > > > >> +FindAcpiFacsFromFadt (
> > > > >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> > > > >> +  )
> > > > >> +{
> > > > >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> > > > >> +  UINT64                                        Data64;
> > > > >> +
> > > > >> +  if (Fadt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  if (Fadt->Header.Revision <
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> > > > >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > > >> +  } else {
> > > > >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> > > > >> +    if (Data64 != 0) {
> > > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Data64;
> > > > >> +    } else {
> > > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > > >> +    }
> > > > >> +  }
> > > > >> +  return Facs;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  To find ACPI table in ACPI ConfigurationTable.
> > > > >> +
> > > > >> +  @param AcpiTableGuid  The guid used to find ACPI
> > > ConfigurationTable.
> > > > >> +  @param Signature      ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID  *
> > > > >> +FindAcpiTableInAcpiConfigurationTable (
> > > > >> +  IN EFI_GUID   *AcpiGuid,
> > > > >> +  IN UINT32     Signature
> > > > >> +
> > > > >> +  )
> > > > >> +{
> > > > >> +  EFI_STATUS                                    Status;
> > > > >> +  VOID                                          *Table;
> > > > >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> > > > >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> > > > >> +
> > > > >> +  Rsdp = NULL;
> > > > >> +  //
> > > > >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status =
> > > > >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  if
> > > > >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  Table = NULL;
> > > > >> +
> > > > >> +  //
> > > > >> +  // Search XSDT
> > > > >> +  //
> > > > >> +  if (Rsdp->Revision >=
> > > > >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> > > > >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> > > > Rsdp->XsdtAddress;
> > > > >> +    if (Signature ==
> > > > >>
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > > >> +      //
> > > > >> +      // It is to find FACS ACPI table,
> > > > >> +      // need find FADT first.
> > > > >> +      //
> > > > >> +      Fadt = ScanTableInXSDT (Xsdt,
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > > >> +      Table = FindAcpiFacsFromFadt (Fadt);
> > > > >> +    } else {
> > > > >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  if (Table != NULL) {
> > > > >> +    return Table;
> > > > >> +  }
> > > > >> +
> > > > >> +  //
> > > > >> +  // Search RSDT
> > > > >> +  //
> > > > >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp-
> > > >RsdtAddress;
> > > > >> + if (Signature ==
> > > > >>
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > > >> +    //
> > > > >> +    // It is to find FACS ACPI table,
> > > > >> +    // need find FADT first.
> > > > >> +    //
> > > > >> +    Fadt = ScanTableInRSDT (Rsdt,
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > > >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> > > > >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> > > > >> +
> > > > >> +  return Table;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  This function finds ACPI table by signature.
> > > > >> +  It will find the table in gEfiAcpi20TableGuid system
> > > > >> +configuration table
> > > > first,
> > > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > > >> +
> > > > >> +  @param Signature  ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +EFIAPI
> > > > >> +EfiFindAcpiTableBySignature (
> > > > >> +  IN UINT32     Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  VOID          *Table;
> > > > >> +
> > > > >> +  Table = FindAcpiTableInAcpiConfigurationTable
> > > > >> + (&gEfiAcpi20TableGuid,
> > > > >> Signature);
> > > > >> +  if (Table != NULL) {
> > > > >> +    return Table;
> > > > >> +  }
> > > > >> +
> > > > >> +  return FindAcpiTableInAcpiConfigurationTable
> > > > >> + (&gEfiAcpi10TableGuid,
> > > > >> Signature);
> > > > >> +}
> > > > >> +
> > > > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> b/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> index a6c739ef3d6d..aea20fe67153 100644
> > > > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> @@ -41,6 +41,7 @@ [Sources]
> > > > >>  Console.c
> > > > >>  UefiLib.c
> > > > >>  UefiLibInternal.h
> > > > >> +  Acpi.c
> > > > >>
> > > > >>
> > > > >> [Packages]
> > > > >> @@ -62,6 +63,8 @@ [Guids]
> > > > >>  gEfiEventReadyToBootGuid                      ##
> > > > >> SOMETIMES_CONSUMES  ## Event
> > > > >>  gEfiEventLegacyBootGuid                       ##
> > > > >> SOMETIMES_CONSUMES  ## Event
> > > > >>  gEfiGlobalVariableGuid                        ##
> > > > >> SOMETIMES_CONSUMES  ## Variable
> > > > >> +  gEfiAcpi20TableGuid                           ##
> > > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > > >> +  gEfiAcpi10TableGuid                           ##
> > > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > > >>
> > > > >> [Protocols]
> > > > >>  gEfiDriverBindingProtocolGuid                   ##
> > > > >> SOMETIMES_PRODUCES
> > > > >> --
> > > > >> 2.7.0.windows.1
> > > > >>
> > > > >> _______________________________________________
> > > > >> edk2-devel mailing list
> > > > >> edk2-devel@lists.01.org
> > > > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > edk2-devel@lists.01.org
> > > > > https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

[Zeng, Star]

After offline discussion with Jiewen and Ray, we agree to follow S1 that will make the consumer easier for single ACPI table case.
I will update patches and send new V2 series.

Thanks for all the feedbacks.

Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, September 3, 2018 2:33 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API

I prefer S2.
Single interface is more easy for consumer to remember how to use.

Thanks/Ray

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Monday, September 3, 2018 2:15 PM
> To: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; 
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Younas khan <pmdyounaskhan786@gmail.com>; 
> Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
> 
> I prefer S1.
> I believe that the EfiLocateNextAcpiTable() can also be used in S1.
> 
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Monday, September 3, 2018 2:12 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen 
> > <jiewen.yao@intel.com>; Kinney, Michael D 
> > <michael.d.kinney@intel.com>
> > Cc: edk2-devel@lists.01.org; Younas khan
> <pmdyounaskhan786@gmail.com>;
> > Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > Thanks.
> > Ok, please help and we can have good and flexible interface(s) for 
> > both producer and consumer.
> >
> > First, there are two cases we need to consider.
> > 1. Single table, like FADT, FACS, DSDT, etc.
> > 2. Multiple tables, like SSDT, etc.
> >
> > Then, we have two solutions.
> > S1:
> > One interface for single table case, consumer only needs input 
> > Signature, has no need input previous returned Table. Like
> GetFirstGuidHob?
> > One interface for multiple table case, consumer needs input 
> > Signature and previous returned Table. Like GetNextGuidHob? Could we 
> > add it later based on real request?
> > S2:
> > One interface for both single table and multiple table cases, 
> > consumer needs input Signature and previous returned Table. Does 
> > consumer need input the table header stored in configuration table 
> > by itself(getting
> configuration table)?
> >
> >
> > If we like S2, the interface should be like below?
> >
> > /**
> >   This function locates next ACPI table based on Signature and 
> > previous returned Table.
> >   If the input previous returned Table is NULL, this function will 
> > locate next
> table
> >   in gEfiAcpi20TableGuid system configuration table first, and then 
> > gEfiAcpi10TableGuid
> >   system configuration table.
> >   If the input previous returned Table is not NULL and could be located in
> >   gEfiAcpi20TableGuid system configuration table, this function will 
> > just locate next
> >   table in gEfiAcpi20TableGuid system configuration table, otherwise 
> > gEfiAcpi10TableGuid
> >   system configuration table.
> >
> >   @param Signature  ACPI table signature.
> >   @param Table      Pointer to previous returned Table, or NULL to locate
> > first table.
> >
> >   @return Next ACPI table or NULL if not found.
> >
> > **/
> > EFI_ACPI_COMMON_HEADER *
> > EFIAPI
> > EfiLocateNextAcpiTable (
> >   IN UINT32                  Signature,
> >   IN EFI_ACPI_COMMON_HEADER  *Table
> >   )
> >
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Monday, September 3, 2018 1:09 PM
> > To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen 
> > <jiewen.yao@intel.com>; Kinney, Michael D 
> > <michael.d.kinney@intel.com>
> > Cc: edk2-devel@lists.01.org; Younas khan
> <pmdyounaskhan786@gmail.com>;
> > Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > EfiFindAcpiTableBySignature() API
> >
> > That's fine to be in UefiLib. It's already a combo library.
> >
> > But I do recommend we think about how to handle multiple tables with 
> > same signature.
> > When we are adding new APIs, we not only need to evaluate the 
> > existing real case, but also we need to generalize the real cases 
> > and try to think about the more flexible interface.
> >
> >
> > Thanks/Ray
> >
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Monday, September 3, 2018 11:26 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu 
> > > <ruiyu.ni@intel.com>; Kinney, Michael D 
> > > <michael.d.kinney@intel.com>
> > > Cc: edk2-devel@lists.01.org; Younas khan 
> > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>; 
> > > Zeng, Star <star.zeng@intel.com>
> > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > EfiFindAcpiTableBySignature() API
> > >
> > > Good idea.
> > >
> > > I did consider DSDT and multiple SSDTs cases. But I did not find 
> > > any real case for them.
> > > So I made the code simply for current cases, and the code can be 
> > > easily enhanced later for DSDT, a new API can be added later for 
> > > multiple
> > SSDTs.
> > >
> > > About adding the new API in UefiLib VS new library class, I did 
> > > also consider it and even created code for new library class 
> > > (git@github.com:lzeng14/edk2.git branch 
> > > FindAcpiTableBySignature_UefiAcpiTableLib).
> > > But I remember I did discuss it with Mike and Jiewen, we 
> > > recommended UefiLib as it will do not require any platform change.
> > >
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: Yao, Jiewen
> > > Sent: Saturday, September 1, 2018 7:04 AM
> > > To: Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; 
> > > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan 
> > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > > Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > EfiFindAcpiTableBySignature() API
> > >
> > > Good idea on LocateNextAcpiTable().
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ruiyu
> > > > Sent: Saturday, September 1, 2018 12:29 AM
> > > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; 
> > > > Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan 
> > > > <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> > > > Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > > EfiFindAcpiTableBySignature() API
> > > >
> > > > I think LocateNextAcpiTable() is more proper to handle the 
> > > > multiple tables with same signature. It will carry three 
> > > > parameters, one is the table header stored in configuration 
> > > > table, one is the signature, another is
> > > the previous located table.
> > > > Can we return a common table header other than void*?
> > > >
> > > > Is there better place other than UefiLib?
> > > > Do we need to add a new library class like AcpiLib?
> > > >
> > > > 发自我的 iPhone
> > > >
> > > > > 在 2018年8月31日，下午8:00，Yao, Jiewen
> <jiewen.yao@intel.com>
> > > 写
> > > > 道：
> > > > >
> > > > > Good enhancement.
> > > > >
> > > > > I have 2 additional thought:
> > > > >
> > > > > 1) How to handle DSDT?
> > > > > We have special code to handle FACS, but no DSDT.
> > > > >
> > > > > 2) How to handle SSDT or other multiple ACPI tables?
> > > > > We may have multiple SSDT. Usually, it is identified as OEMID.
> > > > > Do we want to provide similar function for them?
> > > > >
> > > > > Anyway, just *additional* thought. :-) Current implementation 
> > > > > is good enough for check in.
> > > > >
> > > > > Reviewed-by: Jiewen.yao@intel.com
> > > > >
> > > > > Thank you
> > > > > Yao Jiewen
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On 
> > > > >> Behalf Of
> > > > Star
> > > > >> Zeng
> > > > >> Sent: Friday, August 31, 2018 7:29 PM
> > > > >> To: edk2-devel@lists.01.org
> > > > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas 
> > > > >> khan <pmdyounaskhan786@gmail.com>; Yao, Jiewen
> > > <jiewen.yao@intel.com>;
> > > > Gao,
> > > > >> Liming <liming.gao@intel.com>; Zeng, Star 
> > > > >> <star.zeng@intel.com>
> > > > >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> > > > >> EfiFindAcpiTableBySignature() API
> > > > >>
> > > > >> 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 adds new EfiFindAcpiTableBySignature() API in 
> > > > >> UefiLib for the request and also the following patch to 
> > > > >> 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>
> > > > >> Contributed-under: TianoCore Contribution Agreement 1.1
> > > > >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > > >> ---
> > > > >> MdePkg/Include/Library/UefiLib.h   |  17 +++
> > > > >> MdePkg/Library/UefiLib/Acpi.c      | 226
> > > > >> +++++++++++++++++++++++++++++++++++++
> > > > >> MdePkg/Library/UefiLib/UefiLib.inf |   3 +
> > > > >> 3 files changed, 246 insertions(+) create mode 100644 
> > > > >> MdePkg/Library/UefiLib/Acpi.c
> > > > >>
> > > > >> diff --git a/MdePkg/Include/Library/UefiLib.h
> > > > >> b/MdePkg/Include/Library/UefiLib.h
> > > > >> index f80067f11103..8dd25f324fd2 100644
> > > > >> --- a/MdePkg/Include/Library/UefiLib.h
> > > > >> +++ b/MdePkg/Include/Library/UefiLib.h
> > > > >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> > > > >>  IN     UINT64                    OpenMode,
> > > > >>  IN     UINT64                    Attributes
> > > > >>  );
> > > > >> +
> > > > >> +/**
> > > > >> +  This function finds ACPI table by signature.
> > > > >> +  It will find the table in gEfiAcpi20TableGuid system 
> > > > >> +configuration table
> > > > first,
> > > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > > >> +
> > > > >> +  @param Signature  ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +EFIAPI
> > > > >> +EfiFindAcpiTableBySignature (
> > > > >> +  IN UINT32     Signature
> > > > >> +  );
> > > > >> +
> > > > >> #endif
> > > > >> diff --git a/MdePkg/Library/UefiLib/Acpi.c 
> > > > >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index 
> > > > >> 000000000000..5cb93966b59f
> > > > >> --- /dev/null
> > > > >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> > > > >> @@ -0,0 +1,226 @@
> > > > >> +/** @file
> > > > >> +  This module provides help function for finding ACPI table.
> > > > >> +
> > > > >> +  Copyright (c) 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.
> > > > >> +
> > > > >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN
> "AS
> > > IS"
> > > > >> BASIS,
> > > > >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER
> > > > >> EXPRESS OR IMPLIED.
> > > > >> +
> > > > >> +**/
> > > > >> +
> > > > >> +#include "UefiLibInternal.h"
> > > > >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> > > > >> +
> > > > >> +/**
> > > > >> +  This function scans ACPI table in RSDT.
> > > > >> +
> > > > >> +  @param Rsdt       ACPI RSDT
> > > > >> +  @param Signature  ACPI table signature
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +ScanTableInRSDT (
> > > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
> > > > >> +  IN UINT32                         Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  UINTN                              Index;
> > > > >> +  UINT32                             EntryCount;
> > > > >> +  UINT32                             *EntryPtr;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER        *Table;
> > > > >> +
> > > > >> +  if (Rsdt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  EntryCount = (Rsdt->Length - sizeof
> > > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > > >> sizeof(UINT32);
> > > > >> +
> > > > >> +  EntryPtr = (UINT32 *)(Rsdt + 1);  for (Index = 0; Index < 
> > > > >> + EntryCount; Index ++, EntryPtr ++) {
> > > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER
> *)((UINTN)(*EntryPtr));
> > > > >> +    if (Table->Signature == Signature) {
> > > > >> +      return Table;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  return NULL;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  This function scans ACPI table in XSDT.
> > > > >> +
> > > > >> +  @param Xsdt       ACPI XSDT
> > > > >> +  @param Signature  ACPI table signature
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +ScanTableInXSDT (
> > > > >> +  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
> > > > >> +  IN UINT32                         Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  UINTN                          Index;
> > > > >> +  UINT32                         EntryCount;
> > > > >> +  UINT64                         EntryPtr;
> > > > >> +  UINTN                          BasePtr;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER    *Table;
> > > > >> +
> > > > >> +  if (Xsdt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  EntryCount = (Xsdt->Length - sizeof
> > > > >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> > > > >> sizeof(UINT64);
> > > > >> +
> > > > >> +  BasePtr = (UINTN)(Xsdt + 1);  for (Index = 0; Index < 
> > > > >> + EntryCount; Index ++) {
> > > > >> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * 
> > > > >> + sizeof(UINT64)),
> > > > >> sizeof(UINT64));
> > > > >> +    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> > > > >> +    if (Table->Signature == Signature) {
> > > > >> +      return Table;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  return NULL;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  To find Facs in FADT.
> > > > >> +
> > > > >> +  @param Fadt   FADT table pointer
> > > > >> +
> > > > >> +  @return Facs table pointer or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE * 
> > > > >> +FindAcpiFacsFromFadt (
> > > > >> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> > > > >> +  )
> > > > >> +{
> > > > >> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> > > > >> +  UINT64                                        Data64;
> > > > >> +
> > > > >> +  if (Fadt == NULL) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  if (Fadt->Header.Revision <
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> > > > >> +    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > > >> +  } else {
> > > > >> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> > > > >> +    if (Data64 != 0) {
> > > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Data64;
> > > > >> +    } else {
> > > > >> +      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> > > > >> *)(UINTN)Fadt->FirmwareCtrl;
> > > > >> +    }
> > > > >> +  }
> > > > >> +  return Facs;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  To find ACPI table in ACPI ConfigurationTable.
> > > > >> +
> > > > >> +  @param AcpiTableGuid  The guid used to find ACPI
> > > ConfigurationTable.
> > > > >> +  @param Signature      ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID  *
> > > > >> +FindAcpiTableInAcpiConfigurationTable (
> > > > >> +  IN EFI_GUID   *AcpiGuid,
> > > > >> +  IN UINT32     Signature
> > > > >> +
> > > > >> +  )
> > > > >> +{
> > > > >> +  EFI_STATUS                                    Status;
> > > > >> +  VOID                                          *Table;
> > > > >> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
> > > > >> +  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
> > > > >> +  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
> > > > >> +
> > > > >> +  Rsdp = NULL;
> > > > >> +  //
> > > > >> +  // Find ACPI ConfigurationTable (RSD_PTR)  //  Status = 
> > > > >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);  
> > > > >> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> > > > >> +    return NULL;
> > > > >> +  }
> > > > >> +
> > > > >> +  Table = NULL;
> > > > >> +
> > > > >> +  //
> > > > >> +  // Search XSDT
> > > > >> +  //
> > > > >> +  if (Rsdp->Revision >=
> > > > >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> > > > >> +    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> > > > Rsdp->XsdtAddress;
> > > > >> +    if (Signature ==
> > > > >>
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > > >> +      //
> > > > >> +      // It is to find FACS ACPI table,
> > > > >> +      // need find FADT first.
> > > > >> +      //
> > > > >> +      Fadt = ScanTableInXSDT (Xsdt,
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > > >> +      Table = FindAcpiFacsFromFadt (Fadt);
> > > > >> +    } else {
> > > > >> +      Table = ScanTableInXSDT (Xsdt, Signature);
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  if (Table != NULL) {
> > > > >> +    return Table;
> > > > >> +  }
> > > > >> +
> > > > >> +  //
> > > > >> +  // Search RSDT
> > > > >> +  //
> > > > >> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp-
> > > >RsdtAddress;
> > > > >> + if (Signature ==
> > > > >>
> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> > > > >> +    //
> > > > >> +    // It is to find FACS ACPI table,
> > > > >> +    // need find FADT first.
> > > > >> +    //
> > > > >> +    Fadt = ScanTableInRSDT (Rsdt,
> > > > >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> > > > >> +    Table = FindAcpiFacsFromFadt (Fadt);  } else {
> > > > >> +    Table = ScanTableInRSDT (Rsdt, Signature);  }
> > > > >> +
> > > > >> +  return Table;
> > > > >> +}
> > > > >> +
> > > > >> +/**
> > > > >> +  This function finds ACPI table by signature.
> > > > >> +  It will find the table in gEfiAcpi20TableGuid system 
> > > > >> +configuration table
> > > > first,
> > > > >> +  and then gEfiAcpi10TableGuid system configuration table.
> > > > >> +
> > > > >> +  @param Signature  ACPI table signature.
> > > > >> +
> > > > >> +  @return ACPI table or NULL if not found.
> > > > >> +
> > > > >> +**/
> > > > >> +VOID *
> > > > >> +EFIAPI
> > > > >> +EfiFindAcpiTableBySignature (
> > > > >> +  IN UINT32     Signature
> > > > >> +  )
> > > > >> +{
> > > > >> +  VOID          *Table;
> > > > >> +
> > > > >> +  Table = FindAcpiTableInAcpiConfigurationTable
> > > > >> + (&gEfiAcpi20TableGuid,
> > > > >> Signature);
> > > > >> +  if (Table != NULL) {
> > > > >> +    return Table;
> > > > >> +  }
> > > > >> +
> > > > >> +  return FindAcpiTableInAcpiConfigurationTable
> > > > >> + (&gEfiAcpi10TableGuid,
> > > > >> Signature);
> > > > >> +}
> > > > >> +
> > > > >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> b/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> index a6c739ef3d6d..aea20fe67153 100644
> > > > >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > > > >> @@ -41,6 +41,7 @@ [Sources]
> > > > >>  Console.c
> > > > >>  UefiLib.c
> > > > >>  UefiLibInternal.h
> > > > >> +  Acpi.c
> > > > >>
> > > > >>
> > > > >> [Packages]
> > > > >> @@ -62,6 +63,8 @@ [Guids]
> > > > >>  gEfiEventReadyToBootGuid                      ##
> > > > >> SOMETIMES_CONSUMES  ## Event
> > > > >>  gEfiEventLegacyBootGuid                       ##
> > > > >> SOMETIMES_CONSUMES  ## Event
> > > > >>  gEfiGlobalVariableGuid                        ##
> > > > >> SOMETIMES_CONSUMES  ## Variable
> > > > >> +  gEfiAcpi20TableGuid                           ##
> > > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > > >> +  gEfiAcpi10TableGuid                           ##
> > > > >> SOMETIMES_CONSUMES  ## SystemTable
> > > > >>
> > > > >> [Protocols]
> > > > >>  gEfiDriverBindingProtocolGuid                   ##
> > > > >> SOMETIMES_PRODUCES
> > > > >> --
> > > > >> 2.7.0.windows.1
> > > > >>
> > > > >> _______________________________________________
> > > > >> edk2-devel mailing list
> > > > >> edk2-devel@lists.01.org
> > > > >> https://lists.01.org/mailman/listinfo/edk2-devel
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > edk2-devel@lists.01.org
> > > > > https://lists.01.org/mailman/listinfo/edk2-devel

[PATCH 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiFindAcpiTableBySignature()

[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 IntelVTdDxe 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>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        | 136 +--------------------
 1 file changed, 3 insertions(+), 133 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 24723fe4972a..1e8c1f620c98 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -868,116 +868,6 @@ ParseDmarAcpiTableRmrr (
 }
 
 /**
-  This function scan ACPI table in RSDT.
-
-  @param[in]  Rsdt      ACPI RSDT
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
-  IN RSDT_TABLE                   *Rsdt,
-  IN UINT32                       Signature
-  )
-{
-  UINTN                         Index;
-  UINT32                        EntryCount;
-  UINT32                        *EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER   *Table;
-
-  EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
-  EntryPtr = &Rsdt->Entry;
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
-    if ((Table != NULL) && (Table->Signature == Signature)) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in XSDT.
-
-  @param[in]  Xsdt      ACPI XSDT
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
-  IN XSDT_TABLE                   *Xsdt,
-  IN UINT32                       Signature
-  )
-{
-  UINTN                        Index;
-  UINT32                       EntryCount;
-  UINT64                       EntryPtr;
-  UINTN                        BasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER  *Table;
-
-  EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
-  BasePtr = (UINTN)(&(Xsdt->Entry));
-  for (Index = 0; Index < EntryCount; Index ++) {
-    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
-    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
-    if ((Table != NULL) && (Table->Signature == Signature)) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in RSDP.
-
-  @param[in]  Rsdp      ACPI RSDP
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
-  IN EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
-  IN UINT32                                       Signature
-  )
-{
-  EFI_ACPI_DESCRIPTION_HEADER                    *AcpiTable;
-  RSDT_TABLE                                     *Rsdt;
-  XSDT_TABLE                                     *Xsdt;
-
-  AcpiTable = NULL;
-
-  //
-  // Check ACPI2.0 table
-  //
-  Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
-  Xsdt = NULL;
-  if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
-    Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
-  }
-  //
-  // Check Xsdt
-  //
-  if (Xsdt != NULL) {
-    AcpiTable = ScanTableInXSDT (Xsdt, Signature);
-  }
-  //
-  // Check Rsdt
-  //
-  if ((AcpiTable == NULL) && (Rsdt != NULL)) {
-    AcpiTable = ScanTableInRSDT (Rsdt, Signature);
-  }
-
-  return AcpiTable;
-}
-
-/**
   Get the DMAR ACPI table.
 
   @retval EFI_SUCCESS           The DMAR ACPI table is got.
@@ -989,33 +879,13 @@ GetDmarAcpiTable (
   VOID
   )
 {
-  VOID                              *AcpiTable;
-  EFI_STATUS                        Status;
-
   if (mAcpiDmarTable != NULL) {
     return EFI_ALREADY_STARTED;
   }
 
-  AcpiTable = NULL;
-  Status = EfiGetSystemConfigurationTable (
-             &gEfiAcpi20TableGuid,
-             &AcpiTable
-             );
-  if (EFI_ERROR (Status)) {
-    Status = EfiGetSystemConfigurationTable (
-               &gEfiAcpi10TableGuid,
-               &AcpiTable
-               );
-  }
-  if (EFI_ERROR (Status)) {
-    return EFI_NOT_FOUND;
-  }
-  ASSERT (AcpiTable != NULL);
-
-  mAcpiDmarTable = FindAcpiPtr (
-                      (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
-                      EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
-                      );
+  mAcpiDmarTable = (EFI_ACPI_DMAR_HEADER *) EfiFindAcpiTableBySignature (
+                                              EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
+                                              );
   if (mAcpiDmarTable == NULL) {
     return EFI_NOT_FOUND;
   }
-- 
2.7.0.windows.1

[PATCH 3/6] MdeModulePkg S3SaveStateDxe: Use new EfiFindAcpiTableBySignature()

[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 S3SaveStateDxe 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: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c        | 208 +--------------------
 .../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf         |   3 +-
 2 files changed, 5 insertions(+), 206 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
index 3f99023f110f..11f1890b5494 100644
--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
@@ -22,8 +22,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/LockBoxLib.h>
 #include <Library/PcdLib.h>
 #include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
 #include <Guid/AcpiS3Context.h>
-#include <Guid/Acpi.h>
 #include <IndustryStandard/Acpi.h>
 #include <Protocol/LockBox.h>
 
@@ -76,208 +76,6 @@ AllocateMemoryBelow4G (
 }
 
 /**
-
-  This function scan ACPI table in RSDT.
-
-  @param Rsdt      ACPI RSDT
-  @param Signature ACPI table signature
-
-  @return ACPI table
-
-**/
-VOID *
-ScanTableInRSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER    *Rsdt,
-  IN UINT32                         Signature
-  )
-{
-  UINTN                              Index;
-  UINT32                             EntryCount;
-  UINT32                             *EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER        *Table;
-
-  if (Rsdt == NULL) {
-    return NULL;
-  }
-
-  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
-  EntryPtr = (UINT32 *)(Rsdt + 1);
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
-    if (Table->Signature == Signature) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-
-  This function scan ACPI table in XSDT.
-
-  @param Xsdt      ACPI XSDT
-  @param Signature ACPI table signature
-
-  @return ACPI table
-
-**/
-VOID *
-ScanTableInXSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER    *Xsdt,
-  IN UINT32                         Signature
-  )
-{
-  UINTN                          Index;
-  UINT32                         EntryCount;
-  UINT64                         EntryPtr;
-  UINTN                          BasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER    *Table;
-
-  if (Xsdt == NULL) {
-    return NULL;
-  }
-
-  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
-  BasePtr = (UINTN)(Xsdt + 1);
-  for (Index = 0; Index < EntryCount; Index ++) {
-    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
-    Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
-    if (Table->Signature == Signature) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  To find Facs in FADT.
-
-  @param Fadt   FADT table pointer
-
-  @return  Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *
-FindAcpiFacsFromFadt (
-  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt
-  )
-{
-  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
-  UINT64                                        Data64;
-
-  if (Fadt == NULL) {
-    return NULL;
-  }
-
-  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
-    Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
-  } else {
-    if (Fadt->FirmwareCtrl != 0) {
-      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
-    } else {
-      CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
-      Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Data64;
-    }
-  }
-  return Facs;
-}
-
-/**
-  To find Facs in Acpi tables.
-
-  To find Firmware ACPI control strutcure in Acpi Tables since the S3 waking vector is stored
-  in the table.
-
-  @param AcpiTableGuid   The guid used to find ACPI table in UEFI ConfigurationTable.
-
-  @return  Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *
-FindAcpiFacsTableByAcpiGuid (
-  IN EFI_GUID  *AcpiTableGuid
-  )
-{
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
-  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
-  EFI_ACPI_DESCRIPTION_HEADER                   *Xsdt;
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
-  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
-  UINTN                                         Index;
-
-  Rsdp  = 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;
-  }
-
-  //
-  // Search XSDT
-  //
-  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
-    Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
-    Fadt = ScanTableInXSDT (Xsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
-    if (Fadt != NULL) {
-      Facs = FindAcpiFacsFromFadt (Fadt);
-      if (Facs != NULL) {
-        return Facs;
-      }
-    }
-  }
-
-  //
-  // Search RSDT
-  //
-  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
-  Fadt = ScanTableInRSDT (Rsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
-  if (Fadt != NULL) {
-    Facs = FindAcpiFacsFromFadt (Fadt);
-    if (Facs != NULL) {
-      return Facs;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  To find Facs in Acpi tables.
-
-  To find Firmware ACPI control strutcure in Acpi Tables since the S3 waking vector is stored
-  in the table.
-
-  @return  Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *
-FindAcpiFacsTable (
-  VOID
-  )
-{
-  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
-
-  Facs = FindAcpiFacsTableByAcpiGuid (&gEfiAcpi20TableGuid);
-  if (Facs != NULL) {
-    return Facs;
-  }
-
-  return FindAcpiFacsTableByAcpiGuid (&gEfiAcpi10TableGuid);
-}
-
-/**
   The function will check if long mode waking vector is supported.
 
   @param[in] Facs   Pointer to FACS table.
@@ -460,7 +258,9 @@ AcpiS3ContextSaveOnEndOfDxe (
   //
   // Get ACPI Table because we will save its position to variable
   //
-  Facs = (EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *) FindAcpiFacsTable ();
+  Facs = (EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *) EfiFindAcpiTableBySignature (
+                                                            EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE
+                                                            );
   AcpiS3Context->AcpiFacsTable = (EFI_PHYSICAL_ADDRESS) (UINTN) Facs;
   ASSERT (AcpiS3Context->AcpiFacsTable != 0);
 
diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
index 36ed80cc7995..2c01f4959933 100644
--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
@@ -49,12 +49,11 @@ [LibraryClasses]
   PcdLib
   HobLib
   LockBoxLib
+  UefiLib
 
 [Guids]
   gEfiAcpiVariableGuid                       ## PRODUCES  ## UNDEFINED # LockBox Save Data.
   gEfiAcpiS3ContextGuid                      ## PRODUCES  ## UNDEFINED # LockBox Save Data.
-  gEfiAcpi20TableGuid                        ## SOMETIMES_CONSUMES  ## SystemTable
-  gEfiAcpi10TableGuid                        ## SOMETIMES_CONSUMES  ## SystemTable
   gEfiEndOfDxeEventGroupGuid                 ## CONSUMES  ## Event
 
 [Protocols]
-- 
2.7.0.windows.1

[PATCH 4/6] PcAtChipsetPkg PcRtc: Use new EfiFindAcpiTableBySignature()

[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 PcatRealTimeClockRuntimeDxe 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>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 +---------------------
 1 file changed, 3 insertions(+), 77 deletions(-)

diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 2105acf35f7b..72daff719f29 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -1203,49 +1203,6 @@ IsWithinOneDay (
 }
 
 /**
-  This function find ACPI table with the specified signature in RSDT or XSDT.
-
-  @param Sdt              ACPI RSDT or XSDT.
-  @param Signature        ACPI table signature.
-  @param TablePointerSize Size of table pointer: 4 or 8.
-
-  @return ACPI table or NULL if not found.
-**/
-VOID *
-ScanTableInSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER    *Sdt,
-  IN UINT32                         Signature,
-  IN UINTN                          TablePointerSize
-  )
-{
-  UINTN                          Index;
-  UINTN                          EntryCount;
-  UINTN                          EntryBase;
-  EFI_ACPI_DESCRIPTION_HEADER    *Table;
-
-  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / TablePointerSize;
-
-  EntryBase = (UINTN) (Sdt + 1);
-  for (Index = 0; Index < EntryCount; Index++) {
-    //
-    // When TablePointerSize is 4 while sizeof (VOID *) is 8, make sure the upper 4 bytes are zero.
-    //
-    Table = 0;
-    CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), TablePointerSize);
-
-    if (Table == NULL) {
-      continue;
-    }
-
-    if (Table->Signature == Signature) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
   Get the century RTC address from the ACPI FADT table.
 
   @return  The century RTC address or 0 if not found.
@@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
   VOID
   )
 {
-  EFI_STATUS                                    Status;
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
 
-  Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) &Rsdp);
-  if (EFI_ERROR (Status)) {
-    Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, (VOID **) &Rsdp);
-  }
-
-  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
-    return 0;
-  }
-
-  Fadt = NULL;
-
-  //
-  // Find FADT in XSDT
-  //
-  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 0) {
-    Fadt = ScanTableInSDT (
-             (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
-             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
-             sizeof (UINTN)
-             );
-  }
-
-  //
-  // Find FADT in RSDT
-  //
-  if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
-    Fadt = ScanTableInSDT (
-             (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
-             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
-             sizeof (UINT32)
-             );
-  }
+  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiFindAcpiTableBySignature (
+                                                         EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+                                                         );
 
   if ((Fadt != NULL) &&
       (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)
-- 
2.7.0.windows.1

[PATCH 5/6] ShellPkg DpDynamicCommand: Use new EfiFindAcpiTableBySignature()

[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 DpDynamicCommand 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: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c      | 136 +--------------------
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h      |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni    |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf |   2 -
 .../DpDynamicCommand/DpDynamicCommand.inf          |   2 -
 5 files changed, 3 insertions(+), 139 deletions(-)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index 2c094b63c94a..3b9549c2be8e 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -118,116 +118,6 @@ DumpStatistics( void )
 }
 
 /**
-  This function scan ACPI table in RSDT.
-
-  @param  Rsdt        ACPI RSDT
-  @param  Signature   ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
-  IN RSDT_TABLE                   *Rsdt,
-  IN UINT32                       Signature
-  )
-{
-  UINTN                         Index;
-  UINT32                        EntryCount;
-  UINT32                        *EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER   *Table;
-
-  EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
-  EntryPtr = &Rsdt->Entry;
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
-    if (Table->Signature == Signature) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in XSDT.
-
-  @param  Xsdt       ACPI XSDT
-  @param  Signature  ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
-  IN XSDT_TABLE                   *Xsdt,
-  IN UINT32                       Signature
-  )
-{
-  UINTN                        Index;
-  UINT32                       EntryCount;
-  UINT64                       EntryPtr;
-  UINTN                        BasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER  *Table;
-
-  EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
-  BasePtr = (UINTN)(&(Xsdt->Entry));
-  for (Index = 0; Index < EntryCount; Index ++) {
-    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
-    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
-    if (Table->Signature == Signature) {
-      return Table;
-    }
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in RSDP.
-
-  @param  Rsdp       ACPI RSDP
-  @param  Signature  ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
-  IN EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
-  IN UINT32                                       Signature
-  )
-{
-  EFI_ACPI_DESCRIPTION_HEADER                    *AcpiTable;
-  RSDT_TABLE                                     *Rsdt;
-  XSDT_TABLE                                     *Xsdt;
-
-  AcpiTable = NULL;
-
-  //
-  // Check ACPI2.0 table
-  //
-  Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
-  Xsdt = NULL;
-  if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
-    Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
-  }
-  //
-  // Check Xsdt
-  //
-  if (Xsdt != NULL) {
-    AcpiTable = ScanTableInXSDT (Xsdt, Signature);
-  }
-  //
-  // Check Rsdt
-  //
-  if ((AcpiTable == NULL) && (Rsdt != NULL)) {
-    AcpiTable = ScanTableInRSDT (Rsdt, Signature);
-  }
-
-  return AcpiTable;
-}
-
-/**
   Get Boot performance table form Acpi table.
 
 **/
@@ -235,31 +125,11 @@ EFI_STATUS
 GetBootPerformanceTable (
   )
 {
-  EFI_STATUS                  Status;
-  VOID                        *AcpiTable;
   FIRMWARE_PERFORMANCE_TABLE  *FirmwarePerformanceTable;
 
-  AcpiTable = NULL;
-
-  Status = EfiGetSystemConfigurationTable (
-             &gEfiAcpi20TableGuid,
-             &AcpiTable
-             );
-  if (EFI_ERROR (Status)) {
-    Status = EfiGetSystemConfigurationTable (
-               &gEfiAcpi10TableGuid,
-               &AcpiTable
-                 );
-  }
-  if (EFI_ERROR(Status) || AcpiTable == NULL) {
-    ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_TABLE_FAIL), mDpHiiHandle);
-    return Status;
-  }
-
-  FirmwarePerformanceTable = FindAcpiPtr (
-                      (EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
-                      EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
-                      );
+  FirmwarePerformanceTable = (FIRMWARE_PERFORMANCE_TABLE *) EfiFindAcpiTableBySignature (
+                                                              EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
+                                                              );
   if (FirmwarePerformanceTable == NULL) {
     ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_FPDT_FAIL), mDpHiiHandle);
     return EFI_NOT_FOUND;
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
index 0e6e9422effc..db8472f6558f 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
@@ -21,7 +21,6 @@
 #include <Guid/Performance.h>
 #include <Guid/ExtendedFirmwarePerformance.h>
 #include <Guid/FirmwarePerformance.h>
-#include <Guid/Acpi.h>
 
 #include <Protocol/HiiPackageList.h>
 #include <Protocol/DevicePath.h>
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
index 1d6f25b8c252..7fc2b8d1c966 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
@@ -94,7 +94,6 @@
 #string STR_DP_INCOMPLETE              #language en-US  " I "
 #string STR_DP_COMPLETE                #language en-US  "   "
 #string STR_ALIT_UNKNOWN               #language en-US  "Unknown"
-#string STR_DP_GET_ACPI_TABLE_FAIL     #language en-US  "Fail to get ACPI Table\n"
 #string STR_DP_GET_ACPI_FPDT_FAIL      #language en-US  "Fail to get Firmware Performance Data Table (FPDT) in ACPI Table\n"
 
 #string STR_GET_HELP_DP         #language en-US ""
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
index cedb333b285c..3a35fca310b0 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
@@ -59,8 +59,6 @@ [LibraryClasses]
 
 [Guids]
   gPerformanceProtocolGuid                                ## CONSUMES ## SystemTable
-  gEfiAcpi20TableGuid                                     ## CONSUMES ## SystemTable
-  gEfiAcpi10TableGuid                                     ## CONSUMES ## SystemTable
 
 [Protocols]
   gEfiLoadedImageProtocolGuid                             ## CONSUMES
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
index 8fd3bbd5df83..62a3b5246500 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
@@ -60,8 +60,6 @@ [LibraryClasses]
 
 [Guids]
   gPerformanceProtocolGuid                                ## CONSUMES ## SystemTable
-  gEfiAcpi20TableGuid                                     ## CONSUMES
-  gEfiAcpi10TableGuid                                     ## CONSUMES
 
 [Protocols]
   gEfiLoadedImageProtocolGuid                             ## CONSUMES
-- 
2.7.0.windows.1

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

[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 <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>
-- 
2.7.0.windows.1

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

[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 <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

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

[Zeng, Star]

Thanks Laszlo. :)

Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Saturday, September 1, 2018 4:34 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Younas khan <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
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 <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