PSCI: Lookup list of parent nodes to lock only once

When acquiring or releasing the power domain locks for a given CPU the
parent nodes are looked up by walking the up the PD tree list on both the
acquire and release path, only one set of lookups is needed. Fetch the
parent nodes first and pass this list into both the acquire and release
functions to avoid the double lookup.

This also allows us to not have to do this lookup after coherency has
been exited during the core power down sequence. The shared struct
psci_cpu_pd_nodes is not placed in coherent memory like is done
for psci_non_cpu_pd_nodes and doing so would negatively affect
performance. With this patch we remove the need to have it in coherent
memory by moving the access out of psci_release_pwr_domain_locks().
Signed-off-by: Andrew F. Davis <afd@ti.com>
Change-Id: I7b9cfa9d31148dea0f5e21091c8b45ef7fe4c4ab

lib/psci/psci_common.c

@@ -568,35 +568,35 @@ unsigned int psci_find_target_suspend_lvl(const psci_power_state_t *state_info)
 }
 
 /*******************************************************************************
- * This function is passed a cpu_index and the highest level in the topology
- * tree that the operation should be applied to. It picks up locks in order of
- * increasing power domain level in the range specified.
+ * This function is passed the highest level in the topology tree that the
+ * operation should be applied to and a list of node indexes. It picks up locks
+ * from the node index list in order of increasing power domain level in the
+ * range specified.
  ******************************************************************************/
-void psci_acquire_pwr_domain_locks(unsigned int end_pwrlvl, int cpu_idx)
+void psci_acquire_pwr_domain_locks(unsigned int end_pwrlvl,
+				   const unsigned int *parent_nodes)
 {
-	unsigned int parent_idx = psci_cpu_pd_nodes[cpu_idx].parent_node;
+	unsigned int parent_idx;
 	unsigned int level;
 
 	/* No locking required for level 0. Hence start locking from level 1 */
 	for (level = PSCI_CPU_PWR_LVL + 1U; level <= end_pwrlvl; level++) {
+		parent_idx = parent_nodes[level - 1U];
 		psci_lock_get(&psci_non_cpu_pd_nodes[parent_idx]);
-		parent_idx = psci_non_cpu_pd_nodes[parent_idx].parent_node;
 	}
 }
 
 /*******************************************************************************
- * This function is passed a cpu_index and the highest level in the topology
- * tree that the operation should be applied to. It releases the locks in order
- * of decreasing power domain level in the range specified.
+ * This function is passed the highest level in the topology tree that the
+ * operation should be applied to and a list of node indexes. It releases the
+ * locks in order of decreasing power domain level in the range specified.
  ******************************************************************************/
-void psci_release_pwr_domain_locks(unsigned int end_pwrlvl, int cpu_idx)
+void psci_release_pwr_domain_locks(unsigned int end_pwrlvl,
+				   const unsigned int *parent_nodes)
 {
-	unsigned int parent_idx, parent_nodes[PLAT_MAX_PWR_LVL] = {0};
+	unsigned int parent_idx;
 	unsigned int level;
 
-	/* Get the parent nodes */
-	psci_get_parent_pwr_domain_nodes(cpu_idx, end_pwrlvl, parent_nodes);
-
 	/* Unlock top down. No unlocking required for level 0. */
 	for (level = end_pwrlvl; level >= PSCI_CPU_PWR_LVL + 1U; level--) {
 		parent_idx = parent_nodes[level - 1U];
@@ -764,6 +764,7 @@ void psci_warmboot_entrypoint(void)
 {
 	unsigned int end_pwrlvl;
 	int cpu_idx = (int) plat_my_core_pos();
+	unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0};
 	psci_power_state_t state_info = { {PSCI_LOCAL_STATE_RUN} };
 
 	/*
@@ -781,12 +782,15 @@ void psci_warmboot_entrypoint(void)
 	 */
 	end_pwrlvl = get_power_on_target_pwrlvl();
 
+	/* Get the parent nodes */
+	psci_get_parent_pwr_domain_nodes(cpu_idx, end_pwrlvl, parent_nodes);
+
 	/*
 	 * This function acquires the lock corresponding to each power level so
 	 * that by the time all locks are taken, the system topology is snapshot
 	 * and state management can be done safely.
 	 */
-	psci_acquire_pwr_domain_locks(end_pwrlvl, cpu_idx);
+	psci_acquire_pwr_domain_locks(end_pwrlvl, parent_nodes);
 
 	psci_get_target_local_pwr_states(end_pwrlvl, &state_info);
 
@@ -831,7 +835,7 @@ void psci_warmboot_entrypoint(void)
 	 * This loop releases the lock corresponding to each power level
 	 * in the reverse order to which they were acquired.
 	 */
-	psci_release_pwr_domain_locks(end_pwrlvl, cpu_idx);
+	psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes);
 }
 
 /*******************************************************************************

lib/psci/psci_off.c

@@ -45,6 +45,7 @@ int psci_do_cpu_off(unsigned int end_pwrlvl)
 	int rc = PSCI_E_SUCCESS;
 	int idx = (int) plat_my_core_pos();
 	psci_power_state_t state_info;
+	unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0};
 
 	/*
 	 * This function must only be called on platforms where the
@@ -55,12 +56,21 @@ int psci_do_cpu_off(unsigned int end_pwrlvl)
 	/* Construct the psci_power_state for CPU_OFF */
 	psci_set_power_off_state(&state_info);
 
