On Thu, Jun 28, 2012 at 05:46:02PM +0100, Peter Maydell wrote: > 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...
It makes the dump-guest-memory command work for arm-softmmu. The resulting core dump can be analysed with a tool such as the crash utility. > > > 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? struct elf_prstatus from the Linux kernel's include/linux/elfcore.h. > > > + > > +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?) No idea. Cc'ing Wen, who added the APIs. > > > +{ > > + 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? CPSR and orig_r0. orig_r0 is not useful, but I think we can save the CPSR in there. > > > + 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? I'll use TARGET_WORDS_BIGENDIAN to check. Though it appears we don't have a armbe-softmmu? > > > + 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 ? The implementations are to make the dump-guest-memory command build. A full implementation would add support for the "-p" option which afaics is supposed to walk the page tables and dump only the pages which are mapped instead of the complete RAM. I personally have no need for this option, so they are only null implementations which result in an error if this option is used. The current config code keeps memory-mapping.c and memory-mapping-stub.c exclusive. I think we should be able to make some changes there to allow us to use memory-mapping-stub.c instead of this arch_memory_mapping.c.