Re: [Qemu-devel] [PATCH] ppc: Fix size of ppc64 xer register (fwd)

[David Gibson]

On Fri, Feb 23, 2018 at 05:29:56PM +0000, Michael Matz wrote:
> The normal gdb definition of the XER registers is only 32 bit,
> and that's what the current version of power64-core.xml also
> says (seems copied from gdb's).  But qemu's idea of the XER register
> is target_ulong (in CPUPPCState, ppc_gdb_register_len and
> ppc_cpu_gdb_read_register)
> 
> That mismatch leads to the following message when attaching
> with gdb:
> 
>   Truncated register 32 in remote 'g' packet
> 
> (and following on that qemu stops responding).  The simple fix is
> to say the truth in the .xml file.  But the better fix is to
> actually make it 32bit on the wire, as old gdbs don't support
> XML files for describing registers.  Also the XER state in qemu
> doesn't seem to use the high 32 bits, so sending it off to gdb
> doesn't seem worthwhile.
> 
> Signed-off-by: Michael Matz <address@hidden>

Sorry I've taken so long to look at this.  I've now applied it to my
ppc-for-2.13 branch (since it's not a regression, I don't think it's
justified to include it during the 2.12 hard freeze).

In future, please CC me directly on mails (as ppc maintainer) and also
CC address@hidden  I might never have spotted this if Alex Graf
hadn't forwarded a pointer to me.

> ---
>  target/ppc/gdbstub.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 7a33813..b6f6693 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -37,10 +37,10 @@ static int ppc_gdb_register_len_apple(int n)
>      case 65+32: /* msr */
>      case 67+32: /* lr */
>      case 68+32: /* ctr */
> -    case 69+32: /* xer */
>      case 70+32: /* fpscr */
>          return 8;
>      case 66+32: /* cr */
> +    case 69+32: /* xer */
>          return 4;
>      default:
>          return 0;
> @@ -61,6 +61,8 @@ static int ppc_gdb_register_len(int n)
>          return 8;
>      case 66:
>          /* cr */
> +    case 69:
> +        /* xer */
>          return 4;
>      case 64:
>          /* nip */
> @@ -70,8 +72,6 @@ static int ppc_gdb_register_len(int n)
>          /* lr */
>      case 68:
>          /* ctr */
> -    case 69:
> -        /* xer */
>          return sizeof(target_ulong);
>      case 70:
>          /* fpscr */
> @@ -152,7 +152,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>              gdb_get_regl(mem_buf, env->ctr);
>              break;
>          case 69:
> -            gdb_get_regl(mem_buf, env->xer);
> +            gdb_get_reg32(mem_buf, env->xer);
>              break;
>          case 70:
>              gdb_get_reg32(mem_buf, env->fpscr);
> @@ -208,7 +208,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, uint8_t 
> *mem_buf, int n)
>              gdb_get_reg64(mem_buf, env->ctr);
>              break;
>          case 69 + 32:
> -            gdb_get_reg64(mem_buf, env->xer);
> +            gdb_get_reg32(mem_buf, env->xer);
>              break;
>          case 70 + 32:
>              gdb_get_reg64(mem_buf, env->fpscr);
> @@ -259,7 +259,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>              env->ctr = ldtul_p(mem_buf);
>              break;
>          case 69:
> -            env->xer = ldtul_p(mem_buf);
> +            env->xer = ldl_p(mem_buf);
>              break;
>          case 70:
>              /* fpscr */
> @@ -309,7 +309,7 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, 
> uint8_t *mem_buf, int n)
>              env->ctr = ldq_p(mem_buf);
>              break;
>          case 69 + 32:
> -            env->xer = ldq_p(mem_buf);
> +            env->xer = ldl_p(mem_buf);
>              break;
>          case 70 + 32:
>              /* fpscr */

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature