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 9FF0FC54EE9 for ; Tue, 20 Sep 2022 13:32:36 +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: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=kCI03ks7iNLtNXV2JTTGZtN0kIGodOgQNLG6cPd75nU=; b=n3hmzZ8U0hyp7E MyCuPa2eCV3ik89Cl9IqKrOCmkn3YtxnSelm0x6fUXFJm1yhdpDDJ5OzlKh9WyCXMvY5niFBfBEJC G88Kk2DYLBnjXQNu0PeI9ef8JUwooJYrN4+T86vNgUu29zyO1U2KXUaBMMBspoFLkYH0w05QneLgz vpWrO3oBJvlJiZZ1mLuH0JvfY0nykdhmF0SuXk7oxVqmSB4DGvXzV1TxdpwGh4lClMiHK7r0pxfOB /X17mfVhpCzhel7gxYlWn3BNXLR8mqXTV/l6Fk7a9OEp6HzetF26uf00GBxnHxcPOr3GIgReA+Ewq qoLpG24KKM/pNm0UCrYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oadLP-0046a3-4S; Tue, 20 Sep 2022 13:31:23 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oadLL-0046YZ-Dl for linux-arm-kernel@lists.infradead.org; Tue, 20 Sep 2022 13:31:21 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id EB266629B5; Tue, 20 Sep 2022 13:31:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C83DAC433C1; Tue, 20 Sep 2022 13:31:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1663680678; bh=F3hAN7VsAKea35iH5axTvSeh2ieoDiMwjetwOG0ISMY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KN4EFNzkfXXNvii6HTn/8WvmkvWtgm0DX4U730xYP31ywS+JUd9pLqvr1S/OQbuUs IB8MdzUVYaSsdWNYd7TYqdO3/JZUczc9MrYaffi5lgNs6wQ7etGu0Mk0olvhoHc7gQ aRcqBZJsPbRh8454nYXSWB0UenPlJjGRGBvXeAYX/IAKXc9/NGd/XZy/l3Se/0Mxv/ tF/S668jfDUln8zu/EvvTGPBCgygmF/veJEwXMN8PoncJeqfNi66Xmg+/tYRkOdJK+ jSxC00oxTH87HBOAmaibB5NQ+9+j7Yz3+6ZEKpsxoROntpjsvVcE+xsSs+0pCNKLZb rgGJ8jo2DtIuQ== Date: Tue, 20 Sep 2022 06:31:16 -0700 From: Nathan Chancellor To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, ardb@kernel.org, catalin.marinas@arm.com, james.morse@arm.com, joey.gouly@arm.com, maz@kernel.org, will@kernel.org, llvm@lists.linux.dev Subject: Re: [PATCH v2 7/8] arm64: alternatives: add alternative_has_feature_*() Message-ID: References: <20220912162210.3626215-1-mark.rutland@arm.com> <20220912162210.3626215-8-mark.rutland@arm.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-20220920_063119_574644_C4911ABA X-CRM114-Status: GOOD ( 38.95 ) 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 Tue, Sep 20, 2022 at 01:09:27PM +0100, Mark Rutland wrote: > On Mon, Sep 19, 2022 at 10:01:02AM -0700, Nathan Chancellor wrote: > > Hi Mark, > > Hi Nathan, > > > > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h > > > index 189c31be163ce..eaba9ec127897 100644 > > > --- a/arch/arm64/include/asm/alternative-macros.h > > > +++ b/arch/arm64/include/asm/alternative-macros.h > > > @@ -213,4 +213,45 @@ alternative_endif > > > #define ALTERNATIVE(oldinstr, newinstr, ...) \ > > > _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1) > > > > > > +#ifndef __ASSEMBLY__ > > > + > > > +#include > > > +#include > > > + > > > +static __always_inline bool > > > +alternative_has_feature_likely(unsigned long feature) > > > +{ > > > + BUILD_BUG_ON(feature >= ARM64_NCAPS); > > > + > > > + asm_volatile_goto( > > > + ALTERNATIVE("b %l[l_no]", "nop", %[feature]) > > > + : > > > + : [feature] "i" (feature) > > > + : > > > + : l_no); > > > + > > > + return true; > > > +l_no: > > > + return false; > > > +} > > > + > > > +static __always_inline bool > > > +alternative_has_feature_unlikely(unsigned long feature) > > > +{ > > > + BUILD_BUG_ON(feature >= ARM64_NCAPS); > > > + > > > + asm_volatile_goto( > > > + ALTERNATIVE("nop", "b %l[l_yes]", %[feature]) > > > + : > > > + : [feature] "i" (feature) > > > + : > > > + : l_yes); > > > + > > > + return false; > > > +l_yes: > > > + return true; > > > +} > > > + > > > +#endif /* __ASSEMBLY__ */ > > > + > > > #endif /* __ASM_ALTERNATIVE_MACROS_H */ > > > > The addition of BUILD_BUG_ON() in these functions ends up blowing up > > when building with clang + CONFIG_LTO: > > > > In file included from kernel/bounds.c:10: > > In file included from ./include/linux/page-flags.h:10: > > In file included from ./include/linux/bug.h:5: > > In file included from ./arch/arm64/include/asm/bug.h:26: > > In file included from ./include/asm-generic/bug.h:5: > > In file included from ./include/linux/compiler.h:248: > > In file included from ./arch/arm64/include/asm/rwonce.h:11: > > ./arch/arm64/include/asm/alternative-macros.h:224:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > BUILD_BUG_ON(feature >= ARM64_NCAPS); > > ^ > > ./arch/arm64/include/asm/alternative-macros.h:241:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] > > BUILD_BUG_ON(feature >= ARM64_NCAPS); > > ^ > > 2 errors generated. > > > > As you can see from the include chain, alternative-macros.h can end up > > being included from bug.h through compiler.h and rwonce.h, which will be > > before the definition of BUILD_BUG_ON() in build_bug.h, so it does not > > get expanded by the preprocessor. > > Sorry for this! Totally okay, these include chains have bit us before :) > > I could not come up with a clean solution other than moving these functions > > into their own header so that they do not appear in the alternative-macros.h > > include path (see below). Would that be acceptable (the file name is a > > placeholder, I could not come up with anything better) or do you have any > > other suggestions? > > I think that'd be ok; I'd suggest `alternative_branch.h` if we do that... > > > Another solution that might work is open coding these BUILD_BUG_ON() > > calls with compiletime_assert() directly, as that should be available at > > this point. > > ... but this sounds slightly nicer to me. We already use > compiletime_assert_atomic_type() elsewhere, so we're already relying on > compiletime_assert(), and the diff looks simple enough: This diff looks a lot better to me too. I build tested it against defconfig and allmodconfig, with and without CONFIG_LTO_CLANG_THIN and I saw no additional failures. If you send it along formally, please feel free to add: Tested-by: Nathan Chancellor > ---->8---- > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h > index 4a2a98d6d2227..966767debaa3a 100644 > --- a/arch/arm64/include/asm/alternative-macros.h > +++ b/arch/arm64/include/asm/alternative-macros.h > @@ -215,13 +215,13 @@ alternative_endif > > #ifndef __ASSEMBLY__ > > -#include > #include > > static __always_inline bool > alternative_has_feature_likely(unsigned long feature) > { > - BUILD_BUG_ON(feature >= ARM64_NCAPS); > + compiletime_assert(feature < ARM64_NCAPS, > + "feature must be < ARM64_NCAPS"); > > asm_volatile_goto( > ALTERNATIVE_CB("b %l[l_no]", %[feature], alt_cb_patch_nops) > @@ -238,7 +238,8 @@ alternative_has_feature_likely(unsigned long feature) > static __always_inline bool > alternative_has_feature_unlikely(unsigned long feature) > { > - BUILD_BUG_ON(feature >= ARM64_NCAPS); > + compiletime_assert(feature < ARM64_NCAPS, > + "feature must be < ARM64_NCAPS"); > > asm_volatile_goto( > ALTERNATIVE("nop", "b %l[l_yes]", %[feature]) > ---->8---- > > ... and seems to work in a local build test (with LLVM 14.0.0 and LTO_THIN). > > If that sounds ok to you I can spin that as a patch shortly. > > Thanks, > Mark. > > > > > Cheers, > > Nathan > > > > diff --git a/arch/arm64/include/asm/alternative-likely.h b/arch/arm64/include/asm/alternative-likely.h > > new file mode 100644 > > index 000000000000..f4e63d65e9eb > > --- /dev/null > > +++ b/arch/arm64/include/asm/alternative-likely.h > > @@ -0,0 +1,47 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __ASM_ALTERNATIVE_LIKELY_H > > +#define __ASM_ALTERNATIVE_LIKELY_H > > + > > +#ifndef __ASSEMBLY__ > > + > > +#include > > +#include > > +#include > > + > > +static __always_inline bool > > +alternative_has_feature_likely(unsigned long feature) > > +{ > > + BUILD_BUG_ON(feature >= ARM64_NCAPS); > > + > > + asm_volatile_goto( > > + ALTERNATIVE_CB("b %l[l_no]", %[feature], alt_cb_patch_nops) > > + : > > + : [feature] "i" (feature) > > + : > > + : l_no); > > + > > + return true; > > +l_no: > > + return false; > > +} > > + > > +static __always_inline bool > > +alternative_has_feature_unlikely(unsigned long feature) > > +{ > > + BUILD_BUG_ON(feature >= ARM64_NCAPS); > > + > > + asm_volatile_goto( > > + ALTERNATIVE("nop", "b %l[l_yes]", %[feature]) > > + : > > + : [feature] "i" (feature) > > + : > > + : l_yes); > > + > > + return false; > > +l_yes: > > + return true; > > +} > > + > > +#endif /* __ASSEMBLY__ */ > > + > > +#endif /* __ASM_ALTERNATIVE_LIKELY_H */ > > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h > > index 4a2a98d6d222..189c31be163c 100644 > > --- a/arch/arm64/include/asm/alternative-macros.h > > +++ b/arch/arm64/include/asm/alternative-macros.h > > @@ -213,45 +213,4 @@ alternative_endif > > #define ALTERNATIVE(oldinstr, newinstr, ...) \ > > _ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1) > > > > -#ifndef __ASSEMBLY__ > > - > > -#include > > -#include > > - > > -static __always_inline bool > > -alternative_has_feature_likely(unsigned long feature) > > -{ > > - BUILD_BUG_ON(feature >= ARM64_NCAPS); > > - > > - asm_volatile_goto( > > - ALTERNATIVE_CB("b %l[l_no]", %[feature], alt_cb_patch_nops) > > - : > > - : [feature] "i" (feature) > > - : > > - : l_no); > > - > > - return true; > > -l_no: > > - return false; > > -} > > - > > -static __always_inline bool > > -alternative_has_feature_unlikely(unsigned long feature) > > -{ > > - BUILD_BUG_ON(feature >= ARM64_NCAPS); > > - > > - asm_volatile_goto( > > - ALTERNATIVE("nop", "b %l[l_yes]", %[feature]) > > - : > > - : [feature] "i" (feature) > > - : > > - : l_yes); > > - > > - return false; > > -l_yes: > > - return true; > > -} > > - > > -#endif /* __ASSEMBLY__ */ > > - > > #endif /* __ASM_ALTERNATIVE_MACROS_H */ > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > > index ba8a47433b5a..147d326c4439 100644 > > --- a/arch/arm64/include/asm/cpufeature.h > > +++ b/arch/arm64/include/asm/cpufeature.h > > @@ -6,7 +6,7 @@ > > #ifndef __ASM_CPUFEATURE_H > > #define __ASM_CPUFEATURE_H > > > > -#include > > +#include > > #include > > #include > > #include > > diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h > > index c503db8e73b0..b28b5cb73387 100644 > > --- a/arch/arm64/include/asm/lse.h > > +++ b/arch/arm64/include/asm/lse.h > > @@ -13,7 +13,7 @@ > > #include > > #include > > #include > > -#include > > +#include > > #include > > #include > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel