* [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.
@ 2023-07-12 11:44 yeoreum.yun
2023-07-13 7:11 ` PierreGondois
2023-07-13 14:24 ` [edk2-devel] " Pedro Falcato
0 siblings, 2 replies; 4+ messages in thread
From: yeoreum.yun @ 2023-07-12 11:44 UTC (permalink / raw)
To: devel; +Cc: yeoreum.yun, zhichao.gao, sami.mujawar, pierre.gondois
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 <yeoreum.yun@arm.com>
---
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")
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.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.
2023-07-12 11:44 [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information yeoreum.yun
@ 2023-07-13 7:11 ` PierreGondois
2023-07-13 14:24 ` [edk2-devel] " Pedro Falcato
1 sibling, 0 replies; 4+ messages in thread
From: PierreGondois @ 2023-07-13 7:11 UTC (permalink / raw)
To: levi.yun, devel; +Cc: zhichao.gao, sami.mujawar
Hello Levi,
The patch looks good to me, also:
Tested-by: Pierre Gondois <pierre.gondois@arm.com>
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 <yeoreum.yun@arm.com>
> ---
> 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")
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.
2023-07-12 11:44 [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information yeoreum.yun
2023-07-13 7:11 ` PierreGondois
@ 2023-07-13 14:24 ` Pedro Falcato
2023-07-14 9:04 ` levi.yun
1 sibling, 1 reply; 4+ messages in thread
From: Pedro Falcato @ 2023-07-13 14:24 UTC (permalink / raw)
To: devel, yeoreum.yun; +Cc: zhichao.gao, sami.mujawar, pierre.gondois
Nit: AcpiView, not Acpivew
On Wed, Jul 12, 2023 at 4:16 PM levi.yun <yeoreum.yun@arm.com> wrote:
>
> 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 <yeoreum.yun@arm.com>
> ---
> 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.
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 = (*(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 = (*(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[] = {
>
> + { 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")
> 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.
>
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.
2023-07-13 14:24 ` [edk2-devel] " Pedro Falcato
@ 2023-07-14 9:04 ` levi.yun
0 siblings, 0 replies; 4+ messages in thread
From: levi.yun @ 2023-07-14 9:04 UTC (permalink / raw)
To: Pedro Falcato, devel@edk2.groups.io
Cc: zhichao.gao@intel.com, Sami Mujawar, Pierre Gondois, nd
Hi, Pedro
> Nit: AcpiView, not Acpivew
> Nit: information
> typo: trigger mode
Thanks to find out type :)
>>
>> + UINT8 TriggerMode;
>>
>> +
>>
>> + TriggerMode = (*(UINT8 *)Ptr);
> You don't need parenthesis here, nor a cast.
Yes Thanks..!
>
>>
>> +
>>
>> + 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).
Yes. I'll remove the parenthesis in around TriggerMode unless script of uncrustify complain.
>
>>
>> + TriggerMode
>>
>> + );
>>
>> +}
>>
>> +
>>
>> +/**
>>
>> + This function prints Polarity information in timer flags.
> polarity (lowercase)
Thanks.
Many Thanks..!
________________________________________
From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: 13 July 2023 15:24
To: devel@edk2.groups.io; Yeo Reum Yun
Cc: zhichao.gao@intel.com; Sami Mujawar; Pierre Gondois
Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information.
Nit: AcpiView, not Acpivew
On Wed, Jul 12, 2023 at 4:16 PM levi.yun <yeoreum.yun@arm.com> wrote:
>
> 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 <yeoreum.yun@arm.com>
> ---
> 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.
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 = (*(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 = (*(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[] = {
>
> + { 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")
> 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.
>
>
>
>
>
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-14 9:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-12 11:44 [PATCH v1 1/1] ShellPkg: Acpivew/GTDT: Print timer flags information yeoreum.yun
2023-07-13 7:11 ` PierreGondois
2023-07-13 14:24 ` [edk2-devel] " Pedro Falcato
2023-07-14 9:04 ` levi.yun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox