[Qemu-devel] Re: [PATCH] ioapic: Do not set irr for masked edge IRQs

[Jan Kiszka <jan.kiszka@web.de>]

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

On 2011-04-09 13:18, Jan Kiszka wrote:
> On 2011-04-09 13:05, Isaku Yamahata wrote:
>> 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: <5ed177d35ab14f3b070a0eba2c49400279a3a14b.1301883258.git.yamahata@valinux.co.jp>
>>>> In-Reply-To: <cover.1301883258.git.yamahata@valinux.co.jp>
>>>> References: <cover.1301883258.git.yamahata@valinux.co.jp>
>>>> From: Isaku Yamahata <yamahata@valinux.co.jp>
>>>> 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.
> 
> Thanks for testing. I would prefer the following more compact wordings.
> 
> Jan
> 
> ---------8<----------
> 
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> 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 <yamahata@valinux.co.jp>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/ioapic.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index 569327d..6c26e82 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -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);
>              }

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]