Sorry for missing this email.
Some minor comments below.
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Wednesday, May 8, 2019 9:44 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ray <ray...@intel.com>;
> sami.muja...@arm.com; girish.pat...@arm.com;
> pierre.gond...@arm.com; matteo.carl...@arm.com; Stephanie.Hughes-
> f...@arm.com; n...@arm.com
> Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: acpiview: Add GT Frame
> Number validation to GTDT parser
>
>
> The ACPI 6.2 specification mandates that the Generic Timer (GT) Block Timer
> Structures must have a frame number in the range 0-7.
>
> Update the GTDT parser to warn if this condition is violated.
>
> Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com>
> ---
>
> The changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/woa_528_acpiview_gt_fram
> e_validate_v1
>
> Notes:
> v1:
> - Add GTDT Frame Number checks [Krzysztof]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> | 47 +++++++++++++++++++-
> 1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> index
> f31c4a2761751c58d4b1d3eb75084e24ec318e7f..572e52385c54932b14630481e
> 85a67d0b211966a 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser
> .c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtPars
> +++ er.c
> @@ -1,7 +1,7 @@
> /** @file
> GTDT table parser
>
> - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @par Reference(s):
> @@ -38,6 +38,21 @@ ValidateGtBlockTimerCount (
> IN VOID* Context
> );
>
> +/**
> + This function validates the GT Frame Number.
> +
> + @param [in] Ptr Pointer to the start of the field data.
> + @param [in] Context Pointer to context specific information e.g. this
> + could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateGtFrameNumber (
> + IN UINT8* Ptr,
> + IN VOID* Context
> + );
> +
> /**
> An ACPI_PARSER array describing the ACPI GTDT Table.
> **/
> @@ -92,7 +107,7 @@ STATIC CONST ACPI_PARSER GtBlockParser[] = {
> An ACPI_PARSER array describing the GT Block timer.
> **/
> STATIC CONST ACPI_PARSER GtBlockTimerParser[] = {
> - {L"Frame Number", 1, 0, L"%d", NULL, NULL, NULL, NULL},
> + {L"Frame Number", 1, 0, L"%d", NULL, NULL, ValidateGtFrameNumber,
> + NULL},
> {L"Reserved", 3, 1, L"%x %x %x", Dump3Chars, NULL, NULL, NULL},
> {L"Physical address (CntBaseX)", 8, 4, L"0x%lx", NULL, NULL, NULL, NULL},
> {L"Physical address (CntEL0BaseX)", 8, 12, L"0x%lx", NULL, NULL, NULL, @@
> -145,6 +160,34 @@ ValidateGtBlockTimerCount (
> }
> }
>
> +/**
> + This function validates the GT Frame Number.
> +
> + @param [in] Ptr Pointer to the start of the field data.
> + @param [in] Context Pointer to context specific information e.g. this
> + could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateGtFrameNumber (
> + IN UINT8* Ptr,
> + IN VOID* Context
> + )
> +{
> + UINT32 FrameNumber;
Refer to ACPI spec and the code blow, seems UINT8 is enough. I know it would
work fine with a larger type.
Maybe I am overwrought.
Thanks,
Zhichao
> +
> + FrameNumber = *(UINT8*)Ptr;
> +
> + if (FrameNumber > 7) {
> + IncrementErrorCount ();
> + Print (
> + L"\nERROR: GT Frame Number = %d. GT Frame Number must be in
> range 0-7.",
> + FrameNumber
> + );
> + }
> +}
> +
> /**
> This function parses the Platform GT Block.
>
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40754): https://edk2.groups.io/g/devel/message/40754
Mute This Topic: https://groups.io/mt/31543249/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
