[Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

[Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

[Rabin Vincent]

A second patchset to add dump-guest-memory support for ARM.

This version of the patchset addresses the following comments from the
previous posting, and also adds some more patches to the core dump code
(patch #4 and #6):

 - memset prstatus to 0 in x86_64_write_elf64_note()
 - handle big endian in dump_write_elf_note()
 - Save CPSR in ARM prstatus
 - set correct ELF endianness for ARM BE 

Rabin Vincent (6):
  dump: create writable files
  dump: extract out note helper
  dump: extract out get note size function
  dump: fix up memory mapping dependencies / stub
  target-arm: add dump-guest-memory support
  dump: fix memory region handling

 Makefile.target          |    3 +-
 configure                |    2 +-
 dump.c                   |   88 ++++++++++++++++--
 include/exec/memory.h    |    7 ++
 include/sysemu/dump.h    |    6 ++
 memory.c                 |   12 +++
 memory_mapping-stub.c    |    5 --
 memory_mapping.c         |    6 +-
 target-arm/Makefile.objs |    2 +-
 target-arm/arch_dump.c   |   61 +++++++++++++
 target-i386/arch_dump.c  |  222 +++++++++++-----------------------------------
 11 files changed, 223 insertions(+), 191 deletions(-)
 create mode 100644 target-arm/arch_dump.c

-- 
1.7.10.4

[Qemu-devel] [PATCHv2 1/6] dump: create writable files

[Rabin Vincent]

The files dump-guest-memory are created as read-only even for the owner.
This non-standard behaviour makes it annoying to deal with the dump
files (eg. rm -f is needed to delete them or saving a new dump by
overwriting the previous one is not possible).  Change the code to
generate files with write permissions set.  If someone requires
read-only files to be created, they can achieve it by setting umask.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 dump.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dump.c b/dump.c
index a25f509..8dd86b4 100644
--- a/dump.c
+++ b/dump.c
@@ -841,7 +841,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
 #endif
 
     if  (strstart(file, "file:", &p)) {
-        fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
+        fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+                       S_IRUSR | S_IWUSR);
         if (fd < 0) {
             error_set(errp, QERR_OPEN_FILE_FAILED, p);
             return;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv2 1/6] dump: create writable files

[Paolo Bonzini]

Il 24/03/2013 18:27, Rabin Vincent ha scritto:
> The files dump-guest-memory are created as read-only even for the owner.
> This non-standard behaviour makes it annoying to deal with the dump
> files (eg. rm -f is needed to delete them or saving a new dump by
> overwriting the previous one is not possible).  Change the code to
> generate files with write permissions set.  If someone requires
> read-only files to be created, they can achieve it by setting umask.
> 
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  dump.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/dump.c b/dump.c
> index a25f509..8dd86b4 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -841,7 +841,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
>  #endif
>  
>      if  (strstart(file, "file:", &p)) {
> -        fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
> +        fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> +                       S_IRUSR | S_IWUSR);
>          if (fd < 0) {
>              error_set(errp, QERR_OPEN_FILE_FAILED, p);
>              return;
> 

Rabim, I think you should resend this patch separately.  Cc
qemu-trivial@nongnu.org and qemu-stable@nongnu.org, please.

Paolo

[Qemu-devel] [PATCHv2 2/6] dump: extract out note helper

[Rabin Vincent]

Make a common helper function out of the x86 code to add ELF notes.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 dump.c                  |   51 ++++++++++++
 include/sysemu/dump.h   |    4 +
 target-i386/arch_dump.c |  208 +++++++++++------------------------------------
 3 files changed, 104 insertions(+), 159 deletions(-)

diff --git a/dump.c b/dump.c
index 8dd86b4..c5e009a 100644
--- a/dump.c
+++ b/dump.c
@@ -465,6 +465,57 @@ static hwaddr get_offset(hwaddr phys_addr,
     return -1;
 }
 
+int dump_write_elf_note(int class, const char *name, uint32_t type,
+                        void *desc, size_t descsz,
+                        write_core_dump_function f, void *opaque)
+{
+    DumpState *s = opaque;
+    int endian = s->dump_info.d_endian;
+    Elf64_Nhdr *note64;
+    Elf32_Nhdr *note32;
+    void *note;
+    char *buf;
+    size_t note_size, name_size, note_head_size;
+    int ret;
+
+    name_size = strlen(name) + 1;
+
+    if (class == ELFCLASS32) {
+        note_head_size = sizeof(Elf32_Nhdr);
+    } else {
+        note_head_size = sizeof(Elf64_Nhdr);
+    }
+    note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    if (class == ELFCLASS32) {
+        note32 = note;
+        note32->n_namesz = cpu_convert_to_target32(name_size, endian);
+        note32->n_descsz = cpu_convert_to_target32(descsz, endian);
+        note32->n_type = cpu_convert_to_target32(type, endian);
+    } else {
+        note64 = note;
+        note64->n_namesz = cpu_convert_to_target64(name_size, endian);
+        note64->n_descsz = cpu_convert_to_target64(descsz, endian);
+        note64->n_type = cpu_convert_to_target64(type, endian);
+    }
+    buf = note;
+    buf += ((note_head_size + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf, desc, descsz);
+
+    ret = f(note, note_size, opaque);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
 static int write_elf_loads(DumpState *s)
 {
     hwaddr offset;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index e25b7cf..b07816a 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -32,4 +32,8 @@ int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
 int cpu_get_dump_info(ArchDumpInfo *info);
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
 
+int dump_write_elf_note(int class, const char *name, uint32_t type, void *desc,
+                        size_t descsz, write_core_dump_function f,
+                        void *opaque);
+
 #endif
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 2cd2f7f..eea7f7f 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -38,66 +38,43 @@ static int x86_64_write_elf64_note(write_core_dump_function f,
                                    CPUArchState *env, int id,
                                    void *opaque)
 {
-    x86_64_user_regs_struct regs;
-    Elf64_Nhdr *note;
-    char *buf;
-    int descsz, note_size, name_size = 5;
-    const char *name = "CORE";
-    int ret;
-
-    regs.r15 = env->regs[15];
-    regs.r14 = env->regs[14];
-    regs.r13 = env->regs[13];
-    regs.r12 = env->regs[12];
-    regs.r11 = env->regs[11];
-    regs.r10 = env->regs[10];
-    regs.r9  = env->regs[9];
-    regs.r8  = env->regs[8];
-    regs.rbp = env->regs[R_EBP];
-    regs.rsp = env->regs[R_ESP];
-    regs.rdi = env->regs[R_EDI];
-    regs.rsi = env->regs[R_ESI];
-    regs.rdx = env->regs[R_EDX];
-    regs.rcx = env->regs[R_ECX];
-    regs.rbx = env->regs[R_EBX];
-    regs.rax = env->regs[R_EAX];
-    regs.rip = env->eip;
-    regs.eflags = env->eflags;
-
-    regs.orig_rax = 0; /* FIXME */
-    regs.cs = env->segs[R_CS].selector;
-    regs.ss = env->segs[R_SS].selector;
-    regs.fs_base = env->segs[R_FS].base;
-    regs.gs_base = env->segs[R_GS].base;
-    regs.ds = env->segs[R_DS].selector;
-    regs.es = env->segs[R_ES].selector;
-    regs.fs = env->segs[R_FS].selector;
-    regs.gs = env->segs[R_GS].selector;
-
-    descsz = sizeof(x86_64_elf_prstatus);
-    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
-    note = g_malloc(note_size);
-
-    memset(note, 0, note_size);
-    note->n_namesz = cpu_to_le32(name_size);
-    note->n_descsz = cpu_to_le32(descsz);
-    note->n_type = cpu_to_le32(NT_PRSTATUS);
-    buf = (char *)note;
-    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
-    memcpy(buf, name, name_size);
-    buf += ((name_size + 3) / 4) * 4;
-    memcpy(buf + 32, &id, 4); /* pr_pid */
-    buf += descsz - sizeof(x86_64_user_regs_struct)-sizeof(target_ulong);
-    memcpy(buf, &regs, sizeof(x86_64_user_regs_struct));
-
-    ret = f(note, note_size, opaque);
-    g_free(note);
-    if (ret < 0) {
-        return -1;
-    }
-
-    return 0;
+    x86_64_elf_prstatus prstatus;
+
+    memset(&prstatus, 0, sizeof(prstatus));
+
+    prstatus.pid = id;
+    prstatus.regs.r15 = env->regs[15];
+    prstatus.regs.r14 = env->regs[14];
+    prstatus.regs.r13 = env->regs[13];
+    prstatus.regs.r12 = env->regs[12];
+    prstatus.regs.r11 = env->regs[11];
+    prstatus.regs.r10 = env->regs[10];
+    prstatus.regs.r9  = env->regs[9];
+    prstatus.regs.r8  = env->regs[8];
+    prstatus.regs.rbp = env->regs[R_EBP];
+    prstatus.regs.rsp = env->regs[R_ESP];
+    prstatus.regs.rdi = env->regs[R_EDI];
+    prstatus.regs.rsi = env->regs[R_ESI];
+    prstatus.regs.rdx = env->regs[R_EDX];
+    prstatus.regs.rcx = env->regs[R_ECX];
+    prstatus.regs.rbx = env->regs[R_EBX];
+    prstatus.regs.rax = env->regs[R_EAX];
+    prstatus.regs.rip = env->eip;
+    prstatus.regs.eflags = env->eflags;
+
+    prstatus.regs.orig_rax = 0; /* FIXME */
+    prstatus.regs.cs = env->segs[R_CS].selector;
+    prstatus.regs.ss = env->segs[R_SS].selector;
+    prstatus.regs.fs_base = env->segs[R_FS].base;
+    prstatus.regs.gs_base = env->segs[R_GS].base;
+    prstatus.regs.ds = env->segs[R_DS].selector;
+    prstatus.regs.es = env->segs[R_ES].selector;
+    prstatus.regs.fs = env->segs[R_FS].selector;
+    prstatus.regs.gs = env->segs[R_GS].selector;
+
+    return dump_write_elf_note(ELFCLASS64, "CORE", NT_PRSTATUS,
+                               &prstatus, sizeof(prstatus),
+                               f, opaque);
 }
 #endif
 
@@ -148,35 +125,12 @@ static int x86_write_elf64_note(write_core_dump_function f, CPUArchState *env,
                                 int id, void *opaque)
 {
     x86_elf_prstatus prstatus;
-    Elf64_Nhdr *note;
-    char *buf;
-    int descsz, note_size, name_size = 5;
-    const char *name = "CORE";
-    int ret;
 
     x86_fill_elf_prstatus(&prstatus, env, id);
-    descsz = sizeof(x86_elf_prstatus);
-    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
-    note = g_malloc(note_size);
-
-    memset(note, 0, note_size);
-    note->n_namesz = cpu_to_le32(name_size);
-    note->n_descsz = cpu_to_le32(descsz);
-    note->n_type = cpu_to_le32(NT_PRSTATUS);
-    buf = (char *)note;
-    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
-    memcpy(buf, name, name_size);
-    buf += ((name_size + 3) / 4) * 4;
-    memcpy(buf, &prstatus, sizeof(prstatus));
-
-    ret = f(note, note_size, opaque);
-    g_free(note);
-    if (ret < 0) {
-        return -1;
-    }
 
-    return 0;
+    return dump_write_elf_note(ELFCLASS64, "CORE", NT_PRSTATUS,
+                               &prstatus, sizeof(prstatus),
+                               f, opaque);
 }
 
 int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
@@ -202,35 +156,12 @@ int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
                          int cpuid, void *opaque)
 {
     x86_elf_prstatus prstatus;
-    Elf32_Nhdr *note;
-    char *buf;
-    int descsz, note_size, name_size = 5;
-    const char *name = "CORE";
-    int ret;
 
     x86_fill_elf_prstatus(&prstatus, env, cpuid);
-    descsz = sizeof(x86_elf_prstatus);
-    note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
-    note = g_malloc(note_size);
-
-    memset(note, 0, note_size);
-    note->n_namesz = cpu_to_le32(name_size);
-    note->n_descsz = cpu_to_le32(descsz);
-    note->n_type = cpu_to_le32(NT_PRSTATUS);
-    buf = (char *)note;
-    buf += ((sizeof(Elf32_Nhdr) + 3) / 4) * 4;
-    memcpy(buf, name, name_size);
-    buf += ((name_size + 3) / 4) * 4;
-    memcpy(buf, &prstatus, sizeof(prstatus));
-
-    ret = f(note, note_size, opaque);
-    g_free(note);
-    if (ret < 0) {
-        return -1;
-    }
 
-    return 0;
+    return dump_write_elf_note(ELFCLASS32, "CORE", NT_PRSTATUS,
+                               &prstatus, sizeof(prstatus),
+                               f, opaque);
 }
 
 /*
@@ -317,69 +248,28 @@ static void qemu_get_cpustate(QEMUCPUState *s, CPUArchState *env)
     s->cr[4] = env->cr[4];
 }
 
-static inline int cpu_write_qemu_note(write_core_dump_function f,
+static inline int cpu_write_qemu_note(int class, write_core_dump_function f,
                                       CPUArchState *env,
-                                      void *opaque,
-                                      int type)
+                                      void *opaque)
 {
     QEMUCPUState state;
-    Elf64_Nhdr *note64;
-    Elf32_Nhdr *note32;
-    void *note;
-    char *buf;
-    int descsz, note_size, name_size = 5, note_head_size;
-    const char *name = "QEMU";
-    int ret;
 
     qemu_get_cpustate(&state, env);
 
-    descsz = sizeof(state);
-    if (type == 0) {
-        note_head_size = sizeof(Elf32_Nhdr);
-    } else {
-        note_head_size = sizeof(Elf64_Nhdr);
-    }
-    note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
-    note = g_malloc(note_size);
-
-    memset(note, 0, note_size);
-    if (type == 0) {
-        note32 = note;
-        note32->n_namesz = cpu_to_le32(name_size);
-        note32->n_descsz = cpu_to_le32(descsz);
-        note32->n_type = 0;
-    } else {
-        note64 = note;
-        note64->n_namesz = cpu_to_le32(name_size);
-        note64->n_descsz = cpu_to_le32(descsz);
-        note64->n_type = 0;
-    }
-    buf = note;
-    buf += ((note_head_size + 3) / 4) * 4;
-    memcpy(buf, name, name_size);
-    buf += ((name_size + 3) / 4) * 4;
-    memcpy(buf, &state, sizeof(state));
-
-    ret = f(note, note_size, opaque);
-    g_free(note);
-    if (ret < 0) {
-        return -1;
-    }
-
-    return 0;
+    return dump_write_elf_note(class, "QEMU", 0, &state, sizeof(state),
+                               f, opaque);
 }
 
 int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
                              void *opaque)
 {
-    return cpu_write_qemu_note(f, env, opaque, 1);
+    return cpu_write_qemu_note(ELFCLASS64, f, env, opaque);
 }
 
 int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
                              void *opaque)
 {
-    return cpu_write_qemu_note(f, env, opaque, 0);
+    return cpu_write_qemu_note(ELFCLASS32, f, env, opaque);
 }
 
 int cpu_get_dump_info(ArchDumpInfo *info)
-- 
1.7.10.4

[Qemu-devel] [PATCHv2 3/6] dump: extract out get note size function

[Rabin Vincent]

Extract out the ELF note size function from i386 so we can use it from
other targets.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 dump.c                  |   15 +++++++++++++++
 include/sysemu/dump.h   |    2 ++
 target-i386/arch_dump.c |   14 ++------------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/dump.c b/dump.c
index c5e009a..4b7d76c 100644
--- a/dump.c
+++ b/dump.c
@@ -465,6 +465,21 @@ static hwaddr get_offset(hwaddr phys_addr,
     return -1;
 }
 
+size_t dump_get_note_size(int class, const char *name, size_t descsz)
+{
+    int name_size = strlen(name) + 1;
+    int note_head_size;
+
+    if (class == ELFCLASS32) {
+        note_head_size = sizeof(Elf32_Nhdr);
+    } else {
+        note_head_size = sizeof(Elf64_Nhdr);
+    }
+
+    return ((note_head_size + 3) / 4 + (name_size + 3) / 4
+            + (descsz + 3) / 4) * 4;
+}
+
 int dump_write_elf_note(int class, const char *name, uint32_t type,
                         void *desc, size_t descsz,
                         write_core_dump_function f, void *opaque)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index b07816a..a06b149 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -36,4 +36,6 @@ int dump_write_elf_note(int class, const char *name, uint32_t type, void *desc,
                         size_t descsz, write_core_dump_function f,
                         void *opaque);
 
+size_t dump_get_note_size(int class, const char *name, size_t descsz);
+
 #endif
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index eea7f7f..49fa024 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -307,18 +307,10 @@ int cpu_get_dump_info(ArchDumpInfo *info)
 
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 {
-    int name_size = 5; /* "CORE" or "QEMU" */
     size_t elf_note_size = 0;
     size_t qemu_note_size = 0;
     int elf_desc_size = 0;
     int qemu_desc_size = 0;
-    int note_head_size;
-
-    if (class == ELFCLASS32) {
-        note_head_size = sizeof(Elf32_Nhdr);
-    } else {
-        note_head_size = sizeof(Elf64_Nhdr);
-    }
 
     if (machine == EM_386) {
         elf_desc_size = sizeof(x86_elf_prstatus);
@@ -330,10 +322,8 @@ ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 #endif
     qemu_desc_size = sizeof(QEMUCPUState);
 
-    elf_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-                     (elf_desc_size + 3) / 4) * 4;
-    qemu_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-                      (qemu_desc_size + 3) / 4) * 4;
+    elf_note_size = dump_get_note_size(class, "CORE", elf_desc_size);
+    qemu_note_size = dump_get_note_size(class, "QEMU", qemu_desc_size);
 
     return (elf_note_size + qemu_note_size) * nr_cpus;
 }
-- 
1.7.10.4

[Qemu-devel] [PATCHv2 4/6] dump: fix up memory mapping dependencies / stub

[Rabin Vincent]

dump.c won't build without the functions from memory_mapping.c (and
memory_mapping-stub.c does not help there), so build memory_mapping.c
when CONFIG_HAVE_CORE_DUMP is set.

  dump.c:84: undefined reference to `memory_mapping_list_free'
  dump.c:819: undefined reference to `memory_mapping_list_init'
  dump.c:827: undefined reference to `memory_mapping_filter'

Allow memory_mapping-stub.c to instead be used for targets which do not
set CONFIG_HAVE_GET_MEMORY_MAPPING.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 Makefile.target       |    3 +--
 memory_mapping-stub.c |    5 -----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 2bd6d14..629f48a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -115,8 +115,7 @@ obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += memory.o savevm.o cputlb.o
-obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o memory_mapping.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
 LIBS+=-lz
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
index 24d5d67..6dd9e36 100644
--- a/memory_mapping-stub.c
+++ b/memory_mapping-stub.c
@@ -15,11 +15,6 @@
 #include "exec/cpu-all.h"
 #include "sysemu/memory_mapping.h"
 
-int qemu_get_guest_memory_mapping(MemoryMappingList *list)
-{
-    return -2;
-}
-
 int cpu_get_memory_mapping(MemoryMappingList *list,
 					                                          CPUArchState *env)
 {
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv2 4/6] dump: fix up memory mapping dependencies / stub

[Paolo Bonzini]

Il 24/03/2013 18:27, Rabin Vincent ha scritto:
> dump.c won't build without the functions from memory_mapping.c (and
> memory_mapping-stub.c does not help there), so build memory_mapping.c
> when CONFIG_HAVE_CORE_DUMP is set.
> 
>   dump.c:84: undefined reference to `memory_mapping_list_free'
>   dump.c:819: undefined reference to `memory_mapping_list_init'
>   dump.c:827: undefined reference to `memory_mapping_filter'
> 
> Allow memory_mapping-stub.c to instead be used for targets which do not
> set CONFIG_HAVE_GET_MEMORY_MAPPING.

The right fix is to add these to memory_mapping-stub.c.

Paolo

[Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Rabin Vincent]

Enable support for the dump-guest-memory monitor command for ARM.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 configure                |    2 +-
 target-arm/Makefile.objs |    2 +-
 target-arm/arch_dump.c   |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 target-arm/arch_dump.c

diff --git a/configure b/configure
index 46a7594..f35786d 100755
--- a/configure
+++ b/configure
@@ -4184,7 +4184,7 @@ if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
   echo "LIBS+=$libs_softmmu $target_libs_softmmu" >> $config_target_mak
   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 d89b57c..93baa12 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_dump.o
 obj-$(CONFIG_KVM) += kvm.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..e568ffb
--- /dev/null
+++ b/target-arm/arch_dump.c
@@ -0,0 +1,61 @@
+#include "cpu.h"
+#include "sysemu/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;
+
+int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+                         int cpuid, void *opaque)
+{
+    return -1;
+}
+
+int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
+                         int cpuid, void *opaque)
+{
+    arm_elf_prstatus prstatus = {.pid = cpuid};
+
+    memcpy(&(prstatus.regs), env->regs, sizeof(env->regs));
+    prstatus.regs[16] = cpsr_read(env);
+
+    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;
+#ifdef TARGET_WORDS_BIGENDIAN
+    info->d_endian = ELFDATA2MSB;
+#else
+    info->d_endian = ELFDATA2LSB;
+#endif
+    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));
+}
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Peter Maydell]

On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:
> Enable support for the dump-guest-memory monitor command for ARM.

Hi. I'm afraid I'm not really familiar with the dump-guest-memory
command/implementation so I have some possibly dumb comments below:

> --- /dev/null
> +++ b/target-arm/arch_dump.c
> @@ -0,0 +1,61 @@
> +#include "cpu.h"
> +#include "sysemu/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;

Can you point me at the spec this struct corresponds to?

> +
> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
> +                         int cpuid, void *opaque)
> +{
> +    return -1;
> +}

Is there any documentation of the API we're implementing here?
(why does it require us to provide 64 bit functions that are
never used?)

> +
> +int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
> +                         int cpuid, void *opaque)
> +{
> +    arm_elf_prstatus prstatus = {.pid = cpuid};
> +
> +    memcpy(&(prstatus.regs), env->regs, sizeof(env->regs));
> +    prstatus.regs[16] = cpsr_read(env);
> +
> +    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;
> +}

Why is it OK to implement this as a no-op? What could we
be doing here that we don't do? What are the effects?

> +
> +int cpu_get_dump_info(ArchDumpInfo *info)
> +{
> +    info->d_machine = EM_ARM;
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    info->d_endian = ELFDATA2MSB;
> +#else
> +    info->d_endian = ELFDATA2LSB;
> +#endif
> +    info->d_class = ELFCLASS32;

Most of this looks like it would be suitable for a default
implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
machine is ELF_MACHINE, class based on TARGET_LONG_BITS".

> +
> +    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));
> +}
> --
> 1.7.10.4
>

thanks
-- PMM

Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Rabin Vincent]

2013/3/24 Peter Maydell <peter.maydell@linaro.org>:
> On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:
>> --- /dev/null
>> +++ b/target-arm/arch_dump.c
>> @@ -0,0 +1,61 @@
>> +#include "cpu.h"
>> +#include "sysemu/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;
>
> Can you point me at the spec this struct corresponds to?

This is elf_prstatus from the Linux kernel's
include/uapi/linux/elfcore.h, with the regset begin ARM regs in this
case.

I don't know if there's a spec.  It doesn't sound like it from the
comments in the kernel file: "This is mostly like the SVR4 structure,
but more Linuxy, with things that Linux does not support and which gdb
doesn't really use excluded."

The x86 implementation in target-i386/arch_dump.c uses the same
elf_prstatus with the x86 regs.

>
>> +
>> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
>> +                         int cpuid, void *opaque)
>> +{
>> +    return -1;
>> +}
>
> Is there any documentation of the API we're implementing here?

I coudn't find any documentation.  It's only x86 that has the API
implemented.

> (why does it require us to provide 64 bit functions that are
> never used?)

I guess the API was made with x86 in mind.  I will see if the
requirement can be removed with some ifdefs in the dump.c file.

(come to think of it, I guess this ARM code will need to use ELFCLASS64
 when we have physical memory > 4GiB (LPAE))

>> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
>> +                             void *opaque)
>> +{
>> +    return 0;
>> +}
>
> Why is it OK to implement this as a no-op? What could we
> be doing here that we don't do? What are the effects?

This is supposed to be used to add some additional information about the
CPU state in an ELF note in a QEMU-specific structure, like the
QEMUCPUState in target-i386/arm-state.c.  It's only useful to implement
this if someone sees a need to add any such information.

>
>> +
>> +int cpu_get_dump_info(ArchDumpInfo *info)
>> +{
>> +    info->d_machine = EM_ARM;
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +    info->d_endian = ELFDATA2MSB;
>> +#else
>> +    info->d_endian = ELFDATA2LSB;
>> +#endif
>> +    info->d_class = ELFCLASS32;
>
> Most of this looks like it would be suitable for a default
> implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
> machine is ELF_MACHINE, class based on TARGET_LONG_BITS".

I will see if this can be moved into the generic dump.c.

Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Peter Maydell]

On 24 March 2013 19:26, Rabin Vincent <rabin@rab.in> wrote:
> 2013/3/24 Peter Maydell <peter.maydell@linaro.org>:
>> On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:

So I guess I should prefix this email by saying that quite
a bit of it is really comments on the existing dump code
rather than the ARM related support you're adding here. I
appreciate that it's annoying to get asked to fix up a bunch
of existing code just because you happened to be the next
person to want to add use case 2 to it. So you should feel free
to push back a bit if any of this seems too burdensome or
unnecessary.

(I do think that the memory region stuff in the other patch
does need fixing though, because (a) vexpress really does
map RAM in two places in the physical address map and (b) we
need to be careful about what we allow to be added to the
memory API, so it remains coherent. So if you only fix up one
thing it should be that.)

>>> --- /dev/null
>>> +++ b/target-arm/arch_dump.c
>>> @@ -0,0 +1,61 @@
>>> +#include "cpu.h"
>>> +#include "sysemu/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;
>>
>> Can you point me at the spec this struct corresponds to?
>
> This is elf_prstatus from the Linux kernel's
> include/uapi/linux/elfcore.h, with the regset begin ARM regs in this
> case.

They don't look very similar to me -- this one has a lot of
pad fields the elf_prstatus doesn't. Also, if the kernel's
struct is a standard one with a cpu-specific regset field,
why isn't QEMU also using a standard struct with a cpu-specific
regset field?

(it seems a bit bogus that we aren't saving the floating point
registers too, but it looks like Linux doesn't do that either,
so I guess there's nowhere in the core file for it.)

> I don't know if there's a spec.  It doesn't sound like it from the
> comments in the kernel file: "This is mostly like the SVR4 structure,
> but more Linuxy, with things that Linux does not support and which gdb
> doesn't really use excluded."
>
> The x86 implementation in target-i386/arch_dump.c uses the same
> elf_prstatus with the x86 regs.
>
>>
>>> +
>>> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
>>> +                         int cpuid, void *opaque)
>>> +{
>>> +    return -1;
>>> +}
>>
>> Is there any documentation of the API we're implementing here?
>
> I coudn't find any documentation.  It's only x86 that has the API
> implemented.

It would be nice if somebody who understood it could add some
doc comments to the header file.

>> (why does it require us to provide 64 bit functions that are
>> never used?)
>
> I guess the API was made with x86 in mind.  I will see if the
> requirement can be removed with some ifdefs in the dump.c file.
>
> (come to think of it, I guess this ARM code will need to use ELFCLASS64
>  when we have physical memory > 4GiB (LPAE))

It would be good to check whether that is correct -- mostly core
files are for dumps of a virtual address space so I don't know
whether gdb/etc would understand an ARM corefile which claimed
ELFCLASS64. (I mean understand it as an AArch32 corefile rather
than AArch64.)

>>> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
>>> +                             void *opaque)
>>> +{
>>> +    return 0;
>>> +}
>>
>> Why is it OK to implement this as a no-op? What could we
>> be doing here that we don't do? What are the effects?
>
> This is supposed to be used to add some additional information about the
> CPU state in an ELF note in a QEMU-specific structure, like the
> QEMUCPUState in target-i386/arm-state.c.  It's only useful to implement
> this if someone sees a need to add any such information.

OK.

>>
>>> +
>>> +int cpu_get_dump_info(ArchDumpInfo *info)
>>> +{
>>> +    info->d_machine = EM_ARM;
>>> +#ifdef TARGET_WORDS_BIGENDIAN
>>> +    info->d_endian = ELFDATA2MSB;
>>> +#else
>>> +    info->d_endian = ELFDATA2LSB;
>>> +#endif
>>> +    info->d_class = ELFCLASS32;
>>
>> Most of this looks like it would be suitable for a default
>> implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
>> machine is ELF_MACHINE, class based on TARGET_LONG_BITS".
>
> I will see if this can be moved into the generic dump.c.

thanks
-- PMM

Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Paolo Bonzini]

Il 24/03/2013 21:39, Peter Maydell ha scritto:
>> >
>> > I guess the API was made with x86 in mind.  I will see if the
>> > requirement can be removed with some ifdefs in the dump.c file.
>> >
>> > (come to think of it, I guess this ARM code will need to use ELFCLASS64
>> >  when we have physical memory > 4GiB (LPAE))
> It would be good to check whether that is correct -- mostly core
> files are for dumps of a virtual address space so I don't know
> whether gdb/etc would understand an ARM corefile which claimed
> ELFCLASS64. (I mean understand it as an AArch32 corefile rather
> than AArch64.)

Note that in this case we can choose between physical and virtual
address space dumps.

Virtual address space is mostly useful with gdb; physical address space
can be used for example with "crash", a post-mortem kernel debugger.

Paolo

Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

[Peter Maydell]

On 4 April 2013 10:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 24/03/2013 21:39, Peter Maydell ha scritto:
>>> > (come to think of it, I guess this ARM code will need to use ELFCLASS64
>>> >  when we have physical memory > 4GiB (LPAE))
>> It would be good to check whether that is correct -- mostly core
>> files are for dumps of a virtual address space so I don't know
>> whether gdb/etc would understand an ARM corefile which claimed
>> ELFCLASS64. (I mean understand it as an AArch32 corefile rather
>> than AArch64.)
>
> Note that in this case we can choose between physical and virtual
> address space dumps.
>
> Virtual address space is mostly useful with gdb; physical address space
> can be used for example with "crash", a post-mortem kernel debugger.

Yeah, my point was really that either way we should be checking
how the kernel and these tools expect to handle an LPAE dump.

-- PMM

[Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

[Rabin Vincent]

RAMBlock.offset does not provide the physical address of the memory
region.  This is available in the MemoryRegion's address.  The wrong
usage leads to incorrect physical addreses in the ELF.  Fix it.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 dump.c                |   19 +++++++++++--------
 include/exec/memory.h |    7 +++++++
 memory.c              |   12 ++++++++++++
 memory_mapping.c      |    6 ++++--
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/dump.c b/dump.c
index 4b7d76c..4b0353a 100644
--- a/dump.c
+++ b/dump.c
@@ -16,6 +16,7 @@
 #include "cpu.h"
 #include "exec/cpu-all.h"
 #include "exec/hwaddr.h"
+#include "exec/memory.h"
 #include "monitor/monitor.h"
 #include "sysemu/kvm.h"
 #include "sysemu/dump.h"
@@ -432,26 +433,28 @@ static hwaddr get_offset(hwaddr phys_addr,
     }
 
     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+        hwaddr baddr = memory_region_get_addr(block->mr);
+
         if (s->has_filter) {
-            if (block->offset >= s->begin + s->length ||
-                block->offset + block->length <= s->begin) {
+            if (baddr >= s->begin + s->length ||
+                baddr + block->length <= s->begin) {
                 /* This block is out of the range */
                 continue;
             }
 
-            if (s->begin <= block->offset) {
-                start = block->offset;
+            if (s->begin <= baddr) {
+                start = baddr;
             } else {
                 start = s->begin;
             }
 
-            size_in_block = block->length - (start - block->offset);
-            if (s->begin + s->length < block->offset + block->length) {
-                size_in_block -= block->offset + block->length -
+            size_in_block = block->length - (start - baddr);
+            if (s->begin + s->length < baddr + block->length) {
+                size_in_block -= baddr + block->length -
                                  (s->begin + s->length);
             }
         } else {
-            start = block->offset;
+            start = baddr;
             size_in_block = block->length;
         }
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..9227190 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -665,6 +665,13 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
                                          unsigned priority);
 
 /**
+ * memory_region_get_addr: Get the address of a memory region
+ *
+ * @mr: the memory region
+ */
+hwaddr memory_region_get_addr(MemoryRegion *mr);
+
+/**
  * memory_region_get_ram_addr: Get the ram address associated with a memory
  *                             region
  *
diff --git a/memory.c b/memory.c
index 92a2196..f90fd19 100644
--- a/memory.c
+++ b/memory.c
@@ -1427,6 +1427,18 @@ void memory_region_set_alias_offset(MemoryRegion *mr, hwaddr offset)
     memory_region_transaction_commit();
 }
 
+hwaddr memory_region_get_addr(MemoryRegion *mr)
+{
+    hwaddr addr = 0;
+
+    while (mr) {
+	    addr += mr->addr;
+	    mr = mr->parent;
+    }
+
+    return addr;
+}
+
 ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
 {
     return mr->ram_addr;
diff --git a/memory_mapping.c b/memory_mapping.c
index ff45b3a..cf0751c 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -13,6 +13,7 @@
 
 #include "cpu.h"
 #include "exec/cpu-all.h"
+#include "exec/memory.h"
 #include "sysemu/memory_mapping.h"
 
 static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
@@ -201,7 +202,7 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
      * address.
      */
     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-        offset = block->offset;
+        offset = memory_region_get_addr(block->mr);
         length = block->length;
         create_new_memory_mapping(list, offset, offset, length);
     }
@@ -214,7 +215,8 @@ void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
     RAMBlock *block;
 
     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-        create_new_memory_mapping(list, block->offset, 0, block->length);
+        create_new_memory_mapping(list, memory_region_get_addr(block->mr),
+                                  0, block->length);
     }
 }
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

[Peter Maydell]

On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:
>  /**
> + * memory_region_get_addr: Get the address of a memory region
> + *
> + * @mr: the memory region
> + */
> +hwaddr memory_region_get_addr(MemoryRegion *mr);

I'm afraid this doesn't make sense. A MemoryRegion by itself has
no "address" -- it could be mapped into several places in several
different address maps or none at all.

-- PMM

Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

[Rabin Vincent]

2013/3/24 Peter Maydell <peter.maydell@linaro.org>:
> On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:
>>  /**
>> + * memory_region_get_addr: Get the address of a memory region
>> + *
>> + * @mr: the memory region
>> + */
>> +hwaddr memory_region_get_addr(MemoryRegion *mr);
>
> I'm afraid this doesn't make sense. A MemoryRegion by itself has
> no "address" -- it could be mapped into several places in several
> different address maps or none at all.

OK.  Do you mean that such a function can be used internally to the dump
code where it gets the MemoryRegion only from the RAMBlock.mr or do you
mean the dump code also shouldn't be doing it that way?

If you mean the latter, could you please suggest an alternative way to
handle this?  The problem is that the dump code assumes that
RAMBlock.offset provides the physical address, and this appears to not
be the case.  For example, with a dump generated from vexpress I get
these Program Headers in the dump:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE           0x0000f4 0x00000000 0x00000000 0x002a0 0x002a0     0
  LOAD           0x000394 0x00000000 0x00000000 0x8000000 0x8000000     0
  LOAD           0x8000394 0x00000000 0x08000000 0x4000000 0x4000000     0
  LOAD           0xc000394 0x00000000 0x0c000000 0x4000000 0x4000000     0
  LOAD           0x10000394 0x00000000 0x10000000 0x2000000 0x2000000     0
  LOAD           0x12000394 0x00000000 0x12000000 0x800000 0x800000     0

The physical addresses are completely wrong, and with the patch I get
the right ones:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE           0x0000f4 0x00000000 0x00000000 0x002a0 0x002a0     0
  LOAD           0x8000394 0x00000000 0x40000000 0x4000000 0x4000000     0
  LOAD           0xc000394 0x00000000 0x44000000 0x4000000 0x4000000     0
  LOAD           0x10000394 0x00000000 0x48000000 0x2000000 0x2000000     0
  LOAD           0x12000394 0x00000000 0x4c000000 0x800000 0x800000     0
  LOAD           0x000394 0x00000000 0x60000000 0x8000000 0x8000000     0

Thanks.

Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

[Peter Maydell]

On 24 March 2013 19:35, Rabin Vincent <rabin@rab.in> wrote:
> 2013/3/24 Peter Maydell <peter.maydell@linaro.org>:
>> On 24 March 2013 17:27, Rabin Vincent <rabin@rab.in> wrote:
>>>  /**
>>> + * memory_region_get_addr: Get the address of a memory region
>>> + *
>>> + * @mr: the memory region
>>> + */
>>> +hwaddr memory_region_get_addr(MemoryRegion *mr);
>>
>> I'm afraid this doesn't make sense. A MemoryRegion by itself has
>> no "address" -- it could be mapped into several places in several
>> different address maps or none at all.
>
> OK.  Do you mean that such a function can be used internally to the dump
> code where it gets the MemoryRegion only from the RAMBlock.mr or do you
> mean the dump code also shouldn't be doing it that way?

I mean that the dump code is wrong if it's trying to ask "what
is the address of this MemoryRegion (or RAMBlock)" at all. That
question doesn't have a well defined single answer.

> If you mean the latter, could you please suggest an alternative way to
> handle this?  The problem is that the dump code assumes that
> RAMBlock.offset provides the physical address, and this appears to not
> be the case.

