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
> > <li...@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)
