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

["Roger Pau Monné" <roger.pau@citrix.com>]

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.