[PATCH v1] Platforms/ARM: Enable acpiview support

[PATCH v1] Platforms/ARM: Enable acpiview support

[Sami Mujawar]

Acpiview is a program that allows examination of ACPI table
contents from the UEFI Shell.

Enable acpiview support as this can help with investigations,
especially at that stage where the tables are not enabling an
OS to boot.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
---
The patch can be seen at:
https://github.com/samimujawar/edk2-platforms/tree/254_enable_acpiview_v1

Notes:
    v1:
    - Enable acpiview support in the UEFI shell for ARM platforms.  [SAMI]

 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 7cb47937329efa1944ec4c156e2be995a755ce6f..0badb24f030a58363e7bee22a7cc1e03040b60aa 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -450,6 +450,7 @@ [Components.common]
       NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
       HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
       PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
       BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v1] Platforms/ARM: Enable acpiview support

[Leif Lindholm]

On Thu, Apr 26, 2018 at 02:01:21PM +0100, Sami Mujawar wrote:
> Acpiview is a program that allows examination of ACPI table
> contents from the UEFI Shell.
> 
> Enable acpiview support as this can help with investigations,
> especially at that stage where the tables are not enabling an
> OS to boot.

I have no objections to this, but I would prefer to see a comment from
Arvind, Thomas or Daniil before merging.
Adding Nariman to cc as well for good measure :)

However, please use --subject-prefix="PATCH edk2-platforms“ in future.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
> ---
> The patch can be seen at:
> https://github.com/samimujawar/edk2-platforms/tree/254_enable_acpiview_v1
> 
> Notes:
>     v1:
>     - Enable acpiview support in the UEFI shell for ARM platforms.  [SAMI]
> 
>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> index 7cb47937329efa1944ec4c156e2be995a755ce6f..0badb24f030a58363e7bee22a7cc1e03040b60aa 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
> @@ -450,6 +450,7 @@ [Components.common]
>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf

Although I would probably move this line up a few to improve sorting.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>        HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
>        PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>        BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 

Re: [PATCH v1] Platforms/ARM: Enable acpiview support

[Thomas Abraham]

On Thu, Apr 26, 2018 at 8:38 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Apr 26, 2018 at 02:01:21PM +0100, Sami Mujawar wrote:
>> Acpiview is a program that allows examination of ACPI table
>> contents from the UEFI Shell.
>>
>> Enable acpiview support as this can help with investigations,
>> especially at that stage where the tables are not enabling an
>> OS to boot.
>
> I have no objections to this, but I would prefer to see a comment from
> Arvind, Thomas or Daniil before merging.
> Adding Nariman to cc as well for good measure :)
>
> However, please use --subject-prefix="PATCH edk2-platforms“ in future.
>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
>> ---
>> The patch can be seen at:
>> https://github.com/samimujawar/edk2-platforms/tree/254_enable_acpiview_v1
>>
>> Notes:
>>     v1:
>>     - Enable acpiview support in the UEFI shell for ARM platforms.  [SAMI]
>>
>>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
>> index 7cb47937329efa1944ec4c156e2be995a755ce6f..0badb24f030a58363e7bee22a7cc1e03040b60aa 100644
>> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
>> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
>> @@ -1,5 +1,5 @@
>>  #
>> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
>> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>>  #
>>  #  This program and the accompanying materials
>>  #  are licensed and made available under the terms and conditions of the BSD License
>> @@ -450,6 +450,7 @@ [Components.common]
>>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
>
> Although I would probably move this line up a few to improve sorting.
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
>>        HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
>>        PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>>        BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf

Tested this patch with the Base RevC AEMv8 fvp and acpiview command
showed a very useful dump of the ACPI tables

Tested-by: Thomas Abraham <thomas.abraham@arm.com>

Re: [PATCH v1] Platforms/ARM: Enable acpiview support

[Leif Lindholm]

On Thu, Apr 26, 2018 at 02:01:21PM +0100, Sami Mujawar wrote:
> Acpiview is a program that allows examination of ACPI table
> contents from the UEFI Shell.
> 
> Enable acpiview support as this can help with investigations,
> especially at that stage where the tables are not enabling an
> OS to boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>

Please keep reviewed-by to what has been reviewed on the mailing list.
(I know Evan has reviewed this and would confirm on list if asked to,
so I'll keep it in for patches currently in my queue.)

> ---
> The patch can be seen at:
> https://github.com/samimujawar/edk2-platforms/tree/254_enable_acpiview_v1
> 
> Notes:
>     v1:
>     - Enable acpiview support in the UEFI shell for ARM platforms.  [SAMI]
> 
>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> index 7cb47937329efa1944ec4c156e2be995a755ce6f..0badb24f030a58363e7bee22a7cc1e03040b60aa 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
> @@ -450,6 +450,7 @@ [Components.common]
>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf

Unless you object to my moving that line 3 rows up before I push:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>        HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
>        PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
>        BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 

Re: [PATCH v1] Platforms/ARM: Enable acpiview support

[Leif Lindholm]

On Fri, Apr 27, 2018 at 06:32:46PM +0530, Thomas Abraham wrote:
> On Thu, Apr 26, 2018 at 8:38 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Apr 26, 2018 at 02:01:21PM +0100, Sami Mujawar wrote:
> >> Acpiview is a program that allows examination of ACPI table
> >> contents from the UEFI Shell.
> >>
> >> Enable acpiview support as this can help with investigations,
> >> especially at that stage where the tables are not enabling an
> >> OS to boot.
> >
> > I have no objections to this, but I would prefer to see a comment from
> > Arvind, Thomas or Daniil before merging.
> > Adding Nariman to cc as well for good measure :)
> >
> > However, please use --subject-prefix="PATCH edk2-platforms“ in future.
> >
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> >> Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
> >> ---
> >> The patch can be seen at:
> >> https://github.com/samimujawar/edk2-platforms/tree/254_enable_acpiview_v1
> >>
> >> Notes:
> >>     v1:
> >>     - Enable acpiview support in the UEFI shell for ARM platforms.  [SAMI]
> >>
> >>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> >> index 7cb47937329efa1944ec4c156e2be995a755ce6f..0badb24f030a58363e7bee22a7cc1e03040b60aa 100644
> >> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> >> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> >> @@ -1,5 +1,5 @@
> >>  #
> >> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> >> +#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
> >>  #
> >>  #  This program and the accompanying materials
> >>  #  are licensed and made available under the terms and conditions of the BSD License
> >> @@ -450,6 +450,7 @@ [Components.common]
> >>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> >>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
> >>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
> >> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
> >
> > Although I would probably move this line up a few to improve sorting.
> >
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> >>        HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> >>        PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> >>        BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
> 
> Tested this patch with the Base RevC AEMv8 fvp and acpiview command
> showed a very useful dump of the ACPI tables
> 
> Tested-by: Thomas Abraham <thomas.abraham@arm.com>

Thanks.

Sami acknowledged my moving lines around off-list.
Pushed as 5fc0e7b54b.

/
    Leif