[RFC] Add xmlRegisters qSupported on ppc-lynx...

[RFC] Add xmlRegisters qSupported on ppc-lynx...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

Hi Pedro,

I was wondering if you good give some feedback on something I am trying
to do.  Basically, I am working on gdbserver for powerpc-lynx178.
Version 178 of LynxOS is based on an older version of LynxOS where XCOFF
is still in use.  The object format is actually identical to the one
used on AIX.

The problem is that GDB and GDBserver use different register numberings.
On the target side, the register number is the same as the ones we see
on versions 4.x and 5.x. But on the GDB side, because the objfect format
is the same as on AIX, the architecture ends up being set to
"rs6000:6000" (instead of "powerpc:common"). As a result, there are
a few discrepancies in terms of a couple of registers ("fpscr" is the
one that attracted my attention).

So, my idea was to have GDBserver tell GDB what the number is.
Unlike what I am seeing on x86 targets, I don't think we need
anything fancy because, for now, the register map is completely
static. So we don't need to configure the register map besides
setting it at initialization (init_registers_powerpc_32).

The first thing I did was update rs6000-tdep.c so that GDB would
ask for xmlRegister support. And to my surprise, that was almost
enough to get things working. All I had to do next was to update
configure.tgt to include the appropriate XML files, and things
started working. I thought I would have to confirm support for
xmlRegister first, but it looks like it's not the case.

Can you tell me what you think of the following patch? Am I going
to need to do something a little more elaborate, for instance?
I am also worrying about breaking other power targets, but I hope
not. Hopefully GDB sends a qXfer:features:read:target.xml packet,
gets an error, and falls back to the default architecture...

Thanks!
-- 
Joel

[-- Attachment #2: 0001-gdbserver-lynx-ppc-xml.diff --]
[-- Type: text/x-diff, Size: 1399 bytes --]

diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index d037c8e..fd26caf 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -230,6 +230,9 @@ case "${target}" in
 			;;
   powerpc-*-lynxos*)	srv_regobj="powerpc-32.o"
 			srv_tgtobj="lynx-low.o lynx-ppc-low.o"
+                        srv_xmlfiles="rs6000/powerpc-32.xml"
+                        srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
+                        srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
 			srv_lynxos=yes
 			;;
   s390*-*-linux*)	srv_regobj="s390-linux32.o"
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index f848700..2252f54 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -79,6 +79,7 @@
 #include "features/rs6000/powerpc-860.c"
 #include "features/rs6000/powerpc-e500.c"
 #include "features/rs6000/rs6000.c"
+#include "remote.h"
 
 /* Determine if regnum is an SPE pseudo-register.  */
 #define IS_SPE_PSEUDOREG(tdep, regnum) ((tdep)->ppc_ev0_regnum >= 0 \
@@ -4284,6 +4285,9 @@ _initialize_rs6000_tdep (void)
   initialize_tdesc_powerpc_e500 ();
   initialize_tdesc_rs6000 ();
 
