From 86a518bba4f4d7c9016fc5b104fe1e58b00ad756 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:35:55 +0000 Subject: [PATCH 01/13] dump: Use ERRP_GUARD() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau 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 e766ce1d7d..b91e9d8c12 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; } From 046bc4160bc780eaacc2d702a2589f1a7a01188d Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Thu, 7 Apr 2022 09:48:24 +0000 Subject: [PATCH 02/13] dump: Remove the sh_info variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau Message-Id: <20220407094824.5074-1-frankja@linux.ibm.com> --- dump/dump.c | 44 +++++++++++++++++++++++-------------------- include/sysemu/dump.h | 3 +-- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index b91e9d8c12..010b272da0 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) + diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index 250143cb5a..b463fc9c02 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; From 862a395858e5a302ed5921487777acdc95a3a31b Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:35:57 +0000 Subject: [PATCH 03/13] dump: Introduce shdr_num to decrease complexity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau Message-Id: <20220330123603.107120-4-frankja@linux.ibm.com> --- dump/dump.c | 24 ++++++++++++------------ include/sysemu/dump.h | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index 010b272da0..285ed4475b 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; diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index b463fc9c02..19458bffbd 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; From 344107e07bd81546474a54ab83800158ca953059 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:35:58 +0000 Subject: [PATCH 04/13] dump: Remove the section if when calculating the memory offset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau 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 285ed4475b..9c80680eb2 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; From e71d353360bb09a8e784e35d78370c691f6ea185 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:35:59 +0000 Subject: [PATCH 05/13] dump: Add more offset variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Marc-André Lureau Reviewed-by: Richard Henderson Message-Id: <20220330123603.107120-6-frankja@linux.ibm.com> --- dump/dump.c | 35 +++++++++++++++-------------------- include/sysemu/dump.h | 4 ++++ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index 9c80680eb2..7f226257ee 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; diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index 19458bffbd..ffc2ea1072 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; From 05bbaa5040ccb3419e8b93af8040485430e2db42 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:36:00 +0000 Subject: [PATCH 06/13] dump: Introduce dump_is_64bit() helper function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau 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 7f226257ee..b063db1340 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; From bc7d558017e6700f9a05c61b0b638a8994945f0d Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:36:01 +0000 Subject: [PATCH 07/13] dump: Consolidate phdr note writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau 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 b063db1340..0d95fc5b7a 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) { From 5ff2e5a3e1e67930e523486e39549a33fcf97227 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:36:02 +0000 Subject: [PATCH 08/13] dump: Cleanup dump_begin write functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Richard Henderson Reviewed-by: Marc-André Lureau 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 0d95fc5b7a..929ef95351 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; - } } } From c68124738bc29017e4254c898bc40be7be477af7 Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Wed, 30 Mar 2022 12:36:03 +0000 Subject: [PATCH 09/13] dump: Consolidate elf note function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Marc-André Lureau 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 929ef95351..4d9658ffa2 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) From a64b4e179a85cefd8fd492e019430185e19fa32e Mon Sep 17 00:00:00 2001 From: Viktor Prutyanov Date: Wed, 6 Apr 2022 20:15:55 +0300 Subject: [PATCH 10/13] include/qemu: rename Windows context definitions to expose bitness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context structure in 64-bit Windows differs from 32-bit one and it should be reflected in its name. Signed-off-by: Viktor Prutyanov Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau Message-Id: <20220406171558.199263-2-viktor.prutyanov@redhat.com> --- contrib/elf2dmp/main.c | 6 +++--- dump/win_dump.c | 14 +++++++------- include/qemu/win_dump_defs.h | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c index 20b477d582..b9fc6d230c 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 fbdbb7bd93..e9215e4fd5 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); } } diff --git a/include/qemu/win_dump_defs.h b/include/qemu/win_dump_defs.h index 145096e8ee..5a5e5a5e09 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 */ From fb21efe99a5ce6a52abe47eaa286603453e18073 Mon Sep 17 00:00:00 2001 From: Viktor Prutyanov Date: Wed, 6 Apr 2022 20:15:56 +0300 Subject: [PATCH 11/13] dump/win_dump: add helper macros for Windows dump header access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Marc-André Lureau 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 e9215e4fd5..d733918038 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)) { From c4fe30921f42d2dba4bc51458839039f71e13a6b Mon Sep 17 00:00:00 2001 From: Viktor Prutyanov Date: Wed, 6 Apr 2022 20:15:57 +0300 Subject: [PATCH 12/13] include/qemu: add 32-bit Windows dump structures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Marc-André Lureau 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 5a5e5a5e09..73a44e2408 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 */ From f5daa8293b292929cb429f154e926191ba8e040c Mon Sep 17 00:00:00 2001 From: Viktor Prutyanov Date: Wed, 6 Apr 2022 20:15:58 +0300 Subject: [PATCH 13/13] dump/win_dump: add 32-bit guest Windows support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Philippe Mathieu-Daudé [ misc error handling fixes to avoid compiler warning ] Reviewed-by: Marc-André Lureau 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 d733918038..fd91350fbb 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 8476277aa9..dd4006d355 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,