Re: [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

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 <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Evan Lloyd <Evan.Lloyd@arm.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@ml01.01.org; Leif Lindholm <leif.lindholm@linaro.org>; ard.biesheuvel@linaro.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), we 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 Carsey, Jaben
Sent: Tuesday, December 20, 2016 11:56 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Evan Lloyd <Evan.Lloyd@arm.com<mailto:Evan.Lloyd@arm.com>>
Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.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 the DpLib), which extends the UEFI Shell, but does not mix the existing profiles.

-Jaben

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, December 19, 2016 6:59 PM
> To: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; Evan Lloyd
> <Evan.Lloyd@arm.com<mailto:Evan.Lloyd@arm.com>>
> Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; edk2-
> devel@ml01.01.org<mailto:devel@ml01.01.org>
> Subject: RE: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI tables
> Importance: High
>
> Evan,
> Thanks for the answers. BSD license of your code is great! Thanks for your
> 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 fixed 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 INF
> generating standalone "acpiview.efi" (New INF I want)
>
> When creating INF #2, in order to embed the help message into the resource
> 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  = TRUE" to let build tool embed HII
> resource (string)
>     into the resource section.
> 2. write code in entrypoint of application to retrieve the string resource 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 build 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
>                                              = 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 APP 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 <Evan.Lloyd@arm.com<mailto:Evan.Lloyd@arm.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>;
> > edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>;
> > ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>
> > 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 <Jaben.carsey@intel.com<mailto:Jaben.carsey@intel.com>>
> >
> > 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 <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> > > Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; Leif Lindholm
> > > <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@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<mailto:edk2-devel@ml01.01.org>
> > > >Cc: Carsey, Jaben; Leif Lindholm; ard.biesheuvel@linaro.org<mailto: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 level.
> > > 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 anxiety.
> > >
> > > Regards,
> > > Evan
> > >
> > >
> > > >
> > > >Thanks/Ray
> > > >
> > > >> -----Original Message-----
> > > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > >> Behalf Of evan.lloyd@arm.com<mailto:evan.lloyd@arm.com>
> > > >> Sent: Saturday, December 17, 2016 2:26 AM
> > > >> To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> > > >> Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; Ni, Ruiyu
> > > ><ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>;
> > > >> Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Ard Biesheuvel
> > > >> <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> > > >> Subject: [edk2] [PATCH] ShellPkg: Add acpiview tool to dump ACPI
> > > >> tables
> > > >>
> > > >> From: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
> > > >>
> > > >> 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 to 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 installed.
> > > >> '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 output.
> > > >> Other table types will be displayed as an array of Hexadecimal bytes.
> > > >>
> > > >> 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 <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
> > > >> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com<mailto:evan.lloyd@arm.com>>
> > > >> ---
> > > >>  ShellPkg/ShellPkg.dec                                                        |   2 +
> > > >>
> > > >>
> > > ...
> > > >> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> > > >>
> > > >> _______________________________________________
> > > >> edk2-devel mailing list
> > > >> edk2-devel@lists.01.org<mailto: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<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel