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

[David Hildenbrand]

> On 6 October 2014 16:08, Cornelia Huck <address@hidden> wrote:
> > On Tue, 30 Sep 2014 17:23:47 +0200
> > 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*".
> >>
> >> 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.
> >>
> >> Signed-off-by: David Hildenbrand <address@hidden>
> >> Acked-by: Cornelia Huck <address@hidden>
> >> Acked-by: Christian Borntraeger <address@hidden>
> >> Signed-off-by: Jens Freimann <address@hidden>
> >> Reviewed-by: Andreas Färber <address@hidden>
> >> Cc: Andrzej Zaborowski <address@hidden>
> >> Cc: Peter Maydell <address@hidden>
> >> Cc: Vassili Karpov (malc) <address@hidden>
> >> CC: Edgar Iglesias <address@hidden>
> >> CC: Richard Henderson <address@hidden>
> >> ---
> >>  gdbstub.c                   | 19 ++++++++++++-------
> >>  include/qom/cpu.h           |  2 ++
> >>  target-arm/cpu64.c          |  1 +
> >>  target-ppc/translate_init.c |  2 ++
> >>  target-s390x/cpu.c          |  1 +
> >>  5 files changed, 18 insertions(+), 7 deletions(-)
> >>
> > I will send this with the next pile of s390x updates, unless someone on
> > cc: has any objections.
> 
> I'm still hoping for an answer about why this is setting
> the name for 64 bit ARM and not 32 bit ARM, and whether
> there are any cases which need to actually be able to set
> the architecture name in a more complicated name than
> simply a string. I raised those in the last lot of review
> and there doesn't seem to have been any answer.
> 
> thanks
> -- PMM
> 

Hi Peter,

actually the questions were addressed in the last review. Haven't received any
answer from you to my reply. Maybe some mails got lost in the system.

32bit arm:
-"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."
- Not included in this patch as Edgar provided a patch in the previous thread
  (that's why he is on cc) that can easily be adopted. I don't want to simply
  include his effort in my patch :) And we have to make sure that this name is
  the right one.

More complicated names:
- "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")."
- The same should apply for all architectures. So we can set (or even build)
  the proper string when also specifying the core xml file.

Do you have something in mind like your "powerpc:common" and "powerpc:e500"
example? To build the names based on some pattern?

I don't think that we can generalize the name format here (at least "aarch64"
makes me assume that :) ). I think it would be enough to set the complete
strings in the class init functions.

What do you think? Any suggestions?

Thanks!

David