[Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr

[Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr

[Tristan Gingold]

From: Fabien Chouteau <chouteau@adacore.com>

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: Tristan Gingold <gingold@adacore.com>
---
 gdbstub.c                   |    5 +++--
 target-ppc/translate_init.c |    5 ++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 5c9a1c9..70870fb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -722,7 +722,7 @@ static int cpu_gdb_read_register(CPUState *env, uint8_t *mem_buf, int n)
             {
                 if (gdb_has_xml)
                     return 0;
-                GET_REG32(0); /* fpscr */
+                GET_REG32(env->fpscr);
             }
         }
     }
@@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
             /* fpscr */
             if (gdb_has_xml)
                 return 0;
-            return 4;
+            env->fpscr = ldtul_p(mem_buf);
+            return sizeof(target_ulong);
         }
     }
     return 0;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index dfcd949..5d856f5 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9381,8 +9381,7 @@ static int gdb_get_float_reg(CPUState *env, uint8_t *mem_buf, int n)
         return 8;
     }
     if (n == 32) {
-        /* FPSCR not implemented  */
-        memset(mem_buf, 0, 4);
+        stl_p(mem_buf, env->fpscr);
         return 4;
     }
     return 0;
@@ -9395,7 +9394,7 @@ static int gdb_set_float_reg(CPUState *env, uint8_t *mem_buf, int n)
         return 8;
     }
     if (n == 32) {
-        /* FPSCR not implemented  */
+        env->fpscr = ldl_p(mem_buf);
         return 4;
     }
     return 0;
-- 
1.7.3.GIT

Re: [Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr

[Peter Maydell]

On 15 February 2011 08:59, Tristan Gingold <gingold@adacore.com> wrote:
> @@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
>             /* fpscr */
>             if (gdb_has_xml)
>                 return 0;
> -            return 4;
> +            env->fpscr = ldtul_p(mem_buf);
> +            return sizeof(target_ulong);
>         }
>     }
>     return 0;

Not a PPC expert, but this doesn't look right; for instance if you change
the rounding mode by fiddling with the FPSCR in the debugger this
won't update the softfloat rounding mode settings. (that is, it lets the
visible state in env->fpscr get out of sync with the hidden state of the
model). Also we probably shouldn't be letting the debugger change
reserved fpscr bits.

(Side note: linux-user/signal.c:restore_user_regs() appears to have
a similar fpscr-related bug.)

-- PMM

Re: [Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr

[Tristan Gingold]

On Feb 15, 2011, at 11:22 AM, Peter Maydell wrote:

> On 15 February 2011 08:59, Tristan Gingold <gingold@adacore.com> wrote:
>> @@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
>>             /* fpscr */
>>             if (gdb_has_xml)
>>                 return 0;
>> -            return 4;
>> +            env->fpscr = ldtul_p(mem_buf);
>> +            return sizeof(target_ulong);
>>         }
>>     }
>>     return 0;
> 
> Not a PPC expert, but this doesn't look right; for instance if you change
> the rounding mode by fiddling with the FPSCR in the debugger this
> won't update the softfloat rounding mode settings. (that is, it lets the
> visible state in env->fpscr get out of sync with the hidden state of the
> model). Also we probably shouldn't be letting the debugger change
> reserved fpscr bits.

Indeed, you're right.  We initially were interested in reading fpscr, and I wrote the writing part without thinking enough.

Tristan.