linux-kernel - Re: [PATCH] kbuild, x86: revert macros in extended asm workarounds

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: <834EC537-EC42-4563-AA39-3326681ED968@vmware.com>
Date:   Mon, 17 Dec 2018 01:00:50 +0000
From:   Nadav Amit <namit@...are.com>
To:     Borislav Petkov <bp@...en8.de>
CC:     Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
        Richard Biener <rguenther@...e.de>,
        Logan Gunthorpe <logang@...tatee.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Segher Boessenkool <segher@...nel.crashing.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        "linux-sparse@...r.kernel.org" <linux-sparse@...r.kernel.org>,
        Alok Kataria <akataria@...are.com>,
        Juergen Gross <jgross@...e.com>,
        Andy Lutomirski <luto@...nel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH] kbuild, x86: revert macros in extended asm workarounds

> On Dec 16, 2018, at 2:00 AM, Borislav Petkov <bp@...en8.de> wrote:
> 
> On Sun, Dec 16, 2018 at 02:33:39AM +0000, Nadav Amit wrote:
>> In general, I think that from the start it was clear that the motivation for
>> the patch-set is not just performance and also better code. For example, I
>> see no reason to revert the PV-changes or the lock-prefix changes that
>> improved the code readability.
> 
> One thing that has caught my eye with the asm macros, which actually
> decreases readability, is that I can't see the macro properly expanded
> when I do
> 
> make <filename>.s
> 
> For example, I get
> 
> #APP
> # 164 "./arch/x86/include/asm/cpufeature.h" 1
>        STATIC_CPU_HAS bitnum=$8 cap_byte="boot_cpu_data+35(%rip)" feature=123 t_yes=.L75 t_no=.L78 always=117  #, MEM[(const char *)&boot_cpu_data + 35B],,,,
> # 0 "" 2
>        .loc 11 164 2 view .LVU480
> #NO_APP
> 
> but I'd like to see the actual asm as it is really helpful when hacking
> on inline asm stuff. And I haven't found a way to make gcc expand asm
> macros in .s output.

You’re right, although there were already 72 assembly macros for defined in
x86 .h files, and some may find the unexpanded macro in the ‘.s’ file more
friendly (well, a small comment in inline assembly could have resolved this
issue).

Anyhow, using gnu asm listings should be a relatively reasonable workaround
for this limitation (unless you want to hack the code before assembly).

For example, using ` as -alm arch/x86/kernel/macros.s arch/x86/kvm/vmx.s `

would give you:

 421                    # ./arch/x86/include/asm/cpufeature.h:164:      asm_volatile_goto("STATIC_CPU_HAS bitnum=%[bitnum] "
 422                            .file 8 "./arch/x86/include/asm/cpufeature.h"
 423                            .loc 8 164 0
 424                    #APP
 425                    # 164 "./arch/x86/include/asm/cpufeature.h" 1
 426                            STATIC_CPU_HAS bitnum=$2 cap_byte="boot_cpu_data+38(%rip)" feature=145 t_yes=.L17 t_no=.L18 always
 426                    > 1:
 426 00d8 E9000000      >  jmp 6f
 426      00
 426                    > 2:
 426                    >  .skip -(((5f-4f)- (2b-1b))>0)* ((5f-4f)- (2b-1b)),0x90
 426                    > 3:
 426                    >  .section .altinstructions,"a"
 426 000d 00000000      >  .long 1b - .
 426 0011 00000000      >  .long 4f - .
 426 0015 7500          >  .word 117

…

This can be incorporated into a makefile option, I suppose.


> Now, assuming the gcc inline patch will be backported to gcc8, I think
> we should be covered on all modern distros going forward. So I think we
> should revert at least the more complex macros.

I understand, and perhaps STATIC_CPU_HAS is not a good use-case (once
inlining is resolved in a different manner). I think that the main question
should be whether the whole infrastructure should be removed or to be
selective. 

In the case of exception tables, for instance, the result is much cleaner,
as it allows to consolidate the C and assembly implementations. There is an
alternative solution of turning the assembly macros into C macros, which
would make the Make system hacks go away, but would make the code not as
nice.