From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.3066.1689232270257698635 for ; Thu, 13 Jul 2023 00:11:10 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DDD2B12FC; Thu, 13 Jul 2023 00:11:51 -0700 (PDT) Received: from [10.34.100.101] (e126645.nice.arm.com [10.34.100.101]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B661F3F73F; Thu, 13 Jul 2023 00:11:08 -0700 (PDT) Message-ID: <050bb213-315a-7ce9-44b2-e156a47d250f@arm.com> Date: Thu, 13 Jul 2023 09:11:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information. To: "levi.yun" , devel@edk2.groups.io Cc: zhichao.gao@intel.com, sami.mujawar@arm.com References: <20230712114431.1145755-1-yeoreum.yun@arm.com> From: "PierreGondois" In-Reply-To: <20230712114431.1145755-1-yeoreum.yun@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Levi, The patch looks good to me, also: Tested-by: Pierre Gondois Regards, Pierre On 7/12/23 13:44, levi.yun wrote: > Currently, GTDT only prints the value of timer flags in hex. > This change prints the detail informaiton about Timer flags in GTDT. > > before: > Shell> acpiview -s GTDT > ... > Non-Secure EL1 timer FLAGS : 0x2 > Virtual timer GSIV : 0x1B > Virtual timer FLAGS : 0x2 > ... > > after: > Shell> acpiview -s GTDT > ... > Non-Secure EL1 timer FLAGS : 0x2 > Timer Interrupt Mode : Level Trigger(0) > Timer Interrupt Polarity : Active Low(1) > Always-on Capability : 0 > Reserved : 0 > > Virtual timer GSIV : 0x1B > Virtual timer FLAGS : 0x2 > > Signed-off-by: levi.yun > --- > The changes can be seen at https://github.com/LeviYeoReum/edk2/compare/master...LeviYeoReum:edk2:refs.geads/2711_gtdt_flags_v1 > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 111 +++++++++++++++++--- > 1 file changed, 98 insertions(+), 13 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c > index e62927098a010a0e1dad8361dcfc6559d32dcebf..0001d4231e88c03fa1e296539e9fbc76eb5dd7e1 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c > @@ -84,33 +84,118 @@ ValidateGtFrameNumber ( > } > } > > +/** > + This function prints Triggermode information in timer flags. > + > + @param [in] Format Print format. > + @param [in] Ptr Pointer to the start of the field data. > +**/ > +STATIC > +VOID > +EFIAPI > +PrintTimerInterruptMode ( > + IN CONST CHAR16 *Format OPTIONAL, > + IN UINT8 *Ptr > + ) > +{ > + UINT8 TriggerMode; > + > + TriggerMode = (*(UINT8 *)Ptr); > + > + Print ( > + L"%s(%d)", > + ((TriggerMode) ? L"Edge Trigger" : L"Level Trigger"), > + TriggerMode > + ); > +} > + > +/** > + This function prints Polarity information in timer flags. > + > + @param [in] Format Print format. > + @param [in] Ptr Pointer to the start of the field data. > +**/ > +STATIC > +VOID > +EFIAPI > +PrintTimerInterruptPolarity ( > + IN CONST CHAR16 *Format OPTIONAL, > + IN UINT8 *Ptr > + ) > +{ > + UINT8 Polarity; > + > + Polarity = (*(UINT8 *)Ptr); > + > + Print ( > + L"%s(%d)", > + ((Polarity) ? L"Active Low" : L"Active High"), > + Polarity > + ); > +} > + > +/** > + An ACPI_PARSER array describing the Timer Flags Field in GTDT Table. > +**/ > +STATIC CONST ACPI_PARSER TimerFlagsParser[] = { > + { L"Timer Interrupt Mode", 1, 0, NULL, PrintTimerInterruptMode, NULL, NULL, NULL }, > + { L"Timer Interrupt Polarity", 1, 1, NULL, PrintTimerInterruptPolarity, NULL, NULL, NULL }, > + { L"Always-on Capability", 1, 2, L"%d", NULL, NULL, NULL, NULL }, > + { L"Reserved", 29, 3, L"%d", NULL, NULL, NULL, NULL }, > +}; > + > +/** > + This function parses the Timer Flags. > + > + @param [in] Format Print format. > + @param [in] Ptr Pointer to the start of the Timer flags. > + **/ > +STATIC > +VOID > +EFIAPI > +DumpTimerFlags ( > + IN CONST CHAR16 *Format OPTIONAL, > + IN UINT8 *Ptr > + ) > +{ > + DumpUint32 (L"0x%x\n", Ptr); > + ParseAcpiBitFields ( > + TRUE, > + 2, > + NULL, > + Ptr, > + 4, > + PARSER_PARAMS (TimerFlagsParser) > + ); > +} > + > /** > An ACPI_PARSER array describing the ACPI GTDT Table. > **/ > STATIC CONST ACPI_PARSER GtdtParser[] = { > PARSE_ACPI_HEADER (&AcpiHdrInfo), > - { L"CntControlBase Physical Address",8, 36, L"0x%lx", NULL, NULL, > + { L"CntControlBase Physical Address",8, 36, L"0x%lx", NULL, NULL, > NULL, NULL }, > - { L"Reserved", 4, 44, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Secure EL1 timer GSIV", 4, 48, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Secure EL1 timer FLAGS", 4, 52, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Reserved", 4, 44, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Secure EL1 timer GSIV", 4, 48, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Secure EL1 timer FLAGS", 4, 52, NULL, DumpTimerFlags, NULL,NULL, NULL }, > > - { L"Non-Secure EL1 timer GSIV", 4, 56, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Non-Secure EL1 timer FLAGS", 4, 60, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Non-Secure EL1 timer GSIV", 4, 56, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Non-Secure EL1 timer FLAGS", 4, 60, NULL, DumpTimerFlags, NULL,NULL, NULL }, > > - { L"Virtual timer GSIV", 4, 64, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Virtual timer FLAGS", 4, 68, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Virtual timer GSIV", 4, 64, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Virtual timer FLAGS", 4, 68, L"0x%x", DumpTimerFlags, NULL,NULL, NULL }, > > - { L"Non-Secure EL2 timer GSIV", 4, 72, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Non-Secure EL2 timer FLAGS", 4, 76, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Non-Secure EL2 timer GSIV", 4, 72, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Non-Secure EL2 timer FLAGS", 4, 76, L"0x%x", DumpTimerFlags, NULL,NULL, NULL }, > > - { L"CntReadBase Physical address", 8, 80, L"0x%lx", NULL, NULL,NULL, NULL }, > + { L"CntReadBase Physical address", 8, 80, L"0x%lx", NULL, NULL,NULL, NULL }, > { L"Platform Timer Count", 4, 88, L"%d", NULL, > (VOID **)&GtdtPlatformTimerCount, NULL, NULL }, > { L"Platform Timer Offset", 4, 92, L"0x%x", NULL, > (VOID **)&GtdtPlatformTimerOffset,NULL, NULL }, > - { L"Virtual EL2 Timer GSIV", 4, 96, L"0x%x", NULL, NULL,NULL, NULL }, > - { L"Virtual EL2 Timer Flags", 4, 100, L"0x%x", NULL, NULL,NULL, NULL } > + { L"Virtual EL2 Timer GSIV", 4, 96, L"0x%x", NULL, NULL,NULL, NULL }, > + { L"Virtual EL2 Timer Flags", 4, 100, L"0x%x", NULL, NULL,NULL, NULL } > }; > > /** > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")