Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml

[Edgar E. Iglesias]

On Wed, Sep 03, 2014 at 11:59:45AM +0200, David Hildenbrand wrote:
> > On Wed, Sep 03, 2014 at 11:37:24AM +0200, David Hildenbrand wrote:
> > > > [ccing Andreas in case he wants to review the QOM aspects of this,
> > > > though they're fairly straightforward I think.]
> > > > 
> > > > On 29 August 2014 14:52, Jens Freimann <address@hidden> wrote:
> > > > > From: David Hildenbrand <address@hidden>
> > > > >
> > > > > This patch provides the name of the architecture in the target.xml if 
> > > > > available.
> > > > >
> > > > > This allows the remote gdb to detect the target architecture on its 
> > > > > own - so
> > > > > there is no need to specify it manually (e.g. if gdb is started 
> > > > > without a
> > > > > binary) using "set arch *arch_name*".
> > > > 
> > > > This is neat; I didn't realise gdb let you do this.
> > > > 
> > > > > The name of the architecture has been added to all archs that provide 
> > > > > a
> > > > > target.xml (by supplying a gdb_core_xml_file) and have a unique 
> > > > > architecture
> > > > > name in gdb's feature xml files.
> > > > 
> > > > What about 32-bit ARM? You set the architecture name for AArch64
> > > > but not the 32 bit case.
> > > > 
> > > 
> > > Well, my point was to not break anything :)
> > > 
> > > On my way through the possible architecture names
> > > (binutils-gdb/gdb/features/*.xml), I wasn't able to come up with the 
> > > right name
> > > for arm 32 bit (arm-core.xml) - they don't specify any. This patch 
> > > therefore
> > > adapts to the xml files from gdb.
> > > 
> > > The architecture should be known at the same point when specifying the 
> > > xml file.
> > > So if anyone can come up with the proper arm name in the future (or even 
> > > some
> > > kind of detection algorithm), it can simply be set in target-arm/cpu.c 
> > > (after
> > > "arm-core.xml").
> > 
> > Hi,
> > 
> > I've got some similar patches in my tree. I used the following:
> > 
> 
> Thanks! So "arm" seems to be the proper name for arm32, right?
> 
> > 
> > commit 26932a453da466d111b67c37b93dec71fb3ae111
> > Author: Edgar E. Iglesias <address@hidden>
> > Date:   Wed Aug 20 19:22:10 2014 +1000
> > 
> >     gdbstub: Emit the CPUs GDB architecture if available
> >     
> >     Allows GDB to autodetect the architecture.
> >     
> >     Signed-off-by: Edgar E. Iglesias <address@hidden>
> > 
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 7f82186..5b62c50 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -604,6 +604,11 @@ static const char *get_feature_xml(const char *p, 
> > const char **newp,
> >                  pstrcat(target_xml, sizeof(target_xml), r->xml);
> >                  pstrcat(target_xml, sizeof(target_xml), "\"/>");
> >              }
> > +            if (cc->gdb_arch) {
> > +                pstrcat(target_xml, sizeof(target_xml), "<architecture>");
> > +                pstrcat(target_xml, sizeof(target_xml), cc->gdb_arch);
> > +                pstrcat(target_xml, sizeof(target_xml), "</architecture>");
> > +            }
> 
> Please not that "gdb-target.dtd" specifies the architecture to come directly 
> at
> the beginning of the target "section".
> 
> Putting it after the xml-includes, to the end of the target section makes the
> whole XML failing to be recognized on my tests with s390x.

Hmm, interesting. It worked here with a multi-arch gdb.

Cheers,
Edgar

> 
> David
> 
> >              pstrcat(target_xml, sizeof(target_xml), "</target>");
> >          }
> >          return target_xml;
> > 
>