Re: [PATCH] x86/time: Correctly update the domain watchdog in the shared info page

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

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)
>