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

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

On 04/06/13 12:00, George Dunlap wrote:
> 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.

This could be solved by also adding a call to update_vcpu_system_time in
pt_intr_post, but given that I'm not sure any more if it's not best to
just do the call to update_vcpu_system_time in hvm_set_guest_time and
get done with it.


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