From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH v2] xen: fix initialization of wallclock time for PVHVM on migration Date: Wed, 12 Jun 2013 10:36:27 +0100 Message-ID: References: <51B767E0.6050007@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <51B767E0.6050007@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: George Dunlap , "xen-devel@lists.xen.org" , Jan Beulich , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org 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. This is a bit of a hack, but the whole idea of inferring shared-info bitness from sampling some cpu's execution mode is a bit odd (but necessary). -- Keir > ~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 =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 > =