From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f54.google.com (mail-vs1-f54.google.com [209.85.217.54]) by mx.groups.io with SMTP id smtpd.web10.9640.1689258276124746196 for ; Thu, 13 Jul 2023 07:24:36 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=U/9YqQrU; spf=pass (domain: gmail.com, ip: 209.85.217.54, mailfrom: pedro.falcato@gmail.com) Received: by mail-vs1-f54.google.com with SMTP id ada2fe7eead31-4435fa903f2so249887137.1 for ; Thu, 13 Jul 2023 07:24:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689258275; x=1691850275; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=owJ6BIuREP827Sg61sXZV4A27Q70IjKYHxk+MGl7Ch0=; b=U/9YqQrUcpiQIqINzQjgXEyaytOdNKxtEpSOVhZrc/ognIEwQnt6TNSO0zdqS+eYku /N5C7HGMxO6upb4mmI3h9e/P1VPR594dTJkZo6zvfe2yRA1wb0PfHj9tJtSvNwQUtlvL XYXpai0XxoxQf8tj7ZdNgEVNp+vu3xWEPIUrfKiOAbz/npap/lc4BmHJoML1WIBWfNPA 2aLEx/k95irNINh2P7QQZCz+f7QH6+xn93IQ9BmdiieGkeoJD8+zH5VpHDRJceM9Lo6l 5aJU3R/PgiqtESZeyalHMNwDkyLukPf2xiXyN3LZGPk5NtnfAin0gOVizFMhEKThRRyD 78og== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689258275; x=1691850275; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=owJ6BIuREP827Sg61sXZV4A27Q70IjKYHxk+MGl7Ch0=; b=dUKdEA+qFNwEIvlFoMU7p6EgmWEQmuObxFlKL2C253OP8zYqqQhj1EInky0hY6T+R1 6rcPgLYjxdfIA9fj2+hEqRVzbOE9QUhzalXFsuO1yP3oCWkFGowVS8d2Mj4naKIqQwgV EKTW6u8Y6yJfhTWdRILBmOr4xPysluYdCGQ/7jDFeiUwH7M3QqFPwYCx+SnbNrooB2bT jYYT/XNrfkQb9BiFZO9B0P0soh6D+HjwwQKOvkQkmxZJyB39itxNl+2pwzmAKMMpBUYk qyJiP41rCyh2ngVP72aEIyHG9uSMKc4RBTFbkjThJy1xGf7mQKgD4ZjhmLadnVXWLmvK hGtA== X-Gm-Message-State: ABy/qLa68vV7swCiywUAQDjEfk4Rd8NqRuxQzyM4DmmYneyJr4C5MHps NLJAJuuWqjM8b0FcVlxEyT89QvZoy+q+AXAP15OMVVwY X-Google-Smtp-Source: APBJJlFYafmklVGsdINscZTQDjq1moGhDIR22aMrxE25W3GxPi0UwCsrdlhxC4cMFG8QEOBFceVB7lB5ne5JVQE48xA= X-Received: by 2002:a05:6102:3641:b0:444:481d:f1 with SMTP id s1-20020a056102364100b00444481d00f1mr603523vsu.19.1689258274836; Thu, 13 Jul 2023 07:24:34 -0700 (PDT) MIME-Version: 1.0 References: <20230712114431.1145755-1-yeoreum.yun@arm.com> In-Reply-To: <20230712114431.1145755-1-yeoreum.yun@arm.com> From: "Pedro Falcato" Date: Thu, 13 Jul 2023 15:24:22 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information. To: devel@edk2.groups.io, yeoreum.yun@arm.com Cc: zhichao.gao@intel.com, sami.mujawar@arm.com, pierre.gondois@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Nit: AcpiView, not Acpivew On Wed, Jul 12, 2023 at 4:16=E2=80=AFPM levi.yun wrot= e: > > Currently, GTDT only prints the value of timer flags in hex. > This change prints the detail informaiton about Timer flags in GTDT. Nit: information > > 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/ma= ster...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/Gt= dtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/Gtdt= Parser.c > index e62927098a010a0e1dad8361dcfc6559d32dcebf..0001d4231e88c03fa1e296539= e9fbc76eb5dd7e1 100644 > --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParse= r.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParse= r.c > @@ -84,33 +84,118 @@ ValidateGtFrameNumber ( > } > > } > > > > +/** > > + This function prints Triggermode information in timer flags. typo: trigger mode > > + > > + @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 =3D (*(UINT8 *)Ptr); You don't need parenthesis here, nor a cast. > > + > > + Print ( > > + L"%s(%d)", Personal opinion, but a space between %s and (%d) would look better maybe? > > + ((TriggerMode) ? L"Edge Trigger" : L"Level Trigger"), You don't need parenthesis around TriggerMode either AFAIK (unless this is some uncrustify BS). > > + TriggerMode > > + ); > > +} > > + > > +/** > > + This function prints Polarity information in timer flags. polarity (lowercase) > > + > > + @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 =3D (*(UINT8 *)Ptr); > > + > > + Print ( > > + L"%s(%d)", > > + ((Polarity) ? L"Active Low" : L"Active High"), > > + Polarity > > + ); Same comments as the above function > > +} > > + > > +/** > > + An ACPI_PARSER array describing the Timer Flags Field in GTDT Table. > > +**/ > > +STATIC CONST ACPI_PARSER TimerFlagsParser[] =3D { > > + { L"Timer Interrupt Mode", 1, 0, NULL, PrintTimerInterruptMode, = NULL, NULL, NULL }, > > + { L"Timer Interrupt Polarity", 1, 1, NULL, PrintTimerInterruptPolari= ty, 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[] =3D { > > 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, DumpTimerFl= ags, 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, DumpTimerFl= ags, 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", DumpTimerFl= ags, 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", DumpTimerFl= ags, 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") > IMPORTANT NOTICE: The contents of this email and any attachments are conf= idential and may also be privileged. If you are not the intended recipient,= please notify the sender immediately and do not disclose the contents to a= ny other person, use it for any purpose, or store or copy the information i= n any medium. Thank you. > > >=20 > >