Re: [PATCH v3 05/10] KVM: arm/arm64: don't clear exit request from caller

[Christoffer Dall <cdall@linaro.org>]

On Wed, May 03, 2017 at 06:06:30PM +0200, Andrew Jones wrote:
> VCPU requests that the receiver should handle should only be cleared
> by the receiver. 

I cannot parse this sentence.

> Not only does this properly implement the protocol,
> but also avoids bugs where one VCPU clears another VCPU's request,
> before the receiving VCPU has had a chance to see it.

Is this an actual race we have currently or just something thay may
happen later.  Im' not sure.

> ARM VCPUs
> currently only handle one request, EXIT, and handling it is achieved
> by checking pause to see if the VCPU should sleep.

This makes sense.  So forget my comment on the previous patch about
getting rid of the pause flag.

> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arch/arm/kvm/arm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 9174ed13135a..7be0d9b0c63a 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -553,7 +553,6 @@ void kvm_arm_resume_vcpu(struct kvm_vcpu *vcpu)
>  {
>  	struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
>  
> -	kvm_clear_request(KVM_REQ_VCPU_EXIT, vcpu);
>  	vcpu->arch.pause = false;
>  	swake_up(wq);
>  }
> @@ -625,7 +624,14 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  		update_vttbr(vcpu->kvm);
>  
> -		if (vcpu->arch.power_off || vcpu->arch.pause)
> +		if (kvm_request_pending(vcpu)) {
> +			if (kvm_check_request(KVM_REQ_VCPU_EXIT, vcpu)) {
> +				if (vcpu->arch.pause)
> +					vcpu_sleep(vcpu);
> +			}

Can we factor out this bit to a separate function,
kvm_handle_vcpu_requests() or something like that?

> +		}
> +
> +		if (vcpu->arch.power_off)
>  			vcpu_sleep(vcpu);
>  
>  		/*
> -- 
> 2.9.3
> 
Thanks,
-Christoffer