Good idea. I did consider DSDT and multiple SSDTs cases. But I did not find any real case for them. So I made the code simply for current cases, and the code can be easily enhanced later for DSDT, a new API can be added later for multiple SSDTs.
About adding the new API in UefiLib VS new library class, I did also consider it and even created code for new library class (g...@github.com:lzeng14/edk2.git branch FindAcpiTableBySignature_UefiAcpiTableLib). But I remember I did discuss it with Mike and Jiewen, we recommended UefiLib as it will do not require any platform change. Thanks, Star -----Original Message----- From: Yao, Jiewen Sent: Saturday, September 1, 2018 7:04 AM To: Ni, Ruiyu <ruiyu...@intel.com> Cc: Zeng, Star <star.z...@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kin...@intel.com>; Younas khan <pmdyounaskhan...@gmail.com>; Gao, Liming <liming....@intel.com> Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API Good idea on LocateNextAcpiTable(). > -----Original Message----- > From: Ni, Ruiyu > Sent: Saturday, September 1, 2018 12:29 AM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: Zeng, Star <star.z...@intel.com>; edk2-devel@lists.01.org; Kinney, > Michael D <michael.d.kin...@intel.com>; Younas khan > <pmdyounaskhan...@gmail.com>; Gao, Liming <liming....@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....@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....@intel.com > > > > Thank you > > Yao Jiewen > > > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-boun...@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.kin...@intel.com>; Younas khan > >> <pmdyounaskhan...@gmail.com>; Yao, Jiewen <jiewen....@intel.com>; > Gao, > >> Liming <liming....@intel.com>; Zeng, Star <star.z...@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 <pmdyounaskhan...@gmail.com> > >> Cc: Michael D Kinney <michael.d.kin...@intel.com> > >> Cc: Liming Gao <liming....@intel.com> > >> Cc: Jiewen Yao <jiewen....@intel.com> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Star Zeng <star.z...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
