From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42998) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHyEv-00078R-94 for qemu-devel@nongnu.org; Tue, 19 Mar 2013 11:14:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHyEp-0005Iq-Gh for qemu-devel@nongnu.org; Tue, 19 Mar 2013 11:14:13 -0400 Message-ID: <514880BD.8040009@adacore.com> Date: Tue, 19 Mar 2013 16:14:05 +0100 From: Fabien Chouteau MIME-Version: 1.0 References: <1363694598-19799-1-git-send-email-chouteau@adacore.com> <1F098328-BA3A-40BC-955B-9CDFF470D257@suse.de> In-Reply-To: <1F098328-BA3A-40BC-955B-9CDFF470D257@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] PPC/GDB: handle read and write of fpscr List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 03/19/2013 01:10 PM, Alexander Graf wrote: > > On 19.03.2013, at 13:03, Fabien Chouteau wrote: > >> Although the support of this register may be uncomplete, there are no >> reason to prevent the debugger from reading or writing it. >> >> Signed-off-by: Fabien Chouteau >> --- >> gdbstub.c | 3 ++- >> target-ppc/translate_init.c | 2 +- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/gdbstub.c b/gdbstub.c >> index e414ad9..d23d9c5 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -781,7 +781,8 @@ static int cpu_gdb_write_register(CPUPPCState *env, uint8_t *mem_buf, int n) >> /* fpscr */ >> if (gdb_has_xml) >> return 0; >> - return 4; >> + env->fpscr = ldtul_p(mem_buf); > > Check out helper_store_fpscr() in target-ppc/fpu_helper.c. Storing fpscr has a bunch of side effects that won't happen when you just set the env variable. I'd prefer not to enable users to set fpscr when we can't guarantee that the updated values are actually used. > > Can't you just call the helper function here? > I can if I include helper.h, unfortunately this file is named helpers.h (note the 's') for m68k so I wont be able to include it for all platforms. Either I put an include in the middle of the source (in a #if defined (TARGET_PPC) section), I don't know if this is allowed in Qemu: @@ -691,6 +695,8 @@ static int cpu_gdb_write_register(CPUX86State *env, uint8_t *mem_buf, int n) #elif defined (TARGET_PPC) +#include "helper.h" + /* Old gdb always expects FP registers. Newer (xml-aware) gdb only expects whatever the target description contains. Due to a historical mishap the FP registers appear in between core integer or I just add a new PPC section after the others includes: @@ -42,6 +42,10 @@ #include "sysemu/kvm.h" #include "qemu/bitops.h" +#if defined (TARGET_PPC) +#include "helper.h" +#endif + #ifndef TARGET_CPU_MEMORY_RW_DEBUG static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr, uint8_t *buf, int len, int is_write) -- Fabien Chouteau