From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>,
the arch/x86 maintainers <x86@kernel.org>,
Dan Williams <dan.j.williams@intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Andi Kleen <ak@linux.intel.com>,
Andrew Lutomirski <luto@kernel.org>
Subject: Re: [RFC v2 PATCH 6/7] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS
Date: Thu, 8 Feb 2018 12:35:45 -0600 [thread overview]
Message-ID: <20180208183545.meosbkyibfe4evnp@treble> (raw)
In-Reply-To: <20180207212915.GA4633@isilmar-4.linta.de>
On Wed, Feb 07, 2018 at 10:29:15PM +0100, Dominik Brodowski wrote:
> On Wed, Feb 07, 2018 at 12:44:41PM -0800, Linus Torvalds wrote:
> > On Wed, Feb 7, 2018 at 12:15 PM, Dominik Brodowski
> > <linux@dominikbrodowski.net> wrote:
> > >
> > > Note: The testb $3, CS(%rsp) instruction in idtentry() does not need
> > > modification. Previously %rsp was manually decreased by 15*8; with
> > > this patch, %rsp is decreased by 15 pushq instructions. Moreover,
> > > error_entry did and does the exact same test (with offset=8) after
> > > the registers have been moved/pushed and cleared.
> >
> > So this has the problem that now those save/clear instructions will
> > all be done in that idtentry macro.
> >
> > So now that code will be duplicated for all the users of that macro.
> >
> > The old code did the saving in the common error_entry and
> > paranoid_entry routines, in order to be able to share all the code,
> > and making the duplicated stub functions generated by the idtentry
> > macro smaller.
> >
> > Now, admittedly the new push sequence is much smaller than the old
> > movq sequence, so the duplication doesn't hurt as much, but it's still
> > likely quite noticeable.
> >
> > So this removes lines of asm code, but it adds a lot of instructions
> > to the end result thanks to the macro, I think.
>
> Indeed, that is the case (see below). However, if we want to switch to
> PUSH instructions and do this in a routine which is call'ed and which
> ret'urns, %rsp needs to be moved around even more often than the old
> ALLOC_PT_GPREGS_ON_STACK macro did (which you wanted to get rid of,
> IIUYC). Or do I miss something?
>
> text data bss dec hex filename
> 19500 0 0 19500 4c2c arch/x86/entry/entry_64.o-orig
> 19510 0 0 19510 4c36 arch/x86/entry/entry_64.o-3_of_7
> 21105 0 0 21105 5271 arch/x86/entry/entry_64.o-5_of_7
> 24307 0 0 24307 5ef3 arch/x86/entry/entry_64.o-7_of_7
>
> In any case, here's a v2.1 for this patch 6/7, which silences an objtool
> warning in error_entry.
>
> Thanks,
> Dominik
I like it. Some minor unwind hint improvements below, feel free to fold
them into this patch:
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ae1b09a2e872..d7bd489fa360 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1135,7 +1135,6 @@ idtentry machine_check do_mce has_error_code=0 paranoid=1
* Return: ebx=0: need swapgs on exit, ebx=1: otherwise
*/
ENTRY(paranoid_entry)
- UNWIND_HINT_FUNC
cld
movl $1, %ebx
movl $MSR_GS_BASE, %ecx
@@ -1149,7 +1148,7 @@ ENTRY(paranoid_entry)
SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
ret
-END(paranoid_entry)
+ENDPROC(paranoid_entry)
/*
* "Paranoid" exit path from exception stack. This is invoked
@@ -1184,7 +1183,6 @@ END(paranoid_exit)
* Return: EBX=0: came from user mode; EBX=1: otherwise
*/
ENTRY(error_entry)
- UNWIND_HINT_FUNC
UNWIND_HINT_REGS offset=8
cld
testb $3, CS+8(%rsp)
next prev parent reply other threads:[~2018-02-08 18:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-07 20:15 [RFC v2 PATCH 0/7] x86/entry: simplify and unify SAVE/POP_REGS Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 1/7] x86/entry: merge SAVE_C_REGS and SAVE_EXTRA_REGS, remove unused extensions Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 2/7] x86/entry: merge POP_C_REGS and POP_EXTRA_REGS Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 3/7] x86/entry: interleave XOR register clearing with PUSH instructions Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 4/7] x86/entry: introduce PUSH_AND_CLEAN_REGS Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 5/7] x86/entry: use PUSH_AND_CLEAN_REGS in more cases Dominik Brodowski
2018-02-07 20:15 ` [RFC v2 PATCH 6/7] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS Dominik Brodowski
2018-02-07 20:44 ` Linus Torvalds
2018-02-07 21:29 ` Dominik Brodowski
2018-02-07 21:58 ` Linus Torvalds
2018-02-08 7:20 ` Dominik Brodowski
2018-02-08 9:47 ` Ingo Molnar
2018-02-08 17:39 ` Linus Torvalds
2018-02-08 18:35 ` Josh Poimboeuf [this message]
2018-02-07 20:15 ` [RFC v2 PATCH 7/7] x86/entry: indent PUSH_AND_CLEAR_REGS and POP_REGS properly Dominik Brodowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180208183545.meosbkyibfe4evnp@treble \
--to=jpoimboe@redhat.com \
--cc=ak@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=luto@kernel.org \
--cc=mingo@kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.