Re: [PATCH v4 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C

[Andy Lutomirski]

On Thu, Jul 2, 2015 at 5:09 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Mon, Jun 29, 2015 at 12:33:46PM -0700, Andy Lutomirski wrote:
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>>  arch/x86/entry/entry_64.S        | 63 +++++++++++-----------------------------
>>  arch/x86/entry/entry_64_compat.S |  5 ++++
>>  2 files changed, 22 insertions(+), 46 deletions(-)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index ce2c9049abef..08a37ec049f0 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -508,7 +508,16 @@ END(irq_entries_start)
>>
>>       testb   $3, CS(%rsp)
>>       jz      1f
>> +
>> +     /*
>> +      * IRQ from user mode.  Switch to kernel gsbase and inform context
>> +      * tracking that we're in kernel mode.
>> +      */
>>       SWAPGS
>> +#ifdef CONFIG_CONTEXT_TRACKING
>> +     call enter_from_user_mode
>> +#endif
>
> I think you can make this much cleaner by getting rid of the ifdeffery
> and pushing it into the enter_from_user_mode() function:
>
> __visible void enter_from_user_mode(void)
> {
> #ifdef CONFIG_CONTEXT_TRACKING
>
>         ...
>
> #endif
> }
>
> The disadvantage of all that cleanliness is that we get one dumb
>
>         call enter_from_user_mode
>
> there to this abomination:
>
> ffffffff810014f0 <enter_from_user_mode>:
> ffffffff810014f0:       e8 db 97 67 00          callq  ffffffff8167acd0 <__fentry__>
> ffffffff810014f5:       55                      push   %rbp
> ffffffff810014f6:       48 89 e5                mov    %rsp,%rbp
> ffffffff810014f9:       5d                      pop    %rbp
> ffffffff810014fa:       c3                      retq
> ffffffff810014fb:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
>
> which sux.
>
> We sure could use LTO here.
>

You mean link-time asm optimizations?  Turning off frame pointers in
leaf functions as long as rbp is still preserved might not be so
terrible either.

I'm torn on this one.  In principle, you're right, or we could have a
macro CALL_ENTER_FROM_USER_MODE that does nothing if context tracking
is off.  OTOH, that's also kind of messy.

If we move even more of this stuff into C, then this problem goes
away.  "call enter_from_user_mode" turns into "enter_from_user_mode()"
or similar.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/