[PATCH v1] Platform/ARM: Fix platform timer offset in GTDT

[PATCH v1] Platform/ARM: Fix platform timer offset in GTDT

[Sami Mujawar]

The FVP_PLATFORM_TIMER_COUNT is the sum of the memory
mapped platform timers and the watchdog timers.

The watchdog timers can be disabled by setting the
FVP_WATCHDOG_COUNT (defined by PcdWatchdogCount)
to zero.

On the VExpress platform, if the FVP_WATCHDOG_COUNT is
set to zero, the FVP_PLATFORM_TIMER_COUNT is 1 as
VExpress has one memory mapped timer.

The code however incorrectly sets the platform timer
offset to zero in the GTDT. This causes the OS to read
the platform timer information from an invalid offset,
and may crash.

Updated the GTDT table to set the platform timer offset
to zero only when the FVP_PLATFORM_TIMER_COUNT is zero.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>
---

Notes:
    v1:
     - Fix platform timer offset in GTDT for FVP                  [SAMI]

 Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
index ae570574b298094ff468f30b78fbd8c98db506c5..1cb4b498300cf1a08514835677154eace1dd1803 100644
--- a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
+++ b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
@@ -108,7 +108,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
     FVP_GTDT_GTIMER_FLAGS,                                // UINT32  NonSecurePL2TimerFlags
     FVP_CNT_READ_BASE_ADDRESS,                            // UINT64  CntReadBasePhysicalAddress
     FVP_PLATFORM_TIMER_COUNT,                             // UINT32  PlatformTimerCount
-#if (FVP_WATCHDOG_COUNT != 0)
+#if (FVP_PLATFORM_TIMER_COUNT != 0)
     sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32  PlatfromTimerOffset
 #else
     0
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v1] Platform/ARM: Fix platform timer offset in GTDT

[Ard Biesheuvel]

On 24 April 2018 at 16:19, Sami Mujawar <sami.mujawar@arm.com> wrote:
> The FVP_PLATFORM_TIMER_COUNT is the sum of the memory
> mapped platform timers and the watchdog timers.
>
> The watchdog timers can be disabled by setting the
> FVP_WATCHDOG_COUNT (defined by PcdWatchdogCount)
> to zero.
>
> On the VExpress platform, if the FVP_WATCHDOG_COUNT is
> set to zero, the FVP_PLATFORM_TIMER_COUNT is 1 as
> VExpress has one memory mapped timer.
>
> The code however incorrectly sets the platform timer
> offset to zero in the GTDT. This causes the OS to read
> the platform timer information from an invalid offset,
> and may crash.
>
> Updated the GTDT table to set the platform timer offset
> to zero only when the FVP_PLATFORM_TIMER_COUNT is zero.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Reviewed-by: Evan Lloyd <evan.lloyd@arm.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as ed9be80fa9521edc2ef959d493904d4800e64ca1

Thanks

> ---
>
> Notes:
>     v1:
>      - Fix platform timer offset in GTDT for FVP                  [SAMI]
>
>  Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> index ae570574b298094ff468f30b78fbd8c98db506c5..1cb4b498300cf1a08514835677154eace1dd1803 100644
> --- a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> +++ b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> @@ -108,7 +108,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>      FVP_GTDT_GTIMER_FLAGS,                                // UINT32  NonSecurePL2TimerFlags
>      FVP_CNT_READ_BASE_ADDRESS,                            // UINT64  CntReadBasePhysicalAddress
>      FVP_PLATFORM_TIMER_COUNT,                             // UINT32  PlatformTimerCount
> -#if (FVP_WATCHDOG_COUNT != 0)
> +#if (FVP_PLATFORM_TIMER_COUNT != 0)
>      sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32  PlatfromTimerOffset
>  #else
>      0
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>