From 949b6188d8ee5c1659b863a59503714ebd3109da Mon Sep 17 00:00:00 2001 From: Chris Kay Date: Thu, 7 Feb 2019 11:49:32 +0000 Subject: [PATCH 1/2] cmn600: Fix inappropriately-initialised global state The CMN600 module does not account for resets of the configured dependent clock (if any) when allocating context structures or reconfiguring the network. The consequence of this is that resetting the clock multiple times will eventually cause invalid register writes, a heap allocation overflow, and lost allocated memory. Change-Id: If5883fa63e085eb9b5f394b9c4196f68f3b8f1e5 Signed-off-by: Chris Kay --- module/cmn600/src/mod_cmn600.c | 134 ++++++++++++++++----------------- 1 file changed, 63 insertions(+), 71 deletions(-) diff --git a/module/cmn600/src/mod_cmn600.c b/module/cmn600/src/mod_cmn600.c index f8d01d9f3..7c1e95bbc 100644 --- a/module/cmn600/src/mod_cmn600.c +++ b/module/cmn600/src/mod_cmn600.c @@ -5,6 +5,7 @@ * SPDX-License-Identifier: BSD-3-Clause */ +#include #include #include #include @@ -53,19 +54,9 @@ struct cmn600_ctx { struct cmn600_rnsam_reg **internal_rnsam_table; struct mod_log_api *log_api; -} *ctx; - -static void process_external_node(unsigned int node_id, void *node) -{ - static unsigned int entry = 0; - assert(entry < ctx->external_rnsam_count); - - ctx->external_rnsam_table[entry].node_id = node_id; - ctx->external_rnsam_table[entry].node = node; - - entry++; -} + bool initialized; +} *ctx; static void process_node_hnf(struct cmn600_hnf_reg *hnf) { @@ -117,29 +108,6 @@ static void process_node_hnf(struct cmn600_hnf_reg *hnf) CMN600_PPU_PWPR_DYN_EN; } -static void process_internal_node(void *node) -{ - static unsigned int rnsam_entry = 0; - enum node_type node_type; - - node_type = get_node_type(node); - - switch (node_type) { - case NODE_TYPE_HN_F: - process_node_hnf(node); - break; - - case NODE_TYPE_RN_SAM: - assert(rnsam_entry < ctx->internal_rnsam_count); - ctx->internal_rnsam_table[rnsam_entry++] = node; - break; - - default: - /* Nothing to be done for other node types */ - break; - } -} - /* * Scan the CMN600 to find out: * - Number of external RN-SAM nodes @@ -228,29 +196,49 @@ static void cmn600_configure(void) unsigned int xp_idx; unsigned int node_count; unsigned int node_idx; + unsigned int xrnsam_entry; + unsigned int irnsam_entry; struct cmn600_mxp_reg *xp; - struct node_header *node; + void *node; + enum node_type node_type; const struct mod_cmn600_config *config = ctx->config; assert(get_node_type(ctx->root) == NODE_TYPE_CFG); + xrnsam_entry = 0; + irnsam_entry = 0; + /* Traverse cross points (XP) */ xp_count = get_node_child_count(ctx->root); for (xp_idx = 0; xp_idx < xp_count; xp_idx++) { - xp = get_child_node(config->base, ctx->root, xp_idx); assert(get_node_type(xp) == NODE_TYPE_XP); /* Traverse nodes */ node_count = get_node_child_count(xp); for (node_idx = 0; node_idx < node_count; node_idx++) { - node = get_child_node(config->base, xp, node_idx); + node_type = get_node_type(node); - if (is_child_external(xp, node_idx)) - process_external_node(get_child_node_id(xp, node_idx), node); - else - process_internal_node(node); + if (node_type == NODE_TYPE_RN_SAM) { + if (is_child_external(xp, node_idx)) { + unsigned int node_id = get_child_node_id(xp, node_idx); + + fwk_assert(xrnsam_entry < ctx->external_rnsam_count); + + ctx->external_rnsam_table[xrnsam_entry].node_id = node_id; + ctx->external_rnsam_table[xrnsam_entry].node = node; + + xrnsam_entry++; + } else { + fwk_assert(irnsam_entry < ctx->internal_rnsam_count); + + ctx->internal_rnsam_table[irnsam_entry] = node; + + irnsam_entry++; + } + } else if (node_type == NODE_TYPE_HN_F) + process_node_hnf(node); } } } @@ -354,39 +342,41 @@ static int cmn600_setup(void) { unsigned int rnsam_idx; - cmn600_discovery(); + if (!ctx->initialized) { + cmn600_discovery(); - /* - * Allocate resources based on the discovery - */ + /* + * Allocate resources based on the discovery + */ - /* Pointers for the internal RN-SAM nodes */ - if (ctx->internal_rnsam_count != 0) { - ctx->internal_rnsam_table = fwk_mm_calloc(ctx->internal_rnsam_count, - sizeof(*ctx->internal_rnsam_table)); - if (ctx->internal_rnsam_table == NULL) - return FWK_E_NOMEM; - } + /* Pointers for the internal RN-SAM nodes */ + if (ctx->internal_rnsam_count != 0) { + ctx->internal_rnsam_table = fwk_mm_calloc( + ctx->internal_rnsam_count, sizeof(*ctx->internal_rnsam_table)); + if (ctx->internal_rnsam_table == NULL) + return FWK_E_NOMEM; + } - /* Tuples for the external RN-RAM nodes (including their node IDs) */ - if (ctx->external_rnsam_count != 0) { - ctx->external_rnsam_table = fwk_mm_calloc(ctx->external_rnsam_count, - sizeof(*ctx->external_rnsam_table)); - if (ctx->external_rnsam_table == NULL) - return FWK_E_NOMEM; - } + /* Tuples for the external RN-RAM nodes (including their node IDs) */ + if (ctx->external_rnsam_count != 0) { + ctx->external_rnsam_table = fwk_mm_calloc( + ctx->external_rnsam_count, sizeof(*ctx->external_rnsam_table)); + if (ctx->external_rnsam_table == NULL) + return FWK_E_NOMEM; + } - /* Cache groups */ - if (ctx->hnf_count != 0) { - /* - * Allocate enough group descriptors to accommodate all expected HN-F - * nodes in the system - */ - ctx->hnf_cache_group = fwk_mm_calloc(ctx->hnf_count / - CMN600_HNF_CACHE_GROUP_ENTRIES_PER_GROUP, - sizeof(*ctx->hnf_cache_group)); - if (ctx->hnf_cache_group == NULL) - return FWK_E_NOMEM; + /* Cache groups */ + if (ctx->hnf_count != 0) { + /* + * Allocate enough group descriptors to accommodate all expected + * HN-F nodes in the system. + */ + ctx->hnf_cache_group = fwk_mm_calloc( + ctx->hnf_count / CMN600_HNF_CACHE_GROUP_ENTRIES_PER_GROUP, + sizeof(*ctx->hnf_cache_group)); + if (ctx->hnf_cache_group == NULL) + return FWK_E_NOMEM; + } } cmn600_configure(); @@ -397,6 +387,8 @@ static int cmn600_setup(void) ctx->log_api->log(MOD_LOG_GROUP_DEBUG, MOD_NAME "Done\n"); + ctx->initialized = true; + return FWK_SUCCESS; } -- GitLab From 8a6f1e46957c91241cf0e78037365969c36b5f84 Mon Sep 17 00:00:00 2001 From: Chris Kay Date: Fri, 8 Feb 2019 10:52:34 +0000 Subject: [PATCH 2/2] system_power: Fix extended PPU support This patch fixes an issue in the support for extended PPU handling in system_power, where the ext_ppu_apis field represented a pointer to an API structure, whilst being initialised like a pointer to an array of pointers to API structures. This manifested itself in a crash whenever the module tried to iterate over the array to perform power management tasks. Change-Id: I81d071d82f8ed0f677f585a83670ed519cdb8d26 Signed-off-by: Chris Kay --- module/system_power/src/mod_system_power.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/system_power/src/mod_system_power.c b/module/system_power/src/mod_system_power.c index 21f618cfa..2b7eaca1c 100644 --- a/module/system_power/src/mod_system_power.c +++ b/module/system_power/src/mod_system_power.c @@ -32,7 +32,7 @@ struct system_power_ctx { const struct mod_pd_driver_api *sys1_api; /* Pointer to array of extended PPU power domain driver APIs */ - const struct mod_pd_driver_api *ext_ppu_apis; + const struct mod_pd_driver_api **ext_ppu_apis; /* Power domain module restricted API pointer */ const struct mod_pd_restricted_api *mod_pd_restricted_api; @@ -65,7 +65,7 @@ static void ext_ppus_set_state(enum mod_pd_state state) unsigned int i; for (i = 0; i < system_power_ctx.config->ext_ppus_count; i++) { - system_power_ctx.ext_ppu_apis[i].set_state( + system_power_ctx.ext_ppu_apis[i]->set_state( system_power_ctx.config->ext_ppus[i].ppu_id, state); } -- GitLab