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

[Christian Borntraeger <borntraeger@de.ibm.com>]

Ping?

On 12/03/2015 01:14 PM, Christian Borntraeger 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*".
> 
> The name of the architecture is provided by a callback that can
> be implemented by all architectures. The arm implementation has
> special handling for iwmmxt and returns arm otherwise. This can
> be extended if necessary.
> 
> Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> [rework to use a callback]
> ---
> v1->v2: replace the fixed string with a callback
> 
>  gdbstub.c                   | 21 ++++++++++++++-------
>  include/qom/cpu.h           |  3 +++
>  target-arm/cpu.c            | 12 ++++++++++++
>  target-arm/cpu64.c          |  6 ++++++
>  target-ppc/translate_init.c | 10 ++++++++++
>  target-s390x/cpu.c          |  6 ++++++
>  6 files changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 9c29aa0..6a674b2 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -540,13 +540,20 @@ 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) {
> +                gchar *arch = cc->gdb_arch_name(cpu);
> +                pstrcat(target_xml, sizeof(target_xml), "<architecture>");
> +                pstrcat(target_xml, sizeof(target_xml), arch);
> +                pstrcat(target_xml, sizeof(target_xml), "</architecture>");
> +		g_free(arch);
> +            }
> +            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 51a1323..c3c2812 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -114,6 +114,8 @@ struct TranslationBlock;
>   * @gdb_core_xml_file: File name for core registers GDB XML description.
>   * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>   *           before the insn which triggers a watchpoint rather than after it.
> + * @gdb_arch_name: Optional callback that returns the architecture name known
> + * to GDB. The caller must free the returned string with g_free.
>   * @cpu_exec_enter: Callback for cpu_exec preparation.
>   * @cpu_exec_exit: Callback for cpu_exec cleanup.
>   * @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec.
> @@ -168,6 +170,7 @@ typedef struct CPUClass {
>      const struct VMStateDescription *vmsd;
>      int gdb_num_core_regs;
>      const char *gdb_core_xml_file;
> +    gchar * (*gdb_arch_name)(CPUState *cpu);
>      bool gdb_stop_before_watchpoint;
> 
>      void (*cpu_exec_enter)(CPUState *cpu);
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 30739fc..771cf91 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1393,6 +1393,17 @@ static int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw,
>  }
>  #endif
> 
> +static gchar *arm_gdb_arch_name(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> +        return g_strdup("iwmmxt");
> +    }
> +    return g_strdup("arm");
> +}
> +
>  static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> @@ -1423,6 +1434,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  #endif
>      cc->gdb_num_core_regs = 26;
>      cc->gdb_core_xml_file = "arm-core.xml";
> +    cc->gdb_arch_name = arm_gdb_arch_name;
>      cc->gdb_stop_before_watchpoint = true;
>      cc->debug_excp_handler = arm_debug_excp_handler;
> 
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..ad1991d 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -286,6 +286,11 @@ static void aarch64_cpu_set_pc(CPUState *cs, vaddr value)
>      }
>  }
> 
> +static gchar *aarch64_gdb_arch_name(CPUState *cs)
> +{
> +    return g_strdup("aarch64");
> +}
> +
>  static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
> @@ -299,6 +304,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_gdb_arch_name;
>  }
> 
>  static void aarch64_cpu_register(const ARMCPUInfo *info)
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index e88dc7f..e36d0a3 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9681,6 +9681,15 @@ static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
>      return pcc->pvr == pvr;
>  }
> 
> +static gchar *ppc_gdb_arch_name(CPUState *cs)
> +{
> +#if defined(TARGET_PPC64)
> +    return g_strdup("powerpc:common64");
> +#else
> +    return g_strdup("powerpc:common");
> +#endif
> +}
> +
>  static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> @@ -9725,6 +9734,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>      cc->gdb_num_core_regs = 71 + 32;
>  #endif
> 
> +    cc->gdb_arch_name = ppc_gdb_arch_name;
>  #if defined(TARGET_PPC64)
>      cc->gdb_core_xml_file = "power64-core.xml";
>  #else
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 48ecd34..69f914f 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -369,6 +369,11 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
>  }
>  #endif
> 
> +static gchar *s390_gdb_arch_name(CPUState *cs)
> +{
> +    return g_strdup("s390:64-bit");
> +}
> +
>  static void s390_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      S390CPUClass *scc = S390_CPU_CLASS(oc);
> @@ -405,6 +410,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
> 
>      cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
>      cc->gdb_core_xml_file = "s390x-core64.xml";
> +    cc->gdb_arch_name = s390_gdb_arch_name;
> 
>      /*
>       * Reason: s390_cpu_initfn() calls cpu_exec_init(), which saves
>