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 <[email protected]>
---
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);
}
dotraplinkage void
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 <[email protected]>
> ---
Reviewed-by: Sean Christopherson <[email protected]>
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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
> ---
> 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.
--
Josh
On Thu, Oct 24, 2019 at 12:35:27AM +0200, Thomas Gleixner wrote:
> 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 <[email protected]>
> > > ---
> > > 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.
Ah. I guess we could remove the idtentry for 64-bit then? Anyway the
above would be a good comment for the function.
--
Josh
On Wed, 23 Oct 2019, Josh Poimboeuf wrote:
> On Thu, Oct 24, 2019 at 12:35:27AM +0200, Thomas Gleixner wrote:
> > 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 <[email protected]>
> > > > ---
> > > > 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.
>
> Ah. I guess we could remove the idtentry for 64-bit then? Anyway the
> above would be a good comment for the function.
That doesn't buy much. Who knows how many other CPUs issue vector 15
occasionally and will then crash and burn. Better safe than sorry :)
Thanks,
tglx
On 10/23/19 2:27 PM, 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 <[email protected]>
> ---
> arch/x86/kernel/traps.c | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Alexandre Chartre <[email protected]>
alex.
The following commit has been merged into the x86/entry branch of tip:
Commit-ID: e039dd815941e785203142261397da6ec64d20cc
Gitweb: https://git.kernel.org/tip/e039dd815941e785203142261397da6ec64d20cc
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 25 Feb 2020 22:36:40 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 27 Feb 2020 14:48:39 +01:00
x86/traps: Remove pointless irq enable from do_spurious_interrupt_bug()
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 <[email protected]>
Reviewed-by: Sean Christopherson <[email protected]>
Reviewed-by: Alexandre Chartre <[email protected]>
Reviewed-by: Frederic Weisbecker <[email protected]>
Reviewed-by: Andy Lutomirski <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/traps.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 6ef00eb..474b8cb 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -862,7 +862,6 @@ do_simd_coprocessor_error(struct pt_regs *regs, long error_code)
dotraplinkage void
do_spurious_interrupt_bug(struct pt_regs *regs, long error_code)
{
- cond_local_irq_enable(regs);
}
dotraplinkage void