diff mbox

[Linaro-uefi] Platforms/ARM: fix gtdt.asl for VExpressPkg

Message ID 1468432952-5786-1-git-send-email-fu.wei@linaro.org
State Superseded
Headers show

Commit Message

Fu Wei Fu July 13, 2016, 6:02 p.m. UTC
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(-)

Comments

Graeme Gregory July 14, 2016, 2:03 p.m. UTC | #1
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,
>      EFI_ACPI_RESERVED_BYTE,                               // UINT8 Reserved
> -    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 CntBaseX
> -      0xFFFFFFFFFFFFFFFF,                                   // UINT64 CntEL0BaseX
> -      GT_BLOCK_FRAME1_GSIV,                                 // UINT32 GTxPhysicalTimerGSIV
> -      0,                                                    // UINT32 GTxPhysicalTimerFlags
> +      FVP_GT_BLOCK_FRAME0_CTL_BASE,                         // UINT64 CntBaseX
> +      FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
> +      FVP_GT_BLOCK_FRAME0_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
> +      FVP_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 CntBaseX
> +      FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
> +      FVP_GT_BLOCK_FRAME1_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
> +      FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
> +      0,                                                    // UINT32 GTxVirtualTimerGSIV
> +      0,                                                    // UINT32 GTxVirtualTimerFlags
> +      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
> +    }
> +  },
> +  {
> +    {
> +      EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                // UINT8 Type
> +      sizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 Length
> +      EFI_ACPI_RESERVED_BYTE,                                 // UINT8 Reserved
> +      FVP_SBSA_WATCHDOG_REFRESH_BASE,                         // UINT64 RefreshFramePhysicalAddress
> +      FVP_SBSA_WATCHDOG_CONTROL_BASE,                         // UINT64 WatchdogControlFramePhysicalAddress
> +      FVP_SBSA_WATCHDOG_GSIV,                                 // UINT32 WatchdogTimerGSIV
> +      FVP_SBSA_WATCHDOG_FLAGS                                 // UINT32 WatchdogTimerFlags
>      }
>    }
>  };
> --
> 2.5.5
>

