Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register

[Jan Kiszka]

On 2012-09-04 16:42, Paolo Bonzini wrote:
> Il 04/09/2012 16:29, Maciej W. Rozycki ha scritto:
>>  So first of all, the *output* of the 8259A is always edge triggered, 
>> regardless of whether it's the master or one of the slaves (only one slave 
>> is used in the PC/AT architecture, but up to eight are supported; the 
>> PC/XT had none).  
> 
> I swear I read all your message :) but this seems to be the crux.  It means
> that something like this ought to fix the bug too.  Matthew, can you post
> your code or test it?
> 
> diff --git a/hw/i8259.c b/hw/i8259.c
> index 53daf78..3dc1dff 100644
> --- a/hw/i8259.c
> +++ b/hw/i8259.c
> @@ -104,12 +104,11 @@ static void pic_update_irq(PICCommonState *s)
>      int irq;
>  
>      irq = pic_get_irq(s);
> +    qemu_irq_lower(s->int_out[0]);
>      if (irq >= 0) {
>          DPRINTF("pic%d: imr=%x irr=%x padd=%d\n",
>                  s->master ? 0 : 1, s->imr, s->irr, s->priority_add);
>          qemu_irq_raise(s->int_out[0]);
> -    } else {
> -        qemu_irq_lower(s->int_out[0]);
>      }
>  }

I don't think this can be correct in all scenario. E.g., we also call
pic_update_irq in case a level-triggered input was updated but didn't
change the output state of the PIC (high-high transition). With your
patch, the output will now generate edges.

What I'm trying to understand and translate from the description is
rather "note that for inputs a high-to-low transition cancels the
interrupt as in the level-triggered mode." This is surely not what we do
right now. OTOH, I'm afraid that switching to this mode in the PIC can
cause problems elsewhere, with devices that actually inject short
low-high-low signals. Still wrapping my head around it...

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux