[Qemu-devel] [PATCH 0/5] target-arm: enable qmp-dump-guest-memory

[Qemu-devel] [PATCH 0/5] target-arm: enable qmp-dump-guest-memory

[Andrew Jones]

This series brings qmp-dump-guest-memory to arm and aarch64
targets. I've detailed my testing and the results in the
following table.

arm/aarch64 kvm guest kdump testing (P - PASS, F - FAIL). Testing done
with a latest mainline crash utility patched with [*]

.-----------------------------------------------------------------------.
|				Host	| arm32	| arm64	| arm64	| arm64	|
|---------------------------------------|-------|-------|-------|-------|
|				Guest	| arm32	| arm64	| arm64	| arm32	|
|---------------------------------------|-------|-------|-------|-------|
|				Pagesize| 4K	| 4K	| 64K	| 4K	|
|=======================================================================|
| kdump in guest			| F[1]	| P[2]	| P[3]	| F[1]	|
|---------------------------------------|-------|-------|-------|-------|
| qmp-dump-guest-memory <filename>[4]	| P	| P	| P	| P	|
|---------------------------------------|-------|-------|-------|-------|
| qmp-dump-guest-memory -z <filename>[5]| F[8]	| P	| P	| F[8]	|
|---------------------------------------|-------|-------|-------|-------|
| qmp-dump-guest-memory -l <filename>[6]| F[8]	| P	| P	| F[8]	|
|---------------------------------------|-------|-------|-------|-------|
| qmp-dump-guest-memory -s <filename>[7]| F[8]	| P	| P	| F[8]	|
.-----------------------------------------------------------------------.

[1] Kernel v4.4-rc1 crashes with a NULL pointer dereference at virtual
    address 00000000 in a memcpy (crash_kexec/machine_kexec/fncpy/memcpy).
    Needs kernel debugging.
[2] Not sure about mainline, but works with the RHEL kernel,
    makedumpfile does not yet support arm64 with 4K pages, but using
    'core_collector cp' in /etc/kdump.conf allows saving an uncompressed
    elf file.
[3] Not sure about mainline, but works with the RHEL kernel,
    uses makedumpfile, thus generates a makedumpfile formatted file
    using zlib compression.
[4] No format specified, creates an uncompressed elf formatted file.
[5] makedumpfile format, with zlib compression
[6] makedumpfile format, with lzo compression
[7] makedumpfile format, with snappy compression
[8] The crash utility doesn't seem to like arm32 dumps in makedumpfile
    format. Looks like the physical page bitmap is all zeros? Needs
    qemu and crash debugging.

Additional notes:
1) QEMU also has scripts/dump-guest-memory.py, which can and should be
   updated to support multiple architectures, pagesizes, and physbases.
   This is currently left as future work.

[*] https://www.redhat.com/archives/crash-utility/2015-November/msg00031.html

Andrew Jones (5):
  qapi-schema: dump-guest-memory: Improve text
  dump: qemunotes aren't commonly needed
  dump: allow target to set the page size
  dump: allow target to set the phys_base
  target-arm: support QMP dump-guest-memory

 dump.c                      | 129 +++++++++++++++----------
 include/sysemu/dump-arch.h  |   9 +-
 include/sysemu/dump.h       |  11 +--
 qapi-schema.json            |   4 +-
 qom/cpu.c                   |   4 +-
 target-arm/Makefile.objs    |   3 +-
 target-arm/arch_dump.c      | 222 ++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu-qom.h        |   5 +
 target-arm/cpu.c            |   3 +
 target-ppc/arch_dump.c      |   6 --
 target-ppc/cpu-qom.h        |   2 -
 target-ppc/translate_init.c |   1 -
 target-s390x/arch_dump.c    |   6 --
 target-s390x/cpu-qom.h      |   2 -
 target-s390x/cpu.c          |   1 -
 15 files changed, 321 insertions(+), 87 deletions(-)
 create mode 100644 target-arm/arch_dump.c

-- 
2.4.3

[Qemu-devel] [PATCH 1/5] qapi-schema: dump-guest-memory: Improve text

[Andrew Jones]

dump-guest-memory is supported by more than just x86, however
the paging option is not.

(No functional change.)

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 qapi-schema.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423fa7973..ebbad01c40948 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2089,8 +2089,7 @@
 # @dump-guest-memory
 #
 # Dump guest's memory to vmcore. It is a synchronous operation that can take
-# very long depending on the amount of guest memory. This command is only
-# supported on i386 and x86_64.
+# very long depending on the amount of guest memory.
 #
 # @paging: if true, do paging to get guest's memory mapping. This allows
 #          using gdb to process the core file.
@@ -2106,6 +2105,7 @@
 #             2. The guest can be in real-mode even if paging is enabled. For
 #                example, the guest uses ACPI to sleep, and ACPI sleep state
 #                goes in real-mode
+#             3. Currently only supported on i386 and x86_64.
 #
 # @protocol: the filename or file descriptor of the vmcore. The supported
 #            protocols are:
-- 
2.4.3

[Qemu-devel] [PATCH 2/5] dump: qemunotes aren't commonly needed

[Andrew Jones]

Only one of three architectures implementing qmp-dump-guest-memory write
qemu notes. And, another architecture (arm/aarch64) is coming, which
won't use them either. Make the common implementation truly common.

(No functional change.)

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 qom/cpu.c                   | 4 ++--
 target-ppc/arch_dump.c      | 6 ------
 target-ppc/cpu-qom.h        | 2 --
 target-ppc/translate_init.c | 1 -
 target-s390x/arch_dump.c    | 6 ------
 target-s390x/cpu-qom.h      | 2 --
 target-s390x/cpu.c          | 1 -
 7 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index fb80d13a3f491..8f537a470f3c4 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -130,7 +130,7 @@ int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
 static int cpu_common_write_elf32_qemunote(WriteCoreDumpFunction f,
                                            CPUState *cpu, void *opaque)
 {
-    return -1;
+    return 0;
 }
 
 int cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cpu,
@@ -159,7 +159,7 @@ int cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
 static int cpu_common_write_elf64_qemunote(WriteCoreDumpFunction f,
                                            CPUState *cpu, void *opaque)
 {
-    return -1;
+    return 0;
 }
 
 int cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
index 5acafc68a4e39..816dbc21340d9 100644
--- a/target-ppc/arch_dump.c
+++ b/target-ppc/arch_dump.c
@@ -278,9 +278,3 @@ int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     return ppc64_write_all_elf64_notes("CORE", f, cpu, cpuid, opaque);
 }
-
-int ppc64_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
-                                   CPUState *cpu, void *opaque)
-{
-    return 0;
-}
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index bc20504b3d55a..7d5e2b36a997c 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -125,8 +125,6 @@ int ppc_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_read_register_apple(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg);
-int ppc64_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
-                                   CPUState *cpu, void *opaque);
 int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
                                int cpuid, void *opaque);
 #ifndef CONFIG_USER_ONLY
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index e88dc7fc7aa38..4ab2d927b01bb 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9712,7 +9712,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->vmsd = &vmstate_ppc_cpu;
 #if defined(TARGET_PPC64)
     cc->write_elf64_note = ppc64_cpu_write_elf64_note;
-    cc->write_elf64_qemunote = ppc64_cpu_write_elf64_qemunote;
 #endif
 #endif
     cc->cpu_exec_enter = ppc_cpu_exec_enter;
diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
index dab63eb44f270..549a99b357e3f 100644
--- a/target-s390x/arch_dump.c
+++ b/target-s390x/arch_dump.c
@@ -246,9 +246,3 @@ ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 
     return (elf_note_size) * nr_cpus;
 }
-
-int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
-                                  CPUState *cpu, void *opaque)
-{
-    return 0;
-}
diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
index 491c1b8769669..029a44af246cc 100644
--- a/target-s390x/cpu-qom.h
+++ b/target-s390x/cpu-qom.h
@@ -91,8 +91,6 @@ void s390_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
 int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
                               int cpuid, void *opaque);
 
-int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
-                                  CPUState *cpu, void *opaque);
 hwaddr s390_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 hwaddr s390_cpu_get_phys_addr_debug(CPUState *cpu, vaddr addr);
 int s390_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 189a2afc0fe1f..e5a3f65029075 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -353,7 +353,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
     cc->vmsd = &vmstate_s390_cpu;
     cc->write_elf64_note = s390_cpu_write_elf64_note;
-    cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote;
     cc->cpu_exec_interrupt = s390_cpu_exec_interrupt;
     cc->debug_excp_handler = s390x_cpu_debug_excp_handler;
 #endif
-- 
2.4.3

[Qemu-devel] [PATCH 3/5] dump: allow target to set the page size

[Andrew Jones]

This is necessary for targets that don't have TARGET_PAGE_SIZE ==
real-target-page-size. The target should set the page size to the
correct one, if known, or, if not known, to the maximum page size
it supports.

(No functional change.)

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 dump.c                     | 127 ++++++++++++++++++++++++++++-----------------
 include/sysemu/dump-arch.h |   8 +--
 include/sysemu/dump.h      |  10 +---
 3 files changed, 85 insertions(+), 60 deletions(-)

diff --git a/dump.c b/dump.c
index 78b7d843ceb29..e1d9bae9e89d1 100644
--- a/dump.c
+++ b/dump.c
@@ -347,18 +347,18 @@ static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t start,
     int64_t i;
     Error *local_err = NULL;
 
-    for (i = 0; i < size / TARGET_PAGE_SIZE; i++) {
-        write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
-                   TARGET_PAGE_SIZE, &local_err);
+    for (i = 0; i < size / s->dump_info.page_size; i++) {
+        write_data(s, block->host_addr + start + i * s->dump_info.page_size,
+                   s->dump_info.page_size, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             return;
         }
     }
 
-    if ((size % TARGET_PAGE_SIZE) != 0) {
-        write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
-                   size % TARGET_PAGE_SIZE, &local_err);
+    if ((size % s->dump_info.page_size) != 0) {
+        write_data(s, block->host_addr + start + i * s->dump_info.page_size,
+                   size % s->dump_info.page_size, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             return;
@@ -737,7 +737,7 @@ static void create_header32(DumpState *s, Error **errp)
 
     strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
     dh->header_version = cpu_to_dump32(s, 6);
-    block_size = TARGET_PAGE_SIZE;
+    block_size = s->dump_info.page_size;
     dh->block_size = cpu_to_dump32(s, block_size);
     sub_hdr_size = sizeof(struct KdumpSubHeader32) + s->note_size;
     sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
@@ -837,7 +837,7 @@ static void create_header64(DumpState *s, Error **errp)
 
     strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
     dh->header_version = cpu_to_dump32(s, 6);
-    block_size = TARGET_PAGE_SIZE;
+    block_size = s->dump_info.page_size;
     dh->block_size = cpu_to_dump32(s, block_size);
     sub_hdr_size = sizeof(struct KdumpSubHeader64) + s->note_size;
     sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
@@ -933,6 +933,11 @@ static void write_dump_header(DumpState *s, Error **errp)
     }
 }
 
+static size_t dump_bitmap_get_bufsize(DumpState *s)
+{
+    return s->dump_info.page_size;
+}
+
 /*
  * set dump_bitmap sequencely. the bit before last_pfn is not allowed to be
  * rewritten, so if need to set the first bit, set last_pfn and pfn to 0.
@@ -946,6 +951,8 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
     off_t old_offset, new_offset;
     off_t offset_bitmap1, offset_bitmap2;
     uint32_t byte, bit;
+    size_t bitmap_bufsize = dump_bitmap_get_bufsize(s);
+    size_t bits_per_buf = bitmap_bufsize * CHAR_BIT;
 
     /* should not set the previous place */
     assert(last_pfn <= pfn);
@@ -956,14 +963,14 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
      * making new_offset be bigger than old_offset can also sync remained data
      * into vmcore.
      */
-    old_offset = BUFSIZE_BITMAP * (last_pfn / PFN_BUFBITMAP);
-    new_offset = BUFSIZE_BITMAP * (pfn / PFN_BUFBITMAP);
+    old_offset = bitmap_bufsize * (last_pfn / bits_per_buf);
+    new_offset = bitmap_bufsize * (pfn / bits_per_buf);
 
     while (old_offset < new_offset) {
         /* calculate the offset and write dump_bitmap */
         offset_bitmap1 = s->offset_dump_bitmap + old_offset;
         if (write_buffer(s->fd, offset_bitmap1, buf,
-                         BUFSIZE_BITMAP) < 0) {
+                         bitmap_bufsize) < 0) {
             return -1;
         }
 
@@ -971,17 +978,17 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
         offset_bitmap2 = s->offset_dump_bitmap + s->len_dump_bitmap +
                          old_offset;
         if (write_buffer(s->fd, offset_bitmap2, buf,
-                         BUFSIZE_BITMAP) < 0) {
+                         bitmap_bufsize) < 0) {
             return -1;
         }
 
-        memset(buf, 0, BUFSIZE_BITMAP);
-        old_offset += BUFSIZE_BITMAP;
+        memset(buf, 0, bitmap_bufsize);
+        old_offset += bitmap_bufsize;
     }
 
     /* get the exact place of the bit in the buf, and set it */
