Re: [PATCH 07/29] x86/entry: Sprinkle ENDBR dust

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org, joao@overdrivepizza.com, hjl.tools@gmail.com,
	andrew.cooper3@citrix.com, linux-kernel@vger.kernel.org,
	ndesaulniers@google.com, keescook@chromium.org,
	samitolvanen@google.com, mark.rutland@arm.com,
	alyssa.milburn@intel.com
Subject: Re: [PATCH 07/29] x86/entry: Sprinkle ENDBR dust
Date: Fri, 18 Feb 2022 16:36:35 -0800	[thread overview]
Message-ID: <20220219003635.uhzrzsuy5pjtp5jm@treble> (raw)
In-Reply-To: <20220218171408.984834165@infradead.org>

On Fri, Feb 18, 2022 at 05:49:09PM +0100, Peter Zijlstra wrote:
> -	.align 8
> +
> +	.align IDT_ALIGN
>  SYM_CODE_START(irq_entries_start)
>      vector=FIRST_EXTERNAL_VECTOR
>      .rept NR_EXTERNAL_VECTORS
> -	UNWIND_HINT_IRET_REGS
> +	UNWIND_HINT_IRET_REGS entry=1
>  0 :
> +	ENDBR
>  	.byte	0x6a, vector
>  	jmp	asm_common_interrupt
> -	nop
>  	/* Ensure that the above is 8 bytes max */

"IDT_ALIGN bytes max" ?

> -	. = 0b + 8
> +	.fill 0b + IDT_ALIGN - ., 1, 0x90
>  	vector = vector+1
>      .endr
>  SYM_CODE_END(irq_entries_start)
>  
>  #ifdef CONFIG_X86_LOCAL_APIC
> -	.align 8
> +	.align IDT_ALIGN
>  SYM_CODE_START(spurious_entries_start)
>      vector=FIRST_SYSTEM_VECTOR
>      .rept NR_SYSTEM_VECTORS
> -	UNWIND_HINT_IRET_REGS
> +	UNWIND_HINT_IRET_REGS entry=1
>  0 :
> +	ENDBR
>  	.byte	0x6a, vector
>  	jmp	asm_spurious_interrupt
> -	nop
>  	/* Ensure that the above is 8 bytes max */

Ditto

> -	. = 0b + 8
> +	.fill 0b + IDT_ALIGN - ., 1, 0x90
>  	vector = vector+1
>      .endr

>  SYM_CODE_END(spurious_entries_start)
> --- a/arch/x86/include/asm/segment.h
> +++ b/arch/x86/include/asm/segment.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/const.h>
>  #include <asm/alternative.h>
> +#include <asm/ibt.h>
>  
>  /*
>   * Constructor for a conventional segment GDT (or LDT) entry.
> @@ -275,7 +276,11 @@ static inline void vdso_read_cpunode(uns
>   * vector has no error code (two bytes), a 'push $vector_number' (two
>   * bytes), and a jump to the common entry code (up to five bytes).
>   */
> +#ifdef CONFIG_X86_IBT
> +#define EARLY_IDT_HANDLER_SIZE 13
> +#else
>  #define EARLY_IDT_HANDLER_SIZE 9
> +#endif

Might want to add a sentence to the comment above: With IDT enabled,
ENDBR adds another four bytes.

>  /*
>   * xen_early_idt_handler_array is for Xen pv guests: for each entry in
> --- a/arch/x86/include/asm/unwind_hints.h
> +++ b/arch/x86/include/asm/unwind_hints.h
> @@ -11,7 +11,7 @@
>  	UNWIND_HINT sp_reg=ORC_REG_UNDEFINED type=UNWIND_HINT_TYPE_CALL end=1
>  .endm
>  
> -.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0
> +.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0 entry=1
>  	.if \base == %rsp
>  		.if \indirect
>  			.set sp_reg, ORC_REG_SP_INDIRECT
> @@ -33,9 +33,17 @@
>  	.set sp_offset, \offset
>  
>  	.if \partial
> -		.set type, UNWIND_HINT_TYPE_REGS_PARTIAL
> +		.if \entry
> +		.set type, UNWIND_HINT_TYPE_REGS_ENTRY
> +		.else
> +		.set type, UNWIND_HINT_TYPE_REGS_EXIT
> +		.endif
>  	.elseif \extra == 0
> -		.set type, UNWIND_HINT_TYPE_REGS_PARTIAL
> +		.if \entry
> +		.set type, UNWIND_HINT_TYPE_REGS_ENTRY
> +		.else
> +		.set type, UNWIND_HINT_TYPE_REGS_EXIT
> +		.endif
>  		.set sp_offset, \offset + (16*8)

'extra' is apparently no longer needed and can be shown the door.

-- 
Josh