Re: [PATCH 2/5] KVM: arm/arm64: replace vcpu->arch.pause with a vcpu-request

[Christoffer Dall <cdall@linaro.org>]

On Wed, Mar 08, 2017 at 03:44:11PM +0100, Andrew Jones wrote:
> On Wed, Mar 08, 2017 at 06:33:12AM -0800, Christoffer Dall wrote:
> > >  static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
> > > @@ -621,7 +617,7 @@ 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 (vcpu->arch.power_off || __kvm_request_test(KVM_REQ_VCPU_PAUSE, vcpu))
> > >  			vcpu_sleep(vcpu);
> > >  
> > >  		/*
> > > @@ -644,8 +640,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> > >  			run->exit_reason = KVM_EXIT_INTR;
> > >  		}
> > >  
> > > +		vcpu->mode = IN_GUEST_MODE;
> > > +		smp_mb(); /* ensure vcpu->mode is visible to kvm_vcpu_kick */
> > 
> > I think this comment is misleading, because this smp_mb() is really
> > about ensuring that with respect to other CPUs, the write to vcpu->mode
> > is observable before the read of kvm_request_pending below, and the
> > corresponding other barrier is the barrier implied in cmpxchg used by
> > kvm_vcpu_exiting_guest_mode, which gets called by kvm_vcpu_kick(), which
> > is called after __kvm_set_request.
> 
> Agreed
> 

Just an adjustment to our conclusion from last week:

Will Deacon clarified that the cmpxchg doesn't have barrier semantics if
the cmpxchg operation fails.  My brain hurts trying to work out if we're
still safe in that case.  Thoughts?

Thanks,
-Christoffer