Re: [PATCH RFC 1/2] KVM: don't check for PF_VCPU when yielding

[Paolo Bonzini <pbonzini@redhat.com>]

On 25/11/2014 17:04, David Hildenbrand wrote:
> As some architectures (e.g. s390) can't disable preemption while
> entering/leaving the guest, they won't receive the yield in all situations.
> 
> kvm_enter_guest() has to be called with preemption_disabled and will set
> PF_VCPU. After that point e.g. s390 reenables preemption and starts to execute the
> guest. The thread might therefore be scheduled out between kvm_enter_guest() and
> kvm_exit_guest(), resulting in PF_VCPU being set but not being run.
> 
> Please note that preemption has to stay enabled in order to correctly process
> page faults on s390.
> 
> Current code takes PF_VCPU as a hint that the VCPU thread is running and
> therefore needs no yield. yield_to() checks whether the target thread is running,
> so let's use the inbuilt functionality to make it independent of PF_VCPU and
> preemption.
> 
> Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> ---
>  virt/kvm/kvm_main.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5b45330..184f52e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1782,10 +1782,6 @@ int kvm_vcpu_yield_to(struct kvm_vcpu *target)
>  	rcu_read_unlock();
>  	if (!task)
>  		return ret;
> -	if (task->flags & PF_VCPU) {
> -		put_task_struct(task);
> -		return ret;
> -	}
>  	ret = yield_to(task, 1);
>  	put_task_struct(task);
>  
> 

Applied with a rewritten commit message:

KVM: don't check for PF_VCPU when yielding

kvm_enter_guest() has to be called with preemption disabled and will
set PF_VCPU.  Current code takes PF_VCPU as a hint that the VCPU thread
is running and therefore needs no yield.

However, the check on PF_VCPU is wrong on s390, where preemption
has to stay enabled on s390 in order to correctly process page faults.
Thus, s390 reenables preemption and starts to execute the guest.
The thread might be scheduled out between kvm_enter_guest() and
kvm_exit_guest(), resulting in PF_VCPU being set but not being run.
When this happens, the opportunity for directed yield is missed.

However, this check is done already in kvm_vcpu_on_spin before calling
kvm_vcpu_yield_loop:

        if (!ACCESS_ONCE(vcpu->preempted))
                continue;

so the check on PF_VCPU is superfluous in general, and this patch 
removes it.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>