On 20 June 2012 18:28, Rabin Vincent <ra...@rab.in> wrote: > Add a minimal dump-guest-memory support for ARM. The -p option is not > supported and we don't add any QEMU-specific notes.
So what does this patch give us? This commit message is pretty short and I couldn't find a cover message for the patchset... > Signed-off-by: Rabin Vincent <ra...@rab.in> > --- > configure | 4 +-- > target-arm/Makefile.objs | 2 +- > target-arm/arch_dump.c | 59 > ++++++++++++++++++++++++++++++++++++++ > target-arm/arch_memory_mapping.c | 13 +++++++++ > 4 files changed, 75 insertions(+), 3 deletions(-) > create mode 100644 target-arm/arch_dump.c > create mode 100644 target-arm/arch_memory_mapping.c > > diff --git a/configure b/configure > index b68c0ca..a20ad19 100755 > --- a/configure > +++ b/configure > @@ -3727,7 +3727,7 @@ case "$target_arch2" in > fi > esac > case "$target_arch2" in > - i386|x86_64) > + arm|i386|x86_64) > echo "CONFIG_HAVE_GET_MEMORY_MAPPING=y" >> $config_target_mak > esac > if test "$target_arch2" = "ppc64" -a "$fdt" = "yes"; then > @@ -3746,7 +3746,7 @@ if test "$target_softmmu" = "yes" ; then > echo "subdir-$target: subdir-libcacard" >> $config_host_mak > fi > case "$target_arch2" in > - i386|x86_64) > + arm|i386|x86_64) > echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak > esac > fi > diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs > index f447c4f..837b374 100644 > --- a/target-arm/Makefile.objs > +++ b/target-arm/Makefile.objs > @@ -1,5 +1,5 @@ > obj-y += arm-semi.o > -obj-$(CONFIG_SOFTMMU) += machine.o > +obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o > obj-y += translate.o op_helper.o helper.o cpu.o > obj-y += neon_helper.o iwmmxt_helper.o > > diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c > new file mode 100644 > index 0000000..47a7e40 > --- /dev/null > +++ b/target-arm/arch_dump.c > @@ -0,0 +1,59 @@ > +#include "cpu.h" > +#include "cpu-all.h" > +#include "dump.h" > +#include "elf.h" > + > +typedef struct { > + char pad1[24]; > + uint32_t pid; > + char pad2[44]; > + uint32_t regs[18]; > + char pad3[4]; > +} arm_elf_prstatus; I'm guessing this is following some specification's structure layout; what specification? > + > +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env, > + int cpuid, void *opaque) Should these APIs really be taking a CPUArchState* rather rather than an ARMCPU* ? (Andreas?) > +{ > + return -1; > +} > + > +int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env, > + int cpuid, void *opaque) > +{ > + arm_elf_prstatus prstatus; > + > + memset(&prstatus, 0, sizeof(prstatus)); > + memcpy(&(prstatus.regs), env->regs, sizeof(env->regs)); This looks a bit odd -- env->regs[] is a 16 word array but prstatus.regs is 18 words. What are the last two words for? > + prstatus.pid = cpuid; > + > + return dump_write_elf_note(ELFCLASS32, "CORE", NT_PRSTATUS, > + &prstatus, sizeof(prstatus), > + f, opaque); > +} > + > +int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env, > + void *opaque) > +{ > + return -1; > +} > + > +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env, > + void *opaque) > +{ > + return 0; > +} > + > +int cpu_get_dump_info(ArchDumpInfo *info) > +{ > + info->d_machine = EM_ARM; > + info->d_endian = ELFDATA2LSB; ...even for big endian ARM? > + info->d_class = ELFCLASS32; > + > + return 0; > +} > + > +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus) > +{ > + return nr_cpus * dump_get_note_size(ELFCLASS32, "CORE", > + sizeof(arm_elf_prstatus)); > +} > diff --git a/target-arm/arch_memory_mapping.c > b/target-arm/arch_memory_mapping.c > new file mode 100644 > index 0000000..eeaaf09 > --- /dev/null > +++ b/target-arm/arch_memory_mapping.c > @@ -0,0 +1,13 @@ > +#include "cpu.h" > +#include "cpu-all.h" > +#include "memory_mapping.h" > + > +bool cpu_paging_enabled(CPUArchState *env) > +{ > + return 0; > +} > + > +int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env) > +{ > + return -1; > +} Why do we need these null implementations and why do they work better than the default ones in memory_mapping-stub.c ? (memory_mapping.h could use some documentation comments describing the purpose and API of these functions IMHO.) -- PMM