Re: [edk2-devel] [PATCH v1-resend 1/1] ShellPkg: Add revision check for DSDT Header on Arm

["Edward Pickup" <edward.pickup@arm.com>]

Hi Zhichao,

Would it be possible to get some feedback on this patch?

Regards,

Edward

On 20/07/2022 09:59, Edward Pickup via groups.io wrote:
> Bugzilla: 3995 (https://bugzilla.tianocore.org/show_bug.cgi?id=3995)
>
> ACPI 6.4 spec states that if the revision field in the DSDT header is less
> than 2, then all integers are restricted in width to 32 bits, including in
> SSDTs.
>
> Arm Base boot requirements state that platforms must conform to ACPI 6.3
> or later, and that legacy tables are not supported.
>
> Adds a check for this field and raise warning if revision is less
> than 2 on arm.
>
> Signed-off-by: Edward Pickup <edward.pickup@arm.com>
> ---
>
> The changes can be seen at https://github.com/edpickup/edk2/tree/2073_add_DSDT_revision_check_v1_resend
>
> Notes:
>      v1:
>        - Resending with rebase and bugzilla id
>        - No code changes since previous submit.
>          Ref: https://edk2.groups.io/g/devel/message/89378
>
>   ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser.c
> index e75e4622239ed03b171adc92d32b5eeb75a8edef..7f30a8409bb711d4edf77ed04319c628ef8aea73 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser.c
> @@ -1,7 +1,7 @@
>   /** @file
>
>     DSDT table parser
>
>   
>
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
>
> +  Copyright (c) 2016 - 2022, ARM Limited. All rights reserved.
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
>     @par Reference(s):
>
> @@ -39,4 +39,20 @@ ParseAcpiDsdt (
>     }
>
>   
>
>     DumpAcpiHeader (Ptr);
>
> +
>
> +  // As per 19.6.29 in the version 6.4 of the ACPI spec, a revision less than 2
>
> +  // restricts integers to 32 bit width. This may not be intended, raise a
>
> +  // warning
>
> + #if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_ARM)
>
> +  if (AcpiTableRevision < 2) {
>
> +    IncrementWarningCount ();
>
> +    Print (
>
> +      L"WARNING: DSDT Table Revision less than 2. Integer width restricted to "
>
> +      L"32 bits. Table Revision = %d.\n",
>
> +      AcpiTableRevision
>
> +      );
>
> +    return;
>
> +  }
>
> +
>
> + #endif
>
>   }
>