[Qemu-devel] Re: [PATCH] ioapic: when switches to level trigger mode, interrupts raised repeatedly.

[Isaku Yamahata]

On Sat, Apr 09, 2011 at 10:38:10AM +0200, Jan Kiszka wrote:
> On 2011-04-04 04:15, Isaku Yamahata wrote:
> > On Mon, Apr 04, 2011 at 08:42:07AM +0900, Isaku Yamahata wrote:
> >> > 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 5ed177d35ab14f3b070a0eba2c49400279a3a14b Mon Sep 17 00:00:00 2001
> > Message-Id: <address@hidden>
> > In-Reply-To: <address@hidden>
> > References: <address@hidden>
> > From: Isaku Yamahata <address@hidden>
> > 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.
> 
> ...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.


>From a6c92855357a24da6a0d8d6e76dcca735a4be885 Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
In-Reply-To: <address@hidden>
References: <address@hidden>
From: Isaku Yamahata <address@hidden>
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.

>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).

> 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 #1
> Call 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
> handlers:
> [<ffffffff8120e3c5>] (e1000_intr+0x0/0x103)
> Disabling IRQ #18

Signed-off-by: Jan Kiszka <address@hidden>
Tested-by: Isaku Yamahata <address@hidden>
---
 hw/ioapic.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/ioapic.c b/hw/ioapic.c
index 569327d..42c5037 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -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);
             }
-- 
1.7.1.1



-- 
yamahata