+  /* Tell remote stub that we support XML target description.  */
+  register_remote_support_xml ("powerpc");
+
   /* Add root prefix command for all "set powerpc"/"show powerpc"
      commands.  */
   add_prefix_cmd ("powerpc", no_class, set_powerpc_command,

Re: [RFC] Add xmlRegisters qSupported on ppc-lynx...

[Pedro Alves]

On 06/05/2012 06:03 PM, Joel Brobecker wrote:

> Hi Pedro,
> 
> I was wondering if you good give some feedback on something I am trying
> to do.  Basically, I am working on gdbserver for powerpc-lynx178.
> Version 178 of LynxOS is based on an older version of LynxOS where XCOFF
> is still in use.  The object format is actually identical to the one
> used on AIX.
> 
> The problem is that GDB and GDBserver use different register numberings.
> On the target side, the register number is the same as the ones we see
> on versions 4.x and 5.x. But on the GDB side, because the objfect format
> is the same as on AIX, the architecture ends up being set to
> "rs6000:6000" (instead of "powerpc:common"). As a result, there are
> a few discrepancies in terms of a couple of registers ("fpscr" is the
> one that attracted my attention).
> 
> So, my idea was to have GDBserver tell GDB what the number is.
> Unlike what I am seeing on x86 targets, I don't think we need
> anything fancy because, for now, the register map is completely
> static. So we don't need to configure the register map besides
> setting it at initialization (init_registers_powerpc_32).
> 
> The first thing I did was update rs6000-tdep.c so that GDB would
> ask for xmlRegister support. And to my surprise, that was almost
> enough to get things working. 


I'm very surprised that changed anything at all.  I actually think it didn't.

> All I had to do next was to update
> configure.tgt to include the appropriate XML files, and things
> started working. I thought I would have to confirm support for
> xmlRegister first, but it looks like it's not the case.


It's harmless, but you don't need this.  XML target descriptions
work without it.  Witness that PowerPC gdbserver works with XML target
descriptions nowadays (and was in fact the first port to use them).
Use of that feature in qSupported for a given architecture is actually useless
if it isn't done at the same time GDB actually gains support for xml target
descriptions for that given architecture.  XML target descriptions are supported
today even without GDB broadcasting support with xmlRegister first, so even if
xmlRegister isn't sent in qSupported, the remote can't know whether GDB supports
XML descriptions or not.  The xmlRegister packet was added for x86, at the same time
XML support for x86 was added, because gdbserver needed to behave different depending
on whether GDB knows or not about x86 XML descriptions.  On the gdbserver side,
the xmlRegister qSupported feature is handled by the backend, not common code
(see x86_linux_process_qsupported).  So if you haven't added similar
backend code, this is having absolutely no effect, other than seeing the
xmlRegister feature reported by the qSupported packet.

> 
> Can you tell me what you think of the following patch? Am I going
> to need to do something a little more elaborate, for instance?


You just need the configure.srv change, actually.

The powerpc-32.o file, built from the generated powerpc-32.c file,
which is in turn built from regformats/rs6000/powerpc-32.dat
is already pointing the target description at powerpc-32.xml.
But since that xml file and its includes weren't actually being
included in the build (with srv_xmlfiles, as you're now doing), gdbserver
was ending up returning no description (see get_features_xml).  So the
simply fix is to just list the needed xml files, like you did.

> I am also worrying about breaking other power targets, but I hope
> not. Hopefully GDB sends a qXfer:features:read:target.xml packet,

> gets an error, and falls back to the default architecture...


The register_remote_support_xml call has zero influence on this.

> 
> Thanks!
> -- Joel
> 
> 
> 0001-gdbserver-lynx-ppc-xml.diff
> 
> 
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index d037c8e..fd26caf 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -230,6 +230,9 @@ case "${target}" in
>  			;;
>    powerpc-*-lynxos*)	srv_regobj="powerpc-32.o"
>  			srv_tgtobj="lynx-low.o lynx-ppc-low.o"
> +                        srv_xmlfiles="rs6000/powerpc-32.xml"
> +                        srv_xmlfiles="${srv_xmlfiles} rs6000/power-core.xml"
> +                        srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
>  			srv_lynxos=yes
>  			;;
>    s390*-*-linux*)	srv_regobj="s390-linux32.o"


-- 
Pedro Alves

Re: [RFC] Add xmlRegisters qSupported on ppc-lynx...

[Joel Brobecker]

> XML target descriptions are supported today even without GDB
> broadcasting support with xmlRegister first, so even if xmlRegister
> isn't sent in qSupported, the remote can't know whether GDB supports
> XML descriptions or not.

Ah, OK! This is really nice. I missed the query packet in the
unmodified version when I first started looking at this issue.

> You just need the configure.srv change, actually.

Thanks for the detailed feedback. I will test that, and send a patch.

-- 
Joel