RE: IO-APIC + timer doesn't work

RE: IO-APIC + timer doesn't work

[Lu, Yinghai]

-----Original Message-----
From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 
Sent: Thursday, December 21, 2006 12:47 PM
To: Lu, Yinghai
>> +static int add_irq_entry(int type, int irqflag, int bus, int irq,
int apic, int
>> pin)

>This is fairly sane but probably belongs in mptable.c as a helper.

mparse.c?


>I am still trying to understand this enable_8259A_irq(0) case.
>As far as I can tell this is a very backwards way of enabling
>an ExtINT, as such it shouldn't be used until later.

>YH do you have any insight why on some Nvidia chipsets we apic 0 pin 2
doesn't
>work for the timer interrupt.  I thought that was what we were using in
LinuxBIOS
>for the mptable.

CK804's has problem. But later one seems fixed that problem.

YH

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

"Lu, Yinghai" <yinghai.lu@amd.com> writes:

> -----Original Message-----
> From: ebiederm@xmission.com [mailto:ebiederm@xmission.com] 
> Sent: Thursday, December 21, 2006 12:47 PM
> To: Lu, Yinghai
>>> +static int add_irq_entry(int type, int irqflag, int bus, int irq,
> int apic, int
>>> pin)
>
>>This is fairly sane but probably belongs in mptable.c as a helper.
>
> mparse.c?

yep.

>>I am still trying to understand this enable_8259A_irq(0) case.
>>As far as I can tell this is a very backwards way of enabling
>>an ExtINT, as such it shouldn't be used until later.
>
>>YH do you have any insight why on some Nvidia chipsets we apic 0 pin 2
> doesn't
>>work for the timer interrupt.  I thought that was what we were using in
> LinuxBIOS
>>for the mptable.
>
> CK804's has problem. But later one seems fixed that problem.

Do you have any details?

Eric

Linux 2.6.20-rc1

[Linus Torvalds]

Ok, the two-week merge period is over, and -rc1 is out there.

I'm _really_ hoping that we can keep the 2.6.20 release calmer and without 
any of the dragging-out-due-to-core-changes that we've had lately. We 
didn't actually merge any really core changes here, with the biggest 
conceptual one being the "work_struct" split into regular work and 
"delayed" work, so I'm hoping we can really end up with an easy 2.6.20 
release.

Some of the commits there are pretty big patches, but more than a couple 
of them are due to fairly straightforward search-and-replace things (like 
a largely scripted removal of unnecessary casts of the return value of 
"kmalloc()", for example, or the switch to "ktermios" for the tty layer, 
or the introduction of "struct path" in the VFS layer instead of keeping 
the f_{dentry,vfsmnt} entries separate, or indeed the removal of SLAB_xxx 
constant names in favour of the standard GFP_xxx ones).

So while the patch itself isn't actually all that much smaller than usual, 
at least my personal gut feel is that the actual changes are not as 
intrusive, just in some cases have big diffs.

But both the diffstat and the shortlog are still too big to fit in the 
kernel mailing list limits, so you'll just have to take my word for it. Or 
get the git repo, and do your own delving into things with

	git log v2.6.19..v2.6.20-rc1 | git shortlog

There _are_ a few areas of note:

 - the aforementioned "workqueue" changes (where we still have some work 
   to do to finalize the proper actions on all architectures: it's being 
   somewhat discussed on the arch mailing lists, hopefully we'll have it 
   all resolved by -rc2, and it doesn't really worry me)

 - lockless page cache (RCU lookups of radix trees)

 - kvm driver for all those crazy virtualization people to play with

 - networking updates (DCCP, address-family agnostic connection tracking 
   in netfilter, sparse byte order annotations, yadda yadda)

 - HID layer separated out of the USB stuff (bluetooth apparently wants 
   the HID stuff too)

 - tons and tons of driver (ftape removal, ATA, pcmcia, i2c, 
   infiniband, dvb, networking..) and architecture updates (arm, mips, 
   powerpc, sh)

and probably some I just forgot about entirely.

		Linus

Re: IO-APIC + timer doesn't work (was: Linux 2.6.20-rc1)

[Linus Torvalds]

On Sat, 16 Dec 2006, Tobias Diedrich wrote:
> 
> 2.6.20-rc1 won't boot with the error message "IO-APIC + timer
> doesn't work! Try using the 'noapic' kernel parameter".
> However, IO-APIC seems to work just fine with 2.6.19-rc6 and I'd
> rather like to continue using it. :)

Can you try "git bisect" on this?

It's really easy: since you know that v2.6.20-rc1 doesn't work, and 
v2.6.19-rc6 _does_ work, just get the current kernel git tree, and then do

	git bisect start
	git bisect good v2.6.19-rc6
	git bisect bad v2.6.20-rc1

and it will pick a kernel for you to try. Just compile that, boot with it, 
and if it works, say "git bisect good" (and if it doesn't work, just do 
"git bisect bad" instead). It will give you a new kernel, and in a few 
tries you'll have been able to narrow down exactly where it breaks (ok, 
more than "a few" - there's 3728 commits in that range, so it's more like 
"twelve reboots later").

That said, it's likely one of not a lot of commits that are broken, as 
shown by

	git log v2.6.19-rc6..v2.6.20-rc1 arch/x86_64/kernel/io_apic.c

and it's *probably* commit b0268726: "[PATCH] x86-64: Try multiple timer 
variants in check_timer"

But a few other people also added to the Cc in case they have ideas.

			Linus

*** snip snip, left the most relevant info here
***
*** others: please look up the post on linux-kernel
*** for config info if you care

