diff mbox

[5/5] gdb: provide the name of the architecture in the target.xml

Message ID 20140903094540.GA16778@toto
State New
Headers show

Commit Message

Edgar E. Iglesias Sept. 3, 2014, 9:45 a.m. UTC
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 <jfrei@linux.vnet.ibm.com> wrote:
> > > From: David Hildenbrand <dahi@linux.vnet.ibm.com>
> > >
> > > 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:


commit 26932a453da466d111b67c37b93dec71fb3ae111
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
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 <edgar.iglesias@xilinx.com>

Comments

David Hildenbrand Sept. 3, 2014, 9:59 a.m. UTC | #1
> 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 <jfrei@linux.vnet.ibm.com> wrote:
> > > > From: David Hildenbrand <dahi@linux.vnet.ibm.com>
> > > >
> > > > 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 <edgar.iglesias@xilinx.com>
> 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 <edgar.iglesias@xilinx.com>
> 
> 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.

David

>              pstrcat(target_xml, sizeof(target_xml), "</target>");
>          }
>          return target_xml;
>
Edgar E. Iglesias Sept. 3, 2014, 10 a.m. UTC | #2
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 <jfrei@linux.vnet.ibm.com> wrote:
> > > > > From: David Hildenbrand <dahi@linux.vnet.ibm.com>
> > > > >
> > > > > 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 <edgar.iglesias@xilinx.com>
> > 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 <edgar.iglesias@xilinx.com>
> > 
> > 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;
> > 
>
diff mbox

Patch

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>");
+            }
             pstrcat(target_xml, sizeof(target_xml), "</target>");
         }
         return target_xml;

commit a0b166c59491b154c05b963649f561c1e48aa706
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Date:   Wed Aug 20 19:21:28 2014 +1000

    target-arm: Provide GDB arch names
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index b726b6a..d1704f5 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1163,6 +1163,7 @@  static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->get_phys_page_debug = arm_cpu_get_phys_page_debug;
     cc->vmsd = &vmstate_arm_cpu;
 #endif
+    cc->gdb_arch = "arm";
     cc->gdb_num_core_regs = 26;
     cc->gdb_core_xml_file = "arm-core.xml";
 }
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index fc9c991..c099d13 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -215,6 +215,7 @@  static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_write_register = aarch64_cpu_gdb_write_register;
     cc->gdb_num_core_regs = 34;
     cc->gdb_core_xml_file = "aarch64-core.xml";
+    cc->gdb_arch = "aarch64";
 }
 
 static void aarch64_cpu_register(const ARMCPUInfo *info)

commit 0aca15f7829e6e8a94639ddd75bcfdfd3b034d2e
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Date:   Wed Aug 20 19:20:35 2014 +1000

    qom/cpu: Add a gdb_arch field to the CPUClass
    
    Used to optionally set the GDB architecture of the CPU.
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 4d0908b..46b72e7 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -147,6 +147,7 @@  typedef struct CPUClass {
                                 void *opaque);
 
     const struct VMStateDescription *vmsd;
+    const char *gdb_arch;
     int gdb_num_core_regs;
     const char *gdb_core_xml_file;
 } CPUClass;

commit eaa8bac08e300c9516d04c3425c3794a1bd893b8
Author: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Date:   Wed Aug 20 11:26:52 2014 +1000

    configure: gdbstub: Include ARM xml descritions in AArch64 builds
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

diff --git a/configure b/configure
index fce5801..3a95948 100755
--- a/configure
+++ b/configure
@@ -4976,7 +4976,9 @@  case "$target_name" in
   aarch64)
     TARGET_BASE_ARCH=arm
     bflt="yes"
-    gdb_xml_files="aarch64-core.xml aarch64-fpu.xml"
+    arm_gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
+    gdb_xml_files="aarch64-core.xml aarch64-fpu.xml $arm_gdb_xml_files"
+    echo $gdb_xml_files
   ;;
   cris)
   ;;