Re: [PATCH v3 8/7] TESTING_ONLY x86/entry: reduce static footprint of idtentry

From: Linus Torvalds
Date: Mon Feb 12 2018 - 14:17:53 EST


On Mon, Feb 12, 2018 at 1:37 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> Ok, so this does not look _that_ complicated, and the .text savings are
> significant:

Honestly, I think we should do it. 3kB of assembly code is noticeable.

Also, that patch actually allows more cleanups and simplifications.
Look at the "interrupt" macro, which is used by 'apicinterrupt3', and
has a number of uses that way.

That code could be unified a lot, right now it does:

testb $3, CS-ORIG_RAX(%rsp)
jz 1f
SWAPGS
call switch_to_thread_stack
1:

ALLOC_PT_GPREGS_ON_STACK
SAVE_C_REGS
SAVE_EXTRA_REGS
ENCODE_FRAME_POINTER

testb $3, CS(%rsp)
jz 1f

/*
* IRQ from user mode.
*
* We need to tell lockdep that IRQs are off. We can't do this until
* we fix gsbase, and we should do it before enter_from_user_mode
* (which can take locks). Since TRACE_IRQS_OFF idempotent,
* the simplest way to handle it is to just call it twice if
* we enter from user mode. There's no reason to optimize this since
* TRACE_IRQS_OFF is a no-op if lockdep is off.
*/
TRACE_IRQS_OFF

CALL_enter_from_user_mode

1:
ENTER_IRQ_STACK old_rsp=%rdi
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF

and *all* of that could be in a helper function rather than be
duplicated. and the apicinterrupt3 macro should end up just expanding
to

callq helper
pushq $~(\num)
callq \fn
jmp ret_from_intr

instead of expanding to all that code.

But that would require that same "save_ret" logic.

So it's not just the idtentry cases that can use this trick.

I admit that the trick isn't pretty, but it's not *horribly* ugly either.

Linus