From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35187) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDaf1-0005Su-8Z for qemu-devel@nongnu.org; Tue, 20 Jan 2015 10:24:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDaex-0006QX-2J for qemu-devel@nongnu.org; Tue, 20 Jan 2015 10:24:07 -0500 Message-ID: <54BE72EC.9090602@ilande.co.uk> Date: Tue, 20 Jan 2015 15:23:24 +0000 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1419294981-17368-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1419294981-17368-5-git-send-email-mark.cave-ayland@ilande.co.uk> <54BE6CE5.4070409@redhat.com> In-Reply-To: <54BE6CE5.4070409@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/7] ppc: move sdr1 value change detection logic to helper_store_sdr1() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de On 20/01/15 14:57, Paolo Bonzini wrote: > On 23/12/2014 01:36, Mark Cave-Ayland wrote: >> Otherwise when cpu_post_load calls ppc_store_sdr1() when restoring a VM >> snapshot the value is deemed unchanged and so the internal env->htab* >> variables aren't set correctly. >> >> Signed-off-by: Mark Cave-Ayland >> CC: Paolo Bonzini >> --- >> target-ppc/misc_helper.c | 7 ++++++- >> target-ppc/mmu_helper.c | 35 +++++++++++++++-------------------- >> 2 files changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c >> index a577b3a..6b12ca8 100644 >> --- a/target-ppc/misc_helper.c >> +++ b/target-ppc/misc_helper.c >> @@ -77,8 +77,13 @@ void helper_msr_facility_check(CPUPPCState *env, uint32_t bit, >> >> void helper_store_sdr1(CPUPPCState *env, target_ulong val) >> { >> + PowerPCCPU *cpu = ppc_env_get_cpu(env); >> + >> if (!env->external_htab) { >> - ppc_store_sdr1(env, val); >> + if (env->spr[SPR_SDR1] != val) { >> + ppc_store_sdr1(env, val); >> + tlb_flush(CPU(cpu), 1); > > Possibly stupid question: should this tlb_flush be in ppc_store_sdr1, > maybe guarded by "if (tcg_enabled())"? > > Apart from this, the patch is okay. Thanks Paolo. I based this patch upon a comment in a slightly earlier thread here: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg03146.html. Is this still relevant or would you still like me to make the change? This is a little beyond my area of knowledge, but at the very least I can test any suggested changes under TCG fairly easily. ATB, Mark.