From: "Zeng, Star" <star.zeng@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
"Ni, Ruiyu" <ruiyu.ni@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Younas khan <pmdyounaskhan786@gmail.com>,
"Gao, Liming" <liming.gao@intel.com>,
"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
Date: Mon, 3 Sep 2018 03:26:09 +0000 [thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BBB49F2@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AD37F11@shsmsx102.ccr.corp.intel.com>
Good idea.
I did consider DSDT and multiple SSDTs cases. But I did not find any real case for them.
So I made the code simply for current cases, and the code can be easily enhanced later for DSDT, a new API can be added later for multiple SSDTs.
About adding the new API in UefiLib VS new library class, I did also consider it and even created code for new library class (git@github.com:lzeng14/edk2.git branch FindAcpiTableBySignature_UefiAcpiTableLib).
But I remember I did discuss it with Mike and Jiewen, we recommended UefiLib as it will do not require any platform change.
Thanks,
Star
-----Original Message-----
From: Yao, Jiewen
Sent: Saturday, September 1, 2018 7:04 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new EfiFindAcpiTableBySignature() API
Good idea on LocateNextAcpiTable().
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Saturday, September 1, 2018 12:29 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney,
> Michael D <michael.d.kinney@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> EfiFindAcpiTableBySignature() API
>
> I think LocateNextAcpiTable() is more proper to handle the multiple
> tables with same signature. It will carry three parameters, one is the
> table header stored in configuration table, one is the signature, another is the previous located table.
> Can we return a common table header other than void*?
>
> Is there better place other than UefiLib?
> Do we need to add a new library class like AcpiLib?
>
> 发自我的 iPhone
>
> > 在 2018年8月31日,下午8:00,Yao, Jiewen <jiewen.yao@intel.com> 写
> 道:
> >
> > Good enhancement.
> >
> > I have 2 additional thought:
> >
> > 1) How to handle DSDT?
> > We have special code to handle FACS, but no DSDT.
> >
> > 2) How to handle SSDT or other multiple ACPI tables?
> > We may have multiple SSDT. Usually, it is identified as OEMID.
> > Do we want to provide similar function for them?
> >
> > Anyway, just *additional* thought. :-) Current implementation is
> > good enough for check in.
> >
> > Reviewed-by: Jiewen.yao@intel.com
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> >> Of
> Star
> >> Zeng
> >> Sent: Friday, August 31, 2018 7:29 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan
> >> <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Gao,
> >> Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: [edk2] [PATCH 1/6] MdePkg UefiLib: Add new
> >> EfiFindAcpiTableBySignature() API
> >>
> >> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> >> Request to add a library function for GetAcpiTable() in order to
> >> get ACPI table using signature as input.
> >>
> >> After evaluation, we found there are many duplicated code to find
> >> ACPI table by signature in different modules.
> >>
> >> This patch adds new EfiFindAcpiTableBySignature() API in UefiLib
> >> for the request and also the following patch to remove the
> >> duplicated code.
> >>
> >> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >> Cc: Liming Gao <liming.gao@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Star Zeng <star.zeng@intel.com>
> >> ---
> >> MdePkg/Include/Library/UefiLib.h | 17 +++
> >> MdePkg/Library/UefiLib/Acpi.c | 226
> >> +++++++++++++++++++++++++++++++++++++
> >> MdePkg/Library/UefiLib/UefiLib.inf | 3 +
> >> 3 files changed, 246 insertions(+)
> >> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> >>
> >> diff --git a/MdePkg/Include/Library/UefiLib.h
> >> b/MdePkg/Include/Library/UefiLib.h
> >> index f80067f11103..8dd25f324fd2 100644
> >> --- a/MdePkg/Include/Library/UefiLib.h
> >> +++ b/MdePkg/Include/Library/UefiLib.h
> >> @@ -1595,4 +1595,21 @@ EfiOpenFileByDevicePath (
> >> IN UINT64 OpenMode,
> >> IN UINT64 Attributes
> >> );
> >> +
> >> +/**
> >> + This function finds ACPI table by signature.
> >> + It will find the table in gEfiAcpi20TableGuid system
> >> +configuration table
> first,
> >> + and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> + @param Signature ACPI table signature.
> >> +
> >> + @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> + IN UINT32 Signature
> >> + );
> >> +
> >> #endif
> >> diff --git a/MdePkg/Library/UefiLib/Acpi.c
> >> b/MdePkg/Library/UefiLib/Acpi.c new file mode 100644 index
> >> 000000000000..5cb93966b59f
> >> --- /dev/null
> >> +++ b/MdePkg/Library/UefiLib/Acpi.c
> >> @@ -0,0 +1,226 @@
> >> +/** @file
> >> + This module provides help function for finding ACPI table.
> >> +
> >> + Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> >> + This program and the accompanying materials are licensed and
> >> + made available under the terms and conditions of the
> BSD
> >> License
> >> + which accompanies this distribution. The full text of the
> >> + license may be
> >> found at
> >> + http://opensource.org/licenses/bsd-license.php.
> >> +
> >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> >> BASIS,
> >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> >> EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include "UefiLibInternal.h"
> >> +#include <IndustryStandard/Acpi.h> #include <Guid/Acpi.h>
> >> +
> >> +/**
> >> + This function scans ACPI table in RSDT.
> >> +
> >> + @param Rsdt ACPI RSDT
> >> + @param Signature ACPI table signature
> >> +
> >> + @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInRSDT (
> >> + IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
> >> + IN UINT32 Signature
> >> + )
> >> +{
> >> + UINTN Index;
> >> + UINT32 EntryCount;
> >> + UINT32 *EntryPtr;
> >> + EFI_ACPI_DESCRIPTION_HEADER *Table;
> >> +
> >> + if (Rsdt == NULL) {
> >> + return NULL;
> >> + }
> >> +
> >> + EntryCount = (Rsdt->Length - sizeof
> >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT32);
> >> +
> >> + EntryPtr = (UINT32 *)(Rsdt + 1); for (Index = 0; Index <
> >> + EntryCount; Index ++, EntryPtr ++) {
> >> + Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
> >> + if (Table->Signature == Signature) {
> >> + return Table;
> >> + }
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +/**
> >> + This function scans ACPI table in XSDT.
> >> +
> >> + @param Xsdt ACPI XSDT
> >> + @param Signature ACPI table signature
> >> +
> >> + @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +ScanTableInXSDT (
> >> + IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
> >> + IN UINT32 Signature
> >> + )
> >> +{
> >> + UINTN Index;
> >> + UINT32 EntryCount;
> >> + UINT64 EntryPtr;
> >> + UINTN BasePtr;
> >> + EFI_ACPI_DESCRIPTION_HEADER *Table;
> >> +
> >> + if (Xsdt == NULL) {
> >> + return NULL;
> >> + }
> >> +
> >> + EntryCount = (Xsdt->Length - sizeof
> >> + (EFI_ACPI_DESCRIPTION_HEADER)) /
> >> sizeof(UINT64);
> >> +
> >> + BasePtr = (UINTN)(Xsdt + 1);
> >> + for (Index = 0; Index < EntryCount; Index ++) {
> >> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index *
> >> + sizeof(UINT64)),
> >> sizeof(UINT64));
> >> + Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
> >> + if (Table->Signature == Signature) {
> >> + return Table;
> >> + }
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +/**
> >> + To find Facs in FADT.
> >> +
> >> + @param Fadt FADT table pointer
> >> +
> >> + @return Facs table pointer or NULL if not found.
> >> +
> >> +**/
> >> +EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
> >> +FindAcpiFacsFromFadt (
> >> + IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> >> + )
> >> +{
> >> + EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
> >> + UINT64 Data64;
> >> +
> >> + if (Fadt == NULL) {
> >> + return NULL;
> >> + }
> >> +
> >> + if (Fadt->Header.Revision <
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> >> + Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> + } else {
> >> + CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> >> + if (Data64 != 0) {
> >> + Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Data64;
> >> + } else {
> >> + Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE
> >> *)(UINTN)Fadt->FirmwareCtrl;
> >> + }
> >> + }
> >> + return Facs;
> >> +}
> >> +
> >> +/**
> >> + To find ACPI table in ACPI ConfigurationTable.
> >> +
> >> + @param AcpiTableGuid The guid used to find ACPI ConfigurationTable.
> >> + @param Signature ACPI table signature.
> >> +
> >> + @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +FindAcpiTableInAcpiConfigurationTable (
> >> + IN EFI_GUID *AcpiGuid,
> >> + IN UINT32 Signature
> >> +
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + VOID *Table;
> >> + EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
> >> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
> >> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
> >> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> >> +
> >> + Rsdp = NULL;
> >> + //
> >> + // Find ACPI ConfigurationTable (RSD_PTR) // Status =
> >> + EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp); if
> >> + (EFI_ERROR (Status) || (Rsdp == NULL)) {
> >> + return NULL;
> >> + }
> >> +
> >> + Table = NULL;
> >> +
> >> + //
> >> + // Search XSDT
> >> + //
> >> + if (Rsdp->Revision >=
> >> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> >> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)
> Rsdp->XsdtAddress;
> >> + if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> + //
> >> + // It is to find FACS ACPI table,
> >> + // need find FADT first.
> >> + //
> >> + Fadt = ScanTableInXSDT (Xsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> + Table = FindAcpiFacsFromFadt (Fadt);
> >> + } else {
> >> + Table = ScanTableInXSDT (Xsdt, Signature);
> >> + }
> >> + }
> >> +
> >> + if (Table != NULL) {
> >> + return Table;
> >> + }
> >> +
> >> + //
> >> + // Search RSDT
> >> + //
> >> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> >> + if (Signature ==
> >> EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> >> + //
> >> + // It is to find FACS ACPI table,
> >> + // need find FADT first.
> >> + //
> >> + Fadt = ScanTableInRSDT (Rsdt,
> >> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
> >> + Table = FindAcpiFacsFromFadt (Fadt); } else {
> >> + Table = ScanTableInRSDT (Rsdt, Signature); }
> >> +
> >> + return Table;
> >> +}
> >> +
> >> +/**
> >> + This function finds ACPI table by signature.
> >> + It will find the table in gEfiAcpi20TableGuid system
> >> +configuration table
> first,
> >> + and then gEfiAcpi10TableGuid system configuration table.
> >> +
> >> + @param Signature ACPI table signature.
> >> +
> >> + @return ACPI table or NULL if not found.
> >> +
> >> +**/
> >> +VOID *
> >> +EFIAPI
> >> +EfiFindAcpiTableBySignature (
> >> + IN UINT32 Signature
> >> + )
> >> +{
> >> + VOID *Table;
> >> +
> >> + Table = FindAcpiTableInAcpiConfigurationTable
> >> + (&gEfiAcpi20TableGuid,
> >> Signature);
> >> + if (Table != NULL) {
> >> + return Table;
> >> + }
> >> +
> >> + return FindAcpiTableInAcpiConfigurationTable
> >> + (&gEfiAcpi10TableGuid,
> >> Signature);
> >> +}
> >> +
> >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> >> b/MdePkg/Library/UefiLib/UefiLib.inf
> >> index a6c739ef3d6d..aea20fe67153 100644
> >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> >> @@ -41,6 +41,7 @@ [Sources]
> >> Console.c
> >> UefiLib.c
> >> UefiLibInternal.h
> >> + Acpi.c
> >>
> >>
> >> [Packages]
> >> @@ -62,6 +63,8 @@ [Guids]
> >> gEfiEventReadyToBootGuid ##
> >> SOMETIMES_CONSUMES ## Event
> >> gEfiEventLegacyBootGuid ##
> >> SOMETIMES_CONSUMES ## Event
> >> gEfiGlobalVariableGuid ##
> >> SOMETIMES_CONSUMES ## Variable
> >> + gEfiAcpi20TableGuid ##
> >> SOMETIMES_CONSUMES ## SystemTable
> >> + gEfiAcpi10TableGuid ##
> >> SOMETIMES_CONSUMES ## SystemTable
> >>
> >> [Protocols]
> >> gEfiDriverBindingProtocolGuid ##
> >> SOMETIMES_PRODUCES
> >> --
> >> 2.7.0.windows.1
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2018-09-03 3:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-31 11:29 [PATCH 0/6] Add new EfiFindAcpiTableBySignature() API Star Zeng
2018-08-31 11:29 ` [PATCH 1/6] MdePkg UefiLib: " Star Zeng
2018-08-31 11:57 ` Yao, Jiewen
2018-08-31 16:28 ` Ni, Ruiyu
2018-08-31 16:29 ` Ni, Ruiyu
2018-08-31 23:04 ` Yao, Jiewen
2018-09-03 3:26 ` Zeng, Star [this message]
2018-09-03 5:09 ` Ni, Ruiyu
2018-09-03 6:11 ` Zeng, Star
2018-09-03 6:14 ` Yao, Jiewen
2018-09-03 6:32 ` Ni, Ruiyu
2018-09-05 10:02 ` Zeng, Star
2018-08-31 11:29 ` [PATCH 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiFindAcpiTableBySignature() Star Zeng
2018-08-31 11:29 ` [PATCH 3/6] MdeModulePkg S3SaveStateDxe: " Star Zeng
2018-08-31 11:29 ` [PATCH 4/6] PcAtChipsetPkg PcRtc: " Star Zeng
2018-08-31 11:29 ` [PATCH 5/6] ShellPkg DpDynamicCommand: " Star Zeng
2018-08-31 11:29 ` [PATCH 6/6] UefiCpuPkg PiSmmCpuDxeSmm: " Star Zeng
2018-08-31 20:33 ` Laszlo Ersek
2018-09-03 3:28 ` Zeng, Star
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103BBB49F2@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox