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

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

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 <aurelien@aurel32.net>
> 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.

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.

Acked-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Cheers