Re: KVM: x86: fix pvclock guest stopped flag reporting

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Aug 02, 2012 at 12:43:58PM -0300, Marcelo Tosatti wrote:
> On Thu, Aug 02, 2012 at 06:42:25PM +0300, Avi Kivity wrote:
> > On 08/02/2012 06:33 PM, Marcelo Tosatti wrote:
> > > 
> > > kvm_guest_time_update unconditionally clears hv_clock.flags field, 
> > > so the notification never reaches the guest.
> > > 
> > > Fix it by allowing PVCLOCK_GUEST_STOPPED to passthrough.
> > > 
> > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > > 
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 3a53bcc..e8ce10f 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -1217,7 +1217,9 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> > >  	vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
> > >  	vcpu->last_kernel_ns = kernel_ns;
> > >  	vcpu->last_guest_tsc = tsc_timestamp;
> > > -	vcpu->hv_clock.flags = 0;
> > > +	/* only support PVCLOCK_GUEST_STOPPED flag ATM */
> > > +	if (vcpu->hv_clock.flags != PVCLOCK_GUEST_STOPPED)
> > > +		vcpu->hv_clock.flags = 0;
> > >  
> > 
> > Seems a little risky.  Should we store the flag in a separate bool and
> > mix it in instead of RMWing it?
> 
> This is not guest memory. Its a host copy, the actual data is copied
> (safely via the version mechanism) below in that function.
> 
> Are you ok with it now?

The patch is still incomplete as we must clear the flag in our copy
if the guest clears it.