From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2D63282154 for ; Mon, 19 Dec 2016 21:54:25 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP; 19 Dec 2016 21:54:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,377,1477983600"; d="scan'208,217";a="204816946" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 19 Dec 2016 21:54:24 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Dec 2016 21:54:23 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 19 Dec 2016 21:54:23 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.54]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.77]) with mapi id 14.03.0248.002; Tue, 20 Dec 2016 13:54:19 +0800 From: "Yao, Jiewen" To: "Ni, Ruiyu" , "Carsey, Jaben" , Evan Lloyd CC: "Carsey, Jaben" , "edk2-devel@ml01.01.org" , Leif Lindholm , "ard.biesheuvel@linaro.org" Thread-Topic: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables Thread-Index: AQHSV8nhfrd+vdgk1UaQNKJ6nLAROaEPCe4g///hgwCAACKtgIABFlEg//+P4ACAAJMo8IAACSiAgAAA/BA= Date: Tue, 20 Dec 2016 05:54:19 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8C6CBD@shsmsx102.ccr.corp.intel.com> References: <20161216182547.616-1-evan.lloyd@arm.com> <734D49CCEBEEF84792F5B80ED585239D5B836C04@SHSMSX103.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5B8371E1@SHSMSX103.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8C6C52@shsmsx102.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5B839479@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5B839479@SHSMSX103.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Dec 2016 05:54:25 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I like this idea - Shell.efi should only support commands defined by Shell = spec. Then we should not port code to be a shell library. I have concern on ShellPkg\Library\UefiDpLib, because DP depends on TimerLi= b and TimerLib is a platform specific library. I do not object the idea to move DP from performancepkg to shellPkg. I would suggest DP had better be a standard shell application, rather than = a shell lib and included in a shell bin. Now back to acpiview: First, it is great to have such tool in UEFI shell. Second, I found the function is incomplete. 1) Some ACPI defined tables are not complete. Take MADT as example, i= t only supports EFI_ACPI_6_1_GIC/EFI_ACPI_6_1_GICD/EFI_ACPI_6_1_GIC_MSI_FRA= ME/EFI_ACPI_6_1_GICR/EFI_ACPI_6_1_GIC_ITS. 2) Some ACPI defined tables are missing. Such as BGRT, FPDT. 3) I found some ACPI reserved tables are added here. Such as DBG2, IO= RT. 4) But not all ACPI reserved tables are added. Such as HPET, TPM2. May I know what criteria we are using to select which feature is supported = or not supported? Or it is just something from start and still need some work to make it comp= lete? Third, I found we need a long - "switch (*AcpiTableSignature) case" to dump= /parse ACPI table specific function. Do you think it will be better, that we provide a register function to let = each ACPI table register a parser? For example: RegisterDumpAcpiTable (EFI_ACPI_1_0_APIC_SIGNATURE, DumpAcpiMA= DT); Then the core logic just need use a table-driven way to find out the corres= ponding parser function by a SIGNATURE. Finally, I agree with Ruiyu that a standalone tool might be more proper. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Tuesday, December 20, 2016 1:18 PM To: Yao, Jiewen ; Carsey, Jaben ; Evan Lloyd Cc: Carsey, Jaben ; edk2-devel@ml01.01.org; Leif Li= ndholm ; ard.biesheuvel@linaro.org Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables Jiewen, I think Shell.efi should only support commands defined by Shell spec. All other commands should be provided in standalone application way. Jaben, What's your idea? Thanks/Ray From: Yao, Jiewen Sent: Tuesday, December 20, 2016 12:50 PM To: Carsey, Jaben >; = Ni, Ruiyu >; Evan Lloyd > Cc: Carsey, Jaben >; = edk2-devel@ml01.01.org; Leif Lindholm >; ard.biesheuvel@linar= o.org Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables Hi Jaben/Ruiyu There is one thing I am not clear, and hope you guys can clarify. 1) When we add a new shell application (not defined by SHELL spec), w= e can choices: a) ShellPkg, such as UefiDpLib, Ping.c, Ifconfig.c b) Non-ShellPkg, such as IpsecConfig, VConfig. Where we should choose? 2) What is default rule to SHELL BINARY generation? a) Including only the commands defined by SHELL spec? b) Include all commands even not defined by SHELL spec? Which criteria we are using now? Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Cars= ey, Jaben Sent: Tuesday, December 20, 2016 11:56 AM To: Ni, Ruiyu >; Evan Lloyd <= Evan.Lloyd@arm.com> Cc: Carsey, Jaben >; = edk2-devel@ml01.01.org; Leif Lindholm >; ard.biesheuvel@linar= o.org Subject: Re: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables We could make it into it's own profile instead of the Debug profile (like t= he DpLib), which extends the UEFI Shell, but does not mix the existing prof= iles. -Jaben > -----Original Message----- > From: Ni, Ruiyu > Sent: Monday, December 19, 2016 6:59 PM > To: Carsey, Jaben >= ; Evan Lloyd > > > Cc: Leif Lindholm >; ard.biesheuvel@linaro.org; edk2- > devel@ml01.01.org > Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tabl= es > Importance: High > > Evan, > Thanks for the answers. BSD license of your code is great! Thanks for you= r > contribution. > But I do have some other thoughts in below. > > Jaben, Evan, > The implementation style of "acpiview" is to embed it into "Shell.efi" as= an > internal command, in "Debug1" profile. > > I am afraid it's a violation of Shell spec because Shell spec defines a f= ixed list of > commands that belongs to "Debug1" profile, while "acpiview" isn't in the = list. > On the other side, some people may want a standalone "acpiview.efi" that = he > use it in any UEFI Shell environment. > > How about we provide two INF files for this tool: > 1. NULL class INF providing constructor/destructor(Current INF) 2. APP IN= F > generating standalone "acpiview.efi" (New INF I want) > > When creating INF #2, in order to embed the help message into the resourc= e > section of EFI file so that Shell core can find it and display when "-?" = is typed, we > need to: > 1. declare "UEFI_HII_RESOURCE_SECTION =3D TRUE" to let build tool embed = HII > resource (string) > into the resource section. > 2. write code in entrypoint of application to retrieve the string resourc= e from > ImageHandle, and > free the resource in Unload. > Sample code is in > MdeModulePkg/Universal/HiiResourcesSampleDxe/HiiResourcesSample.c > 3. Explicitly reference the token ID of help string in C file so that bui= ld tool > doesn't optimize out > the help string from HII resource. > Sample code in MdeModulePkg/Application/HelloWorld/HelloWorld.c: > GLOBAL_REMOVE_IF_UNREFERENCED EFI_STRING_ID mStringHelpTokenId > =3D STRING_TOKEN > (STR_HELLO_WORLD_HELP_INFORMATION); > > The step 2&3 can be put in a new .C file that only be referenced by the A= PP INF > file. > > Since now we provide both library and application for acpiview, how about= put it > in new location ShellPkg/Application/AcpiView directory? > > What's your option? If you agree, I can help to create the APP INF. > > Thanks/Ray > > > -----Original Message----- > > From: Carsey, Jaben > > Sent: Tuesday, December 20, 2016 2:01 AM > > To: Evan Lloyd >; Ni, Rui= yu >; > > edk2-devel@ml01.01.org > > Cc: Leif Lindholm >; > > ard.biesheuvel@linaro.org; Carsey, Ja= ben > > > Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI > > tables > > > > I am good with this addition. The BSD is a huge advantage for me. > > > > Reviewed-by: Jaben Carsey > > > > > I will allow Ray to resolve any issues he and push it. > > > > -Jaben > > > > > -----Original Message----- > > > From: Evan Lloyd [mailto:Evan.Lloyd@arm.com] > > > Sent: Monday, December 19, 2016 7:57 AM > > > To: Ni, Ruiyu >; edk2-d= evel@ml01.01.org > > > Cc: Carsey, Jaben >; Leif Lindholm > > > >; ard.bies= heuvel@linaro.org > > > Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI > > > tables > > > Importance: High > > > > > > Hi Ray. > > > Good question. Answer inline. > > > > > > >-----Original Message----- > > > >From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] > > > >Sent: 19 December 2016 09:49 > > > >To: Evan Lloyd; edk2-devel@ml01.01.org > > > >Cc: Carsey, Jaben; Leif Lindholm; ard.biesheuvel@linaro.org > > > >Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump > > > >ACPI tables > > > > > > > >I happened to find another version of acpi dump tool in shell. > > > >Binary can be downloaded from: https://acpica.org/downloads/uefi- > > > >support > > > >Source can be downloaded from: https://github.com/acpica/acpica > > > > > > > >Are there any differences between the above one and yours? > > > > > > The main differences are: > > > 1.acpiview provides a formatted dump of non-AML tables at the Shell l= evel. > > > This provides the equivalent of using acpidump -b, then > > > transferring the file to use "iasl -d" on another system. However, > > > it works on systems where the tables do not yet enable an OS to > > > boot, or have limited > > disk functionality. > > > 2.It provides a small amount of specification consistency > > > checking/error reporting at the Shell level. We have found that to > > > be an invaluable aid to debugging. > > > 3.It has a normal UEFI BSD license, so doesn't instigate legal anxiet= y. > > > > > > Regards, > > > Evan > > > > > > > > > > > > > >Thanks/Ray > > > > > > > >> -----Original Message----- > > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > > > >> Behalf Of evan.lloyd@arm.com > > > >> Sent: Saturday, December 17, 2016 2:26 AM > > > >> To: edk2-devel@ml01.01.org > > > >> Cc: Carsey, Jaben >; Ni, Ruiyu > > > >>; > > > >> Leif Lindholm >; Ard Biesheuvel > > > >> > > > > >> Subject: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI > > > >> tables > > > >> > > > >> From: Sami Mujawar > > > > >> > > > >> This program is provided to allow examination of ACPI table > > > >> contents from the UEFI Shell. This can help with investigations, > > > >> especially at that stage where the tables are not enabling an OS t= o boot. > > > >> The program is not exhaustive, and only encapsulates detailed > > > >> knowledge of a limited number of table types. > > > >> > > > >> Default behaviour is to display the content of all tables installe= d. > > > >> 'Known' table types will be parsed and displayed with > > > >> descriptions and field values. Where appropriate a degree of > > > >> consistency checking is done and errors may be reported in the out= put. > > > >> Other table types will be displayed as an array of Hexadecimal byt= es. > > > >> > > > >> To facilitate debugging, the -t and -b options can be used to > > > >> generate a binary file image of a table that can be copied > > > >> elsewhere for investigation using tools such as those provided by > > > >> acpica.org. This is especially relevant for AML type tables like > > > >> DSDT and > > SSDT. > > > >> > > > >> The inspiration for this is the existing smbiosview Debug1 Shell > > > >> command, and the command is also intended for Debug1. > > > >> > > > >> Many tables are not explicitly handled, in part because no > > > >> examples are available for our testing. > > > >> > > > >> The program is designed to be extended to new tables with minimal > > > >> effort, and contributions are invited. > > > >> > > > >> The code is available for examination at: > > > >> https://github.com/EvanLloyd/tianocore/tree/651_acpiview_v1 > > > >> > > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > > >> Signed-off-by: Sami Mujawar > > > > >> Signed-off-by: Evan Lloyd > > > > >> --- > > > >> ShellPkg/ShellPkg.dec = | 2 + > > > >> > > > >> > > > ... > > > >> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > > > >> > > > >> _______________________________________________ > > > >> edk2-devel mailing list > > > >> edk2-devel@lists.01.org > > > >> https://lists.01.org/mailman/listinfo/edk2-devel > > > IMPORTANT NOTICE: The contents of this email and any attachments are > > > confidential and may also be privileged. If you are not the intended > > > recipient, please notify the sender immediately and do not disclose > > > the contents to any other person, use it for any purpose, or store > > > or copy the information in any medium. Thank you. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel