From patchwork Sat Dec 25 22:22:14 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aurelien Jarno X-Patchwork-Id: 76704 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 46705B70E9 for ; Sun, 26 Dec 2010 09:23:55 +1100 (EST) Received: from localhost ([127.0.0.1]:43935 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PWcWm-0008A0-3k for incoming@patchwork.ozlabs.org; Sat, 25 Dec 2010 17:23:52 -0500 Received: from [140.186.70.92] (port=33693 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PWcVe-0007r8-7p for qemu-devel@nongnu.org; Sat, 25 Dec 2010 17:22:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PWcVc-0006U4-Mb for qemu-devel@nongnu.org; Sat, 25 Dec 2010 17:22:42 -0500 Received: from hall.aurel32.net ([88.191.126.93]:39502) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PWcVc-0006Th-Eq for qemu-devel@nongnu.org; Sat, 25 Dec 2010 17:22:40 -0500 Received: from [80.214.254.14] (helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PWcVQ-0005AG-3d; Sat, 25 Dec 2010 23:22:30 +0100 Received: from aurel32 by volta.aurel32.net with local (Exim 4.72) (envelope-from ) id 1PWcVD-0002I8-0G; Sat, 25 Dec 2010 23:22:15 +0100 Date: Sat, 25 Dec 2010 23:22:14 +0100 From: Aurelien Jarno To: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [PATCH] MIPS interrupts and -icount Message-ID: <20101225222214.GA4464@volta.aurel32.net> References: <20100722113218.GC28205@edde.se.axis.com> <20100724225545.GD7190@ohm.aurel32.net> <20100725000754.GB25828@laped.lan> <20100725030807.GF7190@ohm.aurel32.net> <20100725054649.GC25828@laped.lan> <20101221222843.GA32645@volta.aurel32.net> <20101222161239.GA20860@laped.Belkin> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20101222161239.GA20860@laped.Belkin> X-Mailer: Mutt 1.5.20 (2009-06-14) User-Agent: Mutt/1.5.20 (2009-06-14) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) Cc: qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 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(). Based on a patch from Edgar E. Iglesias Signed-off-by: Aurelien Jarno Acked-by: Edgar E. Iglesias --- cpu-exec.c | 6 +----- target-mips/cpu.h | 8 ++++++++ target-mips/exec.h | 18 +++++++++++++++--- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 39e5eea..8c9fb8b 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -454,11 +454,7 @@ int cpu_exec(CPUState *env1) } #elif defined(TARGET_MIPS) if ((interrupt_request & CPU_INTERRUPT_HARD) && - cpu_mips_hw_interrupts_pending(env) && - (env->CP0_Status & (1 << CP0St_IE)) && - !(env->CP0_Status & (1 << CP0St_EXL)) && - !(env->CP0_Status & (1 << CP0St_ERL)) && - !(env->hflags & MIPS_HFLAG_DM)) { + cpu_mips_hw_interrupts_pending(env)) { /* Raise it */ env->exception_index = EXCP_EXT_INTERRUPT; env->error_code = 0; diff --git a/target-mips/cpu.h b/target-mips/cpu.h index c1f211f..2419aa9 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -532,6 +532,14 @@ static inline int cpu_mips_hw_interrupts_pending(CPUState *env) int32_t status; int r; + if (!(env->CP0_Status & (1 << CP0St_IE)) || + (env->CP0_Status & (1 << CP0St_EXL)) || + (env->CP0_Status & (1 << CP0St_ERL)) || + (env->hflags & MIPS_HFLAG_DM)) { + /* Interrupts are disabled */ + return 0; + } + pending = env->CP0_Cause & CP0Ca_IP_mask; status = env->CP0_Status & CP0Ca_IP_mask; diff --git a/target-mips/exec.h b/target-mips/exec.h index af61b54..1273654 100644 --- a/target-mips/exec.h +++ b/target-mips/exec.h @@ -19,10 +19,22 @@ register struct CPUMIPSState *env asm(AREG0); static inline int cpu_has_work(CPUState *env) { - return (env->interrupt_request & - (CPU_INTERRUPT_HARD | CPU_INTERRUPT_TIMER)); -} + int has_work = 0; + + /* It is implementation dependent if non-enabled interrupts + wake-up the CPU, however most of the implementations only + check for interrupts that can be taken. */ + if ((env->interrupt_request & CPU_INTERRUPT_HARD) && + cpu_mips_hw_interrupts_pending(env)) { + has_work = 1; + } + if (env->interrupt_request & CPU_INTERRUPT_TIMER) { + has_work = 1; + } + + return has_work; +} static inline int cpu_halted(CPUState *env) {