Re: [edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
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 ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Younas khan ; Yao, Jiewen ; Gao, Liming 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 > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Jiewen Yao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Star Zeng > --- > 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 > + > #include > #include > #include > @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath ( > IN UINT64OpenMode, > IN UINT64Attributes > ); > + > +/** > + 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 Signatu
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 Cc: Michael D Kinney Cc: Liming Gao Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- 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 + #include #include #include @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath ( IN UINT64OpenMode, IN UINT64Attributes ); + +/** + 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 ..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. + 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. + +**/ +
[edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
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 Cc: Michael D Kinney Cc: Liming Gao Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- 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 + #include #include #include @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath ( IN UINT64OpenMode, IN UINT64Attributes ); + +/** + 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 ..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. + 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 +#include + +/** + 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
