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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.06.13 at 11:10, Roger Pau Monne <roger.pau@xxxxxxxxxx> 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.

So at the first glance I was thinking this would be fixing a regression
from commit ae5092f420e87a4a6b541bf581378c8cc0ee3a99, but
after a closer look it looks like this was done even earlier before.
Can you confirm this (not the least because this would have
implications on the need to backport this change)?

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -343,6 +343,12 @@ void hvm_do_resume(struct vcpu *v)
>      ioreq_t *p;
>  
>      pt_restore_timer(v);
> +    /*
> +     * Update vcpu_info, since the call to pt_restore_timer can change
> +     * the value in v->arch.hvm_vcpu.stime_offset that is used
> +     * to calculate the TSC in vcpu_info->time.
> +     */
> +    update_vcpu_system_time(v);

Adding it here means, unless I'm mistaken, the one in
context_switch() is now pointless, so I'd encourage you to
gate that one on !is_hvm_vcpu() (with a comment saying that
in this case it's being done in hvm_do_resume()).

Jan


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