That is also wrong, both for the reason you state and also because
ramblocks can be aliased into multiple physical addresses too.
I suspect you need to change anything that's trying to iterate
through memory with "for each block in ram_list" to instead actually
iterate through the system address space and say "if this is RAM
then...". For instance with the vexpress-a9 you presumably want
a LOAD section for both physaddr 0 (the alias) and physaddr
0x60000000 (the usual address).

-- PMM

Re: [Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

[Andreas Färber]

Hi,

Am 24.03.2013 18:27, schrieb Rabin Vincent:
> A second patchset to add dump-guest-memory support for ARM.
> 
> This version of the patchset addresses the following comments from the
> previous posting, and also adds some more patches to the core dump code
> (patch #4 and #6):
> 
>  - memset prstatus to 0 in x86_64_write_elf64_note()
>  - handle big endian in dump_write_elf_note()
>  - Save CPSR in ARM prstatus
>  - set correct ELF endianness for ARM BE 

This still does not address the architectural issue that I brought up.
That would affect the equivalent s390x patch as well.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

[Rabin Vincent]

2013/3/25 Andreas Färber <afaerber@suse.de>
> This still does not address the architectural issue that I brought up.

I guess you mean the CPUArchState stuff?  AFAICS Wen Congyang (the
author of the dump code) had some answers/questions for you:

http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00382.html
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00384.html

Re: [Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

[Andreas Färber]

Am 29.03.2013 09:36, schrieb Rabin Vincent:
> 2013/3/25 Andreas Färber <afaerber@suse.de>
>> This still does not address the architectural issue that I brought up.
> 
> I guess you mean the CPUArchState stuff?  AFAICS Wen Congyang (the
> author of the dump code) had some answers/questions for you:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00382.html
> http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00384.html

Yes, that is what I'm referring to, and I don't see answers to my
questions...

If as I understand we are talking about a property that depends solely
on the target CPU then it should be implemented on CPUClass level (which
is the one above ARMCPU, S390CPU etc.) and not require changes to
configure at all. CPUArchState is by contrast a per-target type.
qom/cpu.c would implement the dummy versions and in your case
target-arm/cpu.c (in Jens' case target-s390x/cpu.c) should override that
behavior by setting cc->whatever to a static function that actually
implements the functionality. I would supply you with a patch for that
myself, but I am rather busy with downstream ATM and Igor is poking me
for x86 CPU review, so if either of you or Wen Congyang could fix that
design flaw before making it worse I would appreciate that!

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [RFC] make write_elf_xx functions part of CPUClass, use CPUState

[Jens Freimann]

Hi Andreas,

not sure if this is what you had in mind. Does it at least go into the right direction?
It is pretty much untested and won't compile as is. I just wanted to get your opinion 
before putting more work into this.

This patch adds 4 write_elf_XX functions to the CPUClass which can be implemented
by XXXCPUState, e.g. S390CPUstate.

Would it make sense to have paging_enabled/HAVE_MEMORY_MAPPING also as a property
of the CPU object?

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 dump.c                   | 28 +++++++++++++++++++++++----
 include/qom/cpu.h        | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump.h    |  9 +--------
 qom/cpu.c                | 44 +++++++++++++++++++++++++++++++++++++++++++
 target-s390x/arch_dump.c | 25 ++++--------------------
 target-s390x/arch_dump.h | 14 ++++++++++++++
 target-s390x/cpu.c       |  6 ++++++
 7 files changed, 142 insertions(+), 33 deletions(-)
 create mode 100644 target-s390x/arch_dump.h

diff --git a/dump.c b/dump.c
index 574c292..df49845 100644
--- a/dump.c
+++ b/dump.c
@@ -272,13 +272,19 @@ static int write_elf64_notes(DumpState *s)
 {
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int ret;
     int id;
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
         cpu = ENV_GET_CPU(env);
+        cc = CPU_GET_CLASS(cpu);
         id = cpu_index(cpu);
-        ret = cpu_write_elf64_note(fd_write_vmcore, env, id, s);
+        if (cc->write_elf64_note)
+            ret = cc->write_elf64_note(fd_write_vmcore, cpu, id, s);
+        else {
+            ret = 0;
+        }
         if (ret < 0) {
             dump_error(s, "dump: failed to write elf notes.\n");
             return -1;
@@ -286,7 +292,11 @@ static int write_elf64_notes(DumpState *s)
     }
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf64_qemunote(fd_write_vmcore, env, s);
+        if (cc->write_elf64_qemunote)
+            ret = cc->write_elf64_qemunote(fd_write_vmcore, cpu, s);
+        else {
+            ret = 0;
+        }
         if (ret < 0) {
             dump_error(s, "dump: failed to write CPU status.\n");
             return -1;
@@ -324,13 +334,19 @@ static int write_elf32_notes(DumpState *s)
 {
     CPUArchState *env;
     CPUState *cpu;
+    CPUClass *cc;
     int ret;
     int id;
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
         cpu = ENV_GET_CPU(env);
+        cc = CPU_GET_CLASS(cpu);
         id = cpu_index(cpu);
-        ret = cpu_write_elf32_note(fd_write_vmcore, env, id, s);
+        if (cc->write_elf32_note) {
+            ret = cc->write_elf32_note(fd_write_vmcore, cpu, id, s);
+        } else {
+            ret = 0; 
+        }
         if (ret < 0) {
             dump_error(s, "dump: failed to write elf notes.\n");
             return -1;
@@ -338,7 +354,11 @@ static int write_elf32_notes(DumpState *s)
     }
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf32_qemunote(fd_write_vmcore, env, s);
+        if (cc->write_elf32_qemunote) {
+            ret = cc->write_elf32_qemunote(fd_write_vmcore, cpu, s);
+        } else {
+            ret = 0; 
+        }
         if (ret < 0) {
             dump_error(s, "dump: failed to write CPU status.\n");
             return -1;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index ab2657c..53199f1 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -24,6 +24,8 @@
 #include "hw/qdev-core.h"
 #include "qemu/thread.h"
 
+typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);
+
 /**
  * SECTION:cpu
  * @section_id: QEMU-cpu
@@ -55,6 +57,14 @@ typedef struct CPUClass {
     ObjectClass *(*class_by_name)(const char *cpu_model);
 
     void (*reset)(CPUState *cpu);
+    int (*write_elf64_note)(write_core_dump_function f, CPUState *cpu,
+            int cpuid, void *opaque);
+    int (*write_elf64_qemunote)(write_core_dump_function f, CPUState *cpu,
+            void *opaque);
+    int (*write_elf32_note)(write_core_dump_function f, CPUState *cpu,
+            int cpuid, void *opaque);
+    int (*write_elf32_qemunote)(write_core_dump_function f, CPUState *cpu,
+            void *opaque);
 } CPUClass;
 
 struct KVMState;
@@ -116,6 +126,45 @@ struct CPUState {
     int cpu_index; /* used by alpha TCG */
 };
 
+/**
+ * cpu_write_elf64_note:
+ * @f: pointer to a function that writes memory to a file 
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU 
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
+        int cpuid, void *opaque);
+
+/**
+ * cpu_write_elf64_qemunote:
+ * @f: pointer to a function that writes memory to a file 
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU 
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf64_qemunote(write_core_dump_function f, CPUState *cpu,
+        void *opaque);
+
+/**
+ * cpu_write_elf32_note:
+ * @f: pointer to a function that writes memory to a file 
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU 
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf32_note(write_core_dump_function f, CPUState *cpu,
+        int cpuid, void *opaque);
+
+/**
+ * cpu_write_elf32_qemunote:
+ * @f: pointer to a function that writes memory to a file 
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU 
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf32_qemunote(write_core_dump_function f, CPUState *cpu,
+        void *opaque);
 
 /**
  * cpu_reset:
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index b03efd4..e5f9f7d 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -16,6 +16,7 @@
 
 #include "qapi/error.h"
 #include "include/qapi/qmp/qerror.h"
+#include "qom/cpu.h"
 
 typedef struct ArchDumpInfo {
     int d_machine;  /* Architecture */
@@ -24,14 +25,6 @@ typedef struct ArchDumpInfo {
 } ArchDumpInfo;
 
 typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);
-int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
-                                                  int cpuid, void *opaque);
-int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
-                                                  int cpuid, void *opaque);
-int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
-                                                          void *opaque);
-int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
-                                                          void *opaque);
 int cpu_get_dump_info(ArchDumpInfo *info);
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
 
diff --git a/qom/cpu.c b/qom/cpu.c
index 0a2194d..e2822d0 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -21,6 +21,50 @@
 #include "qom/cpu.h"
 #include "qemu-common.h"
 
+int cpu_write_elf32_qemunote(write_core_dump_function f, CPUState *cpu,
+        void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    if (klass->write_elf32_qemunote != NULL) {
+        return (*klass->write_elf32_qemunote)(f, cpu, opaque);
+    }
+    return -1;
+}
+
+int cpu_write_elf32_note(write_core_dump_function f, CPUState *cpu,
+        int cpuid, void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    if (klass->write_elf32_note != NULL) {
+        return (*klass->write_elf32_note)(f, cpu, cpuid, opaque);
+    }
+    return -1;
+}
+
+int cpu_write_elf64_qemunote(write_core_dump_function f, CPUState *cpu,
+        void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    if (klass->write_elf64_note != NULL) {
+        return (*klass->write_elf64_qemunote)(f, cpu, opaque);
+    }
+    return -1;
+}
+
+int cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
+        int cpuid, void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    if (klass->write_elf64_note != NULL) {
+        return (*klass->write_elf64_note)(f, cpu, cpuid, opaque);
+    }
+    return -1;
+}
+
 void cpu_reset(CPUState *cpu)
 {
     CPUClass *klass = CPU_GET_CLASS(cpu);
diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
index 18f2652..c594874 100644
--- a/target-s390x/arch_dump.c
+++ b/target-s390x/arch_dump.c
@@ -13,6 +13,7 @@
 
 #include "cpu.h"
 #include "elf.h"
+#include "arch_dump.h"
 #include "exec/cpu-all.h"
 #include "sysemu/dump.h"
 #include "sysemu/kvm.h"
@@ -186,10 +187,11 @@ static int s390x_write_all_elf64_notes(const char *note_name, write_core_dump_fu
 }
 
 
-int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+int s390_cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
                          int cpuid, void *opaque)
 {
-    return s390x_write_all_elf64_notes("CORE", f, env, cpuid, opaque);
+    S390CPU *_cpu = S390_CPU(cpu);
+    return s390x_write_all_elf64_notes("CORE", f, &_cpu->env, cpuid, opaque);
 }
 
 int cpu_get_dump_info(ArchDumpInfo *info)
