Re: [PATCH] x86/APIC: Remove workaround Pentium 3AP APIC_ESR erratum

[Javi Merino <javi.merino@xxxxxxxxx>]

On Tue, Nov 26, 2024 at 08:58:59PM +0000, Andrew Cooper wrote:
> The SDM instructs software to write 0 to ESR prior to reading it.  However,
> due to an original Pentium erratum, most logic skips the write based on there
> being more than 3 LVTs; a stand-in to identify the Pentium.
> 
> Xen, being 64bit, doesn't need compatibility for i586 processors.
> 
> Introduce a new apic_read_esr() helper, quoting the SDM to explain why a
> function named apic_read_esr() has a write in it too.
> 
> Use the new helper throughout apic.c and smpboot.c, which allows us to remove
> some useless reads of APIC_LVR.  This in turn removes the external callers of
> get_maxlvt(), so make it local to apic.c
> 
> No practical change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Javi Merino <javi.merino@xxxxxxxxx>
> 
> Based on Javi's patch correcting error_interrupt()

Fair enough.  I was only looking at error_interrupt() and missed the
bigger picture. This patch is more comprehensive and this is very nice:

> Bloat-o-meter reports:
> 
>   add/remove: 0/1 grow/shrink: 0/3 up/down: 0/-269 (-269)
>   Function                                     old     new   delta
>   get_maxlvt                                    48       -     -48
>   __cpu_up                                    1465    1417     -48
>   clear_local_APIC                            1109    1050     -59
>   setup_local_APIC                             942     828    -114
> ---
>  xen/arch/x86/apic.c             | 29 ++++++++++-------------------
>  xen/arch/x86/include/asm/apic.h | 24 +++++++++++++++++++++++-
>  xen/arch/x86/smpboot.c          | 17 ++++-------------
>  3 files changed, 37 insertions(+), 33 deletions(-)

Reviewed-by: Javi Merino <javi.merino@xxxxxxxxx>