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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/11/2024 8:20 am, Jan Beulich wrote:
> On 26.11.2024 21:58, Andrew Cooper wrote:
>> @@ -1389,8 +1381,7 @@ static void cf_check error_interrupt(void)
>>      unsigned int i;
>>  
>>      /* First tickle the hardware, only then report what went on. -- REW */
>> -    apic_write(APIC_ESR, 0);
>> -    v = apic_read(APIC_ESR);
>> +    v = apic_read_esr();
>>      ack_APIC_irq();
> As indicated in the earlier reply, I think this comment should be dropped,

This one probably can, but ...

> plus ...
>
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -422,7 +422,7 @@ void asmlinkage start_secondary(void *unused)
>>  static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
>>  {
>>      unsigned long send_status = 0, accept_status = 0;
>> -    int maxlvt, timeout, i;
>> +    int timeout, i;
>>  
>>      /*
>>       * Normal AP startup uses an INIT-SIPI-SIPI sequence.
>> @@ -444,8 +444,7 @@ static int wakeup_secondary_cpu(int phys_apicid, 
>> unsigned long start_eip)
>>      /*
>>       * Be paranoid about clearing APIC errors.
>>       */
>> -    apic_write(APIC_ESR, 0);
>> -    apic_read(APIC_ESR);
>> +    apic_read_esr();
> ... the one here.

... this one is still correct in place.


I almost had a second function called apic_clear_esr() which was just
(void)apic_read_esr().

I could put that back in if you think it would be clearer ?

>  With that and with Javi's change folded into here,
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew