Re: [PATCH 45/79] [PATCH] fix apic acking of irqs

[Maciej W. Rozycki]

On Tue, 25 Mar 2008, Glauber Costa wrote:

> the only ESR mention I see in setup_local_APIC() is this:
> 
>          /* Pound the ESR really hard over the head with a big hammer - mbligh
> */
>         if (esr_disable) {
>                 apic_write(APIC_ESR, 0);
>                 apic_write(APIC_ESR, 0);
>                 apic_write(APIC_ESR, 0);
>                 apic_write(APIC_ESR, 0);
>         }
> which seems more like a disablement.

 There is more later on...

> the bootup code does clean it, tough, by writing and reading the ESR.

... basically for the original Pentium and Pentium/MMX APIC you only had 
to read the ESR to get at the bits.  The read would clear them as well as 
a side-effect.  Although at that stage already it was mentioned in the 
spec that for future compatibility a write of zero beforehand (ignored as 
the register was r/o) should be performed.  Which indeed became a 
requirement from PentiumPro onwards as with these processors it was the 
write that copied the internal error latches into the visible ESR 
register.  Except that some Pentium APICs had an erratum, where ESR was 
indeed r/w and the leading write of zero would actually clear the register 
losing the recorded state, so it had to be avoided despite the 
recommendation.  Hence the code you can see within:

if (integrated && !esr_disable) {
}

I suppose other APIC implementers were not that keen on keeping bug 
compatibility, so chances are other APIC core work just fine as specified 
by the architecture (for whatever the meaning of "fine" is).

 Note the usual APIC error interrupt handler is smp_error_interrupt().

> >  I have asked this question already: what kind of CPU are you running on?
> > Do you really need to have CONFIG_X86_GOOD_APIC clear with it?
> > 
> My testings that triggered that were with qemu, with randconfigs. Probably it
> has a good apic, but it is good that it triggered anyway. Otherwise I'd never
> see it.

 Ah, I see -- it may be worth checking what actual hardware does and 
fixing QEMU if necessary for it to match reality then. ;-)

 OTOH, if actual modern hardware triggered such an error, then for the 
sake of a generic "runs everywhere" kernel either ack_APIC_irq() or even 
apic_write_around() could be modified to perform a run-time check if 
configured with !CONFIG_X86_GOOD_APIC and avoid the read if unnecessary; 
it's an erratum workaround after all and SMP Pentium systems suffering 
from this bug (UP Pentium systems did not nor had a way to enable the 
local APIC normally) are probably an insignificant minority if any at all 
left these days.  Therefore it should be a negligible sacrifice of 
performance.

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/