linux-kernel - Re: [PATCH V4 0/7] x86/entry: Clean up entry code

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: <CAJhGHyAbggnqcKRoL90sO6i8vG2o04Po_s_S92a6xTNfWVWHVw@mail.gmail.com>
Date:   Wed, 6 Apr 2022 23:57:02 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     LKML <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        Lai Jiangshan <jiangshan.ljs@...group.com>
Subject: Re: [PATCH V4 0/7] x86/entry: Clean up entry code

Hello, Borislav

Could you please review it again? The patches can still be applied
perfectly with the newest tip/master 7bcafc1e843a ("Merge x86/cpu
into tip/master") and work well.  And the patches almost have
nothing changed since your last review except only squashing and
reordering.


Hello, tglx

I’d like to hear your views on the patches.  It is a part of the
patchset which converts ASM code to C code which I think is a nice
foil for your previous excellent x86/entry work.  It reduces the
ASM code without any functionality change and makes entry code more
readable and maintainable.  I came up with the idea when I was
reviewing your patches.


Thanks
Lai


On Fri, Mar 18, 2022 at 10:29 PM Lai Jiangshan <jiangshanlai@...il.com> wrote:
>
> From: Lai Jiangshan <jiangshan.ljs@...group.com>
>
> This patchset moves the stack-switch code to the place where
> error_entry() return, unravels error_entry() from XENpv and makes
> entry_INT80_compat use idtentry macro.
>
> This patchset is highly related to XENpv, because it does the extra
> cleanup to convert SWAPGS to swapgs after major cleanup is done.
>
> The patches are the 4th version to pick patches from the patchset
> https://lore.kernel.org/lkml/20211126101209.8613-1-jiangshanlai@gmail.com/
> which converts ASM code to C code.  These patches are prepared for that
> purpose.  But this patchset has it own value: it simplifies the stack
> switch, avoids leaving the old stack inside a function call, and
> separates XENpv code with native code without adding new code.
>
> Peter said in V3:
> >       So AFAICT these patches are indeed correct.
> >
> >       I'd love for some of the other x86 people to also look at this,
> >       but a tentative ACK on this.
>
> Other interactions in V3:
>         Peter raised several questions and I think I answered them and I
>         don't think the code need to be updated unless I missed some
>         points. (Except reordering the patches)
>
>         Josh asked to remove UNWIND_HINT_REGS in patch5, but I think
>         UNWIND_HINT_REGS is old code before this patchset and I don't
>         want to do a cleanup that is not relate to preparing converting
>         ASM code C code in this patchset.  He also asked to remove
>         ENCODE_FRAME_POINTER in xenpv case, and I think it just
>         complicates the code for just optimizing out a single assignment
>         to %rbp.  I would not always stick to these reasons of mine,
>         but I just keep the code unchanged since he hasn't emphasized it
>         again nor other people has requested it.
>
> Changed from V3:
>         Only reorder the int80 thing as the last patch to make patches
>         ordering more natural. (Both orders are correct)
>
> Changed from V2:
>         Make the patch of folding int80 thing as the first patch
>         Add more changelog in "Switch the stack after error_entry() returns"
>
> Changed from V1
>         Squash cleanup patches converting SWAPGS to swapgs into one patch
>
>         Use my official email address (Ant Group).  The work is backed
>         by my company and I was incorrectly misunderstood that
>         XXX@...ux.alibaba.com is the only portal for opensource work
>         in the corporate group.
>
> [V3]: https://lore.kernel.org/lkml/20220315073949.7541-1-jiangshanlai@gmail.com/
> [V2]: https://lore.kernel.org/lkml/20220303035434.20471-1-jiangshanlai@gmail.com/
> [V1]: https://lore.kernel.org/lkml/20211208110833.65366-1-jiangshanlai@gmail.com/
>
> Lai Jiangshan (7):
>   x86/traps: Move pt_regs only in fixup_bad_iret()
>   x86/entry: Switch the stack after error_entry() returns
>   x86/entry: move PUSH_AND_CLEAR_REGS out of error_entry
>   x86/entry: Move cld to the start of idtentry
>   x86/entry: Don't call error_entry for XENPV
>   x86/entry: Convert SWAPGS to swapgs and remove the definition of
>     SWAPGS
>   x86/entry: Use idtentry macro for entry_INT80_compat
>
>  arch/x86/entry/entry_64.S        |  61 +++++++++++++-----
>  arch/x86/entry/entry_64_compat.S | 105 +------------------------------
>  arch/x86/include/asm/idtentry.h  |  47 ++++++++++++++
>  arch/x86/include/asm/irqflags.h  |   8 ---
>  arch/x86/include/asm/proto.h     |   4 --
>  arch/x86/include/asm/traps.h     |   2 +-
>  arch/x86/kernel/traps.c          |  17 ++---
>  7 files changed, 100 insertions(+), 144 deletions(-)
>
> --
> 2.19.1.6.gb485710b
>