Re: [PATCH v5 10/13] ARM: KVM: Guest wait-for-interrupts (WFI) support

[Avi Kivity <avi@redhat.com>]

On 12/11/2011 12:25 PM, Christoffer Dall wrote:
> From: Christoffer Dall <cdall@cs.columbia.edu>
>
> 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