David Gibson <da...@gibson.dropbear.id.au> writes:
> On Thu, Jan 24, 2019 at 06:20:02PM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 23/01/2019 04:01, Fabiano Rosas wrote:
>> > These will be used to let GDB know about PPC's Special Purpose
>> > Registers (SPR).
>> >
>> > They take an index based on the order the registers appear in the XML
>> > file sent by QEMU to GDB. This index does not match the actual
>> > location of the registers in the env->spr array so the
>> > gdb_find_spr_idx function does that conversion.
>> >
>> > Signed-off-by: Fabiano Rosas <faro...@linux.ibm.com>
>> > ---
>> > target/ppc/translate_init.inc.c | 54 ++++++++++++++++++++++++++++++++-
>> > 1 file changed, 53 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/target/ppc/translate_init.inc.c
>> > b/target/ppc/translate_init.inc.c
>> > index 710064a25d..f29ac3558a 100644
>> > --- a/target/ppc/translate_init.inc.c
>> > +++ b/target/ppc/translate_init.inc.c
>> > @@ -9487,6 +9487,55 @@ static bool avr_need_swap(CPUPPCState *env)
>> > #endif
>> > }
>> >
>> > +#if !defined(CONFIG_USER_ONLY)
>> > +static int gdb_find_spr_idx(CPUPPCState *env, int n)
>> > +{
>> > + int i;
>> > +
>> > + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
>> > + ppc_spr_t *spr = &env->spr_cb[i];
>> > +
>> > + if (spr->name && spr->gdb_id == n) {
>> > + return i;
>> > + }
>> > + }
>> > + return -1;
>> > +}
>> > +
>> > +static int gdb_get_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
>> > +{
>> > + int reg;
>> > + int len;
>> > +
>> > + reg = gdb_find_spr_idx(env, n);
>> > + if (reg < 0) {
>> > + return 0;
>> > + }
>> > +
>> > + len = TARGET_LONG_SIZE;
>> > + stn_p(mem_buf, len, env->spr[reg]);
>> > + ppc_maybe_bswap_register(env, mem_buf, len);
>>
>>
>> I am confused by this as it produces different results depending on the
>> guest mode:
>
>
> Hm, yeah, I thought the bswap here looked odd, but it wasn't obvious
> to me if it was bogus here, or just a bogus gdb interface we have to
> work around.
>
>> (gdb) p $pvr
>> $1 = 0x14c0000000000
>> (gdb) c
>> Continuing.
>> Program received signal SIGINT, Interrupt.
>> (gdb) p $pvr
>> $2 = 0x4c0100
>
> But that behaviour definitely looks wrong.
GDB detects the endianness by looking at the ELF headers:
(gdb) p/x $pvr
$1 = 0x1024b0000000000
(gdb) file ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf
Reading symbols from ~/qemu/roms/SLOF/board-qemu/llfw/stage1.elf...done.
(gdb) show endian
The target endianness is set automatically (currently big endian)
(gdb) p/x $pvr
$2 = 0x4b0201
(gdb) c
Continuing.
(gdb) ^C
Program received signal SIGINT, Interrupt.
0x74a70c00000000c0 in ?? ()
(gdb) file vmlinux
Reading symbols from vmlinux...done.
(gdb) show endian
The target endianness is set automatically (currently little endian)
(gdb) p/x $pvr
$3 = 0x4b0201
The maybe_bswap_register is done due to QEMU having TARGET_WORDS_BIGENDIAN set
even after we have changed into LE mode.
>> First print is when I stopped the guest in the SLOF firmware (so it is
>> big-endian) and then I continued and stopped gdb when the guest booted a
>> little-endian system; the KVM host is little endian, the machine running
>> gdb is LE too.
>>
>> QEMU monitor prints the same 0x4c0100 in both cases.
>>
>> I am adding the inventor of maybe_bswap_register() in cc: for
>> assistance. Swapping happens:
>> - once for BE: after stn_p()
>> *(unsigned long *)mem_buf is 0x14c0000000000
>> - twice for LE.
>>
>>
>>
>>
>>
>> > + return len;
>> > +}
>> > +
>> > +static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
>> > +{
>> > + int reg;
>> > + int len;
>> > +
>> > + reg = gdb_find_spr_idx(env, n);
>> > + if (reg < 0) {
>> > + return 0;
>> > + }
>> > +
>> > + len = TARGET_LONG_SIZE;
>> > + ppc_maybe_bswap_register(env, mem_buf, len);
>> > + env->spr[reg] = ldn_p(mem_buf, len);
>> > +
>> > + return len;
>> > +}
>> > +#endif
>> > +
>> > static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
>> > {
>> > if (n < 32) {
>> > @@ -9716,7 +9765,10 @@ static void ppc_cpu_realize(DeviceState *dev, Error
>> > **errp)
>> > gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
>> > 32, "power-vsx.xml", 0);
>> > }
>> > -
>> > +#ifndef CONFIG_USER_ONLY
>> > + gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
>> > + pcc->gdb_num_sprs, "power-spr.xml", 0);
>> > +#endif
>> > qemu_init_vcpu(cs);
>> >
>> > pcc->parent_realize(dev, errp);
>> >
>>