> http://tdiedrich.de/~ranma/2.6.20-rc1-oops.jpg
> (netconsole is configured but doesn't work for some reason, haven't
> looked into that so far)
> [...]
> ENABLING IO-APIC IRQs
> ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 disabled(3) .. failed
> ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled(7)APIC error on CPU0: 04(40)
>  .. failed
> ..TIMER: trying IO-APIC=0 PIN=2 fallback with 8259 IRQ0 disabled(3) .. failed
> ...trying to set up timer as Virtual Wire IRQ... failed.
> ...trying to set up timer as ExtINT IRQ... failed :(.
> Kernel panic - not syncing: IO-APIC + timer doesn't work! Try using the 'noapic' kernel parameter
> 
> The board is an ASUS M2N SLI Deluxe (Athlon64/nforce).

Re: IO-APIC + timer doesn't work (was: Linux 2.6.20-rc1)

[Linus Torvalds]

On Sun, 17 Dec 2006, Tobias Diedrich wrote:
>
> With commit b0268726 backed out, 2.6.20-rc1 boots fine.

Ok. It's sad, because that thing really did clean stuff up, and seemed 
like a nice and robust approach.

Your dmesg is kind of interesting:

..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled(7)APIC error on CPU0: 04(40)
 .. failed

where that APIC error on CPU0 seems to be a "Send accept error" and "Send 
illegal vector" thing. I think we actually got the interrupt there, but 
because we had some APIC setup bug, we didn't accept it properly, and it 
resulted in that "APIC error" thing. Maybe. 

This is a long shot, but I wonder if we should _wait_ for the APIC to 
stabilize after we've unmasked the IRQ. Ie, if you could undo the back-out 
(going back to the broken situation), and try the patch below, and see if 
it makes a difference.

Unlikely, I know. I don't see anything wrong with the code, though, but 
maybe I'm just blind.

Eric, Andi, Yinghai, do you see anything here to explain why that commit 
breaks?

		Linus

---
diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 2a1dcd5..a8a09e0 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -294,7 +294,7 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 
 DO_ACTION( __mask,             0, |= 0x00010000, io_apic_sync(entry->apic) )
 						/* mask = 1 */
-DO_ACTION( __unmask,           0, &= 0xfffeffff, )
+DO_ACTION( __unmask,           0, &= 0xfffeffff, io_apic_sync(entry->apic) )
 						/* mask = 0 */
 
 static void mask_IO_APIC_irq (unsigned int irq)

Re: IO-APIC + timer doesn't work

[Linus Torvalds]

On Sun, 17 Dec 2006, Tobias Diedrich wrote:
> 
> No such luck, it still panics and the APIC error is also unchanged.

Ok. I don't see anything wrong off-hand, but I'll keep the patch in the 
tree in the hopes that Andi and/or Eric can see what's wrong and solve it.

If we don't find a solution, I'll have to revert it, but let's give it a 
few more days. 

Tobias, can you please make sure to remind me about this if nothing seems 
to happen? 

Thanks,

			Linus

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 17 Dec 2006, Tobias Diedrich wrote:
>> 
>> No such luck, it still panics and the APIC error is also unchanged.
>
> Ok. I don't see anything wrong off-hand, but I'll keep the patch in the 
> tree in the hopes that Andi and/or Eric can see what's wrong and solve it.
>
> If we don't find a solution, I'll have to revert it, but let's give it a 
> few more days. 
>
> Tobias, can you please make sure to remind me about this if nothing seems 
> to happen? 

Just skimming for differences the first test seems to be missing an
umask_IO_APIC_irq(0);

It would be good to know which case is working before this change was made.

Eric

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sun, 17 Dec 2006, Tobias Diedrich wrote:
>> 
>> No such luck, it still panics and the APIC error is also unchanged.
>
> Ok. I don't see anything wrong off-hand, but I'll keep the patch in the 
> tree in the hopes that Andi and/or Eric can see what's wrong and solve it.
>
> If we don't find a solution, I'll have to revert it, but let's give it a 
> few more days. 
>
> Tobias, can you please make sure to remind me about this if nothing seems 
> to happen? 

Actually can anyone tell me how try_apic_pin is supposed to work at
all?

It doesn't appear to be programming the io_apic.

So either I am missing something or I have found a real problem.

Eric

Re: IO-APIC + timer doesn't work

[Len Brown]

On Sunday 17 December 2006 00:22, Eric W. Biederman wrote:

> Actually can anyone tell me how try_apic_pin is supposed to work at
> all?
> 
> It doesn't appear to be programming the io_apic.

magic:-)

ACPI can't even _describe_ the scenarios being tried by check_timer(),
which is trying to navigate the minefield of all possible undocumented
chipset dependent bugs.  (ie, tinkering with the PIT when in IOAPIC mode...)

The chipset vendors can create new bugs in this area
faster than we can fix them, and there is a reason for this.

The public info on Windows says that they use 100HZ IRQ0 8254 only for UP.
On SMP, they use 64 HZ RTC on IRQ8 instead.

This means that for the population of system vendors that validate only with Windows,
only those timers are getting validated, and Linux on IRQ0 is exposed to HW bugs.

So the RTC looks like a safe path for a validated periodic ticker
when our first choice doesn't work.  But the RTC isn't without problems.
It can tick only in powers of 2 HZ, and 100/250/300/1000 are not powers of 2.
Dunno if close counts -- 256 is close to our 250, and 1024 is close to our 1000,
but we don't have any choices close to 100 or 300 HZ.

-Len

ps.
Moving to the RTC from the PIT would move us 3 years forward
in hardware technology, from 1981 to 1984:-)

Re: IO-APIC + timer doesn't work

[Tobias Diedrich]

Linus Torvalds wrote:
> On Sun, 17 Dec 2006, Tobias Diedrich wrote:
> > 
> > No such luck, it still panics and the APIC error is also unchanged.
> 
> Ok. I don't see anything wrong off-hand, but I'll keep the patch in the 
> tree in the hopes that Andi and/or Eric can see what's wrong and solve it.
> 
> If we don't find a solution, I'll have to revert it, but let's give it a 
> few more days. 
> 
> Tobias, can you please make sure to remind me about this if nothing seems 
> to happen? 

Sure.

BTW, I'm also wondering if this secondary Oops is supposed to happen:
http://www.tdiedrich.de/~ranma/2.6.20-rc1-oops2.jpg
I guess the NMI watchdog is never disabled after the test failed?

|[68.908000] Kernel panic - not syncing: IO-APIC + timer doesn't work! Try using the 'noapic' kernel parameter
|[68.908002]
[~4 seconds later]
|[68.908300] NMI Watchdog detected LOCKUP on CPU 0
  ^^^^^^^^^ wrong timestamp?
|[73.637325] CPU 0
|[73.637451] Modules linked in:
|[73.637579] Pid: 1, comm: swapper Not tainted 2.6.20-rc1-amd64 #27
[...]

Re: IO-APIC + timer doesn't work

[Linus Torvalds]

On Sun, 17 Dec 2006, Tobias Diedrich wrote:
> 
> BTW, I'm also wondering if this secondary Oops is supposed to happen:

Well, if the timer doesn't work, then the NMI watchdog will trigger. So 
it's "supposed" to happen in the sense that yeah, it's kind of expected, 
but it's really bsically just a secondary issue. If the timer worked 
properly, you'd never see it. 

So it's just fallout from the original problem you have, and not 
interesting in itself.

		Linus

Re: IO-APIC + timer doesn't work (was: Linux 2.6.20-rc1)

[Tobias Diedrich]

Linus Torvalds wrote:

> Your dmesg is kind of interesting:
> 
> ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled(7)APIC error on CPU0: 04(40)
>  .. failed
> 
> where that APIC error on CPU0 seems to be a "Send accept error" and "Send 
> illegal vector" thing. I think we actually got the interrupt there, but 
> because we had some APIC setup bug, we didn't accept it properly, and it 
> resulted in that "APIC error" thing. Maybe. 

I just tried changing the code so the "8259 IRQ0 enabled" case is
tested first and with that it boots fine.


Index: linux-2.6.20-rc1/arch/x86_64/kernel/io_apic.c
===================================================================
--- linux-2.6.20-rc1.orig/arch/x86_64/kernel/io_apic.c	2006-12-17 00:45:57.000000000 +0100
+++ linux-2.6.20-rc1/arch/x86_64/kernel/io_apic.c	2006-12-17 15:39:40.000000000 +0100
@@ -1615,6 +1615,7 @@
 	 */
 	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	init_8259A(1);
+	enable_8259A_irq(0);
 
 	pin1  = find_isa_irq_pin(0, mp_INT);
 	apic1 = find_isa_irq_apic(0, mp_INT);



[    0.000000] Linux version 2.6.20-rc1-amd64 (ranma@melchior) (gcc version 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)) #28 Sun Dec 17 15:40:22 CET 2006
[    0.000000] Command line: root=/dev/sda5 resume=/dev/sda6 vga=6 apic=verbose apic=verbose ro netconsole=@192.168.8.241/,514@255.255.255.255/
[    0.000000] BIOS-provided physical RAM map:
[    0.000000]  BIOS-e820: 0000000000000000 - 000000000009f800 (usable)
[    0.000000]  BIOS-e820: 000000000009f800 - 00000000000a0000 (reserved)
[    0.000000]  BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
[    0.000000]  BIOS-e820: 0000000000100000 - 000000003fee0000 (usable)
[    0.000000]  BIOS-e820: 000000003fee0000 - 000000003fee3000 (ACPI NVS)
[    0.000000]  BIOS-e820: 000000003fee3000 - 000000003fef0000 (ACPI data)
[    0.000000]  BIOS-e820: 000000003fef0000 - 000000003ff00000 (reserved)
[    0.000000]  BIOS-e820: 00000000f0000000 - 00000000f4000000 (reserved)
[    0.000000]  BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
[    0.000000] Entering add_active_range(0, 0, 159) 0 entries of 256 used
[    0.000000] Entering add_active_range(0, 256, 261856) 1 entries of 256 used
[    0.000000] end_pfn_map = 1048576
[    0.000000] DMI 2.3 present.
[    0.000000] ACPI: RSDP (v000 Nvidia                                ) @ 0x00000000000f7ce0
[    0.000000] ACPI: RSDT (v001 Nvidia AWRDACPI 0x42302e31 AWRD 0x00000000) @ 0x000000003fee3040
[    0.000000] ACPI: FADT (v001 Nvidia AWRDACPI 0x42302e31 AWRD 0x00000000) @ 0x000000003fee30c0
[    0.000000] ACPI: SSDT (v001 PTLTD  POWERNOW 0x00000001  LTP 0x00000001) @ 0x000000003feec2c0
[    0.000000] ACPI: MCFG (v001 Nvidia AWRDACPI 0x42302e31 AWRD 0x00000000) @ 0x000000003feec400
[    0.000000] ACPI: MADT (v001 Nvidia AWRDACPI 0x42302e31 AWRD 0x00000000) @ 0x000000003feec200
[    0.000000] ACPI: DSDT (v001 NVIDIA AWRDACPI 0x00001000 MSFT 0x0100000e) @ 0x0000000000000000
[    0.000000] Entering add_active_range(0, 0, 159) 0 entries of 256 used
[    0.000000] Entering add_active_range(0, 256, 261856) 1 entries of 256 used
[    0.000000] Zone PFN ranges:
[    0.000000]   DMA             0 ->     4096
[    0.000000]   DMA32        4096 ->  1048576
[    0.000000]   Normal    1048576 ->  1048576
[    0.000000] early_node_map[2] active PFN ranges
[    0.000000]     0:        0 ->      159
[    0.000000]     0:      256 ->   261856
[    0.000000] On node 0 totalpages: 261759
[    0.000000]   DMA zone: 56 pages used for memmap
[    0.000000]   DMA zone: 1356 pages reserved
[    0.000000]   DMA zone: 2587 pages, LIFO batch:0
[    0.000000]   DMA32 zone: 3524 pages used for memmap
[    0.000000]   DMA32 zone: 254236 pages, LIFO batch:31
[    0.000000]   Normal zone: 0 pages used for memmap
[    0.000000] Nvidia board detected. Ignoring ACPI timer override.
[    0.000000] If you got timer trouble try acpi_use_timer_override
[    0.000000] ACPI: PM-Timer IO Port: 0x1008
[    0.000000] ACPI: Local APIC address 0xfee00000
[    0.000000] ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled)
[    0.000000] Processor #0 (Bootup-CPU)
[    0.000000] ACPI: LAPIC (acpi_id[0x01] lapic_id[0x01] disabled)
[    0.000000] ACPI: LAPIC_NMI (acpi_id[0x00] high edge lint[0x1])
[    0.000000] ACPI: LAPIC_NMI (acpi_id[0x01] high edge lint[0x1])
[    0.000000] ACPI: IOAPIC (id[0x02] address[0xfec00000] gsi_base[0])
[    0.000000] IOAPIC[0]: apic_id 2, address 0xfec00000, GSI 0-23
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 14 high edge)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 15 high edge)
[    0.000000] ACPI: IRQ9 used by override.
[    0.000000] ACPI: IRQ14 used by override.
[    0.000000] ACPI: IRQ15 used by override.
[    0.000000] Setting APIC routing to flat
[    0.000000] Using ACPI (MADT) for SMP configuration information
[    0.000000] mapped APIC to ffffffffff5fd000 (        fee00000)
[    0.000000] mapped IOAPIC to ffffffffff5fc000 (00000000fec00000)
[    0.000000] Nosave address range: 000000000009f000 - 00000000000a0000
[    0.000000] Nosave address range: 00000000000a0000 - 00000000000f0000
[    0.000000] Nosave address range: 00000000000f0000 - 0000000000100000
[    0.000000] Allocating PCI resources starting at 40000000 (gap: 3ff00000:b0100000)
[    0.000000] Built 1 zonelists.  Total pages: 256823
[    0.000000] Kernel command line: root=/dev/sda5 resume=/dev/sda6 vga=6 apic=verbose apic=verbose ro netconsole=@192.168.8.241/,514@255.255.255.255/
[    0.000000] netconsole: local port 6665
[    0.000000] netconsole: local IP 192.168.8.241
[    0.000000] netconsole: interface eth0
[    0.000000] netconsole: remote port 514
[    0.000000] netconsole: remote IP 255.255.255.255
[    0.000000] netconsole: remote ethernet address ff:ff:ff:ff:ff:ff
[    0.000000] Initializing CPU#0
[    0.000000] PID hash table entries: 4096 (order: 12, 32768 bytes)
[   27.616056] time.c: Using 3.579545 MHz WALL PM GTOD PIT/TSC timer.
[   27.616058] time.c: Detected 2009.285 MHz processor.
[   27.621535] Console: colour VGA+ 80x60
[   27.624157] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes)
[   27.624996] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes)
[   27.625240] Checking aperture...
[   27.625347] CPU 0: aperture @ b2c2000000 size 32 MB
[   27.625454] Aperture too small (32 MB)
[   27.630247] No AGP bridge found
[   27.638099] Memory: 1025356k/1047424k available (3261k kernel code, 21436k reserved, 1445k data, 200k init)
[   27.719244] Calibrating delay using timer specific routine.. 4022.22 BogoMIPS (lpj=6701161)
[   27.719489] Mount-cache hash table entries: 256
[   27.719665] CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line)
[   27.719774] CPU: L2 Cache: 512K (64 bytes/line)
[   27.719899] CPU: AMD Athlon(tm) 64 Processor 3200+ stepping 02
[   27.720052] ACPI: Core revision 20060707
[   27.727794] enabled ExtINT on CPU#0
[   27.727901] ESR value after enabling vector: 00000000, after 00000004
[   27.728238] ENABLING IO-APIC IRQs
[   27.728345] init IO_APIC IRQs
[   27.728417]  IO-APIC (apicid-pin) 2-16, 2-17, 2-18, 2-19, 2-20, 2-21, 2-22, 2-23 not connected.
[   27.728548] ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 disabled<6>Using local APIC timer interrupts.
                                                              ^^^^^^^^
                                                              really
                                                              enabled
[   27.811447] result 12558047
[   27.811553] Detected 12.558 MHz APIC timer.

HTH,

-- 
Tobias						PGP: http://9ac7e0bc.uguu.de
This mail is made of 100% recycled bits

Re: IO-APIC + timer doesn't work (was: Linux 2.6.20-rc1)

[Eric W. Biederman]

Tobias Diedrich <ranma@tdiedrich.de> writes:

> Linus Torvalds wrote:
>
>> Your dmesg is kind of interesting:
>> 
>> ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled(7)APIC error on CPU0:
> 04(40)
>>  .. failed
>> 
>> where that APIC error on CPU0 seems to be a "Send accept error" and "Send 
>> illegal vector" thing. I think we actually got the interrupt there, but 
>> because we had some APIC setup bug, we didn't accept it properly, and it 
>> resulted in that "APIC error" thing. Maybe. 
>
> I just tried changing the code so the "8259 IRQ0 enabled" case is
> tested first and with that it boots fine.

Could you try removing the clear_IO_APIC_pin from try_io_apic_pin.

