Messages in this thread Patch in this message | | | Date | Thu, 8 Feb 2018 12:35:45 -0600 | From | Josh Poimboeuf <> | Subject | Re: [RFC v2 PATCH 6/7] x86/entry: get rid of ALLOC_PT_GPREGS_ON_STACK and SAVE_AND_CLEAR_REGS |
| |
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)
| |