* [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
@ 2018-09-13 10:26 ` Star Zeng
2018-09-14 4:40 ` Ni, Ruiyu
2018-09-13 10:26 ` [PATCH V2 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable() Star Zeng
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:26 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib
for the request and also the following patch to remove the
duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
MdePkg/Include/Library/UefiLib.h | 68 ++++++
MdePkg/Library/UefiLib/Acpi.c | 488 +++++++++++++++++++++++++++++++++++++
MdePkg/Library/UefiLib/UefiLib.inf | 3 +
3 files changed, 559 insertions(+)
create mode 100644 MdePkg/Library/UefiLib/Acpi.c
diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index f80067f11103..cf82ff4a920a 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#ifndef __UEFI_LIB_H__
#define __UEFI_LIB_H__
+#include <IndustryStandard/Acpi.h>
+
#include <Protocol/DriverBinding.h>
#include <Protocol/DriverConfiguration.h>
#include <Protocol/ComponentName.h>
@@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
IN UINT64 OpenMode,
IN UINT64 Attributes
);
+
+/**
+ This function locates next ACPI table in XSDT/RSDT based on Signature and
+ previous returned Table.
+
+ If PreviousTable is NULL:
+ This function will locate the first ACPI table in XSDT/RSDT based on
+ Signature in gEfiAcpi20TableGuid system configuration table first, and then
+ gEfiAcpi10TableGuid system configuration table.
+ This function will locate in XSDT first, and then RSDT.
+ For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+ FADT.
+ For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+ FirmwareCtrl in FADT.
+
+ If PreviousTable is not NULL:
+ 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in XSDT in
+ gEfiAcpi20TableGuid system configuration table.
+ 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi20TableGuid system configuration table.
+ 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi10TableGuid system configuration table.
+
+ If PreviousTable is not NULL and PreviousTable->Signature is not same with
+ Signature, then ASSERT.
+
+ @param Signature ACPI table signature.
+ @param PreviousTable Pointer to previous returned table to locate next
+ table, or NULL to locate first table.
+
+ @return Next ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateNextAcpiTable (
+ IN UINT32 Signature,
+ IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
+ );
+
+/**
+ This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+ This function will locate the first ACPI table in XSDT/RSDT based on
+ Signature in gEfiAcpi20TableGuid system configuration table first, and then
+ gEfiAcpi10TableGuid system configuration table.
+ This function will locate in XSDT first, and then RSDT.
+ For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+ FADT.
+ For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+ FirmwareCtrl in FADT.
+
+ @param Signature ACPI table signature.
+
+ @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+ IN UINT32 Signature
+ );
+
#endif
diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
new file mode 100644
index 000000000000..0793bbdc787f
--- /dev/null
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -0,0 +1,488 @@
+/** @file
+ This module provides help function for finding ACPI table.
+
+ Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php.
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "UefiLibInternal.h"
+#include <IndustryStandard/Acpi.h>
+#include <Guid/Acpi.h>
+
+/**
+ This function scans ACPI table in RSDT.
+
+ @param Rsdt ACPI RSDT.
+ @param Signature ACPI table signature.
+ @param PreviousTable Pointer to previous returned table to locate
+ next table, or NULL to locate first table.
+ @param PreviousTableLocated Pointer to the indicator about whether the
+ previous returned table could be located, or
+ NULL if PreviousTable is NULL.
+
+ If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+ If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+
+ @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+ScanTableInRSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
+ IN UINT32 Signature,
+ IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
+ OUT BOOLEAN *PreviousTableLocated OPTIONAL
+ )
+{
+ UINTN Index;
+ UINT32 EntryCount;
+ UINT32 *EntryPtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+
+ if (PreviousTableLocated != NULL) {
+ ASSERT (PreviousTable != NULL);
+ *PreviousTableLocated = FALSE;
+ } else {
+ ASSERT (PreviousTable == NULL);
+ }
+
+ if (Rsdt == NULL) {
+ return NULL;
+ }
+
+ EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
+
+ EntryPtr = (UINT32 *)(Rsdt + 1);
+ for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
+ Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(*EntryPtr));
+ if (Table->Signature == Signature) {
+ if (PreviousTable != NULL) {
+ if (Table == PreviousTable) {
+ *PreviousTableLocated = TRUE;
+ } else if (*PreviousTableLocated) {
+ //
+ // Return next table.
+ //
+ return Table;
+ }
+ } else {
+ //
+ // Return first table.
+ //
+ return Table;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+/**
+ This function scans ACPI table in XSDT.
+
+ @param Xsdt ACPI XSDT.
+ @param Signature ACPI table signature.
+ @param PreviousTable Pointer to previous returned table to locate
+ next table, or NULL to locate first table.
+ @param PreviousTableLocated Pointer to the indicator about whether the
+ previous returned table could be located, or
+ NULL if PreviousTable is NULL.
+
+ If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+ If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+
+ @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+ScanTableInXSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
+ IN UINT32 Signature,
+ IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
+ OUT BOOLEAN *PreviousTableLocated OPTIONAL
+ )
+{
+ UINTN Index;
+ UINT32 EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+
+ if (PreviousTableLocated != NULL) {
+ ASSERT (PreviousTable != NULL);
+ *PreviousTableLocated = FALSE;
+ } else {
+ ASSERT (PreviousTable == NULL);
+ }
+
+ if (Xsdt == NULL) {
+ return NULL;
+ }
+
+ EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
+
+ BasePtr = (UINTN)(Xsdt + 1);
+ for (Index = 0; Index < EntryCount; Index ++) {
+ CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
+ Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+ if (Table->Signature == Signature) {
+ if (PreviousTable != NULL) {
+ if (Table == PreviousTable) {
+ *PreviousTableLocated = TRUE;
+ } else if (*PreviousTableLocated) {
+ //
+ // Return next table.
+ //
+ return Table;
+ }
+ } else {
+ //
+ // Return first table.
+ //
+ return Table;
+ }
+
+ }
+ }
+
+ return NULL;
+}
+
+/**
+ To locate FACS in FADT.
+
+ @param Fadt FADT table pointer.
+
+ @return FACS table pointer or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiFacsFromFadt (
+ IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
+ )
+{
+ EFI_ACPI_COMMON_HEADER *Facs;
+ UINT64 Data64;
+
+ if (Fadt == NULL) {
+ return NULL;
+ }
+
+ if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
+ Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
+ } else {
+ CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
+ if (Data64 != 0) {
+ Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
+ } else {
+ Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
+ }
+ }
+ return Facs;
+}
+
+/**
+ To locate DSDT in FADT.
+
+ @param Fadt FADT table pointer.
+
+ @return DSDT table pointer or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiDsdtFromFadt (
+ IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
+ )
+{
+ EFI_ACPI_COMMON_HEADER *Dsdt;
+ UINT64 Data64;
+
+ if (Fadt == NULL) {
+ return NULL;
+ }
+
+ if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
+ Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
+ } else {
+ CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
+ if (Data64 != 0) {
+ Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
+ } else {
+ Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
+ }
+ }
+ return Dsdt;
+}
+
+/**
+ To locate ACPI table in ACPI ConfigurationTable.
+
+ @param AcpiTableGuid The GUID used to get ACPI ConfigurationTable.
+ @param Signature ACPI table signature.
+ @param PreviousTable Pointer to previous returned table to locate
+ next table, or NULL to locate first table.
+ @param PreviousTableLocated Pointer to the indicator to return whether the
+ previous returned table could be located or not,
+ or NULL if PreviousTable is NULL.
+
+ If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
+ If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
+ If AcpiGuid is NULL, then ASSERT().
+
+ @return ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+LocateAcpiTableInAcpiConfigurationTable (
+ IN EFI_GUID *AcpiGuid,
+ IN UINT32 Signature,
+ IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
+ OUT BOOLEAN *PreviousTableLocated OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_COMMON_HEADER *Table;
+ EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
+
+ if (PreviousTableLocated != NULL) {
+ ASSERT (PreviousTable != NULL);
+ *PreviousTableLocated = FALSE;
+ } else {
+ ASSERT (PreviousTable == NULL);
+ }
+
+ Rsdp = NULL;
+ //
+ // Get ACPI ConfigurationTable (RSD_PTR)
+ //
+ Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+ return NULL;
+ }
+
+ Table = NULL;
+
+ //
+ // Search XSDT
+ //
+ if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
+ if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+ ASSERT (PreviousTable == NULL);
+ //
+ // It is to locate DSDT,
+ // need to locate FADT first.
+ //
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
+ Xsdt,
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ NULL,
+ NULL
+ );
+ Table = LocateAcpiDsdtFromFadt (Fadt);
+ } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+ ASSERT (PreviousTable == NULL);
+ //
+ // It is to locate FACS,
+ // need to locate FADT first.
+ //
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
+ Xsdt,
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ NULL,
+ NULL
+ );
+ Table = LocateAcpiFacsFromFadt (Fadt);
+ } else {
+ Table = ScanTableInXSDT (
+ Xsdt,
+ Signature,
+ PreviousTable,
+ PreviousTableLocated
+ );
+ }
+ }
+
+ if (Table != NULL) {
+ return Table;
+ } else if ((PreviousTableLocated != NULL) &&
+ *PreviousTableLocated) {
+ //
+ // PreviousTable could be located in XSDT,
+ // but next table could not be located in XSDT.
+ //
+ return NULL;
+ }
+
+ //
+ // Search RSDT
+ //
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
+ if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+ ASSERT (PreviousTable == NULL);
+ //
+ // It is to locate DSDT,
+ // need to locate FADT first.
+ //
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
+ Rsdt,
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ NULL,
+ NULL
+ );
+ Table = LocateAcpiDsdtFromFadt (Fadt);
+ } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+ ASSERT (PreviousTable == NULL);
+ //
+ // It is to locate FACS,
+ // need to locate FADT first.
+ //
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
+ Rsdt,
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+ NULL,
+ NULL
+ );
+ Table = LocateAcpiFacsFromFadt (Fadt);
+ } else {
+ Table = ScanTableInRSDT (
+ Rsdt,
+ Signature,
+ PreviousTable,
+ PreviousTableLocated
+ );
+ }
+
+ return Table;
+}
+
+/**
+ This function locates next ACPI table in XSDT/RSDT based on Signature and
+ previous returned Table.
+
+ If PreviousTable is NULL:
+ This function will locate the first ACPI table in XSDT/RSDT based on
+ Signature in gEfiAcpi20TableGuid system configuration table first, and then
+ gEfiAcpi10TableGuid system configuration table.
+ This function will locate in XSDT first, and then RSDT.
+ For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+ FADT.
+ For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+ FirmwareCtrl in FADT.
+
+ If PreviousTable is not NULL:
+ 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in XSDT in
+ gEfiAcpi20TableGuid system configuration table.
+ 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi20TableGuid system configuration table.
+ 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi10TableGuid system configuration table.
+
+ If PreviousTable is not NULL and PreviousTable->Signature is not same with
+ Signature, then ASSERT.
+
+ @param Signature ACPI table signature.
+ @param PreviousTable Pointer to previous returned table to locate next
+ table, or NULL to locate first table.
+
+ @return Next ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateNextAcpiTable (
+ IN UINT32 Signature,
+ IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
+ )
+{
+ EFI_ACPI_COMMON_HEADER *Table;
+ BOOLEAN TempPreviousTableLocated;
+ BOOLEAN *PreviousTableLocated;
+
+ if (PreviousTable != NULL) {
+ if (PreviousTable->Signature != Signature) {
+ //
+ // PreviousTable->Signature is not same with Signature.
+ //
+ ASSERT (FALSE);
+ return NULL;
+ } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
+ (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
+ (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
+ //
+ // There is only one FADT/DSDT/FACS table,
+ // so don't try to locate next one.
+ //
+ return NULL;
+ }
+
+ PreviousTableLocated = &TempPreviousTableLocated;
+ *PreviousTableLocated = FALSE;
+ } else {
+ PreviousTableLocated = NULL;
+ }
+
+ Table = LocateAcpiTableInAcpiConfigurationTable (
+ &gEfiAcpi20TableGuid,
+ Signature,
+ PreviousTable,
+ PreviousTableLocated
+ );
+ if (Table != NULL) {
+ return Table;
+ } else if ((PreviousTableLocated != NULL) &&
+ *PreviousTableLocated) {
+ //
+ // PreviousTable could be located in gEfiAcpi20TableGuid system
+ // configuration table, but next table could not be located in
+ // gEfiAcpi20TableGuid system configuration table.
+ //
+ return NULL;
+ }
+
+ return LocateAcpiTableInAcpiConfigurationTable (
+ &gEfiAcpi10TableGuid,
+ Signature,
+ PreviousTable,
+ PreviousTableLocated
+ );
+}
+
+/**
+ This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+ This function will locate the first ACPI table in XSDT/RSDT based on
+ Signature in gEfiAcpi20TableGuid system configuration table first, and then
+ gEfiAcpi10TableGuid system configuration table.
+ This function will locate in XSDT first, and then RSDT.
+ For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+ FADT.
+ For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+ FirmwareCtrl in FADT.
+
+ @param Signature ACPI table signature.
+
+ @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+ IN UINT32 Signature
+ )
+{
+ return EfiLocateNextAcpiTable (Signature, NULL);
+}
diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
index a6c739ef3d6d..aea20fe67153 100644
--- a/MdePkg/Library/UefiLib/UefiLib.inf
+++ b/MdePkg/Library/UefiLib/UefiLib.inf
@@ -41,6 +41,7 @@ [Sources]
Console.c
UefiLib.c
UefiLibInternal.h
+ Acpi.c
[Packages]
@@ -62,6 +63,8 @@ [Guids]
gEfiEventReadyToBootGuid ## SOMETIMES_CONSUMES ## Event
gEfiEventLegacyBootGuid ## SOMETIMES_CONSUMES ## Event
gEfiGlobalVariableGuid ## SOMETIMES_CONSUMES ## Variable
+ gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
+ gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
[Protocols]
gEfiDriverBindingProtocolGuid ## SOMETIMES_PRODUCES
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
2018-09-13 10:26 ` [PATCH V2 1/6] MdePkg UefiLib: " Star Zeng
@ 2018-09-14 4:40 ` Ni, Ruiyu
2018-09-17 7:13 ` Zeng, Star
0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ruiyu @ 2018-09-14 4:40 UTC (permalink / raw)
To: Star Zeng, edk2-devel
Cc: Michael D Kinney, Younas khan, Jiewen Yao, Liming Gao
Star,
I have two comments. see below.
On 9/13/2018 6:26 PM, Star Zeng wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order
> to get ACPI table using signature as input.
>
> After evaluation, we found there are many duplicated code to
> find ACPI table by signature in different modules.
>
> This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib
> for the request and also the following patch to remove the
> duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> MdePkg/Include/Library/UefiLib.h | 68 ++++++
> MdePkg/Library/UefiLib/Acpi.c | 488 +++++++++++++++++++++++++++++++++++++
> MdePkg/Library/UefiLib/UefiLib.inf | 3 +
> 3 files changed, 559 insertions(+)
> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
>
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index f80067f11103..cf82ff4a920a 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #ifndef __UEFI_LIB_H__
> #define __UEFI_LIB_H__
>
> +#include <IndustryStandard/Acpi.h>
> +
> #include <Protocol/DriverBinding.h>
> #include <Protocol/DriverConfiguration.h>
> #include <Protocol/ComponentName.h>
> @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
> IN UINT64 OpenMode,
> IN UINT64 Attributes
> );
> +
> +/**
> + This function locates next ACPI table in XSDT/RSDT based on Signature and
> + previous returned Table.
> +
> + If PreviousTable is NULL:
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + If PreviousTable is not NULL:
> + 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in XSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi10TableGuid system configuration table.
> +
1. I don't see difference between the case when PreviousTable is NULL or
not. We don't need to distinguish between the two cases in above comments.
> + If PreviousTable is not NULL and PreviousTable->Signature is not same with
> + Signature, then ASSERT.
I'd suggest to return Invalid Parameter instead of assertion.
> +
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate next
> + table, or NULL to locate first table.
> +
> + @return Next ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateNextAcpiTable (
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
> + );
> +
> +/**
> + This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + @param Signature ACPI table signature.
> +
> + @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> + IN UINT32 Signature
> + );
> +
> #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> new file mode 100644
> index 000000000000..0793bbdc787f
> --- /dev/null
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -0,0 +1,488 @@
> +/** @file
> + This module provides help function for finding ACPI table.
> +
> + Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php.
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "UefiLibInternal.h"
> +#include <IndustryStandard/Acpi.h>
> +#include <Guid/Acpi.h>
> +
> +/**
> + This function scans ACPI table in RSDT.
> +
> + @param Rsdt ACPI RSDT.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator about whether the
> + previous returned table could be located, or
> + NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInRSDT (
> + IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
> +{
> + UINTN Index;
> + UINT32 EntryCount;
> + UINT32 *EntryPtr;
> + EFI_ACPI_COMMON_HEADER *Table;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + if (Rsdt == NULL) {
> + return NULL;
> + }
> +
> + EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
> +
> + EntryPtr = (UINT32 *)(Rsdt + 1);
> + for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(*EntryPtr));
> + if (Table->Signature == Signature) {
> + if (PreviousTable != NULL) {
> + if (Table == PreviousTable) {
> + *PreviousTableLocated = TRUE;
> + } else if (*PreviousTableLocated) {
> + //
> + // Return next table.
> + //
> + return Table;
> + }
> + } else {
> + //
> + // Return first table.
> + //
> + return Table;
> + }
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + This function scans ACPI table in XSDT.
> +
> + @param Xsdt ACPI XSDT.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator about whether the
> + previous returned table could be located, or
> + NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInXSDT (
> + IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
2. Can we merge ScanTableInRSDT and ScanTableInXSDT?
> +{
> + UINTN Index;
> + UINT32 EntryCount;
> + UINT64 EntryPtr;
> + UINTN BasePtr;
> + EFI_ACPI_COMMON_HEADER *Table;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + if (Xsdt == NULL) {
> + return NULL;
> + }
> +
> + EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
> +
> + BasePtr = (UINTN)(Xsdt + 1);
> + for (Index = 0; Index < EntryCount; Index ++) {
> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
> + if (Table->Signature == Signature) {
> + if (PreviousTable != NULL) {
> + if (Table == PreviousTable) {
> + *PreviousTableLocated = TRUE;
> + } else if (*PreviousTableLocated) {
> + //
> + // Return next table.
> + //
> + return Table;
> + }
> + } else {
> + //
> + // Return first table.
> + //
> + return Table;
> + }
> +
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + To locate FACS in FADT.
> +
> + @param Fadt FADT table pointer.
> +
> + @return FACS table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiFacsFromFadt (
> + IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Facs;
> + UINT64 Data64;
> +
> + if (Fadt == NULL) {
> + return NULL;
> + }
> +
> + if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> + } else {
> + CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> + if (Data64 != 0) {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> + } else {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> + }
> + }
> + return Facs;
> +}
> +
> +/**
> + To locate DSDT in FADT.
> +
> + @param Fadt FADT table pointer.
> +
> + @return DSDT table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiDsdtFromFadt (
> + IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Dsdt;
> + UINT64 Data64;
> +
> + if (Fadt == NULL) {
> + return NULL;
> + }
> +
> + if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> + } else {
> + CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
> + if (Data64 != 0) {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> + } else {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> + }
> + }
> + return Dsdt;
> +}
> +
> +/**
> + To locate ACPI table in ACPI ConfigurationTable.
> +
> + @param AcpiTableGuid The GUID used to get ACPI ConfigurationTable.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator to return whether the
> + previous returned table could be located or not,
> + or NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> + If AcpiGuid is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiTableInAcpiConfigurationTable (
> + IN EFI_GUID *AcpiGuid,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
> +{
> + EFI_STATUS Status;
> + EFI_ACPI_COMMON_HEADER *Table;
> + EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + Rsdp = NULL;
> + //
> + // Get ACPI ConfigurationTable (RSD_PTR)
> + //
> + Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> + return NULL;
> + }
> +
> + Table = NULL;
> +
> + //
> + // Search XSDT
> + //
> + if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
> + if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate DSDT,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> + Xsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiDsdtFromFadt (Fadt);
> + } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate FACS,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> + Xsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiFacsFromFadt (Fadt);
> + } else {
> + Table = ScanTableInXSDT (
> + Xsdt,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + }
> + }
> +
> + if (Table != NULL) {
> + return Table;
> + } else if ((PreviousTableLocated != NULL) &&
> + *PreviousTableLocated) {
> + //
> + // PreviousTable could be located in XSDT,
> + // but next table could not be located in XSDT.
> + //
> + return NULL;
> + }
> +
> + //
> + // Search RSDT
> + //
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> + if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate DSDT,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> + Rsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiDsdtFromFadt (Fadt);
> + } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate FACS,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> + Rsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiFacsFromFadt (Fadt);
> + } else {
> + Table = ScanTableInRSDT (
> + Rsdt,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + }
> +
> + return Table;
> +}
> +
> +/**
> + This function locates next ACPI table in XSDT/RSDT based on Signature and
> + previous returned Table.
> +
> + If PreviousTable is NULL:
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + If PreviousTable is not NULL:
> + 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in XSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi10TableGuid system configuration table.
> +
> + If PreviousTable is not NULL and PreviousTable->Signature is not same with
> + Signature, then ASSERT.
> +
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate next
> + table, or NULL to locate first table.
> +
> + @return Next ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateNextAcpiTable (
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Table;
> + BOOLEAN TempPreviousTableLocated;
> + BOOLEAN *PreviousTableLocated;
> +
> + if (PreviousTable != NULL) {
> + if (PreviousTable->Signature != Signature) {
> + //
> + // PreviousTable->Signature is not same with Signature.
> + //
> + ASSERT (FALSE);
> + return NULL;
> + } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
> + (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
> + (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
> + //
> + // There is only one FADT/DSDT/FACS table,
> + // so don't try to locate next one.
> + //
> + return NULL;
> + }
> +
> + PreviousTableLocated = &TempPreviousTableLocated;
> + *PreviousTableLocated = FALSE;
> + } else {
> + PreviousTableLocated = NULL;
> + }
> +
> + Table = LocateAcpiTableInAcpiConfigurationTable (
> + &gEfiAcpi20TableGuid,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + if (Table != NULL) {
> + return Table;
> + } else if ((PreviousTableLocated != NULL) &&
> + *PreviousTableLocated) {
> + //
> + // PreviousTable could be located in gEfiAcpi20TableGuid system
> + // configuration table, but next table could not be located in
> + // gEfiAcpi20TableGuid system configuration table.
> + //
> + return NULL;
> + }
> +
> + return LocateAcpiTableInAcpiConfigurationTable (
> + &gEfiAcpi10TableGuid,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> +}
> +
> +/**
> + This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + @param Signature ACPI table signature.
> +
> + @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> + IN UINT32 Signature
> + )
> +{
> + return EfiLocateNextAcpiTable (Signature, NULL);
> +}
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
> index a6c739ef3d6d..aea20fe67153 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -41,6 +41,7 @@ [Sources]
> Console.c
> UefiLib.c
> UefiLibInternal.h
> + Acpi.c
>
>
> [Packages]
> @@ -62,6 +63,8 @@ [Guids]
> gEfiEventReadyToBootGuid ## SOMETIMES_CONSUMES ## Event
> gEfiEventLegacyBootGuid ## SOMETIMES_CONSUMES ## Event
> gEfiGlobalVariableGuid ## SOMETIMES_CONSUMES ## Variable
> + gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
> + gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
>
> [Protocols]
> gEfiDriverBindingProtocolGuid ## SOMETIMES_PRODUCES
>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
2018-09-14 4:40 ` Ni, Ruiyu
@ 2018-09-17 7:13 ` Zeng, Star
0 siblings, 0 replies; 14+ messages in thread
From: Zeng, Star @ 2018-09-17 7:13 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Younas khan, Yao, Jiewen, Gao, Liming,
Zeng, Star
Ray,
Thanks for the good feedbacks.
I can remove the ASSERT and update the description in the function header.
I can also merge ScanTableInRSDT and ScanTableInXSDT.
Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, September 14, 2018 12:41 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Younas khan <pmdyounaskhan786@gmail.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
Star,
I have two comments. see below.
On 9/13/2018 6:26 PM, Star Zeng wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get
> ACPI table using signature as input.
>
> After evaluation, we found there are many duplicated code to find ACPI
> table by signature in different modules.
>
> This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib for the
> request and also the following patch to remove the duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> MdePkg/Include/Library/UefiLib.h | 68 ++++++
> MdePkg/Library/UefiLib/Acpi.c | 488 +++++++++++++++++++++++++++++++++++++
> MdePkg/Library/UefiLib/UefiLib.inf | 3 +
> 3 files changed, 559 insertions(+)
> create mode 100644 MdePkg/Library/UefiLib/Acpi.c
>
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index f80067f11103..cf82ff4a920a 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #ifndef __UEFI_LIB_H__
> #define __UEFI_LIB_H__
>
> +#include <IndustryStandard/Acpi.h>
> +
> #include <Protocol/DriverBinding.h>
> #include <Protocol/DriverConfiguration.h>
> #include <Protocol/ComponentName.h>
> @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
> IN UINT64 OpenMode,
> IN UINT64 Attributes
> );
> +
> +/**
> + This function locates next ACPI table in XSDT/RSDT based on
> +Signature and
> + previous returned Table.
> +
> + If PreviousTable is NULL:
> + This function will locate the first ACPI table in XSDT/RSDT based
> + on Signature in gEfiAcpi20TableGuid system configuration table
> + first, and then gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then
> + Dsdt in FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first,
> + and then FirmwareCtrl in FADT.
> +
> + If PreviousTable is not NULL:
> + 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in XSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi10TableGuid system configuration table.
> +
1. I don't see difference between the case when PreviousTable is NULL or not. We don't need to distinguish between the two cases in above comments.
> + If PreviousTable is not NULL and PreviousTable->Signature is not same with
> + Signature, then ASSERT.
I'd suggest to return Invalid Parameter instead of assertion.
> +
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate next
> + table, or NULL to locate first table.
> +
> + @return Next ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateNextAcpiTable (
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
> + );
> +
> +/**
> + This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + @param Signature ACPI table signature.
> +
> + @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> + IN UINT32 Signature
> + );
> +
> #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> new file mode 100644
> index 000000000000..0793bbdc787f
> --- /dev/null
> +++ b/MdePkg/Library/UefiLib/Acpi.c
> @@ -0,0 +1,488 @@
> +/** @file
> + This module provides help function for finding ACPI table.
> +
> + Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php.
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "UefiLibInternal.h"
> +#include <IndustryStandard/Acpi.h>
> +#include <Guid/Acpi.h>
> +
> +/**
> + This function scans ACPI table in RSDT.
> +
> + @param Rsdt ACPI RSDT.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator about whether the
> + previous returned table could be located, or
> + NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInRSDT (
> + IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
> +{
> + UINTN Index;
> + UINT32 EntryCount;
> + UINT32 *EntryPtr;
> + EFI_ACPI_COMMON_HEADER *Table;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + if (Rsdt == NULL) {
> + return NULL;
> + }
> +
> + EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
> +
> + EntryPtr = (UINT32 *)(Rsdt + 1);
> + for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(*EntryPtr));
> + if (Table->Signature == Signature) {
> + if (PreviousTable != NULL) {
> + if (Table == PreviousTable) {
> + *PreviousTableLocated = TRUE;
> + } else if (*PreviousTableLocated) {
> + //
> + // Return next table.
> + //
> + return Table;
> + }
> + } else {
> + //
> + // Return first table.
> + //
> + return Table;
> + }
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + This function scans ACPI table in XSDT.
> +
> + @param Xsdt ACPI XSDT.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator about whether the
> + previous returned table could be located, or
> + NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +ScanTableInXSDT (
> + IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
2. Can we merge ScanTableInRSDT and ScanTableInXSDT?
> +{
> + UINTN Index;
> + UINT32 EntryCount;
> + UINT64 EntryPtr;
> + UINTN BasePtr;
> + EFI_ACPI_COMMON_HEADER *Table;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + if (Xsdt == NULL) {
> + return NULL;
> + }
> +
> + EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
> +
> + BasePtr = (UINTN)(Xsdt + 1);
> + for (Index = 0; Index < EntryCount; Index ++) {
> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
> + if (Table->Signature == Signature) {
> + if (PreviousTable != NULL) {
> + if (Table == PreviousTable) {
> + *PreviousTableLocated = TRUE;
> + } else if (*PreviousTableLocated) {
> + //
> + // Return next table.
> + //
> + return Table;
> + }
> + } else {
> + //
> + // Return first table.
> + //
> + return Table;
> + }
> +
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + To locate FACS in FADT.
> +
> + @param Fadt FADT table pointer.
> +
> + @return FACS table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiFacsFromFadt (
> + IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Facs;
> + UINT64 Data64;
> +
> + if (Fadt == NULL) {
> + return NULL;
> + }
> +
> + if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> + } else {
> + CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
> + if (Data64 != 0) {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> + } else {
> + Facs = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->FirmwareCtrl;
> + }
> + }
> + return Facs;
> +}
> +
> +/**
> + To locate DSDT in FADT.
> +
> + @param Fadt FADT table pointer.
> +
> + @return DSDT table pointer or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiDsdtFromFadt (
> + IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Dsdt;
> + UINT64 Data64;
> +
> + if (Fadt == NULL) {
> + return NULL;
> + }
> +
> + if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> + } else {
> + CopyMem (&Data64, &Fadt->XDsdt, sizeof(UINT64));
> + if (Data64 != 0) {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Data64;
> + } else {
> + Dsdt = (EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt->Dsdt;
> + }
> + }
> + return Dsdt;
> +}
> +
> +/**
> + To locate ACPI table in ACPI ConfigurationTable.
> +
> + @param AcpiTableGuid The GUID used to get ACPI ConfigurationTable.
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate
> + next table, or NULL to locate first table.
> + @param PreviousTableLocated Pointer to the indicator to return whether the
> + previous returned table could be located or not,
> + or NULL if PreviousTable is NULL.
> +
> + If PreviousTable is NULL and PreviousTableLocated is not NULL, then ASSERT().
> + If PreviousTable is not NULL and PreviousTableLocated is NULL, then ASSERT().
> + If AcpiGuid is NULL, then ASSERT().
> +
> + @return ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +LocateAcpiTableInAcpiConfigurationTable (
> + IN EFI_GUID *AcpiGuid,
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable, OPTIONAL
> + OUT BOOLEAN *PreviousTableLocated OPTIONAL
> + )
> +{
> + EFI_STATUS Status;
> + EFI_ACPI_COMMON_HEADER *Table;
> + EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> +
> + if (PreviousTableLocated != NULL) {
> + ASSERT (PreviousTable != NULL);
> + *PreviousTableLocated = FALSE;
> + } else {
> + ASSERT (PreviousTable == NULL);
> + }
> +
> + Rsdp = NULL;
> + //
> + // Get ACPI ConfigurationTable (RSD_PTR)
> + //
> + Status = EfiGetSystemConfigurationTable(AcpiGuid, (VOID **)&Rsdp);
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> + return NULL;
> + }
> +
> + Table = NULL;
> +
> + //
> + // Search XSDT
> + //
> + if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
> + if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate DSDT,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> + Xsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiDsdtFromFadt (Fadt);
> + } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate FACS,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInXSDT (
> + Xsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiFacsFromFadt (Fadt);
> + } else {
> + Table = ScanTableInXSDT (
> + Xsdt,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + }
> + }
> +
> + if (Table != NULL) {
> + return Table;
> + } else if ((PreviousTableLocated != NULL) &&
> + *PreviousTableLocated) {
> + //
> + // PreviousTable could be located in XSDT,
> + // but next table could not be located in XSDT.
> + //
> + return NULL;
> + }
> +
> + //
> + // Search RSDT
> + //
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> + if (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate DSDT,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> + Rsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiDsdtFromFadt (Fadt);
> + } else if (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
> + ASSERT (PreviousTable == NULL);
> + //
> + // It is to locate FACS,
> + // need to locate FADT first.
> + //
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) ScanTableInRSDT (
> + Rsdt,
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + NULL,
> + NULL
> + );
> + Table = LocateAcpiFacsFromFadt (Fadt);
> + } else {
> + Table = ScanTableInRSDT (
> + Rsdt,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + }
> +
> + return Table;
> +}
> +
> +/**
> + This function locates next ACPI table in XSDT/RSDT based on Signature and
> + previous returned Table.
> +
> + If PreviousTable is NULL:
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + If PreviousTable is not NULL:
> + 1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in XSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi20TableGuid system configuration table.
> + 3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi10TableGuid system configuration table.
> +
> + If PreviousTable is not NULL and PreviousTable->Signature is not same with
> + Signature, then ASSERT.
> +
> + @param Signature ACPI table signature.
> + @param PreviousTable Pointer to previous returned table to locate next
> + table, or NULL to locate first table.
> +
> + @return Next ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateNextAcpiTable (
> + IN UINT32 Signature,
> + IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
> + )
> +{
> + EFI_ACPI_COMMON_HEADER *Table;
> + BOOLEAN TempPreviousTableLocated;
> + BOOLEAN *PreviousTableLocated;
> +
> + if (PreviousTable != NULL) {
> + if (PreviousTable->Signature != Signature) {
> + //
> + // PreviousTable->Signature is not same with Signature.
> + //
> + ASSERT (FALSE);
> + return NULL;
> + } else if ((Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) ||
> + (Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) ||
> + (Signature == EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE)) {
> + //
> + // There is only one FADT/DSDT/FACS table,
> + // so don't try to locate next one.
> + //
> + return NULL;
> + }
> +
> + PreviousTableLocated = &TempPreviousTableLocated;
> + *PreviousTableLocated = FALSE;
> + } else {
> + PreviousTableLocated = NULL;
> + }
> +
> + Table = LocateAcpiTableInAcpiConfigurationTable (
> + &gEfiAcpi20TableGuid,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> + if (Table != NULL) {
> + return Table;
> + } else if ((PreviousTableLocated != NULL) &&
> + *PreviousTableLocated) {
> + //
> + // PreviousTable could be located in gEfiAcpi20TableGuid system
> + // configuration table, but next table could not be located in
> + // gEfiAcpi20TableGuid system configuration table.
> + //
> + return NULL;
> + }
> +
> + return LocateAcpiTableInAcpiConfigurationTable (
> + &gEfiAcpi10TableGuid,
> + Signature,
> + PreviousTable,
> + PreviousTableLocated
> + );
> +}
> +
> +/**
> + This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> + This function will locate the first ACPI table in XSDT/RSDT based on
> + Signature in gEfiAcpi20TableGuid system configuration table first, and then
> + gEfiAcpi10TableGuid system configuration table.
> + This function will locate in XSDT first, and then RSDT.
> + For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> + FADT.
> + For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> + FirmwareCtrl in FADT.
> +
> + @param Signature ACPI table signature.
> +
> + @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> + IN UINT32 Signature
> + )
> +{
> + return EfiLocateNextAcpiTable (Signature, NULL);
> +}
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
> index a6c739ef3d6d..aea20fe67153 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -41,6 +41,7 @@ [Sources]
> Console.c
> UefiLib.c
> UefiLibInternal.h
> + Acpi.c
>
>
> [Packages]
> @@ -62,6 +63,8 @@ [Guids]
> gEfiEventReadyToBootGuid ## SOMETIMES_CONSUMES ## Event
> gEfiEventLegacyBootGuid ## SOMETIMES_CONSUMES ## Event
> gEfiGlobalVariableGuid ## SOMETIMES_CONSUMES ## Variable
> + gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
> + gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
>
> [Protocols]
> gEfiDriverBindingProtocolGuid ## SOMETIMES_PRODUCES
>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
2018-09-13 10:26 ` [PATCH V2 1/6] MdePkg UefiLib: " Star Zeng
@ 2018-09-13 10:26 ` Star Zeng
2018-09-13 10:26 ` [PATCH V2 3/6] MdeModulePkg S3SaveStateDxe: " Star Zeng
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:26 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates IntelVTdDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c | 136 +--------------------
1 file changed, 3 insertions(+), 133 deletions(-)
diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 24723fe4972a..92b09f985533 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -868,116 +868,6 @@ ParseDmarAcpiTableRmrr (
}
/**
- This function scan ACPI table in RSDT.
-
- @param[in] Rsdt ACPI RSDT
- @param[in] Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
- IN RSDT_TABLE *Rsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT32 *EntryPtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
- EntryPtr = &Rsdt->Entry;
- for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
- Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
- if ((Table != NULL) && (Table->Signature == Signature)) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
- This function scan ACPI table in XSDT.
-
- @param[in] Xsdt ACPI XSDT
- @param[in] Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
- IN XSDT_TABLE *Xsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT64 EntryPtr;
- UINTN BasePtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
- BasePtr = (UINTN)(&(Xsdt->Entry));
- for (Index = 0; Index < EntryCount; Index ++) {
- CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
- Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
- if ((Table != NULL) && (Table->Signature == Signature)) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
- This function scan ACPI table in RSDP.
-
- @param[in] Rsdp ACPI RSDP
- @param[in] Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
- IN EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
- IN UINT32 Signature
- )
-{
- EFI_ACPI_DESCRIPTION_HEADER *AcpiTable;
- RSDT_TABLE *Rsdt;
- XSDT_TABLE *Xsdt;
-
- AcpiTable = NULL;
-
- //
- // Check ACPI2.0 table
- //
- Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
- Xsdt = NULL;
- if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
- Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
- }
- //
- // Check Xsdt
- //
- if (Xsdt != NULL) {
- AcpiTable = ScanTableInXSDT (Xsdt, Signature);
- }
- //
- // Check Rsdt
- //
- if ((AcpiTable == NULL) && (Rsdt != NULL)) {
- AcpiTable = ScanTableInRSDT (Rsdt, Signature);
- }
-
- return AcpiTable;
-}
-
-/**
Get the DMAR ACPI table.
@retval EFI_SUCCESS The DMAR ACPI table is got.
@@ -989,33 +879,13 @@ GetDmarAcpiTable (
VOID
)
{
- VOID *AcpiTable;
- EFI_STATUS Status;
-
if (mAcpiDmarTable != NULL) {
return EFI_ALREADY_STARTED;
}
- AcpiTable = NULL;
- Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi20TableGuid,
- &AcpiTable
- );
- if (EFI_ERROR (Status)) {
- Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi10TableGuid,
- &AcpiTable
- );
- }
- if (EFI_ERROR (Status)) {
- return EFI_NOT_FOUND;
- }
- ASSERT (AcpiTable != NULL);
-
- mAcpiDmarTable = FindAcpiPtr (
- (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
- EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
- );
+ mAcpiDmarTable = (EFI_ACPI_DMAR_HEADER *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
+ );
if (mAcpiDmarTable == NULL) {
return EFI_NOT_FOUND;
}
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH V2 3/6] MdeModulePkg S3SaveStateDxe: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
2018-09-13 10:26 ` [PATCH V2 1/6] MdePkg UefiLib: " Star Zeng
2018-09-13 10:26 ` [PATCH V2 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable() Star Zeng
@ 2018-09-13 10:26 ` Star Zeng
2018-09-13 10:26 ` [PATCH V2 4/6] PcAtChipsetPkg PcRtc: " Star Zeng
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:26 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao,
Jian J Wang
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates S3SaveStateDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c | 208 +--------------------
.../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf | 3 +-
2 files changed, 5 insertions(+), 206 deletions(-)
diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
index 3f99023f110f..fadafd2b608b 100644
--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
@@ -22,8 +22,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/LockBoxLib.h>
#include <Library/PcdLib.h>
#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
#include <Guid/AcpiS3Context.h>
-#include <Guid/Acpi.h>
#include <IndustryStandard/Acpi.h>
#include <Protocol/LockBox.h>
@@ -76,208 +76,6 @@ AllocateMemoryBelow4G (
}
/**
-
- This function scan ACPI table in RSDT.
-
- @param Rsdt ACPI RSDT
- @param Signature ACPI table signature
-
- @return ACPI table
-
-**/
-VOID *
-ScanTableInRSDT (
- IN EFI_ACPI_DESCRIPTION_HEADER *Rsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT32 *EntryPtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- if (Rsdt == NULL) {
- return NULL;
- }
-
- EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
- EntryPtr = (UINT32 *)(Rsdt + 1);
- for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
- Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
- if (Table->Signature == Signature) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
-
- This function scan ACPI table in XSDT.
-
- @param Xsdt ACPI XSDT
- @param Signature ACPI table signature
-
- @return ACPI table
-
-**/
-VOID *
-ScanTableInXSDT (
- IN EFI_ACPI_DESCRIPTION_HEADER *Xsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT64 EntryPtr;
- UINTN BasePtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- if (Xsdt == NULL) {
- return NULL;
- }
-
- EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
- BasePtr = (UINTN)(Xsdt + 1);
- for (Index = 0; Index < EntryCount; Index ++) {
- CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
- Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
- if (Table->Signature == Signature) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
- To find Facs in FADT.
-
- @param Fadt FADT table pointer
-
- @return Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
-FindAcpiFacsFromFadt (
- IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
- )
-{
- EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
- UINT64 Data64;
-
- if (Fadt == NULL) {
- return NULL;
- }
-
- if (Fadt->Header.Revision < EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
- Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
- } else {
- if (Fadt->FirmwareCtrl != 0) {
- Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Fadt->FirmwareCtrl;
- } else {
- CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
- Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Data64;
- }
- }
- return Facs;
-}
-
-/**
- To find Facs in Acpi tables.
-
- To find Firmware ACPI control strutcure in Acpi Tables since the S3 waking vector is stored
- in the table.
-
- @param AcpiTableGuid The guid used to find ACPI table in UEFI ConfigurationTable.
-
- @return Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
-FindAcpiFacsTableByAcpiGuid (
- IN EFI_GUID *AcpiTableGuid
- )
-{
- EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
- EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
- EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
- UINTN Index;
-
- Rsdp = NULL;
- //
- // found ACPI table RSD_PTR from system table
- //
- for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
- if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), AcpiTableGuid)) {
- //
- // A match was found.
- //
- Rsdp = gST->ConfigurationTable[Index].VendorTable;
- break;
- }
- }
-
- if (Rsdp == NULL) {
- return NULL;
- }
-
- //
- // Search XSDT
- //
- if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
- Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->XsdtAddress;
- Fadt = ScanTableInXSDT (Xsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
- if (Fadt != NULL) {
- Facs = FindAcpiFacsFromFadt (Fadt);
- if (Facs != NULL) {
- return Facs;
- }
- }
- }
-
- //
- // Search RSDT
- //
- Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
- Fadt = ScanTableInRSDT (Rsdt, EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE);
- if (Fadt != NULL) {
- Facs = FindAcpiFacsFromFadt (Fadt);
- if (Facs != NULL) {
- return Facs;
- }
- }
-
- return NULL;
-}
-
-/**
- To find Facs in Acpi tables.
-
- To find Firmware ACPI control strutcure in Acpi Tables since the S3 waking vector is stored
- in the table.
-
- @return Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *
-FindAcpiFacsTable (
- VOID
- )
-{
- EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *Facs;
-
- Facs = FindAcpiFacsTableByAcpiGuid (&gEfiAcpi20TableGuid);
- if (Facs != NULL) {
- return Facs;
- }
-
- return FindAcpiFacsTableByAcpiGuid (&gEfiAcpi10TableGuid);
-}
-
-/**
The function will check if long mode waking vector is supported.
@param[in] Facs Pointer to FACS table.
@@ -460,7 +258,9 @@ AcpiS3ContextSaveOnEndOfDxe (
//
// Get ACPI Table because we will save its position to variable
//
- Facs = (EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *) FindAcpiFacsTable ();
+ Facs = (EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE
+ );
AcpiS3Context->AcpiFacsTable = (EFI_PHYSICAL_ADDRESS) (UINTN) Facs;
ASSERT (AcpiS3Context->AcpiFacsTable != 0);
diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
index 744cf8ab3270..e385356dee1b 100644
--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
@@ -49,12 +49,11 @@ [LibraryClasses]
PcdLib
HobLib
LockBoxLib
+ UefiLib
[Guids]
gEfiAcpiVariableGuid ## PRODUCES ## UNDEFINED # LockBox Save Data.
gEfiAcpiS3ContextGuid ## PRODUCES ## UNDEFINED # LockBox Save Data.
- gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
- gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event
[Protocols]
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
` (2 preceding siblings ...)
2018-09-13 10:26 ` [PATCH V2 3/6] MdeModulePkg S3SaveStateDxe: " Star Zeng
@ 2018-09-13 10:26 ` Star Zeng
2018-09-14 4:41 ` Ni, Ruiyu
2018-09-13 10:26 ` [PATCH V2 5/6] ShellPkg DpDynamicCommand: " Star Zeng
2018-09-13 10:27 ` [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: " Star Zeng
5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:26 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao,
Ruiyu Ni
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates PcatRealTimeClockRuntimeDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 +---------------------
1 file changed, 3 insertions(+), 77 deletions(-)
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 2105acf35f7b..7965eb8aa55b 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -1203,49 +1203,6 @@ IsWithinOneDay (
}
/**
- This function find ACPI table with the specified signature in RSDT or XSDT.
-
- @param Sdt ACPI RSDT or XSDT.
- @param Signature ACPI table signature.
- @param TablePointerSize Size of table pointer: 4 or 8.
-
- @return ACPI table or NULL if not found.
-**/
-VOID *
-ScanTableInSDT (
- IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
- IN UINT32 Signature,
- IN UINTN TablePointerSize
- )
-{
- UINTN Index;
- UINTN EntryCount;
- UINTN EntryBase;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / TablePointerSize;
-
- EntryBase = (UINTN) (Sdt + 1);
- for (Index = 0; Index < EntryCount; Index++) {
- //
- // When TablePointerSize is 4 while sizeof (VOID *) is 8, make sure the upper 4 bytes are zero.
- //
- Table = 0;
- CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), TablePointerSize);
-
- if (Table == NULL) {
- continue;
- }
-
- if (Table->Signature == Signature) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
Get the century RTC address from the ACPI FADT table.
@return The century RTC address or 0 if not found.
@@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
VOID
)
{
- EFI_STATUS Status;
- EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) &Rsdp);
- if (EFI_ERROR (Status)) {
- Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, (VOID **) &Rsdp);
- }
-
- if (EFI_ERROR (Status) || (Rsdp == NULL)) {
- return 0;
- }
-
- Fadt = NULL;
-
- //
- // Find FADT in XSDT
- //
- if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 0) {
- Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINTN)
- );
- }
-
- //
- // Find FADT in RSDT
- //
- if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
- Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINT32)
- );
- }
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+ );
if ((Fadt != NULL) &&
(Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 ` [PATCH V2 4/6] PcAtChipsetPkg PcRtc: " Star Zeng
@ 2018-09-14 4:41 ` Ni, Ruiyu
2018-09-14 4:42 ` Ni, Ruiyu
0 siblings, 1 reply; 14+ messages in thread
From: Ni, Ruiyu @ 2018-09-14 4:41 UTC (permalink / raw)
To: Star Zeng, edk2-devel
Cc: Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao
On 9/13/2018 6:26 PM, Star Zeng wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order
> to get ACPI table using signature as input.
>
> After evaluation, we found there are many duplicated code to
> find ACPI table by signature in different modules.
>
> This patch updates PcatRealTimeClockRuntimeDxe to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 +---------------------
> 1 file changed, 3 insertions(+), 77 deletions(-)
>
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index 2105acf35f7b..7965eb8aa55b 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -1203,49 +1203,6 @@ IsWithinOneDay (
> }
>
> /**
> - This function find ACPI table with the specified signature in RSDT or XSDT.
> -
> - @param Sdt ACPI RSDT or XSDT.
> - @param Signature ACPI table signature.
> - @param TablePointerSize Size of table pointer: 4 or 8.
> -
> - @return ACPI table or NULL if not found.
> -**/
> -VOID *
> -ScanTableInSDT (
> - IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
> - IN UINT32 Signature,
> - IN UINTN TablePointerSize
> - )
> -{
> - UINTN Index;
> - UINTN EntryCount;
> - UINTN EntryBase;
> - EFI_ACPI_DESCRIPTION_HEADER *Table;
> -
> - EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / TablePointerSize;
> -
> - EntryBase = (UINTN) (Sdt + 1);
> - for (Index = 0; Index < EntryCount; Index++) {
> - //
> - // When TablePointerSize is 4 while sizeof (VOID *) is 8, make sure the upper 4 bytes are zero.
> - //
> - Table = 0;
> - CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), TablePointerSize);
> -
> - if (Table == NULL) {
> - continue;
> - }
> -
> - if (Table->Signature == Signature) {
> - return Table;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -/**
> Get the century RTC address from the ACPI FADT table.
>
> @return The century RTC address or 0 if not found.
> @@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
> VOID
> )
> {
> - EFI_STATUS Status;
> - EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
>
> - Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) &Rsdp);
> - if (EFI_ERROR (Status)) {
> - Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, (VOID **) &Rsdp);
> - }
> -
> - if (EFI_ERROR (Status) || (Rsdp == NULL)) {
> - return 0;
> - }
> -
> - Fadt = NULL;
> -
> - //
> - // Find FADT in XSDT
> - //
> - if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 0) {
> - Fadt = ScanTableInSDT (
> - (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> - sizeof (UINTN)
> - );
> - }
> -
> - //
> - // Find FADT in RSDT
> - //
> - if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
> - Fadt = ScanTableInSDT (
> - (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> - sizeof (UINT32)
> - );
> - }
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable (
> + EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
> + );
>
> if ((Fadt != NULL) &&
> (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)
>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V2 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()
2018-09-14 4:41 ` Ni, Ruiyu
@ 2018-09-14 4:42 ` Ni, Ruiyu
0 siblings, 0 replies; 14+ messages in thread
From: Ni, Ruiyu @ 2018-09-14 4:42 UTC (permalink / raw)
To: Star Zeng, edk2-devel
Cc: Michael D Kinney, Younas khan, Jiewen Yao, Liming Gao
On 9/14/2018 12:41 PM, Ni, Ruiyu wrote:
> On 9/13/2018 6:26 PM, Star Zeng wrote:
>> https://bugzilla.tianocore.org/show_bug.cgi?id=967
>> Request to add a library function for GetAcpiTable() in order
>> to get ACPI table using signature as input.
>>
>> After evaluation, we found there are many duplicated code to
>> find ACPI table by signature in different modules.
>>
>> This patch updates PcatRealTimeClockRuntimeDxe to use new
>> EfiLocateFirstAcpiTable() and remove the duplicated code.
>>
>> Cc: Younas khan <pmdyounaskhan786@gmail.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80
>> +---------------------
>> 1 file changed, 3 insertions(+), 77 deletions(-)
>>
>> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
>> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
>> index 2105acf35f7b..7965eb8aa55b 100644
>> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
>> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
>> @@ -1203,49 +1203,6 @@ IsWithinOneDay (
>> }
>> /**
>> - This function find ACPI table with the specified signature in RSDT
>> or XSDT.
>> -
>> - @param Sdt ACPI RSDT or XSDT.
>> - @param Signature ACPI table signature.
>> - @param TablePointerSize Size of table pointer: 4 or 8.
>> -
>> - @return ACPI table or NULL if not found.
>> -**/
>> -VOID *
>> -ScanTableInSDT (
>> - IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>> - IN UINT32 Signature,
>> - IN UINTN TablePointerSize
>> - )
>> -{
>> - UINTN Index;
>> - UINTN EntryCount;
>> - UINTN EntryBase;
>> - EFI_ACPI_DESCRIPTION_HEADER *Table;
>> -
>> - EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
>> TablePointerSize;
>> -
>> - EntryBase = (UINTN) (Sdt + 1);
>> - for (Index = 0; Index < EntryCount; Index++) {
>> - //
>> - // When TablePointerSize is 4 while sizeof (VOID *) is 8, make
>> sure the upper 4 bytes are zero.
>> - //
>> - Table = 0;
>> - CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize),
>> TablePointerSize);
>> -
>> - if (Table == NULL) {
>> - continue;
>> - }
>> -
>> - if (Table->Signature == Signature) {
>> - return Table;
>> - }
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> -/**
>> Get the century RTC address from the ACPI FADT table.
>> @return The century RTC address or 0 if not found.
>> @@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
>> VOID
>> )
>> {
>> - EFI_STATUS Status;
>> - EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
>> - Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
>> **) &Rsdp);
>> - if (EFI_ERROR (Status)) {
>> - Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid,
>> (VOID **) &Rsdp);
>> - }
>> -
>> - if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>> - return 0;
>> - }
>> -
>> - Fadt = NULL;
>> -
>> - //
>> - // Find FADT in XSDT
>> - //
>> - if (Rsdp->Revision >=
>> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION &&
>> Rsdp->XsdtAddress != 0) {
>> - Fadt = ScanTableInSDT (
>> - (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
>> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>> - sizeof (UINTN)
>> - );
>> - }
>> -
>> - //
>> - // Find FADT in RSDT
>> - //
>> - if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
>> - Fadt = ScanTableInSDT (
>> - (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
>> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>> - sizeof (UINT32)
>> - );
>> - }
>> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *)
>> EfiLocateFirstAcpiTable (
>> +
>> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
>> + );
>> if ((Fadt != NULL) &&
>> (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century <
>> 0x80)
>>
>
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>
By the way, can we remove the Acpi10/Acpi20 GUID references from INF file?
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
` (3 preceding siblings ...)
2018-09-13 10:26 ` [PATCH V2 4/6] PcAtChipsetPkg PcRtc: " Star Zeng
@ 2018-09-13 10:26 ` Star Zeng
2018-09-17 6:54 ` Ni, Ruiyu
2018-09-13 10:27 ` [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: " Star Zeng
5 siblings, 1 reply; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:26 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao,
Ruiyu Ni, Dandan Bi
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates DpDynamicCommand to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 136 +--------------------
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h | 1 -
ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni | 1 -
ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf | 2 -
.../DpDynamicCommand/DpDynamicCommand.inf | 2 -
5 files changed, 3 insertions(+), 139 deletions(-)
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index 2c094b63c94a..c14931055cbf 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -118,116 +118,6 @@ DumpStatistics( void )
}
/**
- This function scan ACPI table in RSDT.
-
- @param Rsdt ACPI RSDT
- @param Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
- IN RSDT_TABLE *Rsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT32 *EntryPtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
-
- EntryPtr = &Rsdt->Entry;
- for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
- Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
- if (Table->Signature == Signature) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
- This function scan ACPI table in XSDT.
-
- @param Xsdt ACPI XSDT
- @param Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
- IN XSDT_TABLE *Xsdt,
- IN UINT32 Signature
- )
-{
- UINTN Index;
- UINT32 EntryCount;
- UINT64 EntryPtr;
- UINTN BasePtr;
- EFI_ACPI_DESCRIPTION_HEADER *Table;
-
- EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
-
- BasePtr = (UINTN)(&(Xsdt->Entry));
- for (Index = 0; Index < EntryCount; Index ++) {
- CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), sizeof(UINT64));
- Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
- if (Table->Signature == Signature) {
- return Table;
- }
- }
-
- return NULL;
-}
-
-/**
- This function scan ACPI table in RSDP.
-
- @param Rsdp ACPI RSDP
- @param Signature ACPI table signature
-
- @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
- IN EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
- IN UINT32 Signature
- )
-{
- EFI_ACPI_DESCRIPTION_HEADER *AcpiTable;
- RSDT_TABLE *Rsdt;
- XSDT_TABLE *Xsdt;
-
- AcpiTable = NULL;
-
- //
- // Check ACPI2.0 table
- //
- Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
- Xsdt = NULL;
- if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
- Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
- }
- //
- // Check Xsdt
- //
- if (Xsdt != NULL) {
- AcpiTable = ScanTableInXSDT (Xsdt, Signature);
- }
- //
- // Check Rsdt
- //
- if ((AcpiTable == NULL) && (Rsdt != NULL)) {
- AcpiTable = ScanTableInRSDT (Rsdt, Signature);
- }
-
- return AcpiTable;
-}
-
-/**
Get Boot performance table form Acpi table.
**/
@@ -235,31 +125,11 @@ EFI_STATUS
GetBootPerformanceTable (
)
{
- EFI_STATUS Status;
- VOID *AcpiTable;
FIRMWARE_PERFORMANCE_TABLE *FirmwarePerformanceTable;
- AcpiTable = NULL;
-
- Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi20TableGuid,
- &AcpiTable
- );
- if (EFI_ERROR (Status)) {
- Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi10TableGuid,
- &AcpiTable
- );
- }
- if (EFI_ERROR(Status) || AcpiTable == NULL) {
- ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_TABLE_FAIL), mDpHiiHandle);
- return Status;
- }
-
- FirmwarePerformanceTable = FindAcpiPtr (
- (EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
- EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
- );
+ FirmwarePerformanceTable = (FIRMWARE_PERFORMANCE_TABLE *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
+ );
if (FirmwarePerformanceTable == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_FPDT_FAIL), mDpHiiHandle);
return EFI_NOT_FOUND;
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
index 0e6e9422effc..db8472f6558f 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
@@ -21,7 +21,6 @@
#include <Guid/Performance.h>
#include <Guid/ExtendedFirmwarePerformance.h>
#include <Guid/FirmwarePerformance.h>
-#include <Guid/Acpi.h>
#include <Protocol/HiiPackageList.h>
#include <Protocol/DevicePath.h>
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
index 1d6f25b8c252..7fc2b8d1c966 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
@@ -94,7 +94,6 @@
#string STR_DP_INCOMPLETE #language en-US " I "
#string STR_DP_COMPLETE #language en-US " "
#string STR_ALIT_UNKNOWN #language en-US "Unknown"
-#string STR_DP_GET_ACPI_TABLE_FAIL #language en-US "Fail to get ACPI Table\n"
#string STR_DP_GET_ACPI_FPDT_FAIL #language en-US "Fail to get Firmware Performance Data Table (FPDT) in ACPI Table\n"
#string STR_GET_HELP_DP #language en-US ""
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
index cedb333b285c..3a35fca310b0 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
@@ -59,8 +59,6 @@ [LibraryClasses]
[Guids]
gPerformanceProtocolGuid ## CONSUMES ## SystemTable
- gEfiAcpi20TableGuid ## CONSUMES ## SystemTable
- gEfiAcpi10TableGuid ## CONSUMES ## SystemTable
[Protocols]
gEfiLoadedImageProtocolGuid ## CONSUMES
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
index 8fd3bbd5df83..62a3b5246500 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf
@@ -60,8 +60,6 @@ [LibraryClasses]
[Guids]
gPerformanceProtocolGuid ## CONSUMES ## SystemTable
- gEfiAcpi20TableGuid ## CONSUMES
- gEfiAcpi10TableGuid ## CONSUMES
[Protocols]
gEfiLoadedImageProtocolGuid ## CONSUMES
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 ` [PATCH V2 5/6] ShellPkg DpDynamicCommand: " Star Zeng
@ 2018-09-17 6:54 ` Ni, Ruiyu
0 siblings, 0 replies; 14+ messages in thread
From: Ni, Ruiyu @ 2018-09-17 6:54 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org
Cc: Younas khan, Kinney, Michael D, Gao, Liming, Yao, Jiewen,
Bi, Dandan
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Thanks/Ray
> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 6:27 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>
> Subject: [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new
> EfiLocateFirstAcpiTable()
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get ACPI table
> using signature as input.
>
> After evaluation, we found there are many duplicated code to find ACPI
> table by signature in different modules.
>
> This patch updates DpDynamicCommand to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 136 +------------
> --------
> ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h | 1 -
> ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni | 1 -
> ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf | 2 -
> .../DpDynamicCommand/DpDynamicCommand.inf | 2 -
> 5 files changed, 3 insertions(+), 139 deletions(-)
>
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> index 2c094b63c94a..c14931055cbf 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> @@ -118,116 +118,6 @@ DumpStatistics( void ) }
>
> /**
> - This function scan ACPI table in RSDT.
> -
> - @param Rsdt ACPI RSDT
> - @param Signature ACPI table signature
> -
> - @return ACPI table
> -**/
> -VOID *
> -ScanTableInRSDT (
> - IN RSDT_TABLE *Rsdt,
> - IN UINT32 Signature
> - )
> -{
> - UINTN Index;
> - UINT32 EntryCount;
> - UINT32 *EntryPtr;
> - EFI_ACPI_DESCRIPTION_HEADER *Table;
> -
> - EntryCount = (Rsdt->Header.Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
> -
> - EntryPtr = &Rsdt->Entry;
> - for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> - Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
> - if (Table->Signature == Signature) {
> - return Table;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -/**
> - This function scan ACPI table in XSDT.
> -
> - @param Xsdt ACPI XSDT
> - @param Signature ACPI table signature
> -
> - @return ACPI table
> -**/
> -VOID *
> -ScanTableInXSDT (
> - IN XSDT_TABLE *Xsdt,
> - IN UINT32 Signature
> - )
> -{
> - UINTN Index;
> - UINT32 EntryCount;
> - UINT64 EntryPtr;
> - UINTN BasePtr;
> - EFI_ACPI_DESCRIPTION_HEADER *Table;
> -
> - EntryCount = (Xsdt->Header.Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
> -
> - BasePtr = (UINTN)(&(Xsdt->Entry));
> - for (Index = 0; Index < EntryCount; Index ++) {
> - CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
> sizeof(UINT64));
> - Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
> - if (Table->Signature == Signature) {
> - return Table;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -/**
> - This function scan ACPI table in RSDP.
> -
> - @param Rsdp ACPI RSDP
> - @param Signature ACPI table signature
> -
> - @return ACPI table
> -**/
> -VOID *
> -FindAcpiPtr (
> - IN EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
> - IN UINT32 Signature
> - )
> -{
> - EFI_ACPI_DESCRIPTION_HEADER *AcpiTable;
> - RSDT_TABLE *Rsdt;
> - XSDT_TABLE *Xsdt;
> -
> - AcpiTable = NULL;
> -
> - //
> - // Check ACPI2.0 table
> - //
> - Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
> - Xsdt = NULL;
> - if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
> - Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
> - }
> - //
> - // Check Xsdt
> - //
> - if (Xsdt != NULL) {
> - AcpiTable = ScanTableInXSDT (Xsdt, Signature);
> - }
> - //
> - // Check Rsdt
> - //
> - if ((AcpiTable == NULL) && (Rsdt != NULL)) {
> - AcpiTable = ScanTableInRSDT (Rsdt, Signature);
> - }
> -
> - return AcpiTable;
> -}
> -
> -/**
> Get Boot performance table form Acpi table.
>
> **/
> @@ -235,31 +125,11 @@ EFI_STATUS
> GetBootPerformanceTable (
> )
> {
> - EFI_STATUS Status;
> - VOID *AcpiTable;
> FIRMWARE_PERFORMANCE_TABLE *FirmwarePerformanceTable;
>
> - AcpiTable = NULL;
> -
> - Status = EfiGetSystemConfigurationTable (
> - &gEfiAcpi20TableGuid,
> - &AcpiTable
> - );
> - if (EFI_ERROR (Status)) {
> - Status = EfiGetSystemConfigurationTable (
> - &gEfiAcpi10TableGuid,
> - &AcpiTable
> - );
> - }
> - if (EFI_ERROR(Status) || AcpiTable == NULL) {
> - ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DP_GET_ACPI_TABLE_FAIL), mDpHiiHandle);
> - return Status;
> - }
> -
> - FirmwarePerformanceTable = FindAcpiPtr (
> - (EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER
> *)AcpiTable,
> -
> EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
> - );
> + FirmwarePerformanceTable = (FIRMWARE_PERFORMANCE_TABLE *)
> EfiLocateFirstAcpiTable (
> +
> EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
> + );
> if (FirmwarePerformanceTable == NULL) {
> ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DP_GET_ACPI_FPDT_FAIL), mDpHiiHandle);
> return EFI_NOT_FOUND;
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
> index 0e6e9422effc..db8472f6558f 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
> @@ -21,7 +21,6 @@
> #include <Guid/Performance.h>
> #include <Guid/ExtendedFirmwarePerformance.h>
> #include <Guid/FirmwarePerformance.h>
> -#include <Guid/Acpi.h>
>
> #include <Protocol/HiiPackageList.h>
> #include <Protocol/DevicePath.h>
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> index 1d6f25b8c252..7fc2b8d1c966 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> @@ -94,7 +94,6 @@
> #string STR_DP_INCOMPLETE #language en-US " I "
> #string STR_DP_COMPLETE #language en-US " "
> #string STR_ALIT_UNKNOWN #language en-US "Unknown"
> -#string STR_DP_GET_ACPI_TABLE_FAIL #language en-US "Fail to get ACPI
> Table\n"
> #string STR_DP_GET_ACPI_FPDT_FAIL #language en-US "Fail to get
> Firmware Performance Data Table (FPDT) in ACPI Table\n"
>
> #string STR_GET_HELP_DP #language en-US ""
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
> b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
> index cedb333b285c..3a35fca310b0 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf
> @@ -59,8 +59,6 @@ [LibraryClasses]
>
> [Guids]
> gPerformanceProtocolGuid ## CONSUMES ## SystemTable
> - gEfiAcpi20TableGuid ## CONSUMES ## SystemTable
> - gEfiAcpi10TableGuid ## CONSUMES ## SystemTable
>
> [Protocols]
> gEfiLoadedImageProtocolGuid ## CONSUMES
> diff --git
> a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> index 8fd3bbd5df83..62a3b5246500 100644
> ---
> a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> +++
> b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.i
> nf
> @@ -60,8 +60,6 @@ [LibraryClasses]
>
> [Guids]
> gPerformanceProtocolGuid ## CONSUMES ## SystemTable
> - gEfiAcpi20TableGuid ## CONSUMES
> - gEfiAcpi10TableGuid ## CONSUMES
>
> [Protocols]
> gEfiLoadedImageProtocolGuid ## CONSUMES
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:26 [PATCH V2 0/6] Add new EfiLocateXXXAcpiTable() APIs Star Zeng
` (4 preceding siblings ...)
2018-09-13 10:26 ` [PATCH V2 5/6] ShellPkg DpDynamicCommand: " Star Zeng
@ 2018-09-13 10:27 ` Star Zeng
2018-09-13 10:38 ` Laszlo Ersek
2018-09-14 0:19 ` Dong, Eric
5 siblings, 2 replies; 14+ messages in thread
From: Star Zeng @ 2018-09-13 10:27 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao,
Ruiyu Ni, Eric Dong, Laszlo Ersek
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates PiSmmCpuDxeSmm to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +-
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 84 ++------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 3 +-
3 files changed, 6 insertions(+), 85 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index a7fb7b0b1482..0fdc1b134ea3 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -4,7 +4,7 @@
# This SMM driver performs SMM initialization, deploy SMM Entry Vector,
# provides CPU specific services in SMM.
#
-# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
#
# This program and the accompanying materials
@@ -114,8 +114,6 @@ [Protocols]
[Guids]
gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it is used for S3 boot.
gEfiGlobalVariableGuid ## SOMETIMES_PRODUCES ## Variable:L"SmmProfileData"
- gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
- gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index a743cf64f907..91b8e7ddb991 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,7 +1,7 @@
/** @file
Enable SMM profile.
-Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
This program and the accompanying materials
@@ -726,84 +726,6 @@ InitPaging (
}
/**
- To find FADT in ACPI tables.
-
- @param AcpiTableGuid The GUID used to find ACPI table in UEFI ConfigurationTable.
-
- @return FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *
-FindAcpiFadtTableByAcpiGuid (
- IN EFI_GUID *AcpiTableGuid
- )
-{
- EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
- EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- UINTN Index;
- UINT32 Data32;
- Rsdp = NULL;
- Rsdt = NULL;
- Fadt = NULL;
- //
- // found ACPI table RSD_PTR from system table
- //
- for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
- if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), AcpiTableGuid)) {
- //
- // A match was found.
- //
- Rsdp = gST->ConfigurationTable[Index].VendorTable;
- break;
- }
- }
-
- if (Rsdp == NULL) {
- return NULL;
- }
-
- Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
- if (Rsdt == NULL || Rsdt->Signature != EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
- return NULL;
- }
-
- for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt->Length; Index = Index + sizeof (UINT32)) {
-
- Data32 = *(UINT32 *) ((UINT8 *) Rsdt + Index);
- Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *) (UINTN) Data32;
- if (Fadt->Header.Signature == EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
- break;
- }
- }
-
- if (Fadt == NULL || Fadt->Header.Signature != EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
- return NULL;
- }
-
- return Fadt;
-}
-
-/**
- To find FADT in ACPI tables.
-
- @return FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *
-FindAcpiFadtTable (
- VOID
- )
-{
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-
- Fadt = FindAcpiFadtTableByAcpiGuid (&gEfiAcpi20TableGuid);
- if (Fadt != NULL) {
- return Fadt;
- }
-
- return FindAcpiFadtTableByAcpiGuid (&gEfiAcpi10TableGuid);
-}
-
-/**
To get system port address of the SMI Command Port in FADT table.
**/
@@ -814,7 +736,9 @@ GetSmiCommandPort (
{
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
- Fadt = FindAcpiFadtTable ();
+ Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+ );
ASSERT (Fadt != NULL);
mSmiCommandPort = Fadt->SmiCmd;
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index bacb2f8ad32c..f93e90c143d2 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -1,7 +1,7 @@
/** @file
SMM profile internal header file.
-Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -16,7 +16,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define _SMM_PROFILE_INTERNAL_H_
#include <Guid/GlobalVariable.h>
-#include <Guid/Acpi.h>
#include <Protocol/SmmReadyToLock.h>
#include <Library/UefiRuntimeServicesTableLib.h>
#include <Library/DxeServicesTableLib.h>
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:27 ` [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: " Star Zeng
@ 2018-09-13 10:38 ` Laszlo Ersek
2018-09-14 0:19 ` Dong, Eric
1 sibling, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2018-09-13 10:38 UTC (permalink / raw)
To: Star Zeng, edk2-devel
Cc: Younas khan, Michael D Kinney, Liming Gao, Jiewen Yao, Ruiyu Ni,
Eric Dong
On 09/13/18 12:27, Star Zeng wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order
> to get ACPI table using signature as input.
>
> After evaluation, we found there are many duplicated code to
> find ACPI table by signature in different modules.
>
> This patch updates PiSmmCpuDxeSmm to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 84 ++------------------------
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 3 +-
> 3 files changed, 6 insertions(+), 85 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()
2018-09-13 10:27 ` [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: " Star Zeng
2018-09-13 10:38 ` Laszlo Ersek
@ 2018-09-14 0:19 ` Dong, Eric
1 sibling, 0 replies; 14+ messages in thread
From: Dong, Eric @ 2018-09-14 0:19 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org
Cc: Younas khan, Kinney, Michael D, Gao, Liming, Yao, Jiewen,
Ni, Ruiyu, Laszlo Ersek
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 13, 2018 6:27 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH V2 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new
> EfiLocateFirstAcpiTable()
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get ACPI table
> using signature as input.
>
> After evaluation, we found there are many duplicated code to find ACPI
> table by signature in different modules.
>
> This patch updates PiSmmCpuDxeSmm to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
>
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 84 ++----------------------
> --
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 3 +-
> 3 files changed, 6 insertions(+), 85 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index a7fb7b0b1482..0fdc1b134ea3 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -4,7 +4,7 @@
> # This SMM driver performs SMM initialization, deploy SMM Entry Vector, #
> provides CPU specific services in SMM.
> #
> -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2018, Intel Corporation. All rights
> +reserved.<BR>
> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> # # This
> program and the accompanying materials @@ -114,8 +114,6 @@ [Protocols]
> [Guids]
> gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it is
> used for S3 boot.
> gEfiGlobalVariableGuid ## SOMETIMES_PRODUCES ##
> Variable:L"SmmProfileData"
> - gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
> - gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
> gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
> gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index a743cf64f907..91b8e7ddb991 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,7 +1,7 @@
> /** @file
> Enable SMM profile.
>
> -Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>
> This program and the accompanying materials @@ -726,84 +726,6 @@
> InitPaging ( }
>
> /**
> - To find FADT in ACPI tables.
> -
> - @param AcpiTableGuid The GUID used to find ACPI table in UEFI
> ConfigurationTable.
> -
> - @return FADT table pointer.
> -**/
> -EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE * -
> FindAcpiFadtTableByAcpiGuid (
> - IN EFI_GUID *AcpiTableGuid
> - )
> -{
> - EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
> - EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> - UINTN Index;
> - UINT32 Data32;
> - Rsdp = NULL;
> - Rsdt = NULL;
> - Fadt = NULL;
> - //
> - // found ACPI table RSD_PTR from system table
> - //
> - for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
> - if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid),
> AcpiTableGuid)) {
> - //
> - // A match was found.
> - //
> - Rsdp = gST->ConfigurationTable[Index].VendorTable;
> - break;
> - }
> - }
> -
> - if (Rsdp == NULL) {
> - return NULL;
> - }
> -
> - Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
> - if (Rsdt == NULL || Rsdt->Signature !=
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
> - return NULL;
> - }
> -
> - for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt-
> >Length; Index = Index + sizeof (UINT32)) {
> -
> - Data32 = *(UINT32 *) ((UINT8 *) Rsdt + Index);
> - Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *)
> (UINTN) Data32;
> - if (Fadt->Header.Signature ==
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> - break;
> - }
> - }
> -
> - if (Fadt == NULL || Fadt->Header.Signature !=
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> - return NULL;
> - }
> -
> - return Fadt;
> -}
> -
> -/**
> - To find FADT in ACPI tables.
> -
> - @return FADT table pointer.
> -**/
> -EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE * -FindAcpiFadtTable (
> - VOID
> - )
> -{
> - EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
> -
> - Fadt = FindAcpiFadtTableByAcpiGuid (&gEfiAcpi20TableGuid);
> - if (Fadt != NULL) {
> - return Fadt;
> - }
> -
> - return FindAcpiFadtTableByAcpiGuid (&gEfiAcpi10TableGuid); -}
> -
> -/**
> To get system port address of the SMI Command Port in FADT table.
>
> **/
> @@ -814,7 +736,9 @@ GetSmiCommandPort (
> {
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
>
> - Fadt = FindAcpiFadtTable ();
> + Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *)
> EfiLocateFirstAcpiTable (
> +
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
> + );
> ASSERT (Fadt != NULL);
>
> mSmiCommandPort = Fadt->SmiCmd;
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> index bacb2f8ad32c..f93e90c143d2 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> @@ -1,7 +1,7 @@
> /** @file
> SMM profile internal header file.
>
> -Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials are licensed and made
> available under the terms and conditions of the BSD License which
> accompanies this distribution. The full text of the license may be found at
> @@ -16,7 +16,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #define _SMM_PROFILE_INTERNAL_H_
>
> #include <Guid/GlobalVariable.h>
> -#include <Guid/Acpi.h>
> #include <Protocol/SmmReadyToLock.h>
> #include <Library/UefiRuntimeServicesTableLib.h>
> #include <Library/DxeServicesTableLib.h>
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 14+ messages in thread