@@ -222,25 +224,6 @@ ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 
 }
 
-int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
-                         int cpuid, void *opaque)
-{
-    return 0;
-}
-
-
-int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
-                             void *opaque)
-{
-    return 0;
-}
-
-int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
-                             void *opaque)
-{
-    return 0;
-}
-
 int arch_check_parameter(bool paging, bool has_filter, int64_t begin, int64_t length, 
                           Error **errp)
 {
diff --git a/target-s390x/arch_dump.h b/target-s390x/arch_dump.h
new file mode 100644
index 0000000..8ebc24e
--- /dev/null
+++ b/target-s390x/arch_dump.h
@@ -0,0 +1,14 @@
+/*
+ * dump guest memory implementation 
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+int s390_cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
+                         int cpuid, void *opaque);
+
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 14a9e9d..14b25d8 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -27,6 +27,7 @@
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "hw/hw.h"
+#include "arch_dump.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/s390x/sclp.h"
 #include "sysemu/arch_init.h"
@@ -231,6 +232,11 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     scc->parent_reset = cc->reset;
     cc->reset = s390_cpu_reset;
 
+    cc->write_elf64_note = s390_cpu_write_elf64_note;
+    cc->write_elf64_qemunote = NULL;
+    cc->write_elf32_note = NULL;
+    cc->write_elf32_qemunote = NULL;
+
     dc->vmsd = &vmstate_s390_cpu;
 }
 
-- 
1.8.0.1

Re: [Qemu-devel] [RFC] make write_elf_xx functions part of CPUClass, use CPUState

[Andreas Färber]

Hi Jens,

Am 09.04.2013 14:09, schrieb Jens Freimann:
> Hi Andreas,
> 
> not sure if this is what you had in mind. Does it at least go into the right direction?
> It is pretty much untested and won't compile as is. I just wanted to get your opinion 
> before putting more work into this.
> 
> This patch adds 4 write_elf_XX functions to the CPUClass which can be implemented
> by XXXCPUState, e.g. S390CPUstate.

That is mostly like I expected it, comments inline.

> 
> Would it make sense to have paging_enabled/HAVE_MEMORY_MAPPING also as a property
> of the CPU object?

Hm, I don't think QOM properties make much sense for the internal
restructuring - I'm not sure if we already have some indication whether
this can be used from QMP for a given target? That would be the only use
case for HAVE_MEMORY_MAPPING as QOM property that I could think of. for
paging_enabled() I think the only users would be internal so this could
be a QOM method/function just like the write_* functions.

> 
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
>  dump.c                   | 28 +++++++++++++++++++++++----
>  include/qom/cpu.h        | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/sysemu/dump.h    |  9 +--------
>  qom/cpu.c                | 44 +++++++++++++++++++++++++++++++++++++++++++
>  target-s390x/arch_dump.c | 25 ++++--------------------
>  target-s390x/arch_dump.h | 14 ++++++++++++++
>  target-s390x/cpu.c       |  6 ++++++
>  7 files changed, 142 insertions(+), 33 deletions(-)
>  create mode 100644 target-s390x/arch_dump.h
> 
> diff --git a/dump.c b/dump.c
> index 574c292..df49845 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -272,13 +272,19 @@ static int write_elf64_notes(DumpState *s)
>  {
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int ret;
>      int id;
>  
>      for (env = first_cpu; env != NULL; env = env->next_cpu) {
>          cpu = ENV_GET_CPU(env);
> +        cc = CPU_GET_CLASS(cpu);
>          id = cpu_index(cpu);
> -        ret = cpu_write_elf64_note(fd_write_vmcore, env, id, s);
> +        if (cc->write_elf64_note)
> +            ret = cc->write_elf64_note(fd_write_vmcore, cpu, id, s);
> +        else {
> +            ret = 0;
> +        }

Why are you changing the logic here?

>          if (ret < 0) {
>              dump_error(s, "dump: failed to write elf notes.\n");
>              return -1;
> @@ -286,7 +292,11 @@ static int write_elf64_notes(DumpState *s)
>      }
>  
>      for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        ret = cpu_write_elf64_qemunote(fd_write_vmcore, env, s);
> +        if (cc->write_elf64_qemunote)
> +            ret = cc->write_elf64_qemunote(fd_write_vmcore, cpu, s);
> +        else {
> +            ret = 0;
> +        }

Dito?

>          if (ret < 0) {
>              dump_error(s, "dump: failed to write CPU status.\n");
>              return -1;
> @@ -324,13 +334,19 @@ static int write_elf32_notes(DumpState *s)
>  {
>      CPUArchState *env;
>      CPUState *cpu;
> +    CPUClass *cc;
>      int ret;
>      int id;
>  
>      for (env = first_cpu; env != NULL; env = env->next_cpu) {
>          cpu = ENV_GET_CPU(env);
> +        cc = CPU_GET_CLASS(cpu);
>          id = cpu_index(cpu);
> -        ret = cpu_write_elf32_note(fd_write_vmcore, env, id, s);
> +        if (cc->write_elf32_note) {
> +            ret = cc->write_elf32_note(fd_write_vmcore, cpu, id, s);
> +        } else {
> +            ret = 0; 
> +        }

Dito?

>          if (ret < 0) {
>              dump_error(s, "dump: failed to write elf notes.\n");
>              return -1;
> @@ -338,7 +354,11 @@ static int write_elf32_notes(DumpState *s)
>      }
>  
>      for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        ret = cpu_write_elf32_qemunote(fd_write_vmcore, env, s);
> +        if (cc->write_elf32_qemunote) {
> +            ret = cc->write_elf32_qemunote(fd_write_vmcore, cpu, s);
> +        } else {
> +            ret = 0; 
> +        }

Dito?

>          if (ret < 0) {
>              dump_error(s, "dump: failed to write CPU status.\n");
>              return -1;
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index ab2657c..53199f1 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -24,6 +24,8 @@
>  #include "hw/qdev-core.h"
>  #include "qemu/thread.h"
>  
> +typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);

You're only copying it (which may lead to duplicate typedef warnings
btw) but it would be nice to name it WriteCoreDump or so here.

> +
>  /**
>   * SECTION:cpu
>   * @section_id: QEMU-cpu
> @@ -55,6 +57,14 @@ typedef struct CPUClass {
>      ObjectClass *(*class_by_name)(const char *cpu_model);
>  
>      void (*reset)(CPUState *cpu);
> +    int (*write_elf64_note)(write_core_dump_function f, CPUState *cpu,
> +            int cpuid, void *opaque);
> +    int (*write_elf64_qemunote)(write_core_dump_function f, CPUState *cpu,
> +            void *opaque);
> +    int (*write_elf32_note)(write_core_dump_function f, CPUState *cpu,
> +            int cpuid, void *opaque);
> +    int (*write_elf32_qemunote)(write_core_dump_function f, CPUState *cpu,
> +            void *opaque);
>  } CPUClass;
>  
>  struct KVMState;
> @@ -116,6 +126,45 @@ struct CPUState {
>      int cpu_index; /* used by alpha TCG */
>  };
>  
> +/**
> + * cpu_write_elf64_note:
> + * @f: pointer to a function that writes memory to a file 
> + * @cpu: The CPU whose memory is to be dumped
> + * @cpuid: ID number of the CPU 
> + * @opaque: pointer to the CPUState struct
> + */
> +int cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
> +        int cpuid, void *opaque);
> +
> +/**
> + * cpu_write_elf64_qemunote:
> + * @f: pointer to a function that writes memory to a file 
> + * @cpu: The CPU whose memory is to be dumped
> + * @cpuid: ID number of the CPU 
> + * @opaque: pointer to the CPUState struct
> + */
> +int cpu_write_elf64_qemunote(write_core_dump_function f, CPUState *cpu,
> +        void *opaque);
> +
> +/**
> + * cpu_write_elf32_note:
> + * @f: pointer to a function that writes memory to a file 
> + * @cpu: The CPU whose memory is to be dumped
> + * @cpuid: ID number of the CPU 
> + * @opaque: pointer to the CPUState struct
> + */
> +int cpu_write_elf32_note(write_core_dump_function f, CPUState *cpu,
> +        int cpuid, void *opaque);
> +
> +/**
> + * cpu_write_elf32_qemunote:
> + * @f: pointer to a function that writes memory to a file 
> + * @cpu: The CPU whose memory is to be dumped
> + * @cpuid: ID number of the CPU 
> + * @opaque: pointer to the CPUState struct
> + */
> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUState *cpu,
> +        void *opaque);
>  
>  /**
>   * cpu_reset:
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index b03efd4..e5f9f7d 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -16,6 +16,7 @@
>  
>  #include "qapi/error.h"
>  #include "include/qapi/qmp/qerror.h"
> +#include "qom/cpu.h"
>  
>  typedef struct ArchDumpInfo {
>      int d_machine;  /* Architecture */
> @@ -24,14 +25,6 @@ typedef struct ArchDumpInfo {
>  } ArchDumpInfo;
>  
>  typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);
> -int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
> -                                                  int cpuid, void *opaque);
> -int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
> -                                                  int cpuid, void *opaque);
> -int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
> -                                                          void *opaque);
> -int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
> -                                                          void *opaque);
>  int cpu_get_dump_info(ArchDumpInfo *info);
>  ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
>  
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 0a2194d..e2822d0 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -21,6 +21,50 @@
>  #include "qom/cpu.h"
>  #include "qemu-common.h"
>  
> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUState *cpu,
> +        void *opaque)
> +{
> +    CPUClass *klass = CPU_GET_CLASS(cpu);
> +
> +    if (klass->write_elf32_qemunote != NULL) {
> +        return (*klass->write_elf32_qemunote)(f, cpu, opaque);
> +    }
> +    return -1;
> +}
> +
> +int cpu_write_elf32_note(write_core_dump_function f, CPUState *cpu,
> +        int cpuid, void *opaque)
> +{
> +    CPUClass *klass = CPU_GET_CLASS(cpu);
> +
> +    if (klass->write_elf32_note != NULL) {
> +        return (*klass->write_elf32_note)(f, cpu, cpuid, opaque);
> +    }
> +    return -1;
> +}
> +
> +int cpu_write_elf64_qemunote(write_core_dump_function f, CPUState *cpu,
> +        void *opaque)
> +{
> +    CPUClass *klass = CPU_GET_CLASS(cpu);
> +
> +    if (klass->write_elf64_note != NULL) {
> +        return (*klass->write_elf64_qemunote)(f, cpu, opaque);
> +    }
> +    return -1;
> +}
> +
> +int cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
> +        int cpuid, void *opaque)
> +{
> +    CPUClass *klass = CPU_GET_CLASS(cpu);
> +
> +    if (klass->write_elf64_note != NULL) {
> +        return (*klass->write_elf64_note)(f, cpu, cpuid, opaque);
> +    }
> +    return -1;
> +}
> +

