Re: [PATCH 4.4 05/53] x86/asm: Use register variable to get stack pointer value

Greg Kroah-Hartman
2018-02-06T22:37:09Z

On Tue, Feb 06, 2018 at 01:59:41PM -0800, Matthias Kaehlcke wrote:
> Hi Greg,
>
> El Mon, Jan 22, 2018 at 09:39:57AM +0100 Greg Kroah-Hartman ha dit:
>
> > 4.4-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Andrey Ryabinin <aryabinin@virtuozzo.com>
> >
> > commit 196bd485ee4f03ce4c690bfcf38138abfcd0a4bc upstream.
> >
> > Currently we use current_stack_pointer() function to get the value
> > of the stack pointer register. Since commit:
> >
> > f5caf621ee35 ("x86/asm: Fix inline asm call constraints for Clang")
> >
> > ... we have a stack register variable declared. It can be used instead of
> > current_stack_pointer() function which allows to optimize away some
> > excessive "mov %rsp, %<dst>" instructions:
> >
> > -mov %rsp,%rdx
> > -sub %rdx,%rax
> > -cmp $0x3fff,%rax
> > -ja ffffffff810722fd <ist_begin_non_atomic+0x2d>
> >
> > +sub %rsp,%rax
> > +cmp $0x3fff,%rax
> > +ja ffffffff810722fa <ist_begin_non_atomic+0x2a>
> >
> > Remove current_stack_pointer(), rename __asm_call_sp to current_stack_pointer
> > and use it instead of the removed function.
> >
> > Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> > Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Link: http://lkml.kernel.org/r/20170929141537.29167-1-aryabinin@virtuozzo.com
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > [dwmw2: We want ASM_CALL_CONSTRAINT for retpoline]
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.ku>
> > Signed-off-by: Razvan Ghitulete <rga@amazon.de>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> We recently merged this patch to the Chrome OS kernel tree and it
> broke our x86 builds with clang:
>
> arch/x86/include/asm/asm.h:116:50: error: register 'rsp' unsuitable for global register variables on this target
> register unsigned long current_stack_pointer asm(_ASM_SP);
> ^
> arch/x86/include/asm/asm.h:41:18: note: expanded from macro '_ASM_SP'
> #define _ASM_SP __ASM_REG(sp)
> ^
> arch/x86/include/asm/asm.h:24:32: note: expanded from macro '__ASM_REG'
> #define __ASM_REG(reg) __ASM_SEL_RAW(e##reg, r##reg)
> ^
> arch/x86/include/asm/asm.h:19:29: note: expanded from macro '__ASM_SEL_RAW'
> # define __ASM_SEL_RAW(a,b) __ASM_FORM_RAW(b)
> ^
> arch/x86/include/asm/asm.h:10:32: note: expanded from macro '__ASM_FORM_RAW'
> # define __ASM_FORM_RAW(x) #x
> ^
> <scratch space>:4:1: note: expanded from here
> "rsp"
> ^
> 1 error generated.
>
>
> This can be fixed by also integrating the following patch:
>
> commit 520a13c530aeb5f63e011d668c42db1af19ed349
> Author: Josh Poimboeuf <jpoimboe@redhat.com>
> Date: Thu Sep 28 16:58:26 2017 -0500
>
> x86/asm: Fix inline asm call constraints for GCC 4.4
>
>
> Admittedly a v4.4 kernel built with clang + LTS merges is a very
> special case and we can fix this in Chrome OS by integrating the above
> patch locally. Still it would be good to get it into stable to avoid
> others from running into this, especially since the fix is very
> simple.
>
> Actually I just noticed that the patch also isn't in v4.9, which could
> extend the number of affected 'users' significantly, so I think we
> almost certainly want Josh's patch in stable.

That patch doesn't apply cleanly to the 4.4.y or 4.9.y trees anymore.
It seems that only one hunk of it is really needed, the #ifndef change,
right? If so, I'll be glad to apply that portion.

thanks,

greg k-h