Re: [kvm-ppc-devel] [PATCH 2 of 2] Add PowerPC KVM guest

[Hollis Blanchard <hollisb@us.ibm.com>]

On Fri, 2008-03-28 at 16:34 -0500, Jerone Young wrote: 
> # HG changeset patch
> # User Jerone Young <jyoung5@us.ibm.com>
> # Date 1206739218 18000
> # Node ID 21179d0ab8a62ecc24d18be2da1600694aeca71a
> # Parent  e48cf2ad6c85c457ff64c04b83960fc305420842
> Add PowerPC KVM guest wait handling support
> 
> This patch handles a guest that is in a wait state. This ensures that
> the guest is not allways eating up 100% cpu when it is idle. 
> 
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -177,6 +177,11 @@ static int kvmppc_can_deliver_interrupt(
>  static int kvmppc_can_deliver_interrupt(struct kvm_vcpu *vcpu, int interrupt)
>  {
>  	int r;
> +
> +	/* clear guest wait state */
> +	if (vcpu->arch.msr & MSR_WE) {
> +		vcpu->arch.msr &= ~MSR_WE;
> +	}
> 
>  	switch (interrupt) {
>  	case BOOKE_INTERRUPT_CRITICAL:

This code definitely does not belong here. Why didn't you put it in
kvmppc_deliver_interrupt()?

> @@ -294,6 +299,21 @@ int kvmppc_handle_exit(struct kvm_run *r
> 
>  	run->exit_reason = KVM_EXIT_UNKNOWN;
>  	run->ready_for_interrupt_injection = 1;
> +
> +	/* handle guest vcpu that is in wait state */
> +	if (vcpu->arch.msr & MSR_WE) {
> +		DECLARE_WAITQUEUE(wait, current);
> +		add_wait_queue(&vcpu->wq, &wait);
> +
> +		while(vcpu->arch.pending_exceptions) {
> +			set_current_state(TASK_INTERRUPTIBLE);
> +			schedule();
> +			kvmppc_check_and_deliver_interrupts(vcpu);
> +		}
> +
> +		set_current_state(TASK_RUNNING);
> +		remove_wait_queue(&vcpu->wq, &wait);
> +	}
> 
>  	switch (exit_nr) {
>  	case BOOKE_INTERRUPT_MACHINE_CHECK:

This code also definitely does not belong here. For starters, the mtmsr
emulation hasn't yet run, so vcpu->arch.msr hasn't been updated.

Why are you calling kvmppc_check_and_deliver_interrupts() inside the
loop?? I don't understand this code flow at all.

You don't seem to have included the patch that adds vcpu->wq.

Wouldn't you want to notify the waitqueue from
kvmppc_deliver_interrupt()?

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel