Message ID | 20220929150227.1028556-1-mark.rutland@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: fix the build with binutils 2.27 | expand |
On 2022-09-29 16:02, Mark Rutland wrote: > Jon Hunter reports that for some toolchains the build has been broken > since commit: > > 4c0bd995d73ed889 ("arm64: alternatives: have callbacks take a cap") > > ... with a stream of build-time splats of the form: > > | CC arch/arm64/kvm/hyp/vhe/debug-sr.o > | /tmp/ccY3kbki.s: Assembler messages: > | /tmp/ccY3kbki.s:1600: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1600: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1600: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1600: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1600: Error: junk at end of line, first unrecognized character > | is `L' > | /tmp/ccY3kbki.s:1723: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1723: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1723: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1723: Error: found 'L', expected: ')' > | /tmp/ccY3kbki.s:1723: Error: junk at end of line, first unrecognized character > | is `L' > | scripts/Makefile.build:249: recipe for target > | 'arch/arm64/kvm/hyp/vhe/debug-sr.o' failed > > The issue here is that older versions of binutils (up to and including > 2.27.0) don't like an 'L' suffix on constants. For plain assembly files, > UL() avoids this suffix, but in C files this gets added, and so for > inline assembly we can't directly use a constant defined with `UL()`. > > We could avoid this by passing the constant as an input parameter, but > this isn't practical given the way we use the alternative macros. > Instead, just open code the constant without the `UL` suffix, and for > consistency do this for both the inline assembly macro and the regular > assembly macro. Couldn't we just remove the UL() and avoid inconsistently open-coding anything? TBH I fail to see why it's there in the first place - 1 << 15 is well within the range of int on arm64, and I don't see ARM64_CB_BIT being used in any wider arithmetic anywhere either :/ Thanks, Robin. > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Fixes: 4c0bd995d73ed889 ("arm64: alternatives: have callbacks take a cap") > Reported-by: Jon Hunter <jonathanh@nvidia.com> > Link: https://lore.kernel.org/linux-arm-kernel/3cecc3a5-30b0-f0bd-c3de-9e09bd21909b@nvidia.com/ > Tested-by: Jon Hunter <jonathanh@nvidia.com> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > --- > arch/arm64/include/asm/alternative-macros.h | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h > index 966767debaa3..b5ac0b85c9bb 100644 > --- a/arch/arm64/include/asm/alternative-macros.h > +++ b/arch/arm64/include/asm/alternative-macros.h > @@ -2,12 +2,18 @@ > #ifndef __ASM_ALTERNATIVE_MACROS_H > #define __ASM_ALTERNATIVE_MACROS_H > > +#include <linux/bits.h> > #include <linux/const.h> > > #include <asm/cpucaps.h> > #include <asm/insn-def.h> > > -#define ARM64_CB_BIT (UL(1) << 15) > +/* > + * Binutils 2.27.0 can't handle a 'UL' suffix on constants, so for the assembly > + * macros below we must use we must use `(1 << ARM64_CB_SHIFT)`. > + */ > +#define ARM64_CB_SHIFT 15 > +#define ARM64_CB_BIT BIT(ARM64_CB_SHIFT) > > #if ARM64_NCAPS >= ARM64_CB_BIT > #error "cpucaps have overflown ARM64_CB_BIT" > @@ -80,7 +86,7 @@ > __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg)) > > #define ALTERNATIVE_CB(oldinstr, feature, cb) \ > - __ALTERNATIVE_CFG_CB(oldinstr, ARM64_CB_BIT | (feature), 1, cb) > + __ALTERNATIVE_CFG_CB(oldinstr, (1 << ARM64_CB_SHIFT) | (feature), 1, cb) > #else > > #include <asm/assembler.h> > @@ -150,7 +156,7 @@ > .macro alternative_cb cap, cb > .set .Lasm_alt_mode, 0 > .pushsection .altinstructions, "a" > - altinstruction_entry 661f, \cb, ARM64_CB_BIT | \cap, 662f-661f, 0 > + altinstruction_entry 661f, \cb, (1 << ARM64_CB_SHIFT) | \cap, 662f-661f, 0 > .popsection > 661: > .endm
On Thu, 29 Sep 2022 16:02:27 +0100, Mark Rutland wrote: > Jon Hunter reports that for some toolchains the build has been broken > since commit: > > 4c0bd995d73ed889 ("arm64: alternatives: have callbacks take a cap") > > ... with a stream of build-time splats of the form: > > [...] Applied to arm64 (for-next/alternatives), thanks! [1/1] arm64: fix the build with binutils 2.27 https://git.kernel.org/arm64/c/ba00c2a04fa5
diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h index 966767debaa3..b5ac0b85c9bb 100644 --- a/arch/arm64/include/asm/alternative-macros.h +++ b/arch/arm64/include/asm/alternative-macros.h @@ -2,12 +2,18 @@ #ifndef __ASM_ALTERNATIVE_MACROS_H #define __ASM_ALTERNATIVE_MACROS_H +#include <linux/bits.h> #include <linux/const.h> #include <asm/cpucaps.h> #include <asm/insn-def.h> -#define ARM64_CB_BIT (UL(1) << 15) +/* + * Binutils 2.27.0 can't handle a 'UL' suffix on constants, so for the assembly + * macros below we must use we must use `(1 << ARM64_CB_SHIFT)`. + */ +#define ARM64_CB_SHIFT 15 +#define ARM64_CB_BIT BIT(ARM64_CB_SHIFT) #if ARM64_NCAPS >= ARM64_CB_BIT #error "cpucaps have overflown ARM64_CB_BIT" @@ -80,7 +86,7 @@ __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg)) #define ALTERNATIVE_CB(oldinstr, feature, cb) \ - __ALTERNATIVE_CFG_CB(oldinstr, ARM64_CB_BIT | (feature), 1, cb) + __ALTERNATIVE_CFG_CB(oldinstr, (1 << ARM64_CB_SHIFT) | (feature), 1, cb) #else #include <asm/assembler.h> @@ -150,7 +156,7 @@ .macro alternative_cb cap, cb .set .Lasm_alt_mode, 0 .pushsection .altinstructions, "a" - altinstruction_entry 661f, \cb, ARM64_CB_BIT | \cap, 662f-661f, 0 + altinstruction_entry 661f, \cb, (1 << ARM64_CB_SHIFT) | \cap, 662f-661f, 0 .popsection 661: .endm