[PATCH 03/16] ARM: b.L: introduce helpers for platform coherency exit/setup
Will Deacon
will.deacon at arm.com
Thu Jan 10 18:13:59 EST 2013
On Thu, Jan 10, 2013 at 12:20:38AM +0000, Nicolas Pitre wrote:
> From: Dave Martin <dave.martin at linaro.org>
>
> This provides helper methods to coordinate between CPUs coming down
> and CPUs going up, as well as documentation on the used algorithms,
> so that cluster teardown and setup
> operations are not done for a cluster simultaneously.
[...]
> +int __init bL_cluster_sync_init(void (*power_up_setup)(void))
> +{
> + unsigned int i, j, mpidr, this_cluster;
> +
> + BUILD_BUG_ON(BL_SYNC_CLUSTER_SIZE * BL_NR_CLUSTERS != sizeof bL_sync);
> + BUG_ON((unsigned long)&bL_sync & (__CACHE_WRITEBACK_GRANULE - 1));
> +
> + /*
> + * Set initial CPU and cluster states.
> + * Only one cluster is assumed to be active at this point.
> + */
> + for (i = 0; i < BL_NR_CLUSTERS; i++) {
> + bL_sync.clusters[i].cluster = CLUSTER_DOWN;
> + bL_sync.clusters[i].inbound = INBOUND_NOT_COMING_UP;
> + for (j = 0; j < BL_CPUS_PER_CLUSTER; j++)
> + bL_sync.clusters[i].cpus[j].cpu = CPU_DOWN;
> + }
> + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
We have a helper for this...
> + this_cluster = (mpidr >> 8) & 0xf;
... and also this, thanks to Lorenzo's recent patches.
> + for_each_online_cpu(i)
> + bL_sync.clusters[this_cluster].cpus[i].cpu = CPU_UP;
> + bL_sync.clusters[this_cluster].cluster = CLUSTER_UP;
> + sync_mem(&bL_sync);
> +
> + if (power_up_setup) {
> + bL_power_up_setup_phys = virt_to_phys(power_up_setup);
> + sync_mem(&bL_power_up_setup_phys);
> + }
> +
> + return 0;
> +}
> diff --git a/arch/arm/common/bL_head.S b/arch/arm/common/bL_head.S
> index 9d351f2b4c..f7a64ac127 100644
> --- a/arch/arm/common/bL_head.S
> +++ b/arch/arm/common/bL_head.S
> @@ -7,11 +7,19 @@
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> + *
> + *
> + * Refer to Documentation/arm/big.LITTLE/cluster-pm-race-avoidance.txt
> + * for details of the synchronisation algorithms used here.
> */
>
> #include <linux/linkage.h>
> #include <asm/bL_entry.h>
>
> +.if BL_SYNC_CLUSTER_CPUS
> +.error "cpus must be the first member of struct bL_cluster_sync_struct"
> +.endif
> +
> .macro pr_dbg cpu, string
> #if defined(CONFIG_DEBUG_LL) && defined(DEBUG)
> b 1901f
> @@ -52,12 +60,82 @@ ENTRY(bL_entry_point)
> 2: pr_dbg r4, "kernel bL_entry_point\n"
>
> /*
> - * MMU is off so we need to get to bL_entry_vectors in a
> + * MMU is off so we need to get to various variables in a
> * position independent way.
> */
> adr r5, 3f
> - ldr r6, [r5]
> + ldmia r5, {r6, r7, r8}
> add r6, r5, r6 @ r6 = bL_entry_vectors
> + ldr r7, [r5, r7] @ r7 = bL_power_up_setup_phys
> + add r8, r5, r8 @ r8 = bL_sync
> +
> + mov r0, #BL_SYNC_CLUSTER_SIZE
> + mla r8, r0, r10, r8 @ r8 = bL_sync cluster base
> +
> + @ Signal that this CPU is coming UP:
> + mov r0, #CPU_COMING_UP
> + mov r5, #BL_SYNC_CPU_SIZE
> + mla r5, r9, r5, r8 @ r5 = bL_sync cpu address
> + strb r0, [r5]
> +
> + dsb
Why is a dmb not enough here? In fact, the same goes for most of these
other than the one preceeding the sev. Is there an interaction with the
different mappings for the cluster data that I've missed?
> +
> + @ At this point, the cluster cannot unexpectedly enter the GOING_DOWN
> + @ state, because there is at least one active CPU (this CPU).
> +
> + @ Check if the cluster has been set up yet:
> + ldrb r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
> + cmp r0, #CLUSTER_UP
> + beq cluster_already_up
> +
> + @ Signal that the cluster is being brought up:
> + mov r0, #INBOUND_COMING_UP
> + strb r0, [r8, #BL_SYNC_CLUSTER_INBOUND]
> +
> + dsb
> +
> + @ Any CPU trying to take the cluster into CLUSTER_GOING_DOWN from this
> + @ point onwards will observe INBOUND_COMING_UP and abort.
> +
> + @ Wait for any previously-pending cluster teardown operations to abort
> + @ or complete:
> +cluster_teardown_wait:
> + ldrb r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
> + cmp r0, #CLUSTER_GOING_DOWN
> + wfeeq
> + beq cluster_teardown_wait
> +
> + @ power_up_setup is responsible for setting up the cluster:
> +
> + cmp r7, #0
> + mov r0, #1 @ second (cluster) affinity level
> + blxne r7 @ Call power_up_setup if defined
> +
> + @ Leave the cluster setup critical section:
> +
> + dsb
> + mov r0, #INBOUND_NOT_COMING_UP
> + strb r0, [r8, #BL_SYNC_CLUSTER_INBOUND]
> + mov r0, #CLUSTER_UP
> + strb r0, [r8, #BL_SYNC_CLUSTER_CLUSTER]
> + dsb
> + sev
> +
> +cluster_already_up:
> + @ If a platform-specific CPU setup hook is needed, it is
> + @ called from here.
> +
> + cmp r7, #0
> + mov r0, #0 @ first (CPU) affinity level
> + blxne r7 @ Call power_up_setup if defined
> +
> + @ Mark the CPU as up:
> +
> + dsb
> + mov r0, #CPU_UP
> + strb r0, [r5]
> + dsb
> + sev
>
> bL_entry_gated:
> ldr r5, [r6, r4, lsl #2] @ r5 = CPU entry vector
> @@ -70,6 +148,8 @@ bL_entry_gated:
> .align 2
>
> 3: .word bL_entry_vectors - .
> + .word bL_power_up_setup_phys - 3b
> + .word bL_sync - 3b
>
> ENDPROC(bL_entry_point)
>
> @@ -79,3 +159,7 @@ ENDPROC(bL_entry_point)
> .type bL_entry_vectors, #object
> ENTRY(bL_entry_vectors)
> .space 4 * BL_NR_CLUSTERS * BL_CPUS_PER_CLUSTER
> +
> + .type bL_power_up_setup_phys, #object
> +ENTRY(bL_power_up_setup_phys)
> + .space 4 @ set by bL_cluster_sync_init()
> diff --git a/arch/arm/include/asm/bL_entry.h b/arch/arm/include/asm/bL_entry.h
> index 942d7f9f19..167394d9a0 100644
> --- a/arch/arm/include/asm/bL_entry.h
> +++ b/arch/arm/include/asm/bL_entry.h
> @@ -15,8 +15,37 @@
> #define BL_CPUS_PER_CLUSTER 4
> #define BL_NR_CLUSTERS 2
>
> +/* Definitions for bL_cluster_sync_struct */
> +#define CPU_DOWN 0x11
> +#define CPU_COMING_UP 0x12
> +#define CPU_UP 0x13
> +#define CPU_GOING_DOWN 0x14
> +
> +#define CLUSTER_DOWN 0x21
> +#define CLUSTER_UP 0x22
> +#define CLUSTER_GOING_DOWN 0x23
> +
> +#define INBOUND_NOT_COMING_UP 0x31
> +#define INBOUND_COMING_UP 0x32
Do these numbers signify anything? Why not 0, 1, 2 etc?
> +
> +/* This is a complete guess. */
> +#define __CACHE_WRITEBACK_ORDER 6
Is this CONFIG_ARM_L1_CACHE_SHIFT?
> +#define __CACHE_WRITEBACK_GRANULE (1 << __CACHE_WRITEBACK_ORDER)
> +
> +/* Offsets for the bL_cluster_sync_struct members, for use in asm: */
> +#define BL_SYNC_CLUSTER_CPUS 0
Why not use asm-offsets.h for this?
> +#define BL_SYNC_CPU_SIZE __CACHE_WRITEBACK_GRANULE
> +#define BL_SYNC_CLUSTER_CLUSTER \
> + (BL_SYNC_CLUSTER_CPUS + BL_SYNC_CPU_SIZE * BL_CPUS_PER_CLUSTER)
> +#define BL_SYNC_CLUSTER_INBOUND \
> + (BL_SYNC_CLUSTER_CLUSTER + __CACHE_WRITEBACK_GRANULE)
> +#define BL_SYNC_CLUSTER_SIZE \
> + (BL_SYNC_CLUSTER_INBOUND + __CACHE_WRITEBACK_GRANULE)
> +
Hmm, this looks pretty fragile to me but again, you need this stuff at
compile time. Is there an architected maximum value for the writeback
granule? Failing that, we may as well just use things like
__cacheline_aligned if we're only using the L1 alignment anyway.
Will
More information about the linux-arm-kernel
mailing list