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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/11/2024 10:03 am, Javi Merino wrote:
> 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>

Thanks.  Are you happy for me to merge the two patches?

~Andrew