public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
@ 2018-09-19  1:28 Star Zeng
  2018-09-19  1:35 ` Gao, Liming
  2018-09-19  2:08 ` Yao, Jiewen
  0 siblings, 2 replies; 9+ messages in thread
From: Star Zeng @ 2018-09-19  1:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao

It updates FrameworkUefiLib and is the supplement to
https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
 
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>
---
 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428 +++++++++++++++++++++
 .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
 2 files changed, 431 insertions(+)
 create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c

diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
new file mode 100644
index 000000000000..e16ccc7e7b74
--- /dev/null
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
@@ -0,0 +1,428 @@
+/** @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 XSDT/RSDT.
+
+  @param Sdt                    ACPI XSDT/RSDT.
+  @param TablePointerSize       Size of table pointer: 8(XSDT) or 4(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 *
+ScanTableInSDT (
+  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
+  IN  UINTN                         TablePointerSize,
+  IN  UINT32                        Signature,
+  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
+  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
+  )
+{
+  UINTN                             Index;
+  UINTN                             EntryCount;
+  UINT64                            EntryPtr;
+  UINTN                             BasePtr;
+  EFI_ACPI_COMMON_HEADER            *Table;
+
+  if (PreviousTableLocated != NULL) {
+    ASSERT (PreviousTable != NULL);
+    *PreviousTableLocated = FALSE;
+  } else {
+    ASSERT (PreviousTable == NULL);
+  }
+
+  if (Sdt == NULL) {
+    return NULL;
+  }
+
+  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / TablePointerSize;
+
+  BasePtr = (UINTN)(Sdt + 1);
+  for (Index = 0; Index < EntryCount; Index ++) {
+    EntryPtr = 0;
+    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize), TablePointerSize);
+    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 *) ScanTableInSDT (
+               Xsdt,
+               sizeof (UINT64),
+               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 *) ScanTableInSDT (
+               Xsdt,
+               sizeof (UINT64),
+               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+               NULL,
+               NULL
+               );
+      Table = LocateAcpiFacsFromFadt (Fadt);
+    } else {
+      Table = ScanTableInSDT (
+                Xsdt,
+                sizeof (UINT64),
+                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 *) ScanTableInSDT (
+             Rsdt,
+             sizeof (UINT32),
+             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 *) ScanTableInSDT (
+             Rsdt,
+             sizeof (UINT32),
+             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+             NULL,
+             NULL
+             );
+    Table = LocateAcpiFacsFromFadt (Fadt);
+  } else {
+    Table = ScanTableInSDT (
+              Rsdt,
+              sizeof (UINT32),
+              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.
+
+  It's not supported that PreviousTable is not NULL but PreviousTable->Signature
+  is not same with Signature, NULL will be returned.
+
+  @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.
+      //
+      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
index 182d20fca051..2d333b1b4b2a 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
@@ -37,6 +37,7 @@ [Sources]
   Console.c
   UefiLib.c
   UefiLibInternal.h
+  Acpi.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -55,6 +56,8 @@ [LibraryClasses]
 [Guids]
   gEfiEventReadyToBootGuid                      ## SOMETIMES_CONSUMES  ## Event
   gEfiEventLegacyBootGuid                       ## SOMETIMES_CONSUMES  ## Event
+  gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
+  gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
 
 [Protocols]
   gEfiDriverBindingProtocolGuid                 ## SOMETIMES_PRODUCES
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  1:28 [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs Star Zeng
@ 2018-09-19  1:35 ` Gao, Liming
  2018-09-19  2:08 ` Yao, Jiewen
  1 sibling, 0 replies; 9+ messages in thread
From: Gao, Liming @ 2018-09-19  1:35 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Yao, Jiewen

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Zeng, Star
>Sent: Wednesday, September 19, 2018 9:28 AM
>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>
>Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
>EfiLocateXXXAcpiTable APIs
>
>It updates FrameworkUefiLib and is the supplement to
>https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
>
>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>
>---
> IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
>+++++++++++++++++++++
> .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
> 2 files changed, 431 insertions(+)
> create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
>
>diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
>b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
>new file mode 100644
>index 000000000000..e16ccc7e7b74
>--- /dev/null
>+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
>@@ -0,0 +1,428 @@
>+/** @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 XSDT/RSDT.
>+
>+  @param Sdt                    ACPI XSDT/RSDT.
>+  @param TablePointerSize       Size of table pointer: 8(XSDT) or 4(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 *
>+ScanTableInSDT (
>+  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
>+  IN  UINTN                         TablePointerSize,
>+  IN  UINT32                        Signature,
>+  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
>+  OUT BOOLEAN                       *PreviousTableLocated OPTIONAL
>+  )
>+{
>+  UINTN                             Index;
>+  UINTN                             EntryCount;
>+  UINT64                            EntryPtr;
>+  UINTN                             BasePtr;
>+  EFI_ACPI_COMMON_HEADER            *Table;
>+
>+  if (PreviousTableLocated != NULL) {
>+    ASSERT (PreviousTable != NULL);
>+    *PreviousTableLocated = FALSE;
>+  } else {
>+    ASSERT (PreviousTable == NULL);
>+  }
>+
>+  if (Sdt == NULL) {
>+    return NULL;
>+  }
>+
>+  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>TablePointerSize;
>+
>+  BasePtr = (UINTN)(Sdt + 1);
>+  for (Index = 0; Index < EntryCount; Index ++) {
>+    EntryPtr = 0;
>+    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
>TablePointerSize);
>+    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 *)
>ScanTableInSDT (
>+               Xsdt,
>+               sizeof (UINT64),
>+               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 *)
>ScanTableInSDT (
>+               Xsdt,
>+               sizeof (UINT64),
>+               EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>+               NULL,
>+               NULL
>+               );
>+      Table = LocateAcpiFacsFromFadt (Fadt);
>+    } else {
>+      Table = ScanTableInSDT (
>+                Xsdt,
>+                sizeof (UINT64),
>+                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 *)
>ScanTableInSDT (
>+             Rsdt,
>+             sizeof (UINT32),
>+             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 *)
>ScanTableInSDT (
>+             Rsdt,
>+             sizeof (UINT32),
>+             EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>+             NULL,
>+             NULL
>+             );
>+    Table = LocateAcpiFacsFromFadt (Fadt);
>+  } else {
>+    Table = ScanTableInSDT (
>+              Rsdt,
>+              sizeof (UINT32),
>+              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.
>+
>+  It's not supported that PreviousTable is not NULL but PreviousTable-
>>Signature
>+  is not same with Signature, NULL will be returned.
>+
>+  @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.
>+      //
>+      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
>b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
>index 182d20fca051..2d333b1b4b2a 100644
>--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
>+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
>@@ -37,6 +37,7 @@ [Sources]
>   Console.c
>   UefiLib.c
>   UefiLibInternal.h
>+  Acpi.c
>
> [Packages]
>   MdePkg/MdePkg.dec
>@@ -55,6 +56,8 @@ [LibraryClasses]
> [Guids]
>   gEfiEventReadyToBootGuid                      ## SOMETIMES_CONSUMES  ##
>Event
>   gEfiEventLegacyBootGuid                       ## SOMETIMES_CONSUMES  ## Event
>+  gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES  ##
>SystemTable
>+  gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES  ##
>SystemTable
>
> [Protocols]
>   gEfiDriverBindingProtocolGuid                 ## SOMETIMES_PRODUCES
>--
>2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  1:28 [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs Star Zeng
  2018-09-19  1:35 ` Gao, Liming
@ 2018-09-19  2:08 ` Yao, Jiewen
  2018-09-19  2:10   ` Zeng, Star
  1 sibling, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2018-09-19  2:08 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Gao, Liming

Hi
I do not recommend we add more new feature to a to-be-deprecated package.

Can we just return UNSUPPORTED? 


> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 19, 2018 9:28 AM
> 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>
> Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
> EfiLocateXXXAcpiTable APIs
> 
> It updates FrameworkUefiLib and is the supplement to
> https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> 
> 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>
> ---
>  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
> +++++++++++++++++++++
>  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
>  2 files changed, 431 insertions(+)
>  create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> 
> diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> new file mode 100644
> index 000000000000..e16ccc7e7b74
> --- /dev/null
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> @@ -0,0 +1,428 @@
> +/** @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 XSDT/RSDT.
> +
> +  @param Sdt                    ACPI XSDT/RSDT.
> +  @param TablePointerSize       Size of table pointer: 8(XSDT) or
> 4(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 *
> +ScanTableInSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
> +  IN  UINTN                         TablePointerSize,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated
> OPTIONAL
> +  )
> +{
> +  UINTN                             Index;
> +  UINTN                             EntryCount;
> +  UINT64                            EntryPtr;
> +  UINTN                             BasePtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  if (Sdt == NULL) {
> +    return NULL;
> +  }
> +
> +  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> TablePointerSize;
> +
> +  BasePtr = (UINTN)(Sdt + 1);
> +  for (Index = 0; Index < EntryCount; Index ++) {
> +    EntryPtr = 0;
> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
> +    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 *)
> ScanTableInSDT (
> +               Xsdt,
> +               sizeof (UINT64),
> +
> 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 *)
> ScanTableInSDT (
> +               Xsdt,
> +               sizeof (UINT64),
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiFacsFromFadt (Fadt);
> +    } else {
> +      Table = ScanTableInSDT (
> +                Xsdt,
> +                sizeof (UINT64),
> +                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 *)
> ScanTableInSDT (
> +             Rsdt,
> +             sizeof (UINT32),
> +
> 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 *)
> ScanTableInSDT (
> +             Rsdt,
> +             sizeof (UINT32),
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiFacsFromFadt (Fadt);
> +  } else {
> +    Table = ScanTableInSDT (
> +              Rsdt,
> +              sizeof (UINT32),
> +              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.
> +
> +  It's not supported that PreviousTable is not NULL but
> PreviousTable->Signature
> +  is not same with Signature, NULL will be returned.
> +
> +  @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.
> +      //
> +      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> index 182d20fca051..2d333b1b4b2a 100644
> --- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> @@ -37,6 +37,7 @@ [Sources]
>    Console.c
>    UefiLib.c
>    UefiLibInternal.h
> +  Acpi.c
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -55,6 +56,8 @@ [LibraryClasses]
>  [Guids]
>    gEfiEventReadyToBootGuid                      ##
> SOMETIMES_CONSUMES  ## Event
>    gEfiEventLegacyBootGuid                       ##
> SOMETIMES_CONSUMES  ## Event
> +  gEfiAcpi20TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> +  gEfiAcpi10TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> 
>  [Protocols]
>    gEfiDriverBindingProtocolGuid                 ##
> SOMETIMES_PRODUCES
> --
> 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  2:08 ` Yao, Jiewen
@ 2018-09-19  2:10   ` Zeng, Star
  2018-09-19  2:13     ` Yao, Jiewen
  0 siblings, 1 reply; 9+ messages in thread
From: Zeng, Star @ 2018-09-19  2:10 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Gao, Liming, Zeng, Star

You mean like below for the case?
ASSERT(FALSE);
return NULL;

Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Wednesday, September 19, 2018 10:08 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs

Hi
I do not recommend we add more new feature to a to-be-deprecated package.

Can we just return UNSUPPORTED? 


> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 19, 2018 9:28 AM
> 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>
> Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> EfiLocateXXXAcpiTable APIs
> 
> It updates FrameworkUefiLib and is the supplement to 
> https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> 
> 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>
> ---
>  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
> +++++++++++++++++++++
>  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
>  2 files changed, 431 insertions(+)
>  create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> 
> diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> new file mode 100644
> index 000000000000..e16ccc7e7b74
> --- /dev/null
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> @@ -0,0 +1,428 @@
> +/** @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 XSDT/RSDT.
> +
> +  @param Sdt                    ACPI XSDT/RSDT.
> +  @param TablePointerSize       Size of table pointer: 8(XSDT) or
> 4(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 *
> +ScanTableInSDT (
> +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
> +  IN  UINTN                         TablePointerSize,
> +  IN  UINT32                        Signature,
> +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable, OPTIONAL
> +  OUT BOOLEAN                       *PreviousTableLocated
> OPTIONAL
> +  )
> +{
> +  UINTN                             Index;
> +  UINTN                             EntryCount;
> +  UINT64                            EntryPtr;
> +  UINTN                             BasePtr;
> +  EFI_ACPI_COMMON_HEADER            *Table;
> +
> +  if (PreviousTableLocated != NULL) {
> +    ASSERT (PreviousTable != NULL);
> +    *PreviousTableLocated = FALSE;
> +  } else {
> +    ASSERT (PreviousTable == NULL);
> +  }
> +
> +  if (Sdt == NULL) {
> +    return NULL;
> +  }
> +
> +  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> TablePointerSize;
> +
> +  BasePtr = (UINTN)(Sdt + 1);
> +  for (Index = 0; Index < EntryCount; Index ++) {
> +    EntryPtr = 0;
> +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
> +    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 *)
> ScanTableInSDT (
> +               Xsdt,
> +               sizeof (UINT64),
> +
> 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 *)
> ScanTableInSDT (
> +               Xsdt,
> +               sizeof (UINT64),
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +               NULL,
> +               NULL
> +               );
> +      Table = LocateAcpiFacsFromFadt (Fadt);
> +    } else {
> +      Table = ScanTableInSDT (
> +                Xsdt,
> +                sizeof (UINT64),
> +                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 *)
> ScanTableInSDT (
> +             Rsdt,
> +             sizeof (UINT32),
> +
> 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 *)
> ScanTableInSDT (
> +             Rsdt,
> +             sizeof (UINT32),
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +             NULL,
> +             NULL
> +             );
> +    Table = LocateAcpiFacsFromFadt (Fadt);  } else {
> +    Table = ScanTableInSDT (
> +              Rsdt,
> +              sizeof (UINT32),
> +              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.
> +
> +  It's not supported that PreviousTable is not NULL but
> PreviousTable->Signature
> +  is not same with Signature, NULL will be returned.
> +
> +  @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.
> +      //
> +      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> index 182d20fca051..2d333b1b4b2a 100644
> --- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> @@ -37,6 +37,7 @@ [Sources]
>    Console.c
>    UefiLib.c
>    UefiLibInternal.h
> +  Acpi.c
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> @@ -55,6 +56,8 @@ [LibraryClasses]
>  [Guids]
>    gEfiEventReadyToBootGuid                      ##
> SOMETIMES_CONSUMES  ## Event
>    gEfiEventLegacyBootGuid                       ##
> SOMETIMES_CONSUMES  ## Event
> +  gEfiAcpi20TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> +  gEfiAcpi10TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> 
>  [Protocols]
>    gEfiDriverBindingProtocolGuid                 ##
> SOMETIMES_PRODUCES
> --
> 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  2:10   ` Zeng, Star
@ 2018-09-19  2:13     ` Yao, Jiewen
  2018-09-19  2:15       ` Zeng, Star
  0 siblings, 1 reply; 9+ messages in thread
From: Yao, Jiewen @ 2018-09-19  2:13 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Gao, Liming

Yes.

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 19, 2018 10:11 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
> EfiLocateXXXAcpiTable APIs
> 
> You mean like below for the case?
> ASSERT(FALSE);
> return NULL;
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, September 19, 2018 10:08 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
> EfiLocateXXXAcpiTable APIs
> 
> Hi
> I do not recommend we add more new feature to a to-be-deprecated
> package.
> 
> Can we just return UNSUPPORTED?
> 
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Wednesday, September 19, 2018 9:28 AM
> > 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>
> > Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
> > EfiLocateXXXAcpiTable APIs
> >
> > It updates FrameworkUefiLib and is the supplement to
> > https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> >
> > 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>
> > ---
> >  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
> > +++++++++++++++++++++
> >  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
> >  2 files changed, 431 insertions(+)
> >  create mode 100644
> IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> >
> > diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > new file mode 100644
> > index 000000000000..e16ccc7e7b74
> > --- /dev/null
> > +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > @@ -0,0 +1,428 @@
> > +/** @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 XSDT/RSDT.
> > +
> > +  @param Sdt                    ACPI XSDT/RSDT.
> > +  @param TablePointerSize       Size of table pointer: 8(XSDT) or
> > 4(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 *
> > +ScanTableInSDT (
> > +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
> > +  IN  UINTN                         TablePointerSize,
> > +  IN  UINT32                        Signature,
> > +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable,
> OPTIONAL
> > +  OUT BOOLEAN                       *PreviousTableLocated
> > OPTIONAL
> > +  )
> > +{
> > +  UINTN                             Index;
> > +  UINTN                             EntryCount;
> > +  UINT64                            EntryPtr;
> > +  UINTN                             BasePtr;
> > +  EFI_ACPI_COMMON_HEADER            *Table;
> > +
> > +  if (PreviousTableLocated != NULL) {
> > +    ASSERT (PreviousTable != NULL);
> > +    *PreviousTableLocated = FALSE;
> > +  } else {
> > +    ASSERT (PreviousTable == NULL);
> > +  }
> > +
> > +  if (Sdt == NULL) {
> > +    return NULL;
> > +  }
> > +
> > +  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER))
> /
> > TablePointerSize;
> > +
> > +  BasePtr = (UINTN)(Sdt + 1);
> > +  for (Index = 0; Index < EntryCount; Index ++) {
> > +    EntryPtr = 0;
> > +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> > TablePointerSize);
> > +    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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +               NULL,
> > +               NULL
> > +               );
> > +      Table = LocateAcpiFacsFromFadt (Fadt);
> > +    } else {
> > +      Table = ScanTableInSDT (
> > +                Xsdt,
> > +                sizeof (UINT64),
> > +                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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +             NULL,
> > +             NULL
> > +             );
> > +    Table = LocateAcpiFacsFromFadt (Fadt);  } else {
> > +    Table = ScanTableInSDT (
> > +              Rsdt,
> > +              sizeof (UINT32),
> > +              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.
> > +
> > +  It's not supported that PreviousTable is not NULL but
> > PreviousTable->Signature
> > +  is not same with Signature, NULL will be returned.
> > +
> > +  @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.
> > +      //
> > +      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > index 182d20fca051..2d333b1b4b2a 100644
> > --- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > +++
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > @@ -37,6 +37,7 @@ [Sources]
> >    Console.c
> >    UefiLib.c
> >    UefiLibInternal.h
> > +  Acpi.c
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > @@ -55,6 +56,8 @@ [LibraryClasses]
> >  [Guids]
> >    gEfiEventReadyToBootGuid                      ##
> > SOMETIMES_CONSUMES  ## Event
> >    gEfiEventLegacyBootGuid                       ##
> > SOMETIMES_CONSUMES  ## Event
> > +  gEfiAcpi20TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> > +  gEfiAcpi10TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> >
> >  [Protocols]
> >    gEfiDriverBindingProtocolGuid                 ##
> > SOMETIMES_PRODUCES
> > --
> > 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  2:13     ` Yao, Jiewen
@ 2018-09-19  2:15       ` Zeng, Star
  2018-09-19  2:39         ` Zeng, Star
  0 siblings, 1 reply; 9+ messages in thread
From: Zeng, Star @ 2018-09-19  2:15 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Gao, Liming, Zeng, Star

I think it is a good idea.

Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Wednesday, September 19, 2018 10:13 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs

Yes.

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 19, 2018 10:11 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; 
> Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> EfiLocateXXXAcpiTable APIs
> 
> You mean like below for the case?
> ASSERT(FALSE);
> return NULL;
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, September 19, 2018 10:08 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> EfiLocateXXXAcpiTable APIs
> 
> Hi
> I do not recommend we add more new feature to a to-be-deprecated 
> package.
> 
> Can we just return UNSUPPORTED?
> 
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Wednesday, September 19, 2018 9:28 AM
> > 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>
> > Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> > EfiLocateXXXAcpiTable APIs
> >
> > It updates FrameworkUefiLib and is the supplement to 
> > https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> >
> > 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>
> > ---
> >  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
> > +++++++++++++++++++++
> >  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
> >  2 files changed, 431 insertions(+)
> >  create mode 100644
> IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> >
> > diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > new file mode 100644
> > index 000000000000..e16ccc7e7b74
> > --- /dev/null
> > +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > @@ -0,0 +1,428 @@
> > +/** @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 XSDT/RSDT.
> > +
> > +  @param Sdt                    ACPI XSDT/RSDT.
> > +  @param TablePointerSize       Size of table pointer: 8(XSDT) or
> > 4(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 *
> > +ScanTableInSDT (
> > +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
> > +  IN  UINTN                         TablePointerSize,
> > +  IN  UINT32                        Signature,
> > +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable,
> OPTIONAL
> > +  OUT BOOLEAN                       *PreviousTableLocated
> > OPTIONAL
> > +  )
> > +{
> > +  UINTN                             Index;
> > +  UINTN                             EntryCount;
> > +  UINT64                            EntryPtr;
> > +  UINTN                             BasePtr;
> > +  EFI_ACPI_COMMON_HEADER            *Table;
> > +
> > +  if (PreviousTableLocated != NULL) {
> > +    ASSERT (PreviousTable != NULL);
> > +    *PreviousTableLocated = FALSE;
> > +  } else {
> > +    ASSERT (PreviousTable == NULL);  }
> > +
> > +  if (Sdt == NULL) {
> > +    return NULL;
> > +  }
> > +
> > +  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER))
> /
> > TablePointerSize;
> > +
> > +  BasePtr = (UINTN)(Sdt + 1);
> > +  for (Index = 0; Index < EntryCount; Index ++) {
> > +    EntryPtr = 0;
> > +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * 
> > + TablePointerSize),
> > TablePointerSize);
> > +    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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +               NULL,
> > +               NULL
> > +               );
> > +      Table = LocateAcpiFacsFromFadt (Fadt);
> > +    } else {
> > +      Table = ScanTableInSDT (
> > +                Xsdt,
> > +                sizeof (UINT64),
> > +                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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +             NULL,
> > +             NULL
> > +             );
> > +    Table = LocateAcpiFacsFromFadt (Fadt);  } else {
> > +    Table = ScanTableInSDT (
> > +              Rsdt,
> > +              sizeof (UINT32),
> > +              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.
> > +
> > +  It's not supported that PreviousTable is not NULL but
> > PreviousTable->Signature
> > +  is not same with Signature, NULL will be returned.
> > +
> > +  @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.
> > +      //
> > +      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > index 182d20fca051..2d333b1b4b2a 100644
> > --- 
> > a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > +++
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > @@ -37,6 +37,7 @@ [Sources]
> >    Console.c
> >    UefiLib.c
> >    UefiLibInternal.h
> > +  Acpi.c
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > @@ -55,6 +56,8 @@ [LibraryClasses]
> >  [Guids]
> >    gEfiEventReadyToBootGuid                      ##
> > SOMETIMES_CONSUMES  ## Event
> >    gEfiEventLegacyBootGuid                       ##
> > SOMETIMES_CONSUMES  ## Event
> > +  gEfiAcpi20TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> > +  gEfiAcpi10TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> >
> >  [Protocols]
> >    gEfiDriverBindingProtocolGuid                 ##
> > SOMETIMES_PRODUCES
> > --
> > 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
@ 2018-09-19  2:38 Star Zeng
  2018-09-19  5:58 ` Yao, Jiewen
  0 siblings, 1 reply; 9+ messages in thread
From: Star Zeng @ 2018-09-19  2:38 UTC (permalink / raw)
  To: edk2-devel
  Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao

It updates FrameworkUefiLib and is the supplement to
https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html

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.

V2:
ASSERT(FALSE) and return NULL instead of real implementation for the
new APIs as the IntelFrameworkPkg is a to-be-deprecated packages.

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>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---
 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 88 ++++++++++++++++++++++
 .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |  1 +
 2 files changed, 89 insertions(+)
 create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c

diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
new file mode 100644
index 000000000000..4b41abd036f9
--- /dev/null
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
@@ -0,0 +1,88 @@
+/** @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>
+
+/**
+  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.
+
+  It's not supported that PreviousTable is not NULL but PreviousTable->Signature
+  is not same with Signature, NULL will be returned.
+
+  @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
+  )
+{
+  ASSERT (FALSE);
+  return NULL;
+}
+
+/**
+  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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
index 182d20fca051..086d898ee7dc 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
@@ -37,6 +37,7 @@ [Sources]
   Console.c
   UefiLib.c
   UefiLibInternal.h
+  Acpi.c
 
 [Packages]
   MdePkg/MdePkg.dec
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  2:15       ` Zeng, Star
@ 2018-09-19  2:39         ` Zeng, Star
  0 siblings, 0 replies; 9+ messages in thread
From: Zeng, Star @ 2018-09-19  2:39 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org
  Cc: Younas khan, Kinney, Michael D, Gao, Liming, Zeng, Star

Just sent V2 patch at https://lists.01.org/pipermail/edk2-devel/2018-September/029849.html with this idea.

Thanks,
Star
-----Original Message-----
From: Zeng, Star 
Sent: Wednesday, September 19, 2018 10:15 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs

I think it is a good idea.

Thanks,
Star
-----Original Message-----
From: Yao, Jiewen
Sent: Wednesday, September 19, 2018 10:13 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs

Yes.

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, September 19, 2018 10:11 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; 
> Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> EfiLocateXXXAcpiTable APIs
> 
> You mean like below for the case?
> ASSERT(FALSE);
> return NULL;
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, September 19, 2018 10:08 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Younas khan <pmdyounaskhan786@gmail.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> EfiLocateXXXAcpiTable APIs
> 
> Hi
> I do not recommend we add more new feature to a to-be-deprecated 
> package.
> 
> Can we just return UNSUPPORTED?
> 
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Wednesday, September 19, 2018 9:28 AM
> > 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>
> > Subject: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new 
> > EfiLocateXXXAcpiTable APIs
> >
> > It updates FrameworkUefiLib and is the supplement to 
> > https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> >
> > 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>
> > ---
> >  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 428
> > +++++++++++++++++++++
> >  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |   3 +
> >  2 files changed, 431 insertions(+)
> >  create mode 100644
> IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> >
> > diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > new file mode 100644
> > index 000000000000..e16ccc7e7b74
> > --- /dev/null
> > +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> > @@ -0,0 +1,428 @@
> > +/** @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 XSDT/RSDT.
> > +
> > +  @param Sdt                    ACPI XSDT/RSDT.
> > +  @param TablePointerSize       Size of table pointer: 8(XSDT) or
> > 4(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 *
> > +ScanTableInSDT (
> > +  IN  EFI_ACPI_DESCRIPTION_HEADER   *Sdt,
> > +  IN  UINTN                         TablePointerSize,
> > +  IN  UINT32                        Signature,
> > +  IN  EFI_ACPI_COMMON_HEADER        *PreviousTable,
> OPTIONAL
> > +  OUT BOOLEAN                       *PreviousTableLocated
> > OPTIONAL
> > +  )
> > +{
> > +  UINTN                             Index;
> > +  UINTN                             EntryCount;
> > +  UINT64                            EntryPtr;
> > +  UINTN                             BasePtr;
> > +  EFI_ACPI_COMMON_HEADER            *Table;
> > +
> > +  if (PreviousTableLocated != NULL) {
> > +    ASSERT (PreviousTable != NULL);
> > +    *PreviousTableLocated = FALSE;
> > +  } else {
> > +    ASSERT (PreviousTable == NULL);  }
> > +
> > +  if (Sdt == NULL) {
> > +    return NULL;
> > +  }
> > +
> > +  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER))
> /
> > TablePointerSize;
> > +
> > +  BasePtr = (UINTN)(Sdt + 1);
> > +  for (Index = 0; Index < EntryCount; Index ++) {
> > +    EntryPtr = 0;
> > +    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * 
> > + TablePointerSize),
> > TablePointerSize);
> > +    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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +               Xsdt,
> > +               sizeof (UINT64),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +               NULL,
> > +               NULL
> > +               );
> > +      Table = LocateAcpiFacsFromFadt (Fadt);
> > +    } else {
> > +      Table = ScanTableInSDT (
> > +                Xsdt,
> > +                sizeof (UINT64),
> > +                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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > 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 *)
> > ScanTableInSDT (
> > +             Rsdt,
> > +             sizeof (UINT32),
> > +
> > EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> > +             NULL,
> > +             NULL
> > +             );
> > +    Table = LocateAcpiFacsFromFadt (Fadt);  } else {
> > +    Table = ScanTableInSDT (
> > +              Rsdt,
> > +              sizeof (UINT32),
> > +              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.
> > +
> > +  It's not supported that PreviousTable is not NULL but
> > PreviousTable->Signature
> > +  is not same with Signature, NULL will be returned.
> > +
> > +  @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.
> > +      //
> > +      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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > index 182d20fca051..2d333b1b4b2a 100644
> > ---
> > a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > +++
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> > @@ -37,6 +37,7 @@ [Sources]
> >    Console.c
> >    UefiLib.c
> >    UefiLibInternal.h
> > +  Acpi.c
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > @@ -55,6 +56,8 @@ [LibraryClasses]
> >  [Guids]
> >    gEfiEventReadyToBootGuid                      ##
> > SOMETIMES_CONSUMES  ## Event
> >    gEfiEventLegacyBootGuid                       ##
> > SOMETIMES_CONSUMES  ## Event
> > +  gEfiAcpi20TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> > +  gEfiAcpi10TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> >
> >  [Protocols]
> >    gEfiDriverBindingProtocolGuid                 ##
> > SOMETIMES_PRODUCES
> > --
> > 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs
  2018-09-19  2:38 Star Zeng
@ 2018-09-19  5:58 ` Yao, Jiewen
  0 siblings, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2018-09-19  5:58 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Younas khan, Gao, Liming, Zeng, Star

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Wednesday, September 19, 2018 10:38 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new
> EfiLocateXXXAcpiTable APIs
> 
> It updates FrameworkUefiLib and is the supplement to
> https://lists.01.org/pipermail/edk2-devel/2018-September/029757.html
> 
> 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.
> 
> V2:
> ASSERT(FALSE) and return NULL instead of real implementation for the
> new APIs as the IntelFrameworkPkg is a to-be-deprecated packages.
> 
> 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>
> Reviewed-by: Liming Gao <liming.gao@intel.com>
> ---
>  IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c  | 88
> ++++++++++++++++++++++
>  .../Library/FrameworkUefiLib/FrameworkUefiLib.inf  |  1 +
>  2 files changed, 89 insertions(+)
>  create mode 100644 IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> 
> diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> new file mode 100644
> index 000000000000..4b41abd036f9
> --- /dev/null
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/Acpi.c
> @@ -0,0 +1,88 @@
> +/** @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>
> +
> +/**
> +  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.
> +
> +  It's not supported that PreviousTable is not NULL but
> PreviousTable->Signature
> +  is not same with Signature, NULL will be returned.
> +
> +  @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
> +  )
> +{
> +  ASSERT (FALSE);
> +  return NULL;
> +}
> +
> +/**
> +  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/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> index 182d20fca051..086d898ee7dc 100644
> --- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
> @@ -37,6 +37,7 @@ [Sources]
>    Console.c
>    UefiLib.c
>    UefiLibInternal.h
> +  Acpi.c
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> --
> 2.7.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-09-19  5:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-19  1:28 [PATCH] IntelFrameworkPkg FrameworkUefiLib: Add new EfiLocateXXXAcpiTable APIs Star Zeng
2018-09-19  1:35 ` Gao, Liming
2018-09-19  2:08 ` Yao, Jiewen
2018-09-19  2:10   ` Zeng, Star
2018-09-19  2:13     ` Yao, Jiewen
2018-09-19  2:15       ` Zeng, Star
2018-09-19  2:39         ` Zeng, Star
  -- strict thread matches above, loose matches on Subject: below --
2018-09-19  2:38 Star Zeng
2018-09-19  5:58 ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox