From patchwork Wed Nov 7 18:03:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 1711841 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 4F6823FCAE for ; Wed, 7 Nov 2012 18:05:36 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TW9ye-0000ow-Nm; Wed, 07 Nov 2012 18:03:48 +0000 Received: from mail-qc0-f177.google.com ([209.85.216.177]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TW9yb-0000np-SZ for linux-arm-kernel@lists.infradead.org; Wed, 07 Nov 2012 18:03:46 +0000 Received: by mail-qc0-f177.google.com with SMTP id u28so887541qcs.36 for ; Wed, 07 Nov 2012 10:03:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version:content-type:x-gm-message-state; bh=Dviicy/Spb1LyOvmtd4nBcOuGr2dC9qwHn+QONeKTEk=; b=CDJcLJ9QT5+K8SN5Ytn3bbDYokLG2ruXtm0lPlyXdxGYxOVdXY6C2HD6QIcroIKDRA Hu8YOyxRM5U+LosKj95S0mwmpweXXqbOVicuRpHZtG9eSmpf40zYbdiZjJQi9EnU2j0a m03OUqw7AyeY6sazKQYzTlSO4HlnjvVNt18YT2uRBKHxrSoZW936DnqpcQtpWkNKd4Ig kKVp7UqWV3req43VoXmM3Dj46JgcRxgEctciySV4IPd0HS2bjUhpy3kDw8Ie5pAwCnpP eB6AmZtuGiJJMd+iNApmb+YZ/7kdQDPinzd1xYsnjriDRsa4X2W/1KkXuQMGMXp0300/ MLyg== Received: by 10.49.82.98 with SMTP id h2mr9354131qey.14.1352311422500; Wed, 07 Nov 2012 10:03:42 -0800 (PST) Received: from xanadu.home (modemcable203.213-202-24.mc.videotron.ca. [24.202.213.203]) by mx.google.com with ESMTPS id g9sm14384189qaj.18.2012.11.07.10.03.41 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 07 Nov 2012 10:03:41 -0800 (PST) Date: Wed, 7 Nov 2012 13:03:40 -0500 (EST) From: Nicolas Pitre To: Will Deacon Subject: Re: [PATCH] ARM: setup_mm_for_reboot(): use flush_cache_louis() In-Reply-To: <20121107100835.GB23305@mudshark.cambridge.arm.com> Message-ID: References: <20121106220458.GP28327@n2100.arm.linux.org.uk> <20121107095106.GA23305@mudshark.cambridge.arm.com> <20121107095635.GV28327@n2100.arm.linux.org.uk> <20121107100835.GB23305@mudshark.cambridge.arm.com> User-Agent: Alpine 2.02 (LFD 1266 2009-07-14) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQkMHN9OH/0BI2RX66D86ic+cYogb8uMC+Gg8TX6IHqTvviRGNprCOZZnvKGQyUuU/SA5+ha X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20121107_130345_988556_71454DF1 X-CRM114-Status: GOOD ( 25.31 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.6 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.216.177 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Santosh Shilimkar , Lorenzo Pieralisi , Russell King - ARM Linux , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Wed, 7 Nov 2012, Will Deacon wrote: > On Wed, Nov 07, 2012 at 09:56:35AM +0000, Russell King - ARM Linux wrote: > > On Wed, Nov 07, 2012 at 09:51:06AM +0000, Will Deacon wrote: > > > Wouldn't the L2 flush in this case be included with the code that turns off > > > caching? For reboot, that's currently done in __sort_restart -- the cache > > > flush in setup_mm_for_reboot is just to ensure that the idmap tables are > > > visible to the hardware walker iirc. > > > > Good point - but it does raise another issue. Why do we do this flush and > > TLB invalidate afterwards in setup_mm_for_reboot() ? It makes sense if > > we change existing page tables, but we don't anymore, we're just switching > > them, and cpu_switch_mm() will do whatever's necessary to make the new > > page tables visible. So I think we can get rid of that flusing in there. > > Hmm, I'm not sure about that. Looking at cpu_v7_switch_mm (the 2-level > version) for example, we set the ASID and then set the new TTBR. There's no > flushing of page tables like we get in set_pte and no TLB flushing either. > > Now, given that the idmap_pgd is populated as an early_initcall, I really > doubt we need that flush_cache_all but the TLB invalidate is surely > required? Why wouldn't cpu_switch_mm() take care of that already if that is necessary? Hmmm, I suppose the asid of the init task isn't associated with the idmap in any way, hence the need for flushing the tlbs. I'd not rely on the early_initcall to assume the new page table is moved out of the cache though. What about this alternate patch: Acked-by: Will Deacon diff --git a/arch/arm/mm/idmap.c b/arch/arm/mm/idmap.c index ab88ed4f8e..1ab429761c 100644 --- a/arch/arm/mm/idmap.c +++ b/arch/arm/mm/idmap.c @@ -92,6 +92,9 @@ static int __init init_static_idmap(void) (long long)idmap_start, (long long)idmap_end); identity_mapping_add(idmap_pgd, idmap_start, idmap_end); + /* Flush L1 for the hardware to see this page table content */ + flush_cache_louis(); + return 0; } early_initcall(init_static_idmap); @@ -103,12 +106,12 @@ early_initcall(init_static_idmap); */ void setup_mm_for_reboot(void) { - /* Clean and invalidate L1. */ - flush_cache_all(); - /* Switch to the identity mapping. */ cpu_switch_mm(idmap_pgd, &init_mm); - /* Flush the TLB. */ + /* + * On ARMv7, the ASID of the init task is not associated with + * this mapping. TLBs must be flushed. + */ local_flush_tlb_all(); }