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

2014-09-03 Thread David Hildenbrand
 [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 jf...@linux.vnet.ibm.com wrote:
  From: David Hildenbrand d...@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).

David




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

2014-09-03 Thread Edgar E. Iglesias
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 jf...@linux.vnet.ibm.com wrote:
   From: David Hildenbrand d...@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.igles...@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.igles...@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);
+}
 pstrcat(target_xml, sizeof(target_xml), /target);
 }
 return target_xml;

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

target-arm: Provide GDB arch names

Signed-off-by: Edgar E. Iglesias edgar.igles...@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.igles...@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.igles...@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.igles...@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.igles...@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 

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

2014-09-03 Thread David Hildenbrand
 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 jf...@linux.vnet.ibm.com wrote:
From: David Hildenbrand d...@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.igles...@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.igles...@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;
 




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

2014-09-03 Thread 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 jf...@linux.vnet.ibm.com wrote:
 From: David Hildenbrand d...@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.igles...@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.igles...@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;
  
 



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

2014-09-01 Thread Peter Maydell
[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 jf...@linux.vnet.ibm.com wrote:
 From: David Hildenbrand d...@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.

Are there architectures that might need to specify something
more complicated than always the same string? (ie is there
a case for having the target provide a return architecture name
method rather than a constant string?)

 Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com
 Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
 Acked-by: Christian Borntraeger borntrae...@de.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 Cc: Andrzej Zaborowski balr...@gmail.com
 Cc: Peter Maydell peter.mayd...@linaro.org
 Cc: Vassili Karpov (malc) av1...@comtv.ru
 ---
  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(-)

 diff --git a/gdbstub.c b/gdbstub.c
 index 8afe0b7..af82259 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -523,13 +523,18 @@ static const char *get_feature_xml(const char *p, const 
 char **newp,
  GDBRegisterState *r;
  CPUState *cpu = first_cpu;

 -snprintf(target_xml, sizeof(target_xml),
 - ?xml version=\1.0\?
 - !DOCTYPE target SYSTEM \gdb-target.dtd\
 - target
 - xi:include href=\%s\/,
 - cc-gdb_core_xml_file);
 -
 +pstrcat(target_xml, sizeof(target_xml),
 +?xml version=\1.0\?
 +!DOCTYPE target SYSTEM \gdb-target.dtd\
 +target);
 +if (cc-gdb_arch_name) {
 +pstrcat(target_xml, sizeof(target_xml), architecture);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch_name);
 +pstrcat(target_xml, sizeof(target_xml), /architecture);
 +}
 +pstrcat(target_xml, sizeof(target_xml), xi:include href=\);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_core_xml_file);
 +pstrcat(target_xml, sizeof(target_xml), \/);
  for (r = cpu-gdb_regs; r; r = r-next) {
  pstrcat(target_xml, sizeof(target_xml), xi:include 
 href=\);
  pstrcat(target_xml, sizeof(target_xml), r-xml);
 diff --git a/include/qom/cpu.h b/include/qom/cpu.h
 index 1aafbf5..8828b16 100644
 --- a/include/qom/cpu.h
 +++ b/include/qom/cpu.h
 @@ -98,6 +98,7 @@ struct TranslationBlock;
   * @vmsd: State description for migration.
   * @gdb_num_core_regs: Number of core registers accessible to GDB.
   * @gdb_core_xml_file: File name for core registers GDB XML description.
 + * @gdb_arch_name: Architecture name known to GDB.
   *
   * Represents a CPU family or model.
   */
 @@ -147,6 +148,7 @@ typedef struct CPUClass {
  const struct VMStateDescription *vmsd;
  int gdb_num_core_regs;
  const char *gdb_core_xml_file;
 +const char *gdb_arch_name;
  } CPUClass;

  #ifdef HOST_WORDS_BIGENDIAN
 diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
 index 38d2b84..9df7492 100644
 --- a/target-arm/cpu64.c
 +++ b/target-arm/cpu64.c
 @@ -201,6 +201,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_name = aarch64;
  }

  static void aarch64_cpu_register(const ARMCPUInfo *info)
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 48177ed..7165347 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -9649,8 +9649,10 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
 *data)

  #if defined(TARGET_PPC64)
  cc-gdb_core_xml_file = power64-core.xml;
 +cc-gdb_arch_name = powerpc:common64;
  #else
  cc-gdb_core_xml_file = power-core.xml;
 +cc-gdb_arch_name = powerpc:common;
  #endif
  #ifndef CONFIG_USER_ONLY
  cc-virtio_is_big_endian = ppc_cpu_is_big_endian;
 diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
 index 4b03e42..5dae93c 100644
 --- a/target-s390x/cpu.c
 +++ b/target-s390x/cpu.c
 @@ -262,6 

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

2014-09-01 Thread Andreas Färber
Am 01.09.2014 12:19, schrieb Peter Maydell:
 [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 jf...@linux.vnet.ibm.com wrote:
 From: David Hildenbrand d...@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.
 
 Are there architectures that might need to specify something
 more complicated than always the same string? (ie is there
 a case for having the target provide a return architecture name
 method rather than a constant string?)
 
 Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com
 Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
 Acked-by: Christian Borntraeger borntrae...@de.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 Cc: Andrzej Zaborowski balr...@gmail.com
 Cc: Peter Maydell peter.mayd...@linaro.org
 Cc: Vassili Karpov (malc) av1...@comtv.ru
 ---
  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(-)

 diff --git a/gdbstub.c b/gdbstub.c
 index 8afe0b7..af82259 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -523,13 +523,18 @@ static const char *get_feature_xml(const char *p, 
 const char **newp,
  GDBRegisterState *r;
  CPUState *cpu = first_cpu;

 -snprintf(target_xml, sizeof(target_xml),
 - ?xml version=\1.0\?
 - !DOCTYPE target SYSTEM \gdb-target.dtd\
 - target
 - xi:include href=\%s\/,
 - cc-gdb_core_xml_file);
 -
 +pstrcat(target_xml, sizeof(target_xml),
 +?xml version=\1.0\?
 +!DOCTYPE target SYSTEM \gdb-target.dtd\
 +target);
 +if (cc-gdb_arch_name) {
 +pstrcat(target_xml, sizeof(target_xml), architecture);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch_name);
 +pstrcat(target_xml, sizeof(target_xml), /architecture);
 +}
 +pstrcat(target_xml, sizeof(target_xml), xi:include href=\);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_core_xml_file);
 +pstrcat(target_xml, sizeof(target_xml), \/);
  for (r = cpu-gdb_regs; r; r = r-next) {
  pstrcat(target_xml, sizeof(target_xml), xi:include 
 href=\);
  pstrcat(target_xml, sizeof(target_xml), r-xml);
 diff --git a/include/qom/cpu.h b/include/qom/cpu.h
 index 1aafbf5..8828b16 100644
 --- a/include/qom/cpu.h
 +++ b/include/qom/cpu.h
 @@ -98,6 +98,7 @@ struct TranslationBlock;
   * @vmsd: State description for migration.
   * @gdb_num_core_regs: Number of core registers accessible to GDB.
   * @gdb_core_xml_file: File name for core registers GDB XML description.
 + * @gdb_arch_name: Architecture name known to GDB.
   *
   * Represents a CPU family or model.
   */
 @@ -147,6 +148,7 @@ typedef struct CPUClass {
  const struct VMStateDescription *vmsd;
  int gdb_num_core_regs;
  const char *gdb_core_xml_file;
 +const char *gdb_arch_name;
  } CPUClass;

  #ifdef HOST_WORDS_BIGENDIAN
 diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
 index 38d2b84..9df7492 100644
 --- a/target-arm/cpu64.c
 +++ b/target-arm/cpu64.c
 @@ -201,6 +201,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_name = aarch64;
  }

  static void aarch64_cpu_register(const ARMCPUInfo *info)
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 48177ed..7165347 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -9649,8 +9649,10 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
 *data)

  #if defined(TARGET_PPC64)
  cc-gdb_core_xml_file = power64-core.xml;
 +cc-gdb_arch_name = powerpc:common64;
  #else
  cc-gdb_core_xml_file = power-core.xml;
 +cc-gdb_arch_name = powerpc:common;
  #endif
  #ifndef CONFIG_USER_ONLY
  cc-virtio_is_big_endian = ppc_cpu_is_big_endian;
 diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
 index 4b03e42..5dae93c 100644
 --- 

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

2014-09-01 Thread Christian Borntraeger
On 01/09/14 12:19, Peter Maydell 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 jf...@linux.vnet.ibm.com wrote:
 From: David Hildenbrand d...@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.
 
 Are there architectures that might need to specify something
 more complicated than always the same string? (ie is there
 a case for having the target provide a return architecture name
 method rather than a constant string?)

I dont know and David is on vacation.
Would it make sense to do the other architectures as addon patches or shall we 
wait with this patch until we know what is necessary for all supported 
platforms?


 
 Signed-off-by: David Hildenbrand d...@linux.vnet.ibm.com
 Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
 Acked-by: Christian Borntraeger borntrae...@de.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 Cc: Andrzej Zaborowski balr...@gmail.com
 Cc: Peter Maydell peter.mayd...@linaro.org
 Cc: Vassili Karpov (malc) av1...@comtv.ru
 ---
  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(-)

 diff --git a/gdbstub.c b/gdbstub.c
 index 8afe0b7..af82259 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -523,13 +523,18 @@ static const char *get_feature_xml(const char *p, 
 const char **newp,
  GDBRegisterState *r;
  CPUState *cpu = first_cpu;

 -snprintf(target_xml, sizeof(target_xml),
 - ?xml version=\1.0\?
 - !DOCTYPE target SYSTEM \gdb-target.dtd\
 - target
 - xi:include href=\%s\/,
 - cc-gdb_core_xml_file);
 -
 +pstrcat(target_xml, sizeof(target_xml),
 +?xml version=\1.0\?
 +!DOCTYPE target SYSTEM \gdb-target.dtd\
 +target);
 +if (cc-gdb_arch_name) {
 +pstrcat(target_xml, sizeof(target_xml), architecture);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch_name);
 +pstrcat(target_xml, sizeof(target_xml), /architecture);
 +}
 +pstrcat(target_xml, sizeof(target_xml), xi:include href=\);
 +pstrcat(target_xml, sizeof(target_xml), cc-gdb_core_xml_file);
 +pstrcat(target_xml, sizeof(target_xml), \/);
  for (r = cpu-gdb_regs; r; r = r-next) {
  pstrcat(target_xml, sizeof(target_xml), xi:include 
 href=\);
  pstrcat(target_xml, sizeof(target_xml), r-xml);
 diff --git a/include/qom/cpu.h b/include/qom/cpu.h
 index 1aafbf5..8828b16 100644
 --- a/include/qom/cpu.h
 +++ b/include/qom/cpu.h
 @@ -98,6 +98,7 @@ struct TranslationBlock;
   * @vmsd: State description for migration.
   * @gdb_num_core_regs: Number of core registers accessible to GDB.
   * @gdb_core_xml_file: File name for core registers GDB XML description.
 + * @gdb_arch_name: Architecture name known to GDB.
   *
   * Represents a CPU family or model.
   */
 @@ -147,6 +148,7 @@ typedef struct CPUClass {
  const struct VMStateDescription *vmsd;
  int gdb_num_core_regs;
  const char *gdb_core_xml_file;
 +const char *gdb_arch_name;
  } CPUClass;

  #ifdef HOST_WORDS_BIGENDIAN
 diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
 index 38d2b84..9df7492 100644
 --- a/target-arm/cpu64.c
 +++ b/target-arm/cpu64.c
 @@ -201,6 +201,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_name = aarch64;
  }

  static void aarch64_cpu_register(const ARMCPUInfo *info)
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 48177ed..7165347 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -9649,8 +9649,10 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
 *data)

  #if defined(TARGET_PPC64)
  cc-gdb_core_xml_file = power64-core.xml;
 +cc-gdb_arch_name = powerpc:common64;
  #else
  cc-gdb_core_xml_file = power-core.xml;
 +cc-gdb_arch_name = 

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

2014-09-01 Thread Peter Maydell
On 1 September 2014 11:31, Christian Borntraeger borntrae...@de.ibm.com wrote:
 On 01/09/14 12:19, Peter Maydell 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 jf...@linux.vnet.ibm.com wrote:
 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.

 Are there architectures that might need to specify something
 more complicated than always the same string? (ie is there
 a case for having the target provide a return architecture name
 method rather than a constant string?)

 I dont know and David is on vacation.
 Would it make sense to do the other architectures as addon patches
 or shall we wait with this patch until we know what is necessary for
 all supported platforms?

We don't have to implement all the architectures at once, but
I would prefer it if we can get the QOM API for it right from
the start rather than having to change it (and all the targets
which implemented it the old way) later.

thanks
-- PMM



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

2014-08-29 Thread Jens Freimann
From: David Hildenbrand d...@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*.

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 d...@linux.vnet.ibm.com
Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
Acked-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
Cc: Andrzej Zaborowski balr...@gmail.com
Cc: Peter Maydell peter.mayd...@linaro.org
Cc: Vassili Karpov (malc) av1...@comtv.ru
---
 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(-)

diff --git a/gdbstub.c b/gdbstub.c
index 8afe0b7..af82259 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -523,13 +523,18 @@ static const char *get_feature_xml(const char *p, const 
char **newp,
 GDBRegisterState *r;
 CPUState *cpu = first_cpu;
 
-snprintf(target_xml, sizeof(target_xml),
- ?xml version=\1.0\?
- !DOCTYPE target SYSTEM \gdb-target.dtd\
- target
- xi:include href=\%s\/,
- cc-gdb_core_xml_file);
-
+pstrcat(target_xml, sizeof(target_xml),
+?xml version=\1.0\?
+!DOCTYPE target SYSTEM \gdb-target.dtd\
+target);
+if (cc-gdb_arch_name) {
+pstrcat(target_xml, sizeof(target_xml), architecture);
+pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch_name);
+pstrcat(target_xml, sizeof(target_xml), /architecture);
+}
+pstrcat(target_xml, sizeof(target_xml), xi:include href=\);
+pstrcat(target_xml, sizeof(target_xml), cc-gdb_core_xml_file);
+pstrcat(target_xml, sizeof(target_xml), \/);
 for (r = cpu-gdb_regs; r; r = r-next) {
 pstrcat(target_xml, sizeof(target_xml), xi:include href=\);
 pstrcat(target_xml, sizeof(target_xml), r-xml);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1aafbf5..8828b16 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -98,6 +98,7 @@ struct TranslationBlock;
  * @vmsd: State description for migration.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
  * @gdb_core_xml_file: File name for core registers GDB XML description.
+ * @gdb_arch_name: Architecture name known to GDB.
  *
  * Represents a CPU family or model.
  */
@@ -147,6 +148,7 @@ typedef struct CPUClass {
 const struct VMStateDescription *vmsd;
 int gdb_num_core_regs;
 const char *gdb_core_xml_file;
+const char *gdb_arch_name;
 } CPUClass;
 
 #ifdef HOST_WORDS_BIGENDIAN
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 38d2b84..9df7492 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -201,6 +201,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_name = aarch64;
 }
 
 static void aarch64_cpu_register(const ARMCPUInfo *info)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 48177ed..7165347 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9649,8 +9649,10 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
*data)
 
 #if defined(TARGET_PPC64)
 cc-gdb_core_xml_file = power64-core.xml;
+cc-gdb_arch_name = powerpc:common64;
 #else
 cc-gdb_core_xml_file = power-core.xml;
+cc-gdb_arch_name = powerpc:common;
 #endif
 #ifndef CONFIG_USER_ONLY
 cc-virtio_is_big_endian = ppc_cpu_is_big_endian;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 4b03e42..5dae93c 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -262,6 +262,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 dc-vmsd = vmstate_s390_cpu;
 cc-gdb_num_core_regs = S390_NUM_CORE_REGS;
 cc-gdb_core_xml_file = s390x-core64.xml;
+cc-gdb_arch_name = s390:64-bit;
 }
 
 static const TypeInfo s390_cpu_type_info = {
-- 
1.8.5.5