[edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg

PierreGondois posted 5 patches 4 years, 7 months ago
There is a newer version of this series
[edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
Posted by PierreGondois 4 years, 7 months ago
From: Sami Mujawar <sami.mujawar@arm.com>

Acpiview is a command line tool allowing to display, dump, or
check installed ACPI tables. Add the tool to ArmVirt platforms.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index d9abadbe708c..269ac4990a6c 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2021, Arm Limited. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
 #  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
 #  Copyright (c) Microsoft Corporation.
@@ -398,6 +398,7 @@ [Components.common]
       NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
       NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
       NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
 !if $(NETWORK_IP6_ENABLE) == TRUE
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76995): https://edk2.groups.io/g/devel/message/76995
Mute This Topic: https://groups.io/mt/83738491/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
Posted by Laszlo Ersek 4 years, 7 months ago
On 06/23/21 16:06, PierreGondois wrote:
> From: Sami Mujawar <sami.mujawar@arm.com>
> 
> Acpiview is a command line tool allowing to display, dump, or
> check installed ACPI tables. Add the tool to ArmVirt platforms.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index d9abadbe708c..269ac4990a6c 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2021, Arm Limited. All rights reserved.
>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
>  #  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
>  #  Copyright (c) Microsoft Corporation.
> @@ -398,6 +398,7 @@ [Components.common]
>        NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>  !if $(NETWORK_IP6_ENABLE) == TRUE
> 

I disagree with this patch, as it will cause the Shell binary in all
ArmVirtPkg platforms to include the (rather large) ACPIVIEW command.

ACPIVIEW is super useful for when the tables are (dynamically) generated
by the firmware itself, but that does not apply to the Qemu and Xen
platforms.

Note NETWORK_IP6_ENABLE: UefiShellNetwork2CommandsLib is only hooked
into the shell application if NETWORK_IP6_ENABLE is TRUE.

Please add

  DEFINE ACPIVIEW_ENABLE                 = TRUE

to "ArmVirtPkg/ArmVirtKvmTool.dsc", and in "ArmVirtPkg/ArmVirt.dsc.inc",
include the new command lib conditionally on ACPIVIEW_ENABLE being TRUE.
(Can be in the same patch.)

Acked-by: Laszlo Ersek <lersek@redhat.com>


Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77060): https://edk2.groups.io/g/devel/message/77060
Mute This Topic: https://groups.io/mt/83738491/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 5/5] ArmVirtPkg: Enable Acpiview for ArmVirtPkg
Posted by Laszlo Ersek 4 years, 7 months ago
On 06/24/21 14:59, Laszlo Ersek wrote:
> On 06/23/21 16:06, PierreGondois wrote:
>> From: Sami Mujawar <sami.mujawar@arm.com>
>>
>> Acpiview is a command line tool allowing to display, dump, or
>> check installed ACPI tables. Add the tool to ArmVirt platforms.
>>
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index d9abadbe708c..269ac4990a6c 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -1,5 +1,5 @@
>>  #
>> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
>> +#  Copyright (c) 2011-2021, Arm Limited. All rights reserved.
>>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
>>  #  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
>>  #  Copyright (c) Microsoft Corporation.
>> @@ -398,6 +398,7 @@ [Components.common]
>>        NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>>        NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>        NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>> +      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
>>        NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>>        NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>>  !if $(NETWORK_IP6_ENABLE) == TRUE
>>
> 
> I disagree with this patch, as it will cause the Shell binary in all
> ArmVirtPkg platforms to include the (rather large) ACPIVIEW command.
> 
> ACPIVIEW is super useful for when the tables are (dynamically) generated
> by the firmware itself, but that does not apply to the Qemu and Xen
> platforms.
> 
> Note NETWORK_IP6_ENABLE: UefiShellNetwork2CommandsLib is only hooked
> into the shell application if NETWORK_IP6_ENABLE is TRUE.
> 
> Please add
> 
>   DEFINE ACPIVIEW_ENABLE                 = TRUE
> 
> to "ArmVirtPkg/ArmVirtKvmTool.dsc",

To clarify: please place

  DEFINE ACPIVIEW_ENABLE                 = TRUE

in a new [Defines.AARCH64] section in "ArmVirtPkg/ArmVirtKvmTool.dsc",
not in the existent [Defines] section.

This should happen just before !including "ArmVirtPkg/ArmVirt.dsc.inc".

Thanks
Laszlo

> and in "ArmVirtPkg/ArmVirt.dsc.inc",
> include the new command lib conditionally on ACPIVIEW_ENABLE being TRUE.
> (Can be in the same patch.)
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 
> 
> Thanks
> Laszlo
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77062): https://edk2.groups.io/g/devel/message/77062
Mute This Topic: https://groups.io/mt/83738491/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-