This looks good to me.

Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
Fu Wei Fu July 26, 2016, 2:26 a.m. UTC | #2
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,
>>      EFI_ACPI_RESERVED_BYTE,                               // UINT8 Reserved
>> -    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 CntBaseX
>> -      0xFFFFFFFFFFFFFFFF,                                   // UINT64 CntEL0BaseX
>> -      GT_BLOCK_FRAME1_GSIV,                                 // UINT32 GTxPhysicalTimerGSIV
>> -      0,                                                    // UINT32 GTxPhysicalTimerFlags
>> +      FVP_GT_BLOCK_FRAME0_CTL_BASE,                         // UINT64 CntBaseX
>> +      FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>> +      FVP_GT_BLOCK_FRAME0_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>> +      FVP_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 CntBaseX
>> +      FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>> +      FVP_GT_BLOCK_FRAME1_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>> +      FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
>> +      0,                                                    // UINT32 GTxVirtualTimerGSIV
>> +      0,                                                    // UINT32 GTxVirtualTimerFlags
>> +      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
>> +    }
>> +  },
>> +  {
>> +    {
>> +      EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                // UINT8 Type
>> +      sizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 Length
>> +      EFI_ACPI_RESERVED_BYTE,                                 // UINT8 Reserved
>> +      FVP_SBSA_WATCHDOG_REFRESH_BASE,                         // UINT64 RefreshFramePhysicalAddress
>> +      FVP_SBSA_WATCHDOG_CONTROL_BASE,                         // UINT64 WatchdogControlFramePhysicalAddress
>> +      FVP_SBSA_WATCHDOG_GSIV,                                 // UINT32 WatchdogTimerGSIV
>> +      FVP_SBSA_WATCHDOG_FLAGS                                 // UINT32 WatchdogTimerFlags
>>      }
>>    }
>>  };
>> --
>> 2.5.5
>>
>
> This looks good to me.
>
> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
Leif Lindholm Aug. 11, 2016, 11:23 a.m. UTC | #3
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,
>>>      EFI_ACPI_RESERVED_BYTE,                               // UINT8 Reserved
>>> -    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 CntBaseX
>>> -      0xFFFFFFFFFFFFFFFF,                                   // UINT64 CntEL0BaseX
>>> -      GT_BLOCK_FRAME1_GSIV,                                 // UINT32 GTxPhysicalTimerGSIV
>>> -      0,                                                    // UINT32 GTxPhysicalTimerFlags
>>> +      FVP_GT_BLOCK_FRAME0_CTL_BASE,                         // UINT64 CntBaseX
>>> +      FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>>> +      FVP_GT_BLOCK_FRAME0_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>>> +      FVP_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 CntBaseX
>>> +      FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>>> +      FVP_GT_BLOCK_FRAME1_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>>> +      FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
>>> +      0,                                                    // UINT32 GTxVirtualTimerGSIV
>>> +      0,                                                    // UINT32 GTxVirtualTimerFlags
>>> +      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
>>> +    }
>>> +  },
>>> +  {
>>> +    {
>>> +      EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                // UINT8 Type
>>> +      sizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 Length
>>> +      EFI_ACPI_RESERVED_BYTE,                                 // UINT8 Reserved
>>> +      FVP_SBSA_WATCHDOG_REFRESH_BASE,                         // UINT64 RefreshFramePhysicalAddress
>>> +      FVP_SBSA_WATCHDOG_CONTROL_BASE,                         // UINT64 WatchdogControlFramePhysicalAddress
>>> +      FVP_SBSA_WATCHDOG_GSIV,                                 // UINT32 WatchdogTimerGSIV
>>> +      FVP_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
Fu Wei Fu Aug. 23, 2016, 11:41 a.m. UTC | #4
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:
> 1. "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.
> 2. 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,
>>>>      EFI_ACPI_RESERVED_BYTE,                               // UINT8 Reserved
>>>> -    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 CntBaseX
>>>> -      0xFFFFFFFFFFFFFFFF,                                   // UINT64 CntEL0BaseX
>>>> -      GT_BLOCK_FRAME1_GSIV,                                 // UINT32 GTxPhysicalTimerGSIV
>>>> -      0,                                                    // UINT32 GTxPhysicalTimerFlags
>>>> +      FVP_GT_BLOCK_FRAME0_CTL_BASE,                         // UINT64 CntBaseX
>>>> +      FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>>>> +      FVP_GT_BLOCK_FRAME0_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>>>> +      FVP_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 CntBaseX
>>>> +      FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
>>>> +      FVP_GT_BLOCK_FRAME1_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
>>>> +      FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
>>>> +      0,                                                    // UINT32 GTxVirtualTimerGSIV
>>>> +      0,                                                    // UINT32 GTxVirtualTimerFlags
>>>> +      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
>>>> +    }
>>>> +  },
>>>> +  {
>>>> +    {
>>>> +      EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                // UINT8 Type
>>>> +      sizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 Length
>>>> +      EFI_ACPI_RESERVED_BYTE,                                 // UINT8 Reserved
>>>> +      FVP_SBSA_WATCHDOG_REFRESH_BASE,                         // UINT64 RefreshFramePhysicalAddress
>>>> +      FVP_SBSA_WATCHDOG_CONTROL_BASE,                         // UINT64 WatchdogControlFramePhysicalAddress
>>>> +      FVP_SBSA_WATCHDOG_GSIV,                                 // UINT32 WatchdogTimerGSIV
>>>> +      FVP_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.
diff mbox

Patch

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,
     EFI_ACPI_RESERVED_BYTE,                               // UINT8 Reserved
-    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 CntBaseX
-      0xFFFFFFFFFFFFFFFF,                                   // UINT64 CntEL0BaseX
-      GT_BLOCK_FRAME1_GSIV,                                 // UINT32 GTxPhysicalTimerGSIV
-      0,                                                    // UINT32 GTxPhysicalTimerFlags
+      FVP_GT_BLOCK_FRAME0_CTL_BASE,                         // UINT64 CntBaseX
+      FVP_GT_BLOCK_FRAME0_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
+      FVP_GT_BLOCK_FRAME0_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
+      FVP_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 CntBaseX
+      FVP_GT_BLOCK_FRAME1_CTL_EL0_BASE,                     // UINT64 CntEL0BaseX
+      FVP_GT_BLOCK_FRAME1_GSIV,                             // UINT32 GTxPhysicalTimerGSIV
+      FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
+      0,                                                    // UINT32 GTxVirtualTimerGSIV
+      0,                                                    // UINT32 GTxVirtualTimerFlags
+      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
+    }
+  },
+  {
+    {
+      EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG,                // UINT8 Type
+      sizeof(EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE), // UINT16 Length
+      EFI_ACPI_RESERVED_BYTE,                                 // UINT8 Reserved
+      FVP_SBSA_WATCHDOG_REFRESH_BASE,                         // UINT64 RefreshFramePhysicalAddress
+      FVP_SBSA_WATCHDOG_CONTROL_BASE,                         // UINT64 WatchdogControlFramePhysicalAddress
+      FVP_SBSA_WATCHDOG_GSIV,                                 // UINT32 WatchdogTimerGSIV
+      FVP_SBSA_WATCHDOG_FLAGS                                 // UINT32 WatchdogTimerFlags
     }
   }
 };