linux-kernel - Re: [PATCH V3 6/7] x86/entry: Don't call error

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]

Date:   Thu, 17 Mar 2022 00:48:50 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Lai Jiangshan <jiangshan.ljs@...group.com>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH V3 6/7] x86/entry: Don't call error_entry for XENPV

On Wed, Mar 16, 2022 at 11:09 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Tue, Mar 15, 2022 at 03:39:48PM +0800, Lai Jiangshan wrote:
> > From: Lai Jiangshan <jiangshan.ljs@...group.com>
> >
> > When in XENPV, it is already in the task stack, and it can't fault
> > for native_iret() nor native_load_gs_index() since XENPV uses its own
> > pvops for iret and load_gs_index().  And it doesn't need to switch CR3.
> > So there is no reason to call error_entry() in XENPV.
> >
> > Signed-off-by: Lai Jiangshan <jiangshan.ljs@...group.com>
> > ---
> >  arch/x86/entry/entry_64.S | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> > index e4a07276fd1c..ec885c2107de 100644
> > --- a/arch/x86/entry/entry_64.S
> > +++ b/arch/x86/entry/entry_64.S
> > @@ -328,8 +328,17 @@ SYM_CODE_END(ret_from_fork)
> >       PUSH_AND_CLEAR_REGS
> >       ENCODE_FRAME_POINTER
> >
> > -     call    error_entry
> > -     movq    %rax, %rsp                      /* switch stack settled by sync_regs() */
> > +     /*
> > +      * Call error_entry and switch stack settled by sync_regs().
> > +      *
> > +      * When in XENPV, it is already in the task stack, and it can't fault
> > +      * for native_iret() nor native_load_gs_index() since XENPV uses its
> > +      * own pvops for iret and load_gs_index().  And it doesn't need to
> > +      * switch CR3.  So it can skip invoking error_entry().
> > +      */
> > +     ALTERNATIVE "call error_entry; movq %rax, %rsp", \
> > +             "", X86_FEATURE_XENPV
> > +
> >       ENCODE_FRAME_POINTER
> >       UNWIND_HINT_REGS
> >
>
> Oooh, here we go, this is the answer to my question for patch #1, a note
> in the changelog might be nice. Something like:
>
> "This looses a Xen PV optimization, which will be restored in a later
> patch. The superfluous stack switch is just that."


In V2, the change of int80 thing is after this patch.  Maybe that order
of patches is more natural.  I'm sorry to reorder them.