linux-kernel - Re: [RFC PATCH 3/4] x86/asm: Make alternative macro interfaces more clear and consistent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170915172052.5kmtzjk44dywk2k6@treble>
Date:   Fri, 15 Sep 2017 12:20:52 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Andrey Ryabinin <aryabinin@...tuozzo.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...nel.org>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitriy Vyukov <dvyukov@...gle.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC PATCH 3/4] x86/asm: Make alternative macro interfaces more
 clear and consistent

On Fri, Sep 15, 2017 at 07:53:46PM +0300, Andrey Ryabinin wrote:
> 
> 
> On 08/31/2017 08:25 PM, Josh Poimboeuf wrote:
> >
> > There have been a few other ideas which have *almost* worked:
> > 
> > 1) Make the 'register void *__sp asm(_ASM_SP)' a global variable instead
> >    of a local one.  This works for GCC and doesn't break clang.  However
> >    it resulted in a lot of changed code on the GCC side.  It looked like
> >    some optimizations had been disabled, even in functions which
> >    shouldn't have been affected.
> > 
> 
> (For those who wasn't following previous discussion there is the patch - http://lkml.kernel.org/r/<75850bb7-a60e-057d-d88b-afa0c79e94a0@...il.com>
> But, Josh discovered that the patch causes regression in .text size with gcc > 7.x
> http://lkml.kernel.org/r/<20170721132452.ihpws67e3e7ym3al@...ble> )
> 
> I'm not so sure that this is disabled optimization. I assume that global rsp makes
> changes something in gcc's register allocation logic, or something like that leading
> to subtle changes in generated code.
> 
> But what I recently find out, is that this "regression" sometimes is actually improvement in .text size.
> It all depends on .config, e.g:
> 
> 
> allnoconfig:
>                text    data     bss     dec     hex filename
> patched	       938094  207700 1215752 2361546  2408ca allnoconfig_p/vmlinux
> unpatched      938768  211796 1215752 2366316  241b6c allnoconfig/vmlinux                                                                                                                                                         
> 
> 
> defconfig:
>                text    data     bss     dec     hex filename
> patched	       10691469        4882856  876544 16450869         fb0535 defconfig_p/vmlinux
> unpatched      10691035        4882856  876544 16450435         fb0383 defconfig/vmlinux
> 
> allyesconfig:
>                text    data     bss     dec     hex filename
> patched	       159216239       153294411       49692672        362203322       1596c8ba        allyesconfig_p/vmlinux
> unpatched      159224951       153294387       49692672        362212010       1596eaaa        allyesconfig/vmlinux
> 
> 
> So there is regression with Josh's config and defconfig, but allnoconfig and allyesconfig shows improvement.
> Note that sometimes there is the difference in .data size too, don't ask me why.
> 
> Given that, perhaps the best thing to do here would be asking gcc devs why is generated code changed.
> And if such changes are harmless (as I suspect) we can just fix the problem with this simple patch http://lkml.kernel.org/r/<75850bb7-a60e-057d-d88b-afa0c79e94a0@...il.com> ?
> Despite the increased .text on some configs.

Can you change all the other code sites in a similar way, and then run
the numbers again?

  $ git grep __sp |grep register |grep void
  arch/x86/include/asm/alternative.h:	register void *__sp asm(_ASM_SP);				      \
  arch/x86/include/asm/mshyperv.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/mshyperv.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/paravirt_types.h:	register void *__sp asm("esp")
  arch/x86/include/asm/paravirt_types.h:	register void *__sp asm("rsp")
  arch/x86/include/asm/preempt.h:	register void *__sp asm(_ASM_SP);			\
  arch/x86/include/asm/preempt.h:	register void *__sp asm(_ASM_SP);				\
  arch/x86/include/asm/processor.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/rwsem.h:	register void *__sp asm(_ASM_SP);		\
  arch/x86/include/asm/uaccess.h:	register void *__sp asm(_ASM_SP);				\
  arch/x86/include/asm/xen/hypercall.h:	register void *__sp asm(_ASM_SP);
  arch/x86/kvm/emulate.c:	register void *__sp asm(_ASM_SP);
  arch/x86/kvm/vmx.c:	register void *__sp asm(_ASM_SP);
  arch/x86/mm/fault.c:		register void *__sp asm("rsp");

-- 
Josh