[Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Aurelien Jarno]

Hi all,

There is currently a bug concerning the IRQ acknowlege on the MIPS
system emulation. It concerns both the QEMU and Malta boards, though it
is only detectable with a 2.4 kernel and thus on the Malta board. The
symptom is a storm of "We got a spurious interrupt from PIIX4."

This is due to the kernel requesting the interrupt number from the 
i8259A where no interrupt is waiting. In such a case the i8259A answers
by an IRQ 7.

When an hardware interrupt occurs, the i8259A memorizes the interrupt
and sends it to the MIPS CPU. This is done via the pic_irq_request()
function. The result is that the bit 10 of the CP0 Cause register is 
set to one (interrupt 2). But when the interrupt is finished, the i8259a
registers IRR and ISR are cleared, but not the CP0 Cause register. The
CPU always thinks there is an interrupt to serve, which is wrong.

The patch below addresses the problem, but it is just a way to show the
problem and the fix. It is not clean to merge it, except the GT64120
part.

With this patch the problem is fixed, and I see a 12% improvement in
the boot time of a Debian system.

Does anyone has an idea of a sane implementation for that? It seems
only the MIPS platform has to clear a register of the CPU when an
interrupt is finished.

Thanks,
Aurelien



Index: gt64xxx.c
===================================================================
RCS file: /sources/qemu/qemu/hw/gt64xxx.c,v
retrieving revision 1.2
diff -u -d -p -r1.2 gt64xxx.c
--- gt64xxx.c	17 Jan 2007 23:35:01 -0000	1.2
+++ gt64xxx.c	23 Jan 2007 00:34:14 -0000
@@ -433,7 +436,8 @@ static uint32_t gt64120_readl (void *opa
         val = s->regs[saddr];
         break;
     case GT_PCI0_IACK:
- 	val = pic_intack_read(isa_pic);
+        /* Read the IRQ number */ 
+        val = pic_read_irq(isa_pic);
         break;
 
     /* SDRAM Parameters */
Index: i8259.c
===================================================================
RCS file: /sources/qemu/qemu/hw/i8259.c,v
retrieving revision 1.19
diff -u -d -p -r1.19 i8259.c
--- i8259.c	25 Jun 2006 18:15:31 -0000	1.19
+++ i8259.c	23 Jan 2007 00:34:15 -0000
@@ -161,6 +161,8 @@ void pic_update_irq(PicState2 *s)
 #endif
         s->irq_request(s->irq_request_opaque, 1);
     }
+    else
+        s->irq_request(s->irq_request_opaque, 0);
 }
 
 #ifdef DEBUG_IRQ_LATENCY
Index: mips_malta.c
===================================================================
RCS file: /sources/qemu/qemu/hw/mips_malta.c,v
retrieving revision 1.4
diff -u -d -p -r1.4 mips_malta.c
--- mips_malta.c	20 Jan 2007 00:29:01 -0000	1.4
+++ mips_malta.c	23 Jan 2007 00:34:15 -0000
@@ -62,7 +62,7 @@ static void pic_irq_request(void *opaque
         cpu_interrupt(env, CPU_INTERRUPT_HARD);
     } else {
 	env->CP0_Cause &= ~0x00000400;
-        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
+        /* cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); */
     }
 }
 
Index: mips_r4k.c
===================================================================
RCS file: /sources/qemu/qemu/hw/mips_r4k.c,v
retrieving revision 1.31
diff -u -d -p -r1.31 mips_r4k.c
--- mips_r4k.c	6 Jan 2007 02:24:15 -0000	1.31
+++ mips_r4k.c	23 Jan 2007 00:34:15 -0000
@@ -44,7 +44,7 @@ static void pic_irq_request(void *opaque
         cpu_interrupt(env, CPU_INTERRUPT_HARD);
     } else {
 	env->CP0_Cause &= ~0x00000400;
-        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
+        /* cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); */
     }
 }
 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Aurelien Jarno]

Hi all,

Some news on that point.

After a discussion with Paul Brook, Thiemo Seufer and Ralf Baechle on
IRC yesterday, we got convinced that the current IRQ handling is not
correct.

The hardware interrupt is currently deasserted by the CPU itself (in
cpu-exec.c). It should be deasserted by the interrupt controller (the
i8259a in our case), so that pending interrupts are not missed. This is
wrong for MIPS, but also for x86_64 and PowerPC. ARM is correctly
implemented though.

Then after playing with the current code, I am sure we are missing a
simple interrupt controller for the MIPS CPU. It supports 6 hardware
interrupts (IP2 to IP7) and we are using two of them in the current
emulation: one for the i8259a and the other for the timer. In both case
the current code assert and deassert a CPU_INTERRUPT_HARD.

The interrupt controller should assert and deassert the
CPU_INTERRUPT_HARD upon the contents of the CP0 cause and CP0 status (ie
mask) registers. Currently we are totally ignoring that interrupts can
be masked and leave this task to the operating system.

