From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B925FC48BE5 for ; Wed, 16 Jun 2021 15:06:42 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 70E5060FE5 for ; Wed, 16 Jun 2021 15:06:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 70E5060FE5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=PFob8cVpLHeNy+H98726TZTb9U2LpYYeBe2jI1Z4sSk=; b=iJg+AWGTfYFLhD 3CMZ/j0GTNkDpTSqAwUUPF9PjiiCZDo6VmfeRHYC9yUXLPazu1W/VJWesyYRrzeBYvPbsu2W3QW3z J1km1pWe5pnT0jrxCDuR1ldxCLNNoHrABle2JK331CfzRnX+FsZyxgitGWLHpTiADOhg3wDRIb1fD aALpaGv97VsNb1zVCgZNM2fhK9AB4/WM2EDs3PuCNtHJZF2W6+eZfY8xV7GPQ3Th9gDSpQ4FcyeYQ p8eMukAj08rRCKM6RHWmGBICUTvF229gFZS1XaZl2xmTSF8TDtI+WA5bJNYBHqjkx/iSaa8O0NVEX 9DmMsL1ls4u0WvFHBWVg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltX6I-006os2-AQ; Wed, 16 Jun 2021 15:05:06 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltX6D-006oqT-Kl for linux-arm-kernel@lists.infradead.org; Wed, 16 Jun 2021 15:05:03 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 5519F600EF; Wed, 16 Jun 2021 15:04:59 +0000 (UTC) Date: Wed, 16 Jun 2021 16:04:56 +0100 From: Catalin Marinas To: "Russell King (Oracle)" Cc: Peter Zijlstra , Andy Lutomirski , x86@kernel.org, Dave Hansen , LKML , linux-mm@kvack.org, Andrew Morton , Mathieu Desnoyers , Nicholas Piggin , linux-arm-kernel@lists.infradead.org, Will Deacon Subject: Re: [PATCH 7/8] membarrier: Remove arm (32) support for SYNC_CORE Message-ID: <20210616150456.GC22433@arm.com> References: <2142129092ff9aa00e600c42a26c4015b7f5ceec.1623813516.git.luto@kernel.org> <20210616103446.GC22278@shell.armlinux.org.uk> <20210616132226.GD22278@shell.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210616132226.GD22278@shell.armlinux.org.uk> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210616_080501_758889_385600BA X-CRM114-Status: GOOD ( 36.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jun 16, 2021 at 02:22:27PM +0100, Russell King wrote: > On Wed, Jun 16, 2021 at 01:10:58PM +0200, Peter Zijlstra wrote: > > On Wed, Jun 16, 2021 at 11:34:46AM +0100, Russell King (Oracle) wrote: > > > On Wed, Jun 16, 2021 at 12:20:06PM +0200, Peter Zijlstra wrote: > > > > On Wed, Jun 16, 2021 at 12:16:27PM +0200, Peter Zijlstra wrote: > > > > > On Tue, Jun 15, 2021 at 08:21:12PM -0700, Andy Lutomirski wrote: > > > > > > On arm32, the only way to safely flush icache from usermode is to call > > > > > > cacheflush(2). This also handles any required pipeline flushes, so > > > > > > membarrier's SYNC_CORE feature is useless on arm. Remove it. > > > > > > > > > > So SYNC_CORE is there to help an architecture that needs to do something > > > > > per CPU. If I$ invalidation is broadcast and I$ invalidation also > > > > > triggers the flush of any uarch caches derived from it (if there are > > > > > any). > > > > > > > > Incomplete sentence there: + then we don't need SYNC_CORE. > > > > > > > > > Now arm_syscall() NR(cacheflush) seems to do flush_icache_user_range(), > > > > > which, if I read things right, end up in arch/arm/mm/*.S, but that > > > > > doesn't consider cache_ops_need_broadcast(). > > > > > > > > > > Will suggests that perhaps ARM 11MPCore might need this due to their I$ > > > > > flush maybe not being broadcast > > > > > > If it leaves other cores with incoherent I cache, then that's already > > > a problem for SMP cores, since there could be no guarantee that the > > > modifications made by one core will be visible to some other core that > > > ends up running that code - and there is little option for userspace to > > > work around that except by pinning the thread making the modifications > > > and subsequently executing the code to a core. > > > > That's where SYNC_CORE can help. Or you make sys_cacheflush() do a > > system wide IPI. > > If it's a problem, then it needs fixing. sys_cacheflush() is used to > implement GCC's __builtin___clear_cache(). I'm not sure who added this > to gcc. I'm surprised that it works. I guess it's just luck that the thread doing the code writing doesn't migrate before the sys_cacheflush() call. > > > The same is also true of flush_icache_range() - which is used when > > > loading a kernel module. In the case Will is referring to, these alias > > > to the same code. > > > > Yes, cache_ops_need_broadcast() seems to be missing in more places. > > Likely only in places where we care about I/D coherency - as the data > cache is required to be PIPT on these SMP platforms. We had similar issue with the cache maintenance for DMA. The hack we employed (in cache.S) is relying on the MESI protocol internals and forcing a read/write for ownership before the D-cache maintenance. Luckily ARM11MPCore doesn't do speculative data loads to trigger some migration back. The simpler fix for flush_icache_range() is to disable preemption, read a word in a cacheline to force any dirty lines on another CPU to be evicted and then issue the D-cache maintenance (for those cache lines which are still dirty on the current CPU). It's a hack that only works on ARM11MPCore. Newer MP cores are saner. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel