* Re: [edk2-devel] [PATCH v1-resend 1/1] ShellPkg: Add revision check for DSDT Header on Arm
2022-08-23 9:44 ` [edk2-devel] [PATCH v1-resend 1/1] ShellPkg: Add revision check for DSDT Header on Arm Edward Pickup
@ 2022-08-24 2:30 ` Gao, Zhichao
0 siblings, 0 replies; 2+ messages in thread
From: Gao, Zhichao @ 2022-08-24 2:30 UTC (permalink / raw)
To: devel@edk2.groups.io, edward.pickup@arm.com
Cc: Ni, Ray, sami.mujawar@arm.com, nd@arm.com
Hi Edward,
Sorry, I missed this patch. Please feel free to inform the maintainers/reviewers if the patch didn't get response for a long time.
The patch looks good to me.
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Thanks,
Zhichao
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Edward
> Pickup
> Sent: Tuesday, August 23, 2022 5:44 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; sami.mujawar@arm.com; nd@arm.com
> Subject: Re: [edk2-devel] [PATCH v1-resend 1/1] ShellPkg: Add revision
> check for DSDT Header on Arm
>
> 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_v
> 1_
> > 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..7f30a8409bb711d4edf77ed0
> 4319
> > c628ef8aea73 100644
> > ---
> >
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtParser
> > .c
> > +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dsdt/DsdtPa
> > +++ rser.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
> >
> > }
> >
>
>
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread