diff options
| author | Russell King <rmk+kernel@arm.linux.org.uk> | 2010-12-18 05:53:12 -0500 |
|---|---|---|
| committer | Russell King <rmk+kernel@arm.linux.org.uk> | 2010-12-20 10:09:13 -0500 |
| commit | 3705ff6da538aff6dba535e2e9cbcbb9456d0d53 (patch) | |
| tree | 348fcec2be9d41e1839686a6c03f0b5479d7e4f9 | |
| parent | ed3768a8d9dc2d345d4f27eb44ee1e4825056c08 (diff) | |
ARM: Fix subtle race in CPU pen_release hotplug code
There is a subtle race in the CPU hotplug code, where a CPU which has
been offlined can online itself before being requested, which results
in things going astray on the next online/offline cycle.
What happens in the normal online/offline/online cycle is:
CPU0 CPU3
requests boot of CPU3
pen_release = 3
flush cache line
checks pen_release, reads 3
starts boot
pen_release = -1
... requests CPU3 offline ...
... dies ...
checks pen_release, reads -1
requests boot of CPU3
pen_release = 3
flush cache line
checks pen_release, reads 3
starts boot
pen_release = -1
However, as the write of -1 of pen_release is not fully flushed back to
memory, and the checking of pen_release is done with caches disabled,
this allows CPU3 the opportunity to read the old value of pen_release:
CPU0 CPU3
requests boot of CPU3
pen_release = 3
flush cache line
checks pen_release, reads 3
starts boot
pen_release = -1
... requests CPU3 offline ...
... dies ...
checks pen_release, reads 3
starts boot
pen_release = -1
requests boot of CPU3
pen_release = 3
flush cache line
Fix this by grouping the write of pen_release along with its cache line
flushing code to ensure that any update to pen_release is always pushed
out to physical memory.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
| -rw-r--r-- | arch/arm/mach-realview/platsmp.c | 19 | ||||
| -rw-r--r-- | arch/arm/mach-s5pv310/platsmp.c | 20 | ||||
| -rw-r--r-- | arch/arm/mach-ux500/platsmp.c | 22 | ||||
| -rw-r--r-- | arch/arm/mach-vexpress/platsmp.c | 20 |
4 files changed, 60 insertions, 21 deletions
diff --git a/arch/arm/mach-realview/platsmp.c b/arch/arm/mach-realview/platsmp.c index 226c63102a00..bb8d6c4e4315 100644 --- a/arch/arm/mach-realview/platsmp.c +++ b/arch/arm/mach-realview/platsmp.c | |||
| @@ -36,6 +36,19 @@ extern void realview_secondary_startup(void); | |||
| 36 | */ | 36 | */ |
| 37 | volatile int __cpuinitdata pen_release = -1; | 37 | volatile int __cpuinitdata pen_release = -1; |
| 38 | 38 | ||
| 39 | /* | ||
| 40 | * Write pen_release in a way that is guaranteed to be visible to all | ||
| 41 | * observers, irrespective of whether they're taking part in coherency | ||
| 42 | * or not. This is necessary for the hotplug code to work reliably. | ||
| 43 | */ | ||
| 44 | static void write_pen_release(int val) | ||
| 45 | { | ||
| 46 | pen_release = val; | ||
| 47 | smp_wmb(); | ||
| 48 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 49 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 50 | } | ||
| 51 | |||
| 39 | static void __iomem *scu_base_addr(void) | 52 | static void __iomem *scu_base_addr(void) |
| 40 | { | 53 | { |
| 41 | if (machine_is_realview_eb_mp()) | 54 | if (machine_is_realview_eb_mp()) |
| @@ -64,8 +77,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu) | |||
| 64 | * let the primary processor know we're out of the | 77 | * let the primary processor know we're out of the |
| 65 | * pen, then head off into the C entry point | 78 | * pen, then head off into the C entry point |
| 66 | */ | 79 | */ |
| 67 | pen_release = -1; | 80 | write_pen_release(-1); |
| 68 | smp_wmb(); | ||
| 69 | 81 | ||
| 70 | /* | 82 | /* |
| 71 | * Synchronise with the boot thread. | 83 | * Synchronise with the boot thread. |
| @@ -92,8 +104,7 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle) | |||
| 92 | * Note that "pen_release" is the hardware CPU ID, whereas | 104 | * Note that "pen_release" is the hardware CPU ID, whereas |
| 93 | * "cpu" is Linux's internal ID. | 105 | * "cpu" is Linux's internal ID. |
| 94 | */ | 106 | */ |
| 95 | pen_release = cpu; | 107 | write_pen_release(cpu); |
| 96 | flush_cache_all(); | ||
| 97 | 108 | ||
| 98 | /* | 109 | /* |
| 99 | * Send the secondary CPU a soft interrupt, thereby causing | 110 | * Send the secondary CPU a soft interrupt, thereby causing |
diff --git a/arch/arm/mach-s5pv310/platsmp.c b/arch/arm/mach-s5pv310/platsmp.c index 18aaf5f54033..98c04748ed84 100644 --- a/arch/arm/mach-s5pv310/platsmp.c +++ b/arch/arm/mach-s5pv310/platsmp.c | |||
| @@ -37,6 +37,19 @@ extern void s5pv310_secondary_startup(void); | |||
| 37 | 37 | ||
| 38 | volatile int __cpuinitdata pen_release = -1; | 38 | volatile int __cpuinitdata pen_release = -1; |
| 39 | 39 | ||
| 40 | /* | ||
| 41 | * Write pen_release in a way that is guaranteed to be visible to all | ||
| 42 | * observers, irrespective of whether they're taking part in coherency | ||
| 43 | * or not. This is necessary for the hotplug code to work reliably. | ||
| 44 | */ | ||
| 45 | static void write_pen_release(int val) | ||
| 46 | { | ||
| 47 | pen_release = val; | ||
| 48 | smp_wmb(); | ||
| 49 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 50 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 51 | } | ||
| 52 | |||
| 40 | static void __iomem *scu_base_addr(void) | 53 | static void __iomem *scu_base_addr(void) |
| 41 | { | 54 | { |
| 42 | return (void __iomem *)(S5P_VA_SCU); | 55 | return (void __iomem *)(S5P_VA_SCU); |
| @@ -57,8 +70,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu) | |||
| 57 | * let the primary processor know we're out of the | 70 | * let the primary processor know we're out of the |
| 58 | * pen, then head off into the C entry point | 71 | * pen, then head off into the C entry point |
| 59 | */ | 72 | */ |
| 60 | pen_release = -1; | 73 | write_pen_release(-1); |
| 61 | smp_wmb(); | ||
| 62 | 74 | ||
| 63 | /* | 75 | /* |
| 64 | * Synchronise with the boot thread. | 76 | * Synchronise with the boot thread. |
| @@ -85,9 +97,7 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle) | |||
| 85 | * Note that "pen_release" is the hardware CPU ID, whereas | 97 | * Note that "pen_release" is the hardware CPU ID, whereas |
| 86 | * "cpu" is Linux's internal ID. | 98 | * "cpu" is Linux's internal ID. |
| 87 | */ | 99 | */ |
| 88 | pen_release = cpu; | 100 | write_pen_release(cpu); |
| 89 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 90 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 91 | 101 | ||
| 92 | /* | 102 | /* |
| 93 | * Send the secondary CPU a soft interrupt, thereby causing | 103 | * Send the secondary CPU a soft interrupt, thereby causing |
diff --git a/arch/arm/mach-ux500/platsmp.c b/arch/arm/mach-ux500/platsmp.c index ddedbc80c41f..f71175a766d7 100644 --- a/arch/arm/mach-ux500/platsmp.c +++ b/arch/arm/mach-ux500/platsmp.c | |||
| @@ -27,6 +27,19 @@ | |||
| 27 | */ | 27 | */ |
| 28 | volatile int __cpuinitdata pen_release = -1; | 28 | volatile int __cpuinitdata pen_release = -1; |
| 29 | 29 | ||
| 30 | /* | ||
| 31 | * Write pen_release in a way that is guaranteed to be visible to all | ||
| 32 | * observers, irrespective of whether they're taking part in coherency | ||
| 33 | * or not. This is necessary for the hotplug code to work reliably. | ||
| 34 | */ | ||
| 35 | static void write_pen_release(int val) | ||
| 36 | { | ||
| 37 | pen_release = val; | ||
| 38 | smp_wmb(); | ||
| 39 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 40 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 41 | } | ||
| 42 | |||
| 30 | static DEFINE_SPINLOCK(boot_lock); | 43 | static DEFINE_SPINLOCK(boot_lock); |
| 31 | 44 | ||
| 32 | void __cpuinit platform_secondary_init(unsigned int cpu) | 45 | void __cpuinit platform_secondary_init(unsigned int cpu) |
| @@ -42,7 +55,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu) | |||
| 42 | * let the primary processor know we're out of the | 55 | * let the primary processor know we're out of the |
| 43 | * pen, then head off into the C entry point | 56 | * pen, then head off into the C entry point |
| 44 | */ | 57 | */ |
| 45 | pen_release = -1; | 58 | write_pen_release(-1); |
| 46 | 59 | ||
| 47 | /* | 60 | /* |
| 48 | * Synchronise with the boot thread. | 61 | * Synchronise with the boot thread. |
| @@ -66,9 +79,7 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle) | |||
| 66 | * the holding pen - release it, then wait for it to flag | 79 | * the holding pen - release it, then wait for it to flag |
| 67 | * that it has been released by resetting pen_release. | 80 | * that it has been released by resetting pen_release. |
| 68 | */ | 81 | */ |
| 69 | pen_release = cpu; | 82 | write_pen_release(cpu); |
| 70 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 71 | outer_clean_range(__pa(&pen_release), __pa(&pen_release) + 1); | ||
| 72 | 83 | ||
| 73 | smp_cross_call(cpumask_of(cpu), 1); | 84 | smp_cross_call(cpumask_of(cpu), 1); |
| 74 | 85 | ||
| @@ -89,9 +100,6 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle) | |||
| 89 | 100 | ||
| 90 | static void __init wakeup_secondary(void) | 101 | static void __init wakeup_secondary(void) |
| 91 | { | 102 | { |
| 92 | /* nobody is to be released from the pen yet */ | ||
| 93 | pen_release = -1; | ||
| 94 | |||
| 95 | /* | 103 | /* |
| 96 | * write the address of secondary startup into the backup ram register | 104 | * write the address of secondary startup into the backup ram register |
| 97 | * at offset 0x1FF4, then write the magic number 0xA1FEED01 to the | 105 | * at offset 0x1FF4, then write the magic number 0xA1FEED01 to the |
diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c index d7e0cb994e9d..8ce9fef29555 100644 --- a/arch/arm/mach-vexpress/platsmp.c +++ b/arch/arm/mach-vexpress/platsmp.c | |||
| @@ -34,6 +34,19 @@ extern void vexpress_secondary_startup(void); | |||
| 34 | */ | 34 | */ |
| 35 | volatile int __cpuinitdata pen_release = -1; | 35 | volatile int __cpuinitdata pen_release = -1; |
| 36 | 36 | ||
| 37 | /* | ||
| 38 | * Write pen_release in a way that is guaranteed to be visible to all | ||
| 39 | * observers, irrespective of whether they're taking part in coherency | ||
| 40 | * or not. This is necessary for the hotplug code to work reliably. | ||
| 41 | */ | ||
| 42 | static void write_pen_release(int val) | ||
| 43 | { | ||
| 44 | pen_release = val; | ||
| 45 | smp_wmb(); | ||
| 46 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 47 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 48 | } | ||
| 49 | |||
| 37 | static void __iomem *scu_base_addr(void) | 50 | static void __iomem *scu_base_addr(void) |
| 38 | { | 51 | { |
| 39 | return MMIO_P2V(A9_MPCORE_SCU); | 52 | return MMIO_P2V(A9_MPCORE_SCU); |
| @@ -54,8 +67,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu) | |||
| 54 | * let the primary processor know we're out of the | 67 | * let the primary processor know we're out of the |
| 55 | * pen, then head off into the C entry point | 68 | * pen, then head off into the C entry point |
| 56 | */ | 69 | */ |
| 57 | pen_release = -1; | 70 | write_pen_release(-1); |
| 58 | smp_wmb(); | ||
| 59 | 71 | ||
| 60 | /* | 72 | /* |
| 61 | * Synchronise with the boot thread. | 73 | * Synchronise with the boot thread. |
| @@ -80,9 +92,7 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle) | |||
| 80 | * since we haven't sent them a soft interrupt, they shouldn't | 92 | * since we haven't sent them a soft interrupt, they shouldn't |
| 81 | * be there. | 93 | * be there. |
| 82 | */ | 94 | */ |
| 83 | pen_release = cpu; | 95 | write_pen_release(cpu); |
| 84 | __cpuc_flush_dcache_area((void *)&pen_release, sizeof(pen_release)); | ||
| 85 | outer_clean_range(__pa(&pen_release), __pa(&pen_release + 1)); | ||
| 86 | 96 | ||
| 87 | /* | 97 | /* |
| 88 | * Send the secondary CPU a soft interrupt, thereby causing | 98 | * Send the secondary CPU a soft interrupt, thereby causing |