+	/*
+	 * Get the parent nodes here, this is important to do before we
+	 * initiate the power down sequence as after that point the core may
+	 * have exited coherency and its cache may be disabled, any access to
+	 * shared memory after that (such as the parent node lookup in
+	 * psci_cpu_pd_nodes) can cause coherency issues on some platforms.
+	 */
+	psci_get_parent_pwr_domain_nodes(idx, end_pwrlvl, parent_nodes);
+
 	/*
 	 * This function acquires the lock corresponding to each power
 	 * level so that by the time all locks are taken, the system topology
 	 * is snapshot and state management can be done safely.
 	 */
-	psci_acquire_pwr_domain_locks(end_pwrlvl, idx);
+	psci_acquire_pwr_domain_locks(end_pwrlvl, parent_nodes);
 
 	/*
 	 * Call the cpu off handler registered by the Secure Payload Dispatcher
@@ -122,7 +132,7 @@ exit:
 	 * Release the locks corresponding to each power level in the
 	 * reverse order to which they were acquired.
 	 */
-	psci_release_pwr_domain_locks(end_pwrlvl, idx);
+	psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes);
 
 	/*
 	 * Check if all actions needed to safely power down this cpu have

lib/psci/psci_private.h

@@ -274,8 +274,10 @@ void psci_get_parent_pwr_domain_nodes(int cpu_idx,
 				      unsigned int *node_index);
 void psci_do_state_coordination(unsigned int end_pwrlvl,
 				psci_power_state_t *state_info);
-void psci_acquire_pwr_domain_locks(unsigned int end_pwrlvl, int cpu_idx);
-void psci_release_pwr_domain_locks(unsigned int end_pwrlvl, int cpu_idx);
+void psci_acquire_pwr_domain_locks(unsigned int end_pwrlvl,
+				   const unsigned int *parent_nodes);
+void psci_release_pwr_domain_locks(unsigned int end_pwrlvl,
+				   const unsigned int *parent_nodes);
 int psci_validate_suspend_req(const psci_power_state_t *state_info,
 			      unsigned int is_power_down_state);
 unsigned int psci_find_max_off_lvl(const psci_power_state_t *state_info);

lib/psci/psci_suspend.c

@@ -28,10 +28,13 @@
 static void psci_suspend_to_standby_finisher(int cpu_idx,
 					     unsigned int end_pwrlvl)
 {
+	unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0};
 	psci_power_state_t state_info;
 
-	psci_acquire_pwr_domain_locks(end_pwrlvl,
-				cpu_idx);
+	/* Get the parent nodes */
+	psci_get_parent_pwr_domain_nodes(cpu_idx, end_pwrlvl, parent_nodes);
+
+	psci_acquire_pwr_domain_locks(end_pwrlvl, parent_nodes);
 
 	/*
 	 * Find out which retention states this CPU has exited from until the
@@ -57,8 +60,7 @@ static void psci_suspend_to_standby_finisher(int cpu_idx,
 	 */
 	psci_set_pwr_domains_to_run(end_pwrlvl);
 
-	psci_release_pwr_domain_locks(end_pwrlvl,
-				cpu_idx);
+	psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes);
 }
 
 /*******************************************************************************
@@ -156,6 +158,7 @@ void psci_cpu_suspend_start(const entry_point_info_t *ep,
 {
 	int skip_wfi = 0;
 	int idx = (int) plat_my_core_pos();
+	unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0};
 
 	/*
 	 * This function must only be called on platforms where the
@@ -164,13 +167,15 @@ void psci_cpu_suspend_start(const entry_point_info_t *ep,
 	assert((psci_plat_pm_ops->pwr_domain_suspend != NULL) &&
 	       (psci_plat_pm_ops->pwr_domain_suspend_finish != NULL));
 
+	/* Get the parent nodes */
+	psci_get_parent_pwr_domain_nodes(idx, end_pwrlvl, parent_nodes);
+
 	/*
 	 * This function acquires the lock corresponding to each power
 	 * level so that by the time all locks are taken, the system topology
 	 * is snapshot and state management can be done safely.
 	 */
-	psci_acquire_pwr_domain_locks(end_pwrlvl,
-				      idx);
+	psci_acquire_pwr_domain_locks(end_pwrlvl, parent_nodes);
 
 	/*
 	 * We check if there are any pending interrupts after the delay
@@ -214,8 +219,8 @@ exit:
 	 * Release the locks corresponding to each power level in the
 	 * reverse order to which they were acquired.
 	 */
-	psci_release_pwr_domain_locks(end_pwrlvl,
-				  idx);
+	psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes);
+
 	if (skip_wfi == 1)
 		return;