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

I'm not sure if only updating the vcpu time if the vCPU is running can
lead to a slip in the vcpu time update. As an example, if we call
hvm_set_guest_time with the vCPU not running and change the offset, and
then call it again on resume, but the offset has not changed this time
the vcpu time will not be updated. I would rather update the vcpu time
info every time there's a change in the offset.