From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: 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>,
"Bi, Dandan" <dandan.bi@intel.com>
Subject: Re: [PATCH V2 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()
Date: Mon, 17 Sep 2018 06:54:05 +0000 [thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BE0D673@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <1536834420-16620-6-git-send-email-star.zeng@intel.com>
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
next prev parent reply other threads:[~2018-09-17 6:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
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-14 4:40 ` Ni, Ruiyu
2018-09-17 7:13 ` Zeng, Star
2018-09-13 10:26 ` [PATCH V2 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable() Star Zeng
2018-09-13 10:26 ` [PATCH V2 3/6] MdeModulePkg S3SaveStateDxe: " Star Zeng
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
2018-09-13 10:26 ` [PATCH V2 5/6] ShellPkg DpDynamicCommand: " Star Zeng
2018-09-17 6:54 ` Ni, Ruiyu [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=734D49CCEBEEF84792F5B80ED585239D5BE0D673@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox