[PATCH v1 1/1] ShellPkg/UefiShellAcpiViewCommandLib: acpi version update for GTDT


Shashi Mallela <shashi.mallela@...>
 

Updated the GTDT signature to ACPIv6.3

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index d2f26ff89f12..3f9173523493 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -52,7 +52,7 @@ ACPI_TABLE_PARSER ParserList[] = {
ParseAcpiDsdt},
{EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE, ParseAcpiFacs},
{EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiFadt},
- {EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiGtdt},
+ {EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiGtdt},
{EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE, ParseAcpiIort},
{EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiMadt},
{EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
--
2.18.4


Leif Lindholm
 

Hi Shashi,

I am a little bit confused here.

edk2$ git grep EFI_ACPI_6_._GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE
MdePkg/Include/IndustryStandard/Acpi60.h:#define EFI_ACPI_6_0_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE SIGNATURE_32('G', 'T', 'D', 'T')
MdePkg/Include/IndustryStandard/Acpi61.h:#define EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE SIGNATURE_32('G', 'T', 'D', 'T')
MdePkg/Include/IndustryStandard/Acpi62.h:#define EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE SIGNATURE_32('G', 'T', 'D', 'T')
MdePkg/Include/IndustryStandard/Acpi63.h:#define EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE SIGNATURE_32('G', 'T', 'D', 'T')

What is the benefit of changing the name of the macro used?

Best Regards,

Leif

On Thu, Oct 08, 2020 at 16:57:41 -0400, Shashi Mallela wrote:
Updated the GTDT signature to ACPIv6.3

Signed-off-by: Shashi Mallela <shashi.mallela@linaro.org>
---
ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index d2f26ff89f12..3f9173523493 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -52,7 +52,7 @@ ACPI_TABLE_PARSER ParserList[] = {
ParseAcpiDsdt},
{EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE, ParseAcpiFacs},
{EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiFadt},
- {EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiGtdt},
+ {EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiGtdt},
{EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE, ParseAcpiIort},
{EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, ParseAcpiMadt},
{EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
--
2.18.4


Shashi Mallela
 

Hi Lief,

The macro is only updated to reflect the latest ACPI version being considered for GTDT.Since i had updated the edk2-platform code to use the latest structure definitions from EFI_ACPI_6_3,did the same
here for version number consistency.

Thanks
Shashi


Shashi Mallela
 

Hi Lief,

The macro has only been updated to reflect the latest ACPI version 6.3 and stay in sync with the edk2-platform gtdt updates made for sbsa platform
based on the same ACPI version 6.3.

Thanks
Shashi


Leif Lindholm
 

On Fri, Oct 09, 2020 at 11:27:45 -0700, shashi.mallela@linaro.org wrote:
Hi Lief,

The macro has only been updated to reflect the latest ACPI version
6.3 and stay in sync with the edk2-platform gtdt updates made for
sbsa platform

based on the same ACPI version 6.3.
Which is based on 6.2, which is based on 6.1, which is based on 6.0 as
per my previous email. There is no benefit to changing to a different
macro that expands to exactly the same value.

Yes, it is a bit confusing that we apparently keep adding aliases for
such basic things as signatures of standard tabled for each new
release of ACPI, but it doesn't mean we have to add to the churn by
changing unaffected code simply to "keep up".

This patch has no functional changes, and does not improve the code,
so I would recommend the ShellPkg maintainers ignore it.

/
Leif


Gao, Zhichao
 

Agree with you, Leif. I assume the ACPI header file to keep the table signature to every version to indicate the supported table with specific ACPI version. That's my thought, maybe the maintainer can give a formal interpretation.
But there is not benefit to update it in the tool because the tool is not only support ACPI 6.3.
We can know such table is supported since the specific version of ACPI. But with this update, we loss the info.

Thanks,
Zhichao

-----Original Message-----
From: Leif Lindholm <leif@nuviainc.com>
Sent: Tuesday, October 13, 2020 6:33 PM
To: shashi.mallela@linaro.org
Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
<zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellAcpiViewCommandLib:
acpi version update for GTDT

On Fri, Oct 09, 2020 at 11:27:45 -0700, shashi.mallela@linaro.org wrote:
Hi Lief,

The macro has only been updated to reflect the latest ACPI version
6.3 and stay in sync with the edk2-platform gtdt updates made for sbsa
platform

based on the same ACPI version 6.3.
Which is based on 6.2, which is based on 6.1, which is based on 6.0 as per my
previous email. There is no benefit to changing to a different macro that expands
to exactly the same value.

Yes, it is a bit confusing that we apparently keep adding aliases for such basic
things as signatures of standard tabled for each new release of ACPI, but it
doesn't mean we have to add to the churn by changing unaffected code simply to
"keep up".

This patch has no functional changes, and does not improve the code, so I would
recommend the ShellPkg maintainers ignore it.

/
Leif