All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL v2 00/13] Dump patches
@ 2022-04-22 10:15 marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 01/13] dump: Use ERRP_GUARD() marcandre.lureau
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following changes since commit a74782936dc6e979ce371dabda4b1c05624ea87f:

  Merge tag 'pull-migration-20220421a' of https://gitlab.com/dagrh/qemu into staging (2022-04-21 18:48:18 -0700)

are available in the Git repository at:

  git@gitlab.com:marcandre.lureau/qemu.git tags/dump-pull-request

for you to fetch changes up to f5daa8293b292929cb429f154e926191ba8e040c:

  dump/win_dump: add 32-bit guest Windows support (2022-04-22 13:41:56 +0400)

----------------------------------------------------------------
dump queue

Hi

The "dump" queue, with:
- [PATCH v3/v4 0/9] dump: Cleanup and consolidation
- [PATCH v4 0/4] dump: add 32-bit guest Windows support

v2:
- fix compiler warning in "dump/win_dump: add 32-bit guest Windows support"

----------------------------------------------------------------

Janosch Frank (9):
  dump: Use ERRP_GUARD()
  dump: Remove the sh_info variable
  dump: Introduce shdr_num to decrease complexity
  dump: Remove the section if when calculating the memory offset
  dump: Add more offset variables
  dump: Introduce dump_is_64bit() helper function
  dump: Consolidate phdr note writes
  dump: Cleanup dump_begin write functions
  dump: Consolidate elf note function

Viktor Prutyanov (4):
  include/qemu: rename Windows context definitions to expose bitness
  dump/win_dump: add helper macros for Windows dump header access
  include/qemu: add 32-bit Windows dump structures
  dump/win_dump: add 32-bit guest Windows support

 include/qemu/win_dump_defs.h | 115 ++++++++++-
 include/sysemu/dump.h        |   9 +-
 contrib/elf2dmp/main.c       |   6 +-
 dump/dump.c                  | 372 ++++++++++++++++-------------------
 dump/win_dump.c              | 305 +++++++++++++++++-----------
 hmp-commands.hx              |   2 +-
 6 files changed, 482 insertions(+), 327 deletions(-)

-- 
2.36.0



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PULL v2 01/13] dump: Use ERRP_GUARD()
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 02/13] dump: Remove the sh_info variable marcandre.lureau
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

Let's move to the new way of handling errors before changing the dump
code. This patch has mostly been generated by the coccinelle script
scripts/coccinelle/errp-guard.cocci.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-2-frankja@linux.ibm.com>
---
 dump/dump.c | 144 ++++++++++++++++++++++------------------------------
 1 file changed, 61 insertions(+), 83 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index e766ce1d7d91..b91e9d8c123e 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -389,23 +389,21 @@ static void write_data(DumpState *s, void *buf, int length, Error **errp)
 static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t start,
                          int64_t size, Error **errp)
 {
+    ERRP_GUARD();
     int64_t i;
-    Error *local_err = NULL;
 
     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);
+                   s->dump_info.page_size, errp);
+        if (*errp) {
             return;
         }
     }
 
     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);
+                   size % s->dump_info.page_size, errp);
+        if (*errp) {
             return;
         }
     }
@@ -475,11 +473,11 @@ static void get_offset_range(hwaddr phys_addr,
 
 static void write_elf_loads(DumpState *s, Error **errp)
 {
+    ERRP_GUARD();
     hwaddr offset, filesz;
     MemoryMapping *memory_mapping;
     uint32_t phdr_index = 1;
     uint32_t max_index;
-    Error *local_err = NULL;
 
     if (s->have_section) {
         max_index = s->sh_info;
@@ -493,14 +491,13 @@ static void write_elf_loads(DumpState *s, Error **errp)
                          s, &offset, &filesz);
         if (s->dump_info.d_class == ELFCLASS64) {
             write_elf64_load(s, memory_mapping, phdr_index++, offset,
-                             filesz, &local_err);
+                             filesz, errp);
         } else {
             write_elf32_load(s, memory_mapping, phdr_index++, offset,
-                             filesz, &local_err);
+                             filesz, errp);
         }
 
-        if (local_err) {
-            error_propagate(errp, local_err);
+        if (*errp) {
             return;
         }
 
@@ -513,7 +510,7 @@ static void write_elf_loads(DumpState *s, Error **errp)
 /* write elf header, PT_NOTE and elf note to vmcore. */
 static void dump_begin(DumpState *s, Error **errp)
 {
-    Error *local_err = NULL;
+    ERRP_GUARD();
 
     /*
      * the vmcore's format is:
@@ -541,73 +538,64 @@ static void dump_begin(DumpState *s, Error **errp)
 
     /* write elf header to vmcore */
     if (s->dump_info.d_class == ELFCLASS64) {
-        write_elf64_header(s, &local_err);
+        write_elf64_header(s, errp);
     } else {
-        write_elf32_header(s, &local_err);
+        write_elf32_header(s, errp);
     }
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (*errp) {
         return;
     }
 
     if (s->dump_info.d_class == ELFCLASS64) {
         /* write PT_NOTE to vmcore */
-        write_elf64_note(s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf64_note(s, errp);
+        if (*errp) {
             return;
         }
 
         /* write all PT_LOAD to vmcore */
-        write_elf_loads(s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf_loads(s, errp);
+        if (*errp) {
             return;
         }
 
         /* write section to vmcore */
         if (s->have_section) {
-            write_elf_section(s, 1, &local_err);
-            if (local_err) {
-                error_propagate(errp, local_err);
+            write_elf_section(s, 1, errp);
+            if (*errp) {
                 return;
             }
         }
 
         /* write notes to vmcore */
-        write_elf64_notes(fd_write_vmcore, s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf64_notes(fd_write_vmcore, s, errp);
+        if (*errp) {
             return;
         }
     } else {
         /* write PT_NOTE to vmcore */
-        write_elf32_note(s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf32_note(s, errp);
+        if (*errp) {
             return;
         }
 
         /* write all PT_LOAD to vmcore */
-        write_elf_loads(s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf_loads(s, errp);
+        if (*errp) {
             return;
         }
 
         /* write section to vmcore */
         if (s->have_section) {
-            write_elf_section(s, 0, &local_err);
-            if (local_err) {
-                error_propagate(errp, local_err);
+            write_elf_section(s, 0, errp);
+            if (*errp) {
                 return;
             }
         }
 
         /* write notes to vmcore */
-        write_elf32_notes(fd_write_vmcore, s, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_elf32_notes(fd_write_vmcore, s, errp);
+        if (*errp) {
             return;
         }
     }
@@ -643,9 +631,9 @@ static int get_next_block(DumpState *s, GuestPhysBlock *block)
 /* write all memory to vmcore */
 static void dump_iterate(DumpState *s, Error **errp)
 {
+    ERRP_GUARD();
     GuestPhysBlock *block;
     int64_t size;
-    Error *local_err = NULL;
 
     do {
         block = s->next_block;
@@ -657,9 +645,8 @@ static void dump_iterate(DumpState *s, Error **errp)
                 size -= block->target_end - (s->begin + s->length);
             }
         }
-        write_memory(s, block, s->start, size, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        write_memory(s, block, s->start, size, errp);
+        if (*errp) {
             return;
         }
 
@@ -668,11 +655,10 @@ static void dump_iterate(DumpState *s, Error **errp)
 
 static void create_vmcore(DumpState *s, Error **errp)
 {
-    Error *local_err = NULL;
+    ERRP_GUARD();
 
-    dump_begin(s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    dump_begin(s, errp);
+    if (*errp) {
         return;
     }
 
@@ -809,6 +795,7 @@ static bool note_name_equal(DumpState *s,
 /* write common header, sub header and elf note to vmcore */
 static void create_header32(DumpState *s, Error **errp)
 {
+    ERRP_GUARD();
     DiskDumpHeader32 *dh = NULL;
     KdumpSubHeader32 *kh = NULL;
     size_t size;
@@ -817,7 +804,6 @@ static void create_header32(DumpState *s, Error **errp)
     uint32_t bitmap_blocks;
     uint32_t status = 0;
     uint64_t offset_note;
-    Error *local_err = NULL;
 
     /* write common header, the version of kdump-compressed format is 6th */
     size = sizeof(DiskDumpHeader32);
@@ -893,9 +879,8 @@ static void create_header32(DumpState *s, Error **errp)
     s->note_buf_offset = 0;
 
     /* use s->note_buf to store notes temporarily */
-    write_elf32_notes(buf_write_note, s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    write_elf32_notes(buf_write_note, s, errp);
+    if (*errp) {
         goto out;
     }
     if (write_buffer(s->fd, offset_note, s->note_buf,
@@ -921,6 +906,7 @@ out:
 /* write common header, sub header and elf note to vmcore */
 static void create_header64(DumpState *s, Error **errp)
 {
+    ERRP_GUARD();
     DiskDumpHeader64 *dh = NULL;
     KdumpSubHeader64 *kh = NULL;
     size_t size;
@@ -929,7 +915,6 @@ static void create_header64(DumpState *s, Error **errp)
     uint32_t bitmap_blocks;
     uint32_t status = 0;
     uint64_t offset_note;
-    Error *local_err = NULL;
 
     /* write common header, the version of kdump-compressed format is 6th */
     size = sizeof(DiskDumpHeader64);
@@ -1005,9 +990,8 @@ static void create_header64(DumpState *s, Error **errp)
     s->note_buf_offset = 0;
 
     /* use s->note_buf to store notes temporarily */
-    write_elf64_notes(buf_write_note, s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    write_elf64_notes(buf_write_note, s, errp);
+    if (*errp) {
         goto out;
     }
 
@@ -1463,8 +1447,8 @@ out:
 
 static void create_kdump_vmcore(DumpState *s, Error **errp)
 {
+    ERRP_GUARD();
     int ret;
-    Error *local_err = NULL;
 
     /*
      * the kdump-compressed format is:
@@ -1494,21 +1478,18 @@ static void create_kdump_vmcore(DumpState *s, Error **errp)
         return;
     }
 
-    write_dump_header(s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    write_dump_header(s, errp);
+    if (*errp) {
         return;
     }
 
-    write_dump_bitmap(s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    write_dump_bitmap(s, errp);
+    if (*errp) {
         return;
     }
 
-    write_dump_pages(s, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    write_dump_pages(s, errp);
+    if (*errp) {
         return;
     }
 
@@ -1638,10 +1619,10 @@ static void dump_init(DumpState *s, int fd, bool has_format,
                       DumpGuestMemoryFormat format, bool paging, bool has_filter,
                       int64_t begin, int64_t length, Error **errp)
 {
+    ERRP_GUARD();
     VMCoreInfoState *vmci = vmcoreinfo_find();
     CPUState *cpu;
     int nr_cpus;
-    Error *err = NULL;
     int ret;
 
     s->has_format = has_format;
@@ -1760,9 +1741,8 @@ static void dump_init(DumpState *s, int fd, bool has_format,
 
     /* get memory mapping */
     if (paging) {
-        qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, &err);
-        if (err != NULL) {
-            error_propagate(errp, err);
+        qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, errp);
+        if (*errp) {
             goto cleanup;
         }
     } else {
@@ -1861,33 +1841,32 @@ cleanup:
 /* this operation might be time consuming. */
 static void dump_process(DumpState *s, Error **errp)
 {
-    Error *local_err = NULL;
+    ERRP_GUARD();
     DumpQueryResult *result = NULL;
 
     if (s->has_format && s->format == DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) {
 #ifdef TARGET_X86_64
-        create_win_dump(s, &local_err);
+        create_win_dump(s, errp);
 #endif
     } else if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
-        create_kdump_vmcore(s, &local_err);
+        create_kdump_vmcore(s, errp);
     } else {
-        create_vmcore(s, &local_err);
+        create_vmcore(s, errp);
     }
 
     /* make sure status is written after written_size updates */
     smp_wmb();
     qatomic_set(&s->status,
-               (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
+               (*errp ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED));
 
     /* send DUMP_COMPLETED message (unconditionally) */
     result = qmp_query_dump(NULL);
     /* should never fail */
     assert(result);
-    qapi_event_send_dump_completed(result, !!local_err, (local_err ?
-                                   error_get_pretty(local_err) : NULL));
+    qapi_event_send_dump_completed(result, !!*errp, (*errp ?
+                                                     error_get_pretty(*errp) : NULL));
     qapi_free_DumpQueryResult(result);
 
-    error_propagate(errp, local_err);
     dump_cleanup(s);
 }
 
@@ -1916,10 +1895,10 @@ void qmp_dump_guest_memory(bool paging, const char *file,
                            int64_t length, bool has_format,
                            DumpGuestMemoryFormat format, Error **errp)
 {
+    ERRP_GUARD();
     const char *p;
     int fd = -1;
     DumpState *s;
-    Error *local_err = NULL;
     bool detach_p = false;
 
     if (runstate_check(RUN_STATE_INMIGRATE)) {
@@ -2019,9 +1998,8 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     dump_state_prepare(s);
 
     dump_init(s, fd, has_format, format, paging, has_begin,
-              begin, length, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+              begin, length, errp);
+    if (*errp) {
         qatomic_set(&s->status, DUMP_STATUS_FAILED);
         return;
     }
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 02/13] dump: Remove the sh_info variable
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 01/13] dump: Use ERRP_GUARD() marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 03/13] dump: Introduce shdr_num to decrease complexity marcandre.lureau
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

There's no need to have phdr_num and sh_info at the same time. We can
make phdr_num 32 bit and set PN_XNUM when we write the header if
phdr_num >= PN_XNUM.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220407094824.5074-1-frankja@linux.ibm.com>
---
 include/sysemu/dump.h |  3 +--
 dump/dump.c           | 44 +++++++++++++++++++++++--------------------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 250143cb5a71..b463fc9c0226 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -154,8 +154,7 @@ typedef struct DumpState {
     GuestPhysBlockList guest_phys_blocks;
     ArchDumpInfo dump_info;
     MemoryMappingList list;
-    uint16_t phdr_num;
-    uint32_t sh_info;
+    uint32_t phdr_num;
     bool have_section;
     bool resume;
     bool detached;
diff --git a/dump/dump.c b/dump/dump.c
index b91e9d8c123e..010b272da038 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -123,6 +123,12 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque)
 
 static void write_elf64_header(DumpState *s, Error **errp)
 {
+    /*
+     * phnum in the elf header is 16 bit, if we have more segments we
+     * set phnum to PN_XNUM and write the real number of segments to a
+     * special section.
+     */
+    uint16_t phnum = MIN(s->phdr_num, PN_XNUM);
     Elf64_Ehdr elf_header;
     int ret;
 
@@ -137,9 +143,9 @@ static void write_elf64_header(DumpState *s, Error **errp)
     elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
     elf_header.e_phoff = cpu_to_dump64(s, sizeof(Elf64_Ehdr));
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf64_Phdr));
-    elf_header.e_phnum = cpu_to_dump16(s, s->phdr_num);
+    elf_header.e_phnum = cpu_to_dump16(s, phnum);
     if (s->have_section) {
-        uint64_t shoff = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) * s->sh_info;
+        uint64_t shoff = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) * s->phdr_num;
 
         elf_header.e_shoff = cpu_to_dump64(s, shoff);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf64_Shdr));
@@ -154,6 +160,12 @@ static void write_elf64_header(DumpState *s, Error **errp)
 
 static void write_elf32_header(DumpState *s, Error **errp)
 {
+    /*
+     * phnum in the elf header is 16 bit, if we have more segments we
+     * set phnum to PN_XNUM and write the real number of segments to a
+     * special section.
+     */
+    uint16_t phnum = MIN(s->phdr_num, PN_XNUM);
     Elf32_Ehdr elf_header;
     int ret;
 
@@ -168,9 +180,9 @@ static void write_elf32_header(DumpState *s, Error **errp)
     elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
     elf_header.e_phoff = cpu_to_dump32(s, sizeof(Elf32_Ehdr));
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf32_Phdr));
-    elf_header.e_phnum = cpu_to_dump16(s, s->phdr_num);
+    elf_header.e_phnum = cpu_to_dump16(s, phnum);
     if (s->have_section) {
-        uint32_t shoff = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) * s->sh_info;
+        uint32_t shoff = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) * s->phdr_num;
 
         elf_header.e_shoff = cpu_to_dump32(s, shoff);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf32_Shdr));
@@ -357,12 +369,12 @@ static void write_elf_section(DumpState *s, int type, Error **errp)
     if (type == 0) {
         shdr_size = sizeof(Elf32_Shdr);
         memset(&shdr32, 0, shdr_size);
-        shdr32.sh_info = cpu_to_dump32(s, s->sh_info);
+        shdr32.sh_info = cpu_to_dump32(s, s->phdr_num);
         shdr = &shdr32;
     } else {
         shdr_size = sizeof(Elf64_Shdr);
         memset(&shdr64, 0, shdr_size);
-        shdr64.sh_info = cpu_to_dump32(s, s->sh_info);
+        shdr64.sh_info = cpu_to_dump32(s, s->phdr_num);
         shdr = &shdr64;
     }
 
@@ -477,13 +489,6 @@ static void write_elf_loads(DumpState *s, Error **errp)
     hwaddr offset, filesz;
     MemoryMapping *memory_mapping;
     uint32_t phdr_index = 1;
-    uint32_t max_index;
-
-    if (s->have_section) {
-        max_index = s->sh_info;
-    } else {
-        max_index = s->phdr_num;
-    }
 
     QTAILQ_FOREACH(memory_mapping, &s->list.head, next) {
         get_offset_range(memory_mapping->phys_addr,
@@ -501,7 +506,7 @@ static void write_elf_loads(DumpState *s, Error **errp)
             return;
         }
 
-        if (phdr_index >= max_index) {
+        if (phdr_index >= s->phdr_num) {
             break;
         }
     }
@@ -1800,22 +1805,21 @@ static void dump_init(DumpState *s, int fd, bool has_format,
         s->phdr_num += s->list.num;
         s->have_section = false;
     } else {
+        /* sh_info of section 0 holds the real number of phdrs */
         s->have_section = true;
-        s->phdr_num = PN_XNUM;
-        s->sh_info = 1; /* PT_NOTE */
 
         /* the type of shdr->sh_info is uint32_t, so we should avoid overflow */
         if (s->list.num <= UINT32_MAX - 1) {
-            s->sh_info += s->list.num;
+            s->phdr_num += s->list.num;
         } else {
-            s->sh_info = UINT32_MAX;
+            s->phdr_num = UINT32_MAX;
         }
     }
 
     if (s->dump_info.d_class == ELFCLASS64) {
         if (s->have_section) {
             s->memory_offset = sizeof(Elf64_Ehdr) +
-                               sizeof(Elf64_Phdr) * s->sh_info +
+                               sizeof(Elf64_Phdr) * s->phdr_num +
                                sizeof(Elf64_Shdr) + s->note_size;
         } else {
             s->memory_offset = sizeof(Elf64_Ehdr) +
@@ -1824,7 +1828,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     } else {
         if (s->have_section) {
             s->memory_offset = sizeof(Elf32_Ehdr) +
-                               sizeof(Elf32_Phdr) * s->sh_info +
+                               sizeof(Elf32_Phdr) * s->phdr_num +
                                sizeof(Elf32_Shdr) + s->note_size;
         } else {
             s->memory_offset = sizeof(Elf32_Ehdr) +
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 03/13] dump: Introduce shdr_num to decrease complexity
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 01/13] dump: Use ERRP_GUARD() marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 02/13] dump: Remove the sh_info variable marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 04/13] dump: Remove the section if when calculating the memory offset marcandre.lureau
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

Let's move from a boolean to a int variable which will later enable us
to store the number of sections that are in the dump file.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-4-frankja@linux.ibm.com>
---
 include/sysemu/dump.h |  2 +-
 dump/dump.c           | 24 ++++++++++++------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index b463fc9c0226..19458bffbd1d 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -155,7 +155,7 @@ typedef struct DumpState {
     ArchDumpInfo dump_info;
     MemoryMappingList list;
     uint32_t phdr_num;
-    bool have_section;
+    uint32_t shdr_num;
     bool resume;
     bool detached;
     ssize_t note_size;
diff --git a/dump/dump.c b/dump/dump.c
index 010b272da038..285ed4475b0d 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -144,12 +144,12 @@ static void write_elf64_header(DumpState *s, Error **errp)
     elf_header.e_phoff = cpu_to_dump64(s, sizeof(Elf64_Ehdr));
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf64_Phdr));
     elf_header.e_phnum = cpu_to_dump16(s, phnum);
-    if (s->have_section) {
+    if (s->shdr_num) {
         uint64_t shoff = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) * s->phdr_num;
 
         elf_header.e_shoff = cpu_to_dump64(s, shoff);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf64_Shdr));
-        elf_header.e_shnum = cpu_to_dump16(s, 1);
+        elf_header.e_shnum = cpu_to_dump16(s, s->shdr_num);
     }
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
@@ -181,12 +181,12 @@ static void write_elf32_header(DumpState *s, Error **errp)
     elf_header.e_phoff = cpu_to_dump32(s, sizeof(Elf32_Ehdr));
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf32_Phdr));
     elf_header.e_phnum = cpu_to_dump16(s, phnum);
-    if (s->have_section) {
+    if (s->shdr_num) {
         uint32_t shoff = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) * s->phdr_num;
 
         elf_header.e_shoff = cpu_to_dump32(s, shoff);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf32_Shdr));
-        elf_header.e_shnum = cpu_to_dump16(s, 1);
+        elf_header.e_shnum = cpu_to_dump16(s, s->shdr_num);
     }
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
@@ -565,7 +565,7 @@ static void dump_begin(DumpState *s, Error **errp)
         }
 
         /* write section to vmcore */
-        if (s->have_section) {
+        if (s->shdr_num) {
             write_elf_section(s, 1, errp);
             if (*errp) {
                 return;
@@ -591,7 +591,7 @@ static void dump_begin(DumpState *s, Error **errp)
         }
 
         /* write section to vmcore */
-        if (s->have_section) {
+        if (s->shdr_num) {
             write_elf_section(s, 0, errp);
             if (*errp) {
                 return;
@@ -1802,11 +1802,11 @@ static void dump_init(DumpState *s, int fd, bool has_format,
      */
     s->phdr_num = 1; /* PT_NOTE */
     if (s->list.num < UINT16_MAX - 2) {
+        s->shdr_num = 0;
         s->phdr_num += s->list.num;
-        s->have_section = false;
     } else {
         /* sh_info of section 0 holds the real number of phdrs */
-        s->have_section = true;
+        s->shdr_num = 1;
 
         /* the type of shdr->sh_info is uint32_t, so we should avoid overflow */
         if (s->list.num <= UINT32_MAX - 1) {
@@ -1817,19 +1817,19 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     }
 
     if (s->dump_info.d_class == ELFCLASS64) {
-        if (s->have_section) {
+        if (s->shdr_num) {
             s->memory_offset = sizeof(Elf64_Ehdr) +
                                sizeof(Elf64_Phdr) * s->phdr_num +
-                               sizeof(Elf64_Shdr) + s->note_size;
+                               sizeof(Elf64_Shdr) * s->shdr_num + s->note_size;
         } else {
             s->memory_offset = sizeof(Elf64_Ehdr) +
                                sizeof(Elf64_Phdr) * s->phdr_num + s->note_size;
         }
     } else {
-        if (s->have_section) {
+        if (s->shdr_num) {
             s->memory_offset = sizeof(Elf32_Ehdr) +
                                sizeof(Elf32_Phdr) * s->phdr_num +
-                               sizeof(Elf32_Shdr) + s->note_size;
+                               sizeof(Elf32_Shdr) * s->shdr_num + s->note_size;
         } else {
             s->memory_offset = sizeof(Elf32_Ehdr) +
                                sizeof(Elf32_Phdr) * s->phdr_num + s->note_size;
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 04/13] dump: Remove the section if when calculating the memory offset
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (2 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 03/13] dump: Introduce shdr_num to decrease complexity marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 05/13] dump: Add more offset variables marcandre.lureau
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

When s->shdr_num is 0 we'll add 0 bytes of section headers which is
equivalent to not adding section headers but with the multiplication
we can remove a if/else.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-5-frankja@linux.ibm.com>
---
 dump/dump.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 285ed4475b0d..9c80680eb2a4 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1817,23 +1817,15 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     }
 
     if (s->dump_info.d_class == ELFCLASS64) {
-        if (s->shdr_num) {
-            s->memory_offset = sizeof(Elf64_Ehdr) +
-                               sizeof(Elf64_Phdr) * s->phdr_num +
-                               sizeof(Elf64_Shdr) * s->shdr_num + s->note_size;
-        } else {
-            s->memory_offset = sizeof(Elf64_Ehdr) +
-                               sizeof(Elf64_Phdr) * s->phdr_num + s->note_size;
-        }
+        s->memory_offset = sizeof(Elf64_Ehdr) +
+                           sizeof(Elf64_Phdr) * s->phdr_num +
+                           sizeof(Elf64_Shdr) * s->shdr_num +
+                           s->note_size;
     } else {
-        if (s->shdr_num) {
-            s->memory_offset = sizeof(Elf32_Ehdr) +
-                               sizeof(Elf32_Phdr) * s->phdr_num +
-                               sizeof(Elf32_Shdr) * s->shdr_num + s->note_size;
-        } else {
-            s->memory_offset = sizeof(Elf32_Ehdr) +
-                               sizeof(Elf32_Phdr) * s->phdr_num + s->note_size;
-        }
+        s->memory_offset = sizeof(Elf32_Ehdr) +
+                           sizeof(Elf32_Phdr) * s->phdr_num +
+                           sizeof(Elf32_Shdr) * s->shdr_num +
+                           s->note_size;
     }
 
     return;
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 05/13] dump: Add more offset variables
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (3 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 04/13] dump: Remove the section if when calculating the memory offset marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 06/13] dump: Introduce dump_is_64bit() helper function marcandre.lureau
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

Offset calculations are easy enough to get wrong. Let's add a few
variables to make moving around elf headers and data sections easier.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220330123603.107120-6-frankja@linux.ibm.com>
---
 include/sysemu/dump.h |  4 ++++
 dump/dump.c           | 35 +++++++++++++++--------------------
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 19458bffbd1d..ffc2ea1072f3 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -159,6 +159,10 @@ typedef struct DumpState {
     bool resume;
     bool detached;
     ssize_t note_size;
+    hwaddr shdr_offset;
+    hwaddr phdr_offset;
+    hwaddr section_offset;
+    hwaddr note_offset;
     hwaddr memory_offset;
     int fd;
 
diff --git a/dump/dump.c b/dump/dump.c
index 9c80680eb2a4..7f226257eec3 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -141,13 +141,11 @@ static void write_elf64_header(DumpState *s, Error **errp)
     elf_header.e_machine = cpu_to_dump16(s, s->dump_info.d_machine);
     elf_header.e_version = cpu_to_dump32(s, EV_CURRENT);
     elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
-    elf_header.e_phoff = cpu_to_dump64(s, sizeof(Elf64_Ehdr));
+    elf_header.e_phoff = cpu_to_dump64(s, s->phdr_offset);
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf64_Phdr));
     elf_header.e_phnum = cpu_to_dump16(s, phnum);
     if (s->shdr_num) {
-        uint64_t shoff = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) * s->phdr_num;
-
-        elf_header.e_shoff = cpu_to_dump64(s, shoff);
+        elf_header.e_shoff = cpu_to_dump64(s, s->shdr_offset);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf64_Shdr));
         elf_header.e_shnum = cpu_to_dump16(s, s->shdr_num);
     }
@@ -178,13 +176,11 @@ static void write_elf32_header(DumpState *s, Error **errp)
     elf_header.e_machine = cpu_to_dump16(s, s->dump_info.d_machine);
     elf_header.e_version = cpu_to_dump32(s, EV_CURRENT);
     elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
-    elf_header.e_phoff = cpu_to_dump32(s, sizeof(Elf32_Ehdr));
+    elf_header.e_phoff = cpu_to_dump32(s, s->phdr_offset);
     elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf32_Phdr));
     elf_header.e_phnum = cpu_to_dump16(s, phnum);
     if (s->shdr_num) {
-        uint32_t shoff = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) * s->phdr_num;
-
-        elf_header.e_shoff = cpu_to_dump32(s, shoff);
+        elf_header.e_shoff = cpu_to_dump32(s, s->shdr_offset);
         elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf32_Shdr));
         elf_header.e_shnum = cpu_to_dump16(s, s->shdr_num);
     }
@@ -247,12 +243,11 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
 static void write_elf64_note(DumpState *s, Error **errp)
 {
     Elf64_Phdr phdr;
-    hwaddr begin = s->memory_offset - s->note_size;
     int ret;
 
     memset(&phdr, 0, sizeof(Elf64_Phdr));
     phdr.p_type = cpu_to_dump32(s, PT_NOTE);
-    phdr.p_offset = cpu_to_dump64(s, begin);
+    phdr.p_offset = cpu_to_dump64(s, s->note_offset);
     phdr.p_paddr = 0;
     phdr.p_filesz = cpu_to_dump64(s, s->note_size);
     phdr.p_memsz = cpu_to_dump64(s, s->note_size);
@@ -312,13 +307,12 @@ static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
 
 static void write_elf32_note(DumpState *s, Error **errp)
 {
-    hwaddr begin = s->memory_offset - s->note_size;
     Elf32_Phdr phdr;
     int ret;
 
     memset(&phdr, 0, sizeof(Elf32_Phdr));
     phdr.p_type = cpu_to_dump32(s, PT_NOTE);
-    phdr.p_offset = cpu_to_dump32(s, begin);
+    phdr.p_offset = cpu_to_dump32(s, s->note_offset);
     phdr.p_paddr = 0;
     phdr.p_filesz = cpu_to_dump32(s, s->note_size);
     phdr.p_memsz = cpu_to_dump32(s, s->note_size);
@@ -1817,15 +1811,16 @@ static void dump_init(DumpState *s, int fd, bool has_format,
     }
 
     if (s->dump_info.d_class == ELFCLASS64) {
-        s->memory_offset = sizeof(Elf64_Ehdr) +
-                           sizeof(Elf64_Phdr) * s->phdr_num +
-                           sizeof(Elf64_Shdr) * s->shdr_num +
-                           s->note_size;
+        s->phdr_offset = sizeof(Elf64_Ehdr);
+        s->shdr_offset = s->phdr_offset + sizeof(Elf64_Phdr) * s->phdr_num;
+        s->note_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
+        s->memory_offset = s->note_offset + s->note_size;
     } else {
-        s->memory_offset = sizeof(Elf32_Ehdr) +
-                           sizeof(Elf32_Phdr) * s->phdr_num +
-                           sizeof(Elf32_Shdr) * s->shdr_num +
-                           s->note_size;
+
+        s->phdr_offset = sizeof(Elf32_Ehdr);
+        s->shdr_offset = s->phdr_offset + sizeof(Elf32_Phdr) * s->phdr_num;
+        s->note_offset = s->shdr_offset + sizeof(Elf32_Shdr) * s->shdr_num;
+        s->memory_offset = s->note_offset + s->note_size;
     }
 
     return;
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 06/13] dump: Introduce dump_is_64bit() helper function
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (4 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 05/13] dump: Add more offset variables marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 07/13] dump: Consolidate phdr note writes marcandre.lureau
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

Checking d_class in dump_info leads to lengthy conditionals so let's
shorten things a bit by introducing a helper function.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-7-frankja@linux.ibm.com>
---
 dump/dump.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 7f226257eec3..b063db134021 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -54,6 +54,11 @@ static Error *dump_migration_blocker;
       DIV_ROUND_UP((name_size), 4) +                    \
       DIV_ROUND_UP((desc_size), 4)) * 4)
 
+static inline bool dump_is_64bit(DumpState *s)
+{
+    return s->dump_info.d_class == ELFCLASS64;
+}
+
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
 {
     if (s->dump_info.d_endian == ELFDATA2LSB) {
@@ -488,7 +493,7 @@ static void write_elf_loads(DumpState *s, Error **errp)
         get_offset_range(memory_mapping->phys_addr,
                          memory_mapping->length,
                          s, &offset, &filesz);
-        if (s->dump_info.d_class == ELFCLASS64) {
+        if (dump_is_64bit(s)) {
             write_elf64_load(s, memory_mapping, phdr_index++, offset,
                              filesz, errp);
         } else {
@@ -536,7 +541,7 @@ static void dump_begin(DumpState *s, Error **errp)
      */
 
     /* write elf header to vmcore */
-    if (s->dump_info.d_class == ELFCLASS64) {
+    if (dump_is_64bit(s)) {
         write_elf64_header(s, errp);
     } else {
         write_elf32_header(s, errp);
@@ -545,7 +550,7 @@ static void dump_begin(DumpState *s, Error **errp)
         return;
     }
 
-    if (s->dump_info.d_class == ELFCLASS64) {
+    if (dump_is_64bit(s)) {
         /* write PT_NOTE to vmcore */
         write_elf64_note(s, errp);
         if (*errp) {
@@ -756,7 +761,7 @@ static void get_note_sizes(DumpState *s, const void *note,
     uint64_t name_sz;
     uint64_t desc_sz;
 
-    if (s->dump_info.d_class == ELFCLASS64) {
+    if (dump_is_64bit(s)) {
         const Elf64_Nhdr *hdr = note;
         note_head_sz = sizeof(Elf64_Nhdr);
         name_sz = tswap64(hdr->n_namesz);
@@ -1016,10 +1021,10 @@ out:
 
 static void write_dump_header(DumpState *s, Error **errp)
 {
-    if (s->dump_info.d_class == ELFCLASS32) {
-        create_header32(s, errp);
-    } else {
+    if (dump_is_64bit(s)) {
         create_header64(s, errp);
+    } else {
+        create_header32(s, errp);
     }
 }
 
@@ -1706,8 +1711,8 @@ static void dump_init(DumpState *s, int fd, bool has_format,
         uint32_t size;
         uint16_t format;
 
-        note_head_size = s->dump_info.d_class == ELFCLASS32 ?
-            sizeof(Elf32_Nhdr) : sizeof(Elf64_Nhdr);
+        note_head_size = dump_is_64bit(s) ?
+            sizeof(Elf64_Nhdr) : sizeof(Elf32_Nhdr);
 
         format = le16_to_cpu(vmci->vmcoreinfo.guest_format);
         size = le32_to_cpu(vmci->vmcoreinfo.size);
@@ -1810,7 +1815,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
         }
     }
 
-    if (s->dump_info.d_class == ELFCLASS64) {
+    if (dump_is_64bit(s)) {
         s->phdr_offset = sizeof(Elf64_Ehdr);
         s->shdr_offset = s->phdr_offset + sizeof(Elf64_Phdr) * s->phdr_num;
         s->note_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 07/13] dump: Consolidate phdr note writes
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (5 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 06/13] dump: Introduce dump_is_64bit() helper function marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 08/13] dump: Cleanup dump_begin write functions marcandre.lureau
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

There's no need to have two write functions. Let's rather have two
functions that set the data for elf 32/64 and then write it in a
common function.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-8-frankja@linux.ibm.com>
---
 dump/dump.c | 94 +++++++++++++++++++++++++++--------------------------
 1 file changed, 48 insertions(+), 46 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index b063db134021..0d95fc5b7a3c 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -245,24 +245,15 @@ static void write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
     }
 }
 
-static void write_elf64_note(DumpState *s, Error **errp)
+static void write_elf64_phdr_note(DumpState *s, Elf64_Phdr *phdr)
 {
-    Elf64_Phdr phdr;
-    int ret;
-
-    memset(&phdr, 0, sizeof(Elf64_Phdr));
-    phdr.p_type = cpu_to_dump32(s, PT_NOTE);
-    phdr.p_offset = cpu_to_dump64(s, s->note_offset);
-    phdr.p_paddr = 0;
-    phdr.p_filesz = cpu_to_dump64(s, s->note_size);
-    phdr.p_memsz = cpu_to_dump64(s, s->note_size);
-    phdr.p_vaddr = 0;
-
-    ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s);
-    if (ret < 0) {
-        error_setg_errno(errp, -ret,
-                         "dump: failed to write program header table");
-    }
+    memset(phdr, 0, sizeof(*phdr));
+    phdr->p_type = cpu_to_dump32(s, PT_NOTE);
+    phdr->p_offset = cpu_to_dump64(s, s->note_offset);
+    phdr->p_paddr = 0;
+    phdr->p_filesz = cpu_to_dump64(s, s->note_size);
+    phdr->p_memsz = cpu_to_dump64(s, s->note_size);
+    phdr->p_vaddr = 0;
 }
 
 static inline int cpu_index(CPUState *cpu)
@@ -310,24 +301,15 @@ static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
     write_guest_note(f, s, errp);
 }
 
-static void write_elf32_note(DumpState *s, Error **errp)
+static void write_elf32_phdr_note(DumpState *s, Elf32_Phdr *phdr)
 {
-    Elf32_Phdr phdr;
-    int ret;
-
-    memset(&phdr, 0, sizeof(Elf32_Phdr));
-    phdr.p_type = cpu_to_dump32(s, PT_NOTE);
-    phdr.p_offset = cpu_to_dump32(s, s->note_offset);
-    phdr.p_paddr = 0;
-    phdr.p_filesz = cpu_to_dump32(s, s->note_size);
-    phdr.p_memsz = cpu_to_dump32(s, s->note_size);
-    phdr.p_vaddr = 0;
-
-    ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s);
-    if (ret < 0) {
-        error_setg_errno(errp, -ret,
-                         "dump: failed to write program header table");
-    }
+    memset(phdr, 0, sizeof(*phdr));
+    phdr->p_type = cpu_to_dump32(s, PT_NOTE);
+    phdr->p_offset = cpu_to_dump32(s, s->note_offset);
+    phdr->p_paddr = 0;
+    phdr->p_filesz = cpu_to_dump32(s, s->note_size);
+    phdr->p_memsz = cpu_to_dump32(s, s->note_size);
+    phdr->p_vaddr = 0;
 }
 
 static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
@@ -357,6 +339,32 @@ static void write_elf32_notes(WriteCoreDumpFunction f, DumpState *s,
     write_guest_note(f, s, errp);
 }
 
+static void write_elf_phdr_note(DumpState *s, Error **errp)
+{
+    ERRP_GUARD();
+    Elf32_Phdr phdr32;
+    Elf64_Phdr phdr64;
+    void *phdr;
+    size_t size;
+    int ret;
+
+    if (dump_is_64bit(s)) {
+        write_elf64_phdr_note(s, &phdr64);
+        size = sizeof(phdr64);
+        phdr = &phdr64;
+    } else {
+        write_elf32_phdr_note(s, &phdr32);
+        size = sizeof(phdr32);
+        phdr = &phdr32;
+    }
+
+    ret = fd_write_vmcore(phdr, size, s);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret,
+                         "dump: failed to write program header table");
+    }
+}
+
 static void write_elf_section(DumpState *s, int type, Error **errp)
 {
     Elf32_Shdr shdr32;
@@ -550,13 +558,13 @@ static void dump_begin(DumpState *s, Error **errp)
         return;
     }
 
-    if (dump_is_64bit(s)) {
-        /* write PT_NOTE to vmcore */
-        write_elf64_note(s, errp);
-        if (*errp) {
-            return;
-        }
+    /* write PT_NOTE to vmcore */
+    write_elf_phdr_note(s, errp);
+    if (*errp) {
+        return;
+    }
 
+    if (dump_is_64bit(s)) {
         /* write all PT_LOAD to vmcore */
         write_elf_loads(s, errp);
         if (*errp) {
@@ -577,12 +585,6 @@ static void dump_begin(DumpState *s, Error **errp)
             return;
         }
     } else {
-        /* write PT_NOTE to vmcore */
-        write_elf32_note(s, errp);
-        if (*errp) {
-            return;
-        }
-
         /* write all PT_LOAD to vmcore */
         write_elf_loads(s, errp);
         if (*errp) {
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 08/13] dump: Cleanup dump_begin write functions
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (6 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 07/13] dump: Consolidate phdr note writes marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 09/13] dump: Consolidate elf note function marcandre.lureau
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

There's no need to have a gigantic if in there let's move the elf
32/64 bit logic into the section, segment or note code.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-9-frankja@linux.ibm.com>
---
 dump/dump.c | 42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 0d95fc5b7a3c..929ef953515c 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -564,46 +564,26 @@ static void dump_begin(DumpState *s, Error **errp)
         return;
     }
 
-    if (dump_is_64bit(s)) {
-        /* write all PT_LOAD to vmcore */
-        write_elf_loads(s, errp);
+    /* write all PT_LOAD to vmcore */
+    write_elf_loads(s, errp);
+    if (*errp) {
+        return;
+    }
+
+    /* write section to vmcore */
+    if (s->shdr_num) {
+        write_elf_section(s, 1, errp);
         if (*errp) {
             return;
         }
+    }
 
-        /* write section to vmcore */
-        if (s->shdr_num) {
-            write_elf_section(s, 1, errp);
-            if (*errp) {
-                return;
-            }
-        }
-
+    if (dump_is_64bit(s)) {
         /* write notes to vmcore */
         write_elf64_notes(fd_write_vmcore, s, errp);
-        if (*errp) {
-            return;
-        }
     } else {
-        /* write all PT_LOAD to vmcore */
-        write_elf_loads(s, errp);
-        if (*errp) {
-            return;
-        }
-
-        /* write section to vmcore */
-        if (s->shdr_num) {
-            write_elf_section(s, 0, errp);
-            if (*errp) {
-                return;
-            }
-        }
-
         /* write notes to vmcore */
         write_elf32_notes(fd_write_vmcore, s, errp);
-        if (*errp) {
-            return;
-        }
     }
 }
 
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 09/13] dump: Consolidate elf note function
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (7 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 08/13] dump: Cleanup dump_begin write functions marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 10/13] include/qemu: rename Windows context definitions to expose bitness marcandre.lureau
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Janosch Frank <frankja@linux.ibm.com>

Just like with the other write functions let's move the 32/64 bit elf
handling to a function to improve readability.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220330123603.107120-10-frankja@linux.ibm.com>
---
 dump/dump.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 929ef953515c..4d9658ffa24f 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -519,6 +519,15 @@ static void write_elf_loads(DumpState *s, Error **errp)
     }
 }
 
+static void write_elf_notes(DumpState *s, Error **errp)
+{
+    if (dump_is_64bit(s)) {
+        write_elf64_notes(fd_write_vmcore, s, errp);
+    } else {
+        write_elf32_notes(fd_write_vmcore, s, errp);
+    }
+}
+
 /* write elf header, PT_NOTE and elf note to vmcore. */
 static void dump_begin(DumpState *s, Error **errp)
 {
@@ -578,13 +587,8 @@ static void dump_begin(DumpState *s, Error **errp)
         }
     }
 
