qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: agraf@suse.de, pbonzini@redhat.com, aik@ozlabs.ru
Cc: lvivier@redhat.com, peter.maydell@linaro.org,
	mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org,
	qemu-ppc@nongnu.org, David Gibson <david@gibson.dropbear.id.au>
Subject: [Qemu-devel] [PATCH] target-ppc: Correct KVM synchronization for ppc_hash64_set_external_hpt()
Date: Fri,  1 Apr 2016 14:52:47 +1100	[thread overview]
Message-ID: <1459482767-12573-1-git-send-email-david@gibson.dropbear.id.au> (raw)

ppc_hash64_set_external_hpt() was added in e5c0d3c "target-ppc: Add helpers
for updating a CPU's SDR1 and external HPT".  This helper contains a
cpu_synchronize_state() since it may need to push state back to KVM
afterwards.

This turns out to break things when it is used in the reset path, which is
the only current user.  It appears that kvm_vcpu_dirty is not being set
early in the reset path, so the cpu_synchronize_state() is clobbering state
set up by the early part of the cpu reset path with stale state from KVM.

To fix this, remove the cpu_synchronize_state() from
ppc_hash64_set_external_hpt().  Any future non-reset-path users will need
to manually invoke cpu_synchronize_state().

Reported-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/mmu-hash64.c | 2 --
 1 file changed, 2 deletions(-)

Paolo, Peter,

This seems like the right minimal fix in the qemu-2.6 timeframe to fix
the actual bug.  However, longer term it seems like the correct thing
to do might be to set kvm_vcpu_dirty early in the reset path.  Thoughts?

diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 72c4ab5..caf41ce 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -283,8 +283,6 @@ void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
     CPUPPCState *env = &cpu->env;
     Error *local_err = NULL;
 
-    cpu_synchronize_state(CPU(cpu));
-
     if (hpt) {
         env->external_htab = hpt;
     } else {
-- 
2.5.5

             reply	other threads:[~2016-04-01  3:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-01  3:52 David Gibson [this message]
2016-04-01 10:28 ` [Qemu-devel] [PATCH] target-ppc: Correct KVM synchronization for ppc_hash64_set_external_hpt() Paolo Bonzini
2016-04-04  1:10   ` David Gibson
2016-04-04  2:59     ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1459482767-12573-1-git-send-email-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=lvivier@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).