This isn't a complete fix but I believe for your hardware it will
fix the problem and it points at what the real fix is.  

Not properly programming the io_apic for the case we want to test.

Eric

Re: IO-APIC + timer doesn't work (was: Linux 2.6.20-rc1)

[Tobias Diedrich]

Eric W. Biederman wrote:
> Tobias Diedrich <ranma@tdiedrich.de> writes:
> 
> > Linus Torvalds wrote:
> >
> >> Your dmesg is kind of interesting:
> >> 
> >> ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled(7)APIC error on CPU0:
> > 04(40)
> >>  .. failed
> >> 
> >> where that APIC error on CPU0 seems to be a "Send accept error" and "Send 
> >> illegal vector" thing. I think we actually got the interrupt there, but 
> >> because we had some APIC setup bug, we didn't accept it properly, and it 
> >> resulted in that "APIC error" thing. Maybe. 
> >
> > I just tried changing the code so the "8259 IRQ0 enabled" case is
> > tested first and with that it boots fine.
> 
> Could you try removing the clear_IO_APIC_pin from try_io_apic_pin.
> 
> This isn't a complete fix but I believe for your hardware it will
> fix the problem and it points at what the real fix is.  
> 
> Not properly programming the io_apic for the case we want to test.

Yes, this works:

|[   27.535937] init IO_APIC IRQs
|[   27.536009]  IO-APIC (apicid-pin) 2-16, 2-17, 2-18, 2-19, 2-20, 2-21, 2-22, 2-23 not connected.
|[   27.536140] ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 disabled<3> (clear_IO_APIC_pin not called)<3> .. failed
|[   27.569357] ..TIMER: trying IO-APIC=0 PIN=0 with 8259 IRQ0 enabled<3> .. works
|[   27.602547] Using local APIC timer interrupts.

I can also report, that updating the BIOS to version 0609 (released
last week or so, also adds the long-missing HPET support) also makes
the problem go away since the first testcase then already works.
I'm currently running with the BIOS downgraded to version 0402.

|[   23.646371] ENABLING IO-APIC IRQs
|[   23.646477] init IO_APIC IRQs
|[   23.646479]  IO-APIC (apicid-pin) 2-0, 2-16, 2-17, 2-18, 2-19, 2-20, 2-21, 2-22, 2-23 not connected.
|[   23.646674] ..TIMER: trying IO-APIC=0 PIN=2 with 8259 IRQ0 disabled<3> .. works
|[   23.679872] Using local APIC timer interrupts.

Index: linux-2.6.20-rc1/arch/x86_64/kernel/io_apic.c
===================================================================
--- linux-2.6.20-rc1.orig/arch/x86_64/kernel/io_apic.c	2006-12-18 15:56:38.000000000 +0100
+++ linux-2.6.20-rc1/arch/x86_64/kernel/io_apic.c	2006-12-18 16:04:15.000000000 +0100
@@ -1586,9 +1586,11 @@
 			setup_nmi();
 			enable_8259A_irq(0);
 		}
+		apic_printk(APIC_QUIET, KERN_ERR " .. works\n");
 		return 1;
 	}
-	clear_IO_APIC_pin(apic, pin);
+	printk(KERN_ERR " (clear_IO_APIC_pin not called)");
+	/* clear_IO_APIC_pin(apic, pin); */
 	apic_printk(APIC_QUIET, KERN_ERR " .. failed\n");
 	return 0;
 }

HTH,

-- 
Tobias						PGP: http://9ac7e0bc.uguu.de
このメールは十割再利用されたビットで作られています。

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

Tobias Diedrich <ranma+kernel@tdiedrich.de> writes:
> Eric W. Biederman wrote:
>> Could you try removing the clear_IO_APIC_pin from try_io_apic_pin.
>> 
>> This isn't a complete fix but I believe for your hardware it will
>> fix the problem and it points at what the real fix is.  
>> 
>> Not properly programming the io_apic for the case we want to test.
>
> Yes, this works:

Thanks.  The bug is simply that the new code doesn't setup the
ioapic for the cases it intends to test.  But it does clear out
the original programming.  So if the normal good case doesn't work the
code is going to have problems.


> I can also report, that updating the BIOS to version 0609 (released
> last week or so, also adds the long-missing HPET support) also makes
> the problem go away since the first testcase then already works.
> I'm currently running with the BIOS downgraded to version 0402.

Nice to hear, so this is clearly a software setup problem in the BIOS.

Andi do you think you could address this problem?

Eric

Re: IO-APIC + timer doesn't work

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

On 12/18/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Thanks.  The bug is simply that the new code doesn't setup the
> ioapic for the cases it intends to test.  But it does clear out
> the original programming.  So if the normal good case doesn't work the
> code is going to have problems.

Please check the patch.

[-- Attachment #2: timers_12182006.patch --]
[-- Type: text/x-patch, Size: 4598 bytes --]

[PATCH] x86_64: check_timer with io apic setup before try_apic_pin

add io apic setup before try_apic_pin

cc: Andi Kleen <ak@suse.de>
cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 2a1dcd5..06982b4 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -273,10 +273,17 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	struct irq_pin_list *entry = irq_2_pin + irq;
 
 	BUG_ON(irq >= NR_IRQS);
-	while (entry->next)
+	while (entry->next) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
+		if (entry->pin == -1) 
+			break;
 		entry = irq_2_pin + entry->next;
+	}
 
 	if (entry->pin != -1) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
 		entry->next = first_free_entry;
 		entry = irq_2_pin + entry->next;
 		if (++first_free_entry >= PIN_MAP_SIZE)
@@ -286,6 +293,24 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	entry->pin = pin;
 }
 
+static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
+{
+	struct irq_pin_list *entry = irq_2_pin + irq;
+
+	BUG_ON(irq >= NR_IRQS);
+
+	while (entry) {
+		if (entry->apic == apic && entry->pin == pin) {
+			entry->apic = -1;
+			entry->pin = -1;
+			break;
+		}
+		if (entry->next) 
+			entry = irq_2_pin + entry->next;
+	}
+
+}
+
 
 #define DO_ACTION(name,R,ACTION, FINAL)					\
 									\
@@ -1570,6 +1630,21 @@ static inline void unlock_ExtINT_logic(void)
  * fanatically on his truly buggy board.
  */
 
+static int set_try_apic_pin(int apic, int pin, int irq)
+{
+	int idx;
+	int ret = -1;
+	idx = find_irq_entry(apic,pin,mp_INT);
+
+	if(idx != -1) {
+		add_pin_to_irq(irq, apic, pin);
+		setup_IO_APIC_irq(apic, pin, idx, irq);
+		ret = 0;
+	}
+
+	return ret;
+}
+
 static int try_apic_pin(int apic, int pin, char *msg)
 {
 	apic_printk(APIC_VERBOSE, KERN_INFO
@@ -1588,7 +1663,7 @@ static int try_apic_pin(int apic, int pin, char *msg)
 		}
 		return 1;
 	}
-	clear_IO_APIC_pin(apic, pin);
+
 	apic_printk(APIC_QUIET, KERN_ERR " .. failed\n");
 	return 0;
 }
@@ -1599,12 +1674,12 @@ static void check_timer(void)
 	int apic1, pin1, apic2, pin2;
 	int vector;
 	cpumask_t mask;
+	int i;
 
 	/*
 	 * get/set the timer IRQ vector:
 	 */
 	disable_8259A_irq(0);
-	vector = assign_irq_vector(0, TARGET_CPUS, &mask);
 
 	/*
 	 * Subtle, code in do_timer_interrupt() expects an AEOI
@@ -1622,32 +1697,49 @@ static void check_timer(void)
 	apic2 = ioapic_i8259.apic;
 
 	/* Do this first, otherwise we get double interrupts on ATI boards */
-	if ((pin1 != -1) && try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
-		return;
+	if (pin1 != -1) {
+		/* set_try_apic_pin will call disable_8259A_irq */
+		set_try_apic_pin(apic1, pin1, 0);
+		unmask_IO_APIC_irq(0);
+		if (try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
+			return;
 
-	/* Now try again with IRQ0 8259A enabled.
-	   Assumes timer is on IO-APIC 0 ?!? */
-	enable_8259A_irq(0);
-	unmask_IO_APIC_irq(0);
-	if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
-		return;
-	disable_8259A_irq(0);
+		/* Now try again with IRQ0 8259A enabled.
+		   Assumes timer is on IO-APIC 0 ?!? */
+		enable_8259A_irq(0);
+		if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
+			return;
+		disable_8259A_irq(0);
+
+    	        clear_IO_APIC_pin(apic1, pin1);
+		remove_pin_to_irq(0, apic1, pin1);
+	}
 
 	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
 	   on Nvidia boards */
-	if (!(apic1 == 0 && pin1 == 0) &&
-	    try_apic_pin(0, 0, "fallback with 8259 IRQ0 disabled"))
-		return;
-	if (!(apic1 == 0 && pin1 == 2) &&
-	    try_apic_pin(0, 2, "fallback with 8259 IRQ0 disabled"))
-		return;
+	for (i = 0; i <= 2; i += 2) 
+	if (!(apic1 == 0 && pin1 == i)) {
+		/* set_try_apic_pin will call disable_8259A_irq */
+		if (!set_try_apic_pin(0, i, 0) ) {
+			unmask_IO_APIC_irq(0);
+			if (try_apic_pin(0, i, "fallback with 8259 IRQ0 disabled"))
+				return;
+			clear_IO_APIC_pin(0, i);
+			remove_pin_to_irq(0, 0, i);
+	    	}
+	}
+
+	vector = assign_irq_vector(0, TARGET_CPUS, &mask);
 
 	/* Then try pure 8259A routing on the 8259 as reported by BIOS*/
-	enable_8259A_irq(0);
 	if (pin2 != -1) {
 		setup_ExtINT_IRQ0_pin(apic2, pin2, vector);
+		add_pin_to_irq(0, apic2, pin2);
+		enable_8259A_irq(0);
 		if (try_apic_pin(apic2,pin2,"8259A broadcast ExtINT from BIOS"))
 			return;
+		clear_IO_APIC_pin(apic2, pin2);
+		remove_pin_to_irq(0, apic2, pin2);
 	}
 
 	/* Tried all possibilities to go through the IO-APIC. Now come the

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

"Yinghai Lu" <yinghai.lu@amd.com> writes:

> On 12/18/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Thanks.  The bug is simply that the new code doesn't setup the
>> ioapic for the cases it intends to test.  But it does clear out
>> the original programming.  So if the normal good case doesn't work the
>> code is going to have problems.
>
> Please check the patch.

Getting there but I don't think we are quite there yet.

One of the issues that this does not address is that currently our probe
order in check_timer is wrong.  We should first check what the BIOS
has told us about.  And only if that fails should we start guessing,
common configurations.

So the pin2 case should be tested right after the pin1 case as we do
currently.  On most new boards that will be a complete noop.

But it is better than our current blind guess at using ExtINT mode.

I figure after we try what the BIOS has told us about and that
has failed we should first try the common irq 0 apic mappings,
and then try the common ExtINT mappings.

The current code causes me to want to scream, it is so silly.

Eric

Re: IO-APIC + timer doesn't work

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

On 12/19/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
> So the pin2 case should be tested right after the pin1 case as we do
> currently.  On most new boards that will be a complete noop.
>
> But it is better than our current blind guess at using ExtINT mode.
>
> I figure after we try what the BIOS has told us about and that
> has failed we should first try the common irq 0 apic mappings,
> and then try the common ExtINT mappings.

Please check if this one is ok.

[-- Attachment #2: timers_12192006.patch --]
[-- Type: text/x-patch, Size: 6199 bytes --]

[PATCH] x86_64: check_timer with io apic setup before try_apic_pin

add io apic setup before try_apic_pin

cc: Andi Kleen <ak@suse.de>
cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 2a1dcd5..6d09fc0 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -273,10 +273,17 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	struct irq_pin_list *entry = irq_2_pin + irq;
 
 	BUG_ON(irq >= NR_IRQS);
-	while (entry->next)
+	while (entry->next) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
+		if (entry->pin == -1) 
+			break;
 		entry = irq_2_pin + entry->next;
+	}
 
 	if (entry->pin != -1) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
 		entry->next = first_free_entry;
 		entry = irq_2_pin + entry->next;
 		if (++first_free_entry >= PIN_MAP_SIZE)
@@ -286,6 +293,24 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	entry->pin = pin;
 }
 
+static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
+{
+	struct irq_pin_list *entry = irq_2_pin + irq;
+
+	BUG_ON(irq >= NR_IRQS);
+
+	while (entry) {
+		if (entry->apic == apic && entry->pin == pin) {
+			entry->apic = -1;
+			entry->pin = -1;
+			break;
+		}
+		if (entry->next) 
+			entry = irq_2_pin + entry->next;
+	}
+
+}
+
 
 #define DO_ACTION(name,R,ACTION, FINAL)					\
 									\
@@ -367,6 +392,34 @@ static int find_irq_entry(int apic, int pin, int type)
 	return -1;
 }
 
+static int add_irq_entry(int type, int irqflag, int bus, int irq, int apic, int pin)
+{
+        struct mpc_config_intsrc intsrc;
+	int idx;
+
+        intsrc.mpc_type = MP_INTSRC;
+        intsrc.mpc_irqflag = irqflag; /* conforming */
+        intsrc.mpc_srcbus = bus;
+        intsrc.mpc_dstapic = (apic != -1) ? mp_ioapics[apic].mpc_apicid: MP_APIC_ALL;
+
+        intsrc.mpc_irqtype = type;
+
+        intsrc.mpc_srcbusirq = irq;
+        intsrc.mpc_dstirq = pin;
+
+        mp_irqs [mp_irq_entries] = intsrc;
+        Dprintk("Int: type %d, pol %d, trig %d, bus %d,"
+                " IRQ %02x, APIC ID %x, APIC INT %02x\n",
+                        intsrc.mpc_irqtype, intsrc.mpc_irqflag & 3,
+                        (intsrc.mpc_irqflag >> 2) & 3, intsrc.mpc_srcbus,
+                        intsrc.mpc_srcbusirq, intsrc.mpc_dstapic, intsrc.mpc_dstirq);
+        idx = mp_irq_entries;
+	if (++mp_irq_entries >= MAX_IRQ_SOURCES)
+                panic("Max # of irq sources exceeded!!\n");
+	return idx;
+
+}
+
 /*
  * Find the pin to which IRQ[irq] (ISA) is connected
  */
@@ -1570,6 +1658,22 @@ static inline void unlock_ExtINT_logic(void)
  * fanatically on his truly buggy board.
  */
 
+static void set_try_apic_pin(int apic, int pin, int type)
+{
+	int idx;
+	int irq = 0;
+	int bus = 0; /* MP_ISA_BUS */
+	int irqflag = 5; /* MP_IRQ_TRIGGER_EDGE|MP_IRQ_POLARITY_HIGH */
+
+	idx = find_irq_entry(apic,pin,type);
+
+	if (idx == -1) 
+		idx = add_irq_entry(type, irqflag, bus, irq, apic, pin);
+
+	add_pin_to_irq(irq, apic, pin);
+	setup_IO_APIC_irq(apic, pin, idx, irq);
+}
+
 static int try_apic_pin(int apic, int pin, char *msg)
 {
 	apic_printk(APIC_VERBOSE, KERN_INFO
@@ -1588,7 +1692,7 @@ static int try_apic_pin(int apic, int pin, char *msg)
 		}
 		return 1;
 	}
-	clear_IO_APIC_pin(apic, pin);
+
 	apic_printk(APIC_QUIET, KERN_ERR " .. failed\n");
 	return 0;
 }
