[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture i
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml |
Date: |
Mon, 01 Sep 2014 12:25:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
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 <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.
>
> 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 <address@hidden>
>> Acked-by: Cornelia Huck <address@hidden>
>> Acked-by: Christian Borntraeger <address@hidden>
>> Signed-off-by: Jens Freimann <address@hidden>
>> Cc: Andrzej Zaborowski <address@hidden>
>> Cc: Peter Maydell <address@hidden>
>> Cc: Vassili Karpov (malc) <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(-)
>>
>> 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
Looks good to me and even got documented,
Reviewed-by: Andreas Färber <address@hidden>
Thanks,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg