From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43468) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlDYp-0000OT-5h for qemu-devel@nongnu.org; Sun, 01 Jul 2012 02:23:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SlDYn-00076Y-2c for qemu-devel@nongnu.org; Sun, 01 Jul 2012 02:23:06 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:58391) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlDYm-000764-Mj for qemu-devel@nongnu.org; Sun, 01 Jul 2012 02:23:04 -0400 Received: by pbbro12 with SMTP id ro12so6879552pbb.4 for ; Sat, 30 Jun 2012 23:23:01 -0700 (PDT) Sender: Rabin Vincent Date: Sun, 1 Jul 2012 11:52:45 +0530 From: Rabin Vincent Message-ID: <20120701062245.GA2741@latitude> References: <1340213303-596-1-git-send-email-rabin@rab.in> <1340213303-596-4-git-send-email-rabin@rab.in> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 4/4] target-arm: add minimal dump-guest-memory support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org, Andreas =?iso-8859-1?Q?F=E4rber?= On Thu, Jun 28, 2012 at 05:46:02PM +0100, Peter Maydell wrote: > On 20 June 2012 18:28, Rabin Vincent 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 > > --- > >  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.