I would rather make these four totally trivial, passing through the
return value always. See below.

>  void cpu_reset(CPUState *cpu)
>  {
>      CPUClass *klass = CPU_GET_CLASS(cpu);
> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
> index 18f2652..c594874 100644
> --- a/target-s390x/arch_dump.c
> +++ b/target-s390x/arch_dump.c
> @@ -13,6 +13,7 @@
>  
>  #include "cpu.h"
>  #include "elf.h"
> +#include "arch_dump.h"
>  #include "exec/cpu-all.h"
>  #include "sysemu/dump.h"
>  #include "sysemu/kvm.h"
> @@ -186,10 +187,11 @@ static int s390x_write_all_elf64_notes(const char *note_name, write_core_dump_fu
>  }
>  
>  
> -int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
> +int s390_cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
>                           int cpuid, void *opaque)
>  {
> -    return s390x_write_all_elf64_notes("CORE", f, env, cpuid, opaque);
> +    S390CPU *_cpu = S390_CPU(cpu);

To avoid this name conflict, please name the argument "cs" above, then
this can be just "cpu".

> +    return s390x_write_all_elf64_notes("CORE", f, &_cpu->env, cpuid, opaque);
>  }
>  
>  int cpu_get_dump_info(ArchDumpInfo *info)
> @@ -222,25 +224,6 @@ ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
>  
>  }
>  
> -int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
> -                         int cpuid, void *opaque)
> -{
> -    return 0;
> -}
> -
> -
> -int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
> -                             void *opaque)
> -{
> -    return 0;
> -}
> -
> -int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
> -                             void *opaque)
> -{
> -    return 0;
> -}
> -
>  int arch_check_parameter(bool paging, bool has_filter, int64_t begin, int64_t length, 
>                            Error **errp)
>  {
> diff --git a/target-s390x/arch_dump.h b/target-s390x/arch_dump.h
> new file mode 100644
> index 0000000..8ebc24e
> --- /dev/null
> +++ b/target-s390x/arch_dump.h
> @@ -0,0 +1,14 @@
> +/*
> + * dump guest memory implementation 
> + *
> + * Copyright 2012 IBM Corp.
> + * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +int s390_cpu_write_elf64_note(write_core_dump_function f, CPUState *cpu,
> +                         int cpuid, void *opaque);

Suggest to place this into cpu-qom.h if not much more gets added to it.

> +
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 14a9e9d..14b25d8 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -27,6 +27,7 @@
>  #include "qemu-common.h"
>  #include "qemu/timer.h"
>  #include "hw/hw.h"
> +#include "arch_dump.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/s390x/sclp.h"
>  #include "sysemu/arch_init.h"
> @@ -231,6 +232,11 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
>      scc->parent_reset = cc->reset;
>      cc->reset = s390_cpu_reset;
>  
> +    cc->write_elf64_note = s390_cpu_write_elf64_note;
> +    cc->write_elf64_qemunote = NULL;
> +    cc->write_elf32_note = NULL;
> +    cc->write_elf32_qemunote = NULL;

Suggest to avoid NULL functions by implementing dummy versions in
qom/cpu.c, which return a value compatible with its call sites. Then
s390x etc. only overwrite the functions whose functionality they wish to
override, keeping the call sites (both wrappers and their callers) simpler.

cc->paging_enabled (or so) should also get overridden here.

And quite obviously this RFC is based on top of the s390x patch whereas
I was hoping to insert this rework before, to simplify both arm and
s390x patches.

Regards,
Andreas

> +
>      dc->vmsd = &vmstate_s390_cpu;
>  }
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 0/2] qom: make cpu_write_elfXX_ functions part of CPUClass

[Jens Freimann]

Hi Andreas,

I have two patches to make dump-guest-memory related
cpu_write_elfxx_note functions members of CPUClass and pass CPUstate
as an argument instead of CPUArchState.

This is preparation for s390 patches which will follow on Monday.

Jens Freimann (2):
  qom: Convert cpu_write_elfXX_note functions to CPUState
  i386: use CPUClass->write_elf* functions

 dump-stub.c             | 28 ----------------------------
 dump.c                  |  8 ++++----
 include/qom/cpu.h       | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump.h   |  9 ---------
 qom/cpu.c               | 32 ++++++++++++++++++++++++++++++++
 target-i386/arch_dump.c | 25 +++++++++++++------------
 target-i386/cpu-qom.h   |  9 +++++++++
 target-i386/cpu.c       |  7 +++++++
 8 files changed, 114 insertions(+), 53 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/2] qom: Convert cpu_write_elfXX_note functions to CPUState

[Jens Freimann]

Convert cpu_write_elfXX_note functions to CPUClass methods and pass
CPUState as argument

Signed-off-by: "Jens Freimann <jfrei@linux.vnet.ibm.com"
---
 dump-stub.c           | 28 ----------------------------
 dump.c                |  8 ++++----
 include/qom/cpu.h     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/sysemu/dump.h |  8 --------
 qom/cpu.c             | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 85 insertions(+), 40 deletions(-)

diff --git a/dump-stub.c b/dump-stub.c
index a9d0b3c..b3f42cb 100644
--- a/dump-stub.c
+++ b/dump-stub.c
@@ -24,34 +24,6 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     error_set(errp, QERR_UNSUPPORTED);
 }
 
-int cpu_write_elf64_note(write_core_dump_function f,
-                                       CPUArchState *env, int cpuid,
-                                       void *opaque)
-{
-    return -1;
-}
-
-int cpu_write_elf32_note(write_core_dump_function f,
-                                       CPUArchState *env, int cpuid,
-                                       void *opaque)
-{
-    return -1;
-}
-
-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 -1;
-}
-
 int cpu_get_dump_info(ArchDumpInfo *info)
 {
     return -1;
diff --git a/dump.c b/dump.c
index b34f143..c0d3da5 100644
--- a/dump.c
+++ b/dump.c
@@ -282,7 +282,7 @@ static int write_elf64_notes(DumpState *s)
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
         cpu = ENV_GET_CPU(env);
         id = cpu_index(cpu);
-        ret = cpu_write_elf64_note(fd_write_vmcore, env, id, s);
+        ret = cpu_write_elf64_note(fd_write_vmcore, cpu, id, s);
         if (ret < 0) {
             dump_error(s, "dump: failed to write elf notes.\n");
             return -1;
@@ -290,7 +290,7 @@ static int write_elf64_notes(DumpState *s)
     }
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf64_qemunote(fd_write_vmcore, env, s);
+        ret = cpu_write_elf64_qemunote(fd_write_vmcore, cpu, s);
         if (ret < 0) {
             dump_error(s, "dump: failed to write CPU status.\n");
             return -1;
@@ -334,7 +334,7 @@ static int write_elf32_notes(DumpState *s)
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
         cpu = ENV_GET_CPU(env);
         id = cpu_index(cpu);
-        ret = cpu_write_elf32_note(fd_write_vmcore, env, id, s);
+        ret = cpu_write_elf32_note(fd_write_vmcore, cpu, id, s);
         if (ret < 0) {
             dump_error(s, "dump: failed to write elf notes.\n");
             return -1;
@@ -342,7 +342,7 @@ static int write_elf32_notes(DumpState *s)
     }
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf32_qemunote(fd_write_vmcore, env, s);
+        ret = cpu_write_elf32_qemunote(fd_write_vmcore, cpu, s);
         if (ret < 0) {
             dump_error(s, "dump: failed to write CPU status.\n");
             return -1;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3664a1b..f23dc22 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -24,6 +24,8 @@
 #include "hw/qdev-core.h"
 #include "qemu/thread.h"
 
+typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
+
 /**
  * SECTION:cpu
  * @section_id: QEMU-cpu
@@ -60,6 +62,14 @@ typedef struct CPUClass {
     void (*do_interrupt)(CPUState *cpu);
 
     const struct VMStateDescription *vmsd;
+    int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
+            int cpuid, void *opaque);
+    int (*write_elf64_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+            void *opaque);
+    int (*write_elf32_note)(WriteCoreDumpFunction f, CPUState *cpu,
+            int cpuid, void *opaque);
+    int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
+            void *opaque);
 } CPUClass;
 
 struct KVMState;
@@ -125,6 +135,45 @@ struct CPUState {
     uint32_t halted; /* used by alpha, cris, ppc TCG */
 };
 
