From patchwork Thu Apr 18 18:25:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 2462101 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork1.kernel.org (Postfix) with ESMTP id 73DE43FD8C for ; Thu, 18 Apr 2013 18:26:01 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UStWq-0006uT-D5; Thu, 18 Apr 2013 18:25:52 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UStWn-0005gJ-R0; Thu, 18 Apr 2013 18:25:49 +0000 Received: from caramon.arm.linux.org.uk ([78.32.30.218]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UStWj-0005fx-4b for linux-arm-kernel@lists.infradead.org; Thu, 18 Apr 2013 18:25:47 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=2PkWMQeo7RQ+IIsCUboPD2i+yTsdKNUVNpOD5rwHvr0=; b=LZlqFOs79XT2TyAJlcAbT+x0ncYCQ22HohzD2pm5JMBn0HtUxGp5/3WCHHlJqCsn61zm+bDCxjtcIpk3c9PsSMSLQOly07B0TPYhGAC8f6ejRq6KnKY5w85QX9V04FxBAS070qqvS2xjXIjEEmf4hNmELz3xwWb/Ab6EygkwM/Q=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:55903) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1UStWb-0005eR-0N; Thu, 18 Apr 2013 19:25:37 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1UStWZ-00060d-BI; Thu, 18 Apr 2013 19:25:35 +0100 Date: Thu, 18 Apr 2013 19:25:34 +0100 From: Russell King - ARM Linux To: Kevin Bracey Subject: Re: [PATCH] ARM: smp: call platform's cpu_die in ipi_cpu_stop Message-ID: <20130418182534.GD14496@n2100.arm.linux.org.uk> References: <1365251824-4852-1-git-send-email-kevin@bracey.fi> <1365251824-4852-2-git-send-email-kevin@bracey.fi> <20130418171801.GC14496@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130418171801.GC14496@n2100.arm.linux.org.uk> User-Agent: Mutt/1.5.19 (2009-01-05) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130418_142546_697844_F04BE910 X-CRM114-Status: GOOD ( 35.16 ) X-Spam-Score: -5.0 (-----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-5.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [78.32.30.218 listed in list.dnswl.org] -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Thu, Apr 18, 2013 at 06:18:01PM +0100, Russell King - ARM Linux wrote: > On Sat, Apr 06, 2013 at 03:37:04PM +0300, Kevin Bracey wrote: > > When hotplugging out, both cpu_kill and cpu_die have always been called. > > But when smp_send_stop was used in machine_shutdown or panic, only > > cpu_kill was called. > > > > This causes problems for co-ordination between cpu_kill and cpu_die, as > > attempted by shmobile. So add cpu_die call to ipi_cpu_stop, to ensure > > that cpu_kill and cpu_die calls always occur together. > > Actually, I'd prefer to pull more code out of the platforms. Now > that we have flush_cache_louis(), we can flush the L1 cache for the > CPU going down in cpu_die() itself. We just need to be careful > with that complete() call to ensure that becomes visible to other > cores before power is cut to the L1 cache. > > That's fine though - because it must become visible for __cpu_die() > to continue (otherwise it will time out). > > That should render shmobile's cpu_dead thing unnecessary, because the > platform independent code will do the required synchronisation. So, something like the below (which is a combined patch of three). The key points here being that: 1. We flush the L1 cache in cpu_die(), which pushes any dirty cache lines out of the L1 cache and invalidates it. At the point this function completes, any data in the L1 cache has been pushed out and all cache lines are invalid. 2. We complete(), which allows __cpu_die() to proceed and call platform_cpu_kill(). This may create dirty cache lines which this CPU exclusively owns, but the CPU in __cpu_die() will gain those cache lines before wait_for_completion() can return - not only the completion counter but also the spinlock, which this CPU will have ended up reading and writing - and so can no longer be owned by the dying CPU. 3. The following mb() is belt and braces to ensure that the completion is visible. It isn't strictly required because the spinlocks will do the necessary stuff to ensure this. 4. Any dirtying of the cache after this rellay doesn't matter _if_ only the stack is touched, or other data is only read. 5. I've left those platforms which disable the L1 cache, then flush alone; that _should_ be removable with this patch as well, but as a separate patch. Now, with this patch applied, we guarantee that we push out any data that matters from the dying CPU before platform_cpu_kill() is called. That should mean that shmobile can remove that whole cpu_dead thing. I've tested this on OMAP, and it appears to work from a simple test of CPU hotplug. This patch(set) also kills the cpu_disable() that tegra has which is just a copy of the generic version. arch/arm/kernel/smp.c | 4 ++++ arch/arm/mach-exynos/hotplug.c | 1 - arch/arm/mach-highbank/hotplug.c | 4 ---- arch/arm/mach-imx/hotplug.c | 2 -- arch/arm/mach-msm/hotplug.c | 4 ---- arch/arm/mach-omap2/omap-hotplug.c | 3 --- arch/arm/mach-prima2/hotplug.c | 3 --- arch/arm/mach-realview/hotplug.c | 2 -- arch/arm/mach-shmobile/smp-sh73a0.c | 8 -------- arch/arm/mach-spear13xx/hotplug.c | 2 -- arch/arm/mach-tegra/common.h | 1 - arch/arm/mach-tegra/hotplug.c | 10 ---------- arch/arm/mach-tegra/platsmp.c | 1 - arch/arm/mach-ux500/hotplug.c | 3 --- arch/arm/mach-vexpress/hotplug.c | 2 -- 15 files changed, 4 insertions(+), 46 deletions(-) diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 1f2cccc..d0cb2e1 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -230,10 +230,14 @@ void __ref cpu_die(void) idle_task_exit(); local_irq_disable(); + + /* Flush the data out of the L1 cache for this CPU. */ + flush_cache_louis(); mb(); /* Tell __cpu_die() that this CPU is now safe to dispose of */ RCU_NONIDLE(complete(&cpu_died)); + mb(); /* * actual CPU shutdown procedure is at least platform (if not diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c index c3f825b..af90cfa 100644 --- a/arch/arm/mach-exynos/hotplug.c +++ b/arch/arm/mach-exynos/hotplug.c @@ -28,7 +28,6 @@ static inline void cpu_enter_lowpower_a9(void) { unsigned int v; - flush_cache_all(); asm volatile( " mcr p15, 0, %1, c7, c5, 0\n" " mcr p15, 0, %1, c7, c10, 4\n" diff --git a/arch/arm/mach-highbank/hotplug.c b/arch/arm/mach-highbank/hotplug.c index f30c528..35dd42e 100644 --- a/arch/arm/mach-highbank/hotplug.c +++ b/arch/arm/mach-highbank/hotplug.c @@ -15,8 +15,6 @@ */ #include -#include - #include "core.h" #include "sysregs.h" @@ -28,8 +26,6 @@ extern void secondary_startup(void); */ void __ref highbank_cpu_die(unsigned int cpu) { - flush_cache_all(); - highbank_set_cpu_jump(cpu, phys_to_virt(0)); highbank_set_core_pwr(); diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c index 361a253..5e91112 100644 --- a/arch/arm/mach-imx/hotplug.c +++ b/arch/arm/mach-imx/hotplug.c @@ -11,7 +11,6 @@ */ #include -#include #include #include "common.h" @@ -20,7 +19,6 @@ static inline void cpu_enter_lowpower(void) { unsigned int v; - flush_cache_all(); asm volatile( "mcr p15, 0, %1, c7, c5, 0\n" " mcr p15, 0, %1, c7, c10, 4\n" diff --git a/arch/arm/mach-msm/hotplug.c b/arch/arm/mach-msm/hotplug.c index 750446f..326a872 100644 --- a/arch/arm/mach-msm/hotplug.c +++ b/arch/arm/mach-msm/hotplug.c @@ -10,16 +10,12 @@ #include #include -#include #include #include "common.h" static inline void cpu_enter_lowpower(void) { - /* Just flush the cache. Changing the coherency is not yet - * available on msm. */ - flush_cache_all(); } static inline void cpu_leave_lowpower(void) diff --git a/arch/arm/mach-omap2/omap-hotplug.c b/arch/arm/mach-omap2/omap-hotplug.c index e712d17..ceb30a5 100644 --- a/arch/arm/mach-omap2/omap-hotplug.c +++ b/arch/arm/mach-omap2/omap-hotplug.c @@ -35,9 +35,6 @@ void __ref omap4_cpu_die(unsigned int cpu) unsigned int boot_cpu = 0; void __iomem *base = omap_get_wakeupgen_base(); - flush_cache_all(); - dsb(); - /* * we're ready for shutdown now, so do it */ diff --git a/arch/arm/mach-prima2/hotplug.c b/arch/arm/mach-prima2/hotplug.c index f4b17cb..0ab2f8b 100644 --- a/arch/arm/mach-prima2/hotplug.c +++ b/arch/arm/mach-prima2/hotplug.c @@ -10,13 +10,10 @@ #include #include -#include #include static inline void platform_do_lowpower(unsigned int cpu) { - flush_cache_all(); - /* we put the platform to just WFI */ for (;;) { __asm__ __volatile__("dsb\n\t" "wfi\n\t" diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c index 53818e5..ac22dd4 100644 --- a/arch/arm/mach-realview/hotplug.c +++ b/arch/arm/mach-realview/hotplug.c @@ -12,7 +12,6 @@ #include #include -#include #include #include @@ -20,7 +19,6 @@ static inline void cpu_enter_lowpower(void) { unsigned int v; - flush_cache_all(); asm volatile( " mcr p15, 0, %1, c7, c5, 0\n" " mcr p15, 0, %1, c7, c10, 4\n" diff --git a/arch/arm/mach-shmobile/smp-sh73a0.c b/arch/arm/mach-shmobile/smp-sh73a0.c index acb46a9..2f1ef1b 100644 --- a/arch/arm/mach-shmobile/smp-sh73a0.c +++ b/arch/arm/mach-shmobile/smp-sh73a0.c @@ -119,14 +119,6 @@ static int sh73a0_cpu_kill(unsigned int cpu) static void sh73a0_cpu_die(unsigned int cpu) { - /* - * The ARM MPcore does not issue a cache coherency request for the L1 - * cache when powering off single CPUs. We must take care of this and - * further caches. - */ - dsb(); - flush_cache_all(); - /* Set power off mode. This takes the CPU out of the MP cluster */ scu_power_mode(scu_base_addr(), SCU_PM_POWEROFF); diff --git a/arch/arm/mach-spear13xx/hotplug.c b/arch/arm/mach-spear13xx/hotplug.c index a7d2dd1..d97749c 100644 --- a/arch/arm/mach-spear13xx/hotplug.c +++ b/arch/arm/mach-spear13xx/hotplug.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -21,7 +20,6 @@ static inline void cpu_enter_lowpower(void) { unsigned int v; - flush_cache_all(); asm volatile( " mcr p15, 0, %1, c7, c5, 0\n" " dsb\n" diff --git a/arch/arm/mach-tegra/common.h b/arch/arm/mach-tegra/common.h index 32f8eb3..5900cc4 100644 --- a/arch/arm/mach-tegra/common.h +++ b/arch/arm/mach-tegra/common.h @@ -2,4 +2,3 @@ extern struct smp_operations tegra_smp_ops; extern int tegra_cpu_kill(unsigned int cpu); extern void tegra_cpu_die(unsigned int cpu); -extern int tegra_cpu_disable(unsigned int cpu); diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c index a599f6e..e8323bc 100644 --- a/arch/arm/mach-tegra/hotplug.c +++ b/arch/arm/mach-tegra/hotplug.c @@ -12,7 +12,6 @@ #include #include -#include #include #include "sleep.h" @@ -47,15 +46,6 @@ void __ref tegra_cpu_die(unsigned int cpu) BUG(); } -int tegra_cpu_disable(unsigned int cpu) -{ - /* - * we don't allow CPU 0 to be shutdown (it is still too special - * e.g. clock tick interrupts) - */ - return cpu == 0 ? -EPERM : 0; -} - #ifdef CONFIG_ARCH_TEGRA_2x_SOC extern void tegra20_hotplug_shutdown(void); void __init tegra20_hotplug_init(void) diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c index 2c6b3d5..ec33ec8 100644 --- a/arch/arm/mach-tegra/platsmp.c +++ b/arch/arm/mach-tegra/platsmp.c @@ -192,6 +192,5 @@ struct smp_operations tegra_smp_ops __initdata = { #ifdef CONFIG_HOTPLUG_CPU .cpu_kill = tegra_cpu_kill, .cpu_die = tegra_cpu_die, - .cpu_disable = tegra_cpu_disable, #endif }; diff --git a/arch/arm/mach-ux500/hotplug.c b/arch/arm/mach-ux500/hotplug.c index 2f6af25..1c55a55 100644 --- a/arch/arm/mach-ux500/hotplug.c +++ b/arch/arm/mach-ux500/hotplug.c @@ -12,7 +12,6 @@ #include #include -#include #include #include @@ -24,8 +23,6 @@ */ void __ref ux500_cpu_die(unsigned int cpu) { - flush_cache_all(); - /* directly enter low power state, skipping secure registers */ for (;;) { __asm__ __volatile__("dsb\n\t" "wfi\n\t" diff --git a/arch/arm/mach-vexpress/hotplug.c b/arch/arm/mach-vexpress/hotplug.c index a141b98..f0ce6b8 100644 --- a/arch/arm/mach-vexpress/hotplug.c +++ b/arch/arm/mach-vexpress/hotplug.c @@ -12,7 +12,6 @@ #include #include -#include #include #include @@ -20,7 +19,6 @@ static inline void cpu_enter_lowpower(void) { unsigned int v; - flush_cache_all(); asm volatile( "mcr p15, 0, %1, c7, c5, 0\n" " mcr p15, 0, %1, c7, c10, 4\n"