From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amzoS-0005qT-6d for qemu-devel@nongnu.org; Mon, 04 Apr 2016 04:24:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1amzoO-0002kV-W6 for qemu-devel@nongnu.org; Mon, 04 Apr 2016 04:24:44 -0400 References: <1459745351-13138-1-git-send-email-david@gibson.dropbear.id.au> From: Laurent Vivier Message-ID: <570224C3.6010702@redhat.com> Date: Mon, 4 Apr 2016 10:24:35 +0200 MIME-Version: 1.0 In-Reply-To: <1459745351-13138-1-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: David Gibson , pbonzini@redhat.com, peter.maydell@linaro.org Cc: mtosatti@redhat.com, qemu-ppc@nongnu.org, agraf@suse.de, qemu-devel@nongnu.org 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. > > 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 incorrect > 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. > > This patch moves the call to cpu_synchronize_all_states() into > qemu_system_reset() for safety, so it is always called. AFAICT this should > be safe for the handful of callers outside vl.c - these all appear to be in > places where the cpu state is already synchronized so the extra call > will be a no-op. > > Signed-off-by: David Gibson > --- > vl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > 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. > > Therefore, I'm hoping to apply this ASAP to 2.6. > > Laurent, could you please verify that this does indeed address the > problem with an incorrect MSR on entry. This fixes the unset SF bit of MSR in my test case, so: Tested-by: Laurent Vivier Laurent