Hi Alexei,
Great thanks for pointing it out. That is a wrong definistion.
On 16 August 2016 at 21:32, Alexei Fedorov Alexei.Fedorov@arm.com wrote:
These definitions are wrong:
+#define GTDT_TIMER_SECURE EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY +#define GTDT_TIMER_NON_SECURE 0
+#define FVP_GTDT_GTIMER_FLAGS (GTDT_TIMER_NON_SECURE | GTDT_TIMER_ACTIVE_HIGH | GTDT_TIMER_EDGE_TRIGGERED)
Because:
- "Table 5-118 Flag Definitions: Virtual Timer, EL2 timers, and Secure & Non-Secure EL1 timers" doesn't list security bit flags for these timers, so GTDT_TIMER_SECURE & GTDT_TIMER_NON_SECURE shouldn't exist.
- Why is GTDT_TIMER_SECURE defined as equal to EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY which is BIT2?
Secure Timer is BIT0 & only defined in GTx Common Flags for GT Block Timer Structure
it should be GTDT_TIMER_SAVE_CONTEXT, I copied the code from AMD's file but didn't correct it. There is a same problem at Platforms/AMD/Styx/AcpiTables/Gtdt.c
I have posted the v2 patch to correct it.
Alexei.
-----Original Message----- From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Leif Lindholm Sent: 11 August 2016 12:23 To: Fu Wei; Evan Lloyd Cc: Linaro ACPI Mailman List; hanjun.guo@linaro.org; Linaro UEFI Mailman List Subject: Re: [Linaro-uefi] [PATCH] Platforms/ARM: fix gtdt.asl for VExpressPkg
Sorry, lost track of this one.
No objection from me, but I'd prefer a nod from Evan.
Regards,
Leif
On 26 July 2016 at 03:26, Fu Wei fu.wei@linaro.org wrote:
Hi all,
I guess this patch haven't been merged yet. So do I need to fix it in anywhere or can some one help to merge it ?
Thank! :-)
On 14 July 2016 at 22:03, G Gregory graeme.gregory@linaro.org wrote:
On 13 July 2016 at 19:02, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Add Memory-mapped GT and SBSA Generic Watchdog timer info base on Foundation Model.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Fu Wei fu.wei@linaro.org
Platforms/ARM/VExpress/AcpiTables/Gtdt.aslc | 128 ++++++++++++++++++++++------ 1 file changed, 101 insertions(+), 27 deletions(-)
diff --git a/Platforms/ARM/VExpress/AcpiTables/Gtdt.aslc b/Platforms/ARM/VExpress/AcpiTables/Gtdt.aslc index 142249f..8ed9a31 100644 --- a/Platforms/ARM/VExpress/AcpiTables/Gtdt.aslc +++ b/Platforms/ARM/VExpress/AcpiTables/Gtdt.aslc @@ -19,21 +19,70 @@ #include <Library/PcdLib.h> #include <IndustryStandard/Acpi61.h>
-#define SECURE_TIMER_EL1_GSIV 0x1D -#define NON_SECURE_TIMER_EL1_GSIV 0x1E -#define VIRTUAL_TIMER_GSIV 0x1B -#define NON_SECURE_EL2_GSIV 0x1A +#define FVP_SYSTEM_TIMER_BASE_ADDRESS 0x000000002a430000 +#define FVP_CNT_READ_BASE_ADDRESS 0x000000002a800000
-#define GT_BLOCK_CTL_BASE 0x000000002A810000 -#define GT_BLOCK_FRAME1_CTL_BASE 0x000000002A820000 -#define GT_BLOCK_FRAME1_GSIV 0x29 +#define FVP_SECURE_TIMER_EL1_GSIV 0x1D +#define FVP_NON_SECURE_TIMER_EL1_GSIV 0x1E +#define FVP_VIRTUAL_TIMER_GSIV 0x1B +#define FVP_NON_SECURE_EL2_GSIV 0x1A
+#define GTDT_TIMER_EDGE_TRIGGERED EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE +#define GTDT_TIMER_LEVEL_TRIGGERED 0 +#define GTDT_TIMER_ACTIVE_LOW EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY +#define GTDT_TIMER_ACTIVE_HIGH 0 +#define GTDT_TIMER_SECURE EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY +#define GTDT_TIMER_NON_SECURE 0
+#define FVP_GTDT_GTIMER_FLAGS (GTDT_TIMER_NON_SECURE | GTDT_TIMER_ACTIVE_HIGH | GTDT_TIMER_EDGE_TRIGGERED)
+#define FVP_PLATFORM_TIMER_COUNT 2 +#define FVP_TIMER_FRAMES_COUNT 2 +#define FVP_WATCHDOG_COUNT 1
+#define FVP_GT_BLOCK_CTL_BASE 0x000000002A810000 +#define FVP_GT_BLOCK_FRAME0_CTL_BASE 0x000000002A820000 +#define FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE 0xFFFFFFFFFFFFFFFF +#define FVP_GT_BLOCK_FRAME0_GSIV 0x39
+#define FVP_GT_BLOCK_FRAME1_CTL_BASE 0x000000002A830000 +#define FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE 0xFFFFFFFFFFFFFFFF +#define FVP_GT_BLOCK_FRAME1_GSIV 0x3A
+#define GTX_TIMER_EDGE_TRIGGERED EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_FLAG_TIMER_INTERRUPT_MODE +#define GTX_TIMER_LEVEL_TRIGGERED 0 +#define GTX_TIMER_ACTIVE_LOW EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_FLAG_TIMER_INTERRUPT_POLARITY +#define GTX_TIMER_ACTIVE_HIGH 0
+#define FVP_GTX_TIMER_FLAGS (GTX_TIMER_ACTIVE_HIGH | GTX_TIMER_LEVEL_TRIGGERED)
+#define GTX_TIMER_SECURE EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_SECURE_TIMER +#define GTX_TIMER_NON_SECURE 0 +#define GTX_TIMER_SAVE_CONTEXT EFI_ACPI_6_1_GTDT_GT_BLOCK_COMMON_FLAG_ALWAYS_ON_CAPABILITY +#define GTX_TIMER_LOSE_CONTEXT 0
+#define FVP_GTX_COMMON_FLAGS (GTX_TIMER_SAVE_CONTEXT | GTX_TIMER_SECURE)
+#define FVP_SBSA_WATCHDOG_REFRESH_BASE 0x000000002a450000 +#define FVP_SBSA_WATCHDOG_CONTROL_BASE 0x000000002a440000 +#define FVP_SBSA_WATCHDOG_GSIV 0x3B
+#define SBSA_WATCHDOG_EDGE_TRIGGERED EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_TIMER_INTERRUPT_MODE +#define SBSA_WATCHDOG_LEVEL_TRIGGERED 0 +#define SBSA_WATCHDOG_ACTIVE_LOW EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_TIMER_INTERRUPT_POLARITY +#define SBSA_WATCHDOG_ACTIVE_HIGH 0 +#define SBSA_WATCHDOG_SECURE EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER +#define SBSA_WATCHDOG_NON_SECURE 0
+#define FVP_SBSA_WATCHDOG_FLAGS (SBSA_WATCHDOG_NON_SECURE | SBSA_WATCHDOG_ACTIVE_HIGH | SBSA_WATCHDOG_LEVEL_TRIGGERED)
#pragma pack (1)
typedef struct { EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE Gtdt; EFI_ACPI_6_1_GTDT_GT_BLOCK_STRUCTURE GtBlock;
- EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE Frames[1];
- EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE Frames[FVP_TIMER_FRAMES_COUNT];
- EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE Watchdogs[FVP_WATCHDOG_COUNT];
} FVP_GENERIC_TIMER_DESCRIPTION_TABLES;
#pragma pack () @@ -45,27 +94,28 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = { FVP_GENERIC_TIMER_DESCRIPTION_TABLES, EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION ),
- 0xFFFFFFFFFFFFFFFF, // UINT64 PhysicalAddress
- FVP_SYSTEM_TIMER_BASE_ADDRESS, // UINT64 PhysicalAddress EFI_ACPI_RESERVED_DWORD, // UINT32 Reserved
- SECURE_TIMER_EL1_GSIV, // UINT32 SecurePL1TimerGSIV
- EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE, // UINT32 SecurePL1TimerFlags
- NON_SECURE_TIMER_EL1_GSIV, // UINT32 NonSecurePL1TimerGSIV
- EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE, // UINT32 NonSecurePL1TimerFlags
- VIRTUAL_TIMER_GSIV, // UINT32 VirtualTimerGSIV
- EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE, // UINT32 VirtualTimerFlags
- NON_SECURE_EL2_GSIV, // UINT32 NonSecurePL2TimerGSIV
- EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE, // UINT32 NonSecurePL2TimerFlags
- 0xFFFFFFFFFFFFFFFF, // UINT64 CntReadBasePhysicalAddress
- 1, // UINT32 PlatformTimerCount
- FVP_SECURE_TIMER_EL1_GSIV, // UINT32 SecurePL1TimerGSIV
- FVP_GTDT_GTIMER_FLAGS, // UINT32 SecurePL1TimerFlags
- FVP_NON_SECURE_TIMER_EL1_GSIV, // UINT32 NonSecurePL1TimerGSIV
- FVP_GTDT_GTIMER_FLAGS, // UINT32 NonSecurePL1TimerFlags
- FVP_VIRTUAL_TIMER_GSIV, // UINT32 VirtualTimerGSIV
- FVP_GTDT_GTIMER_FLAGS, // UINT32 VirtualTimerFlags
- FVP_NON_SECURE_EL2_GSIV, // UINT32 NonSecurePL2TimerGSIV
- FVP_GTDT_GTIMER_FLAGS, // UINT32 NonSecurePL2TimerFlags
- FVP_CNT_READ_BASE_ADDRESS, // UINT64 CntReadBasePhysicalAddress
- FVP_PLATFORM_TIMER_COUNT, // UINT32 PlatformTimerCount sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 PlatfromTimerOffset }, { EFI_ACPI_6_1_GTDT_GT_BLOCK, // UINT8 Type sizeof(EFI_ACPI_6_1_GTDT_GT_BLOCK_STRUCTURE) // UINT16 Length
+ sizeof(EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE),
+ sizeof(EFI_ACPI_6_1_GTDT_GT_BLOCK_TIMER_STRUCTURE) *FVP_TIMER_FRAMES_COUNT,
- GT_BLOCK_CTL_BASE, // UINT64 CntCtlBase
- 1, // UINT32 GTBlockTimerCount
- FVP_GT_BLOCK_CTL_BASE, // UINT64 CntCtlBase
- FVP_TIMER_FRAMES_COUNT, // UINT32 GTBlockTimerCount sizeof(EFI_ACPI_6_1_GTDT_GT_BLOCK_STRUCTURE) // UINT32 GTBlockTimerOffset }, {
@@ -74,13 +124,37 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = { {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE}, // UINT8 Reserved[3]
GT_BLOCK_FRAME1_CTL_BASE, // UINT64 CntBaseX0xFFFFFFFFFFFFFFFF, // UINT64 CntEL0BaseXGT_BLOCK_FRAME1_GSIV, // UINT32 GTxPhysicalTimerGSIV0, // UINT32 GTxPhysicalTimerFlags
FVP_GT_BLOCK_FRAME0_CTL_BASE, // UINT64 CntBaseXFVP_GT_BLOCK_FRAME0_CTL_EL0_BASE, // UINT64 CntEL0BaseXFVP_GT_BLOCK_FRAME0_GSIV, // UINT32 GTxPhysicalTimerGSIVFVP_GTX_TIMER_FLAGS, // UINT32 GTxPhysicalTimerFlags 0, // UINT32 GTxVirtualTimerGSIV 0, // UINT32 GTxVirtualTimerFlags
0 // UINT32 GTxCommonFlags
FVP_GTX_COMMON_FLAGS // UINT32 GTxCommonFlags- },
- {
1, // UINT8 GTFrameNumber{EFI_ACPI_RESERVED_BYTE,EFI_ACPI_RESERVED_BYTE,EFI_ACPI_RESERVED_BYTE}, // UINT8 Reserved[3]FVP_GT_BLOCK_FRAME1_CTL_BASE, // UINT64 CntBaseXFVP_GT_BLOCK_FRAME1_CTL_EL0_BASE, // UINT64 CntEL0BaseXFVP_GT_BLOCK_FRAME1_GSIV, // UINT32 GTxPhysicalTimerGSIVFVP_GTX_TIMER_FLAGS, // UINT32 GTxPhysicalTimerFlags0, // UINT32 GTxVirtualTimerGSIV0, // UINT32 GTxVirtualTimerFlagsFVP_GTX_COMMON_FLAGS // UINT32 GTxCommonFlags- }
- },
- {
- {
EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG, // UINT8 Typesizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 LengthEFI_ACPI_RESERVED_BYTE, // UINT8 ReservedFVP_SBSA_WATCHDOG_REFRESH_BASE, // UINT64 RefreshFramePhysicalAddressFVP_SBSA_WATCHDOG_CONTROL_BASE, // UINT64 WatchdogControlFramePhysicalAddressFVP_SBSA_WATCHDOG_GSIV, // UINT32 WatchdogTimerGSIVFVP_SBSA_WATCHDOG_FLAGS // UINT32 WatchdogTimerFlags};
2.5.5
This looks good to me.
Reviewed-by: Graeme Gregory graeme.gregory@linaro.org
-- Best regards,
Fu Wei Software Engineer Red Hat
Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi 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.