Re: [PATCH v9 16/16] ARM: KVM: Guest wait-for-interrupts (WFI) support

[Avi Kivity <avi@redhat.com>]

On 07/03/2012 12:02 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 putting the
> 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.
> 
>  
>  int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *v)
> @@ -538,6 +546,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  		 */
>  		cond_resched();
>  
> +		if (vcpu->arch.wait_for_interrupts)
> +			kvm_vcpu_block(vcpu);
> +
>  		update_vttbr(vcpu->kvm);
>  
>  		/*
> @@ -635,6 +646,8 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level)
>  	 * trigger a world-switch round on the running physical CPU to set the
>  	 * virtual IRQ/FIQ fields in the HCR appropriately.
>  	 */
> +	if (irq_level->level)
> +		vcpu->arch.wait_for_interrupts = 0;

What, no memory barriers, etc?

Is it actually needed?  We can clear it instead after calling
kvm_vcpu_block() above, so the variable is only accessed from the vcpu
thread.  The savings in pain medication are measurable.

>  	kvm_vcpu_kick(vcpu);
>  
>  	return 0;
> diff --git a/arch/arm/kvm/emulate.c b/arch/arm/kvm/emulate.c
> index 99432d8..564add2 100644
> +/**
> + * 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);
>  	vcpu->stat.wfi_exits++;
> +	if (!vcpu->arch.irq_lines)
> +		vcpu->arch.wait_for_interrupts = 1;
>  

Or you could just call kvm_vcpu_block() here without having the
variable.  But eventually you'll need it since you want to expose wfi
state to userspace for live migration.

-- 
error compiling committee.c: too many arguments to function