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

[Fabien Chouteau]

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 <address@hidden>
>> ---
>> 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