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

["Yao, Jiewen" <jiewen.yao@intel.com>]

Good idea on LocateNextAcpiTable().


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