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

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

On 11/06/13 18:02, Keir Fraser wrote:
> On 11/06/2013 17:41, "Roger Pau Monne" <roger.pau@citrix.com> wrote:
>
>> Call update_domain_wallclock_time on hvm_latch_shinfo_size even if
>> the bitness of the guest has already been set, this fixes the problem
>> with the wallclock not being set for PVHVM guests on resume from
>> migration.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Jan Beulich <JBeulich@suse.com>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> May as well write directly into d->arch.has_32bit_shinfo and get rid of
> new_has_32bit. But apart from that:
>
> Acked-by: Keir Fraser <keir@xen.org>

Yikes - we have had a patch in the XenServer patch queue for donkeys
years which implements the same fix as this.

I had still not manage to decide whether it was a gross hack which we
needed to discard or whether it needed upstreaming.

I guess this answers the question.

~Andrew

>
>> ---
>> Since this is a bug fix, I think it is suitable for inclusion in the
>> 4.3 release, and backported to older releases.
>> ---
>>  xen/arch/x86/hvm/hvm.c |   20 ++++++--------------
>>  1 files changed, 6 insertions(+), 14 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index a962ce2..0dcfd81 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3404,21 +3404,13 @@ static void hvm_latch_shinfo_size(struct domain *d)
>>       */
>>      if ( current->domain == d ) {
>>          new_has_32bit = (hvm_guest_x86_mode(current) != 8);
>> -        if (new_has_32bit != d->arch.has_32bit_shinfo) {
>> +        if (new_has_32bit != d->arch.has_32bit_shinfo)
>>              d->arch.has_32bit_shinfo = new_has_32bit;
>> -            /*
>> -             * Make sure that the timebase in the shared info
>> -             * structure is correct for its new bit-ness.  We should
>> -             * arguably try to convert the other fields as well, but
>> -             * that's much more problematic (e.g. what do you do if
>> -             * you're going from 64 bit to 32 bit and there's an event
>> -             * channel pending which doesn't exist in the 32 bit
>> -             * version?).  Just setting the wallclock time seems to be
>> -             * sufficient for everything we do, even if it is a bit of
>> -             * a hack.
>> -             */
>> -            update_domain_wallclock_time(d);
>> -        }
>> +        /*
>> +         * Make sure that the timebase in the shared info
>> +         * structure is correct.
>> +         */
>> +        update_domain_wallclock_time(d);
>>      }
>>  }
>>  
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel