From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51971) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SF5Ba-0000gf-F6 for qemu-devel@nongnu.org; Tue, 03 Apr 2012 10:58:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SF5BY-0000NN-JX for qemu-devel@nongnu.org; Tue, 03 Apr 2012 10:58:18 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:64708) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SF5BY-0000N6-Ee for qemu-devel@nongnu.org; Tue, 03 Apr 2012 10:58:16 -0400 Received: by iafj26 with SMTP id j26so6952777iaf.4 for ; Tue, 03 Apr 2012 07:58:12 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1331569485-3559-1-git-send-email-peter.maydell@linaro.org> Date: Tue, 3 Apr 2012 10:58:11 -0400 Message-ID: From: Christoffer Dall Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] gdbstub: Synchronize CPU state unconditionally in gdb_set_cpu_pc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: patches@linaro.org, qemu-devel@nongnu.org, Alexander Graf On Tue, Apr 3, 2012 at 7:57 AM, Peter Maydell wr= ote: > Ping? > sorry about late response. > > On 12 March 2012 16:24, Peter Maydell wrote: >> Synchronize the CPU state via cpu_sychronize_state() unconditionally >> in gdb_set_cpu_pc() rather than only in some of the target ifdef >> ladder cases. >> >> We can divide the CPUs into three categories: >> =A0* non-KVM targets: no change of behaviour since we will use the >> =A0 kvm-stub.c no-op function. >> =A0* i386 and s390: no change of behaviour since they were already >> =A0 calling this function >> =A0* PPC (in KVM mode): this fixes an error: failing to synchronise >> =A0 was accidental and probably a bug. >> >> This also paves the way for other targets (specifically ARM) which >> can add KVM support in future without having to add another target >> specific change to this bit of code. >> >> Signed-off-by: Peter Maydell >> --- >> Alex: could you test the KVM PPC case, please, since that's the only >> one where we actually change behaviour here? >> >> =A0gdbstub.c | =A0 =A03 +-- >> =A01 files changed, 1 insertions(+), 2 deletions(-) >> >> diff --git a/gdbstub.c b/gdbstub.c >> index ef95ac2..776dcc5 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -1904,8 +1904,8 @@ static void gdb_breakpoint_remove_all(void) >> >> =A0static void gdb_set_cpu_pc(GDBState *s, target_ulong pc) >> =A0{ >> -#if defined(TARGET_I386) >> =A0 =A0 cpu_synchronize_state(s->c_cpu); >> +#if defined(TARGET_I386) >> =A0 =A0 s->c_cpu->eip =3D pc; >> =A0#elif defined (TARGET_PPC) >> =A0 =A0 s->c_cpu->nip =3D pc; >> @@ -1930,7 +1930,6 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulo= ng pc) >> =A0#elif defined (TARGET_ALPHA) >> =A0 =A0 s->c_cpu->pc =3D pc; >> =A0#elif defined (TARGET_S390X) >> - =A0 =A0cpu_synchronize_state(s->c_cpu); >> =A0 =A0 s->c_cpu->psw.addr =3D pc; >> =A0#elif defined (TARGET_LM32) >> =A0 =A0 s->c_cpu->pc =3D pc; >> -- >> 1.7.1 >> looks good to me - much cleaner than what I had to do for ARM otherwise. -Christoffer