Re: x86: Spurious vectors not handled robustly

From: Thomas Gleixner <tglx@linutronix.de>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jan Beulich <jbeulich@suse.com>
Subject: Re: x86: Spurious vectors not handled robustly
Date: Mon, 24 Jun 2019 17:26:29 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1906241725340.32342@nanos.tec.linutronix.de> (raw)
In-Reply-To: <alpine.DEB.2.21.1906241541290.32342@nanos.tec.linutronix.de>

On Mon, 24 Jun 2019, Thomas Gleixner wrote:
>  
> +#ifdef CONFIG_X86_LOCAL_APIC
> +	.align 8
> +ENTRY(spurious_entries_start)
> +    vector=FIRST_SYSTEM_VECTOR
> +    .rept (NR_VECTORS - FIRST_SYSTEM_VECTOR)
> +	pushl	$(~vector+0x80)			/* Note: always in signed byte range */
> +    vector=vector+1
> +	jmp	common_spurious_vector

Moo. Not syncing the compile machine and the laptop! That should obviously be

 +	jmp	common_spurious

> +	.align	8
> +    .endr
> +END(spurious_entries_start)
> +
> +common_spurious:
> +	ASM_CLAC
> +	addl	$-0x80, (%esp)			/* Adjust vector into the [-256, -1] range */
> +	SAVE_ALL switch_stacks=1
> +	ENCODE_FRAME_POINTER
> +	TRACE_IRQS_OFF
> +	movl	%esp, %eax
> +	call	smp_spurious_interrupt
> +	jmp	ret_from_intr
> +ENDPROC(common_interrupt)
> +#endif
> +
>  /*
>   * the CPU automatically disables interrupts when executing an IRQ vector,
>   * so IRQ-flags tracing has to follow that:
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -375,6 +375,18 @@ ENTRY(irq_entries_start)
>      .endr
>  END(irq_entries_start)
>  
> +	.align 8
> +ENTRY(spurious_entries_start)
> +    vector=FIRST_SYSTEM_VECTOR
> +    .rept (NR_VECTORS - FIRST_SYSTEM_VECTOR)
> +	UNWIND_HINT_IRET_REGS
> +	pushq	$(~vector+0x80)			/* Note: always in signed byte range */
> +	jmp	common_spurious
> +	.align	8
> +	vector=vector+1
> +    .endr
> +END(spurious_entries_start)
> +
>  .macro DEBUG_ENTRY_ASSERT_IRQS_OFF
>  #ifdef CONFIG_DEBUG_ENTRY
>  	pushq %rax
> @@ -571,10 +583,20 @@ END(interrupt_entry)
>  
>  /* Interrupt entry/exit. */
>  
> -	/*
> -	 * The interrupt stubs push (~vector+0x80) onto the stack and
> -	 * then jump to common_interrupt.
> -	 */
> +/*
> + * The interrupt stubs push (~vector+0x80) onto the stack and
> + * then jump to common_spurious/interrupt.
> + */
> +common_spurious:
> +	addq	$-0x80, (%rsp)			/* Adjust vector to [-256, -1] range */
> +	call	interrupt_entry
> +	UNWIND_HINT_REGS indirect=1
> +	call	smp_spurious_interrupt		/* rdi points to pt_regs */
> +	jmp	ret_from_intr
> +END(common_spurious)
> +_ASM_NOKPROBE(common_spurious)
> +
> +/* common_interrupt is a hotpath. Align it */
>  	.p2align CONFIG_X86_L1_CACHE_SHIFT
>  common_interrupt:
>  	addq	$-0x80, (%rsp)			/* Adjust vector to [-256, -1] range */
> --- a/arch/x86/include/asm/hw_irq.h
> +++ b/arch/x86/include/asm/hw_irq.h
> @@ -150,6 +150,8 @@ extern char irq_entries_start[];
>  #define trace_irq_entries_start irq_entries_start
>  #endif
>  
> +extern char spurious_entries_start[];
> +
>  #define VECTOR_UNUSED		NULL
>  #define VECTOR_RETRIGGERED	((void *)~0UL)
>  
> --- a/arch/x86/kernel/idt.c
> +++ b/arch/x86/kernel/idt.c
> @@ -319,7 +319,8 @@ void __init idt_setup_apic_and_irq_gates
>  #ifdef CONFIG_X86_LOCAL_APIC
>  	for_each_clear_bit_from(i, system_vectors, NR_VECTORS) {
>  		set_bit(i, system_vectors);
> -		set_intr_gate(i, spurious_interrupt);
> +		entry = spurious_entries_start + 8 * (i - FIRST_SYSTEM_VECTOR);
> +		set_intr_gate(i, entry);
>  	}
>  #endif
>  }
> 