From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] qemu-kvm: x86: Fix mismerge in cpu_post_load Date: Sun, 06 Dec 2009 11:44:40 +0100 Message-ID: <4B1B8B18.7020803@web.de> References: <4B18018D.1060305@siemens.com> <20091206103354.GJ20102@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigBCD3D145BEC76ADEE030DA4A" Cc: Avi Kivity , Marcelo Tosatti , kvm To: Gleb Natapov Return-path: Received: from fmmailgate01.web.de ([217.72.192.221]:56535 "EHLO fmmailgate01.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933260AbZLFKol (ORCPT ); Sun, 6 Dec 2009 05:44:41 -0500 In-Reply-To: <20091206103354.GJ20102@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigBCD3D145BEC76ADEE030DA4A Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Gleb Natapov wrote: > On Thu, Dec 03, 2009 at 07:21:01PM +0100, Jan Kiszka wrote: >> Merge 8e2c5ec2f6 forgot to restore some qemu-kvm-specific hooks in >> cpu_post_load. mp_state was readded later on, but tsc was missing, >> breaking the guest timing after resume. Also, reset of halt was droppe= d >> which is obviously required for in-kernel irqchip. >> >> Signed-off-by: Jan Kiszka >> --- >> >> This solves "Problem 1" on my list. >> >> BTW, this patch just made me realize that the TSC MSR belongs to the >> list states that should not be written back unconditionally. Upstream >> does this currently, qemu-kvm not (at the price one more kvm-specific >> hook into generic code). Unlike the other states we discussed, this on= e >> is not "fixable" in the kernel. So I tend to think there is a real nee= d >> for my write-back scope abstraction - which would also be able to hand= le >> the other states cleanly, both in upstream and here. >> >> target-i386/machine.c | 10 +++++++++- >> 1 files changed, 9 insertions(+), 1 deletions(-) >> >> diff --git a/target-i386/machine.c b/target-i386/machine.c >> index 6bd447f..9ac477b 100644 >> --- a/target-i386/machine.c >> +++ b/target-i386/machine.c >> @@ -366,7 +366,15 @@ static int cpu_post_load(void *opaque, int versio= n_id) >> hw_breakpoint_insert(env, i); >> =20 >> tlb_flush(env, 1); >> - kvm_load_mpstate(env); >> + >> + if (kvm_enabled()) { >> + /* when in-kernel irqchip is used, env->halted causes deadloc= k >> + because no userspace IRQs will ever clear this flag */ >> + env->halted =3D 0; >> + > That is strange. env->halted should be used only for "info cpus" output= > in case of in-kernel irqchip. Can you see where it hangs? This line was not directly involved in the regression I saw, it was just the next one (load_tsc). But I simply restored qemu-kvm to the state before the vmstate conversion merge, dropping only obviously unneeded bits (namely the full register write-back). The above line may no longer be required, but I wanted to play safe. If you can explain which qemu-kvm change made this obsolete, please file a removal patch! >=20 >> + kvm_load_tsc(env); >> + kvm_load_mpstate(env); >> + } >> =20 >> return 0; >> } >=20 > -- > Gleb. Jan --------------enigBCD3D145BEC76ADEE030DA4A Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAksbixgACgkQitSsb3rl5xQn0wCgw9DD2IWvZgwrvIvbthzcV+YQ e5EAnAy0y1LHGt/6h+/HIv5nnZir2AG8 =EvK/ -----END PGP SIGNATURE----- --------------enigBCD3D145BEC76ADEE030DA4A--