Re: [Qemu-devel] [PATCH] MIPS interrupts and -icount

[Aurelien Jarno]

On Sun, Dec 26, 2010 at 09:34:20AM +0100, Edgar E. Iglesias wrote:
> On Sat, Dec 25, 2010 at 11:22:14PM +0100, Aurelien Jarno wrote:
> > Hi,
> > 
> > On Wed, Dec 22, 2010 at 05:12:39PM +0100, Edgar E. Iglesias wrote:
> > > Hi, I don't see this problem with the qemu.org test images and neither
> > > with my boards/images. I see QEMU basically not running at all when
> > > the guest is idle. Do you have more info on how to reproduce it?
> > 
> > I am seeing the problem with the MIPS malta board and the images from:
> > 
> > http://people.debian.org/~aurel32/qemu/mips/
> > 
> > > If the CPU hw interrupt line is asserted it means some device is
> > > signaling interrupts. Maybe we are modling the wake up filter
> > > wrongly in target-mips/exec.h, maybe a real MIPS doesn't wakeup from
> > > sleep unless the irq passes the CPUs internal masking? The manuals
> > > are not really clear on this. I'm currently travelling and have
> > > no access to check with a real MIPS hw.
> > 
> > According the manual I have checked, it is implementation dependent if
> > the CPU exits from the WAIT instruction when a non-enabled interrupt is
> > triggered. However for the few implementations I have checked (4k, 5k,
> > 34k), the CPU only wakes-up if the interrupt can be taken.
> > 
> > > If your hw interrupt line is active all the time it sounds to me
> > > like if something is also wrong with either the guest software or a
> > > device model.
> > 
> > The corresponding interrupt line is the timer one. It seems the kernel
> > sometimes choose to ignore the timer instead of stopping it. I am only
> > able to reproduce that with a dyntick enabled kernel.
> >
> > > I think the following patch should restore the previous wait for
> > > interrupt wakeup behaviour to let the MIPS sleep until an irq passes
> > > the internal masking (but I'm not sure this is how real MIPS does it):
> > 
> > It does, thanks a lot.
> > 
> > However, according to the manual I think we should also check if
> > interrupts are enabled (if they are disabled, an interrupt can't be
> > taken). I therefore propose the following patch:
> > 
> > From 9c9e5f7ee1e897e408b1cd9f4c42ddf86c30aabe Mon Sep 17 00:00:00 2001
> > From: Aurelien Jarno <address@hidden>
> > Date: Sat, 25 Dec 2010 22:56:32 +0100
> > Subject: [PATCH] target-mips: fix host CPU consumption when guest is idle
> > 
> > When the CPU is in wait state, do not wake-up if an interrupt can't be
> > taken. This avoid host CPU running at 100% if a device (e.g. timer) has
> > an interrupt line left enabled.
> > 
> > Also factorize code to check if interrupts are enabled in
> > cpu_mips_hw_interrupts_pending().
> 
> Thanks Aurelien,
> 
> It looks good, but one thing that worries me slightly is that streching
> the wakeup filter to include the IE related flags might break using the
> wait insn in polling mode.
> 
> for example:
> 
> di();
> init_hw();
> while (1) {
>     wait_for_interrupt(); /* Power Save.  */
>     do_work();
> };
> 
> I've seem similar constructions in bootcode/firmware for other archs.
> In this case I guess it would be using undefined behaviour on the mips
> though, so I'm OK with either patch.

Yes, the manuals are not fully clear, however it seems to be a possible
behaviour for some implementations, also it was the behaviour prior
to your patch. Note also that in the case above, the CPU can still be
woken-up by an NMI, though it doesn't seem to be implemented in QEMU
yet.

> At some point I'll see if I can check the IE flags behaviour with a real
> 34k and we can finetune the models with follow-up patches if needed.

Ok, that would be nice if you can do it.

> Acked-by: Edgar E. Iglesias <address@hidden>
> 

Thanks for your review.

Cheers,
Aurélien

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net