From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2] xen: fix initialization of wallclock time for PVHVM on migration Date: Wed, 12 Jun 2013 12:29:00 +0100 Message-ID: <51B85B7C.3010203@citrix.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Keir Fraser Cc: George Dunlap , "xen-devel@lists.xen.org" , Jan Beulich , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On 12/06/13 10:36, Keir Fraser wrote: > On 11/06/2013 19:09, "Andrew Cooper" wrote: > >> On 11/06/13 18:02, Keir Fraser wrote: >>> On 11/06/2013 17:41, "Roger Pau Monne" 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=E9 >>>> Cc: Jan Beulich >>>> Cc: Keir Fraser >>>> Cc: George Dunlap >>> 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 >> 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. > I believe the morally right(er) patch would be bigger and more fragile. T= his > is a bit of a hack, but the whole idea of inferring shared-info bitness f= rom > sampling some cpu's execution mode is a bit odd (but necessary). > > -- Keir The way XenServer does this is that the windows drivers explicitly make a hypercall to set their intended bitness using HVM_PARAM_32BIT, value 8, but it is itself a gross hack. ~Andrew > >> ~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 domai= n *d) >>>> */ >>>> if ( current->domain =3D=3D d ) { >>>> new_has_32bit =3D (hvm_guest_x86_mode(current) !=3D 8); >>>> - if (new_has_32bit !=3D d->arch.has_32bit_shinfo) { >>>> + if (new_has_32bit !=3D d->arch.has_32bit_shinfo) >>>> d->arch.has_32bit_shinfo =3D 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 >