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

[Alexander Graf <agraf@suse.de>]

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?


Alex

> +            return sizeof(target_ulong);
>         }
>     }
>     return 0;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 09ad4ba..a5d2cc3 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7693,7 +7693,7 @@ static int gdb_set_float_reg(CPUPPCState *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.9.5
>