Re: [Qemu-devel] [PATCH] PPC/GDB: handle read and write of fpscr

[Fabien Chouteau <chouteau@adacore.com>]

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 <chouteau@adacore.com>
>> ---
>> 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