Re: [Xen-devel] [PATCH] x86/vtsc: update vcpu_time after hvm_set_guest_time

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 06/04/2013 10:58 AM, Roger Pau Monnà wrote:
> On 04/06/13 11:47, George Dunlap wrote:
> > On 06/04/2013 10:10 AM, Roger Pau Monne wrote:
> > > When using a vtsc, hvm_set_guest_time changes hvm_vcpu.stime_offset,
> > > which is used in the vcpu time structure to calculate the
> > > tsc_timestamp, so after updating stime_offset we need to propagate the
> > > change to vcpu_time in order for the guest to get the right time if
> > > using the PV clock.
> > >
> > > This was not done correctly, since in context_switch
> > > update_vcpu_system_time was called before vmx_do_resume, which caused
> > > the vcpu_info time structure to be updated with the wrong values. This
> > > patch fixes this by calling update_vcpu_system_time after the call to
> > > hvm_set_guest_time has happened.
> > Would it make more sense to actually do this in hvm_set_guest_time()
> > instead, so that this window where the vcpu system_time is closed for
> > all callers, not just hvm_do_resume?
> >
> > You could gate calling update_vcpu_system_time on:
> > 1. Whether stime_offset actually changed
> > 2. Whether v is currently running
> This was my first approach, but I thought it was better to fix the
> actual call to update_vcpu_system_time to happen in the right place
> (before the context switch), rather than adding more calls to
> update_vcpu_system_time all over the place.
>
> If later we decide for whatever reason to add more offsets to the vtsc,
> we will also have to add more calls to update_vcpu_system_time in every
> place that we modify those offsets, which doesn't seem right.
But you will have to add those anyway if the guest is running. 
Otherwise you'll have this same window, where vcpu_info.system_time is 
out of sync with hvm_vcpu.stime_offset, and this same kind of bug can 
happen.
 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel