Re: [PATCH] gdb/riscv: Add target description support

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Jim Wilson <jimw@sifive.com> [2019-02-26 11:27:24 -0800]:

> On Tue, Feb 26, 2019 at 10:40 AM Jim Wilson <jimw@sifive.com> wrote:
> > >   (3) Renumbered fflags, frm, and fcsr as 66, 67, and 68.  This is
> > >   where the issues will appear for QEMU, Jim's QEMU patch had adopted
> > >   the "new" default numbering which placed these registers after the
> > >   floating point registers (so they had become 65, 66, and 67).
> 
> qemu just assigns numbers to xml regs if they don't have them,
> incrementing by one for each reg.  If we change the qemu xml files at
> the same time as we change the qemu gdbstub hooks, then qemu will
> always be internally consistent.  These xml register numbers aren't
> user visible anywhere inside qemu, they are only used for
> communication with gdb.
> 
> > > If we want backward compatibility then we should merge this GDB patch,
> > > and fix QEMU asap to avoid having two incompatible versions in the
> > > wild.
> 
> I have no control over qemu.  I can only suggest a patch, and ping it,
> and maybe in a few months it will get merged in.

Sure, I understand that, but I was wrong with the severity of the
problem.  So, there's no rush on changing QEMU.

> 
> > > What I don't understand about all this is why QEMU appears to be
> > > discarding one of the big benefits of xml register descriptions; the
> > > ability to disconnect their register numbering from GDB's register
> > > numbering.
> 
> I don't understand the comment.  We still must have numbers for the
> registers, otherwise we can't communicate with gdb about them.  But
> these xml register numbers don't affect the user or hardware register
> numbers, the qemu gdbstub converts between hardware register numbers
> and xml register numbers.

So, I miss-understood the problem slightly, I'll explain what I was
trying to say, then explain why it doesn't really matter...

Consider this cut-down version the 32bit-fpu.xml for RISC-V:

  <feature name="org.gnu.gdb.riscv.fpu">
    <reg name="ft0" bitsize="32" type="ieee_single"/>
  </feature>

Internally QEMU will have its hardware register number that represents
register $ft0, this could be anything, lets pick 105 at random.

GDB will also have a number that represents register $ft0, this too
could be anything, lets pick 33 at random, this isn't the xml register
number you mention above, this is just GDBs internal number.

If QEMU sends this to GDB:

  <feature name="org.gnu.gdb.riscv.fpu">
    <reg name="ft0" bitsize="32" type="ieee_single" regnum="105"/>
  </feature>

Then internally GDB will call $ft0 #33, but, when it talks to QEMU it
will use #105.

If GDB changes its numbering it will continue to use #105 when talking
to QEMU.

However, what QEMU _actually_ sends is this:

  <feature name="org.gnu.gdb.riscv.fpu">
    <reg name="ft0" bitsize="32" type="ieee_single"/>
  </feature>

An exact copy of GDBs XML file.  Without being told how the remote
would like to number its registers GDB just numbers the registers in
the order that they are sent from the remote, and this establishes
fairly arbitrary numbering scheme that QEMU is forced to adopt, if GDB
ever changes the feature files and QEMU picks these changes up, then
it would have to adopt its GDB <-> QEMU mapping code too.

I'd originally worried that without a specified 'regnum' in the
transmitted xml GDB would use its _internal_ numbering when talking to
QEMU, not the sequence number as its transmitted.  By using the
sequence number we make this a QEMU problem, not a GDB problem, and
that's fine; by which I mean, when QEMU changes its xml, it has to
change its own mapping, but a change in GDB doesn't impact QEMU.

Phew! Sorry for the long email, but I do understand xml target
descriptions more now :)

> 
> > > Jim: I think your comments above indicate you want my fix merged, but
> > > if you could just confirm then I'll get it merged.
> 
> Yes, I think this is OK.  We can worry about qemu later.

OK, will merge shortly...

Thanks,
Andrew