[PATCH] ARM: entry: add .ltorg directive to keep literals in range

[PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Ard Biesheuvel]

LKP reports a build issue on Clang, related to a literal load of
__current issued through the ldr_va macro. This turns out to be due to
the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y,
which means that the ldr_va macro resolves to a pair of LDR
instructions, the first one being a literal load issued too far from its
literal pool.

Due to the introduction of a couple of new uses of this macro in commit
508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"),
the literal pools end up getting rearranged in a way that causes the
literal for __current to go out of range. Let's fix this up by putting a
.ltorg directive in a suitable place in the code.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads")
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/kernel/entry-common.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 7aa3ded4af92..6a447ac67d80 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -298,14 +298,15 @@ local_restart:
 9001:
 	sub	lr, saved_pc, #4
 	str	lr, [sp, #S_PC]
 	get_thread_info tsk
 	b	ret_fast_syscall
 #endif
 ENDPROC(vector_swi)
+	.ltorg
 
 	/*
 	 * This is the really slow path.  We're going to be doing
 	 * context switches, and waiting for our parent to respond.
 	 */
 __sys_trace:
 	add	r0, sp, #S_OFF
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Nathan Chancellor]

On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote:
> LKP reports a build issue on Clang, related to a literal load of
> __current issued through the ldr_va macro. This turns out to be due to
> the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y,
> which means that the ldr_va macro resolves to a pair of LDR
> instructions, the first one being a literal load issued too far from its
> literal pool.

For the record, I do see a similar failure with GNU as, not sure if it
is worth calling that out in the commit message or not:

arch/arm/kernel/entry-common.S: Assembler messages:
arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer

> Due to the introduction of a couple of new uses of this macro in commit
> 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"),
> the literal pools end up getting rearranged in a way that causes the
> literal for __current to go out of range. Let's fix this up by putting a
> .ltorg directive in a suitable place in the code.
> 
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads")
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

With the problematic configuration, I no longer see issues with either
clang or GCC:

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/arm/kernel/entry-common.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 7aa3ded4af92..6a447ac67d80 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -298,14 +298,15 @@ local_restart:
>  9001:
>  	sub	lr, saved_pc, #4
>  	str	lr, [sp, #S_PC]
>  	get_thread_info tsk
>  	b	ret_fast_syscall
>  #endif
>  ENDPROC(vector_swi)
> +	.ltorg
>  
>  	/*
>  	 * This is the really slow path.  We're going to be doing
>  	 * context switches, and waiting for our parent to respond.
>  	 */
>  __sys_trace:
>  	add	r0, sp, #S_OFF
> -- 
> 2.30.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Ard Biesheuvel]

On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote:
> > LKP reports a build issue on Clang, related to a literal load of
> > __current issued through the ldr_va macro. This turns out to be due to
> > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y,
> > which means that the ldr_va macro resolves to a pair of LDR
> > instructions, the first one being a literal load issued too far from its
> > literal pool.
>
> For the record, I do see a similar failure with GNU as, not sure if it
> is worth calling that out in the commit message or not:
>
> arch/arm/kernel/entry-common.S: Assembler messages:
> arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer
>

Correct, the underlying issue is not specific to Clang. I did notice,
though, that the Clang assembler does not deduplicate literal pool
entries like GAS does, which might make Clang more susceptible than
GCC/binutils.

In any case, the fix is not Clang-specific.

> > Due to the introduction of a couple of new uses of this macro in commit
> > 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"),
> > the literal pools end up getting rearranged in a way that causes the
> > literal for __current to go out of range. Let's fix this up by putting a
> > .ltorg directive in a suitable place in the code.
> >
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> With the problematic configuration, I no longer see issues with either
> clang or GCC:
>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
>

Thanks.



> > ---
> >  arch/arm/kernel/entry-common.S | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> > index 7aa3ded4af92..6a447ac67d80 100644
> > --- a/arch/arm/kernel/entry-common.S
> > +++ b/arch/arm/kernel/entry-common.S
> > @@ -298,14 +298,15 @@ local_restart:
> >  9001:
> >       sub     lr, saved_pc, #4
> >       str     lr, [sp, #S_PC]
> >       get_thread_info tsk
> >       b       ret_fast_syscall
> >  #endif
> >  ENDPROC(vector_swi)
> > +     .ltorg
> >
> >       /*
> >        * This is the really slow path.  We're going to be doing
> >        * context switches, and waiting for our parent to respond.
> >        */
> >  __sys_trace:
> >       add     r0, sp, #S_OFF
> > --
> > 2.30.2
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Nick Desaulniers]

On Wed, Jun 1, 2022 at 4:36 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote:
> > > LKP reports a build issue on Clang, related to a literal load of
> > > __current issued through the ldr_va macro. This turns out to be due to
> > > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y,
> > > which means that the ldr_va macro resolves to a pair of LDR
> > > instructions, the first one being a literal load issued too far from its
> > > literal pool.
> >
> > For the record, I do see a similar failure with GNU as, not sure if it
> > is worth calling that out in the commit message or not:
> >
> > arch/arm/kernel/entry-common.S: Assembler messages:
> > arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer
> >
>
> Correct, the underlying issue is not specific to Clang. I did notice,
> though, that the Clang assembler does not deduplicate literal pool
> entries like GAS does, which might make Clang more susceptible than
> GCC/binutils.

Thanks for the report; just circling back on this, but this should be
resolved in ToT (clang-15):
https://github.com/llvm/llvm-project/issues/55816

>
> In any case, the fix is not Clang-specific.

-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Ard Biesheuvel]

On Wed, 8 Jun 2022 at 20:42, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Jun 1, 2022 at 4:36 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote:
> > > > LKP reports a build issue on Clang, related to a literal load of
> > > > __current issued through the ldr_va macro. This turns out to be due to
> > > > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y,
> > > > which means that the ldr_va macro resolves to a pair of LDR
> > > > instructions, the first one being a literal load issued too far from its
> > > > literal pool.
> > >
> > > For the record, I do see a similar failure with GNU as, not sure if it
> > > is worth calling that out in the commit message or not:
> > >
> > > arch/arm/kernel/entry-common.S: Assembler messages:
> > > arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer
> > >
> >
> > Correct, the underlying issue is not specific to Clang. I did notice,
> > though, that the Clang assembler does not deduplicate literal pool
> > entries like GAS does, which might make Clang more susceptible than
> > GCC/binutils.
>
> Thanks for the report; just circling back on this, but this should be
> resolved in ToT (clang-15):
> https://github.com/llvm/llvm-project/issues/55816
>

Wow, that was quick!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel