[PATCH] x86: dovetail: Use appropriate EOI on error interrupts

[PATCH] x86: dovetail: Use appropriate EOI on error interrupts

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Just like for the non-pipelined spurious interrupt, we need to invoke
the non-pipelined __apic_eoi while handling error interrupts. Otherwise,
the local APIC will stop receiving further interrupts, and the system
will stall.

Given that it takes very special conditions to actually trigger an error
interrupt, this issue was very likely not relevant for most deployments.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kernel/apic/apic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 1cd5f59f7962f..278e28c5290d3 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2217,7 +2217,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_error_interrupt)
 	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
 		apic_write(APIC_ESR, 0);
 	v = apic_read(APIC_ESR);
-	apic_eoi();
+	__apic_eoi();
 	atomic_inc(&irq_err_count);
 
 	apic_pr_debug("APIC error on CPU%d: %02x", smp_processor_id(), v);
-- 
2.43.0

Re: [PATCH] x86: dovetail: Use appropriate EOI on error interrupts

[Florian Bezdeka]

On Wed, 2025-06-25 at 10:16 +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Just like for the non-pipelined spurious interrupt, we need to invoke
> the non-pipelined __apic_eoi while handling error interrupts. Otherwise,
> the local APIC will stop receiving further interrupts, and the system
> will stall.
> 
> Given that it takes very special conditions to actually trigger an error
> interrupt, this issue was very likely not relevant for most deployments.

On which hardware is that happening? Asking because of 

/*
 * This interrupt should never happen with our APIC/SMP architecture
 */

infront of the function...

In addition we should properly check which branches are affected.
apic_eoi() vs. ack_APIC_irq() (in 5.10), 

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/x86/kernel/apic/apic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 1cd5f59f7962f..278e28c5290d3 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2217,7 +2217,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_error_interrupt)
>  	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
>  		apic_write(APIC_ESR, 0);
>  	v = apic_read(APIC_ESR);
> -	apic_eoi();
> +	__apic_eoi();
>  	atomic_inc(&irq_err_count);
>  
>  	apic_pr_debug("APIC error on CPU%d: %02x", smp_processor_id(), v);
> -- 
> 2.43.0

Re: [PATCH] x86: dovetail: Use appropriate EOI on error interrupts

[Jan Kiszka]

On 25.06.25 13:16, Florian Bezdeka wrote:
> On Wed, 2025-06-25 at 10:16 +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Just like for the non-pipelined spurious interrupt, we need to invoke
>> the non-pipelined __apic_eoi while handling error interrupts. Otherwise,
>> the local APIC will stop receiving further interrupts, and the system
>> will stall.
>>
>> Given that it takes very special conditions to actually trigger an error
>> interrupt, this issue was very likely not relevant for most deployments.
> 
> On which hardware is that happening? Asking because of 
> 
> /*
>  * This interrupt should never happen with our APIC/SMP architecture
>  */
> 
> infront of the function...
> 

The user was very likely confronted with two errors at once: This one
here, causing the visible problem of getting no more interrupts and the
actual root cause of receiving an illegal IRQ vector on some core. The
latter was most probably caused by the special software deployment there
(half virtualization, half co-operative AMP).

> In addition we should properly check which branches are affected.
> apic_eoi() vs. ack_APIC_irq() (in 5.10), 
> 

Yes, it's going down to 5.10 at least (where it is about
__ack_APIC_irq). We have seen it with 5.10 in fact.

Jan

>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  arch/x86/kernel/apic/apic.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>> index 1cd5f59f7962f..278e28c5290d3 100644
>> --- a/arch/x86/kernel/apic/apic.c
>> +++ b/arch/x86/kernel/apic/apic.c
>> @@ -2217,7 +2217,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_error_interrupt)
>>  	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
>>  		apic_write(APIC_ESR, 0);
>>  	v = apic_read(APIC_ESR);
>> -	apic_eoi();
>> +	__apic_eoi();
>>  	atomic_inc(&irq_err_count);
>>  
>>  	apic_pr_debug("APIC error on CPU%d: %02x", smp_processor_id(), v);
>> -- 
>> 2.43.0


-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Re: [PATCH] x86: dovetail: Use appropriate EOI on error interrupts

[Philippe Gerum]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Just like for the non-pipelined spurious interrupt, we need to invoke
> the non-pipelined __apic_eoi while handling error interrupts. Otherwise,
> the local APIC will stop receiving further interrupts, and the system
> will stall.
>
> Given that it takes very special conditions to actually trigger an error
> interrupt, this issue was very likely not relevant for most deployments.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/x86/kernel/apic/apic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 1cd5f59f7962f..278e28c5290d3 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2217,7 +2217,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_error_interrupt)
>  	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
>  		apic_write(APIC_ESR, 0);
>  	v = apic_read(APIC_ESR);
> -	apic_eoi();
> +	__apic_eoi();
>  	atomic_inc(&irq_err_count);
>  
>  	apic_pr_debug("APIC error on CPU%d: %02x", smp_processor_id(), v);

Merged (v6.1.y [__ack_APIC_irq], v6.12.y, v6.15 and upcoming v6.16), thanks.

(short log fixed up slightly to ease commit identification on my end)

-- 
Philippe.