Messages in this thread | | | Date | Sun, 23 Mar 2014 00:00:31 +0000 (GMT) | From | "Maciej W. Rozycki" <> | Subject | Re: [GIT PULL] x86/apic changes for v3.14 |
| |
On Mon, 20 Jan 2014, Ingo Molnar wrote:
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c > index d278736..7f26c9a 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -1982,21 +1989,20 @@ static inline void __smp_error_interrupt(struct pt_regs *regs) > }; > > /* First tickle the hardware, only then report what went on. -- REW */ > - v0 = apic_read(APIC_ESR); > apic_write(APIC_ESR, 0); > - v1 = apic_read(APIC_ESR); > + v = apic_read(APIC_ESR); > ack_APIC_irq(); > atomic_inc(&irq_err_count); > > - apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x(%02x)", > - smp_processor_id(), v0 , v1); > + apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x", > + smp_processor_id(), v); > > - v1 = v1 & 0xff; > - while (v1) { > - if (v1 & 0x1) > + v &= 0xff; > + while (v) { > + if (v & 0x1) > apic_printk(APIC_DEBUG, KERN_CONT " : %s", error_interrupt_reason[i]); > i++; > - v1 >>= 1; > + v >>= 1; > } > > apic_printk(APIC_DEBUG, KERN_CONT "\n");
Sorry to come up with this so late, I've been cleaning up my mailbox from old mailing traffic and came across your change only now.
I'm afraid this interferes with an old Pentium APIC erratum:
"3AP. Writes to Error Register Clears Register
PROBLEM: The APIC Error register is intended to only be read. If there is a write to this register the data in the APIC Error register will be cleared and lost.
IMPLICATION: There is a possibility of clearing the Error register status since the write to the register is not specifically blocked.
WORKAROUND: Writes should not occur to the Pentium processor APIC Error register.
STATUS: For the steppings affected see the Summary Table of Changes at the beginning of this section."
The steppings affected are actually: B1, B3 and B5. Do we want to keep supporting them? I think yes, we already handle the erratum elsewhere (lapic_setup_esr). So how about:
if (lapic_get_maxlvt() > 3) /* Due to the Pentium erratum 3AP. */ apic_write(APIC_ESR, 0); v = apic_read(APIC_ESR);
instead? I can make a patch if that would make your life easier.
There's room for optimisation here, but I think it's not worth the effort as this is a slow path, APIC error interrupts are not supposed to happen and are I believe extremely uncommon with FSB message delivery.
Maciej
| |