-    byte = (pfn % PFN_BUFBITMAP) / CHAR_BIT;
-    bit = (pfn % PFN_BUFBITMAP) % CHAR_BIT;
+    byte = (pfn % bits_per_buf) / CHAR_BIT;
+    bit = (pfn % bits_per_buf) % CHAR_BIT;
     if (value) {
         buf[byte] |= 1u << bit;
     } else {
@@ -991,6 +998,20 @@ static int set_dump_bitmap(uint64_t last_pfn, uint64_t pfn, bool value,
     return 0;
 }
 
+static uint64_t dump_paddr_to_pfn(DumpState *s, uint64_t addr)
+{
+    int target_page_shift = ctz32(s->dump_info.page_size);
+
+    return (addr >> target_page_shift) - ARCH_PFN_OFFSET;
+}
+
+static uint64_t dump_pfn_to_paddr(DumpState *s, uint64_t pfn)
+{
+    int target_page_shift = ctz32(s->dump_info.page_size);
+
+    return (pfn + ARCH_PFN_OFFSET) << target_page_shift;
+}
+
 /*
  * exam every page and return the page frame number and the address of the page.
  * bufptr can be NULL. note: the blocks here is supposed to reflect guest-phys
@@ -1001,16 +1022,16 @@ static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
                           uint8_t **bufptr, DumpState *s)
 {
     GuestPhysBlock *block = *blockptr;
-    hwaddr addr;
+    hwaddr addr, target_page_mask = ~((hwaddr)s->dump_info.page_size - 1);
     uint8_t *buf;
 
     /* block == NULL means the start of the iteration */
     if (!block) {
         block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
         *blockptr = block;
-        assert((block->target_start & ~TARGET_PAGE_MASK) == 0);
-        assert((block->target_end & ~TARGET_PAGE_MASK) == 0);
-        *pfnptr = paddr_to_pfn(block->target_start);
+        assert((block->target_start & ~target_page_mask) == 0);
+        assert((block->target_end & ~target_page_mask) == 0);
+        *pfnptr = dump_paddr_to_pfn(s, block->target_start);
         if (bufptr) {
             *bufptr = block->host_addr;
         }
@@ -1018,10 +1039,10 @@ static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
     }
 
     *pfnptr = *pfnptr + 1;
-    addr = pfn_to_paddr(*pfnptr);
+    addr = dump_pfn_to_paddr(s, *pfnptr);
 
     if ((addr >= block->target_start) &&
-        (addr + TARGET_PAGE_SIZE <= block->target_end)) {
+        (addr + s->dump_info.page_size <= block->target_end)) {
         buf = block->host_addr + (addr - block->target_start);
     } else {
         /* the next page is in the next block */
@@ -1030,9 +1051,9 @@ static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
         if (!block) {
             return false;
         }
-        assert((block->target_start & ~TARGET_PAGE_MASK) == 0);
-        assert((block->target_end & ~TARGET_PAGE_MASK) == 0);
-        *pfnptr = paddr_to_pfn(block->target_start);
+        assert((block->target_start & ~target_page_mask) == 0);
+        assert((block->target_end & ~target_page_mask) == 0);
+        *pfnptr = dump_paddr_to_pfn(s, block->target_start);
         buf = block->host_addr;
     }
 
@@ -1050,9 +1071,11 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
     void *dump_bitmap_buf;
     size_t num_dumpable;
     GuestPhysBlock *block_iter = NULL;
+    size_t bitmap_bufsize = dump_bitmap_get_bufsize(s);
+    size_t bits_per_buf = bitmap_bufsize * CHAR_BIT;
 
     /* dump_bitmap_buf is used to store dump_bitmap temporarily */
-    dump_bitmap_buf = g_malloc0(BUFSIZE_BITMAP);
+    dump_bitmap_buf = g_malloc0(bitmap_bufsize);
 
     num_dumpable = 0;
     last_pfn = 0;
@@ -1074,11 +1097,11 @@ static void write_dump_bitmap(DumpState *s, Error **errp)
 
     /*
      * set_dump_bitmap will always leave the recently set bit un-sync. Here we
-     * set last_pfn + PFN_BUFBITMAP to 0 and those set but un-sync bit will be
-     * synchronized into vmcore.
+     * set the remaining bits from last_pfn to the end of the bitmap buffer to
+     * 0. With those set, the un-sync bit will be synchronized into the vmcore.
      */
     if (num_dumpable > 0) {
-        ret = set_dump_bitmap(last_pfn, last_pfn + PFN_BUFBITMAP, false,
+        ret = set_dump_bitmap(last_pfn, last_pfn + bits_per_buf, false,
                               dump_bitmap_buf, s);
         if (ret < 0) {
             dump_error(s, "dump: failed to sync dump_bitmap", errp);
@@ -1098,8 +1121,8 @@ static void prepare_data_cache(DataCache *data_cache, DumpState *s,
 {
     data_cache->fd = s->fd;
     data_cache->data_size = 0;
-    data_cache->buf_size = BUFSIZE_DATA_CACHE;
-    data_cache->buf = g_malloc0(BUFSIZE_DATA_CACHE);
+    data_cache->buf_size = 4 * dump_bitmap_get_bufsize(s);
+    data_cache->buf = g_malloc0(data_cache->buf_size);
     data_cache->offset = offset;
 }
 
@@ -1193,7 +1216,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
     prepare_data_cache(&page_data, s, offset_data);
 
     /* prepare buffer to store compressed data */
-    len_buf_out = get_len_buf_out(TARGET_PAGE_SIZE, s->flag_compress);
+    len_buf_out = get_len_buf_out(s->dump_info.page_size, s->flag_compress);
     assert(len_buf_out != 0);
 
 #ifdef CONFIG_LZO
@@ -1206,19 +1229,19 @@ static void write_dump_pages(DumpState *s, Error **errp)
      * init zero page's page_desc and page_data, because every zero page
      * uses the same page_data
      */
-    pd_zero.size = cpu_to_dump32(s, TARGET_PAGE_SIZE);
+    pd_zero.size = cpu_to_dump32(s, s->dump_info.page_size);
     pd_zero.flags = cpu_to_dump32(s, 0);
     pd_zero.offset = cpu_to_dump64(s, offset_data);
     pd_zero.page_flags = cpu_to_dump64(s, 0);
-    buf = g_malloc0(TARGET_PAGE_SIZE);
-    ret = write_cache(&page_data, buf, TARGET_PAGE_SIZE, false);
+    buf = g_malloc0(s->dump_info.page_size);
+    ret = write_cache(&page_data, buf, s->dump_info.page_size, false);
     g_free(buf);
     if (ret < 0) {
         dump_error(s, "dump: failed to write page data (zero page)", errp);
         goto out;
     }
 
-    offset_data += TARGET_PAGE_SIZE;
+    offset_data += s->dump_info.page_size;
 
     /*
      * dump memory to vmcore page by page. zero page will all be resided in the
@@ -1226,7 +1249,7 @@ static void write_dump_pages(DumpState *s, Error **errp)
      */
     while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
         /* check zero page */
-        if (is_zero_page(buf, TARGET_PAGE_SIZE)) {
+        if (is_zero_page(buf, s->dump_info.page_size)) {
             ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
                               false);
             if (ret < 0) {
@@ -1248,8 +1271,8 @@ static void write_dump_pages(DumpState *s, Error **errp)
              size_out = len_buf_out;
              if ((s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) &&
                     (compress2(buf_out, (uLongf *)&size_out, buf,
-                               TARGET_PAGE_SIZE, Z_BEST_SPEED) == Z_OK) &&
-                    (size_out < TARGET_PAGE_SIZE)) {
+                               s->dump_info.page_size, Z_BEST_SPEED) == Z_OK) &&
+                    (size_out < s->dump_info.page_size)) {
                 pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_ZLIB);
                 pd.size  = cpu_to_dump32(s, size_out);
 
@@ -1260,9 +1283,9 @@ static void write_dump_pages(DumpState *s, Error **errp)
                 }
 #ifdef CONFIG_LZO
             } else if ((s->flag_compress & DUMP_DH_COMPRESSED_LZO) &&
-                    (lzo1x_1_compress(buf, TARGET_PAGE_SIZE, buf_out,
+                    (lzo1x_1_compress(buf, s->dump_info.page_size, buf_out,
                     (lzo_uint *)&size_out, wrkmem) == LZO_E_OK) &&
-                    (size_out < TARGET_PAGE_SIZE)) {
+                    (size_out < s->dump_info.page_size)) {
                 pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_LZO);
                 pd.size  = cpu_to_dump32(s, size_out);
 
@@ -1274,9 +1297,9 @@ static void write_dump_pages(DumpState *s, Error **errp)
 #endif
 #ifdef CONFIG_SNAPPY
             } else if ((s->flag_compress & DUMP_DH_COMPRESSED_SNAPPY) &&
-                    (snappy_compress((char *)buf, TARGET_PAGE_SIZE,
+                    (snappy_compress((char *)buf, s->dump_info.page_size,
                     (char *)buf_out, &size_out) == SNAPPY_OK) &&
-                    (size_out < TARGET_PAGE_SIZE)) {
+                    (size_out < s->dump_info.page_size)) {
                 pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_SNAPPY);
                 pd.size  = cpu_to_dump32(s, size_out);
 
@@ -1289,13 +1312,14 @@ static void write_dump_pages(DumpState *s, Error **errp)
             } else {
                 /*
                  * fall back to save in plaintext, size_out should be
-                 * assigned TARGET_PAGE_SIZE
+                 * assigned the target's page size
                  */
                 pd.flags = cpu_to_dump32(s, 0);
-                size_out = TARGET_PAGE_SIZE;
+                size_out = s->dump_info.page_size;
                 pd.size = cpu_to_dump32(s, size_out);
 
-                ret = write_cache(&page_data, buf, TARGET_PAGE_SIZE, false);
+                ret = write_cache(&page_data, buf,
+                                  s->dump_info.page_size, false);
                 if (ret < 0) {
                     dump_error(s, "dump: failed to write page data", errp);
                     goto out;
@@ -1430,7 +1454,7 @@ static void get_max_mapnr(DumpState *s)
     GuestPhysBlock *last_block;
 
     last_block = QTAILQ_LAST(&s->guest_phys_blocks.head, GuestPhysBlockHead);
-    s->max_mapnr = paddr_to_pfn(last_block->target_end);
+    s->max_mapnr = dump_paddr_to_pfn(s, last_block->target_end);
 }
 
 static void dump_init(DumpState *s, int fd, bool has_format,
@@ -1489,6 +1513,10 @@ static void dump_init(DumpState *s, int fd, bool has_format,
         goto cleanup;
     }
 
+    if (!s->dump_info.page_size) {
+        s->dump_info.page_size = TARGET_PAGE_SIZE;
+    }
+
     s->note_size = cpu_get_note_size(s->dump_info.d_class,
                                      s->dump_info.d_machine, nr_cpus);
     if (s->note_size < 0) {
@@ -1512,8 +1540,9 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     get_max_mapnr(s);
 
     uint64_t tmp;
-    tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT), TARGET_PAGE_SIZE);
-    s->len_dump_bitmap = tmp * TARGET_PAGE_SIZE;
+    tmp = DIV_ROUND_UP(DIV_ROUND_UP(s->max_mapnr, CHAR_BIT),
+                       s->dump_info.page_size);
+    s->len_dump_bitmap = tmp * s->dump_info.page_size;
 
     /* init for kdump-compressed format */
     if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
diff --git a/include/sysemu/dump-arch.h b/include/sysemu/dump-arch.h
index 9c95cede3d3b7..43358396ea81a 100644
--- a/include/sysemu/dump-arch.h
+++ b/include/sysemu/dump-arch.h
@@ -15,9 +15,11 @@
 #define DUMP_ARCH_H
 
 typedef struct ArchDumpInfo {
-    int d_machine;  /* Architecture */
-    int d_endian;   /* ELFDATA2LSB or ELFDATA2MSB */
-    int d_class;    /* ELFCLASS32 or ELFCLASS64 */
+    int d_machine;           /* Architecture */
+    int d_endian;            /* ELFDATA2LSB or ELFDATA2MSB */
+    int d_class;             /* ELFCLASS32 or ELFCLASS64 */
+    uint32_t page_size;      /* The target's page size. If it's variable and
+                              * unknown, then this should be the maximum. */
 } ArchDumpInfo;
 
 struct GuestPhysBlockList; /* memory_mapping.h */
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 7e4ec5c7d96fb..16cbd8d881fd6 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -20,12 +20,9 @@
 #define VERSION_FLAT_HEADER         (1)    /* version of flattened format */
 #define END_FLAG_FLAT_HEADER        (-1)
 
+#ifndef ARCH_PFN_OFFSET
 #define ARCH_PFN_OFFSET             (0)
-
-#define paddr_to_pfn(X) \
-    (((unsigned long long)(X) >> TARGET_PAGE_BITS) - ARCH_PFN_OFFSET)
-#define pfn_to_paddr(X) \
-    (((unsigned long long)(X) + ARCH_PFN_OFFSET) << TARGET_PAGE_BITS)
+#endif
 
 /*
  * flag for compressed format
@@ -39,9 +36,6 @@
 #define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
 #define DISKDUMP_HEADER_BLOCKS      (1)
-#define BUFSIZE_BITMAP              (TARGET_PAGE_SIZE)
-#define PFN_BUFBITMAP               (CHAR_BIT * BUFSIZE_BITMAP)
-#define BUFSIZE_DATA_CACHE          (TARGET_PAGE_SIZE * 4)
 
 #include "sysemu/dump-arch.h"
 #include "sysemu/memory_mapping.h"
-- 
2.4.3

[Qemu-devel] [PATCH 4/5] dump: allow target to set the physical base

[Andrew Jones]

crash assumes the physical base in the kdump subheader of
makedumpfile formatted dumps is correct. Zero is not correct
for all architectures, so allow it to be changed.

(No functional change.)

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 dump.c                     | 4 ++--
 include/sysemu/dump-arch.h | 1 +
 include/sysemu/dump.h      | 1 -
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/dump.c b/dump.c
index e1d9bae9e89d1..2d4892bec2672 100644
--- a/dump.c
+++ b/dump.c
@@ -775,7 +775,7 @@ static void create_header32(DumpState *s, Error **errp)
 
     /* 64bit max_mapnr_64 */
     kh->max_mapnr_64 = cpu_to_dump64(s, s->max_mapnr);
-    kh->phys_base = cpu_to_dump32(s, PHYS_BASE);
+    kh->phys_base = cpu_to_dump32(s, s->dump_info.phys_base);
     kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
@@ -875,7 +875,7 @@ static void create_header64(DumpState *s, Error **errp)
 
     /* 64bit max_mapnr_64 */
     kh->max_mapnr_64 = cpu_to_dump64(s, s->max_mapnr);
-    kh->phys_base = cpu_to_dump64(s, PHYS_BASE);
+    kh->phys_base = cpu_to_dump64(s, s->dump_info.phys_base);
     kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
diff --git a/include/sysemu/dump-arch.h b/include/sysemu/dump-arch.h
index 43358396ea81a..e25b02e99013c 100644
--- a/include/sysemu/dump-arch.h
+++ b/include/sysemu/dump-arch.h
@@ -20,6 +20,7 @@ typedef struct ArchDumpInfo {
     int d_class;             /* ELFCLASS32 or ELFCLASS64 */
     uint32_t page_size;      /* The target's page size. If it's variable and
                               * unknown, then this should be the maximum. */
+    uint64_t phys_base;      /* The target's physmem base. */
 } ArchDumpInfo;
 
 struct GuestPhysBlockList; /* memory_mapping.h */
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 16cbd8d881fd6..2f04b247bed1b 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -33,7 +33,6 @@
 
 #define KDUMP_SIGNATURE             "KDUMP   "
 #define SIG_LEN                     (sizeof(KDUMP_SIGNATURE) - 1)
-#define PHYS_BASE                   (0)
 #define DUMP_LEVEL                  (1)
 #define DISKDUMP_HEADER_BLOCKS      (1)
 
-- 
2.4.3

[Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones]

Add the support needed for creating prstatus elf notes. This
allows us to use QMP dump-guest-memory.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target-arm/Makefile.objs |   3 +-
 target-arm/arch_dump.c   | 222 +++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu-qom.h     |   5 ++
 target-arm/cpu.c         |   3 +
 4 files changed, 231 insertions(+), 2 deletions(-)
 create mode 100644 target-arm/arch_dump.c

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 9460b409a5a1c..a80eb39743a78 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 psci.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
 obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
@@ -7,6 +7,5 @@ obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += neon_helper.o iwmmxt_helper.o
 obj-y += gdbstub.o
-obj-$(CONFIG_SOFTMMU) += psci.o
 obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
 obj-y += crypto_helper.o
diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c
new file mode 100644
index 0000000000000..5822204e85c72
--- /dev/null
+++ b/target-arm/arch_dump.c
@@ -0,0 +1,222 @@
+/* Support for writing ELF notes for ARM architectures
+ *
+ * Copyright (C) 2015 Red Hat Inc.
+ *
+ * Author: Andrew Jones <drjones@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "cpu.h"
+#include "elf.h"
+#include "sysemu/dump.h"
+
+#define NOTE_NAME       "CORE"
+#define NOTE_NAMESZ     5
+
+struct aarch64_user_regs {
+    uint64_t regs[31];
+    uint64_t sp;
+    uint64_t pc;
+    uint64_t pstate;
+} QEMU_PACKED;
+
+QEMU_BUILD_BUG_ON(sizeof(struct aarch64_user_regs) != 272);
+
+struct aarch64_elf_prstatus {
+    char pad1[32]; /* 32 == offsetof(struct elf_prstatus, pr_pid) */
+    uint32_t pr_pid;
+    char pad2[76]; /* 76 == offsetof(struct elf_prstatus, pr_reg) -
+                            offsetof(struct elf_prstatus, pr_ppid) */
+    struct aarch64_user_regs pr_reg;
+    int pr_fpvalid;
+    char pad3[4];
+} QEMU_PACKED;
+
+QEMU_BUILD_BUG_ON(sizeof(struct aarch64_elf_prstatus) != 392);
+
+struct aarch64_note {
+    Elf64_Nhdr hdr;
+    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
+    struct aarch64_elf_prstatus prstatus;
+} QEMU_PACKED;
+
+QEMU_BUILD_BUG_ON(sizeof(struct aarch64_note) != 412);
+
+static int
+aarch64_write_elf64_note(WriteCoreDumpFunction f, CPUARMState *env,
+                         int id, DumpState *s)
+{
+    struct aarch64_note note;
+    int ret, i;
+
+    memset(&note, 0, sizeof(note));
+
+    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
+    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
+    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
+
+    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
+    note.prstatus.pr_pid = cpu_to_dump32(s, id);
+
+    for (i = 0; i < 31; ++i) {
+        note.prstatus.pr_reg.regs[i] = cpu_to_dump64(s, env->xregs[i]);
+    }
+    note.prstatus.pr_reg.sp = cpu_to_dump64(s, env->xregs[31]);
+    note.prstatus.pr_reg.pc = cpu_to_dump64(s, env->pc);
+    note.prstatus.pr_reg.pstate = cpu_to_dump64(s, pstate_read(env));
+
+    ret = f(&note, sizeof(note), s);
+    if (ret < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+struct arm_user_regs {
+    uint32_t regs[17];
+    char pad[4];
+} QEMU_PACKED;
+
+QEMU_BUILD_BUG_ON(sizeof(struct arm_user_regs) != 72);
+
+struct arm_elf_prstatus {
+    char pad1[24]; /* 24 == offsetof(struct elf_prstatus, pr_pid) */
+    uint32_t pr_pid;
+    char pad2[44]; /* 44 == offsetof(struct elf_prstatus, pr_reg) -
+                            offsetof(struct elf_prstatus, pr_ppid) */
+    struct arm_user_regs pr_reg;
+    int pr_fpvalid;
+} QEMU_PACKED arm_elf_prstatus;
+
+QEMU_BUILD_BUG_ON(sizeof(struct arm_elf_prstatus) != 148);
+
+struct arm_note {
+    Elf32_Nhdr hdr;
+    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
+    struct arm_elf_prstatus prstatus;
+} QEMU_PACKED;
+
+QEMU_BUILD_BUG_ON(sizeof(struct arm_note) != 168);
+
+static int
+arm_write_elf32_note(WriteCoreDumpFunction f, CPUARMState *env,
+                     int id, DumpState *s)
+{
+    struct arm_note note;
+    int ret, i;
+
+    memset(&note, 0, sizeof(note));
+
+    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
+    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
+    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
+
+    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
+    note.prstatus.pr_pid = cpu_to_dump32(s, id);
+
+    for (i = 0; i < 16; ++i) {
+        note.prstatus.pr_reg.regs[i] = cpu_to_dump32(s, env->regs[i]);
+    }
+    note.prstatus.pr_reg.regs[16] = cpu_to_dump32(s, cpsr_read(env));
+
+    ret = f(&note, sizeof(note), s);
+    if (ret < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
+                             int cpuid, void *opaque)
+{
+    CPUARMState *env = &ARM_CPU(cs)->env;
+    int ret;
+
+    if (is_a64(env)) {
+        ret = aarch64_write_elf64_note(f, env, cpuid, opaque);
+    } else {
+        ret = arm_write_elf32_note(f, env, cpuid, opaque);
+    }
+    return ret;
+}
+
+int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
+                             int cpuid, void *opaque)
+{
+    return arm_write_elf32_note(f, &ARM_CPU(cs)->env, cpuid, opaque);
+}
+
+int cpu_get_dump_info(ArchDumpInfo *info,
+                      const GuestPhysBlockList *guest_phys_blocks)
+{
+    ARMCPU *cpu = ARM_CPU(first_cpu);
+    CPUARMState *env = &cpu->env;
+    int cur_el = arm_current_el(env), be;
+    GuestPhysBlock *block;
+    hwaddr lowest_addr = ULLONG_MAX;
+
+    /* Take a best guess at the phys_base. If we get it wrong then crash
+     * will need '--machdep phys_offset=<phys-offset>' added to its command
+     * line, which isn't any worse than assuming we can use zero, but being
+     * wrong. This is the same algorithm the crash utility uses when
+     * attempting to guess as it loads non-dumpfile formatted files.
+     */
+    QTAILQ_FOREACH(block, &guest_phys_blocks->head, next) {
+        if (block->target_start < lowest_addr) {
+            lowest_addr = block->target_start;
+        }
+    }
+
+    if (is_a64(env)) {
+        info->d_machine = EM_AARCH64;
+        info->d_class = ELFCLASS64;
+        if (cur_el == 0) {
+            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
+        } else {
+            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
+        }
+        info->page_size = (1 << 16); /* aarch64 max pagesize */
+        if (lowest_addr != ULLONG_MAX) {
+            info->phys_base = lowest_addr;
+        }
+    } else {
+        info->d_machine = EM_ARM;
+        info->d_class = ELFCLASS32;
+        be = (env->uncached_cpsr & CPSR_E) != 0;
+        info->page_size = (1 << 12);
+        if (lowest_addr < UINT_MAX) {
+            info->phys_base = lowest_addr;
+        }
+    }
+
+    info->d_endian = be ? ELFDATA2MSB : ELFDATA2LSB;
+
+    return 0;
+}
+
+ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
+{
+    size_t note_size;
+
+    if (class == ELFCLASS64) {
+        note_size = sizeof(struct aarch64_note);
+    } else {
+        note_size = sizeof(struct arm_note);
+    }
+
+    return note_size * nr_cpus;
+}
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 25fb1ce0f3f3d..5bd9b7bb9fa7e 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -221,6 +221,11 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
+int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
+                             int cpuid, void *opaque);
+int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
+                             int cpuid, void *opaque);
+
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 30739fc0dfa74..db91a3f9eb467 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1428,6 +1428,9 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->disas_set_info = arm_disas_set_info;
 
+    cc->write_elf64_note = arm_cpu_write_elf64_note;
+    cc->write_elf32_note = arm_cpu_write_elf32_note;
+
     /*
      * Reason: arm_cpu_initfn() calls cpu_exec_init(), which saves
      * the object in cpus -> dangling pointer after final
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/5] qapi-schema: dump-guest-memory: Improve text

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On 11/19/2015 07:53 AM, Andrew Jones wrote:
> dump-guest-memory is supported by more than just x86, however
> the paging option is not.
> 
> (No functional change.)
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  qapi-schema.json | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Peter Maydell]

On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> Add the support needed for creating prstatus elf notes. This
> allows us to use QMP dump-guest-memory.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  target-arm/Makefile.objs |   3 +-
>  target-arm/arch_dump.c   | 222 +++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/cpu-qom.h     |   5 ++
>  target-arm/cpu.c         |   3 +
>  4 files changed, 231 insertions(+), 2 deletions(-)
>  create mode 100644 target-arm/arch_dump.c
>
> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> index 9460b409a5a1c..a80eb39743a78 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 psci.o arch_dump.o
>  obj-$(CONFIG_KVM) += kvm.o
>  obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
>  obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
> @@ -7,6 +7,5 @@ obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
>  obj-y += translate.o op_helper.o helper.o cpu.o
>  obj-y += neon_helper.o iwmmxt_helper.o
>  obj-y += gdbstub.o
> -obj-$(CONFIG_SOFTMMU) += psci.o
>  obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
>  obj-y += crypto_helper.o
> diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c
> new file mode 100644
> index 0000000000000..5822204e85c72
> --- /dev/null
> +++ b/target-arm/arch_dump.c
> @@ -0,0 +1,222 @@
> +/* Support for writing ELF notes for ARM architectures
> + *
> + * Copyright (C) 2015 Red Hat Inc.
> + *
> + * Author: Andrew Jones <drjones@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "cpu.h"
> +#include "elf.h"
> +#include "sysemu/dump.h"
> +
> +#define NOTE_NAME       "CORE"
> +#define NOTE_NAMESZ     5
> +
> +struct aarch64_user_regs {
> +    uint64_t regs[31];
> +    uint64_t sp;
> +    uint64_t pc;
> +    uint64_t pstate;
> +} QEMU_PACKED;

Do you have a pointer to the spec for the format this code is
writing out?

> +
> +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_user_regs) != 272);
> +
> +struct aarch64_elf_prstatus {
> +    char pad1[32]; /* 32 == offsetof(struct elf_prstatus, pr_pid) */
> +    uint32_t pr_pid;
> +    char pad2[76]; /* 76 == offsetof(struct elf_prstatus, pr_reg) -
> +                            offsetof(struct elf_prstatus, pr_ppid) */
> +    struct aarch64_user_regs pr_reg;
> +    int pr_fpvalid;
> +    char pad3[4];
> +} QEMU_PACKED;

No floating point registers?

> +
> +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_elf_prstatus) != 392);
> +
> +struct aarch64_note {
> +    Elf64_Nhdr hdr;
> +    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
> +    struct aarch64_elf_prstatus prstatus;
> +} QEMU_PACKED;
> +
> +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_note) != 412);
> +
> +static int
> +aarch64_write_elf64_note(WriteCoreDumpFunction f, CPUARMState *env,
> +                         int id, DumpState *s)
> +{
> +    struct aarch64_note note;
> +    int ret, i;
> +
> +    memset(&note, 0, sizeof(note));
> +
> +    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
> +    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
> +    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
> +
> +    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
> +    note.prstatus.pr_pid = cpu_to_dump32(s, id);
> +
> +    for (i = 0; i < 31; ++i) {
> +        note.prstatus.pr_reg.regs[i] = cpu_to_dump64(s, env->xregs[i]);
> +    }
> +    note.prstatus.pr_reg.sp = cpu_to_dump64(s, env->xregs[31]);
> +    note.prstatus.pr_reg.pc = cpu_to_dump64(s, env->pc);
> +    note.prstatus.pr_reg.pstate = cpu_to_dump64(s, pstate_read(env));
> +
> +    ret = f(&note, sizeof(note), s);
> +    if (ret < 0) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +struct arm_user_regs {
> +    uint32_t regs[17];
> +    char pad[4];
> +} QEMU_PACKED;
> +
> +QEMU_BUILD_BUG_ON(sizeof(struct arm_user_regs) != 72);
> +
> +struct arm_elf_prstatus {
> +    char pad1[24]; /* 24 == offsetof(struct elf_prstatus, pr_pid) */
> +    uint32_t pr_pid;
> +    char pad2[44]; /* 44 == offsetof(struct elf_prstatus, pr_reg) -
> +                            offsetof(struct elf_prstatus, pr_ppid) */
> +    struct arm_user_regs pr_reg;
> +    int pr_fpvalid;
> +} QEMU_PACKED arm_elf_prstatus;
> +
> +QEMU_BUILD_BUG_ON(sizeof(struct arm_elf_prstatus) != 148);
> +
> +struct arm_note {
> +    Elf32_Nhdr hdr;
> +    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
> +    struct arm_elf_prstatus prstatus;
> +} QEMU_PACKED;
> +
> +QEMU_BUILD_BUG_ON(sizeof(struct arm_note) != 168);
> +
> +static int
> +arm_write_elf32_note(WriteCoreDumpFunction f, CPUARMState *env,
> +                     int id, DumpState *s)
> +{
> +    struct arm_note note;
> +    int ret, i;
> +
> +    memset(&note, 0, sizeof(note));
> +
> +    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
> +    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
> +    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
> +
> +    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
> +    note.prstatus.pr_pid = cpu_to_dump32(s, id);
> +
> +    for (i = 0; i < 16; ++i) {
> +        note.prstatus.pr_reg.regs[i] = cpu_to_dump32(s, env->regs[i]);
> +    }
> +    note.prstatus.pr_reg.regs[16] = cpu_to_dump32(s, cpsr_read(env));
> +
> +    ret = f(&note, sizeof(note), s);
> +    if (ret < 0) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> +                             int cpuid, void *opaque)
> +{
> +    CPUARMState *env = &ARM_CPU(cs)->env;
> +    int ret;
> +
> +    if (is_a64(env)) {

Are you really sure you want the core dump format to depend on
whether the CPU happens to be in 32-bit or 64-bit format at
the point in time we write it out? (Consider a 64-bit kernel
which happens to be running a 32-bit userspace binary.)

> +        ret = aarch64_write_elf64_note(f, env, cpuid, opaque);
> +    } else {
> +        ret = arm_write_elf32_note(f, env, cpuid, opaque);
> +    }
> +    return ret;
> +}
> +
> +int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> +                             int cpuid, void *opaque)
> +{
> +    return arm_write_elf32_note(f, &ARM_CPU(cs)->env, cpuid, opaque);
> +}
> +
> +int cpu_get_dump_info(ArchDumpInfo *info,
> +                      const GuestPhysBlockList *guest_phys_blocks)
> +{
> +    ARMCPU *cpu = ARM_CPU(first_cpu);
> +    CPUARMState *env = &cpu->env;
> +    int cur_el = arm_current_el(env), be;
> +    GuestPhysBlock *block;
> +    hwaddr lowest_addr = ULLONG_MAX;
> +
> +    /* Take a best guess at the phys_base. If we get it wrong then crash
> +     * will need '--machdep phys_offset=<phys-offset>' added to its command
> +     * line, which isn't any worse than assuming we can use zero, but being
> +     * wrong. This is the same algorithm the crash utility uses when
> +     * attempting to guess as it loads non-dumpfile formatted files.
> +     */
> +    QTAILQ_FOREACH(block, &guest_phys_blocks->head, next) {
> +        if (block->target_start < lowest_addr) {
> +            lowest_addr = block->target_start;
> +        }
> +    }
> +
> +    if (is_a64(env)) {
> +        info->d_machine = EM_AARCH64;
> +        info->d_class = ELFCLASS64;
> +        if (cur_el == 0) {
> +            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> +        } else {
> +            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> +        }

Again, are you sure you want the core dump format to depend on
whether we currently happen to be executing a BE userspace
process?

> +        info->page_size = (1 << 16); /* aarch64 max pagesize */
> +        if (lowest_addr != ULLONG_MAX) {
> +            info->phys_base = lowest_addr;
> +        }
> +    } else {
> +        info->d_machine = EM_ARM;
> +        info->d_class = ELFCLASS32;
> +        be = (env->uncached_cpsr & CPSR_E) != 0;

Same remarks here.

(If you do want "BE state as of right this instant", this is
duplicating most of the logic from arm_cpu_is_big_endian().)

> +        info->page_size = (1 << 12);
> +        if (lowest_addr < UINT_MAX) {
> +            info->phys_base = lowest_addr;
> +        }
> +    }
> +
> +    info->d_endian = be ? ELFDATA2MSB : ELFDATA2LSB;
> +
> +    return 0;
> +}
> +
> +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
> +{
> +    size_t note_size;
> +
> +    if (class == ELFCLASS64) {
> +        note_size = sizeof(struct aarch64_note);
> +    } else {
> +        note_size = sizeof(struct arm_note);
> +    }
> +
> +    return note_size * nr_cpus;
> +}
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 25fb1ce0f3f3d..5bd9b7bb9fa7e 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -221,6 +221,11 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>
> +int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> +                             int cpuid, void *opaque);
> +int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> +                             int cpuid, void *opaque);
> +
>  /* Callback functions for the generic timer's timers. */
>  void arm_gt_ptimer_cb(void *opaque);
>  void arm_gt_vtimer_cb(void *opaque);
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 30739fc0dfa74..db91a3f9eb467 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1428,6 +1428,9 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>
>      cc->disas_set_info = arm_disas_set_info;
>
> +    cc->write_elf64_note = arm_cpu_write_elf64_note;
> +    cc->write_elf32_note = arm_cpu_write_elf32_note;
> +
>      /*
>       * Reason: arm_cpu_initfn() calls cpu_exec_init(), which saves
>       * the object in cpus -> dangling pointer after final
> --
> 2.4.3

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/5] dump: qemunotes aren't commonly needed

[Peter Maydell]

On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> Only one of three architectures implementing qmp-dump-guest-memory write
> qemu notes. And, another architecture (arm/aarch64) is coming, which
> won't use them either. Make the common implementation truly common.
>
> (No functional change.)
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones]

On Fri, Nov 20, 2015 at 06:19:14PM +0000, Peter Maydell wrote:
> On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> > Add the support needed for creating prstatus elf notes. This
> > allows us to use QMP dump-guest-memory.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  target-arm/Makefile.objs |   3 +-
> >  target-arm/arch_dump.c   | 222 +++++++++++++++++++++++++++++++++++++++++++++++
> >  target-arm/cpu-qom.h     |   5 ++
> >  target-arm/cpu.c         |   3 +
> >  4 files changed, 231 insertions(+), 2 deletions(-)
> >  create mode 100644 target-arm/arch_dump.c
> >
> > diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> > index 9460b409a5a1c..a80eb39743a78 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 psci.o arch_dump.o
> >  obj-$(CONFIG_KVM) += kvm.o
> >  obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
> >  obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
> > @@ -7,6 +7,5 @@ obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
> >  obj-y += translate.o op_helper.o helper.o cpu.o
> >  obj-y += neon_helper.o iwmmxt_helper.o
> >  obj-y += gdbstub.o
> > -obj-$(CONFIG_SOFTMMU) += psci.o
> >  obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
> >  obj-y += crypto_helper.o
> > diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c
> > new file mode 100644
> > index 0000000000000..5822204e85c72
> > --- /dev/null
> > +++ b/target-arm/arch_dump.c
> > @@ -0,0 +1,222 @@
> > +/* Support for writing ELF notes for ARM architectures
> > + *
> > + * Copyright (C) 2015 Red Hat Inc.
> > + *
> > + * Author: Andrew Jones <drjones@redhat.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "cpu.h"
> > +#include "elf.h"
> > +#include "sysemu/dump.h"
> > +
> > +#define NOTE_NAME       "CORE"
> > +#define NOTE_NAMESZ     5
> > +
> > +struct aarch64_user_regs {
> > +    uint64_t regs[31];
> > +    uint64_t sp;
> > +    uint64_t pc;
> > +    uint64_t pstate;
> > +} QEMU_PACKED;
> 
> Do you have a pointer to the spec for the format this code is
> writing out?

Unfortunately not, and I just searched now again, but I can't anything
except for what I used, which is just struct user_pt_regs from
arch/arm64/include/uapi/asm/ptrace.h. Hmm, maybe we should add ptrace.h
to the update-linux-headers.sh script and then use it from there? We
would also update the other architectures reproducing their respective
structures in their respective arch_dump.c's as well.

> 
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_user_regs) != 272);
> > +
> > +struct aarch64_elf_prstatus {
> > +    char pad1[32]; /* 32 == offsetof(struct elf_prstatus, pr_pid) */
> > +    uint32_t pr_pid;
> > +    char pad2[76]; /* 76 == offsetof(struct elf_prstatus, pr_reg) -
> > +                            offsetof(struct elf_prstatus, pr_ppid) */
> > +    struct aarch64_user_regs pr_reg;
> > +    int pr_fpvalid;
> > +    char pad3[4];
> > +} QEMU_PACKED;
> 
> No floating point registers?

