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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id F3A71C4828D for ; Thu, 1 Feb 2024 16:08:28 +0000 (UTC) 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:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UXpr4fvH2cd/MMJUMcgFMWk2S9ByiYAOSY4kaA0It0k=; b=fVBWpvcn+9QCR8 sEqLiFc+5n+snkFJAfMbBSEurvfyVOAEBglmuaCDs9uF20H3SDNtYMlNwKFjY3MTH+iXtzMKbvRuT oDPXvq3UbPXcPuAzfW4MCwhRDWDb0c2LNlMLaeHbEj1SKXczB0yOk/KeQK+cCy8LBF//firVLpmhO FUaur2jkH3MgWamcHhJvYlIPWBVPai/3ZYaZWuyKOtlkbGRav9yYHB3mpd9kN62OyiFy4wnk4jfx6 s0k5E2nVJ5JaswIVLwf/MGaEiKuWEpZ8n7vK9if8sLFV058/lLxnzkvrgWICvOjm7br6ZiIWISL+y VpGIgbWEVNwu3TnxZExg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVZbs-00000008Yks-2iPf; Thu, 01 Feb 2024 16:08:16 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVZbq-00000008YkX-22TW for linux-arm-kernel@lists.infradead.org; Thu, 01 Feb 2024 16:08:15 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0EC6161F80 for ; Thu, 1 Feb 2024 16:08:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AE7ABC433C7 for ; Thu, 1 Feb 2024 16:08:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706803692; bh=IQySAzz9Zu8aTmpkZztHBF6bTqnVGs1D4j6xRTD+u78=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=cUUzmHEO/igmXwIC/qHOSTaGa/XGD4ablIU+fzN4OHDEgma6p7JHRgSYAk4+Ya3n2 UFyxi6U3C16UEPnSlA+5BFr0BjEXOz5/iqYyRL/jgRJDrjEiNg9jmagTUmeIGAEWRF Eyw9ox/rk8oMtd5CqeN7vAOqfr0BVEQr2noapMR8hhyMOFP1MSaWkM+jWaRU+AtazA rF4lZo2FRJpbpBbe+MRNjcY2aUckFRykLeDMKsr9fbp9Vs0bwnABHdPCEk0YsmDdCd /59/HOaw4muVD+1b1WumiFslSCRt+7tgXW1TuYe++sjdKyHs4eTiLnxA6quYvB44lR u6VyoRx774uFg== Received: by mail-lj1-f178.google.com with SMTP id 38308e7fff4ca-2cf33b909e8so21677771fa.0 for ; Thu, 01 Feb 2024 08:08:12 -0800 (PST) X-Gm-Message-State: AOJu0YxXgcXWe8RrwhGWE3J0PwfH1QRYjDXZ4Bl2/pK2j8XilAayeYbY tU04efzyyWycjBu6l4myRrq5C3u/7O6Wh4iT5eo9Iuld3n1eUQkcLXGWy1vI+/tD2/in3pxDY5t 6Hao+OdNlRb3ygYpVp5N/lSKSkhs= X-Google-Smtp-Source: AGHT+IE3Hk5cExk7Fw6yQzo0QX0tf+LAr5EK/c/iR5/9+KNCkLRMPottyLcZvkC3HNIjYaQ4HPMi76Ysya0jkxhojRs= X-Received: by 2002:a05:651c:157:b0:2cd:2771:a2fb with SMTP id c23-20020a05651c015700b002cd2771a2fbmr2851945ljd.2.1706803690871; Thu, 01 Feb 2024 08:08:10 -0800 (PST) MIME-Version: 1.0 References: <20240131065322.1126831-1-maskray@google.com> <20240201045551.ajg4iqcajyowl2rh@google.com> <20240201091120.pbgr7ng6t2c36fut@google.com> In-Reply-To: From: Ard Biesheuvel Date: Thu, 1 Feb 2024 17:07:59 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] arm64: jump_label: use constraint "S" instead of "i" To: Dave Martin Cc: Fangrui Song , Peter Smith , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Jisheng Zhang , llvm@lists.linux.dev, linux-kernel@vger.kernel.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240201_080814_644245_7ECB0451 X-CRM114-Status: GOOD ( 50.45 ) 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 Thu, 1 Feb 2024 at 12:50, Dave Martin wrote: > > On Thu, Feb 01, 2024 at 01:11:20AM -0800, Fangrui Song wrote: ... > > > > Hmm. Clang's constraint "S" implementation doesn't support a constant > > offset, so > > `static_key_false(&nf_hooks_needed[pf][hook])` in include/linux/netfilter.h:nf_hook > > would not compile with Clang <= 18. > > > > I have a patch https://github.com/llvm/llvm-project/pull/80255 , but > > even if it is accepted and cherry-picked into the 18.x release branch, > > if we still support older Clang, we cannot use "S" unconditionally. > > > > > > So we probably need the following to prepare for -fPIE support in the > > future: > > > > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h > > index 48ddc0f45d22..b8af2f8b0c99 100644 > > --- a/arch/arm64/include/asm/jump_label.h > > +++ b/arch/arm64/include/asm/jump_label.h > > @@ -15,6 +15,16 @@ > > #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE > > +/* > > + * Prefer "S" to support PIC. However, use "i" for Clang 18 and earlier as "S" > > + * on a symbol with a constant offset is not supported. > > + */ > > +#if defined(CONFIG_CC_IS_CLANG) && __clang_major__ <= 18 > > Is a GCC version check needed? Or is the minimum GCC version specified > for building the kernel new enough? > > > +#define JUMP_LABEL_STATIC_KEY_CONSTRAINT "i" > > +#else > > +#define JUMP_LABEL_STATIC_KEY_CONSTRAINT "S" > > +#endif > > + Can we use "Si" instead? > > static __always_inline bool arch_static_branch(struct static_key * const key, > > const bool branch) > > { > > @@ -23,9 +33,9 @@ static __always_inline bool arch_static_branch(struct static_key * const key, > > " .pushsection __jump_table, \"aw\" \n\t" > > " .align 3 \n\t" > > " .long 1b - ., %l[l_yes] - . \n\t" > > - " .quad %c0 - . \n\t" > > + " .quad %0 + %1 - . \n\t" > > " .popsection \n\t" > > - : : "i"(&((char *)key)[branch]) : : l_yes); > > + : : JUMP_LABEL_STATIC_KEY_CONSTRAINT(key), "i"(branch) : : l_yes); > > While this looks reasonable, I'm still not clear now why the asm must do > the addition. > > Can we roll the branch argument into the macro? > > "S"(symbol + constant) seems to do the right thing for AArch64, at least > in GCC. > 'symbol' is a struct static_key pointer, so adding '1' to it will not produce the needed result. ... > > > > >If there's another reason why "S" is advantageous though, I'm happy to > > > > >be corrected. > > > > > > > > I remember that Ard has an RFC > > > > https://lore.kernel.org/linux-arm-kernel/20220427171241.2426592-1-ardb@kernel.org/ > > > > "[RFC PATCH 0/2] arm64: use PIE code generation for KASLR kernel" > > > > and see some recent PIE codegen patches. > > > > > > > > > Building the KASLR kernel without -fpie but linking it with -pie works > > > > > in practice, but it is not something that is explicitly supported by the > > > > > toolchains - it happens to work because the default 'small' code model > > > > > used by both GCC and Clang relies mostly on ADRP+ADD/LDR to generate > > > > > symbol references. > > Does this mean that doing boot-time relocation on a fully statically > linked kernel doesn't bring much benefit? Not sure I follow you here. The boot-time relocation is necessary in any case, to fix up statically initialized pointer variables all over the kernel. > It would tend to be more > painful to do the relocation work, and mean that the kernel text has to > be temporarily writeable, but static linking should have the best > runtime performance. > Not sure what you mean by 'static linking' here. The only thing PIE linking does in the case of the kernel is a) complain if static relocations are used that cannot be fixed up at runtime (e.g., movk/movz sequences) b) emit dynamic relocations that the early startup code can use to fix up all statically initialized pointers >From a codegen point-of-view, there is no difference because we don't use -fpic code. The problem with -fpic codegen is that it makes some assumptions that only hold for shared ELF objects, e.g., avoiding dynamic relocations in .text, using GOT entries even for variables defined in the same compilation so that they can be preempted, keeping all runtime relocatable quantities together so the CoW footprint of the shared library is minimized. None of this matters for the kernel, and as it turns out, the non-PIC small C model produces code that the PIE linker is happy with. TL;DR our code (including inline and out-of-line asm) is already PIC, and this property is relied upon by KASLR. > Maybe it doesn't make as much difference as I thought (certainly ADRP > based addressing should make a fair amount of the PIC/PIE overhead go > away). > The PIC/PIE overhead is in the indirections via the GOT. However, recent linkers will actually perform some relaxations too: if a symbol is defined in the same executable and is not preemptible, a GOT load ADRP LDR can be transparently converted to ADRP ADD referring to the symbol directly. This is also where hidden.h comes in: making all symbol declarations and definitions 'hidden' will allow the compiler to assume that a GOT entry is never needed. So building with -fPIC is currently not need in practice, and creates some complications, which is why we have been avoiding it. But PIE linking non-PIC objects is not guaranteed to remain supported going forward, so it would be better to have a plan B, i.e., being able to turn on -fpic without massive regressions due to GOT overhead, or codegen problems with our asm hacks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel