Patchwork gdbstub/ppc: handle read and write of fpscr

login
register
mail settings
Submitter Tristan Gingold
Date Feb. 15, 2011, 8:59 a.m.
Message ID <1297760349-35256-1-git-send-email-gingold@adacore.com>
Download mbox | patch
Permalink /patch/83224/
State New
Headers show

Comments

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

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;