-    if (dump_is_64bit(s)) {
-        /* write notes to vmcore */
-        write_elf64_notes(fd_write_vmcore, s, errp);
-    } else {
-        /* write notes to vmcore */
-        write_elf32_notes(fd_write_vmcore, s, errp);
-    }
+    /* write notes to vmcore */
+    write_elf_notes(s, errp);
 }
 
 static int get_next_block(DumpState *s, GuestPhysBlock *block)
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 10/13] include/qemu: rename Windows context definitions to expose bitness
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (8 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 09/13] dump: Consolidate elf note function marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 11/13] dump/win_dump: add helper macros for Windows dump header access marcandre.lureau
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson,
	frankja, Viktor Prutyanov

From: Viktor Prutyanov <viktor.prutyanov@redhat.com>

Context structure in 64-bit Windows differs from 32-bit one and it
should be reflected in its name.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220406171558.199263-2-viktor.prutyanov@redhat.com>
---
 include/qemu/win_dump_defs.h |  8 ++++----
 contrib/elf2dmp/main.c       |  6 +++---
 dump/win_dump.c              | 14 +++++++-------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/qemu/win_dump_defs.h b/include/qemu/win_dump_defs.h
index 145096e8ee79..5a5e5a5e0989 100644
--- a/include/qemu/win_dump_defs.h
+++ b/include/qemu/win_dump_defs.h
@@ -97,8 +97,8 @@ typedef struct WinDumpHeader64 {
 #define WIN_CTX_FP  0x00000008L
 #define WIN_CTX_DBG 0x00000010L
 
-#define WIN_CTX_FULL    (WIN_CTX_X64 | WIN_CTX_CTL | WIN_CTX_INT | WIN_CTX_FP)
-#define WIN_CTX_ALL     (WIN_CTX_FULL | WIN_CTX_SEG | WIN_CTX_DBG)
+#define WIN_CTX64_FULL  (WIN_CTX_X64 | WIN_CTX_CTL | WIN_CTX_INT | WIN_CTX_FP)
+#define WIN_CTX64_ALL   (WIN_CTX64_FULL | WIN_CTX_SEG | WIN_CTX_DBG)
 
 #define LIVE_SYSTEM_DUMP    0x00000161
 
@@ -107,7 +107,7 @@ typedef struct WinM128A {
     int64_t high;
 } QEMU_ALIGNED(16) WinM128A;
 
-typedef struct WinContext {
+typedef struct WinContext64 {
     uint64_t PHome[6];
 
     uint32_t ContextFlags;
@@ -174,6 +174,6 @@ typedef struct WinContext {
     uint64_t LastBranchFromRip;
     uint64_t LastExceptionToRip;
     uint64_t LastExceptionFromRip;
-} QEMU_ALIGNED(16) WinContext;
+} QEMU_ALIGNED(16) WinContext64;
 
 #endif /* QEMU_WIN_DUMP_DEFS_H */
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 20b477d582a4..b9fc6d230ca0 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -141,10 +141,10 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t KernBase, struct pdb_reader *pdb,
     return kdbg;
 }
 
-static void win_context_init_from_qemu_cpu_state(WinContext *ctx,
+static void win_context_init_from_qemu_cpu_state(WinContext64 *ctx,
         QEMUCPUState *s)
 {
-    WinContext win_ctx = (WinContext){
+    WinContext64 win_ctx = (WinContext64){
         .ContextFlags = WIN_CTX_X64 | WIN_CTX_INT | WIN_CTX_SEG | WIN_CTX_CTL,
         .MxCsr = INITIAL_MXCSR,
 
@@ -302,7 +302,7 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
     for (i = 0; i < qe->state_nr; i++) {
         uint64_t Prcb;
         uint64_t Context;
-        WinContext ctx;
+        WinContext64 ctx;
         QEMUCPUState *s = qe->state[i];
 
         if (va_space_rw(vs, kdbg->KiProcessorBlock + sizeof(Prcb) * i,
diff --git a/dump/win_dump.c b/dump/win_dump.c
index fbdbb7bd93a6..e9215e4fd5e5 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -188,7 +188,7 @@ try_again:
 }
 
 struct saved_context {
-    WinContext ctx;
+    WinContext64 ctx;
     uint64_t addr;
 };
 
@@ -220,7 +220,7 @@ static void patch_and_save_context(WinDumpHeader64 *h,
         CPUX86State *env = &x86_cpu->env;
         uint64_t Prcb;
         uint64_t Context;
-        WinContext ctx;
+        WinContext64 ctx;
 
         if (cpu_memory_rw_debug(first_cpu,
                 KiProcessorBlock + i * sizeof(uint64_t),
@@ -240,8 +240,8 @@ static void patch_and_save_context(WinDumpHeader64 *h,
 
         saved_ctx[i].addr = Context;
 
-        ctx = (WinContext){
-            .ContextFlags = WIN_CTX_ALL,
+        ctx = (WinContext64){
+            .ContextFlags = WIN_CTX64_ALL,
             .MxCsr = env->mxcsr,
 
             .SegEs = env->segs[0].selector,
@@ -283,13 +283,13 @@ static void patch_and_save_context(WinDumpHeader64 *h,
         };
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext), 0)) {
+                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext64), 0)) {
             error_setg(errp, "win-dump: failed to save CPU #%d context", i);
             return;
         }
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                (uint8_t *)&ctx, sizeof(WinContext), 1)) {
+                (uint8_t *)&ctx, sizeof(WinContext64), 1)) {
             error_setg(errp, "win-dump: failed to write CPU #%d context", i);
             return;
         }
@@ -305,7 +305,7 @@ static void restore_context(WinDumpHeader64 *h,
 
     for (i = 0; i < h->NumberProcessors; i++) {
         if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
-                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext), 1)) {
+                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext64), 1)) {
             warn_report("win-dump: failed to restore CPU #%d context", i);
         }
     }
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 11/13] dump/win_dump: add helper macros for Windows dump header access
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (9 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 10/13] include/qemu: rename Windows context definitions to expose bitness marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 12/13] include/qemu: add 32-bit Windows dump structures marcandre.lureau
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Viktor Prutyanov <viktor.prutyanov@redhat.com>

Perform read access to Windows dump header fields via helper macros.
This is preparation for the next 32-bit guest Windows dump support.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220406171558.199263-3-viktor.prutyanov@redhat.com>
---
 dump/win_dump.c | 100 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 35 deletions(-)

diff --git a/dump/win_dump.c b/dump/win_dump.c
index e9215e4fd5e5..d733918038b2 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -23,11 +23,25 @@
 #include "hw/misc/vmcoreinfo.h"
 #include "win_dump.h"
 
