Re: [PATCH] xen: fix initialization of wallclock time for PVHVM on migration

[Keir Fraser <keir.xen@gmail.com>]

On 11/06/2013 16:05, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 11/06/2013 15:16, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>> Would it be OK to call
>>> update_domain_wallclock_time unconditionally on
>>> hvm_hypercall_page_initialise?
>> 
>> The primary question is - why is what we have not enough for you?
>> In particular I would expect that the call from arch_set_info_guest()
>> (for vCPU 0) should do what you want. Or wait, this is covering PV
>> only. So yes, with the description change I would then withdraw my
>> NACK - apparently no-one really used the shared info wall clock
>> time in a HVM guest so far (or it going wrong post-resume wasn't
>> noticed).
>> 
>> I would, however, prefer the if() immediately preceding the patch
>> context to be pulled out past the domain_lock()ed region, convert it
>> to switch(), and add your code. That was, eventual other post-
>> processing for the various map spaces has a consistent, easily
>> extensible home.
> 
> I apparently made a fix for this to work on initial boot of a 32-bit PVHVM
> guest back in September (a change in hvmloader to not zero the wc fields in
> shared_info). But I agree I now can't see why it works... But it surely does
> as it was tested to do so by Konrad.
> 
> A bit more digging required...

Hmm I can't find any confirmation that my patch actually *did* work. :( I'm
sure I remember testing it though!

My suggestion is we do indeed remove the inner if() in latch_shinfo_size().
Ie. Call update_domain_wallclock_time() even if shinfo size has apparently
not changed. 

We only latch shinfo size on hypercall page initialisation and on setup of
the callback irq. They are start-of-day/resume operations, so removing the
if() should have no bad side effect that I can see. If nothing else it
should make this wallclock-field setup more robust.

 -- Keir

>  -- Keir
> 
>