Re: [patch V2 03/17] x86/traps: Remove pointless irq enable from do_spurious_interrupt_bug()

From: Thomas Gleixner <tglx@linutronix.de>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>, Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-arch@vger.kernel.org,
	Mike Rapoport <rppt@linux.ibm.com>,
	Miroslav Benes <mbenes@suse.cz>
Subject: Re: [patch V2 03/17] x86/traps: Remove pointless irq enable from do_spurious_interrupt_bug()
Date: Thu, 24 Oct 2019 00:35:27 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1910240018230.1852@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20191023213107.m7ishskghswktspp@treble>

[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]

On Wed, 23 Oct 2019, Josh Poimboeuf wrote:

> On Wed, Oct 23, 2019 at 02:27:08PM +0200, Thomas Gleixner wrote:
> > That function returns immediately after conditionally reenabling interrupts which
> > is more than pointless and requires the ASM code to disable interrupts again.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > ---
> >  arch/x86/kernel/traps.c |    1 -
> >  1 file changed, 1 deletion(-)
> > 
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -871,7 +871,6 @@ do_simd_coprocessor_error(struct pt_regs
> >  dotraplinkage void
> >  do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
> >  {
> > -	cond_local_irq_enable(regs);
> >  }
> 
> I think we can just remove this handler altogether.  The Intel and AMD
> manuals say vector 15 (X86_TRAP_SPURIOUS) is reserved.

Right, but this has history. Pentium Pro Erratum:

  PROBLEM: If the APIC subsystem is configured in mixed mode with Virtual
  Wire mode implemented through the local APIC, an interrupt vector of 0Fh
  (Intel reserved encoding) may be generated by the local APIC (Int 15).
  This vector may be generated upon receipt of a spurious interrupt (an
  interrupt which is removed before the system receives the INTA sequence)
  instead of the programmed 8259 spurious interrupt vector.

  IMPLICATION: The spurious interrupt vector programmed in the 8259 is
  normally handled by an operating system’s spurious interrupt
  handler. However, a vector of 0Fh is unknown to some operating systems,
  which would crash if this erratum occurred.

Initially (2.1.) there was a printk() in that handler, which later got
ifdeffed out (2.1.54).

So I rather keep that thing at least as long as we support PPro :) Even if
we ditch that the handler is not really hurting anyone.

Thanks,

	tglx