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 BA11AC47DDB for ; Thu, 1 Feb 2024 04:56:13 +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-Type: Content-Transfer-Encoding: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=/1GA535k1rlQ3UTM0lSyA+qQaeDt1gTRB37HsGqeDyg=; b=RV2kTQD2GWTkH1KyAEcBwps3iP pSm3dxmDWCwbljUGm3Lr33Un8V4C0Md0QBHmNRScDvdd/vW1ka8feCFU+QB0w9LjBAjJUSSin3lyL kKqK60VEqffrJi54EtCjhZ4o6grcDIo1zOnqYoPKqtCfE4SyVaP3yXJduufMlpuslZPyAI9MEz0Sa dhtgEJLzTq8HPx6snjqyYc5uR6KMiNbxaATQpJexL5iNRbbta5d6cUdh2OXRA92PezxgVOsHyv/wO 0JAmrO3vg+GGFIpMzzahoJ13k9ytuW2RIvMYQU2fvJkRVARJ5SXepD002vlQrWxUdye9R/L4pc5cg XgEQWTXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVP7J-00000006ZxT-31jN; Thu, 01 Feb 2024 04:56:01 +0000 Received: from mail-pl1-x62e.google.com ([2607:f8b0:4864:20::62e]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rVP7G-00000006Zww-43D4 for linux-arm-kernel@lists.infradead.org; Thu, 01 Feb 2024 04:56:00 +0000 Received: by mail-pl1-x62e.google.com with SMTP id d9443c01a7336-1d94691de1eso54495ad.1 for ; Wed, 31 Jan 2024 20:55:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706763356; x=1707368156; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=YsqrP80NBPWxBx78FsktitHzUN/DGz3kT5ZrnUUMNWc=; b=HLX2LI4Wwk6w9LNSjL4vqFv/9TlCigXuonfJfoA2CRXcVSdZEooMk0i6ozAdnAbw9m NCS6UVbJeFengwq1QltQrQG+6DYxLHxo29rPGaDVjI3p59wBxAUZBEJOfWEP0b2UrQyI 9sabPb6ThC0LEriHHoD41HL1aAUxwNniTGzWamQdfCWO/Xkse7QdiPvvBfOWFyEKVA9K xzhw48Aj8kNzsbhn66Ch9ZBs4axQBfFISxwezVZMUlBHL2QhsNY7fopNw9MnYZX6WWxU tV/IFwFRGjIy5XaNcrcRdSnWMlNWEvKGvuk5UkmTetpxFrNzS4704MnWjK7PKxGY5Ff/ Awwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706763356; x=1707368156; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=YsqrP80NBPWxBx78FsktitHzUN/DGz3kT5ZrnUUMNWc=; b=rCpXIWg7XqiV5zArm7WbmKUwkewmHNZmBZ8H6ZycWvbAmEpy19n0Wg30N6L9G+/U3t MFwaKqU/h0zuTxA6d9GfAWCc4ya+hBwrZ1WxnILJkb4RJtCPsWuX/ht918KEBxmK48ZS bUayFj8viuEMGTiKNaE1tpTly6Bo0KDoSwGNCxcHeGoBIakVAJsB26he5Huz2z3zhEFo 0yyrH0PXaZn740A2LFIo3wR+B/9WjNtQck8mwjrE2Zq8o9FIOWZOzjRCwdXcnzcL1Zkh 2ssXOXetAatabII0cFeekdOqQvgNhUlOJR8C9ga47YqtGM/bT/VpcRAlq1klhI162tOh lPuA== X-Gm-Message-State: AOJu0YwxmbieweCVSpt4Y26/BxRserCHwtqhWCqRT3qopHF2Bg1WG9M1 ZKX95S2xMGvSMKrAcpxIGxNGx+l1iEqH7sKw2/yGgHJ7LFXYWZwqJ1F9Olbq1g== X-Google-Smtp-Source: AGHT+IHK1/WgkjIKx0flbjpFflqU9a56/6fl9CXpl7gOvyu5gmDaW1mxugbmgLgncwtMdAWnWaNzZg== X-Received: by 2002:a17:902:d58f:b0:1d8:f3c7:fb83 with SMTP id k15-20020a170902d58f00b001d8f3c7fb83mr106370plh.9.1706763356049; Wed, 31 Jan 2024 20:55:56 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCV4Q548nt1ArvLGL2wR2E8vSlBntuR737WWtXt371mCiALATgckvigQx4dwtoGKhCMAatv2D8bgdgK02B+fOCaMn4Bf4YU8F27jYiuApzRhepfS+rHhUyzbSLKP8LIRnPbZFUD1yS1BWQL8Ewyk6oZDXFNulca7j+6S4Gs6tejvqqNk3CHRnFK3eJwLrkzePpZcAvG5g6PVOUh4lIGl5HItVTfnNu63wYd+qACW2eiF+PqyotQ8L71W+jKKENG7//L4f9hcdxI= Received: from google.com ([2620:15c:2d3:205:ce95:ce9d:3dd2:5053]) by smtp.gmail.com with ESMTPSA id o4-20020a170902d4c400b001d8a5c08277sm9616689plg.260.2024.01.31.20.55.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jan 2024 20:55:55 -0800 (PST) Date: Wed, 31 Jan 2024 20:55:51 -0800 From: Fangrui Song To: Dave Martin Cc: Ard Biesheuvel , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Jisheng Zhang , llvm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] arm64: jump_label: use constraint "S" instead of "i" Message-ID: <20240201045551.ajg4iqcajyowl2rh@google.com> References: <20240131065322.1126831-1-maskray@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240131_205559_033990_538342BE X-CRM114-Status: GOOD ( 34.41 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2024-01-31, Dave Martin wrote: >On Wed, Jan 31, 2024 at 08:16:04AM +0100, Ard Biesheuvel wrote: >> Hello Fangrui, >> >> On Wed, 31 Jan 2024 at 07:53, Fangrui Song wrote: >> > >> > The constraint "i" seems to be copied from x86 (and with a redundant >> > modifier "c"). It works with -fno-PIE but not with -fPIE/-fPIC in GCC's >> > aarch64 port. > >(I'm not sure of the exact history, but the "c" may be inherited from >arm, where an output modifier was needed to suppress the "#" that >prefixes immediates in the traditional asm syntax. This does not >actually seem to be required for AArch64: rather while a # is allowed >and still considered good style in handwritten asm code, the syntax >doesn't require it, and the compiler doesn't emit it for "i" arguments, >AFAICT.) The aarch64 one could be inherited from arch/arm/include/asm/jump_label.h (2012), which could in turn be inherited from x86 (2010). Both the constraint "i" and the modifier "c" are generic.. For -fno-pic this combination can be used for every arch. >> > The constraint "S", which denotes a symbol reference (e.g. function, >> > global variable) or label reference, is more appropriate, and has been >> > available in GCC since 2012 and in Clang since 7.0. >> > >> > Signed-off-by: Fangrui Song >> > Link: https://maskray.me/blog/2024-01-30-raw-symbol-names-in-inline-assembly >> > --- >> > arch/arm64/include/asm/jump_label.h | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h >> > index 48ddc0f45d22..31862b3bb33d 100644 >> > --- a/arch/arm64/include/asm/jump_label.h >> > +++ b/arch/arm64/include/asm/jump_label.h >> > @@ -23,9 +23,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 - . \n\t" >> > " .popsection \n\t" >> > - : : "i"(&((char *)key)[branch]) : : l_yes); >> > + : : "S"(&((char *)key)[branch]) : : l_yes); >> >> 'key' is not used as a raw symbol name. We should make this >> >> " .quad %0 + %1 - ." >> >> and >> >> :: "S"(key), "i"(branch) :: l_yes); >> >> if we want to really clean this up. > >This hides more logic in the asm so it's arguably more cryptic >(although the code is fairly cryptic to begin with -- I don't really >see why the argument wasn't written as the equivalent >(char *)key + branch...) I agree that using "S" and "i" would introduce complexity. Using just "S" as this patch does should be clear. All of "i" "s" "S" support a symbol or label reference and a constant offset (can be zero), (in object file, a symbol and an addend; in GCC's term, the sum of a SYMBOL_REF and a CONST_INT). >Anyway, I don't think the "i" versys "S" distinction makes any >difference without -fpic or equivalent, so it is not really relevant >for the kernel (except that "S" breaks compatibility with older >compilers...) > > >I think the main advantage of "S" is that it stops you accidentally >emitting undesirable relocations from asm code that is not written for >the -fpic case. > >But just changing "i" to "S" is not sufficient to port asms to -fpic: >the asms still need to be reviewed. > > >So unless the asm has been reviewed for position-independence, it may >anyway be better to stick with "i" so that the compiler actually chokes >if someone tries to build the code with -fpic. The asm is position-independent. This `.long sym - .` is a common metadata section pattern to support PIC:) Regarding the constraints, I've updated https://maskray.me/blog/2024-01-30-raw-symbol-names-in-inline-assembly to include more details. >Since we are not trying to run arbitraily many running kernels in a >common address space (and not likely to do that), I'm not sure that we >would ever build the kernel with -fpic except for a few special-case >bits like the EFI stub and vDSO... unless I've missed something? > >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. I agree that current -fno-PIE with -shared -Bsymbolic linking is a hack that works as a conincidence, not guaranteed by the toolchain. This jump_label improvement (with no object file difference) fixes an obstacle. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel