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=-8.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 7D649C433E0 for ; Mon, 6 Jul 2020 17:07:23 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 4864A21D7D for ; Mon, 6 Jul 2020 17:07:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="0e1wm12C" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4864A21D7D Authentication-Results: mail.kernel.org; dmarc=none (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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=7jdrYlnWjTF/ao84xk9qwTrbcxgnY2IiGkLO89m+moE=; b=0e1wm12COgtRpoyKO2aTfSUL9 v00miBG2KQuo9ZylJMAodMBNMAQSy6Xmx/JbrFMzTDRMN33XMP2tMuIwBLLreuhgy8GCA6YMMa00Z r/aDN8158rUwNbKuo81cs4BgFL9JaOh7nLM2IBY2kVnUfRzEeEjwE776ev19s2p4UgyM+lRTdiSQq 9H15NAUUUCBRgUVwtSUEO4ZF4fMVal1e5JbWKNAWX2wRQ4HoaGqcGYU8R72HPGFlXM27Z0jY1G9WE 68ia03u2j3uB263+qMmNSMpsl9wr6d1TeaT5eyCtbS2nCXwZ/G599wkGn037MX/xv+NwLha/PExkx WsRx5aQRw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsUZB-00072U-H1; Mon, 06 Jul 2020 17:06:05 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsUZ9-00071Y-3s for linux-arm-kernel@lists.infradead.org; Mon, 06 Jul 2020 17:06:04 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DD1A31FB; Mon, 6 Jul 2020 10:06:01 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 24A0A3F68F; Mon, 6 Jul 2020 10:05:59 -0700 (PDT) Date: Mon, 6 Jul 2020 18:05:57 +0100 From: Dave Martin To: "Paul E. McKenney" Subject: Re: [PATCH 18/18] arm64: lto: Strengthen READ_ONCE() to acquire when CLANG_LTO=y Message-ID: <20200706170556.GE10992@arm.com> References: <20200630173734.14057-1-will@kernel.org> <20200630173734.14057-19-will@kernel.org> <20200701170722.4rte5ssnmrn2uqzg@bakewell.cambridge.arm.com> <20200702072301.GA15963@willie-the-truck> <20200706160023.GB10992@arm.com> <20200706163455.GV9247@paulmck-ThinkPad-P72> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200706163455.GV9247@paulmck-ThinkPad-P72> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200706_130603_259636_6C3AB9D1 X-CRM114-Status: GOOD ( 42.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , "Michael S. Tsirkin" , Peter Zijlstra , Catalin Marinas , Jason Wang , virtualization@lists.linux-foundation.org, Will Deacon , Alan Stern , Sami Tolvanen , Matt Turner , kernel-team@android.com, Marco Elver , Kees Cook , Arnd Bergmann , Boqun Feng , Josh Triplett , Ivan Kokshaysky , linux-arm-kernel@lists.infradead.org, Richard Henderson , Nick Desaulniers , linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org 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 Mon, Jul 06, 2020 at 09:34:55AM -0700, Paul E. McKenney wrote: > On Mon, Jul 06, 2020 at 05:00:23PM +0100, Dave Martin wrote: > > On Thu, Jul 02, 2020 at 08:23:02AM +0100, Will Deacon wrote: > > > On Wed, Jul 01, 2020 at 06:07:25PM +0100, Dave P Martin wrote: > > > > On Tue, Jun 30, 2020 at 06:37:34PM +0100, Will Deacon wrote: > > > > > When building with LTO, there is an increased risk of the compiler > > > > > converting an address dependency headed by a READ_ONCE() invocation > > > > > into a control dependency and consequently allowing for harmful > > > > > reordering by the CPU. > > > > > > > > > > Ensure that such transformations are harmless by overriding the generic > > > > > READ_ONCE() definition with one that provides acquire semantics when > > > > > building with LTO. > > > > > > > > > > Signed-off-by: Will Deacon > > > > > --- > > > > > arch/arm64/include/asm/rwonce.h | 63 +++++++++++++++++++++++++++++++ > > > > > arch/arm64/kernel/vdso/Makefile | 2 +- > > > > > arch/arm64/kernel/vdso32/Makefile | 2 +- > > > > > 3 files changed, 65 insertions(+), 2 deletions(-) > > > > > create mode 100644 arch/arm64/include/asm/rwonce.h > > > > > > > > > > diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h > > > > > new file mode 100644 > > > > > index 000000000000..515e360b01a1 > > > > > --- /dev/null > > > > > +++ b/arch/arm64/include/asm/rwonce.h > > > > > @@ -0,0 +1,63 @@ > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > +/* > > > > > + * Copyright (C) 2020 Google LLC. > > > > > + */ > > > > > +#ifndef __ASM_RWONCE_H > > > > > +#define __ASM_RWONCE_H > > > > > + > > > > > +#ifdef CONFIG_CLANG_LTO > > > > > > > > Don't we have a generic option for LTO that's not specific to Clang. > > > > > > /me looks at the LTO series some more > > > > > > Oh yeah, there's CONFIG_LTO which is selected by CONFIG_LTO_CLANG, which is > > > the non-typoed version of the above. I can switch this to CONFIG_LTO. > > > > > > > Also, can you illustrate code that can only be unsafe with Clang LTO? > > > > > > I don't have a concrete example, but it's an ongoing concern over on the LTO > > > thread [1], so I cooked this to show one way we could deal with it. The main > > > concern is that the whole-program optimisations enabled by LTO may allow the > > > compiler to enumerate possible values for a pointer at link time and replace > > > an address dependency between two loads with a control dependency instead, > > > defeating the dependency ordering within the CPU. > > > > Why can't that happen without LTO? > > Because without LTO, the compiler cannot see all the pointers all at > the same time due to their being in different translation units. > > But yes, if the compiler could see all the pointer values and further > -know- that it was seeing all the pointer values, these optimizations > could happen even without LTO. But it is quite easy to make sure that > the compiler thinks that there are additional pointer values that it > does not know about. Yes of course, but even without LTO the compiler can still apply this optimisation to everything visible in the translation unit, and that can drift as people refactor code over time. Convincing the compiler there are other possible values doesn't help. Even in int foo(int *p) { asm ("" : "+r" (p)); return *p; } Can't the compiler still generate something like this: switch (p) { case &foo: return foo; case &bar: return bar; default: return *p; } ...in which case we still have the same lost ordering guarantee that we were trying to enforce. If foo and bar already happen to be in registers and profiling shows that &foo and &bar are the most likely value of p then this might be a reasonable optimisation in some situations, irrespective of LTO. The underlying problem here seems to be that the necessary ordering rule is not part of what passes for the C memory model prior to C11. If we want to control the data flow, don't we have to wrap the entire dereference in a macro? > > > We likely won't realise if/when this goes wrong, other than impossible to > > > debug, subtle breakage that crops up seemingly randomly. Ideally, we'd be > > > able to detect this sort of thing happening at build time, and perhaps > > > even prevent it with compiler options or annotations, but none of that is > > > close to being available and I'm keen to progress the LTO patches in the > > > meantime because they are a requirement for CFI. > > > > My concern was not so much why LTO makes things dangerous, as why !LTO > > makes things safe... > > Because ignorant compilers are safe compilers! ;-) AFAICT ignorance is no gurantee of ordering in general -- the compiler is free to speculatively invent knowledge any place that the language spec allows it to. !LTO doesn't stop this happening. Hopefully some of the knowledge I invented in my reply is valid... Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel