ioapic: when switches to level trigger mode, interrupts raised repeatedly.
PATCH 1/1 ioapic: when switches to level trigger mode, interrupts raised repeatedly.
- the trigger mode is edge at first
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
- Then the mode is switched to level and it's unmasked.
- the bit of irr is set, so the interrupt is raised repeatedly by ioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.
This patch fixes the issues.
After raising edige, clear the bit of irr.
Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1b
Quoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18
--- hw/ioapic.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
hw/ioapic.c +1 -0
@@ -164,6 +164,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)if (level) { s->irr |= mask;ioapic_service (s); + s->irr &= ~mask; } } }
Signature · 1 line
1.7.1.1
Aurelien Jarno
AppliedQuoted 51 lines · Isaku Yamahata
- the trigger mode is edge at first
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
- Then the mode is switched to level and it's unmasked.
- the bit of irr is set, so the interrupt is raised repeatedly byioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.This patch fixes the issues.
After raising edige, clear the bit of irr.Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1bQuoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18
Quoted 4 lines
Signed-off-by Isaku Yamahata <[email protected]>--- hw/ioapic.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
Thanks, applied.
Quoted 17 lines
hw/ioapic.c +1
-0@@ -164,6 +164,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)if (level) { s->irr |= mask;ioapic_service (s); + s->irr &= ~mask; } } }Signature · 1 line
1.7.1.1
Signature · 2 lines
Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.net
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Quoted 81 lines · Aurelien Jarno
Quoted 51 lines · Isaku Yamahata
- the trigger mode is edge at first
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
- Then the mode is switched to level and it's unmasked.
- the bit of irr is set, so the interrupt is raised repeatedly byioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.This patch fixes the issues.
After raising edige, clear the bit of irr.Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1bQuoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18Quoted 4 lines
Signed-off-by Isaku Yamahata <[email protected]>--- hw/ioapic.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)Thanks, applied.
Quoted 17 lines
hw/ioapic.c +1
-0@@ -164,6 +164,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)if (level) { s->irr |= mask;ioapic_service (s); + s->irr &= ~mask; } } }Signature · 1 line
1.7.1.1Signature · 2 lines
Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.net
Signature · 1 line
yamahata
Quoted 2 lines · Isaku Yamahata
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
Quoted 1 line
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
- Then the mode is switched to level and it's unmasked with the same write.
- the bit of irr is set, so the interrupt is raised repeatedly by ioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.
This patch fixes the issues.
When unmasking the pending interrupt and changing the interrupt mode from edge to level, raise the edge interrupt first and then change the interrupt mode. Thus the pending edge interrupt is raised as edge.
Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1b
Quoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18
--- hw/ioapic.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
hw/ioapic.c +27 -0
@@ -257,6 +257,33 @@ ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val) s->ioredtbl[index] &=0xffffffff ; s->ioredtbl[index] |= (uint64_t )val <<32 ; }else { +uint32_t mask =1 << index; +uint8_t trig_mode = + ((s->ioredtbl[index] >> +IOAPIC_LVT_TRIGGER_MODE_SHIFT ) &1 ); +uint8_t trig_mode_new = + ((val >>IOAPIC_LVT_TRIGGER_MODE_SHIFT ) &1 ); +if ((s->irr & mask) && !(val &IOAPIC_LVT_MASKED ) && + trig_mode ==IOAPIC_TRIGGER_EDGE && + trig_mode_new ==IOAPIC_TRIGGER_LEVEL ) { +/* +* edge interrupt was raised and it has been masked. +* Now it is unmasked and its mode is changed to level. +* +* The device thinks it raised the interrupts as edge, +* thus they don't have to lower it. +* So if we just handle this pending interrupts as +* level, the interrupt will be never lowered. +* So OS interprets this interrupt storm as +* the interrupt line is broken. +* +* To avoid such interrupt storm, unmask the interrupt, +* service the pending interrupts as edge, +* and then change the trigger mode. +*/ + s->ioredtbl[index] &= ~IOAPIC_LVT_MASKED ; +ioapic_service (s); + } s->ioredtbl[index] &= ~0xffffffffULL ; s->ioredtbl[index] |= val; }
Signature · 5 lines
1.7.1.1 -- yamahata
Quoted 14 lines · Isaku Yamahata
Quoted 2 lines · Isaku Yamahata
Quoted 2 lines
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.
Quoted 3 lines
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
...and that is the actual problem. The spec says: "Interrupt Mask—R/W. When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)."
So this should do the trick in a correct way (untested, please validate):
hw/ioapic.c +1 -1
@@ -161,7 +161,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level) } }else {/* edge triggered */ -if (level) { +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }
Jan
Isaku Yamahata
Tested-byQuoted 19 lines · Jan Kiszka
Quoted 14 lines · Isaku Yamahata
Quoted 2 lines · Isaku Yamahata
Quoted 2 lines
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.Quoted 3 lines
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
Quoted 7 lines
...and that is the actual problem. The spec says: "Interrupt Mask?R/W. When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)."
So this should do the trick in a correct way (untested, please validate):
Thank you for referring the spec. It works.
Here's the updated patch with your signed-off-by and my tested-by.
Quoted 1 line
From
a6c92855357a24da6a0d8d6e76dcca735a4be885Mon Sep 17 00:00:00 2001
Message-Id: <a6c92855357a24da6a0d8d6e76dcca735a4be885.1302346833.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH] ioapic: when switches to level trigger mode, interrupts raised repeatedly.
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
the bit must not be set when masked
- Then the mode is switched to level and it's unmasked with the same write.
- the bit of irr is set, so the interrupt is raised repeatedly by ioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.
Quoted 1 line
From the specification, the masked edge triggered interrupt must
be ingored as follows.
3.4.2 I/O redirection table registers: Interrupt Mask Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending).
Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1b
Quoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18
--- hw/ioapic.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
hw/ioapic.c +7 -2
@@ -160,8 +160,13 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask; } }else { -/* edge triggered */ -if (level) { +/* +* edge triggered +* 3.4.2 I/O redirection table registers: Interrupt Mask +* Edge-sensitive interrupts signaled on a masked interrupt pin +* are ignored (i.e., not delivered or held pending). +*/ +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }
Signature · 6 lines
1.7.1.1 -- yamahata
PATCH 1/1 ioapic: Do not set irr for masked edge IRQs
Quoted 25 lines · Isaku Yamahata
Quoted 24 lines · Jan Kiszka
Quoted 14 lines · Isaku Yamahata
Quoted 2 lines · Isaku Yamahata
Quoted 2 lines
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.Quoted 3 lines
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set....and that is the actual problem. The spec says: "Interrupt Mask?R/W. When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)."
Quoted 5 lines
Quoted 2 lines
So this should do the trick in a correct way (untested, please validate):
Thank you for referring the spec. It works.
Here's the updated patch with your signed-off-by and my tested-by.
Thanks for testing. I would prefer the following more compact wordings.
Jan
---------8<----------
From: Jan Kiszka <[email protected]>
So far we set IRR for edge IRQs even if the pin is masked. If the guest later on unmasks and switches the pin to level-triggered mode, irr will remain set, causing an IRQ storm. The point is that setting IRR is not correct in this case according to the spec, and avoiding this resolves the issue.
--- hw/ioapic.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
hw/ioapic.c +3 -2
@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask; } }else { -/* edge triggered */ -if (level) { +/* According to the 82093AA manual, we must ignore edge requests +* if the input pin is masked. */ +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }
Signature · 1 line
1.7.1
Re: [PATCH] ioapic: Do not set irr for masked edge IRQs
Quoted 58 lines · Jan Kiszka
Quoted 25 lines · Isaku Yamahata
Quoted 24 lines · Jan Kiszka
Quoted 14 lines · Isaku Yamahata
Quoted 2 lines · Isaku Yamahata
Quoted 2 lines
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.Quoted 3 lines
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set....and that is the actual problem. The spec says: "Interrupt Mask?R/W. When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)."
Quoted 5 lines
Quoted 2 lines
So this should do the trick in a correct way (untested, please validate):
Thank you for referring the spec. It works.
Here's the updated patch with your signed-off-by and my tested-by.Thanks for testing. I would prefer the following more compact wordings.
Jan
---------8<----------
From: Jan Kiszka <[email protected]>
So far we set IRR for edge IRQs even if the pin is masked. If the guest later on unmasks and switches the pin to level-triggered mode, irr will remain set, causing an IRQ storm. The point is that setting IRR is not correct in this case according to the spec, and avoiding this resolves the issue.
Reported-and-tested-by Isaku Yamahata <[email protected]>Signed-off-by Jan Kiszka <[email protected]>--- hw/ioapic.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)hw/ioapic.c +0
-0@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask;
hw/ioapic.c static void ioapic_set_irq(void *opaque, int vector, int level) · line 168
Quoted 10 lines
hw/ioapic.c +3
-2} }else { -/* edge triggered */ -if (level) { +/* According to the 82093AA manual, we must ignore edge requests +* if the input pin is masked. */ +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }
On first glance, it looks like KVM's in-kernel IOAPIC model is affected by the same issue. As you have the test case at hand, could you run it against qemu-kvm which stresses the kernel version?
TIA,
Jan
Re: [PATCH] ioapic: Do not set irr for masked edge IRQs
Quoted 65 lines · Jan Kiszka
Quoted 64 lines · Jan Kiszka
Quoted 25 lines · Isaku Yamahata
Quoted 24 lines · Jan Kiszka
Quoted 14 lines · Isaku Yamahata
Quoted 2 lines · Isaku Yamahata
Quoted 2 lines
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Here is the corrected patch. The first wrong patch clears the interrupts bit unconditionally. Which caused losing interrupt.
From
5ed177d35ab14f3b070a0eba2c49400279a3a14bMon Sep 17 00:00:00 2001
Message-Id: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: Isaku Yamahata <[email protected]>
Date: Wed, 16 Mar 2011 14:00:13 +0900
Subject: [PATCH 01/30] ioapic: when switches to level trigger mode, interrupts raised repeatedly.Quoted 3 lines
- the trigger mode is edge at first by reset.
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set....and that is the actual problem. The spec says: "Interrupt Mask?R/W. When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)."
Quoted 5 lines
Quoted 2 lines
So this should do the trick in a correct way (untested, please validate):
Thank you for referring the spec. It works.
Here's the updated patch with your signed-off-by and my tested-by.Thanks for testing. I would prefer the following more compact wordings.
Jan
---------8<----------
From: Jan Kiszka <[email protected]>
So far we set IRR for edge IRQs even if the pin is masked. If the guest later on unmasks and switches the pin to level-triggered mode, irr will remain set, causing an IRQ storm. The point is that setting IRR is not correct in this case according to the spec, and avoiding this resolves the issue.
Reported-and-tested-by Isaku Yamahata <[email protected]>Signed-off-by Jan Kiszka <[email protected]>--- hw/ioapic.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)hw/ioapic.c +2
-2@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask; } }else { -/* edge triggered */ -if (level) { +/* According to the 82093AA manual, we must ignore edge requests +* if the input pin is masked. */
Quoted 8 lines
hw/ioapic.c static void ioapic_set_irq(void *opaque, int vector, int level) · line 168
Quoted 4 lines
hw/ioapic.c +1
-0+if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }On first glance, it looks like KVM's in-kernel IOAPIC model is affected by the same issue. As you have the test case at hand, could you run it against qemu-kvm which stresses the kernel version?
On second glance, it should be fine as it clears irr unconditionally on the falling edge. Both approaches look valid to me as irr is just an internal state.
Jan
Re: [PATCH] ioapic: Do not set irr for masked edge IRQs
In reply to Jan KiszkaQuoted 2 lines · Jan Kiszka
On first glance, it looks like KVM's in-kernel IOAPIC model is affected by the same issue.
Agreed.
Quoted 2 lines
As you have the test case at hand, could you run it against qemu-kvm which stresses the kernel version?
Unfortunately I don't have kvm test case yet because I found this issue when trying to change IOAPIC irq routing.
Signature · 1 line
yamahata
Re: [Qemu-devel] [PATCH] ioapic: Do not set irr for masked edge IRQs
In reply to Jan KiszkaQuoted 20 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
So far we set IRR for edge IRQs even if the pin is masked. If the guest later on unmasks and switches the pin to level-triggered mode, irr will remain set, causing an IRQ storm. The point is that setting IRR is not correct in this case according to the spec, and avoiding this resolves the issue.
Reported-and-tested-by Isaku Yamahata <[email protected]>Signed-off-by Jan Kiszka <[email protected]>--- hw/ioapic.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)hw/ioapic.c +0
-0@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask;
hw/ioapic.c static void ioapic_set_irq(void *opaque, int vector, int level) · line 168
Quoted 10 lines
hw/ioapic.c +3
-2} }else { -/* edge triggered */ -if (level) { +/* According to the 82093AA manual, we must ignore edge requests +* if the input pin is masked. */ +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }
Ping?
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] ioapic: Do not set irr for masked edge IRQs
Quoted 25 lines · Jan Kiszka
Quoted 24 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
So far we set IRR for edge IRQs even if the pin is masked. If the guest later on unmasks and switches the pin to level-triggered mode, irr will remain set, causing an IRQ storm. The point is that setting IRR is not correct in this case according to the spec, and avoiding this resolves the issue.
Reported-and-tested-by Isaku Yamahata <[email protected]>Signed-off-by Jan Kiszka <[email protected]>--- hw/ioapic.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)hw/ioapic.c +0
-2@@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, int level) s->irr &= ~mask; } }else { -/* edge triggered */ -if (level) {
Quoted 9 lines
hw/ioapic.c static void ioapic_set_irq(void *opaque, int vector, int level) · line 168
Quoted 6 lines
hw/ioapic.c +3
-0+/* According to the 82093AA manual, we must ignore edge requests +* if the input pin is masked. */ +if (level && !(entry &IOAPIC_LVT_MASKED )) { s->irr |= mask;ioapic_service (s); }Ping?
Done.
Signature · 2 lines
Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.net
Quoted 2 lines · Isaku Yamahata
Thank you for applying. But I found that the patch is wrong and I'm preparing the new one. Can you please revert it?
Reverted for now, I'll ait for some comments on the new patch.
Quoted 87 lines · Aurelien Jarno
Quoted 81 lines · Isaku Yamahata
Quoted 51 lines
- the trigger mode is edge at first
- During initializatoin, the interrupt is raised as edge which is masked. The corresponding bit of irr is set.
- Then the mode is switched to level and it's unmasked.
- the bit of irr is set, so the interrupt is raised repeatedly byioapic_service().
- OS considers that the irq line is broken and falls back to polling mode.This patch fixes the issues.
After raising edige, clear the bit of irr.Quoted 36 lines
Bringing up interface eth0:
Determining IP information for eth0...irq 18: nobody cared (try booting with the "irqpoll" option)
Pid: 4126, comm: ip Not tainted 2.6.38-rc7 #1Call Trace: <IRQ> [<ffffffff8105b009>] ? __report_bad_irq+0x38/0x87 [<ffffffff8105b177>] ? note_interrupt+0x11f/0x188 [<ffffffff8105bacf>] ? handle_fasteoi_irq+0xa7/0xd1 [<ffffffff810046ff>] ? handle_irq+0x83/0x8c [<ffffffff81003eb9>] ? do_IRQ+0x48/0xaf [<ffffffff81300513>] ? ret_from_intr+0x0/0xe [<ffffffff81031ab8>] ? __do_softirq+0x4f/0x114 [<ffffffff81002d6c>] ? call_softirq+0x1c/0x28 [<ffffffff81004647>] ? do_softirq+0x33/0x68 [<ffffffff810316fb>] ? irq_exit+0x36/0x38 [<ffffffff81015f2c>] ? smp_apic_timer_interrupt+0x88/0x96 [<ffffffff81002853>] ? apic_timer_interrupt+0x13/0x20 <EOI> [<ffffffff810177ed>] ? __ioapic_set_affinity+0x68/0x7c [<ffffffff813000f0>] ? _raw_spin_unlock_irqrestore+0x8/0xa [<ffffffff8105a84f>] ? __setup_irq+0x224/0x2cb [<ffffffff8120e3c5>] ? e1000_intr+0x0/0x103 [<ffffffff8105a9c7>] ? request_threaded_irq+0xd1/0x114 [<ffffffff8120e396>] ? e1000_request_irq+0x34/0x63 [<ffffffff8121237d>] ? e1000_open+0x81/0x11f [<ffffffff8129097c>] ? call_netdevice_notifiers+0x45/0x4a [<ffffffff81290d8d>] ? __dev_open+0x97/0xc4 [<ffffffff8128e9c5>] ? __dev_change_flags+0xb9/0x13d [<ffffffff81290cc1>] ? dev_change_flags+0x1c/0x51 [<ffffffff812d0542>] ? devinet_ioctl+0x26e/0x594 [<ffffffff812d174c>] ? inet_ioctl+0x92/0xaa [<ffffffff81281d75>] ? T.1003+0x13/0x32 [<ffffffff81282152>] ? sock_ioctl+0x1f2/0x1ff [<ffffffff810ae2d3>] ? do_vfs_ioctl+0x498/0x4e7 [<ffffffff81281203>] ? sock_alloc_file+0xb3/0x115 [<ffffffff8109f79f>] ? fd_install+0x31/0x5d [<ffffffff810ae364>] ? sys_ioctl+0x42/0x65 [<ffffffff81001f3b>] ? system_call_fastpath+0x16/0x1bQuoted 3 lines
handlers:
[<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
Disabling IRQ #18Quoted 4 lines
Signed-off-by Isaku Yamahata <[email protected]>--- hw/ioapic.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)Thanks, applied.
Quoted 17 lines
hw/ioapic.c +1
-0@@ -164,6 +164,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)if (level) { s->irr |= mask;ioapic_service (s); + s->irr &= ~mask; } } }Signature · 1 line
1.7.1.1Signature · 2 lines
Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.netSignature · 1 line
yamahata
Signature · 2 lines
Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.net