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 >> --- 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 <afaer...@suse.de> 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