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 <matz@suse.de>
---
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 */
--
1.8.1.4
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: alpine.LSU.2.21.1802231729040.10187@wotan.suse.de
Subject: [Qemu-devel] [PATCH] ppc: Fix size of ppc64 xer register (fwd)
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
41b2585f4d ppc: Fix size of ppc64 xer register (fwd)
=== OUTPUT BEGIN ===
Checking PATCH 1/1: ppc: Fix size of ppc64 xer register (fwd)...
ERROR: spaces required around that '+' (ctx:VxV)
#39: FILE: target/ppc/gdbstub.c:43:
+ case 69+32: /* xer */
^
total: 1 errors, 0 warnings, 59 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
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 <matz@suse.de> 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 qemu-ppc@nongnu.org. 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
© 2016 - 2025 Red Hat, Inc.