Re: [PATCH v2] x86: Stop relying on magic jmp behavior for early_idt_handlers

[Ingo Molnar <mingo@kernel.org>]

* 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