@@ -1599,12 +1703,13 @@ static void check_timer(void)
 	int apic1, pin1, apic2, pin2;
 	int vector;
 	cpumask_t mask;
+	int i;
 
 	/*
 	 * get/set the timer IRQ vector:
 	 */
-	disable_8259A_irq(0);
 	vector = assign_irq_vector(0, TARGET_CPUS, &mask);
+	disable_8259A_irq(0);
 
 	/*
 	 * Subtle, code in do_timer_interrupt() expects an AEOI
@@ -1621,33 +1726,51 @@ static void check_timer(void)
 	pin2  = ioapic_i8259.pin;
 	apic2 = ioapic_i8259.apic;
 
-	/* Do this first, otherwise we get double interrupts on ATI boards */
-	if ((pin1 != -1) && try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
-		return;
+	apic_printk(APIC_VERBOSE,KERN_INFO "..TIMER: vector=0x%02X apic1=%d pin1=%d apic2=%d pin2=%d\n",
+		vector, apic1, pin1, apic2, pin2);
 
-	/* Now try again with IRQ0 8259A enabled.
-	   Assumes timer is on IO-APIC 0 ?!? */
-	enable_8259A_irq(0);
-	unmask_IO_APIC_irq(0);
-	if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
-		return;
-	disable_8259A_irq(0);
+	if (pin1 != -1) {
+		/* Do this first, otherwise we get double interrupts on ATI boards */
+		/* set_try_apic_pin will call disable_8259A_irq */
+		set_try_apic_pin(apic1, pin1, mp_INT);
+		unmask_IO_APIC_irq(0);
+		if (try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
+			return;
 
-	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
-	   on Nvidia boards */
-	if (!(apic1 == 0 && pin1 == 0) &&
-	    try_apic_pin(0, 0, "fallback with 8259 IRQ0 disabled"))
-		return;
-	if (!(apic1 == 0 && pin1 == 2) &&
-	    try_apic_pin(0, 2, "fallback with 8259 IRQ0 disabled"))
-		return;
+		/* Now try again with IRQ0 8259A enabled.
+		   Assumes timer is on IO-APIC 0 ?!? */
+		enable_8259A_irq(0);
+		if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
+			return;
+		disable_8259A_irq(0);
+
+    	        clear_IO_APIC_pin(apic1, pin1);
+		remove_pin_to_irq(0, apic1, pin1);
+	}
 
 	/* Then try pure 8259A routing on the 8259 as reported by BIOS*/
-	enable_8259A_irq(0);
 	if (pin2 != -1) {
 		setup_ExtINT_IRQ0_pin(apic2, pin2, vector);
+		add_pin_to_irq(0, apic2, pin2);
+		enable_8259A_irq(0);
 		if (try_apic_pin(apic2,pin2,"8259A broadcast ExtINT from BIOS"))
 			return;
+		clear_IO_APIC_pin(apic2, pin2);
+		remove_pin_to_irq(0, apic2, pin2);
+	}
+
+	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
+	   on Nvidia boards */
+	for (i = 0; i <= 2; i += 2) 
+	if (!(apic1 == 0 && pin1 == i)) {
+		/* set_try_apic_pin will call disable_8259A_irq */
+		set_try_apic_pin(0, i, mp_INT); 
+		unmask_IO_APIC_irq(0);
+		if (try_apic_pin(0, i, "fallback with 8259 IRQ0 disabled"))
+			return;
+
+		clear_IO_APIC_pin(0, i);
+		remove_pin_to_irq(0, 0, i);
 	}
 
 	/* Tried all possibilities to go through the IO-APIC. Now come the

Re: IO-APIC + timer doesn't work

[Tobias Diedrich]

Yinghai Lu wrote:
> On 12/19/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
> >So the pin2 case should be tested right after the pin1 case as we do
> >currently.  On most new boards that will be a complete noop.
> >
> >But it is better than our current blind guess at using ExtINT mode.
> >
> >I figure after we try what the BIOS has told us about and that
> >has failed we should first try the common irq 0 apic mappings,
> >and then try the common ExtINT mappings.
> 
> Please check if this one is ok.

Works fine for me.

FYI I'm off to my parents from Saturday onward, so after that I
can't test any patches for the next one or two weeks.

-- 
Tobias						PGP: http://9ac7e0bc.uguu.de
このメールは十割再利用されたビットで作られています。

Re: IO-APIC + timer doesn't work

[Eric W. Biederman]

"Yinghai Lu" <yinghai.lu@amd.com> writes:

> On 12/19/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> So the pin2 case should be tested right after the pin1 case as we do
>> currently.  On most new boards that will be a complete noop.
>>
>> But it is better than our current blind guess at using ExtINT mode.
>>
>> I figure after we try what the BIOS has told us about and that
>> has failed we should first try the common irq 0 apic mappings,
>> and then try the common ExtINT mappings.
>
> Please check if this one is ok.
>
> [PATCH] x86_64: check_timer with io apic setup before try_apic_pin
>
> add io apic setup before try_apic_pin
>
> cc: Andi Kleen <ak@suse.de>
> cc: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>
>
> diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
> index 2a1dcd5..6d09fc0 100644
> --- a/arch/x86_64/kernel/io_apic.c
> +++ b/arch/x86_64/kernel/io_apic.c
> @@ -273,10 +273,17 @@ static void add_pin_to_irq(unsigned int irq, int apic, int
> pin)
>  	struct irq_pin_list *entry = irq_2_pin + irq;
>  
>  	BUG_ON(irq >= NR_IRQS);
> -	while (entry->next)
> +	while (entry->next) {
> +		if (entry->apic == apic && entry->pin == pin) 
> +			return;
> +		if (entry->pin == -1) 
> +			break;
>  		entry = irq_2_pin + entry->next;
> +	}
>  
>  	if (entry->pin != -1) {
> +		if (entry->apic == apic && entry->pin == pin) 
> +			return;
>  		entry->next = first_free_entry;
>  		entry = irq_2_pin + entry->next;
>  		if (++first_free_entry >= PIN_MAP_SIZE)

This change to add_pin_to_irq looks dubious.

We especially shouldn't hit a pin == -1 while next is still valid.
The problem is that the code that reads this at irq time does not
skip entries with entry->pin == -1.

Fixing the infrastructure should probably be a separate patch
so we don't get too many concepts confused in here.

> @@ -286,6 +293,24 @@ static void add_pin_to_irq(unsigned int irq, int apic, int
> pin)
>  	entry->pin = pin;
>  }
>  
> +static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
> +{
> +	struct irq_pin_list *entry = irq_2_pin + irq;
> +
> +	BUG_ON(irq >= NR_IRQS);
> +
> +	while (entry) {
> +		if (entry->apic == apic && entry->pin == pin) {
> +			entry->apic = -1;
> +			entry->pin = -1;
> +			break;
> +		}
> +		if (entry->next) 
> +			entry = irq_2_pin + entry->next;
> +	}
> +
> +}
> +
This change to remove_pin_to_irq is simply wrong.

> +static int add_irq_entry(int type, int irqflag, int bus, int irq, int apic, int
> pin)
> +{
> +        struct mpc_config_intsrc intsrc;
> +	int idx;
> +
> +        intsrc.mpc_type = MP_INTSRC;
> +        intsrc.mpc_irqflag = irqflag; /* conforming */
> +        intsrc.mpc_srcbus = bus;
> + intsrc.mpc_dstapic = (apic != -1) ? mp_ioapics[apic].mpc_apicid: MP_APIC_ALL;
> +
> +        intsrc.mpc_irqtype = type;
> +
> +        intsrc.mpc_srcbusirq = irq;
> +        intsrc.mpc_dstirq = pin;
> +
> +        mp_irqs [mp_irq_entries] = intsrc;
> +        Dprintk("Int: type %d, pol %d, trig %d, bus %d,"
> +                " IRQ %02x, APIC ID %x, APIC INT %02x\n",
> +                        intsrc.mpc_irqtype, intsrc.mpc_irqflag & 3,
> +                        (intsrc.mpc_irqflag >> 2) & 3, intsrc.mpc_srcbus,
> + intsrc.mpc_srcbusirq, intsrc.mpc_dstapic, intsrc.mpc_dstirq);
> +        idx = mp_irq_entries;
> +	if (++mp_irq_entries >= MAX_IRQ_SOURCES)
> +                panic("Max # of irq sources exceeded!!\n");
> +	return idx;

This is fairly sane but probably belongs in mptable.c as a helper.

>  /*
>   * Find the pin to which IRQ[irq] (ISA) is connected
>   */
> @@ -1570,6 +1658,22 @@ static inline void unlock_ExtINT_logic(void)
>   * fanatically on his truly buggy board.
>   */
>  
> +static void set_try_apic_pin(int apic, int pin, int type)
> +{
> +	int idx;
> +	int irq = 0;
> +	int bus = 0; /* MP_ISA_BUS */
> +	int irqflag = 5; /* MP_IRQ_TRIGGER_EDGE|MP_IRQ_POLARITY_HIGH */
> +
> +	idx = find_irq_entry(apic,pin,type);
> +
> +	if (idx == -1) 
> +		idx = add_irq_entry(type, irqflag, bus, irq, apic, pin);
> +
> +	add_pin_to_irq(irq, apic, pin);
> +	setup_IO_APIC_irq(apic, pin, idx, irq);
> +}
> +
>  static int try_apic_pin(int apic, int pin, char *msg)
>  {
>  	apic_printk(APIC_VERBOSE, KERN_INFO
> @@ -1588,7 +1692,7 @@ static int try_apic_pin(int apic, int pin, char *msg)
>  		}
>  		return 1;
>  	}
> -	clear_IO_APIC_pin(apic, pin);
> +
>  	apic_printk(APIC_QUIET, KERN_ERR " .. failed\n");
>  	return 0;
>  }
> @@ -1599,12 +1703,13 @@ static void check_timer(void)
>  	int apic1, pin1, apic2, pin2;
>  	int vector;
>  	cpumask_t mask;
> +	int i;
>  
>  	/*
>  	 * get/set the timer IRQ vector:
>  	 */
> -	disable_8259A_irq(0);
>  	vector = assign_irq_vector(0, TARGET_CPUS, &mask);
> +	disable_8259A_irq(0);

Moving disable_8259A_irq(0) appears to be useless code motion.
  
>  	/*
>  	 * Subtle, code in do_timer_interrupt() expects an AEOI
> @@ -1621,33 +1726,51 @@ static void check_timer(void)
>  	pin2  = ioapic_i8259.pin;
>  	apic2 = ioapic_i8259.apic;
>  
> -	/* Do this first, otherwise we get double interrupts on ATI boards */
> - if ((pin1 != -1) && try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
> -		return;
> + apic_printk(APIC_VERBOSE,KERN_INFO "..TIMER: vector=0x%02X apic1=%d pin1=%d
> apic2=%d pin2=%d\n",
> +		vector, apic1, pin1, apic2, pin2);
>  
> -	/* Now try again with IRQ0 8259A enabled.
> -	   Assumes timer is on IO-APIC 0 ?!? */
> -	enable_8259A_irq(0);
> -	unmask_IO_APIC_irq(0);
> -	if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
> -		return;
> -	disable_8259A_irq(0);
> +	if (pin1 != -1) {
> + /* Do this first, otherwise we get double interrupts on ATI boards */
> +		/* set_try_apic_pin will call disable_8259A_irq */
> +		set_try_apic_pin(apic1, pin1, mp_INT);
> +		unmask_IO_APIC_irq(0);
> +		if (try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
> +			return;
>  
> -	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
> -	   on Nvidia boards */
> -	if (!(apic1 == 0 && pin1 == 0) &&
> -	    try_apic_pin(0, 0, "fallback with 8259 IRQ0 disabled"))
> -		return;
> -	if (!(apic1 == 0 && pin1 == 2) &&
> -	    try_apic_pin(0, 2, "fallback with 8259 IRQ0 disabled"))
> -		return;
> +		/* Now try again with IRQ0 8259A enabled.
> +		   Assumes timer is on IO-APIC 0 ?!? */
> +		enable_8259A_irq(0);
> +		if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
> +			return;
> +		disable_8259A_irq(0);

I am still trying to understand this enable_8259A_irq(0) case.
As far as I can tell this is a very backwards way of enabling
an ExtINT, as such it shouldn't be used until later.

YH do you have any insight why on some Nvidia chipsets we apic 0 pin 2 doesn't
work for the timer interrupt.  I thought that was what we were using in LinuxBIOS
for the mptable.

Eric

Re: IO-APIC + timer doesn't work

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 35 bytes --]

Please check the revised patch

YH

[-- Attachment #2: timers_12312006.diff --]
[-- Type: text/x-patch, Size: 7190 bytes --]

[PATCH] x86_64: check_timer with io apic setup before try_apic_pin

add io apic setup before try_apic_pin for check_timer

also add remove_irq_to_pin call in io_apic.c

cc: Andi Kleen <ak@suse.de>
cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>

diff --git a/include/asm-x86_64/mpspec.h b/include/asm-x86_64/mpspec.h
index 017fddb..1ddfd4d 100644
--- a/include/asm-x86_64/mpspec.h
+++ b/include/asm-x86_64/mpspec.h
@@ -165,6 +165,7 @@ extern int mp_bus_id_to_pci_bus [MAX_MP_BUSSES];
 extern unsigned int boot_cpu_physical_apicid;
 extern int smp_found_config;
 extern void find_smp_config (void);
+extern int add_irq_entry (int type, int irqflag, int bus, int irq, int apic, int pin);
 extern void get_smp_config (void);
 extern int nr_ioapics;
 extern unsigned char apic_version [MAX_APICS];
diff --git a/arch/x86_64/kernel/mpparse.c b/arch/x86_64/kernel/mpparse.c
index 0807256..a054798 100644
--- a/arch/x86_64/kernel/mpparse.c
+++ b/arch/x86_64/kernel/mpparse.c
@@ -314,6 +314,34 @@ static int __init ELCR_trigger(unsigned int irq)
 	return (inb(port) >> (irq & 7)) & 1;
 }
 
+int add_irq_entry(int type, int irqflag, int bus, int irq, int apic, int pin)
+{
+        struct mpc_config_intsrc intsrc;
+	int idx;
+
+        intsrc.mpc_type = MP_INTSRC;
+        intsrc.mpc_irqflag = irqflag; /* conforming */
+        intsrc.mpc_srcbus = bus;
+        intsrc.mpc_dstapic = (apic != -1) ? mp_ioapics[apic].mpc_apicid: MP_APIC_ALL;
+
+        intsrc.mpc_irqtype = type;
+
+        intsrc.mpc_srcbusirq = irq;
+        intsrc.mpc_dstirq = pin;
+
+        mp_irqs [mp_irq_entries] = intsrc;
+        Dprintk("Int: type %d, pol %d, trig %d, bus %d,"
+                " IRQ %02x, APIC ID %x, APIC INT %02x\n",
+                        intsrc.mpc_irqtype, intsrc.mpc_irqflag & 3,
+                        (intsrc.mpc_irqflag >> 2) & 3, intsrc.mpc_srcbus,
+                        intsrc.mpc_srcbusirq, intsrc.mpc_dstapic, intsrc.mpc_dstirq);
+        idx = mp_irq_entries;
+	if (++mp_irq_entries >= MAX_IRQ_SOURCES)
+                panic("Max # of irq sources exceeded!!\n");
+	return idx;
+
+}
+
 static void __init construct_default_ioirq_mptable(int mpc_default_type)
 {
 	struct mpc_config_intsrc intsrc;
diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 2a1dcd5..ad1a28a 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -273,10 +273,17 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	struct irq_pin_list *entry = irq_2_pin + irq;
 
 	BUG_ON(irq >= NR_IRQS);
-	while (entry->next)
+	while (entry->next) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
+		if (entry->pin == -1) 
+			break;
 		entry = irq_2_pin + entry->next;
+	}
 
 	if (entry->pin != -1) {
+		if (entry->apic == apic && entry->pin == pin) 
+			return;
 		entry->next = first_free_entry;
 		entry = irq_2_pin + entry->next;
 		if (++first_free_entry >= PIN_MAP_SIZE)
@@ -286,6 +293,39 @@ static void add_pin_to_irq(unsigned int irq, int apic, int pin)
 	entry->pin = pin;
 }
 
+static void remove_pin_to_irq(unsigned int irq, int apic, int pin)
+{
+	struct irq_pin_list *entry = irq_2_pin + irq;
+	struct irq_pin_list *pri;
+	struct irq_pin_list *next;
+
+	BUG_ON(irq >= NR_IRQS);
+
+	for (;;) {
+		if (entry->apic == apic && entry->pin == pin) {
+			if(entry->next) {
+				next = irq_2_pin + entry->next;
+				entry->apic = next->apic;
+				entry->pin = next->pin;
+				entry->next = next->next;
+				next->apic = -1;
+				next->pin = -1;
+				next->next = 0;
+			} else {
+				entry->apic = -1;
+				entry->pin = -1;
+			}
+			return;
+		}
+		pri = entry;
+		if (pri->next) 
+			entry = irq_2_pin + pri->next;
+		else
+			break;
+	} 
+
+}
+
 
 #define DO_ACTION(name,R,ACTION, FINAL)					\
 									\
@@ -1570,6 +1610,22 @@ static inline void unlock_ExtINT_logic(void)
  * fanatically on his truly buggy board.
  */
 
+static void set_try_apic_pin(int apic, int pin, int type)
+{
+	int idx;
+	int irq = 0;
+	int bus = 0; /* MP_ISA_BUS */
+	int irqflag = 5; /* MP_IRQ_TRIGGER_EDGE|MP_IRQ_POLARITY_HIGH */
+
+	idx = find_irq_entry(apic,pin,type);
+
+	if (idx == -1) 
+		idx = add_irq_entry(type, irqflag, bus, irq, apic, pin);
+
+	add_pin_to_irq(irq, apic, pin);
+	setup_IO_APIC_irq(apic, pin, idx, irq);
+}
+
 static int try_apic_pin(int apic, int pin, char *msg)
 {
 	apic_printk(APIC_VERBOSE, KERN_INFO
@@ -1588,7 +1644,7 @@ static int try_apic_pin(int apic, int pin, char *msg)
 		}
 		return 1;
 	}
-	clear_IO_APIC_pin(apic, pin);
+
 	apic_printk(APIC_QUIET, KERN_ERR " .. failed\n");
 	return 0;
 }
@@ -1599,6 +1655,7 @@ static void check_timer(void)
 	int apic1, pin1, apic2, pin2;
 	int vector;
 	cpumask_t mask;
+	int i;
 
 	/*
 	 * get/set the timer IRQ vector:
@@ -1621,33 +1678,51 @@ static void check_timer(void)
 	pin2  = ioapic_i8259.pin;
 	apic2 = ioapic_i8259.apic;
 
-	/* Do this first, otherwise we get double interrupts on ATI boards */
-	if ((pin1 != -1) && try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
-		return;
+	apic_printk(APIC_VERBOSE,KERN_INFO "..TIMER: vector=0x%02X apic1=%d pin1=%d apic2=%d pin2=%d\n",
+		vector, apic1, pin1, apic2, pin2);
 
-	/* Now try again with IRQ0 8259A enabled.
-	   Assumes timer is on IO-APIC 0 ?!? */
-	enable_8259A_irq(0);
-	unmask_IO_APIC_irq(0);
-	if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
-		return;
-	disable_8259A_irq(0);
+	if (pin1 != -1) {
+		/* Do this first, otherwise we get double interrupts on ATI boards */
+		/* set_try_apic_pin will call disable_8259A_irq */
+		set_try_apic_pin(apic1, pin1, mp_INT);
+		unmask_IO_APIC_irq(0);
+		if (try_apic_pin(apic1, pin1,"with 8259 IRQ0 disabled"))
+			return;
 
-	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
-	   on Nvidia boards */
-	if (!(apic1 == 0 && pin1 == 0) &&
-	    try_apic_pin(0, 0, "fallback with 8259 IRQ0 disabled"))
-		return;
-	if (!(apic1 == 0 && pin1 == 2) &&
-	    try_apic_pin(0, 2, "fallback with 8259 IRQ0 disabled"))
-		return;
+		/* Now try again with IRQ0 8259A enabled.
+		   Assumes timer is on IO-APIC 0 ?!? */
+		enable_8259A_irq(0);
+		if (try_apic_pin(apic1, pin1, "with 8259 IRQ0 enabled"))
+			return;
+		disable_8259A_irq(0);
+
+    	        clear_IO_APIC_pin(apic1, pin1);
+		remove_pin_to_irq(0, apic1, pin1);
+	}
 
 	/* Then try pure 8259A routing on the 8259 as reported by BIOS*/
-	enable_8259A_irq(0);
 	if (pin2 != -1) {
 		setup_ExtINT_IRQ0_pin(apic2, pin2, vector);
+		add_pin_to_irq(0, apic2, pin2);
+		enable_8259A_irq(0);
 		if (try_apic_pin(apic2,pin2,"8259A broadcast ExtINT from BIOS"))
 			return;
+		clear_IO_APIC_pin(apic2, pin2);
+		remove_pin_to_irq(0, apic2, pin2);
+	}
+
+	/* Always try pin0 and pin2 on APIC 0 to handle buggy timer overrides
+	   on Nvidia boards */
+	for (i = 0; i <= 2; i += 2) 
+	if (!(apic1 == 0 && pin1 == i)) {
+		/* set_try_apic_pin will call disable_8259A_irq */
+		set_try_apic_pin(0, i, mp_INT); 
+		unmask_IO_APIC_irq(0);
+		if (try_apic_pin(0, i, "fallback with 8259 IRQ0 disabled"))
+			return;
+
+		clear_IO_APIC_pin(0, i);
+		remove_pin_to_irq(0, 0, i);
 	}
 
 	/* Tried all possibilities to go through the IO-APIC. Now come the