[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] target-arm: add minimal dump-guest-memory s
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] target-arm: add minimal dump-guest-memory support |
Date: |
Thu, 28 Jun 2012 17:46:02 +0100 |
On 20 June 2012 18:28, Rabin Vincent <address@hidden> 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 <address@hidden>
> ---
> 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