Re: [PATCH edk2-platforms v1] Platform/ARM: Declare FVP Generic Timer Frame #1 as Non-secure

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, May 23, 2018 at 02:51:34PM +0100, AlexeiFedorov wrote:
> From: Alexei Fedorov <alexei.fedorov@arm.com>
> 
> Programming Reference for Base FVPs describes 2 Generic Memory-mapped
> Timer frames with Non-secure access permitted to frame #1.
> However ACPI GTDT lists both timer frames #0 and #1 with
> Secure Timer flag being set in GTx Common Flags field:
>  #define FVP_GTX_COMMON_FLAGS
>    (GTX_TIMER_SAVE_CONTEXT | GTX_TIMER_SECURE)
> Declaring both timer frames as Secure prevents OS running
> in Non-secure state from accessing Generic Timer frame #1.
> 
> This patch fixes the above issue by removal of FVP_GTX_COMMON_FLAGS
> and adding two FVP_GTX_COMMON_FLAGS_S and FVP_GTX_COMMON_FLAGS_NS
> definitions used for Secure frame #0 and Non-secure frame #1
> accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

The patch looks good to me, so if Thomas agrees:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Thomas?

/
    Leif

> ---
> All the changes can be reviewed at:
> https://github.com/AlexeiFedorov/edk2-platforms/tree/262_gtdt_timer_frame_ns_v1
> 
> Notes:
>     v1:
>     - remove FVP_GTX_COMMON_FLAGS                                 [Alexei]
>     - define FVP_GTX_COMMON_FLAGS_S and FVP_GTX_COMMON_FLAGS_NS   [Alexei]
>     - use FVP_GTX_COMMON_FLAGS_S for timer frame #0 and           [Alexei]
>           FVP_GTX_COMMON_FLAGS_NS for frame #1
> 
>  Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> index 1cb4b498300cf1a08514835677154eace1dd1803..7a0b2b686ec849a8385ac5793e5d5855b9ba83ca 100644
> --- a/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> +++ b/Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc
> @@ -1,7 +1,7 @@
>  /** @file
>  *  Generic Timer Description Table (GTDT)
>  *
> -*  Copyright (c) 2012 - 2017, ARM Limited. All rights reserved.
> +*  Copyright (c) 2012 - 2018, ARM Limited. All rights reserved.
>  *  Copyright (c) 2016, Linaro Ltd. All rights reserved
>  *
>  *  This program and the accompanying materials
> @@ -61,7 +61,8 @@
>  #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_GTX_COMMON_FLAGS_S      (GTX_TIMER_SAVE_CONTEXT | GTX_TIMER_SECURE)
> +#define FVP_GTX_COMMON_FLAGS_NS     (GTX_TIMER_SAVE_CONTEXT | GTX_TIMER_NON_SECURE)
>  
>  #define FVP_SBSA_WATCHDOG_REFRESH_BASE     0x000000002a450000
>  #define FVP_SBSA_WATCHDOG_CONTROL_BASE     0x000000002a440000
> @@ -136,7 +137,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>        FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
>        0,                                                    // UINT32 GTxVirtualTimerGSIV
>        0,                                                    // UINT32 GTxVirtualTimerFlags
> -      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
> +      FVP_GTX_COMMON_FLAGS_S                                // UINT32 GTxCommonFlags
>      },
>      {
>        1,                                                    // UINT8 GTFrameNumber
> @@ -149,7 +150,7 @@ FVP_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
>        FVP_GTX_TIMER_FLAGS,                                  // UINT32 GTxPhysicalTimerFlags
>        0,                                                    // UINT32 GTxVirtualTimerGSIV
>        0,                                                    // UINT32 GTxVirtualTimerFlags
> -      FVP_GTX_COMMON_FLAGS                                  // UINT32 GTxCommonFlags
> +      FVP_GTX_COMMON_FLAGS_NS                               // UINT32 GTxCommonFlags
>      }
>    },
>  #if (FVP_WATCHDOG_COUNT != 0)
> -- 
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
>