-static size_t write_run(WinDumpPhyMemRun64 *run, int fd, Error **errp)
+#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
+
+#define _WIN_DUMP_FIELD(f) (h->f)
+#define WIN_DUMP_FIELD(field) _WIN_DUMP_FIELD(field)
+
+#define _WIN_DUMP_FIELD_PTR(f) ((void *)&h->f)
+#define WIN_DUMP_FIELD_PTR(field) _WIN_DUMP_FIELD_PTR(field)
+
+#define _WIN_DUMP_FIELD_SIZE(f) sizeof(h->f)
+#define WIN_DUMP_FIELD_SIZE(field) _WIN_DUMP_FIELD_SIZE(field)
+
+#define WIN_DUMP_CTX_SIZE sizeof(WinContext64)
+
+static size_t write_run(uint64_t base_page, uint64_t page_count,
+        int fd, Error **errp)
 {
     void *buf;
-    uint64_t addr = run->BasePage << TARGET_PAGE_BITS;
-    uint64_t size = run->PageCount << TARGET_PAGE_BITS;
+    uint64_t addr = base_page << TARGET_PAGE_BITS;
+    uint64_t size = page_count << TARGET_PAGE_BITS;
     uint64_t len, l;
     size_t total = 0;
 
@@ -58,13 +72,14 @@ static size_t write_run(WinDumpPhyMemRun64 *run, int fd, Error **errp)
 
 static void write_runs(DumpState *s, WinDumpHeader64 *h, Error **errp)
 {
-    WinDumpPhyMemDesc64 *desc = &h->PhysicalMemoryBlock;
-    WinDumpPhyMemRun64 *run = desc->Run;
+    uint64_t BasePage, PageCount;
     Error *local_err = NULL;
     int i;
 
-    for (i = 0; i < desc->NumberOfRuns; i++) {
-        s->written_size += write_run(run + i, s->fd, &local_err);
+    for (i = 0; i < WIN_DUMP_FIELD(PhysicalMemoryBlock.NumberOfRuns); i++) {
+        BasePage = WIN_DUMP_FIELD(PhysicalMemoryBlock.Run[i].BasePage);
+        PageCount = WIN_DUMP_FIELD(PhysicalMemoryBlock.Run[i].PageCount);
+        s->written_size += write_run(BasePage, PageCount, s->fd, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             return;
@@ -72,11 +87,24 @@ static void write_runs(DumpState *s, WinDumpHeader64 *h, Error **errp)
     }
 }
 
+static int cpu_read_ptr(CPUState *cpu, uint64_t addr, uint64_t *ptr)
+{
+    int ret;
+    uint64_t ptr64;
+
+    ret = cpu_memory_rw_debug(cpu, addr, &ptr64, WIN_DUMP_PTR_SIZE, 0);
+
+    *ptr = ptr64;
+
+    return ret;
+}
+
 static void patch_mm_pfn_database(WinDumpHeader64 *h, Error **errp)
 {
     if (cpu_memory_rw_debug(first_cpu,
-            h->KdDebuggerDataBlock + KDBG_MM_PFN_DATABASE_OFFSET64,
-            (uint8_t *)&h->PfnDatabase, sizeof(h->PfnDatabase), 0)) {
+            WIN_DUMP_FIELD(KdDebuggerDataBlock) + KDBG_MM_PFN_DATABASE_OFFSET64,
+            WIN_DUMP_FIELD_PTR(PfnDatabase),
+            WIN_DUMP_FIELD_SIZE(PfnDatabase), 0)) {
         error_setg(errp, "win-dump: failed to read MmPfnDatabase");
         return;
     }
@@ -86,16 +114,17 @@ static void patch_bugcheck_data(WinDumpHeader64 *h, Error **errp)
 {
     uint64_t KiBugcheckData;
 
-    if (cpu_memory_rw_debug(first_cpu,
-            h->KdDebuggerDataBlock + KDBG_KI_BUGCHECK_DATA_OFFSET64,
-            (uint8_t *)&KiBugcheckData, sizeof(KiBugcheckData), 0)) {
+    if (cpu_read_ptr(first_cpu,
+            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
+                KDBG_KI_BUGCHECK_DATA_OFFSET64,
+            &KiBugcheckData)) {
         error_setg(errp, "win-dump: failed to read KiBugcheckData");
         return;
     }
 
-    if (cpu_memory_rw_debug(first_cpu,
-            KiBugcheckData,
-            h->BugcheckData, sizeof(h->BugcheckData), 0)) {
+    if (cpu_memory_rw_debug(first_cpu, KiBugcheckData,
+            WIN_DUMP_FIELD(BugcheckData),
+            WIN_DUMP_FIELD_SIZE(BugcheckData), 0)) {
         error_setg(errp, "win-dump: failed to read bugcheck data");
         return;
     }
@@ -104,8 +133,8 @@ static void patch_bugcheck_data(WinDumpHeader64 *h, Error **errp)
      * If BugcheckCode wasn't saved, we consider guest OS as alive.
      */
 
-    if (!h->BugcheckCode) {
-        h->BugcheckCode = LIVE_SYSTEM_DUMP;
+    if (!WIN_DUMP_FIELD(BugcheckCode)) {
+        *(uint32_t *)WIN_DUMP_FIELD_PTR(BugcheckCode) = LIVE_SYSTEM_DUMP;
     }
 }
 
@@ -154,7 +183,7 @@ static void check_kdbg(WinDumpHeader64 *h, Error **errp)
 {
     const char OwnerTag[] = "KDBG";
     char read_OwnerTag[4];
-    uint64_t KdDebuggerDataBlock = h->KdDebuggerDataBlock;
+    uint64_t KdDebuggerDataBlock = WIN_DUMP_FIELD(KdDebuggerDataBlock);
     bool try_fallback = true;
 
 try_again:
@@ -173,7 +202,7 @@ try_again:
              * we try to use KDBG obtained by guest driver.
              */
 
-            KdDebuggerDataBlock = h->BugcheckParameter1;
+            KdDebuggerDataBlock = WIN_DUMP_FIELD(BugcheckParameter1);
             try_fallback = false;
             goto try_again;
         } else {
@@ -196,20 +225,21 @@ static void patch_and_save_context(WinDumpHeader64 *h,
                                    struct saved_context *saved_ctx,
                                    Error **errp)
 {
+    uint64_t KdDebuggerDataBlock = WIN_DUMP_FIELD(KdDebuggerDataBlock);
     uint64_t KiProcessorBlock;
     uint16_t OffsetPrcbContext;
     CPUState *cpu;
     int i = 0;
 
-    if (cpu_memory_rw_debug(first_cpu,
-            h->KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET64,
-            (uint8_t *)&KiProcessorBlock, sizeof(KiProcessorBlock), 0)) {
+    if (cpu_read_ptr(first_cpu,
+            KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET64,
+            &KiProcessorBlock)) {
         error_setg(errp, "win-dump: failed to read KiProcessorBlock");
         return;
     }
 
     if (cpu_memory_rw_debug(first_cpu,
-            h->KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET64,
+            KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET64,
             (uint8_t *)&OffsetPrcbContext, sizeof(OffsetPrcbContext), 0)) {
         error_setg(errp, "win-dump: failed to read OffsetPrcbContext");
         return;
@@ -222,17 +252,17 @@ static void patch_and_save_context(WinDumpHeader64 *h,
         uint64_t Context;
         WinContext64 ctx;
 
-        if (cpu_memory_rw_debug(first_cpu,
-                KiProcessorBlock + i * sizeof(uint64_t),
-                (uint8_t *)&Prcb, sizeof(Prcb), 0)) {
+        if (cpu_read_ptr(first_cpu,
+                KiProcessorBlock + i * WIN_DUMP_PTR_SIZE,
+                &Prcb)) {
             error_setg(errp, "win-dump: failed to read"
                              " CPU #%d PRCB location", i);
             return;
         }
 
-        if (cpu_memory_rw_debug(first_cpu,
+        if (cpu_read_ptr(first_cpu,
                 Prcb + OffsetPrcbContext,
-                (uint8_t *)&Context, sizeof(Context), 0)) {
+                &Context)) {
             error_setg(errp, "win-dump: failed to read"
                              " CPU #%d ContextFrame location", i);
             return;
@@ -283,13 +313,13 @@ static void patch_and_save_context(WinDumpHeader64 *h,
         };
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext64), 0)) {
+                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 0)) {
             error_setg(errp, "win-dump: failed to save CPU #%d context", i);
             return;
         }
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                (uint8_t *)&ctx, sizeof(WinContext64), 1)) {
+                &ctx, WIN_DUMP_CTX_SIZE, 1)) {
             error_setg(errp, "win-dump: failed to write CPU #%d context", i);
             return;
         }
@@ -303,9 +333,9 @@ static void restore_context(WinDumpHeader64 *h,
 {
     int i;
 
-    for (i = 0; i < h->NumberProcessors; i++) {
+    for (i = 0; i < WIN_DUMP_FIELD(NumberProcessors); i++) {
         if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
-                (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext64), 1)) {
+                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 1)) {
             warn_report("win-dump: failed to restore CPU #%d context", i);
         }
     }
@@ -337,7 +367,7 @@ void create_win_dump(DumpState *s, Error **errp)
      * should be made from system context.
      */
 
-    first_x86_cpu->env.cr[3] = h->DirectoryTableBase;
+    first_x86_cpu->env.cr[3] = WIN_DUMP_FIELD(DirectoryTableBase);
 
     check_kdbg(h, &local_err);
     if (local_err) {
@@ -347,7 +377,7 @@ void create_win_dump(DumpState *s, Error **errp)
 
     patch_header(h);
 
-    saved_ctx = g_new(struct saved_context, h->NumberProcessors);
+    saved_ctx = g_new(struct saved_context, WIN_DUMP_FIELD(NumberProcessors));
 
     /*
      * Always patch context because there is no way
@@ -360,7 +390,7 @@ void create_win_dump(DumpState *s, Error **errp)
         goto out_free;
     }
 
-    s->total_size = h->RequiredDumpSpace;
+    s->total_size = WIN_DUMP_FIELD(RequiredDumpSpace);
 
     s->written_size = qemu_write_full(s->fd, h, sizeof(*h));
     if (s->written_size != sizeof(*h)) {
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 12/13] include/qemu: add 32-bit Windows dump structures
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (10 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 11/13] dump/win_dump: add helper macros for Windows dump header access marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:15 ` [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
  2022-04-22 13:16 ` [PULL v2 00/13] Dump patches Richard Henderson
  13 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson, frankja

From: Viktor Prutyanov <viktor.prutyanov@redhat.com>

These structures are required to produce 32-bit guest Windows Complete
Memory Dump. Add 32-bit Windows dump header, CPU context and physical
memory descriptor structures along with corresponding definitions.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220406171558.199263-4-viktor.prutyanov@redhat.com>
---
 include/qemu/win_dump_defs.h | 107 +++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/include/qemu/win_dump_defs.h b/include/qemu/win_dump_defs.h
index 5a5e5a5e0989..73a44e2408c2 100644
--- a/include/qemu/win_dump_defs.h
+++ b/include/qemu/win_dump_defs.h
@@ -11,11 +11,22 @@
 #ifndef QEMU_WIN_DUMP_DEFS_H
 #define QEMU_WIN_DUMP_DEFS_H
 
+typedef struct WinDumpPhyMemRun32 {
+    uint32_t BasePage;
+    uint32_t PageCount;
+} QEMU_PACKED WinDumpPhyMemRun32;
+
 typedef struct WinDumpPhyMemRun64 {
     uint64_t BasePage;
     uint64_t PageCount;
 } QEMU_PACKED WinDumpPhyMemRun64;
 
+typedef struct WinDumpPhyMemDesc32 {
+    uint32_t NumberOfRuns;
+    uint32_t NumberOfPages;
+    WinDumpPhyMemRun32 Run[86];
+} QEMU_PACKED WinDumpPhyMemDesc32;
+
 typedef struct WinDumpPhyMemDesc64 {
     uint32_t NumberOfRuns;
     uint32_t unused;
@@ -33,6 +44,39 @@ typedef struct WinDumpExceptionRecord {
     uint64_t ExceptionInformation[15];
 } QEMU_PACKED WinDumpExceptionRecord;
 
+typedef struct WinDumpHeader32 {
+    char Signature[4];
+    char ValidDump[4];
+    uint32_t MajorVersion;
+    uint32_t MinorVersion;
+    uint32_t DirectoryTableBase;
+    uint32_t PfnDatabase;
+    uint32_t PsLoadedModuleList;
+    uint32_t PsActiveProcessHead;
+    uint32_t MachineImageType;
+    uint32_t NumberProcessors;
+    union {
+        struct {
+            uint32_t BugcheckCode;
+            uint32_t BugcheckParameter1;
+            uint32_t BugcheckParameter2;
+            uint32_t BugcheckParameter3;
+            uint32_t BugcheckParameter4;
+        };
+        uint8_t BugcheckData[20];
+    };
+    uint8_t VersionUser[32];
+    uint32_t reserved0;
+    uint32_t KdDebuggerDataBlock;
+    union {
+        WinDumpPhyMemDesc32 PhysicalMemoryBlock;
+        uint8_t PhysicalMemoryBlockBuffer[700];
+    };
+    uint8_t reserved1[3200];
+    uint32_t RequiredDumpSpace;
+    uint8_t reserved2[92];
+} QEMU_PACKED WinDumpHeader32;
+
 typedef struct WinDumpHeader64 {
     char Signature[4];
     char ValidDump[4];
@@ -81,25 +125,49 @@ typedef struct WinDumpHeader64 {
     uint8_t reserved[4018];
 } QEMU_PACKED WinDumpHeader64;
 
+typedef union WinDumpHeader {
+    struct {
+        char Signature[4];
+        char ValidDump[4];
+    };
+    WinDumpHeader32 x32;
+    WinDumpHeader64 x64;
+} WinDumpHeader;
+
 #define KDBG_OWNER_TAG_OFFSET64             0x10
 #define KDBG_MM_PFN_DATABASE_OFFSET64       0xC0
 #define KDBG_KI_BUGCHECK_DATA_OFFSET64      0x88
 #define KDBG_KI_PROCESSOR_BLOCK_OFFSET64    0x218
 #define KDBG_OFFSET_PRCB_CONTEXT_OFFSET64   0x338
 
+#define KDBG_OWNER_TAG_OFFSET           KDBG_OWNER_TAG_OFFSET64
+#define KDBG_MM_PFN_DATABASE_OFFSET     KDBG_MM_PFN_DATABASE_OFFSET64
+#define KDBG_KI_BUGCHECK_DATA_OFFSET    KDBG_KI_BUGCHECK_DATA_OFFSET64
+#define KDBG_KI_PROCESSOR_BLOCK_OFFSET  KDBG_KI_PROCESSOR_BLOCK_OFFSET64
+#define KDBG_OFFSET_PRCB_CONTEXT_OFFSET KDBG_OFFSET_PRCB_CONTEXT_OFFSET64
+
 #define VMCOREINFO_ELF_NOTE_HDR_SIZE    24
+#define VMCOREINFO_WIN_DUMP_NOTE_SIZE64 (sizeof(WinDumpHeader64) + \
+                                         VMCOREINFO_ELF_NOTE_HDR_SIZE)
+#define VMCOREINFO_WIN_DUMP_NOTE_SIZE32 (sizeof(WinDumpHeader32) + \
+                                         VMCOREINFO_ELF_NOTE_HDR_SIZE)
 
 #define WIN_CTX_X64 0x00100000L
+#define WIN_CTX_X86 0x00010000L
 
 #define WIN_CTX_CTL 0x00000001L
 #define WIN_CTX_INT 0x00000002L
 #define WIN_CTX_SEG 0x00000004L
 #define WIN_CTX_FP  0x00000008L
 #define WIN_CTX_DBG 0x00000010L
+#define WIN_CTX_EXT 0x00000020L
 
 #define WIN_CTX64_FULL  (WIN_CTX_X64 | WIN_CTX_CTL | WIN_CTX_INT | WIN_CTX_FP)
 #define WIN_CTX64_ALL   (WIN_CTX64_FULL | WIN_CTX_SEG | WIN_CTX_DBG)
 
+#define WIN_CTX32_FULL (WIN_CTX_X86 | WIN_CTX_CTL | WIN_CTX_INT | WIN_CTX_SEG)
+#define WIN_CTX32_ALL (WIN_CTX32_FULL | WIN_CTX_FP | WIN_CTX_DBG | WIN_CTX_EXT)
+
 #define LIVE_SYSTEM_DUMP    0x00000161
 
 typedef struct WinM128A {
@@ -107,6 +175,40 @@ typedef struct WinM128A {
     int64_t high;
 } QEMU_ALIGNED(16) WinM128A;
 
+typedef struct WinContext32 {
+    uint32_t ContextFlags;
+
+    uint32_t Dr0;
+    uint32_t Dr1;
+    uint32_t Dr2;
+    uint32_t Dr3;
+    uint32_t Dr6;
+    uint32_t Dr7;
+
+    uint8_t  FloatSave[112];
+
+    uint32_t SegGs;
+    uint32_t SegFs;
+    uint32_t SegEs;
+    uint32_t SegDs;
+
+    uint32_t Edi;
+    uint32_t Esi;
+    uint32_t Ebx;
+    uint32_t Edx;
+    uint32_t Ecx;
+    uint32_t Eax;
+
+    uint32_t Ebp;
+    uint32_t Eip;
+    uint32_t SegCs;
+    uint32_t EFlags;
+    uint32_t Esp;
+    uint32_t SegSs;
+
+    uint8_t ExtendedRegisters[512];
+} QEMU_ALIGNED(16) WinContext32;
+
 typedef struct WinContext64 {
     uint64_t PHome[6];
 
@@ -176,4 +278,9 @@ typedef struct WinContext64 {
     uint64_t LastExceptionFromRip;
 } QEMU_ALIGNED(16) WinContext64;
 
+typedef union WinContext {
+    WinContext32 x32;
+    WinContext64 x64;
+} WinContext;
+
 #endif /* QEMU_WIN_DUMP_DEFS_H */
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (11 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 12/13] include/qemu: add 32-bit Windows dump structures marcandre.lureau
@ 2022-04-22 10:15 ` marcandre.lureau
  2022-04-22 10:25   ` Viktor Prutyanov
  2022-04-22 13:16 ` [PULL v2 00/13] Dump patches Richard Henderson
  13 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2022-04-22 10:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, viktor.prutyanov, richard.henderson,
	frankja, Dr. David Alan Gilbert

From: Viktor Prutyanov <viktor.prutyanov@redhat.com>

Before this patch, 'dump-guest-memory -w' was accepting only 64-bit
dump header provided by guest through vmcoreinfo and thus was unable
to produce 32-bit guest Windows dump. So, add 32-bit guest Windows
dumping support.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[ misc error handling fixes to avoid compiler warning ]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220406171558.199263-5-viktor.prutyanov@redhat.com>
---
 dump/win_dump.c | 251 +++++++++++++++++++++++++++++-------------------
 hmp-commands.hx |   2 +-
 2 files changed, 154 insertions(+), 99 deletions(-)

diff --git a/dump/win_dump.c b/dump/win_dump.c
index d733918038b2..fd91350fbb8e 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -23,18 +23,24 @@
 #include "hw/misc/vmcoreinfo.h"
 #include "win_dump.h"
 
-#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
+static size_t win_dump_ptr_size(bool x64)
+{
+    return x64 ? sizeof(uint64_t) : sizeof(uint32_t);
+}
 
-#define _WIN_DUMP_FIELD(f) (h->f)
+#define _WIN_DUMP_FIELD(f) (x64 ? h->x64.f : h->x32.f)
 #define WIN_DUMP_FIELD(field) _WIN_DUMP_FIELD(field)
 
-#define _WIN_DUMP_FIELD_PTR(f) ((void *)&h->f)
+#define _WIN_DUMP_FIELD_PTR(f) (x64 ? (void *)&h->x64.f : (void *)&h->x32.f)
 #define WIN_DUMP_FIELD_PTR(field) _WIN_DUMP_FIELD_PTR(field)
 
-#define _WIN_DUMP_FIELD_SIZE(f) sizeof(h->f)
+#define _WIN_DUMP_FIELD_SIZE(f) (x64 ? sizeof(h->x64.f) : sizeof(h->x32.f))
 #define WIN_DUMP_FIELD_SIZE(field) _WIN_DUMP_FIELD_SIZE(field)
 
-#define WIN_DUMP_CTX_SIZE sizeof(WinContext64)
+static size_t win_dump_ctx_size(bool x64)
+{
+    return x64 ? sizeof(WinContext64) : sizeof(WinContext32);
+}
 
 static size_t write_run(uint64_t base_page, uint64_t page_count,
         int fd, Error **errp)
@@ -70,7 +76,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
     return total;
 }
 
-static void write_runs(DumpState *s, WinDumpHeader64 *h, Error **errp)
+static void write_runs(DumpState *s, WinDumpHeader *h, bool x64, Error **errp)
 {
     uint64_t BasePage, PageCount;
     Error *local_err = NULL;
@@ -87,22 +93,24 @@ static void write_runs(DumpState *s, WinDumpHeader64 *h, Error **errp)
     }
 }
 
-static int cpu_read_ptr(CPUState *cpu, uint64_t addr, uint64_t *ptr)
+static int cpu_read_ptr(bool x64, CPUState *cpu, uint64_t addr, uint64_t *ptr)
 {
     int ret;
+    uint32_t ptr32;
     uint64_t ptr64;
 
-    ret = cpu_memory_rw_debug(cpu, addr, &ptr64, WIN_DUMP_PTR_SIZE, 0);
+    ret = cpu_memory_rw_debug(cpu, addr, x64 ? (void *)&ptr64 : (void *)&ptr32,
+            win_dump_ptr_size(x64), 0);
 
-    *ptr = ptr64;
+    *ptr = x64 ? ptr64 : ptr32;
 
     return ret;
 }
 
-static void patch_mm_pfn_database(WinDumpHeader64 *h, Error **errp)
+static void patch_mm_pfn_database(WinDumpHeader *h, bool x64, Error **errp)
 {
     if (cpu_memory_rw_debug(first_cpu,
-            WIN_DUMP_FIELD(KdDebuggerDataBlock) + KDBG_MM_PFN_DATABASE_OFFSET64,
+            WIN_DUMP_FIELD(KdDebuggerDataBlock) + KDBG_MM_PFN_DATABASE_OFFSET,
             WIN_DUMP_FIELD_PTR(PfnDatabase),
             WIN_DUMP_FIELD_SIZE(PfnDatabase), 0)) {
         error_setg(errp, "win-dump: failed to read MmPfnDatabase");
@@ -110,13 +118,12 @@ static void patch_mm_pfn_database(WinDumpHeader64 *h, Error **errp)
     }
 }
 
-static void patch_bugcheck_data(WinDumpHeader64 *h, Error **errp)
+static void patch_bugcheck_data(WinDumpHeader *h, bool x64, Error **errp)
 {
     uint64_t KiBugcheckData;
 
-    if (cpu_read_ptr(first_cpu,
-            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
-                KDBG_KI_BUGCHECK_DATA_OFFSET64,
+    if (cpu_read_ptr(x64, first_cpu,
+            WIN_DUMP_FIELD(KdDebuggerDataBlock) + KDBG_KI_BUGCHECK_DATA_OFFSET,
             &KiBugcheckData)) {
         error_setg(errp, "win-dump: failed to read KiBugcheckData");
         return;
@@ -141,45 +148,55 @@ static void patch_bugcheck_data(WinDumpHeader64 *h, Error **errp)
 /*
  * This routine tries to correct mistakes in crashdump header.
  */
-static void patch_header(WinDumpHeader64 *h)
+static void patch_header(WinDumpHeader *h, bool x64)
 {
     Error *local_err = NULL;
 
-    h->RequiredDumpSpace = sizeof(WinDumpHeader64) +
-            (h->PhysicalMemoryBlock.NumberOfPages << TARGET_PAGE_BITS);
-    h->PhysicalMemoryBlock.unused = 0;
-    h->unused1 = 0;
+    if (x64) {
+        h->x64.RequiredDumpSpace = sizeof(WinDumpHeader64) +
+            (h->x64.PhysicalMemoryBlock.NumberOfPages << TARGET_PAGE_BITS);
+        h->x64.PhysicalMemoryBlock.unused = 0;
+        h->x64.unused1 = 0;
+    } else {
+        h->x32.RequiredDumpSpace = sizeof(WinDumpHeader32) +
+            (h->x32.PhysicalMemoryBlock.NumberOfPages << TARGET_PAGE_BITS);
+    }
 
-    patch_mm_pfn_database(h, &local_err);
+    patch_mm_pfn_database(h, x64, &local_err);
     if (local_err) {
         warn_report_err(local_err);
         local_err = NULL;
     }
-    patch_bugcheck_data(h, &local_err);
+    patch_bugcheck_data(h, x64, &local_err);
     if (local_err) {
         warn_report_err(local_err);
     }
 }
 
-static void check_header(WinDumpHeader64 *h, Error **errp)
+static bool check_header(WinDumpHeader *h, bool *x64, Error **errp)
 {
     const char Signature[] = "PAGE";
-    const char ValidDump[] = "DU64";
 
     if (memcmp(h->Signature, Signature, sizeof(h->Signature))) {
         error_setg(errp, "win-dump: invalid header, expected '%.4s',"
                          " got '%.4s'", Signature, h->Signature);
-        return;
+        return false;
     }
 
-    if (memcmp(h->ValidDump, ValidDump, sizeof(h->ValidDump))) {
-        error_setg(errp, "win-dump: invalid header, expected '%.4s',"
-                         " got '%.4s'", ValidDump, h->ValidDump);
-        return;
+    if (!memcmp(h->ValidDump, "DUMP", sizeof(h->ValidDump))) {
+        *x64 = false;
+    } else if (!memcmp(h->ValidDump, "DU64", sizeof(h->ValidDump))) {
+        *x64 = true;
+    } else {
+        error_setg(errp, "win-dump: invalid header, expected 'DUMP' or 'DU64',"
+                   " got '%.4s'", h->ValidDump);
+        return false;
     }
+
+    return true;
 }
 
-static void check_kdbg(WinDumpHeader64 *h, Error **errp)
+static void check_kdbg(WinDumpHeader *h, bool x64, Error **errp)
 {
     const char OwnerTag[] = "KDBG";
     char read_OwnerTag[4];
@@ -188,7 +205,7 @@ static void check_kdbg(WinDumpHeader64 *h, Error **errp)
 
 try_again:
     if (cpu_memory_rw_debug(first_cpu,
-            KdDebuggerDataBlock + KDBG_OWNER_TAG_OFFSET64,
+            KdDebuggerDataBlock + KDBG_OWNER_TAG_OFFSET,
             (uint8_t *)&read_OwnerTag, sizeof(read_OwnerTag), 0)) {
         error_setg(errp, "win-dump: failed to read OwnerTag");
         return;
@@ -213,15 +230,19 @@ try_again:
         }
     }
 
-    h->KdDebuggerDataBlock = KdDebuggerDataBlock;
+    if (x64) {
+        h->x64.KdDebuggerDataBlock = KdDebuggerDataBlock;
+    } else {
+        h->x32.KdDebuggerDataBlock = KdDebuggerDataBlock;
+    }
 }
 
 struct saved_context {
-    WinContext64 ctx;
+    WinContext ctx;
     uint64_t addr;
 };
 
-static void patch_and_save_context(WinDumpHeader64 *h,
+static void patch_and_save_context(WinDumpHeader *h, bool x64,
                                    struct saved_context *saved_ctx,
                                    Error **errp)
 {
@@ -231,15 +252,15 @@ static void patch_and_save_context(WinDumpHeader64 *h,
     CPUState *cpu;
     int i = 0;
 
-    if (cpu_read_ptr(first_cpu,
-            KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET64,
+    if (cpu_read_ptr(x64, first_cpu,
+            KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET,
             &KiProcessorBlock)) {
         error_setg(errp, "win-dump: failed to read KiProcessorBlock");
         return;
     }
 
     if (cpu_memory_rw_debug(first_cpu,
-            KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET64,
+            KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET,
             (uint8_t *)&OffsetPrcbContext, sizeof(OffsetPrcbContext), 0)) {
         error_setg(errp, "win-dump: failed to read OffsetPrcbContext");
         return;
@@ -250,17 +271,17 @@ static void patch_and_save_context(WinDumpHeader64 *h,
         CPUX86State *env = &x86_cpu->env;
         uint64_t Prcb;
         uint64_t Context;
-        WinContext64 ctx;
+        WinContext ctx;
 
-        if (cpu_read_ptr(first_cpu,
-                KiProcessorBlock + i * WIN_DUMP_PTR_SIZE,
+        if (cpu_read_ptr(x64, first_cpu,
+                KiProcessorBlock + i * win_dump_ptr_size(x64),
                 &Prcb)) {
             error_setg(errp, "win-dump: failed to read"
                              " CPU #%d PRCB location", i);
             return;
         }
 
-        if (cpu_read_ptr(first_cpu,
+        if (cpu_read_ptr(x64, first_cpu,
                 Prcb + OffsetPrcbContext,
                 &Context)) {
             error_setg(errp, "win-dump: failed to read"
@@ -270,56 +291,88 @@ static void patch_and_save_context(WinDumpHeader64 *h,
 
         saved_ctx[i].addr = Context;
 
-        ctx = (WinContext64){
-            .ContextFlags = WIN_CTX64_ALL,
-            .MxCsr = env->mxcsr,
-
-            .SegEs = env->segs[0].selector,
-            .SegCs = env->segs[1].selector,
-            .SegSs = env->segs[2].selector,
-            .SegDs = env->segs[3].selector,
-            .SegFs = env->segs[4].selector,
-            .SegGs = env->segs[5].selector,
-            .EFlags = cpu_compute_eflags(env),
-
-            .Dr0 = env->dr[0],
-            .Dr1 = env->dr[1],
-            .Dr2 = env->dr[2],
-            .Dr3 = env->dr[3],
-            .Dr6 = env->dr[6],
-            .Dr7 = env->dr[7],
-
-            .Rax = env->regs[R_EAX],
-            .Rbx = env->regs[R_EBX],
-            .Rcx = env->regs[R_ECX],
-            .Rdx = env->regs[R_EDX],
-            .Rsp = env->regs[R_ESP],
-            .Rbp = env->regs[R_EBP],
-            .Rsi = env->regs[R_ESI],
-            .Rdi = env->regs[R_EDI],
-            .R8  = env->regs[8],
-            .R9  = env->regs[9],
-            .R10 = env->regs[10],
-            .R11 = env->regs[11],
-            .R12 = env->regs[12],
-            .R13 = env->regs[13],
-            .R14 = env->regs[14],
-            .R15 = env->regs[15],
-
-            .Rip = env->eip,
-            .FltSave = {
+        if (x64) {
+            ctx.x64 = (WinContext64){
+                .ContextFlags = WIN_CTX64_ALL,
                 .MxCsr = env->mxcsr,
-            },
-        };
+
+                .SegEs = env->segs[0].selector,
+                .SegCs = env->segs[1].selector,
+                .SegSs = env->segs[2].selector,
+                .SegDs = env->segs[3].selector,
+                .SegFs = env->segs[4].selector,
+                .SegGs = env->segs[5].selector,
+                .EFlags = cpu_compute_eflags(env),
+
+                .Dr0 = env->dr[0],
+                .Dr1 = env->dr[1],
+                .Dr2 = env->dr[2],
+                .Dr3 = env->dr[3],
+                .Dr6 = env->dr[6],
+                .Dr7 = env->dr[7],
+
+                .Rax = env->regs[R_EAX],
+                .Rbx = env->regs[R_EBX],
+                .Rcx = env->regs[R_ECX],
+                .Rdx = env->regs[R_EDX],
+                .Rsp = env->regs[R_ESP],
+                .Rbp = env->regs[R_EBP],
+                .Rsi = env->regs[R_ESI],
+                .Rdi = env->regs[R_EDI],
+                .R8  = env->regs[8],
+                .R9  = env->regs[9],
+                .R10 = env->regs[10],
+                .R11 = env->regs[11],
+                .R12 = env->regs[12],
+                .R13 = env->regs[13],
+                .R14 = env->regs[14],
+                .R15 = env->regs[15],
+
+                .Rip = env->eip,
+                .FltSave = {
+                    .MxCsr = env->mxcsr,
+                },
+            };
+        } else {
+            ctx.x32 = (WinContext32){
+                .ContextFlags = WIN_CTX32_FULL | WIN_CTX_DBG,
+
+                .SegEs = env->segs[0].selector,
+                .SegCs = env->segs[1].selector,
+                .SegSs = env->segs[2].selector,
+                .SegDs = env->segs[3].selector,
+                .SegFs = env->segs[4].selector,
+                .SegGs = env->segs[5].selector,
+                .EFlags = cpu_compute_eflags(env),
+
+                .Dr0 = env->dr[0],
+                .Dr1 = env->dr[1],
+                .Dr2 = env->dr[2],
+                .Dr3 = env->dr[3],
+                .Dr6 = env->dr[6],
+                .Dr7 = env->dr[7],
+
+                .Eax = env->regs[R_EAX],
+                .Ebx = env->regs[R_EBX],
+                .Ecx = env->regs[R_ECX],
+                .Edx = env->regs[R_EDX],
+                .Esp = env->regs[R_ESP],
+                .Ebp = env->regs[R_EBP],
+                .Esi = env->regs[R_ESI],
+                .Edi = env->regs[R_EDI],
+
+                .Eip = env->eip,
+            };
+        }
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 0)) {
+                &saved_ctx[i].ctx, win_dump_ctx_size(x64), 0)) {
             error_setg(errp, "win-dump: failed to save CPU #%d context", i);
             return;
         }
 
         if (cpu_memory_rw_debug(first_cpu, Context,
-                &ctx, WIN_DUMP_CTX_SIZE, 1)) {
+                &ctx, win_dump_ctx_size(x64), 1)) {
             error_setg(errp, "win-dump: failed to write CPU #%d context", i);
             return;
         }
@@ -328,14 +381,14 @@ static void patch_and_save_context(WinDumpHeader64 *h,
     }
 }
 
-static void restore_context(WinDumpHeader64 *h,
+static void restore_context(WinDumpHeader *h, bool x64,
                             struct saved_context *saved_ctx)
 {
     int i;
 
     for (i = 0; i < WIN_DUMP_FIELD(NumberProcessors); i++) {
         if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
-                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 1)) {
+                &saved_ctx[i].ctx, win_dump_ctx_size(x64), 1)) {
             warn_report("win-dump: failed to restore CPU #%d context", i);
         }
     }
@@ -343,25 +396,27 @@ static void restore_context(WinDumpHeader64 *h,
 
 void create_win_dump(DumpState *s, Error **errp)
 {
-    WinDumpHeader64 *h = (WinDumpHeader64 *)(s->guest_note +
-            VMCOREINFO_ELF_NOTE_HDR_SIZE);
+    WinDumpHeader *h = (void *)(s->guest_note + VMCOREINFO_ELF_NOTE_HDR_SIZE);
     X86CPU *first_x86_cpu = X86_CPU(first_cpu);
     uint64_t saved_cr3 = first_x86_cpu->env.cr[3];
     struct saved_context *saved_ctx = NULL;
     Error *local_err = NULL;
+    bool x64 = true;
+    size_t hdr_size;
 
-    if (s->guest_note_size != sizeof(WinDumpHeader64) +
-            VMCOREINFO_ELF_NOTE_HDR_SIZE) {
+    if (s->guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32 &&
+            s->guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64) {
         error_setg(errp, "win-dump: invalid vmcoreinfo note size");
         return;
     }
 
-    check_header(h, &local_err);
-    if (local_err) {
+    if (!check_header(h, &x64, &local_err)) {
         error_propagate(errp, local_err);
         return;
     }
 
+    hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
+
     /*
      * Further access to kernel structures by virtual addresses
      * should be made from system context.
@@ -369,13 +424,13 @@ void create_win_dump(DumpState *s, Error **errp)
 
     first_x86_cpu->env.cr[3] = WIN_DUMP_FIELD(DirectoryTableBase);
 
-    check_kdbg(h, &local_err);
+    check_kdbg(h, x64, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out_cr3;
     }
 
-    patch_header(h);
+    patch_header(h, x64);
 
     saved_ctx = g_new(struct saved_context, WIN_DUMP_FIELD(NumberProcessors));
 
@@ -384,7 +439,7 @@ void create_win_dump(DumpState *s, Error **errp)
      * to determine if the system-saved context is valid
      */
 
-    patch_and_save_context(h, saved_ctx, &local_err);
+    patch_and_save_context(h, x64, saved_ctx, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out_free;
@@ -392,20 +447,20 @@ void create_win_dump(DumpState *s, Error **errp)
 
     s->total_size = WIN_DUMP_FIELD(RequiredDumpSpace);
 
-    s->written_size = qemu_write_full(s->fd, h, sizeof(*h));
-    if (s->written_size != sizeof(*h)) {
+    s->written_size = qemu_write_full(s->fd, h, hdr_size);
+    if (s->written_size != hdr_size) {
         error_setg(errp, QERR_IO_ERROR);
         goto out_restore;
     }
 
-    write_runs(s, h, &local_err);
+    write_runs(s, h, x64, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out_restore;
     }
 
 out_restore:
-    restore_context(h, saved_ctx);
+    restore_context(h, x64, saved_ctx);
 out_free:
     g_free(saved_ctx);
 out_cr3:
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9c9..dd4006d3558a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1064,7 +1064,7 @@ ERST
                       "-l: dump in kdump-compressed format, with lzo compression.\n\t\t\t"
                       "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t"
                       "-w: dump in Windows crashdump format (can be used instead of ELF-dump converting),\n\t\t\t"
-                      "    for Windows x64 guests with vmcoreinfo driver only.\n\t\t\t"
+                      "    for Windows x86 and x64 guests with vmcoreinfo driver only.\n\t\t\t"
                       "begin: the starting physical address.\n\t\t\t"
                       "length: the memory size, in bytes.",
         .cmd        = hmp_dump_guest_memory,
-- 
2.36.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support
  2022-04-22 10:15 ` [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
@ 2022-04-22 10:25   ` Viktor Prutyanov
  0 siblings, 0 replies; 16+ messages in thread
From: Viktor Prutyanov @ 2022-04-22 10:25 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: richard.henderson, qemu-devel, Dr. David Alan Gilbert, frankja

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

Hello,

Thank you for the compiler warning fix.

Best regards,
Viktor Prutyanov

On Fri, Apr 22, 2022 at 1:16 PM <marcandre.lureau@redhat.com> wrote:

> From: Viktor Prutyanov <viktor.prutyanov@redhat.com>
>
> Before this patch, 'dump-guest-memory -w' was accepting only 64-bit
> dump header provided by guest through vmcoreinfo and thus was unable
> to produce 32-bit guest Windows dump. So, add 32-bit guest Windows
> dumping support.
>
> Signed-off-by: Viktor Prutyanov <viktor.prutyanov@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> [ misc error handling fixes to avoid compiler warning ]
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-Id: <20220406171558.199263-5-viktor.prutyanov@redhat.com>
> ---
>  dump/win_dump.c | 251 +++++++++++++++++++++++++++++-------------------
>  hmp-commands.hx |   2 +-
>  2 files changed, 154 insertions(+), 99 deletions(-)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index d733918038b2..fd91350fbb8e 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -23,18 +23,24 @@
>  #include "hw/misc/vmcoreinfo.h"
>  #include "win_dump.h"
>
> -#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
> +static size_t win_dump_ptr_size(bool x64)
> +{
> +    return x64 ? sizeof(uint64_t) : sizeof(uint32_t);
> +}
>
> -#define _WIN_DUMP_FIELD(f) (h->f)
> +#define _WIN_DUMP_FIELD(f) (x64 ? h->x64.f : h->x32.f)
>  #define WIN_DUMP_FIELD(field) _WIN_DUMP_FIELD(field)
>
> -#define _WIN_DUMP_FIELD_PTR(f) ((void *)&h->f)
> +#define _WIN_DUMP_FIELD_PTR(f) (x64 ? (void *)&h->x64.f : (void
> *)&h->x32.f)
>  #define WIN_DUMP_FIELD_PTR(field) _WIN_DUMP_FIELD_PTR(field)
>
> -#define _WIN_DUMP_FIELD_SIZE(f) sizeof(h->f)
> +#define _WIN_DUMP_FIELD_SIZE(f) (x64 ? sizeof(h->x64.f) :
> sizeof(h->x32.f))
>  #define WIN_DUMP_FIELD_SIZE(field) _WIN_DUMP_FIELD_SIZE(field)
>
> -#define WIN_DUMP_CTX_SIZE sizeof(WinContext64)
> +static size_t win_dump_ctx_size(bool x64)
> +{
> +    return x64 ? sizeof(WinContext64) : sizeof(WinContext32);
> +}
>
>  static size_t write_run(uint64_t base_page, uint64_t page_count,
>          int fd, Error **errp)
> @@ -70,7 +76,7 @@ static size_t write_run(uint64_t base_page, uint64_t
> page_count,
>      return total;
>  }
>
> -static void write_runs(DumpState *s, WinDumpHeader64 *h, Error **errp)
> +static void write_runs(DumpState *s, WinDumpHeader *h, bool x64, Error
> **errp)
>  {
>      uint64_t BasePage, PageCount;
>      Error *local_err = NULL;
> @@ -87,22 +93,24 @@ static void write_runs(DumpState *s, WinDumpHeader64
> *h, Error **errp)
>      }
>  }
>
> -static int cpu_read_ptr(CPUState *cpu, uint64_t addr, uint64_t *ptr)
> +static int cpu_read_ptr(bool x64, CPUState *cpu, uint64_t addr, uint64_t
> *ptr)
>  {
>      int ret;
> +    uint32_t ptr32;
>      uint64_t ptr64;
>
> -    ret = cpu_memory_rw_debug(cpu, addr, &ptr64, WIN_DUMP_PTR_SIZE, 0);
> +    ret = cpu_memory_rw_debug(cpu, addr, x64 ? (void *)&ptr64 : (void
> *)&ptr32,
> +            win_dump_ptr_size(x64), 0);
>
> -    *ptr = ptr64;
> +    *ptr = x64 ? ptr64 : ptr32;
>
>      return ret;
>  }
>
> -static void patch_mm_pfn_database(WinDumpHeader64 *h, Error **errp)
> +static void patch_mm_pfn_database(WinDumpHeader *h, bool x64, Error
> **errp)
>  {
>      if (cpu_memory_rw_debug(first_cpu,
> -            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> KDBG_MM_PFN_DATABASE_OFFSET64,
> +            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> KDBG_MM_PFN_DATABASE_OFFSET,
>              WIN_DUMP_FIELD_PTR(PfnDatabase),
>              WIN_DUMP_FIELD_SIZE(PfnDatabase), 0)) {
>          error_setg(errp, "win-dump: failed to read MmPfnDatabase");
> @@ -110,13 +118,12 @@ static void patch_mm_pfn_database(WinDumpHeader64
> *h, Error **errp)
>      }
>  }
>
> -static void patch_bugcheck_data(WinDumpHeader64 *h, Error **errp)
> +static void patch_bugcheck_data(WinDumpHeader *h, bool x64, Error **errp)
>  {
>      uint64_t KiBugcheckData;
>
> -    if (cpu_read_ptr(first_cpu,
> -            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> -                KDBG_KI_BUGCHECK_DATA_OFFSET64,
> +    if (cpu_read_ptr(x64, first_cpu,
> +            WIN_DUMP_FIELD(KdDebuggerDataBlock) +
> KDBG_KI_BUGCHECK_DATA_OFFSET,
>              &KiBugcheckData)) {
>          error_setg(errp, "win-dump: failed to read KiBugcheckData");
>          return;
> @@ -141,45 +148,55 @@ static void patch_bugcheck_data(WinDumpHeader64 *h,
> Error **errp)
>  /*
>   * This routine tries to correct mistakes in crashdump header.
>   */
> -static void patch_header(WinDumpHeader64 *h)
> +static void patch_header(WinDumpHeader *h, bool x64)
>  {
>      Error *local_err = NULL;
>
> -    h->RequiredDumpSpace = sizeof(WinDumpHeader64) +
> -            (h->PhysicalMemoryBlock.NumberOfPages << TARGET_PAGE_BITS);
> -    h->PhysicalMemoryBlock.unused = 0;
> -    h->unused1 = 0;
> +    if (x64) {
> +        h->x64.RequiredDumpSpace = sizeof(WinDumpHeader64) +
> +            (h->x64.PhysicalMemoryBlock.NumberOfPages <<
> TARGET_PAGE_BITS);
> +        h->x64.PhysicalMemoryBlock.unused = 0;
> +        h->x64.unused1 = 0;
> +    } else {
> +        h->x32.RequiredDumpSpace = sizeof(WinDumpHeader32) +
> +            (h->x32.PhysicalMemoryBlock.NumberOfPages <<
> TARGET_PAGE_BITS);
> +    }
>
> -    patch_mm_pfn_database(h, &local_err);
> +    patch_mm_pfn_database(h, x64, &local_err);
>      if (local_err) {
>          warn_report_err(local_err);
>          local_err = NULL;
>      }
> -    patch_bugcheck_data(h, &local_err);
> +    patch_bugcheck_data(h, x64, &local_err);
>      if (local_err) {
>          warn_report_err(local_err);
>      }
>  }
>
> -static void check_header(WinDumpHeader64 *h, Error **errp)
> +static bool check_header(WinDumpHeader *h, bool *x64, Error **errp)
>  {
>      const char Signature[] = "PAGE";
> -    const char ValidDump[] = "DU64";
>
>      if (memcmp(h->Signature, Signature, sizeof(h->Signature))) {
>          error_setg(errp, "win-dump: invalid header, expected '%.4s',"
>                           " got '%.4s'", Signature, h->Signature);
> -        return;
> +        return false;
>      }
>
> -    if (memcmp(h->ValidDump, ValidDump, sizeof(h->ValidDump))) {
> -        error_setg(errp, "win-dump: invalid header, expected '%.4s',"
> -                         " got '%.4s'", ValidDump, h->ValidDump);
> -        return;
> +    if (!memcmp(h->ValidDump, "DUMP", sizeof(h->ValidDump))) {
> +        *x64 = false;
> +    } else if (!memcmp(h->ValidDump, "DU64", sizeof(h->ValidDump))) {
> +        *x64 = true;
> +    } else {
> +        error_setg(errp, "win-dump: invalid header, expected 'DUMP' or
> 'DU64',"
> +                   " got '%.4s'", h->ValidDump);
> +        return false;
>      }
> +
> +    return true;
>  }
>
> -static void check_kdbg(WinDumpHeader64 *h, Error **errp)
> +static void check_kdbg(WinDumpHeader *h, bool x64, Error **errp)
>  {
>      const char OwnerTag[] = "KDBG";
>      char read_OwnerTag[4];
> @@ -188,7 +205,7 @@ static void check_kdbg(WinDumpHeader64 *h, Error
> **errp)
>
>  try_again:
>      if (cpu_memory_rw_debug(first_cpu,
> -            KdDebuggerDataBlock + KDBG_OWNER_TAG_OFFSET64,
> +            KdDebuggerDataBlock + KDBG_OWNER_TAG_OFFSET,
>              (uint8_t *)&read_OwnerTag, sizeof(read_OwnerTag), 0)) {
>          error_setg(errp, "win-dump: failed to read OwnerTag");
>          return;
> @@ -213,15 +230,19 @@ try_again:
>          }
>      }
>
> -    h->KdDebuggerDataBlock = KdDebuggerDataBlock;
> +    if (x64) {
> +        h->x64.KdDebuggerDataBlock = KdDebuggerDataBlock;
> +    } else {
> +        h->x32.KdDebuggerDataBlock = KdDebuggerDataBlock;
> +    }
>  }
>
>  struct saved_context {
> -    WinContext64 ctx;
> +    WinContext ctx;
>      uint64_t addr;
>  };
>
> -static void patch_and_save_context(WinDumpHeader64 *h,
> +static void patch_and_save_context(WinDumpHeader *h, bool x64,
>                                     struct saved_context *saved_ctx,
>                                     Error **errp)
>  {
> @@ -231,15 +252,15 @@ static void patch_and_save_context(WinDumpHeader64
> *h,
>      CPUState *cpu;
>      int i = 0;
>
> -    if (cpu_read_ptr(first_cpu,
> -            KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET64,
> +    if (cpu_read_ptr(x64, first_cpu,
> +            KdDebuggerDataBlock + KDBG_KI_PROCESSOR_BLOCK_OFFSET,
>              &KiProcessorBlock)) {
>          error_setg(errp, "win-dump: failed to read KiProcessorBlock");
>          return;
>      }
>
>      if (cpu_memory_rw_debug(first_cpu,
> -            KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET64,
> +            KdDebuggerDataBlock + KDBG_OFFSET_PRCB_CONTEXT_OFFSET,
>              (uint8_t *)&OffsetPrcbContext, sizeof(OffsetPrcbContext), 0))
> {
>          error_setg(errp, "win-dump: failed to read OffsetPrcbContext");
>          return;
> @@ -250,17 +271,17 @@ static void patch_and_save_context(WinDumpHeader64
> *h,
>          CPUX86State *env = &x86_cpu->env;
>          uint64_t Prcb;
>          uint64_t Context;
> -        WinContext64 ctx;
> +        WinContext ctx;
>
> -        if (cpu_read_ptr(first_cpu,
> -                KiProcessorBlock + i * WIN_DUMP_PTR_SIZE,
> +        if (cpu_read_ptr(x64, first_cpu,
> +                KiProcessorBlock + i * win_dump_ptr_size(x64),
>                  &Prcb)) {
>              error_setg(errp, "win-dump: failed to read"
>                               " CPU #%d PRCB location", i);
>              return;
>          }
>
> -        if (cpu_read_ptr(first_cpu,
> +        if (cpu_read_ptr(x64, first_cpu,
>                  Prcb + OffsetPrcbContext,
>                  &Context)) {
>              error_setg(errp, "win-dump: failed to read"
> @@ -270,56 +291,88 @@ static void patch_and_save_context(WinDumpHeader64
> *h,
>
>          saved_ctx[i].addr = Context;
>
> -        ctx = (WinContext64){
> -            .ContextFlags = WIN_CTX64_ALL,
> -            .MxCsr = env->mxcsr,
> -
> -            .SegEs = env->segs[0].selector,
> -            .SegCs = env->segs[1].selector,
> -            .SegSs = env->segs[2].selector,
> -            .SegDs = env->segs[3].selector,
> -            .SegFs = env->segs[4].selector,
> -            .SegGs = env->segs[5].selector,
> -            .EFlags = cpu_compute_eflags(env),
> -
> -            .Dr0 = env->dr[0],
> -            .Dr1 = env->dr[1],
> -            .Dr2 = env->dr[2],
> -            .Dr3 = env->dr[3],
> -            .Dr6 = env->dr[6],
> -            .Dr7 = env->dr[7],
> -
> -            .Rax = env->regs[R_EAX],
> -            .Rbx = env->regs[R_EBX],
> -            .Rcx = env->regs[R_ECX],
> -            .Rdx = env->regs[R_EDX],
> -            .Rsp = env->regs[R_ESP],
> -            .Rbp = env->regs[R_EBP],
> -            .Rsi = env->regs[R_ESI],
> -            .Rdi = env->regs[R_EDI],
> -            .R8  = env->regs[8],
> -            .R9  = env->regs[9],
> -            .R10 = env->regs[10],
> -            .R11 = env->regs[11],
> -            .R12 = env->regs[12],
> -            .R13 = env->regs[13],
> -            .R14 = env->regs[14],
> -            .R15 = env->regs[15],
> -
> -            .Rip = env->eip,
> -            .FltSave = {
> +        if (x64) {
> +            ctx.x64 = (WinContext64){
> +                .ContextFlags = WIN_CTX64_ALL,
>                  .MxCsr = env->mxcsr,
> -            },
> -        };
> +
> +                .SegEs = env->segs[0].selector,
> +                .SegCs = env->segs[1].selector,
> +                .SegSs = env->segs[2].selector,
> +                .SegDs = env->segs[3].selector,
> +                .SegFs = env->segs[4].selector,
> +                .SegGs = env->segs[5].selector,
> +                .EFlags = cpu_compute_eflags(env),
> +
> +                .Dr0 = env->dr[0],
> +                .Dr1 = env->dr[1],
> +                .Dr2 = env->dr[2],
> +                .Dr3 = env->dr[3],
> +                .Dr6 = env->dr[6],
> +                .Dr7 = env->dr[7],
> +
> +                .Rax = env->regs[R_EAX],
> +                .Rbx = env->regs[R_EBX],
> +                .Rcx = env->regs[R_ECX],
> +                .Rdx = env->regs[R_EDX],
> +                .Rsp = env->regs[R_ESP],
> +                .Rbp = env->regs[R_EBP],
> +                .Rsi = env->regs[R_ESI],
> +                .Rdi = env->regs[R_EDI],
> +                .R8  = env->regs[8],
> +                .R9  = env->regs[9],
> +                .R10 = env->regs[10],
> +                .R11 = env->regs[11],
> +                .R12 = env->regs[12],
> +                .R13 = env->regs[13],
> +                .R14 = env->regs[14],
> +                .R15 = env->regs[15],
> +
> +                .Rip = env->eip,
> +                .FltSave = {
> +                    .MxCsr = env->mxcsr,
> +                },
> +            };
> +        } else {
> +            ctx.x32 = (WinContext32){
> +                .ContextFlags = WIN_CTX32_FULL | WIN_CTX_DBG,
> +
> +                .SegEs = env->segs[0].selector,
> +                .SegCs = env->segs[1].selector,
> +                .SegSs = env->segs[2].selector,
> +                .SegDs = env->segs[3].selector,
> +                .SegFs = env->segs[4].selector,
> +                .SegGs = env->segs[5].selector,
> +                .EFlags = cpu_compute_eflags(env),
> +
> +                .Dr0 = env->dr[0],
> +                .Dr1 = env->dr[1],
> +                .Dr2 = env->dr[2],
> +                .Dr3 = env->dr[3],
> +                .Dr6 = env->dr[6],
> +                .Dr7 = env->dr[7],
> +
> +                .Eax = env->regs[R_EAX],
> +                .Ebx = env->regs[R_EBX],
> +                .Ecx = env->regs[R_ECX],
> +                .Edx = env->regs[R_EDX],
> +                .Esp = env->regs[R_ESP],
> +                .Ebp = env->regs[R_EBP],
> +                .Esi = env->regs[R_ESI],
> +                .Edi = env->regs[R_EDI],
> +
> +                .Eip = env->eip,
> +            };
> +        }
>
>          if (cpu_memory_rw_debug(first_cpu, Context,
> -                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 0)) {
> +                &saved_ctx[i].ctx, win_dump_ctx_size(x64), 0)) {
>              error_setg(errp, "win-dump: failed to save CPU #%d context",
> i);
>              return;
>          }
>
>          if (cpu_memory_rw_debug(first_cpu, Context,
> -                &ctx, WIN_DUMP_CTX_SIZE, 1)) {
> +                &ctx, win_dump_ctx_size(x64), 1)) {
>              error_setg(errp, "win-dump: failed to write CPU #%d context",
> i);
>              return;
>          }
> @@ -328,14 +381,14 @@ static void patch_and_save_context(WinDumpHeader64
> *h,
>      }
>  }
>
> -static void restore_context(WinDumpHeader64 *h,
> +static void restore_context(WinDumpHeader *h, bool x64,
>                              struct saved_context *saved_ctx)
>  {
>      int i;
>
>      for (i = 0; i < WIN_DUMP_FIELD(NumberProcessors); i++) {
>          if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr,
> -                &saved_ctx[i].ctx, WIN_DUMP_CTX_SIZE, 1)) {
> +                &saved_ctx[i].ctx, win_dump_ctx_size(x64), 1)) {
>              warn_report("win-dump: failed to restore CPU #%d context", i);
>          }
>      }
> @@ -343,25 +396,27 @@ static void restore_context(WinDumpHeader64 *h,
>
>  void create_win_dump(DumpState *s, Error **errp)
>  {
> -    WinDumpHeader64 *h = (WinDumpHeader64 *)(s->guest_note +
> -            VMCOREINFO_ELF_NOTE_HDR_SIZE);
> +    WinDumpHeader *h = (void *)(s->guest_note +
> VMCOREINFO_ELF_NOTE_HDR_SIZE);
>      X86CPU *first_x86_cpu = X86_CPU(first_cpu);
>      uint64_t saved_cr3 = first_x86_cpu->env.cr[3];
>      struct saved_context *saved_ctx = NULL;
>      Error *local_err = NULL;
> +    bool x64 = true;
> +    size_t hdr_size;
>
> -    if (s->guest_note_size != sizeof(WinDumpHeader64) +
> -            VMCOREINFO_ELF_NOTE_HDR_SIZE) {
> +    if (s->guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE32 &&
> +            s->guest_note_size != VMCOREINFO_WIN_DUMP_NOTE_SIZE64) {
>          error_setg(errp, "win-dump: invalid vmcoreinfo note size");
>          return;
>      }
>
> -    check_header(h, &local_err);
> -    if (local_err) {
> +    if (!check_header(h, &x64, &local_err)) {
>          error_propagate(errp, local_err);
>          return;
>      }
>
> +    hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
> +
>      /*
>       * Further access to kernel structures by virtual addresses
>       * should be made from system context.
> @@ -369,13 +424,13 @@ void create_win_dump(DumpState *s, Error **errp)
>
>      first_x86_cpu->env.cr[3] = WIN_DUMP_FIELD(DirectoryTableBase);
>
> -    check_kdbg(h, &local_err);
> +    check_kdbg(h, x64, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          goto out_cr3;
>      }
>
> -    patch_header(h);
> +    patch_header(h, x64);
>
>      saved_ctx = g_new(struct saved_context,
> WIN_DUMP_FIELD(NumberProcessors));
>
> @@ -384,7 +439,7 @@ void create_win_dump(DumpState *s, Error **errp)
>       * to determine if the system-saved context is valid
>       */
>
> -    patch_and_save_context(h, saved_ctx, &local_err);
> +    patch_and_save_context(h, x64, saved_ctx, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          goto out_free;
> @@ -392,20 +447,20 @@ void create_win_dump(DumpState *s, Error **errp)
>
>      s->total_size = WIN_DUMP_FIELD(RequiredDumpSpace);
>
> -    s->written_size = qemu_write_full(s->fd, h, sizeof(*h));
> -    if (s->written_size != sizeof(*h)) {
> +    s->written_size = qemu_write_full(s->fd, h, hdr_size);
> +    if (s->written_size != hdr_size) {
>          error_setg(errp, QERR_IO_ERROR);
>          goto out_restore;
>      }
>
> -    write_runs(s, h, &local_err);
> +    write_runs(s, h, x64, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          goto out_restore;
>      }
>
>  out_restore:
> -    restore_context(h, saved_ctx);
> +    restore_context(h, x64, saved_ctx);
>  out_free:
>      g_free(saved_ctx);
>  out_cr3:
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8476277aa9c9..dd4006d3558a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1064,7 +1064,7 @@ ERST
>                        "-l: dump in kdump-compressed format, with lzo
> compression.\n\t\t\t"
>                        "-s: dump in kdump-compressed format, with snappy
> compression.\n\t\t\t"
>                        "-w: dump in Windows crashdump format (can be used
> instead of ELF-dump converting),\n\t\t\t"
> -                      "    for Windows x64 guests with vmcoreinfo driver
> only.\n\t\t\t"
> +                      "    for Windows x86 and x64 guests with vmcoreinfo
> driver only.\n\t\t\t"
>                        "begin: the starting physical address.\n\t\t\t"
>                        "length: the memory size, in bytes.",
>          .cmd        = hmp_dump_guest_memory,
> --
> 2.36.0
>
>

[-- Attachment #2: Type: text/html, Size: 23924 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL v2 00/13] Dump patches
  2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
                   ` (12 preceding siblings ...)
  2022-04-22 10:15 ` [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
@ 2022-04-22 13:16 ` Richard Henderson
  13 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2022-04-22 13:16 UTC (permalink / raw)
  To: marcandre.lureau, qemu-devel; +Cc: viktor.prutyanov, frankja

On 4/22/22 03:15, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The following changes since commit a74782936dc6e979ce371dabda4b1c05624ea87f:
> 
>    Merge tag 'pull-migration-20220421a' of https://gitlab.com/dagrh/qemu into staging (2022-04-21 18:48:18 -0700)
> 
> are available in the Git repository at:
> 
>    git@gitlab.com:marcandre.lureau/qemu.git tags/dump-pull-request
> 
> for you to fetch changes up to f5daa8293b292929cb429f154e926191ba8e040c:
> 
>    dump/win_dump: add 32-bit guest Windows support (2022-04-22 13:41:56 +0400)
> 
> ----------------------------------------------------------------
> dump queue
> 
> Hi
> 
> The "dump" queue, with:
> - [PATCH v3/v4 0/9] dump: Cleanup and consolidation
> - [PATCH v4 0/4] dump: add 32-bit guest Windows support
> 
> v2:
> - fix compiler warning in "dump/win_dump: add 32-bit guest Windows support"
> 

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate.


r~


> ----------------------------------------------------------------
> 
> Janosch Frank (9):
>    dump: Use ERRP_GUARD()
>    dump: Remove the sh_info variable
>    dump: Introduce shdr_num to decrease complexity
>    dump: Remove the section if when calculating the memory offset
>    dump: Add more offset variables
>    dump: Introduce dump_is_64bit() helper function
>    dump: Consolidate phdr note writes
>    dump: Cleanup dump_begin write functions
>    dump: Consolidate elf note function
> 
> Viktor Prutyanov (4):
>    include/qemu: rename Windows context definitions to expose bitness
>    dump/win_dump: add helper macros for Windows dump header access
>    include/qemu: add 32-bit Windows dump structures
>    dump/win_dump: add 32-bit guest Windows support
> 
>   include/qemu/win_dump_defs.h | 115 ++++++++++-
>   include/sysemu/dump.h        |   9 +-
>   contrib/elf2dmp/main.c       |   6 +-
>   dump/dump.c                  | 372 ++++++++++++++++-------------------
>   dump/win_dump.c              | 305 +++++++++++++++++-----------
>   hmp-commands.hx              |   2 +-
>   6 files changed, 482 insertions(+), 327 deletions(-)
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-04-22 13:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22 10:15 [PULL v2 00/13] Dump patches marcandre.lureau
2022-04-22 10:15 ` [PULL v2 01/13] dump: Use ERRP_GUARD() marcandre.lureau
2022-04-22 10:15 ` [PULL v2 02/13] dump: Remove the sh_info variable marcandre.lureau
2022-04-22 10:15 ` [PULL v2 03/13] dump: Introduce shdr_num to decrease complexity marcandre.lureau
2022-04-22 10:15 ` [PULL v2 04/13] dump: Remove the section if when calculating the memory offset marcandre.lureau
2022-04-22 10:15 ` [PULL v2 05/13] dump: Add more offset variables marcandre.lureau
2022-04-22 10:15 ` [PULL v2 06/13] dump: Introduce dump_is_64bit() helper function marcandre.lureau
2022-04-22 10:15 ` [PULL v2 07/13] dump: Consolidate phdr note writes marcandre.lureau
2022-04-22 10:15 ` [PULL v2 08/13] dump: Cleanup dump_begin write functions marcandre.lureau
2022-04-22 10:15 ` [PULL v2 09/13] dump: Consolidate elf note function marcandre.lureau
2022-04-22 10:15 ` [PULL v2 10/13] include/qemu: rename Windows context definitions to expose bitness marcandre.lureau
2022-04-22 10:15 ` [PULL v2 11/13] dump/win_dump: add helper macros for Windows dump header access marcandre.lureau
2022-04-22 10:15 ` [PULL v2 12/13] include/qemu: add 32-bit Windows dump structures marcandre.lureau
2022-04-22 10:15 ` [PULL v2 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
2022-04-22 10:25   ` Viktor Prutyanov
2022-04-22 13:16 ` [PULL v2 00/13] Dump patches Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.