From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v5 10/13] ARM: KVM: Guest wait-for-interrupts (WFI) support Date: Mon, 12 Dec 2011 16:12:30 +0200 Message-ID: <4EE60BCE.6010405@redhat.com> References: <20111211102403.21693.6887.stgit@localhost> <20111211102522.21693.14911.stgit@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: android-virt@lists.cs.columbia.edu, kvm@vger.kernel.org, Marc.Zyngier@arm.com, catalin.marinas@arm.com, tech@virtualopensystems.com, peter.maydell@linaro.org To: Christoffer Dall Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41400 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752360Ab1LLOMx (ORCPT ); Mon, 12 Dec 2011 09:12:53 -0500 In-Reply-To: <20111211102522.21693.14911.stgit@localhost> Sender: kvm-owner@vger.kernel.org List-ID: On 12/11/2011 12:25 PM, Christoffer Dall wrote: > From: Christoffer Dall > > When the guest executes a WFI instruction the operation is trapped to > KVM, which emulates the instruction in software. There is no correlation > between a guest executing a WFI instruction and actually puttin the putting (puttin'? putin?) > > hardware into a low-power mode, since a KVM guest is essentially a > process and the WFI instruction can be seen as 'sleep' call from this > process. Therefore, we flag the VCPU to be in wait_for_interrupts mode > and call the main KVM function kvm_vcpu_block() function. This function > will put the thread on a wait-queue and call schedule. > > When an interrupt comes in through KVM_IRQ_LINE (see previous patch) we > signal the VCPU thread and unflag the VCPU to no longer wait for > interrupts. All calls to kvm_arch_vcpu_ioctl_run() result in a call to > kvm_vcpu_block() as long as the VCPU is in wfi-mode. Ah, this addresses my previous comment on this issue. > > return ret; > @@ -454,6 +467,8 @@ static int kvm_arch_vm_ioctl_irq_line(struct kvm *kvm, > if (irq_level->level) { > vcpu->arch.virt_irq |= mask; > vcpu->arch.wait_for_interrupts = 0; > > + if (waitqueue_active(&vcpu->wq)) > + wake_up_interruptible(&vcpu->wq); Not sufficient. If the guest is running, you need to kick it out of guest mode and back into kvm, so that it samples the interrupt lines. Also, racy: racy: vcpu host thread KVM_IRQ_LINE WFI if (!vcpu->arch.virt_irq) vcpu->arch.virt_irq = x vcpu->arch.wait_for_interrupts = 0 vcpu->arch.wait_for_interrupts = 1 if (waitqueue_active()) (fails) schedule() > > +/** > + * kvm_handle_wfi - handle a wait-for-interrupts instruction executed by a guest > + * @vcpu: the vcpu pointer > + * @run: the kvm_run structure pointer > + * > + * Simply sets the wait_for_interrupts flag on the vcpu structure, which will > + * halt execution of world-switches and schedule other host processes until > + * there is an incoming IRQ or FIQ to the VM. > + */ > int kvm_handle_wfi(struct kvm_vcpu *vcpu, struct kvm_run *run) > { > + trace_kvm_wfi(vcpu->arch.regs.pc); > + if (!vcpu->arch.virt_irq) > + vcpu->arch.wait_for_interrupts = 1; Why not just block here? -- error compiling committee.c: too many arguments to function