From cae3ef992e24b9ceb2ba1cd7d2ef0c6faaf29e0a Mon Sep 17 00:00:00 2001 From: Dan Handley Date: Mon, 4 Aug 2014 16:11:15 +0100 Subject: [PATCH 1/4] Remove platform dependency in CCI-400 driver * Create cci_init() function in CCI-400 driver to allow platform to provide arguments needed by the driver (i.e. base address and cluster indices for the ACE slave interfaces). * Rename cci_(en|dis)able_coherency to cci_(en|dis)able_cluster_coherency to make it clear that the driver only enables/disables the coherency of CPU clusters and not other devices connected to the CCI-400. * Update FVP port to use new cci_init() function and remove unnecessary CCI defintions from platform_def.h. Also rename fvp_cci_setup() to fvp_cci_enable() to more clearly differentiate between CCI initialization and enabling. THIS CHANGE REQUIRES PLATFORM PORTS THAT USE THE CCI-400 DRIVER TO BE UPDATED Fixes ARM-software/tf-issues#168 Change-Id: I1946a51409b91217b92285b6375082619f607fec --- drivers/arm/cci400/cci400.c | 58 +++++++++++++++++++++++++++++---- include/drivers/arm/cci400.h | 21 ++++++++++-- plat/fvp/aarch64/fvp_common.c | 18 +++++++--- plat/fvp/bl1_fvp_setup.c | 3 +- plat/fvp/bl31_fvp_setup.c | 4 +-- plat/fvp/fvp_def.h | 7 ++-- plat/fvp/fvp_pm.c | 6 ++-- plat/fvp/fvp_private.h | 3 +- plat/fvp/include/plat_macros.S | 2 +- plat/fvp/include/platform_def.h | 11 ------- 10 files changed, 96 insertions(+), 37 deletions(-) diff --git a/drivers/arm/cci400/cci400.c b/drivers/arm/cci400/cci400.c index af10f21427..6a8737a574 100644 --- a/drivers/arm/cci400/cci400.c +++ b/drivers/arm/cci400/cci400.c @@ -28,34 +28,80 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include +#include #include #include -#include + +#define MAX_CLUSTERS 2 + +static unsigned long cci_base_addr; +static unsigned int cci_cluster_ix_to_iface[MAX_CLUSTERS]; + + +void cci_init(unsigned long cci_base, + int slave_iface3_cluster_ix, + int slave_iface4_cluster_ix) +{ + /* + * Check the passed arguments are valid. The cluster indices must be + * less than MAX_CLUSTERS, not the same as each other and at least one + * of them must be refer to a valid cluster index. + */ + assert(cci_base); + assert(slave_iface3_cluster_ix < MAX_CLUSTERS); + assert(slave_iface4_cluster_ix < MAX_CLUSTERS); + assert(slave_iface3_cluster_ix != slave_iface4_cluster_ix); + assert((slave_iface3_cluster_ix >= 0) || + (slave_iface3_cluster_ix >= 0)); + + cci_base_addr = cci_base; + if (slave_iface3_cluster_ix >= 0) + cci_cluster_ix_to_iface[slave_iface3_cluster_ix] = + SLAVE_IFACE3_OFFSET; + if (slave_iface4_cluster_ix >= 0) + cci_cluster_ix_to_iface[slave_iface4_cluster_ix] = + SLAVE_IFACE4_OFFSET; +} static inline unsigned long get_slave_iface_base(unsigned long mpidr) { - return CCI400_BASE + SLAVE_IFACE_OFFSET(CCI400_SL_IFACE_INDEX(mpidr)); + /* + * We assume the TF topology code allocates affinity instances + * consecutively from zero. + * It is a programming error if this is called without initializing + * the slave interface to use for this cluster. + */ + unsigned int cluster_id = + (mpidr >> MPIDR_AFF1_SHIFT) & MPIDR_AFFLVL_MASK; + + assert(cluster_id < MAX_CLUSTERS); + assert(cci_cluster_ix_to_iface[cluster_id] != 0); + + return cci_base_addr + cci_cluster_ix_to_iface[cluster_id]; } -void cci_enable_coherency(unsigned long mpidr) +void cci_enable_cluster_coherency(unsigned long mpidr) { + assert(cci_base_addr); /* Enable Snoops and DVM messages */ mmio_write_32(get_slave_iface_base(mpidr) + SNOOP_CTRL_REG, DVM_EN_BIT | SNOOP_EN_BIT); /* Wait for the dust to settle down */ - while (mmio_read_32(CCI400_BASE + STATUS_REG) & CHANGE_PENDING_BIT) + while (mmio_read_32(cci_base_addr + STATUS_REG) & CHANGE_PENDING_BIT) ; } -void cci_disable_coherency(unsigned long mpidr) +void cci_disable_cluster_coherency(unsigned long mpidr) { + assert(cci_base_addr); /* Disable Snoops and DVM messages */ mmio_write_32(get_slave_iface_base(mpidr) + SNOOP_CTRL_REG, ~(DVM_EN_BIT | SNOOP_EN_BIT)); /* Wait for the dust to settle down */ - while (mmio_read_32(CCI400_BASE + STATUS_REG) & CHANGE_PENDING_BIT) + while (mmio_read_32(cci_base_addr + STATUS_REG) & CHANGE_PENDING_BIT) ; } diff --git a/include/drivers/arm/cci400.h b/include/drivers/arm/cci400.h index 6246e48076..7756bdfa7d 100644 --- a/include/drivers/arm/cci400.h +++ b/include/drivers/arm/cci400.h @@ -37,7 +37,8 @@ #define SLAVE_IFACE2_OFFSET 0x3000 #define SLAVE_IFACE1_OFFSET 0x2000 #define SLAVE_IFACE0_OFFSET 0x1000 -#define SLAVE_IFACE_OFFSET(index) SLAVE_IFACE0_OFFSET + (0x1000 * index) +#define SLAVE_IFACE_OFFSET(index) SLAVE_IFACE0_OFFSET + \ + (0x1000 * (index)) /* Control and ID register offsets */ #define CTRL_OVERRIDE_REG 0x0 @@ -68,8 +69,22 @@ #ifndef __ASSEMBLY__ /* Function declarations */ -void cci_enable_coherency(unsigned long mpidr); -void cci_disable_coherency(unsigned long mpidr); + +/* + * The CCI-400 driver must be initialized with the base address of the + * CCI-400 device in the platform memory map, and the cluster indices for + * the CCI-400 slave interfaces 3 and 4 respectively. These are the fully + * coherent ACE slave interfaces of CCI-400. + * The cluster indices must either be 0 or 1, corresponding to the level 1 + * affinity instance of the mpidr representing the cluster. A negative cluster + * index indicates that no cluster is present on that slave interface. + */ +void cci_init(unsigned long cci_base, + int slave_iface3_cluster_ix, + int slave_iface4_cluster_ix); + +void cci_enable_cluster_coherency(unsigned long mpidr); +void cci_disable_cluster_coherency(unsigned long mpidr); #endif /* __ASSEMBLY__ */ #endif /* __CCI_400_H__ */ diff --git a/plat/fvp/aarch64/fvp_common.c b/plat/fvp/aarch64/fvp_common.c index 5041511355..89fd8b3ec5 100644 --- a/plat/fvp/aarch64/fvp_common.c +++ b/plat/fvp/aarch64/fvp_common.c @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -243,15 +242,26 @@ uint64_t plat_get_syscnt_freq(void) return counter_base_frequency; } -void fvp_cci_setup(void) +void fvp_cci_init(void) { /* - * Enable CCI-400 for this cluster. No need + * Initialize CCI-400 driver + */ + if (plat_config.flags & CONFIG_HAS_CCI) + cci_init(CCI400_BASE, + CCI400_SL_IFACE3_CLUSTER_IX, + CCI400_SL_IFACE4_CLUSTER_IX); +} + +void fvp_cci_enable(void) +{ + /* + * Enable CCI-400 coherency for this cluster. No need * for locks as no other cpu is active at the * moment */ if (plat_config.flags & CONFIG_HAS_CCI) - cci_enable_coherency(read_mpidr()); + cci_enable_cluster_coherency(read_mpidr()); } void fvp_gic_init(void) diff --git a/plat/fvp/bl1_fvp_setup.c b/plat/fvp/bl1_fvp_setup.c index 0cdb97a950..b1205d4352 100644 --- a/plat/fvp/bl1_fvp_setup.c +++ b/plat/fvp/bl1_fvp_setup.c @@ -110,7 +110,8 @@ void bl1_platform_setup(void) ******************************************************************************/ void bl1_plat_arch_setup(void) { - fvp_cci_setup(); + fvp_cci_init(); + fvp_cci_enable(); fvp_configure_mmu_el3(bl1_tzram_layout.total_base, bl1_tzram_layout.total_size, diff --git a/plat/fvp/bl31_fvp_setup.c b/plat/fvp/bl31_fvp_setup.c index 0693a12af6..69efc9cf03 100644 --- a/plat/fvp/bl31_fvp_setup.c +++ b/plat/fvp/bl31_fvp_setup.c @@ -230,9 +230,9 @@ void bl31_platform_setup(void) ******************************************************************************/ void bl31_plat_arch_setup(void) { + fvp_cci_init(); #if RESET_TO_BL31 - fvp_cci_setup(); - + fvp_cci_enable(); #endif fvp_configure_mmu_el3(BL31_RO_BASE, (BL31_COHERENT_RAM_LIMIT - BL31_RO_BASE), diff --git a/plat/fvp/fvp_def.h b/plat/fvp/fvp_def.h index b371ea942b..dee2a19203 100644 --- a/plat/fvp/fvp_def.h +++ b/plat/fvp/fvp_def.h @@ -184,11 +184,8 @@ * CCI-400 related constants ******************************************************************************/ #define CCI400_BASE 0x2c090000 -#define CCI400_SL_IFACE_CLUSTER0 3 -#define CCI400_SL_IFACE_CLUSTER1 4 -#define CCI400_SL_IFACE_INDEX(mpidr) (mpidr & MPIDR_CLUSTER_MASK ? \ - CCI400_SL_IFACE_CLUSTER1 : \ - CCI400_SL_IFACE_CLUSTER0) +#define CCI400_SL_IFACE3_CLUSTER_IX 0 +#define CCI400_SL_IFACE4_CLUSTER_IX 1 /******************************************************************************* * GIC-400 & interrupt handling related constants diff --git a/plat/fvp/fvp_pm.c b/plat/fvp/fvp_pm.c index 82a663b149..b7e49a278b 100644 --- a/plat/fvp/fvp_pm.c +++ b/plat/fvp/fvp_pm.c @@ -140,7 +140,7 @@ int fvp_affinst_off(unsigned long mpidr, * turned off */ if (get_plat_config()->flags & CONFIG_HAS_CCI) - cci_disable_coherency(mpidr); + cci_disable_cluster_coherency(mpidr); /* * Program the power controller to turn the @@ -215,7 +215,7 @@ int fvp_affinst_suspend(unsigned long mpidr, * turned off */ if (get_plat_config()->flags & CONFIG_HAS_CCI) - cci_disable_coherency(mpidr); + cci_disable_cluster_coherency(mpidr); /* * Program the power controller to turn the @@ -302,7 +302,7 @@ int fvp_affinst_on_finish(unsigned long mpidr, */ fvp_pwrc_write_pponr(mpidr); - fvp_cci_setup(); + fvp_cci_enable(); } break; diff --git a/plat/fvp/fvp_private.h b/plat/fvp/fvp_private.h index 054baa8891..2dcb327ff1 100644 --- a/plat/fvp/fvp_private.h +++ b/plat/fvp/fvp_private.h @@ -77,7 +77,8 @@ void fvp_configure_mmu_el3(unsigned long total_base, unsigned long); int fvp_config_setup(void); -void fvp_cci_setup(void); +void fvp_cci_init(void); +void fvp_cci_enable(void); void fvp_gic_init(void); diff --git a/plat/fvp/include/plat_macros.S b/plat/fvp/include/plat_macros.S index 727b958012..5d11d36484 100644 --- a/plat/fvp/include/plat_macros.S +++ b/plat/fvp/include/plat_macros.S @@ -30,7 +30,7 @@ #include #include #include -#include "platform_def.h" +#include "../fvp_def.h" .section .rodata.gic_reg_name, "aS" gic_regs: diff --git a/plat/fvp/include/platform_def.h b/plat/fvp/include/platform_def.h index 734f28c1cd..c87ba547ec 100644 --- a/plat/fvp/include/platform_def.h +++ b/plat/fvp/include/platform_def.h @@ -156,17 +156,6 @@ ******************************************************************************/ #define IRQ_SEC_PHY_TIMER 29 -/******************************************************************************* - * CCI-400 related constants - ******************************************************************************/ -#define CCI400_BASE 0x2c090000 -#define CCI400_SL_IFACE_CLUSTER0 3 -#define CCI400_SL_IFACE_CLUSTER1 4 -#define CCI400_SL_IFACE_INDEX(mpidr) (mpidr & MPIDR_CLUSTER_MASK ? \ - CCI400_SL_IFACE_CLUSTER1 : \ - CCI400_SL_IFACE_CLUSTER0) - - /******************************************************************************* * Declarations and constants to access the mailboxes safely. Each mailbox is * aligned on the biggest cache line size in the platform. This is known only From 6d16ce0bfee7101a125fc5545ca12e19c3d47cd1 Mon Sep 17 00:00:00 2001 From: Dan Handley Date: Mon, 4 Aug 2014 18:31:43 +0100 Subject: [PATCH 2/4] Remove redundant io_init() function The intent of io_init() was to allow platform ports to provide a data object (io_plat_data_t) to the IO storage framework to allocate into. The abstraction was incomplete because io_plat_data_t uses a platform defined constant and the IO storage framework internally allocates other arrays using platform defined constants. This change simplifies the implementation by instantiating the supporting objects in the IO storage framework itself. There is now no need for the platform to call io_init(). The FVP port has been updated accordingly. THIS CHANGE REQUIRES ALL PLATFORM PORTS THAT USE THE IO STORAGE FRAMEWORK TO BE UDPATED. Change-Id: Ib48ac334de9e538064734334c773f8b43df3a7dc --- docs/porting-guide.md | 15 +++++++++++++++ include/drivers/io_driver.h | 14 +------------- lib/io_storage.c | 29 ++++++++--------------------- plat/fvp/fvp_io_storage.c | 6 +----- 4 files changed, 25 insertions(+), 39 deletions(-) diff --git a/docs/porting-guide.md b/docs/porting-guide.md index eb3b86d968..3070775ae4 100644 --- a/docs/porting-guide.md +++ b/docs/porting-guide.md @@ -218,6 +218,21 @@ be defined as well: the secure memory identified by `TSP_SEC_MEM_BASE` and `TSP_SEC_MEM_SIZE` constants. +If the platform port uses the IO storage framework, the following constants +must also be defined: + +* **#define : MAX_IO_DEVICES** + + Defines the maximum number of registered IO devices. Attempting to register + more devices than this value using `io_register_device()` will fail with + IO_RESOURCES_EXHAUSTED. + +* **#define : MAX_IO_HANDLES** + + Defines the maximum number of open IO handles. Attempting to open more IO + entities than this value using `io_open()` will fail with + IO_RESOURCES_EXHAUSTED. + The following constants are optional. They should be defined when the platform memory layout implies some image overlaying like on FVP. diff --git a/include/drivers/io_driver.h b/include/drivers/io_driver.h index 867abbfa32..adb38b0ff5 100644 --- a/include/drivers/io_driver.h +++ b/include/drivers/io_driver.h @@ -32,7 +32,6 @@ #define __IO_DRIVER_H__ #include -#include /* For MAX_IO_DEVICES */ #include @@ -76,20 +75,9 @@ typedef struct io_dev_funcs { } io_dev_funcs_t; -/* IO platform data - used to track devices registered for a specific - * platform */ -typedef struct io_plat_data { - const io_dev_info_t *devices[MAX_IO_DEVICES]; - unsigned int dev_count; -} io_plat_data_t; - - /* Operations intended to be performed during platform initialisation */ -/* Initialise the IO layer */ -void io_init(io_plat_data_t *data); - -/* Register a device driver */ +/* Register an IO device */ int io_register_device(const io_dev_info_t *dev_info); #endif /* __IO_DRIVER_H__ */ diff --git a/lib/io_storage.c b/lib/io_storage.c index 204310a42a..a3a8186d0f 100644 --- a/lib/io_storage.c +++ b/lib/io_storage.c @@ -31,13 +31,10 @@ #include #include #include +#include #include -#define MAX_DEVICES(plat_data) \ - (sizeof((plat_data)->devices)/sizeof((plat_data)->devices[0])) - - /* Storage for a fixed maximum number of IO entities, definable by platform */ static io_entity_t entity_pool[MAX_IO_HANDLES]; @@ -48,9 +45,11 @@ static io_entity_t *entity_map[MAX_IO_HANDLES]; /* Track number of allocated entities */ static unsigned int entity_count; +/* Array of fixed maximum of registered devices, definable by platform */ +static const io_dev_info_t *devices[MAX_IO_DEVICES]; -/* Used to keep a reference to platform-specific data */ -static io_plat_data_t *platform_data; +/* Number of currently registered devices */ +static unsigned int dev_count; #if DEBUG /* Extra validation functions only used in debug builds */ @@ -167,27 +166,15 @@ static int free_entity(const io_entity_t *entity) /* Exported API */ - -/* Initialise the IO layer */ -void io_init(io_plat_data_t *data) -{ - assert(data != NULL); - platform_data = data; -} - - /* Register a device driver */ int io_register_device(const io_dev_info_t *dev_info) { int result = IO_FAIL; assert(dev_info != NULL); - assert(platform_data != NULL); - unsigned int dev_count = platform_data->dev_count; - - if (dev_count < MAX_DEVICES(platform_data)) { - platform_data->devices[dev_count] = dev_info; - platform_data->dev_count++; + if (dev_count < MAX_IO_DEVICES) { + devices[dev_count] = dev_info; + dev_count++; result = IO_SUCCESS; } else { result = IO_RESOURCES_EXHAUSTED; diff --git a/plat/fvp/fvp_io_storage.c b/plat/fvp/fvp_io_storage.c index 1f695a6dbf..b4a04f19c8 100644 --- a/plat/fvp/fvp_io_storage.c +++ b/plat/fvp/fvp_io_storage.c @@ -35,12 +35,11 @@ #include #include #include +#include #include /* For FOPEN_MODE_... */ #include -#include "fvp_def.h" /* IO devices */ -static io_plat_data_t io_data; static const io_dev_connector_t *sh_dev_con; static uintptr_t sh_dev_spec; static uintptr_t sh_init_params; @@ -172,9 +171,6 @@ void fvp_io_setup (void) { int io_result = IO_FAIL; - /* Initialise the IO layer */ - io_init(&io_data); - /* Register the IO devices on this platform */ io_result = register_io_dev_sh(&sh_dev_con); assert(io_result == IO_SUCCESS); From 935db693285d6b22b781571e9dec748da274ffe1 Mon Sep 17 00:00:00 2001 From: Dan Handley Date: Tue, 12 Aug 2014 14:20:28 +0100 Subject: [PATCH 3/4] Move IO storage source to drivers directory Move the remaining IO storage source file (io_storage.c) from the lib to the drivers directory. This requires that platform ports explicitly add this file to the list of source files. Also move the IO header files to a new sub-directory, include/io. Change-Id: I862b1252a796b3bcac0d93e50b11e7fb2ded93d6 --- Makefile | 2 +- {lib => drivers/io}/io_storage.c | 0 include/drivers/{ => io}/io_driver.h | 0 include/drivers/{ => io}/io_fip.h | 0 include/drivers/{ => io}/io_memmap.h | 0 include/drivers/{ => io}/io_semihosting.h | 0 include/{lib => drivers/io}/io_storage.h | 0 plat/fvp/platform.mk | 1 + 8 files changed, 2 insertions(+), 1 deletion(-) rename {lib => drivers/io}/io_storage.c (100%) rename include/drivers/{ => io}/io_driver.h (100%) rename include/drivers/{ => io}/io_fip.h (100%) rename include/drivers/{ => io}/io_memmap.h (100%) rename include/drivers/{ => io}/io_semihosting.h (100%) rename include/{lib => drivers/io}/io_storage.h (100%) diff --git a/Makefile b/Makefile index fcbf773a08..89e882a6fd 100644 --- a/Makefile +++ b/Makefile @@ -102,7 +102,6 @@ BL_COMMON_SOURCES := common/bl_common.c \ lib/aarch64/misc_helpers.S \ lib/aarch64/xlat_helpers.c \ lib/stdlib/std.c \ - lib/io_storage.c \ plat/common/aarch64/platform_helpers.S BUILD_BASE := ./build @@ -180,6 +179,7 @@ INCLUDES += -Iinclude/bl31 \ -Iinclude/common \ -Iinclude/drivers \ -Iinclude/drivers/arm \ + -Iinclude/drivers/io \ -Iinclude/lib \ -Iinclude/lib/aarch64 \ -Iinclude/plat/common \ diff --git a/lib/io_storage.c b/drivers/io/io_storage.c similarity index 100% rename from lib/io_storage.c rename to drivers/io/io_storage.c diff --git a/include/drivers/io_driver.h b/include/drivers/io/io_driver.h similarity index 100% rename from include/drivers/io_driver.h rename to include/drivers/io/io_driver.h diff --git a/include/drivers/io_fip.h b/include/drivers/io/io_fip.h similarity index 100% rename from include/drivers/io_fip.h rename to include/drivers/io/io_fip.h diff --git a/include/drivers/io_memmap.h b/include/drivers/io/io_memmap.h similarity index 100% rename from include/drivers/io_memmap.h rename to include/drivers/io/io_memmap.h diff --git a/include/drivers/io_semihosting.h b/include/drivers/io/io_semihosting.h similarity index 100% rename from include/drivers/io_semihosting.h rename to include/drivers/io/io_semihosting.h diff --git a/include/lib/io_storage.h b/include/drivers/io/io_storage.h similarity index 100% rename from include/lib/io_storage.h rename to include/drivers/io/io_storage.h diff --git a/plat/fvp/platform.mk b/plat/fvp/platform.mk index 8a33a608b7..ffed7e4e9a 100644 --- a/plat/fvp/platform.mk +++ b/plat/fvp/platform.mk @@ -66,6 +66,7 @@ PLAT_BL_COMMON_SOURCES := drivers/arm/pl011/pl011_console.S \ drivers/io/io_fip.c \ drivers/io/io_memmap.c \ drivers/io/io_semihosting.c \ + drivers/io/io_storage.c \ lib/aarch64/xlat_tables.c \ lib/semihosting/semihosting.c \ lib/semihosting/aarch64/semihosting_call.S \ From 3279f6251ebead329e4b0af2e42a7eae157bba52 Mon Sep 17 00:00:00 2001 From: Dan Handley Date: Mon, 4 Aug 2014 19:53:05 +0100 Subject: [PATCH 4/4] Simplify interface to TZC-400 driver The TZC-400 driver previously allowed the possibility of multiple controller instances to be present in the same executable. This was unnecessary since there will only ever be one instance. This change simplifies the tzc_init() function to only take the base address argument needed by implementation, conforming to the driver initialization model of other drivers. It also hides some of the implementation details that were previously exposed by the API. The FVP port has been updated accordingly. THIS CHANGE REQUIRES ALL PLATFORM PORTS THAT USE THE TZC-400 DRIVER TO BE UPDATED Fixes ARM-software/tf-issues#181 Change-Id: I7b721edf947064989958d8f457d6462d92e742c8 --- drivers/arm/tzc400/tzc400.c | 137 ++++++++++++++++++++++------------- include/drivers/arm/tzc400.h | 30 +++----- plat/fvp/fvp_def.h | 3 - plat/fvp/fvp_security.c | 18 ++--- 4 files changed, 103 insertions(+), 85 deletions(-) diff --git a/drivers/arm/tzc400/tzc400.c b/drivers/arm/tzc400/tzc400.c index 715ea6c0b7..3ab1f3189a 100644 --- a/drivers/arm/tzc400/tzc400.c +++ b/drivers/arm/tzc400/tzc400.c @@ -34,54 +34,88 @@ #include #include -static uint32_t tzc_read_build_config(uint64_t base) +/* + * Implementation defined values used to validate inputs later. + * Filters : max of 4 ; 0 to 3 + * Regions : max of 9 ; 0 to 8 + * Address width : Values between 32 to 64 + */ +typedef struct tzc_instance { + uint64_t base; + uint8_t addr_width; + uint8_t num_filters; + uint8_t num_regions; +} tzc_instance_t; + +tzc_instance_t tzc; + + +static inline uint32_t tzc_read_build_config(uint64_t base) { return mmio_read_32(base + BUILD_CONFIG_OFF); } -static uint32_t tzc_read_gate_keeper(uint64_t base) +static inline uint32_t tzc_read_gate_keeper(uint64_t base) { return mmio_read_32(base + GATE_KEEPER_OFF); } -static void tzc_write_gate_keeper(uint64_t base, uint32_t val) +static inline void tzc_write_gate_keeper(uint64_t base, uint32_t val) { mmio_write_32(base + GATE_KEEPER_OFF, val); } -static void tzc_write_action(uint64_t base, tzc_action_t action) +static inline void tzc_write_action(uint64_t base, tzc_action_t action) { mmio_write_32(base + ACTION_OFF, action); } -static void tzc_write_region_base_low(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_base_low(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_BASE_LOW_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_BASE_LOW_OFF + + REGION_NUM_OFF(region), val); } -static void tzc_write_region_base_high(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_base_high(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_BASE_HIGH_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_BASE_HIGH_OFF + + REGION_NUM_OFF(region), val); } -static void tzc_write_region_top_low(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_top_low(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_TOP_LOW_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_TOP_LOW_OFF + + REGION_NUM_OFF(region), val); } -static void tzc_write_region_top_high(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_top_high(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_TOP_HIGH_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_TOP_HIGH_OFF + + REGION_NUM_OFF(region), val); } -static void tzc_write_region_attributes(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_attributes(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_ATTRIBUTES_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_ATTRIBUTES_OFF + + REGION_NUM_OFF(region), val); } -static void tzc_write_region_id_access(uint64_t base, uint32_t region, uint32_t val) +static inline void tzc_write_region_id_access(uint64_t base, + uint32_t region, + uint32_t val) { - mmio_write_32(base + REGION_ID_ACCESS_OFF + REGION_NUM_OFF(region), val); + mmio_write_32(base + REGION_ID_ACCESS_OFF + + REGION_NUM_OFF(region), val); } static uint32_t tzc_read_component_id(uint64_t base) @@ -130,29 +164,30 @@ static void tzc_set_gate_keeper(uint64_t base, uint8_t filter, uint32_t val) } -void tzc_init(tzc_instance_t *controller) +void tzc_init(uint64_t base) { uint32_t tzc_id, tzc_build; - assert(controller != NULL); + assert(base); + tzc.base = base; /* * We expect to see a tzc400. Check component ID. The TZC-400 TRM shows * component ID is expected to be "0xB105F00D". */ - tzc_id = tzc_read_component_id(controller->base); + tzc_id = tzc_read_component_id(tzc.base); if (tzc_id != TZC400_COMPONENT_ID) { ERROR("TZC : Wrong device ID (0x%x).\n", tzc_id); panic(); } /* Save values we will use later. */ - tzc_build = tzc_read_build_config(controller->base); - controller->num_filters = ((tzc_build >> BUILD_CONFIG_NF_SHIFT) & + tzc_build = tzc_read_build_config(tzc.base); + tzc.num_filters = ((tzc_build >> BUILD_CONFIG_NF_SHIFT) & BUILD_CONFIG_NF_MASK) + 1; - controller->addr_width = ((tzc_build >> BUILD_CONFIG_AW_SHIFT) & + tzc.addr_width = ((tzc_build >> BUILD_CONFIG_AW_SHIFT) & BUILD_CONFIG_AW_MASK) + 1; - controller->num_regions = ((tzc_build >> BUILD_CONFIG_NR_SHIFT) & + tzc.num_regions = ((tzc_build >> BUILD_CONFIG_NR_SHIFT) & BUILD_CONFIG_NR_MASK) + 1; } @@ -166,29 +201,25 @@ void tzc_init(tzc_instance_t *controller) * this cannot be changed. It is, however, possible to change some region 0 * permissions. */ -void tzc_configure_region(const tzc_instance_t *controller, - uint32_t filters, +void tzc_configure_region(uint32_t filters, uint8_t region, uint64_t region_base, uint64_t region_top, tzc_region_attributes_t sec_attr, uint32_t ns_device_access) { - uint64_t max_addr; - - assert(controller != NULL); + assert(tzc.base); /* Do range checks on filters and regions. */ - assert(((filters >> controller->num_filters) == 0) && - (region < controller->num_regions)); + assert(((filters >> tzc.num_filters) == 0) && + (region < tzc.num_regions)); /* * Do address range check based on TZC configuration. A 64bit address is * the max and expected case. */ - max_addr = UINT64_MAX >> (64 - controller->addr_width); - if ((region_top > max_addr) || (region_base >= region_top)) - assert(0); + assert(((region_top <= (UINT64_MAX >> (64 - tzc.addr_width))) && + (region_base < region_top))); /* region_base and (region_top + 1) must be 4KB aligned */ assert(((region_base | (region_top + 1)) & (4096 - 1)) == 0); @@ -200,46 +231,50 @@ void tzc_configure_region(const tzc_instance_t *controller, * All the address registers are 32 bits wide and have a LOW and HIGH * component used to construct a up to a 64bit address. */ - tzc_write_region_base_low(controller->base, region, (uint32_t)(region_base)); - tzc_write_region_base_high(controller->base, region, (uint32_t)(region_base >> 32)); + tzc_write_region_base_low(tzc.base, region, + (uint32_t)(region_base)); + tzc_write_region_base_high(tzc.base, region, + (uint32_t)(region_base >> 32)); - tzc_write_region_top_low(controller->base, region, (uint32_t)(region_top)); - tzc_write_region_top_high(controller->base, region, (uint32_t)(region_top >> 32)); + tzc_write_region_top_low(tzc.base, region, + (uint32_t)(region_top)); + tzc_write_region_top_high(tzc.base, region, + (uint32_t)(region_top >> 32)); /* Assign the region to a filter and set secure attributes */ - tzc_write_region_attributes(controller->base, region, + tzc_write_region_attributes(tzc.base, region, (sec_attr << REGION_ATTRIBUTES_SEC_SHIFT) | filters); /* * Specify which non-secure devices have permission to access this * region. */ - tzc_write_region_id_access(controller->base, region, ns_device_access); + tzc_write_region_id_access(tzc.base, region, ns_device_access); } -void tzc_set_action(const tzc_instance_t *controller, tzc_action_t action) +void tzc_set_action(tzc_action_t action) { - assert(controller != NULL); + assert(tzc.base); /* * - Currently no handler is provided to trap an error via interrupt * or exception. * - The interrupt action has not been tested. */ - tzc_write_action(controller->base, action); + tzc_write_action(tzc.base, action); } -void tzc_enable_filters(const tzc_instance_t *controller) +void tzc_enable_filters(void) { uint32_t state; uint32_t filter; - assert(controller != NULL); + assert(tzc.base); - for (filter = 0; filter < controller->num_filters; filter++) { - state = tzc_get_gate_keeper(controller->base, filter); + for (filter = 0; filter < tzc.num_filters; filter++) { + state = tzc_get_gate_keeper(tzc.base, filter); if (state) { /* The TZC filter is already configured. Changing the * programmer's view in an active system can cause @@ -252,21 +287,21 @@ void tzc_enable_filters(const tzc_instance_t *controller) filter); panic(); } - tzc_set_gate_keeper(controller->base, filter, 1); + tzc_set_gate_keeper(tzc.base, filter, 1); } } -void tzc_disable_filters(const tzc_instance_t *controller) +void tzc_disable_filters(void) { uint32_t filter; - assert(controller != NULL); + assert(tzc.base); /* * We don't do the same state check as above as the Gatekeepers are * disabled after reset. */ - for (filter = 0; filter < controller->num_filters; filter++) - tzc_set_gate_keeper(controller->base, filter, 0); + for (filter = 0; filter < tzc.num_filters; filter++) + tzc_set_gate_keeper(tzc.base, filter, 0); } diff --git a/include/drivers/arm/tzc400.h b/include/drivers/arm/tzc400.h index 03fce546b0..ff8b49ae90 100644 --- a/include/drivers/arm/tzc400.h +++ b/include/drivers/arm/tzc400.h @@ -182,27 +182,17 @@ typedef enum { TZC_REGION_S_RDWR = (TZC_REGION_S_RD | TZC_REGION_S_WR) } tzc_region_attributes_t; -/* - * Implementation defined values used to validate inputs later. - * Filters : max of 4 ; 0 to 3 - * Regions : max of 9 ; 0 to 8 - * Address width : Values between 32 to 64 - */ -typedef struct tzc_instance { - uint64_t base; - uint32_t aid_width; - uint8_t addr_width; - uint8_t num_filters; - uint8_t num_regions; -} tzc_instance_t ; -void tzc_init(tzc_instance_t *controller); -void tzc_configure_region(const tzc_instance_t *controller, uint32_t filters, - uint8_t region, uint64_t region_base, uint64_t region_top, - tzc_region_attributes_t sec_attr, uint32_t ns_device_access); -void tzc_enable_filters(const tzc_instance_t *controller); -void tzc_disable_filters(const tzc_instance_t *controller); -void tzc_set_action(const tzc_instance_t *controller, tzc_action_t action); +void tzc_init(uint64_t base); +void tzc_configure_region(uint32_t filters, + uint8_t region, + uint64_t region_base, + uint64_t region_top, + tzc_region_attributes_t sec_attr, + uint32_t ns_device_access); +void tzc_enable_filters(void); +void tzc_disable_filters(void); +void tzc_set_action(tzc_action_t action); #endif /* __TZC400__ */ diff --git a/plat/fvp/fvp_def.h b/plat/fvp/fvp_def.h index dee2a19203..b2c26fc731 100644 --- a/plat/fvp/fvp_def.h +++ b/plat/fvp/fvp_def.h @@ -239,9 +239,6 @@ * The NSAIDs for this platform as used to program the TZC400. */ -/* The FVP has 4 bits of NSAIDs. Used with TZC FAIL_ID (ACE Lite ID width) */ -#define FVP_AID_WIDTH 4 - /* NSAIDs used by devices in TZC filter 0 on FVP */ #define FVP_NSAID_DEFAULT 0 #define FVP_NSAID_PCI 1 diff --git a/plat/fvp/fvp_security.c b/plat/fvp/fvp_security.c index 0adbbc5ddc..06ab5754d7 100644 --- a/plat/fvp/fvp_security.c +++ b/plat/fvp/fvp_security.c @@ -46,8 +46,6 @@ */ void fvp_security_setup(void) { - tzc_instance_t controller; - /* * The Base FVP has a TrustZone address space controller, the Foundation * FVP does not. Trying to program the device on the foundation FVP will @@ -71,9 +69,7 @@ void fvp_security_setup(void) * - Provide base address of device on platform. * - Provide width of ACE-Lite IDs on platform. */ - controller.base = TZC400_BASE; - controller.aid_width = FVP_AID_WIDTH; - tzc_init(&controller); + tzc_init(TZC400_BASE); /* * Currently only filters 0 and 2 are connected on Base FVP. @@ -87,7 +83,7 @@ void fvp_security_setup(void) */ /* Disable all filters before programming. */ - tzc_disable_filters(&controller); + tzc_disable_filters(); /* * Allow only non-secure access to all DRAM to supported devices. @@ -101,7 +97,7 @@ void fvp_security_setup(void) */ /* Set to cover the first block of DRAM */ - tzc_configure_region(&controller, FILTER_SHIFT(0), 1, + tzc_configure_region(FILTER_SHIFT(0), 1, DRAM1_BASE, DRAM1_END - DRAM1_SEC_SIZE, TZC_REGION_S_NONE, TZC_REGION_ACCESS_RDWR(FVP_NSAID_DEFAULT) | @@ -111,13 +107,13 @@ void fvp_security_setup(void) TZC_REGION_ACCESS_RDWR(FVP_NSAID_VIRTIO_OLD)); /* Set to cover the secure reserved region */ - tzc_configure_region(&controller, FILTER_SHIFT(0), 3, + tzc_configure_region(FILTER_SHIFT(0), 3, (DRAM1_END - DRAM1_SEC_SIZE) + 1 , DRAM1_END, TZC_REGION_S_RDWR, 0x0); /* Set to cover the second block of DRAM */ - tzc_configure_region(&controller, FILTER_SHIFT(0), 2, + tzc_configure_region(FILTER_SHIFT(0), 2, DRAM2_BASE, DRAM2_END, TZC_REGION_S_NONE, TZC_REGION_ACCESS_RDWR(FVP_NSAID_DEFAULT) | TZC_REGION_ACCESS_RDWR(FVP_NSAID_PCI) | @@ -130,8 +126,8 @@ void fvp_security_setup(void) * options we have are for access errors to occur quietly or to * cause an exception. We choose to cause an exception. */ - tzc_set_action(&controller, TZC_ACTION_ERR); + tzc_set_action(TZC_ACTION_ERR); /* Enable filters. */ - tzc_enable_filters(&controller); + tzc_enable_filters(); }