Re: [PATCH v3 for 4.5] x86/viridian: Freeze time reference counter when domain is paused

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/10/14 10:21, Paul Durrant wrote:
> In XenServer system test it has become apparent that versions of Windows
> that make use of the time reference counter enlightenment cannot cope with
> large jumps forward in the value read from the MSR. Specifically,
> suspending a very large domain took approx. 45 minutes to complete and
> when the domain was resumed it was discovered that the WMI (Windows
> Management Instrumentation) service had hung.
>
> The reason a large jump forward is seen by the guest is that, when a guest
> is suspended, the guest stops running when the SCHEDOP_suspend hypercall is
> made, however the MSR value essentially keeps incrementing until the
> tool-stack issues DOMCTL_gethvmcontext.
>
> This patch adds code to freeze the value of the time reference counter
> on domain pause and 'thaw' it on domain unpause, but only thaw it if the
> domain is not shutting down. The absolute value of the counter is then
> saved in the viridian domain context record. This prevents the guest OS
> from experiencing large jumps in the value of the MSR and has been shown
> to reliably fix the problem with WMI.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

One nit, which could be fixed on commit given --whitespace=fix

<snip>

> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
> index 6726168..7d55363 100644
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -450,9 +484,10 @@ static int viridian_save_domain_ctxt(struct domain *d, hvm_domain_context_t *h)
>      if ( !is_viridian_domain(d) )
>          return 0;
>  
> -    ctxt.hypercall_gpa = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
> -    ctxt.guest_os_id   = d->arch.hvm_domain.viridian.guest_os_id.raw;
> -
> +    ctxt.time_ref_count = d->arch.hvm_domain.viridian.time_ref_count.val;
> +    ctxt.hypercall_gpa  = d->arch.hvm_domain.viridian.hypercall_gpa.raw;
> +    ctxt.guest_os_id    = d->arch.hvm_domain.viridian.guest_os_id.raw;
> +  

Introduction of trailing whitespace into the blank line.

~Andrew