From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33515 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PjwmR-0006hN-9I for qemu-devel@nongnu.org; Mon, 31 Jan 2011 11:39:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PjwmL-0007Gs-Gh for qemu-devel@nongnu.org; Mon, 31 Jan 2011 11:39:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59670) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PjwmL-0007Gc-8e for qemu-devel@nongnu.org; Mon, 31 Jan 2011 11:39:01 -0500 Date: Mon, 31 Jan 2011 18:38:58 +0200 From: Gleb Natapov Message-ID: <20110131163858.GA18827@redhat.com> References: <8db93a26b3cbb67e309d05600811dd6a37b34433.1296133797.git.jan.kiszka@siemens.com> <4D468A24.4080800@redhat.com> <4D469ED6.9050805@siemens.com> <4D46B369.3050404@siemens.com> <4D46D7F2.3040502@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4D46D7F2.3040502@siemens.com> Subject: [Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Marcelo Tosatti , Avi Kivity , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote: > On 2011-01-31 14:04, Jan Kiszka wrote: > > On 2011-01-31 12:36, Jan Kiszka wrote: > >> On 2011-01-31 11:08, Avi Kivity wrote: > >>> On 01/27/2011 03:10 PM, Jan Kiszka wrote: > >>>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run > >>>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change > >>>> this service processes will first cause an exit from kvm_cpu_exec > >>>> anyway. And we will have to reenter the kernel on IO exits > >>>> unconditionally, something that the current logic prevents. > >>>> > >>>> Signed-off-by: Jan Kiszka > >>>> --- > >>>> kvm-all.c | 11 ++++++----- > >>>> 1 files changed, 6 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/kvm-all.c b/kvm-all.c > >>>> index 5bfa8c0..46ecc1c 100644 > >>>> --- a/kvm-all.c > >>>> +++ b/kvm-all.c > >>>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env) > >>>> > >>>> DPRINTF("kvm_cpu_exec()\n"); > >>>> > >>>> + if (kvm_arch_process_irqchip_events(env)) { > >>>> + env->exit_request = 0; > >>>> + env->exception_index = EXCP_HLT; > >>>> + return 0; > >>>> + } > >>>> + > >>>> do { > >>>> #ifndef CONFIG_IOTHREAD > >>>> if (env->exit_request) { > >>>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env) > >>>> } > >>> > >>> We check for ->exit_request here > >>> > >>>> #endif > >>>> > >>>> - if (kvm_arch_process_irqchip_events(env)) { > >>>> - ret = 0; > >>>> - break; > >>>> - } > >>>> - > >>> > >>> But this checks for ->interrupt_request. What ensures that we exit when > >>> ->interrupt_request is set? > >> > >> Good question, need to check again. But if that turns out to be an > >> issue, qemu-kvm would be broken as well. I'm just aligning the code here. > >> > > > > The only thing we miss by moving process_irqchip_events is a self-INIT > > of an AP - if such thing exists in real life. In that case, the AP would > > cause a reset of itself, followed by a transition to HALT state. > > I checked again with the Intel spec, and a self-INIT is invalid (at > least when specified via shorthand). So I'm under the impression now > that we can safely ignore this case and leave the patch as is. > > Any different views? > IIRC if you don't use shorthand you can send INIT to self. -- Gleb.