Xen devel - Jul 2013 - [PATCH] x86/time: Correctly update the domain watchdog in the shared info page

For a normal HVM domain on 64bit Xen, the shared info switches to 32bit for
hvmloader, then to the native size for the HVM guest.  This guarantees at
least one transition during which the wallclock information will be updated.

The wallclock for each domain gets updated when dom0 issues a XENPF_settime
hypercall to updated the wallclock base time.

However, for a 64bit domain on 64bit Xen which is resuming from S4,
(i.e. bypassing hvmloader), there are no transitions in the size of the shared
info page, meaning that before the next XENPF_settime from dom0, the domU PV
drivers will find the Unix Epoch in the wallclock rather than the correct
time.

Therefore, manually set the watchdog when creating the domain, so it is set
right from the start of the domain, and when manually adjusting the domain
time offset, as it depends on the same value which has just been adjusted.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <Paul.Durrant@citrix.com>
---
This fixes a bug presence since Xen gained 64bit support, which had been
hacked around in a gross way in XenServer.

It should be backported to all stable releases.
---
 xen/arch/x86/domain.c |    1 +
 xen/arch/x86/time.c   |    1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 874742c..664d598 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -546,6 +546,7 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
         clear_page(d->shared_info);
         share_xen_page_with_guest(
             virt_to_page(d->shared_info), d, XENSHARE_writable);
+        update_domain_wallclock_time(d);
 
         if ( (rc = init_domain_irq_mapping(d)) != 0 )
             goto fail;
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index cf8bc78..f047cb3 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -931,6 +931,7 @@ void domain_set_time_offset(struct domain *d, int32_t
time_offset_seconds)
     d->time_offset_seconds = time_offset_seconds;
     if ( is_hvm_domain(d) )
         rtc_update_clock(d);
+    update_domain_wallclock_time(d);
 }
 
 int cpu_frequency_change(u64 freq)
-- 
1.7.10.4

>>> On 16.07.13 at 11:51, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
Am I right in assuming that the title was supposed to say "wallclock"
instead of "watchdog"?

Jan

On 16/07/13 11:20, Jan Beulich wrote:
>>>> On 16.07.13 at 11:51, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> Am I right in assuming that the title was supposed to say
"wallclock"
> instead of "watchdog"?
>
> Jan
Very much so - I had a brainfart when writing the message.  I shall resend.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On 16/07/2013 10:51, "Andrew Cooper" <andrew.cooper3@citrix.com>
wrote:
> For a normal HVM domain on 64bit Xen, the shared info switches to 32bit for
> hvmloader, then to the native size for the HVM guest.  This guarantees at
> least one transition during which the wallclock information will be
updated.
> 
> The wallclock for each domain gets updated when dom0 issues a XENPF_settime
> hypercall to updated the wallclock base time.
> 
> However, for a 64bit domain on 64bit Xen which is resuming from S4,
> (i.e. bypassing hvmloader), there are no transitions in the size of the
shared
> info page, meaning that before the next XENPF_settime from dom0, the domU
PV
> drivers will find the Unix Epoch in the wallclock rather than the correct
> time.
> 
> Therefore, manually set the watchdog when creating the domain, so it is set
> right from the start of the domain, and when manually adjusting the domain
> time offset, as it depends on the same value which has just been adjusted.
s/watchdog/wallclock/

Also I wonder if this is not fixed by f8e8fd56 already? Not that I''m
averse
to throwing in a few more update_domain_wallclock_time() calls if they are
useful.

 -- Keir
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Paul Durrant <Paul.Durrant@citrix.com>
> ---
> This fixes a bug presence since Xen gained 64bit support, which had been
> hacked around in a gross way in XenServer.
> 
> It should be backported to all stable releases.
> ---
>  xen/arch/x86/domain.c |    1 +
>  xen/arch/x86/time.c   |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 874742c..664d598 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -546,6 +546,7 @@ int arch_domain_create(struct domain *d, unsigned int
> domcr_flags)
>          clear_page(d->shared_info);
>          share_xen_page_with_guest(
>              virt_to_page(d->shared_info), d, XENSHARE_writable);
> +        update_domain_wallclock_time(d);
>  
>          if ( (rc = init_domain_irq_mapping(d)) != 0 )
>              goto fail;
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index cf8bc78..f047cb3 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -931,6 +931,7 @@ void domain_set_time_offset(struct domain *d, int32_t
> time_offset_seconds)
>      d->time_offset_seconds = time_offset_seconds;
>      if ( is_hvm_domain(d) )
>          rtc_update_clock(d);
> +    update_domain_wallclock_time(d);
>  }
>  
>  int cpu_frequency_change(u64 freq)

On 16/07/13 11:32, Keir Fraser wrote:
> On 16/07/2013 10:51, "Andrew Cooper"
<andrew.cooper3@citrix.com> wrote:
>
>> For a normal HVM domain on 64bit Xen, the shared info switches to 32bit
for
>> hvmloader, then to the native size for the HVM guest.  This guarantees
at
>> least one transition during which the wallclock information will be
updated.
>>
>> The wallclock for each domain gets updated when dom0 issues a
XENPF_settime
>> hypercall to updated the wallclock base time.
>>
>> However, for a 64bit domain on 64bit Xen which is resuming from S4,
>> (i.e. bypassing hvmloader), there are no transitions in the size of the
shared
>> info page, meaning that before the next XENPF_settime from dom0, the
domU PV
>> drivers will find the Unix Epoch in the wallclock rather than the
correct
>> time.
>>
>> Therefore, manually set the watchdog when creating the domain, so it is
set
>> right from the start of the domain, and when manually adjusting the
domain
>> time offset, as it depends on the same value which has just been
adjusted.
> s/watchdog/wallclock/
>
> Also I wonder if this is not fixed by f8e8fd56 already? Not that
I''m averse
> to throwing in a few more update_domain_wallclock_time() calls if they are
> useful.
>
>  -- Keir
Already resent to that effect, and not really fixed by f8e8fd56

That certainly fixes one instance of the bug, but still results in a
wrong wallclock between a XEN_DOMCTL_settimeoffset and XENPF_settime.

~Andrew
>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Paul Durrant <Paul.Durrant@citrix.com>
>> ---
>> This fixes a bug presence since Xen gained 64bit support, which had
been
>> hacked around in a gross way in XenServer.
>>
>> It should be backported to all stable releases.
>> ---
>>  xen/arch/x86/domain.c |    1 +
>>  xen/arch/x86/time.c   |    1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 874742c..664d598 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -546,6 +546,7 @@ int arch_domain_create(struct domain *d, unsigned
int
>> domcr_flags)
>>          clear_page(d->shared_info);
>>          share_xen_page_with_guest(
>>              virt_to_page(d->shared_info), d, XENSHARE_writable);
>> +        update_domain_wallclock_time(d);
>>  
>>          if ( (rc = init_domain_irq_mapping(d)) != 0 )
>>              goto fail;
>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>> index cf8bc78..f047cb3 100644
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -931,6 +931,7 @@ void domain_set_time_offset(struct domain *d,
int32_t
>> time_offset_seconds)
>>      d->time_offset_seconds = time_offset_seconds;
>>      if ( is_hvm_domain(d) )
>>          rtc_update_clock(d);
>> +    update_domain_wallclock_time(d);
>>  }
>>  
>>  int cpu_frequency_change(u64 freq)
>