[PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[Jan Beulich]

Ever since 3.0.2 we've been carrying this patch in our products. Since
there was no indication that there would be anything wrong with the
'new' IO-APIC ack method added back then, we finally decided to drop
this patch recently from SLE11, to find that the subsequent release
candidate failed to work on at least on system without using
"ioapic_ack=old". With that knowledge I think it is now reasonable to
include that patch in -unstable, as the introduction of the 'new' ack
method was only to address issues with certain chipsets silently
setting up alternative IRQ routes when RTEs in secondary IO-APICs got
masked.

Here some data on the hardware the issue was reportedly found on:
* intel DQ965CO motherboard (i82Q965 chipset)
* intel 82566DM NIC
* intel ICH8R (82801H)
* Marvell 88SE6101 IDE controller
* nVidia 8600GT graphics
* intel Q6600 CPU
* 8GB RAM

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- 2009-01-08.orig/xen/arch/x86/io_apic.c	2009-01-09 12:43:35.000000000 +0100
+++ 2009-01-08/xen/arch/x86/io_apic.c	2009-01-21 14:11:53.000000000 +0100
@@ -1352,7 +1352,7 @@ static unsigned int startup_level_ioapic
     return 0; /* don't check for pending */
 }
 
-int ioapic_ack_new = 1;
+int ioapic_ack_new = -1;
 static void setup_ioapic_ack(char *s)
 {
     if ( !strcmp(s, "old") )
@@ -1839,6 +1839,8 @@ void __init setup_IO_APIC(void)
     else
         io_apic_irqs = ~PIC_IRQS;
 
+    if (ioapic_ack_new < 0)
+        ioapic_ack_new = (nr_ioapics > 1);
     printk("ENABLING IO-APIC IRQs\n");
     printk(" -> Using %s ACK method\n", ioapic_ack_new ? "new" : "old");
 

RE: [PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[Jiang, Yunhong]

xen-devel-bounces@lists.xensource.com <> wrote:
> Ever since 3.0.2 we've been carrying this patch in our products. Since
> there was no indication that there would be anything wrong with the
> 'new' IO-APIC ack method added back then, we finally decided to drop
> this patch recently from SLE11, to find that the subsequent release
> candidate failed to work on at least on system without using

This is a bit strange, a bit curios that do you know the reason that it not working? 

> "ioapic_ack=old". With that knowledge I think it is now reasonable to
> include that patch in -unstable, as the introduction of the 'new' ack
> method was only to address issues with certain chipsets silently
> setting up alternative IRQ routes when RTEs in secondary IO-APICs got
> masked. 
> 
> Here some data on the hardware the issue was reportedly found on:
> * intel DQ965CO motherboard (i82Q965 chipset)
> * intel 82566DM NIC
> * intel ICH8R (82801H)
> * Marvell 88SE6101 IDE controller
> * nVidia 8600GT graphics
> * intel Q6600 CPU
> * 8GB RAM
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- 2009-01-08.orig/xen/arch/x86/io_apic.c	2009-01-09 12:43:35.000000000
> +0100 +++ 2009-01-08/xen/arch/x86/io_apic.c	2009-01-21
> 14:11:53.000000000 +0100
> @@ -1352,7 +1352,7 @@ static unsigned int startup_level_ioapic
>     return 0; /* don't check for pending */
> }
> 
> -int ioapic_ack_new = 1;
> +int ioapic_ack_new = -1;
> static void setup_ioapic_ack(char *s)
> {
>     if ( !strcmp(s, "old") )
> @@ -1839,6 +1839,8 @@ void __init setup_IO_APIC(void)     else
>         io_apic_irqs = ~PIC_IRQS;
> 
> +    if (ioapic_ack_new < 0)
> +        ioapic_ack_new = (nr_ioapics > 1);
>     printk("ENABLING IO-APIC IRQs\n");
>     printk(" -> Using %s ACK method\n", ioapic_ack_new ? "new" : "old");
> 
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Jan Beulich]

>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 21.01.09 15:23 >>>
>> Ever since 3.0.2 we've been carrying this patch in our products. Since
>> there was no indication that there would be anything wrong with the
>> 'new' IO-APIC ack method added back then, we finally decided to drop
>> this patch recently from SLE11, to find that the subsequent release
>> candidate failed to work on at least on system without using
>
>This is a bit strange, a bit curios that do you know the reason that it not working? 

I too find it strange (otherwise I wouldn't have decided to drop that patch),
but given that it always worked before, we have a solution at hand, and I
don't have an affected machine available for debugging, I wanted to save
myself and the reporter trying to figure out what's going on there from
remote.

Jan

Re: [PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[Keir Fraser]

On 21/01/2009 14:21, "Jan Beulich" <jbeulich@novell.com> wrote:

> "ioapic_ack=old". With that knowledge I think it is now reasonable to
> include that patch in -unstable, as the introduction of the 'new' ack
> method was only to address issues with certain chipsets silently
> setting up alternative IRQ routes when RTEs in secondary IO-APICs got
> masked.

I don't specifically recall that this issue required two IO-APICs. In fact I
think it did not. It was actually something to do with the chipset trying to
distinguish between an OS using 'legacy' routing versus 'mp-bios' routing,
via a rather distasteful IO-APIC hack. Unfortunately the hack was not that
uncommon and I don't think those chipsets had more than one IO-APIC.

Overall I think ack_type new has worked quite well. I was actually about to
remove the old ack_type! (But now I won't ;-) I'm not inclined to take this
patch though.

 -- Keir

RE: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Jiang, Yunhong]

Jan Beulich <mailto:jbeulich@novell.com> wrote:
>>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 21.01.09 15:23 >>>
>>> Ever since 3.0.2 we've been carrying this patch in our products. Since
>>> there was no indication that there would be anything wrong with the
>>> 'new' IO-APIC ack method added back then, we finally decided to drop
>>> this patch recently from SLE11, to find that the subsequent release
>>> candidate failed to work on at least on system without using
>> 
>> This is a bit strange, a bit curios that do you know the reason that it
>> not working? 
> 
> I too find it strange (otherwise I wouldn't have decided to
> drop that patch),
> but given that it always worked before, we have a solution at
> hand, and I
> don't have an affected machine available for debugging, I
> wanted to save
> myself and the reporter trying to figure out what's going on there from
> remote. 

Got it. Just want to make sure it is still under debug otherwise it may bite us in future :-)

> 
> Jan

Re: [PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[Keir Fraser]

On 21/01/2009 14:23, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:

> xen-devel-bounces@lists.xensource.com <> wrote:
>> Ever since 3.0.2 we've been carrying this patch in our products. Since
>> there was no indication that there would be anything wrong with the
>> 'new' IO-APIC ack method added back then, we finally decided to drop
>> this patch recently from SLE11, to find that the subsequent release
>> candidate failed to work on at least on system without using
> 
> This is a bit strange, a bit curios that do you know the reason that it not
> working? 

Yes - this also -- there doesn't seem to be a good reason that the 'new'
ack_type wouldn't work in all cases. We're also somewhat depending on it (or
similar) for unmaskable MSIs.

 -- Keir

RE: [PATCH] x86: change IO-APIC ack method default for single IO-APIC systems

[Jiang, Yunhong]

xen-devel-bounces@lists.xensource.com <> wrote:
> On 21/01/2009 14:21, "Jan Beulich" <jbeulich@novell.com> wrote:
> 
>> "ioapic_ack=old". With that knowledge I think it is now reasonable to
>> include that patch in -unstable, as the introduction of the 'new' ack
>> method was only to address issues with certain chipsets silently
>> setting up alternative IRQ routes when RTEs in secondary IO-APICs got
>> masked.
> 
> I don't specifically recall that this issue required two
> IO-APICs. In fact I
> think it did not. It was actually something to do with the
> chipset trying to
> distinguish between an OS using 'legacy' routing versus 'mp-bios' routing,
> via a rather distasteful IO-APIC hack. Unfortunately the hack
> was not that
> uncommon and I don't think those chipsets had more than one IO-APIC.

I remember there are two IO-APICs, the second one is in a PXH hub, not in the chipset. and it is this external IOAPIC will do something tricky as Jan described. It is really a very old story and maybe I'm wrong.

> 
> Overall I think ack_type new has worked quite well. I was
> actually about to
> remove the old ack_type! (But now I won't ;-) I'm not inclined
> to take this
> patch though.
> 
> -- Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 21.01.09 15:37 >>>
>On 21/01/2009 14:23, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:
>
>> xen-devel-bounces@lists.xensource.com <> wrote:
>>> Ever since 3.0.2 we've been carrying this patch in our products. Since
>>> there was no indication that there would be anything wrong with the
>>> 'new' IO-APIC ack method added back then, we finally decided to drop
>>> this patch recently from SLE11, to find that the subsequent release
>>> candidate failed to work on at least on system without using
>> 
>> This is a bit strange, a bit curios that do you know the reason that it not
>> working? 
>
>Yes - this also -- there doesn't seem to be a good reason that the 'new'
>ack_type wouldn't work in all cases. We're also somewhat depending on it (or
>similar) for unmaskable MSIs.

Right - but MSIs were the only interrupts staying alive on that system with
the 'new' method.

Jan

Re: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 21.01.09 15:35 >>>
>On 21/01/2009 14:21, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>> "ioapic_ack=old". With that knowledge I think it is now reasonable to
>> include that patch in -unstable, as the introduction of the 'new' ack
>> method was only to address issues with certain chipsets silently
>> setting up alternative IRQ routes when RTEs in secondary IO-APICs got
>> masked.
>
>I don't specifically recall that this issue required two IO-APICs. In fact I
>think it did not. It was actually something to do with the chipset trying to
>distinguish between an OS using 'legacy' routing versus 'mp-bios' routing,
>via a rather distasteful IO-APIC hack. Unfortunately the hack was not that
>uncommon and I don't think those chipsets had more than one IO-APIC.

I'm rather certain that it did involve multiple IO-APICs. What the chipsets
were trying to cover was the ACPI vs. no-ACPI case, since secondary IO-APICs
generally can be (or should I say are being/have been at that time on "certain"
OSes) discovered only with ACPI. Hence when an IRQ normally going to a
secondary IO-APIC's pin go masked in that IO-APIC, a replacement route
was automatically established (and not torn down when the mask bit got
cleared again) to a pin of the primary IO-APIC.

>Overall I think ack_type new has worked quite well. I was actually about to
>remove the old ack_type! (But now I won't ;-) I'm not inclined to take this
>patch though.

If I had an affected system to debug the issue, I'd try to do so (though
remembering how long it took to understand the original issue I'm hesitant
to say so). With the above explanation I hope you may reconsider...

Jan

Re: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Keir Fraser]

On 21/01/2009 14:51, "Jan Beulich" <jbeulich@novell.com> wrote:

>> I don't specifically recall that this issue required two IO-APICs. In fact I
>> think it did not. It was actually something to do with the chipset trying to
>> distinguish between an OS using 'legacy' routing versus 'mp-bios' routing,
>> via a rather distasteful IO-APIC hack. Unfortunately the hack was not that
>> uncommon and I don't think those chipsets had more than one IO-APIC.
> 
> I'm rather certain that it did involve multiple IO-APICs. What the chipsets
> were trying to cover was the ACPI vs. no-ACPI case, since secondary IO-APICs
> generally can be (or should I say are being/have been at that time on
> "certain"
> OSes) discovered only with ACPI. Hence when an IRQ normally going to a
> secondary IO-APIC's pin go masked in that IO-APIC, a replacement route
> was automatically established (and not torn down when the mask bit got
> cleared again) to a pin of the primary IO-APIC.

Yes, I think actually you are correct.
http://thread.gmane.org/gmane.os.freebsd.current/67490/focus=67490

>> Overall I think ack_type new has worked quite well. I was actually about to
>> remove the old ack_type! (But now I won't ;-) I'm not inclined to take this
>> patch though.
> 
> If I had an affected system to debug the issue, I'd try to do so (though
> remembering how long it took to understand the original issue I'm hesitant
> to say so). With the above explanation I hope you may reconsider...

Well, it seems not great to avoid the new ack_type for some unknown bug. And
noone else has run with your patch and zero other issues with the new
ack_type have been reported. So this seems to be papering over a rather rare
and potentially nasty underlying issue. On the other hand, perhaps old
ack_type is preferable (faster) if we can be sure we're on a system where it
is safe. Hmmm.

 -- Keir

Re: [PATCH] x86: change IO-APIC ack method defaultforsingle IO-APIC systems

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 21.01.09 17:32 >>>
>Well, it seems not great to avoid the new ack_type for some unknown bug. And
>noone else has run with your patch and zero other issues with the new

Many people have - all users of SLE10 (from GA over SP1 to SP2), SuSE
10.2, 10.3, 11.0, 11.1, and all betas and RCs so far available for SLE11.

>ack_type have been reported. So this seems to be papering over a rather rare
>and potentially nasty underlying issue. On the other hand, perhaps old
>ack_type is preferable (faster) if we can be sure we're on a system where it
>is safe. Hmmm.

Jan

RE: [PATCH] x86: change IO-APIC ack method default forsingle IO-APIC systems

[Tian, Kevin]

>From: Keir Fraser
>Sent: Thursday, January 22, 2009 12:33 AM
>[...]
>and potentially nasty underlying issue. On the other hand, perhaps old
>ack_type is preferable (faster) if we can be sure we're on a 
>system where it
>is safe. Hmmm.
>

yes, it's faster and also better fit RT purpose with latency requirement. new
ack method has uncertain delay with dependency on guest ISR. But
one question here is, how to make sure whether system is safe? Is 
there automatic method to choose, or whitelist, or ask user to manually
change when seeing errors? :-(

Thanks,
Kevin