I will try to write the missing code this night.

Bye,
Aurelien

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Marius Groeger]

On Tue, 23 Jan 2007, Aurelien Jarno wrote:

> There is currently a bug concerning the IRQ acknowlege on the MIPS
> system emulation. It concerns both the QEMU and Malta boards, though it
> is only detectable with a 2.4 kernel and thus on the Malta board. The
> symptom is a storm of "We got a spurious interrupt from PIIX4."
> 
> This is due to the kernel requesting the interrupt number from the 
> i8259A where no interrupt is waiting. In such a case the i8259A answers
> by an IRQ 7.
> 
> When an hardware interrupt occurs, the i8259A memorizes the interrupt
> and sends it to the MIPS CPU. This is done via the pic_irq_request()
> function. The result is that the bit 10 of the CP0 Cause register is 
> set to one (interrupt 2). But when the interrupt is finished, the i8259a
> registers IRR and ISR are cleared, but not the CP0 Cause register. The
> CPU always thinks there is an interrupt to serve, which is wrong.

I can confirm this issue. For our (custom) OS I worked around this by 
manualy clearing CP0 Cause (even though I think I shouldn't be allowed 
to do that since CP0:IP[7-2] are read-only, but that's another 
story... ;-)

> Does anyone has an idea of a sane implementation for that? It seems
> only the MIPS platform has to clear a register of the CPU when an
> interrupt is finished.

What about passing another hook to pic_init which, if set, would be 
called when no more interrupts are pending? Would that be too specific 
this problem to be an acceptable solution?

Regards,
Marius

-- 
Marius Groeger <mgroeger@sysgo.com>
SYSGO AG                      Embedded and Real-Time Software
Voice: +49 6136 9948 0                  FAX: +49 6136 9948 10
www.sysgo.com | www.elinos.com | www.osek.de | www.pikeos.com

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Alexander Voropay]

"Aurelien Jarno" <aurelien@aurel32.net> wrote:

> Then after playing with the current code, I am sure we are missing a
> simple interrupt controller for the MIPS CPU. It supports 6 hardware
> interrupts (IP2 to IP7) and we are using two of them in the current
> emulation: one for the i8259a and the other for the timer. In both case
> the current code assert and deassert a CPU_INTERRUPT_HARD.

 The Galileo GT64xxx chip contains an interrupt controller too (for DMA
cycle indication, built-in Timers e.t.c.). All this interrupt controllers are
daisy-chained: i8259(as part of the PIIX + PCI), GT64xxx and MIPS internal.

P.S.  It should be good to have a well-defined modular IRQ routing architecture
in the Qemu. Moreover, modern ix86 LAPIC/APIC interrupt controller even
more complicated.

--
-=AV=-

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Paul Brook]

> It should be good to have a well-defined modular IRQ routing
> architecture in the Qemu.

We've got most of one for the ARM targets (see hw/arm_pic.h). This file 
contains both the target independent bits and the ARM specific bits for 
emulating the CPU IRQ/FIQ pins.

Annother possibility to abstract this to use a single interrupt line object 
rather than an {object,index} pair. This simplifies code that that raises 
interrupts, at the expense of some complication in the code to create 
interrupt controllers.

Paul

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Aurelien Jarno]

Alexander Voropay a écrit :
> "Aurelien Jarno" <aurelien@aurel32.net> wrote:
> 
>> Then after playing with the current code, I am sure we are missing a
>> simple interrupt controller for the MIPS CPU. It supports 6 hardware
>> interrupts (IP2 to IP7) and we are using two of them in the current
>> emulation: one for the i8259a and the other for the timer. In both case
>> the current code assert and deassert a CPU_INTERRUPT_HARD.
> 
>  The Galileo GT64xxx chip contains an interrupt controller too (for DMA
> cycle indication, built-in Timers e.t.c.). All this interrupt controllers are
> daisy-chained: i8259(as part of the PIIX + PCI), GT64xxx and MIPS internal.
> 

Yes, but this controller is not used on the Malta board.


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [RFC] IRQ acknowledge on MIPS

[Fabrice Bellard]

Paul Brook wrote:
>>It should be good to have a well-defined modular IRQ routing
>>architecture in the Qemu.
> 
> 
> We've got most of one for the ARM targets (see hw/arm_pic.h). This file 
> contains both the target independent bits and the ARM specific bits for 
> emulating the CPU IRQ/FIQ pins.
> 
> Annother possibility to abstract this to use a single interrupt line object 
> rather than an {object,index} pair. This simplifies code that that raises 
> interrupts, at the expense of some complication in the code to create 
> interrupt controllers.

I prefer to have a new type 'QEMUBoolSignal' which can be used for any 
boolean signal. A function could be called to change its level and 
several clients could set a callback ("listener") to be informed that 
its level changes.

Regards,

Fabrice.