From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amzKD-00045r-Cc for qemu-devel@nongnu.org; Mon, 04 Apr 2016 03:53:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1amzK9-0004fo-3m for qemu-devel@nongnu.org; Mon, 04 Apr 2016 03:53:29 -0400 Date: Mon, 4 Apr 2016 17:54:35 +1000 From: David Gibson Message-ID: <20160404075435.GN16485@voom.fritz.box> References: <1459745351-13138-1-git-send-email-david@gibson.dropbear.id.au> <57021844.6030605@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EqVOK5mkaJAMmtSx" Content-Disposition: inline In-Reply-To: <57021844.6030605@redhat.com> Subject: Re: [Qemu-devel] [PATCH] vl: Move cpu_synchronize_all_states() into qemu_system_reset() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: lvivier@redhat.com, peter.maydell@linaro.org, mtosatti@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org --EqVOK5mkaJAMmtSx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 04, 2016 at 09:31:16AM +0200, Paolo Bonzini wrote: >=20 >=20 > On 04/04/2016 06:49, David Gibson wrote: > > There are currently 3 calls to qemu_system_reset() in vl.c. Two of them > > are immediately preceded by a cpu_synchronize_all_states9) and the > > remaining one should be. > >=20 > > The one which doesn't is the very first reset called directly from main= (). > > Without a cpu_synchronize_all_states(), kvm_vcpu_dirty is false at this > > point from the earlier cpu_synchronize_all_post_init(). That's incorre= ct > > because the reset path is quite likely to update the CPU state, and that > > updated state should be pushed back to KVM, not overwritten with stale > > data pushed to KVM immediately after init. > >=20 > > This patch moves the call to cpu_synchronize_all_states() into > > qemu_system_reset() for safety, so it is always called. AFAICT this sh= ould > > be safe for the handful of callers outside vl.c - these all appear to b= e in > > places where the cpu state is already synchronized so the extra call > > will be a no-op. > >=20 > > Signed-off-by: David Gibson > > --- > > vl.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > >=20 > > This fixes a real but on ppc - the incorrect state of kvm_vcpu_dirty > > means that an extra cpu_synchronize_state() in revised code for > > updating the MSR is not a no-op as expected and loads a stale MSR > > value, resulting in an incorrect MSR value on entry to the guest. > >=20 > > Therefore, I'm hoping to apply this ASAP to 2.6. > >=20 > > Laurent, could you please verify that this does indeed address the > > problem with an incorrect MSR on entry. > >=20 > > diff --git a/vl.c b/vl.c > > index bd81ea9..3629336 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -1745,6 +1745,8 @@ void qemu_system_reset(bool report) > > =20 > > mc =3D current_machine ? MACHINE_GET_CLASS(current_machine) : NULL; > > =20 > > + cpu_synchronize_all_states(); > > + > > if (mc && mc->reset) { > > mc->reset(); > > } else { > > @@ -1893,7 +1895,6 @@ static bool main_loop_should_exit(void) > > } > > if (qemu_reset_requested()) { > > pause_all_vcpus(); > > - cpu_synchronize_all_states(); > > qemu_system_reset(VMRESET_REPORT); > > resume_all_vcpus(); > > if (!runstate_check(RUN_STATE_RUNNING) && > > @@ -1903,7 +1904,6 @@ static bool main_loop_should_exit(void) > > } > > if (qemu_wakeup_requested()) { > > pause_all_vcpus(); > > - cpu_synchronize_all_states(); > > qemu_system_reset(VMRESET_SILENT); > > notifier_list_notify(&wakeup_notifiers, &wakeup_reason); > > wakeup_reason =3D QEMU_WAKEUP_REASON_NONE; > >=20 >=20 > Acked-by: Paolo Bonzini Thanks Paolo. Paolo / Peter, should I send this in a pull request for my tree, or should it go through someone else's tree? --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --EqVOK5mkaJAMmtSx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXAh27AAoJEGw4ysog2bOSugYP/Ar/fPUJ5mLKhjs01Nl+cxL3 0Q14TYvxaQULGoSG8HmaE6Aut2ovXgkRBDL2KhKUifdxVFwC+k8w7TTF9Hm2LAVs Vqz07ax+7r9izO+6dvlvf1xkAJPNKrBe3JVpeeDEW+Z3rTCw9LblaL6DrlFIatrK Xyu/Y36C013Ha1Bf5qlX9wxtq9mhsgKNeDY+8GJmd4rULbyJ+3n6UzXOnXFhipqB BBfQs416T6IbQHusAuTuq1X3Vf/AxD/haY8f0Sqv4gxJo9TgL0lGGTi2cFJFt2r/ 7B6oLEchOtM/cj0miICSsWn2+82DyKaeXOHZeBS2V4O9dohNdcLbBViw8hNQiV5X 3jsebcI8vs79sZZB+glR4+sTq1IX8aQiY6CI9CypUzABQvC1Y85AA/aVcgauWoba mWLnliLBT+QYz/Cj3ap0Ciup21sKQOb2U9R/ppYGqwjZFDr152xZniCQ+rRbBrZ9 oBlOgxIvvoqFHWhHeAVs2k5mFvV86YEOvSRdknZcDklgf9VZzMuCtvgyX3fEm/bz TFP9Oym3ExYxu4QQH2/a1YzKIO7XtD2DPJJ9Q6jNB4jeK7mDvSa0gjBjqNlM+m3j f9PL8HsVJWQ1oSpkOGupk5RYXSHPhhgA4qVBEKYnH0UANCSWQDVFNW24oWRpdvVd tjtArn9bzVMuijqLq5SN =DdvV -----END PGP SIGNATURE----- --EqVOK5mkaJAMmtSx--