[PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs

[PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs

[Star Zeng]

It is the V2 patch series of
https://lists.01.org/pipermail/edk2-devel/2018-August/029214.html
It is according to the discussion at
https://lists.01.org/pipermail/edk2-devel/2018-September/029348.html

V2:
1. Add EfiLocateFirstAcpiTable() and EfiLocateNextAcpiTable() instead
   of EfiFindAcpiTableBySignature() to support locating both single
   ACPI table instance and multiple ACPI table instances cases.
2. Support locating DSDT.
3. Support locating multiple ACPI table instances case by
   EfiLocateNextAcpiTable().

Test done:
1. Call EfiLocateFirstAcpiTable() before ACPI configuration table is
   installed, NULL is returned.
2. Call EfiLocateFirstAcpiTable() to locate FACS after FACS is installed
   but FADT is not installed, NULL is returned.
3. Call EfiLocateFirstAcpiTable() to locate FADT/DSDT/FACS/FPDT/DMAR
   at late phase, correct ACPI table pointer is returned.
4. Call EfiLocateNextAcpiTable() to locate SSDTs at late phase, all
   SSDTs are returned correctly.
5. Run same test cases above after setting PcdAcpiExposedTableVersions
   to 0x2, same results are with above.

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

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 EfiLocateXXXAcpiTable() 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 EfiLocateXXXAcpiTable() APIs
  IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable()
  MdeModulePkg S3SaveStateDxe: Use new EfiLocateFirstAcpiTable()
  PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()
  ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()
  UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()

 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        | 136 +-----
 .../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c        | 208 +--------
 .../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf         |   3 +-
 MdePkg/Include/Library/UefiLib.h                   |  68 +++
 MdePkg/Library/UefiLib/Acpi.c                      | 488 +++++++++++++++++++++
 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, 579 insertions(+), 640 deletions(-)
 create mode 100644 MdePkg/Library/UefiLib/Acpi.c

-- 
2.7.0.windows.1

[PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

[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 EfiLocateXXXAcpiTable() APIs 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   |  68 ++++++
 MdePkg/Library/UefiLib/Acpi.c      | 488 +++++++++++++++++++++++++++++++++++++
 MdePkg/Library/UefiLib/UefiLib.inf |   3 +
 3 files changed, 559 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..cf82ff4a920a 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #ifndef __UEFI_LIB_H__
 #define __UEFI_LIB_H__
 
+#include <IndustryStandard/Acpi.h>
+
 #include <Protocol/DriverBinding.h>
 #include <Protocol/DriverConfiguration.h>
 #include <Protocol/ComponentName.h>
@@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
   IN     UINT64                    OpenMode,
   IN     UINT64                    Attributes
   );
+
+/**
+  This function locates next ACPI table in XSDT/RSDT based on Signature and
+  previous returned Table.
+
+  If PreviousTable is NULL:
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  If PreviousTable is not NULL:
+  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+     table, then this function will just locate next table in XSDT in
+     gEfiAcpi20TableGuid system configuration table.
+  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+     table, then this function will just locate next table in RSDT in
+     gEfiAcpi20TableGuid system configuration table.
+  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+     table, then this function will just locate next table in RSDT in
+     gEfiAcpi10TableGuid system configuration table.
+
+  If PreviousTable is not NULL and PreviousTable->Signature is not same with
+  Signature, then ASSERT.
+
+  @param Signature          ACPI table signature.
+  @param PreviousTable      Pointer to previous returned table to locate next
+                            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     *PreviousTable OPTIONAL
+  );
+
+/**
+  This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  @param Signature          ACPI table signature.
+
+  @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+  IN UINT32                     Signature
+  );
+
 #endif
diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
new file mode 100644
index 000000000000..0793bbdc787f
--- /dev/null
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -0,0 +1,488 @@
+/** @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.
+  @param PreviousTable          Pointer to previous returned table to locate
+                                next table, or NULL to locate first table.
+  @param PreviousTableLocated   Pointer to the indicator about whether the
+                                previous returned table could be located, or
+                                NULL if PreviousTable is NULL.
+
+  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+
+  @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+ScanTableInRSDT (
+  IN  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt,
+  IN  UINT32                        Signature,
+  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
+  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
+  )
+{
+  UINTN                             Index;
+  UINT32                            EntryCount;
+  UINT32                            *EntryPtr;
+  EFI_ACPI_COMMON_HEADER            *Table;
+
+  if (PreviousTableLocated != NULL) {
+    ASSERT (PreviousTable != NULL);
+    *PreviousTableLocated = FALSE;
+  } else {
+    ASSERT (PreviousTable == NULL);
+  }
+
+  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_COMMON_HEADER *)((UINTN)(*EntryPtr));
+    if (Table->Signature == Signature) {
+      if (PreviousTable != NULL) {
+        if (Table == PreviousTable) {
+          *PreviousTableLocated = TRUE;
+        } else if (*PreviousTableLocated) {
+          //
+          // Return next table.
+          //
+          return Table;
+        }
+      } else {
+        //
+        // Return first table.
+        //
+        return Table;
+      }
+    }
+  }
+
+  return NULL;
+}
+
+/**
+  This function scans ACPI table in XSDT.
+
+  @param Xsdt                   ACPI XSDT.
+  @param Signature              ACPI table signature.
+  @param PreviousTable          Pointer to previous returned table to locate
+                                next table, or NULL to locate first table.
+  @param PreviousTableLocated   Pointer to the indicator about whether the
+                                previous returned table could be located, or
+                                NULL if PreviousTable is NULL.
+
+  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+
+  @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+ScanTableInXSDT (
+  IN  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt,
+  IN  UINT32                        Signature,
+  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
+  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
+  )
+{
+  UINTN                             Index;
+  UINT32                            EntryCount;
+  UINT64                            EntryPtr;
+  UINTN                             BasePtr;
+  EFI_ACPI_COMMON_HEADER            *Table;
+
+  if (PreviousTableLocated != NULL) {
+    ASSERT (PreviousTable != NULL);
+    *PreviousTableLocated = FALSE;
+  } else {
+    ASSERT (PreviousTable == NULL);
+  }
+
+  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_COMMON_HEADER *)((UINTN)(EntryPtr));
+    if (Table->Signature == Signature) {
+      if (PreviousTable != NULL) {
+        if (Table == PreviousTable) {
+          *PreviousTableLocated = TRUE;
+        } else if (*PreviousTableLocated) {
+          //
+          // Return next table.
+          //
+          return Table;
+        }
+      } else {
+        //
+        // Return first table.
+        //
+        return Table;
+      }
+
+    }
+  }
+
+  return NULL;
+}
+
+/**
+  To locate FACS in FADT.
+
+  @param Fadt   FADT table pointer.
+
+  @return FACS table pointer or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiFacsFromFadt (
+  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
+  )
+{
+  EFI_ACPI_COMMON_HEADER                        *Facs;
+  UINT64                                        Data64;
+
+  if (Fadt == NULL) {
+    return NULL;
+  }
+
+  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
+    Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
+  } else {
+    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
+    if (Data64 != 0) {
+      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
+    } else {
+      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
+    }
+  }
+  return Facs;
+}
+
+/**
+  To locate DSDT in FADT.
+
+  @param Fadt   FADT table pointer.
+
+  @return DSDT table pointer or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiDsdtFromFadt (
+  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
+  )
+{
+  EFI_ACPI_COMMON_HEADER                        *Dsdt;
+  UINT64                                        Data64;
+
+  if (Fadt == NULL) {
+    return NULL;
+  }
+
+  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
+    Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
+  } else {
+    CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
+    if (Data64 != 0) {
+      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
+    } else {
+      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
+    }
+  }
+  return Dsdt;
+}
+
+/**
+  To locate ACPI table in ACPI ConfigurationTable.
+
+  @param AcpiTableGuid          The GUID used to get ACPI ConfigurationTable.
+  @param Signature              ACPI table signature.
+  @param PreviousTable          Pointer to previous returned table to locate
+                                next table, or NULL to locate first table.
+  @param PreviousTableLocated   Pointer to the indicator to return whether the
+                                previous returned table could be located or not,
+                                or NULL if PreviousTable is NULL.
+
+  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+  If AcpiGuid is NULL, then ASSERT().
+
+  @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiTableInAcpiConfigurationTable (
+  IN  EFI_GUID                  *AcpiGuid,
+  IN  UINT32                    Signature,
+  IN  EFI_ACPI_COMMON_HEADER    *PreviousTable, OPTIONAL
+  OUT BOOLEAN                   *PreviousTableLocated OPTIONAL
+  )
+{
+  EFI_STATUS                                    Status;
+  EFI_ACPI_COMMON_HEADER                        *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;
+
+  if (PreviousTableLocated != NULL) {
+    ASSERT (PreviousTable != NULL);
+    *PreviousTableLocated = FALSE;
+  } else {
+    ASSERT (PreviousTable == NULL);
+  }
+
+  Rsdp = NULL;
+  //
+  // Get 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_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+      ASSERT (PreviousTable == NULL);
+      //
+      // It is to locate DSDT,
+      // need to locate FADT first.
+      //
+      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
+               Xsdt,
+               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+               NULL,
+               NULL
+               );
+      Table = LocateAcpiDsdtFromFadt (Fadt);
+    } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+      ASSERT (PreviousTable == NULL);
+      //
+      // It is to locate FACS,
+      // need to locate FADT first.
+      //
+      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
+               Xsdt,
+               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+               NULL,
+               NULL
+               );
+      Table = LocateAcpiFacsFromFadt (Fadt);
+    } else {
+      Table = ScanTableInXSDT (
+                Xsdt,
+                Signature,
+                PreviousTable,
+                PreviousTableLocated
+                );
+    }
+  }
+
+  if (Table != NULL) {
+    return Table;
+  } else if ((PreviousTableLocated != NULL) &&
+              *PreviousTableLocated) {
+    //
+    // PreviousTable could be located in XSDT,
+    // but next table could not be located in XSDT.
+    //
+    return NULL;
+  }
+
+  //
+  // Search RSDT
+  //
+  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
+  if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+    ASSERT (PreviousTable == NULL);
+    //
+    // It is to locate DSDT,
+    // need to locate FADT first.
+    //
+    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
+             Rsdt,
+             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+             NULL,
+             NULL
+             );
+    Table = LocateAcpiDsdtFromFadt (Fadt);
+  } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+    ASSERT (PreviousTable == NULL);
+    //
+    // It is to locate FACS,
+    // need to locate FADT first.
+    //
+    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
+             Rsdt,
+             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+             NULL,
+             NULL
+             );
+    Table = LocateAcpiFacsFromFadt (Fadt);
+  } else {
+    Table = ScanTableInRSDT (
+              Rsdt,
+              Signature,
+              PreviousTable,
+              PreviousTableLocated
+              );
+  }
+
+  return Table;
+}
+
+/**
+  This function locates next ACPI table in XSDT/RSDT based on Signature and
+  previous returned Table.
+
+  If PreviousTable is NULL:
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  If PreviousTable is not NULL:
+  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+     table, then this function will just locate next table in XSDT in
+     gEfiAcpi20TableGuid system configuration table.
+  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+     table, then this function will just locate next table in RSDT in
+     gEfiAcpi20TableGuid system configuration table.
+  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+     table, then this function will just locate next table in RSDT in
+     gEfiAcpi10TableGuid system configuration table.
+
+  If PreviousTable is not NULL and PreviousTable->Signature is not same with
+  Signature, then ASSERT.
+
+  @param Signature          ACPI table signature.
+  @param PreviousTable      Pointer to previous returned table to locate next
+                            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     *PreviousTable OPTIONAL
+  )
+{
+  EFI_ACPI_COMMON_HEADER        *Table;
+  BOOLEAN                       TempPreviousTableLocated;
+  BOOLEAN                       *PreviousTableLocated;
+
+  if (PreviousTable != NULL) {
+    if (PreviousTable->Signature != Signature) {
+      //
+      // PreviousTable->Signature is not same with Signature.
+      //
+      ASSERT (FALSE);
+      return NULL;
+    } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
+               (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
+               (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
+      //
+      // There is only one FADT/DSDT/FACS table,
+      // so don't try to locate next one.
+      //
+      return NULL;
+    }
+
+    PreviousTableLocated = &TempPreviousTableLocated;
+    *PreviousTableLocated = FALSE;
+  } else {
+    PreviousTableLocated = NULL;
+  }
+
+  Table = LocateAcpiTableInAcpiConfigurationTable (
+            &gEfiAcpi20TableGuid,
+            Signature,
+            PreviousTable,
+            PreviousTableLocated
+            );
+  if (Table != NULL) {
+    return Table;
+  } else if ((PreviousTableLocated != NULL) &&
+              *PreviousTableLocated) {
+    //
+    // PreviousTable could be located in gEfiAcpi20TableGuid system
+    // configuration table, but next table could not be located in
+    // gEfiAcpi20TableGuid system configuration table.
+    //
+    return NULL;
+  }
+
+  return LocateAcpiTableInAcpiConfigurationTable (
+           &gEfiAcpi10TableGuid,
+           Signature,
+           PreviousTable,
+           PreviousTableLocated
+           );
+}
+
+/**
+  This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  @param Signature          ACPI table signature.
+
+  @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+  IN UINT32                     Signature
+  )
+{
+  return EfiLocateNextAcpiTable (Signature, NULL);
+}
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

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

[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
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
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..92b09f985533 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 *) EfiLocateFirstAcpiTable (
+                                              EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
+                                              );
   if (mAcpiDmarTable == NULL) {
     return EFI_NOT_FOUND;
   }
-- 
2.7.0.windows.1

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

[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
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: 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..fadafd2b608b 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 *) EfiLocateFirstAcpiTable (
+                                                            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 744cf8ab3270..e385356dee1b 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 V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()

[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
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
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..7965eb8aa55b 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 *) EfiLocateFirstAcpiTable (
+                                                         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 V2 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()

[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
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: 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..c14931055cbf 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 *) EfiLocateFirstAcpiTable (
+                                                              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 V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()

[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
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |  4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c         | 84 ++------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +-
 3 files changed, 6 insertions(+), 85 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index a7fb7b0b1482..0fdc1b134ea3 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -4,7 +4,7 @@
 # This SMM driver performs SMM initialization, deploy SMM Entry Vector,
 # provides CPU specific services in SMM.
 #
-# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 #
 # This program and the accompanying materials
@@ -114,8 +114,6 @@ [Protocols]
 [Guids]
   gEfiAcpiVariableGuid                     ## SOMETIMES_CONSUMES ## HOB # it is used for S3 boot.
   gEfiGlobalVariableGuid                   ## SOMETIMES_PRODUCES ## Variable:L"SmmProfileData"
-  gEfiAcpi20TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable
-  gEfiAcpi10TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable
   gEdkiiPiSmmMemoryAttributesTableGuid     ## CONSUMES ## SystemTable
   gEfiMemoryAttributesTableGuid            ## CONSUMES ## SystemTable
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index a743cf64f907..91b8e7ddb991 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,7 +1,7 @@
 /** @file
 Enable SMM profile.
 
-Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 This program and the accompanying materials
@@ -726,84 +726,6 @@ InitPaging (
 }
 
 /**
-  To find FADT in ACPI tables.
-
-  @param AcpiTableGuid   The GUID used to find ACPI table in UEFI ConfigurationTable.
-
-  @return  FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *
-FindAcpiFadtTableByAcpiGuid (
-  IN EFI_GUID  *AcpiTableGuid
-  )
-{
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
-  EFI_ACPI_DESCRIPTION_HEADER                   *Rsdt;
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE     *Fadt;
-  UINTN                                         Index;
-  UINT32                                        Data32;
-  Rsdp  = NULL;
-  Rsdt  = NULL;
-  Fadt  = NULL;
-  //
-  // found ACPI table RSD_PTR from system table
-  //
-  for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
-    if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), AcpiTableGuid)) {
-      //
-      // A match was found.
-      //
-      Rsdp = gST->ConfigurationTable[Index].VendorTable;
-      break;
-    }
-  }
-
-  if (Rsdp == NULL) {
-    return NULL;
-  }
-
-  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
-  if (Rsdt == NULL || Rsdt->Signature != EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
-    return NULL;
-  }
-
-  for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt->Length; Index = Index + sizeof (UINT32)) {
-
-    Data32  = *(UINT32 *) ((UINT8 *) Rsdt + Index);
-    Fadt    = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *) (UINTN) Data32;
-    if (Fadt->Header.Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-      break;
-    }
-  }
-
-  if (Fadt == NULL || Fadt->Header.Signature != EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-    return NULL;
-  }
-
-  return Fadt;
-}
-
-/**
-  To find FADT in ACPI tables.
-
-  @return  FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *
-FindAcpiFadtTable (
-  VOID
-  )
-{
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-
-  Fadt = FindAcpiFadtTableByAcpiGuid (&gEfiAcpi20TableGuid);
-  if (Fadt != NULL) {
-    return Fadt;
-  }
-
-  return FindAcpiFadtTableByAcpiGuid (&gEfiAcpi10TableGuid);
-}
-
-/**
   To get system port address of the SMI Command Port in FADT table.
 
 **/
@@ -814,7 +736,9 @@ GetSmiCommandPort (
 {
   EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
 
-  Fadt = FindAcpiFadtTable ();
+  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable (
+                                                         EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+                                                         );
   ASSERT (Fadt != NULL);
 
   mSmiCommandPort = Fadt->SmiCmd;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index bacb2f8ad32c..f93e90c143d2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -1,7 +1,7 @@
 /** @file
 SMM profile internal header file.
 
-Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -16,7 +16,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define _SMM_PROFILE_INTERNAL_H_
 
 #include <Guid/GlobalVariable.h>
-#include <Guid/Acpi.h>
 #include <Protocol/SmmReadyToLock.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/DxeServicesTableLib.h>
-- 
2.7.0.windows.1

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

[Laszlo Ersek]

On 09/13/18 12:27, 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
> EfiLocateFirstAcpiTable() and remove the duplicated code.
> 
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |  4 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c         | 84 ++------------------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +-
>  3 files changed, 6 insertions(+), 85 deletions(-)

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

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

[Dong, Eric]

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

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

Re: [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

[Ni, Ruiyu]

Star,
I have two comments. see below.
On 9/13/2018 6:26 PM, 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 adds new EfiLocateXXXAcpiTable() APIs 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   |  68 ++++++
>   MdePkg/Library/UefiLib/Acpi.c      | 488 +++++++++++++++++++++++++++++++++++++
>   MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>   3 files changed, 559 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..cf82ff4a920a 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>   #ifndef __UEFI_LIB_H__
>   #define __UEFI_LIB_H__
>   
> +#include <IndustryStandard/Acpi.h>
> +
>   #include <Protocol/DriverBinding.h>
>   #include <Protocol/DriverConfiguration.h>
>   #include <Protocol/ComponentName.h>
> @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
>     IN     UINT64                    OpenMode,
>     IN     UINT64                    Attributes
>     );
> +
> +/**
> +  This function locates next ACPI table in XSDT/RSDT based on Signature and
> +  previous returned Table.
> +
> +  If PreviousTable is NULL:
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  If PreviousTable is not NULL:
> +  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in XSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi10TableGuid system configuration table.
> +

1. I don't see difference between the case when PreviousTable is NULL or 
not. We don't need to distinguish between the two cases in above comments.


> +  If PreviousTable is not NULL and PreviousTable->Signature is not same with
> +  Signature, then ASSERT.
I'd suggest to return Invalid Parameter instead of assertion.

> +
> +  @param Signature          ACPI table signature.
> +  @param PreviousTable      Pointer to previous returned table to locate next
> +                            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     *PreviousTable OPTIONAL
> +  );
> +
> +/**
> +  This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  @param Signature          ACPI table signature.
> +
> +  @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> +  IN UINT32                     Signature
> +  );
> +
>   #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> new file mode 100644
> index 000000000000..0793bbdc787f
> --- /dev/null
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -0,0 +1,488 @@
> +/** @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.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator about whether the
> +                                previous returned table could be located, or
> +                                NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInRSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
> +  )
> +{
> +  UINTN                             Index;
> +  UINT32                            EntryCount;
> +  UINT32                            *EntryPtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  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_COMMON_HEADER *)((UINTN)(*EntryPtr));
> +    if (Table->Signature == Signature) {
> +      if (PreviousTable != NULL) {
> +        if (Table == PreviousTable) {
> +          *PreviousTableLocated = TRUE;
> +        } else if (*PreviousTableLocated) {
> +          //
> +          // Return next table.
> +          //
> +          return Table;
> +        }
> +      } else {
> +        //
> +        // Return first table.
> +        //
> +        return Table;
> +      }
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  This function scans ACPI table in XSDT.
> +
> +  @param Xsdt                   ACPI XSDT.
> +  @param Signature              ACPI table signature.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator about whether the
> +                                previous returned table could be located, or
> +                                NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInXSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
> +  )

2. Can we merge ScanTableInRSDT and ScanTableInXSDT?

> +{
> +  UINTN                             Index;
> +  UINT32                            EntryCount;
> +  UINT64                            EntryPtr;
> +  UINTN                             BasePtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  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_COMMON_HEADER *)((UINTN)(EntryPtr));
> +    if (Table->Signature == Signature) {
> +      if (PreviousTable != NULL) {
> +        if (Table == PreviousTable) {
> +          *PreviousTableLocated = TRUE;
> +        } else if (*PreviousTableLocated) {
> +          //
> +          // Return next table.
> +          //
> +          return Table;
> +        }
> +      } else {
> +        //
> +        // Return first table.
> +        //
> +        return Table;
> +      }
> +
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  To locate FACS in FADT.
> +
> +  @param Fadt   FADT table pointer.
> +
> +  @return FACS table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiFacsFromFadt (
> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER                        *Facs;
> +  UINT64                                        Data64;
> +
> +  if (Fadt == NULL) {
> +    return NULL;
> +  }
> +
> +  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> +    Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> +  } else {
> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> +    if (Data64 != 0) {
> +      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> +    } else {
> +      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> +    }
> +  }
> +  return Facs;
> +}
> +
> +/**
> +  To locate DSDT in FADT.
> +
> +  @param Fadt   FADT table pointer.
> +
> +  @return DSDT table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiDsdtFromFadt (
> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER                        *Dsdt;
> +  UINT64                                        Data64;
> +
> +  if (Fadt == NULL) {
> +    return NULL;
> +  }
> +
> +  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> +    Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> +  } else {
> +    CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
> +    if (Data64 != 0) {
> +      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> +    } else {
> +      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> +    }
> +  }
> +  return Dsdt;
> +}
> +
> +/**
> +  To locate ACPI table in ACPI ConfigurationTable.
> +
> +  @param AcpiTableGuid          The GUID used to get ACPI ConfigurationTable.
> +  @param Signature              ACPI table signature.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator to return whether the
> +                                previous returned table could be located or not,
> +                                or NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +  If AcpiGuid is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiTableInAcpiConfigurationTable (
> +  IN  EFI_GUID                  *AcpiGuid,
> +  IN  UINT32                    Signature,
> +  IN  EFI_ACPI_COMMON_HEADER    *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                   *PreviousTableLocated OPTIONAL
> +  )
> +{
> +  EFI_STATUS                                    Status;
> +  EFI_ACPI_COMMON_HEADER                        *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;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  Rsdp = NULL;
> +  //
> +  // Get 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_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> +      ASSERT (PreviousTable == NULL);
> +      //
> +      // It is to locate DSDT,
> +      // need to locate FADT first.
> +      //
> +      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> +               Xsdt,
> +               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiDsdtFromFadt (Fadt);
> +    } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +      ASSERT (PreviousTable == NULL);
> +      //
> +      // It is to locate FACS,
> +      // need to locate FADT first.
> +      //
> +      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> +               Xsdt,
> +               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiFacsFromFadt (Fadt);
> +    } else {
> +      Table = ScanTableInXSDT (
> +                Xsdt,
> +                Signature,
> +                PreviousTable,
> +                PreviousTableLocated
> +                );
> +    }
> +  }
> +
> +  if (Table != NULL) {
> +    return Table;
> +  } else if ((PreviousTableLocated != NULL) &&
> +              *PreviousTableLocated) {
> +    //
> +    // PreviousTable could be located in XSDT,
> +    // but next table could not be located in XSDT.
> +    //
> +    return NULL;
> +  }
> +
> +  //
> +  // Search RSDT
> +  //
> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> +  if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> +    ASSERT (PreviousTable == NULL);
> +    //
> +    // It is to locate DSDT,
> +    // need to locate FADT first.
> +    //
> +    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> +             Rsdt,
> +             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiDsdtFromFadt (Fadt);
> +  } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +    ASSERT (PreviousTable == NULL);
> +    //
> +    // It is to locate FACS,
> +    // need to locate FADT first.
> +    //
> +    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> +             Rsdt,
> +             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiFacsFromFadt (Fadt);
> +  } else {
> +    Table = ScanTableInRSDT (
> +              Rsdt,
> +              Signature,
> +              PreviousTable,
> +              PreviousTableLocated
> +              );
> +  }
> +
> +  return Table;
> +}
> +
> +/**
> +  This function locates next ACPI table in XSDT/RSDT based on Signature and
> +  previous returned Table.
> +
> +  If PreviousTable is NULL:
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  If PreviousTable is not NULL:
> +  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in XSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi10TableGuid system configuration table.
> +
> +  If PreviousTable is not NULL and PreviousTable->Signature is not same with
> +  Signature, then ASSERT.
> +
> +  @param Signature          ACPI table signature.
> +  @param PreviousTable      Pointer to previous returned table to locate next
> +                            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     *PreviousTable OPTIONAL
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER        *Table;
> +  BOOLEAN                       TempPreviousTableLocated;
> +  BOOLEAN                       *PreviousTableLocated;
> +
> +  if (PreviousTable != NULL) {
> +    if (PreviousTable->Signature != Signature) {
> +      //
> +      // PreviousTable->Signature is not same with Signature.
> +      //
> +      ASSERT (FALSE);
> +      return NULL;
> +    } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
> +               (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
> +               (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
> +      //
> +      // There is only one FADT/DSDT/FACS table,
> +      // so don't try to locate next one.
> +      //
> +      return NULL;
> +    }
> +
> +    PreviousTableLocated = &TempPreviousTableLocated;
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    PreviousTableLocated = NULL;
> +  }
> +
> +  Table = LocateAcpiTableInAcpiConfigurationTable (
> +            &gEfiAcpi20TableGuid,
> +            Signature,
> +            PreviousTable,
> +            PreviousTableLocated
> +            );
> +  if (Table != NULL) {
> +    return Table;
> +  } else if ((PreviousTableLocated != NULL) &&
> +              *PreviousTableLocated) {
> +    //
> +    // PreviousTable could be located in gEfiAcpi20TableGuid system
> +    // configuration table, but next table could not be located in
> +    // gEfiAcpi20TableGuid system configuration table.
> +    //
> +    return NULL;
> +  }
> +
> +  return LocateAcpiTableInAcpiConfigurationTable (
> +           &gEfiAcpi10TableGuid,
> +           Signature,
> +           PreviousTable,
> +           PreviousTableLocated
> +           );
> +}
> +
> +/**
> +  This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  @param Signature          ACPI table signature.
> +
> +  @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> +  IN UINT32                     Signature
> +  )
> +{
> +  return EfiLocateNextAcpiTable (Signature, NULL);
> +}
> 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
> 


-- 
Thanks,
Ray

Re: [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()

[Ni, Ruiyu]

On 9/13/2018 6:26 PM, 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 PcatRealTimeClockRuntimeDxe to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
> 
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> 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..7965eb8aa55b 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 *) EfiLocateFirstAcpiTable (
> +                                                         EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
> +                                                         );
>   
>     if ((Fadt != NULL) &&
>         (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)
> 

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray

Re: [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()

[Ni, Ruiyu]

On 9/14/2018 12:41 PM, Ni, Ruiyu wrote:
> On 9/13/2018 6:26 PM, 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 PcatRealTimeClockRuntimeDxe to use new
>> EfiLocateFirstAcpiTable() and remove the duplicated code.
>>
>> Cc: Younas khan <pmdyounaskhan786@gmail.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> 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..7965eb8aa55b 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 *) 
>> EfiLocateFirstAcpiTable (
>> +                                                         
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
>> +                                                         );
>>     if ((Fadt != NULL) &&
>>         (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 
>> 0x80)
>>
> 
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> 
By the way, can we remove the Acpi10/Acpi20 GUID references from INF file?

-- 
Thanks,
Ray

Re: [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 6:27 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>
> Subject: [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new
> EfiLocateFirstAcpiTable()
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get ACPI table
> using signature as input.
> 
> After evaluation, we found there are many duplicated code to find ACPI
> table by signature in different modules.
> 
> This patch updates DpDynamicCommand to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
> 
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: 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..c14931055cbf 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 *)
> EfiLocateFirstAcpiTable (
> +
> 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.i
> nf
> b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> index 8fd3bbd5df83..62a3b5246500 100644
> ---
> a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> +++
> b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> @@ -60,8 +60,6 @@ [LibraryClasses]
> 
>  [Guids]
>    gPerformanceProtocolGuid                                ## CONSUMES ## SystemTable
> -  gEfiAcpi20TableGuid                                     ## CONSUMES
> -  gEfiAcpi10TableGuid                                     ## CONSUMES
> 
>  [Protocols]
>    gEfiLoadedImageProtocolGuid                             ## CONSUMES
> --
> 2.7.0.windows.1

Re: [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

[Zeng, Star]

Ray,

Thanks for the good feedbacks.
I can remove the ASSERT and update the description in the function header.
I can also merge ScanTableInRSDT and ScanTableInXSDT.

Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Friday, September 14, 2018 12:41 PM
To: Zeng, Star <star.zeng@intel.com>; 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>
Subject: Re: [edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

Star,
I have two comments. see below.
On 9/13/2018 6:26 PM, 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 adds new EfiLocateXXXAcpiTable() APIs 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   |  68 ++++++
>   MdePkg/Library/UefiLib/Acpi.c      | 488 +++++++++++++++++++++++++++++++++++++
>   MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>   3 files changed, 559 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..cf82ff4a920a 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>   #ifndef __UEFI_LIB_H__
>   #define __UEFI_LIB_H__
>   
> +#include <IndustryStandard/Acpi.h>
> +
>   #include <Protocol/DriverBinding.h>
>   #include <Protocol/DriverConfiguration.h>
>   #include <Protocol/ComponentName.h>
> @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
>     IN     UINT64                    OpenMode,
>     IN     UINT64                    Attributes
>     );
> +
> +/**
> +  This function locates next ACPI table in XSDT/RSDT based on 
> +Signature and
> +  previous returned Table.
> +
> +  If PreviousTable is NULL:
> +  This function will locate the first ACPI table in XSDT/RSDT based 
> + on  Signature in gEfiAcpi20TableGuid system configuration table 
> + first, and then  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then 
> + Dsdt in  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, 
> + and then  FirmwareCtrl in FADT.
> +
> +  If PreviousTable is not NULL:
> +  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in XSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi10TableGuid system configuration table.
> +

1. I don't see difference between the case when PreviousTable is NULL or not. We don't need to distinguish between the two cases in above comments.


> +  If PreviousTable is not NULL and PreviousTable->Signature is not same with
> +  Signature, then ASSERT.
I'd suggest to return Invalid Parameter instead of assertion.

> +
> +  @param Signature          ACPI table signature.
> +  @param PreviousTable      Pointer to previous returned table to locate next
> +                            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     *PreviousTable OPTIONAL
> +  );
> +
> +/**
> +  This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  @param Signature          ACPI table signature.
> +
> +  @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> +  IN UINT32                     Signature
> +  );
> +
>   #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> new file mode 100644
> index 000000000000..0793bbdc787f
> --- /dev/null
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -0,0 +1,488 @@
> +/** @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.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator about whether the
> +                                previous returned table could be located, or
> +                                NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInRSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
> +  )
> +{
> +  UINTN                             Index;
> +  UINT32                            EntryCount;
> +  UINT32                            *EntryPtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  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_COMMON_HEADER *)((UINTN)(*EntryPtr));
> +    if (Table->Signature == Signature) {
> +      if (PreviousTable != NULL) {
> +        if (Table == PreviousTable) {
> +          *PreviousTableLocated = TRUE;
> +        } else if (*PreviousTableLocated) {
> +          //
> +          // Return next table.
> +          //
> +          return Table;
> +        }
> +      } else {
> +        //
> +        // Return first table.
> +        //
> +        return Table;
> +      }
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  This function scans ACPI table in XSDT.
> +
> +  @param Xsdt                   ACPI XSDT.
> +  @param Signature              ACPI table signature.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator about whether the
> +                                previous returned table could be located, or
> +                                NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInXSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
> +  )

2. Can we merge ScanTableInRSDT and ScanTableInXSDT?

> +{
> +  UINTN                             Index;
> +  UINT32                            EntryCount;
> +  UINT64                            EntryPtr;
> +  UINTN                             BasePtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  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_COMMON_HEADER *)((UINTN)(EntryPtr));
> +    if (Table->Signature == Signature) {
> +      if (PreviousTable != NULL) {
> +        if (Table == PreviousTable) {
> +          *PreviousTableLocated = TRUE;
> +        } else if (*PreviousTableLocated) {
> +          //
> +          // Return next table.
> +          //
> +          return Table;
> +        }
> +      } else {
> +        //
> +        // Return first table.
> +        //
> +        return Table;
> +      }
> +
> +    }
> +  }
> +
> +  return NULL;
> +}
> +
> +/**
> +  To locate FACS in FADT.
> +
> +  @param Fadt   FADT table pointer.
> +
> +  @return FACS table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiFacsFromFadt (
> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER                        *Facs;
> +  UINT64                                        Data64;
> +
> +  if (Fadt == NULL) {
> +    return NULL;
> +  }
> +
> +  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> +    Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> +  } else {
> +    CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> +    if (Data64 != 0) {
> +      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> +    } else {
> +      Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> +    }
> +  }
> +  return Facs;
> +}
> +
> +/**
> +  To locate DSDT in FADT.
> +
> +  @param Fadt   FADT table pointer.
> +
> +  @return DSDT table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiDsdtFromFadt (
> +  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *Fadt
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER                        *Dsdt;
> +  UINT64                                        Data64;
> +
> +  if (Fadt == NULL) {
> +    return NULL;
> +  }
> +
> +  if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> +    Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> +  } else {
> +    CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
> +    if (Data64 != 0) {
> +      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> +    } else {
> +      Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> +    }
> +  }
> +  return Dsdt;
> +}
> +
> +/**
> +  To locate ACPI table in ACPI ConfigurationTable.
> +
> +  @param AcpiTableGuid          The GUID used to get ACPI ConfigurationTable.
> +  @param Signature              ACPI table signature.
> +  @param PreviousTable          Pointer to previous returned table to locate
> +                                next table, or NULL to locate first table.
> +  @param PreviousTableLocated   Pointer to the indicator to return whether the
> +                                previous returned table could be located or not,
> +                                or NULL if PreviousTable is NULL.
> +
> +  If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> +  If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +  If AcpiGuid is NULL, then ASSERT().
> +
> +  @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiTableInAcpiConfigurationTable (
> +  IN  EFI_GUID                  *AcpiGuid,
> +  IN  UINT32                    Signature,
> +  IN  EFI_ACPI_COMMON_HEADER    *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                   *PreviousTableLocated OPTIONAL
> +  )
> +{
> +  EFI_STATUS                                    Status;
> +  EFI_ACPI_COMMON_HEADER                        *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;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  Rsdp = NULL;
> +  //
> +  // Get 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_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> +      ASSERT (PreviousTable == NULL);
> +      //
> +      // It is to locate DSDT,
> +      // need to locate FADT first.
> +      //
> +      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> +               Xsdt,
> +               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiDsdtFromFadt (Fadt);
> +    } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +      ASSERT (PreviousTable == NULL);
> +      //
> +      // It is to locate FACS,
> +      // need to locate FADT first.
> +      //
> +      Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> +               Xsdt,
> +               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiFacsFromFadt (Fadt);
> +    } else {
> +      Table = ScanTableInXSDT (
> +                Xsdt,
> +                Signature,
> +                PreviousTable,
> +                PreviousTableLocated
> +                );
> +    }
> +  }
> +
> +  if (Table != NULL) {
> +    return Table;
> +  } else if ((PreviousTableLocated != NULL) &&
> +              *PreviousTableLocated) {
> +    //
> +    // PreviousTable could be located in XSDT,
> +    // but next table could not be located in XSDT.
> +    //
> +    return NULL;
> +  }
> +
> +  //
> +  // Search RSDT
> +  //
> +  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> +  if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> +    ASSERT (PreviousTable == NULL);
> +    //
> +    // It is to locate DSDT,
> +    // need to locate FADT first.
> +    //
> +    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> +             Rsdt,
> +             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiDsdtFromFadt (Fadt);
> +  } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> +    ASSERT (PreviousTable == NULL);
> +    //
> +    // It is to locate FACS,
> +    // need to locate FADT first.
> +    //
> +    Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> +             Rsdt,
> +             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiFacsFromFadt (Fadt);
> +  } else {
> +    Table = ScanTableInRSDT (
> +              Rsdt,
> +              Signature,
> +              PreviousTable,
> +              PreviousTableLocated
> +              );
> +  }
> +
> +  return Table;
> +}
> +
> +/**
> +  This function locates next ACPI table in XSDT/RSDT based on Signature and
> +  previous returned Table.
> +
> +  If PreviousTable is NULL:
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  If PreviousTable is not NULL:
> +  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in XSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi20TableGuid system configuration table.
> +  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> +     table, then this function will just locate next table in RSDT in
> +     gEfiAcpi10TableGuid system configuration table.
> +
> +  If PreviousTable is not NULL and PreviousTable->Signature is not same with
> +  Signature, then ASSERT.
> +
> +  @param Signature          ACPI table signature.
> +  @param PreviousTable      Pointer to previous returned table to locate next
> +                            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     *PreviousTable OPTIONAL
> +  )
> +{
> +  EFI_ACPI_COMMON_HEADER        *Table;
> +  BOOLEAN                       TempPreviousTableLocated;
> +  BOOLEAN                       *PreviousTableLocated;
> +
> +  if (PreviousTable != NULL) {
> +    if (PreviousTable->Signature != Signature) {
> +      //
> +      // PreviousTable->Signature is not same with Signature.
> +      //
> +      ASSERT (FALSE);
> +      return NULL;
> +    } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
> +               (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
> +               (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
> +      //
> +      // There is only one FADT/DSDT/FACS table,
> +      // so don't try to locate next one.
> +      //
> +      return NULL;
> +    }
> +
> +    PreviousTableLocated = &TempPreviousTableLocated;
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    PreviousTableLocated = NULL;
> +  }
> +
> +  Table = LocateAcpiTableInAcpiConfigurationTable (
> +            &gEfiAcpi20TableGuid,
> +            Signature,
> +            PreviousTable,
> +            PreviousTableLocated
> +            );
> +  if (Table != NULL) {
> +    return Table;
> +  } else if ((PreviousTableLocated != NULL) &&
> +              *PreviousTableLocated) {
> +    //
> +    // PreviousTable could be located in gEfiAcpi20TableGuid system
> +    // configuration table, but next table could not be located in
> +    // gEfiAcpi20TableGuid system configuration table.
> +    //
> +    return NULL;
> +  }
> +
> +  return LocateAcpiTableInAcpiConfigurationTable (
> +           &gEfiAcpi10TableGuid,
> +           Signature,
> +           PreviousTable,
> +           PreviousTableLocated
> +           );
> +}
> +
> +/**
> +  This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  @param Signature          ACPI table signature.
> +
> +  @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> +  IN UINT32                     Signature
> +  )
> +{
> +  return EfiLocateNextAcpiTable (Signature, NULL);
> +}
> 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
> 


-- 
Thanks,
Ray