Messages in this thread | | | Date | Fri, 22 May 2015 08:22:24 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v2] x86: Stop relying on magic jmp behavior for early_idt_handlers |
| |
* Andy Lutomirski <luto@kernel.org> wrote:
> --- a/arch/x86/include/asm/segment.h > +++ b/arch/x86/include/asm/segment.h > @@ -231,9 +231,17 @@ > #define TLS_SIZE (GDT_ENTRY_TLS_ENTRIES* 8) > > #ifdef __KERNEL__ > + > +/* > + * early_idt_handlers is an array of entry points. For simplicity, it's > + * a real array. We allocate nine bytes for each entry: two one-byte > + * push instructions and a five-byte jump in the worst case. > + */ > +#define EARLY_IDT_HANDLER_STRIDE 9
So how come that two plus five equals nine? ;-)
It's two two-bype pushes (sometimes a 2-byte NOP), plus a 5-byte jump, in the worst case.
I'd also mention that it's an array of 32 small trampolines that set up parameters and jump to a common entry point.
> +/* Build the early_idt_handlers array */ > ENTRY(early_idt_handlers)
Please rename the function accordingly: early_idt_handlers_array, to make clear this is never jumped to directly, only through the IDT.
> # 36(%esp) %eflags > # 32(%esp) %cs > @@ -531,19 +532,18 @@ ENTRY(early_idt_handlers) > # 24(%rsp) error code > i = 0 > .rept NUM_EXCEPTION_VECTORS > - .if (EXCEPTION_ERRCODE_MASK >> i) & 1 > - ASM_NOP2 > - .else > + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc > + .ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1 > pushl $0 # Dummy error code, to make stack frame uniform > .endif > pushl $i # 20(%esp) Vector number > jmp early_idt_handler > i = i + 1 > .endr > + .fill early_idt_handlers + i * EARLY_IDT_HANDLER_STRIDE - ., 1, 0xcc > ENDPROC(early_idt_handlers) > > - /* This is global to keep gas from relaxing the jumps */ > -ENTRY(early_idt_handler) > +early_idt_handler:
Please rename this as well to make it clearer what's happening, something like:
early_idt_handler_common:
would work for me.
Ditto for the 64-bit side.
Thanks,
Ingo
| |