I have a patch for that written already, but I didn't post it with this
because crash doesn't seem to care about them. gdb could extract and display
the floating point registers, but gdb can't deal with the memory...

Anyway, particularly because I already wrote it, I'm not opposed to adding
it on to the series. The information won't be used by crash, but anybody who
cares can dig it out with gdb.

> 
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_elf_prstatus) != 392);
> > +
> > +struct aarch64_note {
> > +    Elf64_Nhdr hdr;
> > +    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
> > +    struct aarch64_elf_prstatus prstatus;
> > +} QEMU_PACKED;
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct aarch64_note) != 412);
> > +
> > +static int
> > +aarch64_write_elf64_note(WriteCoreDumpFunction f, CPUARMState *env,
> > +                         int id, DumpState *s)
> > +{
> > +    struct aarch64_note note;
> > +    int ret, i;
> > +
> > +    memset(&note, 0, sizeof(note));
> > +
> > +    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
> > +    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
> > +    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
> > +
> > +    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
> > +    note.prstatus.pr_pid = cpu_to_dump32(s, id);
> > +
> > +    for (i = 0; i < 31; ++i) {
> > +        note.prstatus.pr_reg.regs[i] = cpu_to_dump64(s, env->xregs[i]);
> > +    }
> > +    note.prstatus.pr_reg.sp = cpu_to_dump64(s, env->xregs[31]);
> > +    note.prstatus.pr_reg.pc = cpu_to_dump64(s, env->pc);
> > +    note.prstatus.pr_reg.pstate = cpu_to_dump64(s, pstate_read(env));
> > +
> > +    ret = f(&note, sizeof(note), s);
> > +    if (ret < 0) {
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +struct arm_user_regs {
> > +    uint32_t regs[17];
> > +    char pad[4];
> > +} QEMU_PACKED;
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct arm_user_regs) != 72);
> > +
> > +struct arm_elf_prstatus {
> > +    char pad1[24]; /* 24 == offsetof(struct elf_prstatus, pr_pid) */
> > +    uint32_t pr_pid;
> > +    char pad2[44]; /* 44 == offsetof(struct elf_prstatus, pr_reg) -
> > +                            offsetof(struct elf_prstatus, pr_ppid) */
> > +    struct arm_user_regs pr_reg;
> > +    int pr_fpvalid;
> > +} QEMU_PACKED arm_elf_prstatus;
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct arm_elf_prstatus) != 148);
> > +
> > +struct arm_note {
> > +    Elf32_Nhdr hdr;
> > +    char name[QEMU_ALIGN_UP(NOTE_NAMESZ, 4)];
> > +    struct arm_elf_prstatus prstatus;
> > +} QEMU_PACKED;
> > +
> > +QEMU_BUILD_BUG_ON(sizeof(struct arm_note) != 168);
> > +
> > +static int
> > +arm_write_elf32_note(WriteCoreDumpFunction f, CPUARMState *env,
> > +                     int id, DumpState *s)
> > +{
> > +    struct arm_note note;
> > +    int ret, i;
> > +
> > +    memset(&note, 0, sizeof(note));
> > +
> > +    note.hdr.n_namesz = cpu_to_dump32(s, NOTE_NAMESZ);
> > +    note.hdr.n_descsz = cpu_to_dump32(s, sizeof(note.prstatus));
> > +    note.hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
> > +
> > +    memcpy(note.name, NOTE_NAME, NOTE_NAMESZ);
> > +    note.prstatus.pr_pid = cpu_to_dump32(s, id);
> > +
> > +    for (i = 0; i < 16; ++i) {
> > +        note.prstatus.pr_reg.regs[i] = cpu_to_dump32(s, env->regs[i]);
> > +    }
> > +    note.prstatus.pr_reg.regs[16] = cpu_to_dump32(s, cpsr_read(env));
> > +
> > +    ret = f(&note, sizeof(note), s);
> > +    if (ret < 0) {
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> > +                             int cpuid, void *opaque)
> > +{
> > +    CPUARMState *env = &ARM_CPU(cs)->env;
> > +    int ret;
> > +
> > +    if (is_a64(env)) {
> 
> Are you really sure you want the core dump format to depend on
> whether the CPU happens to be in 32-bit or 64-bit format at
> the point in time we write it out? (Consider a 64-bit kernel
> which happens to be running a 32-bit userspace binary.)

I simply forgot to consider the case where a 64-bit kernel would
run a 32-bit userspace binary. I'm actually quite sure we would
want 64-bit in that case, as crash is the only tool we're able to
generate dumps for at this time (gdb requires the 'paging' option
of dump-guest-memory to work). Is there something in the env I can
look at to determine that we have a 64-bit kernel? (Sorry for being
lazy and just asking, rather than reading...)

> 
> > +        ret = aarch64_write_elf64_note(f, env, cpuid, opaque);
> > +    } else {
> > +        ret = arm_write_elf32_note(f, env, cpuid, opaque);
> > +    }
> > +    return ret;
> > +}
> > +
> > +int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> > +                             int cpuid, void *opaque)
> > +{
> > +    return arm_write_elf32_note(f, &ARM_CPU(cs)->env, cpuid, opaque);
> > +}
> > +
> > +int cpu_get_dump_info(ArchDumpInfo *info,
> > +                      const GuestPhysBlockList *guest_phys_blocks)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(first_cpu);
> > +    CPUARMState *env = &cpu->env;
> > +    int cur_el = arm_current_el(env), be;
> > +    GuestPhysBlock *block;
> > +    hwaddr lowest_addr = ULLONG_MAX;
> > +
> > +    /* Take a best guess at the phys_base. If we get it wrong then crash
> > +     * will need '--machdep phys_offset=<phys-offset>' added to its command
> > +     * line, which isn't any worse than assuming we can use zero, but being
> > +     * wrong. This is the same algorithm the crash utility uses when
> > +     * attempting to guess as it loads non-dumpfile formatted files.
> > +     */
> > +    QTAILQ_FOREACH(block, &guest_phys_blocks->head, next) {
> > +        if (block->target_start < lowest_addr) {
> > +            lowest_addr = block->target_start;
> > +        }
> > +    }
> > +
> > +    if (is_a64(env)) {
> > +        info->d_machine = EM_AARCH64;
> > +        info->d_class = ELFCLASS64;
> > +        if (cur_el == 0) {
> > +            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> > +        } else {
> > +            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> > +        }
> 
> Again, are you sure you want the core dump format to depend on
> whether we currently happen to be executing a BE userspace
> process?

We'll want to match the kernel. Hopefully we can determine it.

> 
> > +        info->page_size = (1 << 16); /* aarch64 max pagesize */
> > +        if (lowest_addr != ULLONG_MAX) {
> > +            info->phys_base = lowest_addr;
> > +        }
> > +    } else {
> > +        info->d_machine = EM_ARM;
> > +        info->d_class = ELFCLASS32;
> > +        be = (env->uncached_cpsr & CPSR_E) != 0;
> 
> Same remarks here.
> 
> (If you do want "BE state as of right this instant", this is
> duplicating most of the logic from arm_cpu_is_big_endian().)

I should have used that, but now that I know the instantaneous
endianness is not what I want, then I guess I'll need to come
up with something else anyway.

> 
> > +        info->page_size = (1 << 12);
> > +        if (lowest_addr < UINT_MAX) {
> > +            info->phys_base = lowest_addr;
> > +        }
> > +    }
> > +
> > +    info->d_endian = be ? ELFDATA2MSB : ELFDATA2LSB;
> > +
> > +    return 0;
> > +}
> > +
> > +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
> > +{
> > +    size_t note_size;
> > +
> > +    if (class == ELFCLASS64) {
> > +        note_size = sizeof(struct aarch64_note);
> > +    } else {
> > +        note_size = sizeof(struct arm_note);
> > +    }
> > +
> > +    return note_size * nr_cpus;
> > +}
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index 25fb1ce0f3f3d..5bd9b7bb9fa7e 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -221,6 +221,11 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> >  int arm_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> >  int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> >
> > +int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> > +                             int cpuid, void *opaque);
> > +int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> > +                             int cpuid, void *opaque);
> > +
> >  /* Callback functions for the generic timer's timers. */
> >  void arm_gt_ptimer_cb(void *opaque);
> >  void arm_gt_vtimer_cb(void *opaque);
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 30739fc0dfa74..db91a3f9eb467 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -1428,6 +1428,9 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
> >
> >      cc->disas_set_info = arm_disas_set_info;
> >
> > +    cc->write_elf64_note = arm_cpu_write_elf64_note;
> > +    cc->write_elf32_note = arm_cpu_write_elf32_note;
> > +
> >      /*
> >       * Reason: arm_cpu_initfn() calls cpu_exec_init(), which saves
> >       * the object in cpus -> dangling pointer after final
> > --
> > 2.4.3
> 
> thanks
> -- PMM
>

Thanks for the review!

drew 

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones]

On Fri, Nov 20, 2015 at 04:41:21PM -0500, Andrew Jones wrote:
> On Fri, Nov 20, 2015 at 06:19:14PM +0000, Peter Maydell wrote:
> > On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> > > +
> > > +    if (is_a64(env)) {
> > 
> > Are you really sure you want the core dump format to depend on
> > whether the CPU happens to be in 32-bit or 64-bit format at
> > the point in time we write it out? (Consider a 64-bit kernel
> > which happens to be running a 32-bit userspace binary.)
> 
> I simply forgot to consider the case where a 64-bit kernel would
> run a 32-bit userspace binary. I'm actually quite sure we would
> want 64-bit in that case, as crash is the only tool we're able to
> generate dumps for at this time (gdb requires the 'paging' option
> of dump-guest-memory to work). Is there something in the env I can
> look at to determine that we have a 64-bit kernel? (Sorry for being
> lazy and just asking, rather than reading...)

Duh, I momentarily forgot about arm_el_is_aa64(env, 1). I see we unset
ARM_FEATURE_AARCH64 in aarch64_cpu_set_aarch64, so that should work
fine for our 32bit guests on 64bit hosts.

> > > +    if (is_a64(env)) {
> > > +        info->d_machine = EM_AARCH64;
> > > +        info->d_class = ELFCLASS64;
> > > +        if (cur_el == 0) {
> > > +            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> > > +        } else {
> > > +            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> > > +        }
> > 
> > Again, are you sure you want the core dump format to depend on
> > whether we currently happen to be executing a BE userspace
> > process?
> 
> We'll want to match the kernel. Hopefully we can determine it.

Here's a bigger, duh. I guess I just need to drop all the cur_el
stuff and stick to el==1.

I'll wait to hear back on the 'should we add ptrace.h to linux-headers',
and 'should we add floating point registers, even though crash won't
care' questions before sending a v2.

Thanks,
drew

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones]

On Sat, Nov 21, 2015 at 10:05:37AM -0500, Andrew Jones wrote:
> On Fri, Nov 20, 2015 at 04:41:21PM -0500, Andrew Jones wrote:
> > On Fri, Nov 20, 2015 at 06:19:14PM +0000, Peter Maydell wrote:
> > > On 19 November 2015 at 14:53, Andrew Jones <drjones@redhat.com> wrote:
> > > > +
> > > > +    if (is_a64(env)) {
> > > 
> > > Are you really sure you want the core dump format to depend on
> > > whether the CPU happens to be in 32-bit or 64-bit format at
> > > the point in time we write it out? (Consider a 64-bit kernel
> > > which happens to be running a 32-bit userspace binary.)
> > 
> > I simply forgot to consider the case where a 64-bit kernel would
> > run a 32-bit userspace binary. I'm actually quite sure we would
> > want 64-bit in that case, as crash is the only tool we're able to
> > generate dumps for at this time (gdb requires the 'paging' option
> > of dump-guest-memory to work). Is there something in the env I can
> > look at to determine that we have a 64-bit kernel? (Sorry for being
> > lazy and just asking, rather than reading...)
> 
> Duh, I momentarily forgot about arm_el_is_aa64(env, 1). I see we unset
> ARM_FEATURE_AARCH64 in aarch64_cpu_set_aarch64, so that should work
> fine for our 32bit guests on 64bit hosts.
> 
> > > > +    if (is_a64(env)) {
> > > > +        info->d_machine = EM_AARCH64;
> > > > +        info->d_class = ELFCLASS64;
> > > > +        if (cur_el == 0) {
> > > > +            be = (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> > > > +        } else {
> > > > +            be = (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> > > > +        }
> > > 
> > > Again, are you sure you want the core dump format to depend on
> > > whether we currently happen to be executing a BE userspace
> > > process?
> > 
> > We'll want to match the kernel. Hopefully we can determine it.
> 
> Here's a bigger, duh. I guess I just need to drop all the cur_el
> stuff and stick to el==1.
> 
> I'll wait to hear back on the 'should we add ptrace.h to linux-headers',
> and 'should we add floating point registers, even though crash won't
> care' questions before sending a v2.

Hi Peter,

I've pulled a v2 together that I'll be testing and posting soon. Here's
what I decided to do

1) Throw the fp registers in. Why not?
2) No linux-headers update, as we'd also need 
   include/uapi/linux/elfcore.h and arch/arm/include/asm/user.h.
   However I've added comments stating where the structs come from.
3) Base the vmcore type on the guest kernel, i.e. use arm_el_is_aa64()
   and (env->cp15.sctlr_el[1] & SCTLR_EE). However,
   aarch64_write_elf64_note() will shoehorn 32-bit state into 64-bit
   elf notes when the current state is 32-bit. Those analyzing the
   dumps will need to look at the captured pstate to determine the
   endianness of the registers.

How does that sound?

Thanks,
drew

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Peter Maydell]

On 24 November 2015 at 20:52, Andrew Jones <drjones@redhat.com> wrote:
>
> I've pulled a v2 together that I'll be testing and posting soon. Here's
> what I decided to do
>
> 1) Throw the fp registers in. Why not?
> 2) No linux-headers update, as we'd also need
>    include/uapi/linux/elfcore.h and arch/arm/include/asm/user.h.
>    However I've added comments stating where the structs come from.
> 3) Base the vmcore type on the guest kernel, i.e. use arm_el_is_aa64()
>    and (env->cp15.sctlr_el[1] & SCTLR_EE). However,
>    aarch64_write_elf64_note() will shoehorn 32-bit state into 64-bit
>    elf notes when the current state is 32-bit. Those analyzing the
>    dumps will need to look at the captured pstate to determine the
>    endianness of the registers.

Not sure what you have in mind by "endianness of the registers" --
typically registers aren't thought of as having endianness.

Also, if we're currently executing a 32-bit guest when we take
the core dump, you probably need to call aarch64_sync_32_to_64()
somewhere.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Andrew Jones]

On Tue, Nov 24, 2015 at 09:08:29PM +0000, Peter Maydell wrote:
> On 24 November 2015 at 20:52, Andrew Jones <drjones@redhat.com> wrote:
> >
> > I've pulled a v2 together that I'll be testing and posting soon. Here's
> > what I decided to do
> >
> > 1) Throw the fp registers in. Why not?
> > 2) No linux-headers update, as we'd also need
> >    include/uapi/linux/elfcore.h and arch/arm/include/asm/user.h.
> >    However I've added comments stating where the structs come from.
> > 3) Base the vmcore type on the guest kernel, i.e. use arm_el_is_aa64()
> >    and (env->cp15.sctlr_el[1] & SCTLR_EE). However,
> >    aarch64_write_elf64_note() will shoehorn 32-bit state into 64-bit
> >    elf notes when the current state is 32-bit. Those analyzing the
> >    dumps will need to look at the captured pstate to determine the
> >    endianness of the registers.
> 
> Not sure what you have in mind by "endianness of the registers" --
> typically registers aren't thought of as having endianness.

If a processor that was temporarily switched into the opposite endian
had just read a memory address into a register before generating the
core, then the data in that register will be in the format of that
temporary endianness. The person doing the dump analysis will need to
keep in mind that even though they should interpret a crash-tool read of
that same address in the endianness of the core type, the register will
have it swapped. Or maybe I'm wrong about how that should work?

> 
> Also, if we're currently executing a 32-bit guest when we take
> the core dump, you probably need to call aarch64_sync_32_to_64()
> somewhere.

I think we're covered here. We have

dump_init
  cpu_synchronize_all_states
    ... 
      kvm_arch_get_registers
        if (!is_a64(env)) {
            aarch64_sync_64_to_32(env);
        }

Thanks,
drew

Re: [Qemu-devel] [PATCH 5/5] target-arm: support QMP dump-guest-memory

[Peter Maydell]

On 24 November 2015 at 21:45, Andrew Jones <drjones@redhat.com> wrote:
> On Tue, Nov 24, 2015 at 09:08:29PM +0000, Peter Maydell wrote:
>> Also, if we're currently executing a 32-bit guest when we take
>> the core dump, you probably need to call aarch64_sync_32_to_64()
>> somewhere.
>
> I think we're covered here. We have
>
> dump_init
>   cpu_synchronize_all_states
>     ...
>       kvm_arch_get_registers
>         if (!is_a64(env)) {
>             aarch64_sync_64_to_32(env);
>         }

That won't work if you're taking the core dump on a QEMU
which isn't using KVM...

thanks
-- PMM