diff mbox

gdbstub/ppc: handle read and write of fpscr

Message ID 1297760349-35256-1-git-send-email-gingold@adacore.com
State New
Headers show

Commit Message

Tristan Gingold Feb. 15, 2011, 8:59 a.m. UTC 
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(-)

Comments

Peter Maydell Feb. 15, 2011, 10:22 a.m. UTC | #1 
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
Tristan Gingold Feb. 15, 2011, 10:31 a.m. UTC | #2 
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.
diff mbox

Patch

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;