+/**
+ * cpu_write_elf64_note:
+ * @f: pointer to a function that writes memory to a file
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
+        int cpuid, void *opaque);
+
+/**
+ * cpu_write_elf64_qemunote:
+ * @f: pointer to a function that writes memory to a file
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+        void *opaque);
+
+/**
+ * cpu_write_elf32_note:
+ * @f: pointer to a function that writes memory to a file
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
+        int cpuid, void *opaque);
+
+/**
+ * cpu_write_elf32_qemunote:
+ * @f: pointer to a function that writes memory to a file
+ * @cpu: The CPU whose memory is to be dumped
+ * @cpuid: ID number of the CPU
+ * @opaque: pointer to the CPUState struct
+ */
+int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+        void *opaque);
 
 /**
  * cpu_reset:
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index e25b7cf..75823e5 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -21,14 +21,6 @@ typedef struct ArchDumpInfo {
 } ArchDumpInfo;
 
 typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);
-int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
-                                                  int cpuid, void *opaque);
-int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
-                                                  int cpuid, void *opaque);
-int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
-                                                          void *opaque);
-int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
-                                                          void *opaque);
 int cpu_get_dump_info(ArchDumpInfo *info);
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
 
diff --git a/qom/cpu.c b/qom/cpu.c
index e242dcb..aacc057 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -26,6 +26,38 @@ void cpu_reset_interrupt(CPUState *cpu, int mask)
     cpu->interrupt_request &= ~mask;
 }
 
+int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+        void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    return (*klass->write_elf32_qemunote)(f, cpu, opaque);
+}
+
+int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
+        int cpuid, void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    return (*klass->write_elf32_note)(f, cpu, cpuid, opaque);
+}
+
+int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+        void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    return (*klass->write_elf64_qemunote)(f, cpu, opaque);
+}
+
+int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
+        int cpuid, void *opaque)
+{
+    CPUClass *klass = CPU_GET_CLASS(cpu);
+
+    return (*klass->write_elf64_note)(f, cpu, cpuid, opaque);
+}
+
 void cpu_reset(CPUState *cpu)
 {
     CPUClass *klass = CPU_GET_CLASS(cpu);
-- 
1.7.12.4

[Qemu-devel] [PATCH 2/2] i386: use CPUClass->write_elf* functions

[Jens Freimann]

Rework dump-guest-memory support to implement write_elfXX_note methods
of CPUClass.

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 include/sysemu/dump.h   |  1 -
 target-i386/arch_dump.c | 25 +++++++++++++------------
 target-i386/cpu-qom.h   |  9 +++++++++
 target-i386/cpu.c       |  7 +++++++
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 75823e5..b8c770f 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -20,7 +20,6 @@ typedef struct ArchDumpInfo {
     int d_class;    /* ELFCLASS32 or ELFCLASS64 */
 } ArchDumpInfo;
 
-typedef int (*write_core_dump_function)(void *buf, size_t size, void *opaque);
 int cpu_get_dump_info(ArchDumpInfo *info);
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
 
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 2cd2f7f..dfd23cd 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -15,6 +15,7 @@
 #include "exec/cpu-all.h"
 #include "sysemu/dump.h"
 #include "elf.h"
+#include "qom/cpu.h"
 
 #ifdef TARGET_X86_64
 typedef struct {
@@ -34,7 +35,7 @@ typedef struct {
     char pad3[8];
 } x86_64_elf_prstatus;
 
-static int x86_64_write_elf64_note(write_core_dump_function f,
+static int x86_64_write_elf64_note(WriteCoreDumpFunction f,
                                    CPUArchState *env, int id,
                                    void *opaque)
 {
@@ -144,7 +145,7 @@ static void x86_fill_elf_prstatus(x86_elf_prstatus *prstatus, CPUArchState *env,
     prstatus->pid = id;
 }
 
-static int x86_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+static int x86_write_elf64_note(WriteCoreDumpFunction f, CPUArchState *env,
                                 int id, void *opaque)
 {
     x86_elf_prstatus prstatus;
@@ -179,7 +180,7 @@ static int x86_write_elf64_note(write_core_dump_function f, CPUArchState *env,
     return 0;
 }
 
-int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+int x86_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
                          int cpuid, void *opaque)
 {
     int ret;
@@ -187,10 +188,10 @@ int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
     bool lma = !!(first_cpu->hflags & HF_LMA_MASK);
 
     if (lma) {
-        ret = x86_64_write_elf64_note(f, env, cpuid, opaque);
+        ret = x86_64_write_elf64_note(f, cpu->env_ptr, cpuid, opaque);
     } else {
 #endif
-        ret = x86_write_elf64_note(f, env, cpuid, opaque);
+        ret = x86_write_elf64_note(f, cpu->env_ptr, cpuid, opaque);
 #ifdef TARGET_X86_64
     }
 #endif
@@ -198,7 +199,7 @@ int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
     return ret;
 }
 
-int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
+int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
                          int cpuid, void *opaque)
 {
     x86_elf_prstatus prstatus;
@@ -208,7 +209,7 @@ int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
     const char *name = "CORE";
     int ret;
 
-    x86_fill_elf_prstatus(&prstatus, env, cpuid);
+    x86_fill_elf_prstatus(&prstatus, cpu->env_ptr, cpuid);
     descsz = sizeof(x86_elf_prstatus);
     note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
                 (descsz + 3) / 4) * 4;
@@ -317,7 +318,7 @@ static void qemu_get_cpustate(QEMUCPUState *s, CPUArchState *env)
     s->cr[4] = env->cr[4];
 }
 
-static inline int cpu_write_qemu_note(write_core_dump_function f,
+static inline int cpu_write_qemu_note(WriteCoreDumpFunction f,
                                       CPUArchState *env,
                                       void *opaque,
                                       int type)
@@ -370,16 +371,16 @@ static inline int cpu_write_qemu_note(write_core_dump_function f,
     return 0;
 }
 
-int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
+int x86_cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
                              void *opaque)
 {
-    return cpu_write_qemu_note(f, env, opaque, 1);
+    return cpu_write_qemu_note(f, cpu->env_ptr, opaque, 1);
 }
 
-int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
+int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
                              void *opaque)
 {
-    return cpu_write_qemu_note(f, env, opaque, 0);
+    return cpu_write_qemu_note(f, cpu->env_ptr, opaque, 0);
 }
 
 int cpu_get_dump_info(ArchDumpInfo *info)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 08f9eb6..d575b8b 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -86,4 +86,13 @@ extern const struct VMStateDescription vmstate_x86_cpu;
  */
 void x86_cpu_do_interrupt(CPUState *cpu);
 
+int x86_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
+                         int cpuid, void *opaque);
+int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
+                         int cpuid, void *opaque);
+int x86_cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+                             void *opaque);
+int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
+                             void *opaque);
+
 #endif
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e2302d8..8e4e7a3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -30,6 +30,7 @@
 #include "qemu/config-file.h"
 #include "qapi/qmp/qerror.h"
 
+#include "cpu-qom.h"
 #include "qapi/visitor.h"
 #include "sysemu/arch_init.h"
 
@@ -2285,6 +2286,12 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->reset = x86_cpu_reset;
 
     cc->do_interrupt = x86_cpu_do_interrupt;
+#ifndef CONFIG_USER_ONLY
+    cc->write_elf64_note = x86_cpu_write_elf64_note;
+    cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
+    cc->write_elf32_note = x86_cpu_write_elf32_note;
+    cc->write_elf32_qemunote = x86_cpu_write_elf32_qemunote;
+#endif
     cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
 }
 
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 0/2] qom: make cpu_write_elfXX_ functions part of CPUClass

[Andreas Färber]

Am 19.04.2013 16:45, schrieb Jens Freimann:
> Hi Andreas,
> 
> I have two patches to make dump-guest-memory related
> cpu_write_elfxx_note functions members of CPUClass and pass CPUstate
> as an argument instead of CPUArchState.
> 
> This is preparation for s390 patches which will follow on Monday.
> 
> Jens Freimann (2):
>   qom: Convert cpu_write_elfXX_note functions to CPUState
>   i386: use CPUClass->write_elf* functions

Thanks, squashed and applied to qom-cpu (with changes below):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

* squashed both for bisectability
* fixed argument indentation
* s/klass/cc/g in qom/cpu.c; TODO: class_init still uses k
* dropped including cpu-qom.h in target-i386/cpu.c
* dropped including qom/cpu.h in target-i386/arch_dump.c
* retained stubs from dump-stub.c in qom/cpu.c
* target-i386: reserve "cpu" for X86CPU and avoid env_ptr usage

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg