qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fabiano Rosas <farosas@linux.ibm.com>
To: Alexey Kardashevskiy <aik@ozlabs.ru>,
	David Gibson <david@gibson.dropbear.id.au>
Cc: Richard Henderson <richard.henderson@linaro.org>,
	qemu-ppc@nongnu.org, groug@kaod.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs
Date: Thu, 31 Jan 2019 19:57:28 -0200	[thread overview]
Message-ID: <871s4s1odz.fsf@linux.ibm.com> (raw)
In-Reply-To: <c2c652ad-69af-5288-4861-aaf0e67e9b5b@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 31/01/2019 03:30, Fabiano Rosas wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>>>
>>> but this is a register which does not have endianness, the endianness
>>> appears here because the interface between gdb and qemu is
>>> uint8_t*==bytestream but this interface should have fixed endianness
>>> imho (now it is bigendian afaict).
>>>
>>> Something is not right here...
>> 
>> Having a fixed endianness would not work because GDB have no way of
>> knowing how to represent what comes from the remote end.
>
> It definitely would. A register is stored as "unsigned long" in QEMU and
> all gdb has to do is printf("%lx") and that is it. 

OK, but something is not clear to me. Even if GDB just printf("%lx") the
value, we would still have to bswap when the host is LE, right?

QEMU BE:
 (gdb) x/8xb &env->spr[287]
 0x11391760: 0x00    0x00    0x00    0x00    0x00    0x4e    0x12    0x00

QEMU LE:
 (gdb) x/8xb &env->spr[287]
 0x7ffff5bd98c0: 0x01    0x02    0x4b    0x00    0x00    0x00    0x00    0x00

> The problem is that
> we want to pass it as a byte stream from the gdb_read_register() hook
> all the way to gdb and for some reason gdb does not define endianness of
> that stream but rather tries guessing the endianness which is broken.

GDB does define the endianness of the stream (in a way):

 "Each byte of register data is described by two hex digits. The bytes
 with the register are transmitted in target byte order."

https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#Packets

> Today I was debugging rtas/clientinterface calls which a little endian
> kernel makes and these calls need to switch to the big endian first. And
> gdb goes nuts when this switch happens (note that I did not give an ELF
> to gdb this time so it picked LE itself). Even if it could fetch the
> endianness from QEMU, it would fail as it is an LE bit in MSR which is a
> register which is parsed according to the gdb's idea of endianness :)

I think it would be possible to define another packet for the remote
protocol that informs the endianness explicitly at each time the guest
stops. If you provide more info on how to reproduce that issue I could
put in my list or go bug GDB folks about it.

>> It will
>> always check the target endianness before printing a value, even if it
>> refers to a register:
>> 
>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/findvar.c;hb=HEAD#l49
>> 
>> So in our case the contents of mem_buf need to match both the guest
>> endianness *and* what GDB has set for 'show endian' because it will
>> detect it automatically from the ELF. If it guesses incorrectly because
>> there is no ELF, we need to use the 'set endian' command.
>> 
>> By the way, this is already the behavior for the registers that are
>> already implemented (e.g. $msr). Here's the commit that introduced
>> that:
>> 
>> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8a286ce4502356ce0b97a2424a2cb7
>> 
>> Now, what might be a source of confusion here is the fact that we
>> *always* do a bswap when the host is LE because QEMU thinks that the ppc
>> guest is always BE. That requires the maybe_bswap function to make
>> things right in the end.
>> 
>> What I could do is try to improve this by only swapping when the
>> guest's actual endianness (msr_le) is different from the host's.
>
> The bytestream for registers should have fixed endianness. But looking
> at the gdb code makes me think it is not going to happen :(

Yes, I can't think of a way to fix that without changing the way GDB
exhibits the values or the remote protocol.

May I proceed with this series as it is with the bswaps?

>> That
>> is not entirely within the scope of this patch, though.
>
> True. But since you are touching this, may be you could fix gdb too :)
>
> Does gdb tell QEMU about what endianness it thinks that QEMU is using?
> Or can it read it from QEMU? I cannot easily spot this in QEMU...

GDB currently does not tell and does not ask about endianness. So I
believe there is room for improvement here. I could not find it in the
documentation but I think that GDB supports file transfers and it asks
for the ELF in some scenarios. This approach could be one way of
informing it about the endianness, although it has its own shortcomings.

  reply	other threads:[~2019-01-31 22:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 17:01 [Qemu-devel] [PATCH v4 0/3] ppc/gdbstub: Expose SPRs to GDB Fabiano Rosas
2019-01-22 17:01 ` [Qemu-devel] [PATCH v4 1/3] target/ppc: Add SPRs XML generation code for gdbstub Fabiano Rosas
2019-01-24  6:34   ` Alexey Kardashevskiy
2019-01-22 17:01 ` [Qemu-devel] [PATCH v4 2/3] target/ppc: Add GDB callbacks for SPRs Fabiano Rosas
2019-01-24  7:20   ` [Qemu-devel] [Qemu-ppc] " Alexey Kardashevskiy
2019-01-26  1:50     ` David Gibson
2019-01-28 20:00       ` Fabiano Rosas
2019-01-29  0:28         ` Alexey Kardashevskiy
2019-01-30 16:30           ` Fabiano Rosas
2019-01-31  7:57             ` Alexey Kardashevskiy
2019-01-31 21:57               ` Fabiano Rosas [this message]
2019-02-01  4:02                 ` Alexey Kardashevskiy
2019-02-01 12:01                   ` Fabiano Rosas
2019-02-04  3:25                     ` Alexey Kardashevskiy
2019-01-22 17:01 ` [Qemu-devel] [PATCH v4 3/3] target/ppc: Enable reporting of SPRs to GDB Fabiano Rosas
2019-01-24  7:23   ` Alexey Kardashevskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871s4s1odz.fsf@linux.ibm.com \
    --to=farosas@linux.ibm.com \
    --cc=aik@ozlabs.ru \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).