* [PULL 01/13] dump: Use ERRP_GUARD()
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 02/13] dump: Remove the sh_info variable marcandre.lureau
` (11 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 02/13] dump: Remove the sh_info variable
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
2022-04-21 12:48 ` [PULL 01/13] dump: Use ERRP_GUARD() marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 03/13] dump: Introduce shdr_num to decrease complexity marcandre.lureau
` (10 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 03/13] dump: Introduce shdr_num to decrease complexity
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
2022-04-21 12:48 ` [PULL 01/13] dump: Use ERRP_GUARD() marcandre.lureau
2022-04-21 12:48 ` [PULL 02/13] dump: Remove the sh_info variable marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 04/13] dump: Remove the section if when calculating the memory offset marcandre.lureau
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 04/13] dump: Remove the section if when calculating the memory offset
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (2 preceding siblings ...)
2022-04-21 12:48 ` [PULL 03/13] dump: Introduce shdr_num to decrease complexity marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 05/13] dump: Add more offset variables marcandre.lureau
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 05/13] dump: Add more offset variables
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (3 preceding siblings ...)
2022-04-21 12:48 ` [PULL 04/13] dump: Remove the section if when calculating the memory offset marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 06/13] dump: Introduce dump_is_64bit() helper function marcandre.lureau
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 06/13] dump: Introduce dump_is_64bit() helper function
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (4 preceding siblings ...)
2022-04-21 12:48 ` [PULL 05/13] dump: Add more offset variables marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 07/13] dump: Consolidate phdr note writes marcandre.lureau
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 07/13] dump: Consolidate phdr note writes
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (5 preceding siblings ...)
2022-04-21 12:48 ` [PULL 06/13] dump: Introduce dump_is_64bit() helper function marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 08/13] dump: Cleanup dump_begin write functions marcandre.lureau
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 08/13] dump: Cleanup dump_begin write functions
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (6 preceding siblings ...)
2022-04-21 12:48 ` [PULL 07/13] dump: Consolidate phdr note writes marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 09/13] dump: Consolidate elf note function marcandre.lureau
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 09/13] dump: Consolidate elf note function
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (7 preceding siblings ...)
2022-04-21 12:48 ` [PULL 08/13] dump: Cleanup dump_begin write functions marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 10/13] include/qemu: rename Windows context definitions to expose bitness marcandre.lureau
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 10/13] include/qemu: rename Windows context definitions to expose bitness
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (8 preceding siblings ...)
2022-04-21 12:48 ` [PULL 09/13] dump: Consolidate elf note function marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 11/13] dump/win_dump: add helper macros for Windows dump header access marcandre.lureau
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 11/13] dump/win_dump: add helper macros for Windows dump header access
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (9 preceding siblings ...)
2022-04-21 12:48 ` [PULL 10/13] include/qemu: rename Windows context definitions to expose bitness marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 12/13] include/qemu: add 32-bit Windows dump structures marcandre.lureau
2022-04-21 12:48 ` [PULL 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 12/13] include/qemu: add 32-bit Windows dump structures
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (10 preceding siblings ...)
2022-04-21 12:48 ` [PULL 11/13] dump/win_dump: add helper macros for Windows dump header access marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 12:48 ` [PULL 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
12 siblings, 0 replies; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PULL 13/13] dump/win_dump: add 32-bit guest Windows support
2022-04-21 12:48 [PULL 00/13] Dump patches marcandre.lureau
` (11 preceding siblings ...)
2022-04-21 12:48 ` [PULL 12/13] include/qemu: add 32-bit Windows dump structures marcandre.lureau
@ 2022-04-21 12:48 ` marcandre.lureau
2022-04-21 13:59 ` Marc-André Lureau
12 siblings, 1 reply; 16+ messages in thread
From: marcandre.lureau @ 2022-04-21 12:48 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>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220406171558.199263-5-viktor.prutyanov@redhat.com>
---
dump/win_dump.c | 245 +++++++++++++++++++++++++++++-------------------
hmp-commands.hx | 2 +-
2 files changed, 150 insertions(+), 97 deletions(-)
diff --git a/dump/win_dump.c b/dump/win_dump.c
index d733918038b2..ba508790bcea 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,30 +148,34 @@ 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 void 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',"
@@ -172,14 +183,17 @@ static void check_header(WinDumpHeader64 *h, Error **errp)
return;
}
- 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);
}
}
-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 +202,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 +227,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 +249,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 +268,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 +288,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 +378,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 +393,28 @@ 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;
+ 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);
+ check_header(h, &x64, &local_err);
if (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 +422,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 +437,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 +445,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.35.1.693.g805e0a68082a
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PULL 13/13] dump/win_dump: add 32-bit guest Windows support
2022-04-21 12:48 ` [PULL 13/13] dump/win_dump: add 32-bit guest Windows support marcandre.lureau
@ 2022-04-21 13:59 ` Marc-André Lureau
2022-04-21 17:24 ` Richard Henderson
0 siblings, 1 reply; 16+ messages in thread
From: Marc-André Lureau @ 2022-04-21 13:59 UTC (permalink / raw)
To: QEMU
Cc: viktor.prutyanov, Richard Henderson, Janosch Frank,
Dr. David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 19579 bytes --]
On Thu, Apr 21, 2022 at 5:08 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>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-Id: <20220406171558.199263-5-viktor.prutyanov@redhat.com>
> ---
> dump/win_dump.c | 245 +++++++++++++++++++++++++++++-------------------
> hmp-commands.hx | 2 +-
> 2 files changed, 150 insertions(+), 97 deletions(-)
>
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index d733918038b2..ba508790bcea 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,30 +148,34 @@ 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 void 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',"
> @@ -172,14 +183,17 @@ static void check_header(WinDumpHeader64 *h, Error
> **errp)
> return;
> }
>
> - 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);
> }
> }
>
> -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 +202,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 +227,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 +249,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 +268,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 +288,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 +378,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 +393,28 @@ 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;
> + 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);
> + check_header(h, &x64, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
> + hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
>
The compiler is not smart enough here:
../dump/win_dump.c:416:46: error: 'x64' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
I'll simply initialize the variable to true by default.
+
> /*
> * Further access to kernel structures by virtual addresses
> * should be made from system context.
> @@ -369,13 +422,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 +437,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 +445,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.35.1.693.g805e0a68082a
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 24258 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PULL 13/13] dump/win_dump: add 32-bit guest Windows support
2022-04-21 13:59 ` Marc-André Lureau
@ 2022-04-21 17:24 ` Richard Henderson
0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2022-04-21 17:24 UTC (permalink / raw)
To: Marc-André Lureau, QEMU
Cc: viktor.prutyanov, Janosch Frank, Dr. David Alan Gilbert
On 4/21/22 06:59, Marc-André Lureau wrote:
> - check_header(h, &local_err);
> + check_header(h, &x64, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
> + hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
>
>
> The compiler is not smart enough here:
> ../dump/win_dump.c:416:46: error: 'x64' may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
> hdr_size = x64 ? sizeof(WinDumpHeader64) : sizeof(WinDumpHeader32);
The compiler might do better with
if (!check_header(h, &x64, err)) {
return;
}
where check_header returns false on failure (which is recommended by qapi/error.h). Right
now, it can't see through error_setg() to see that local_err must be set when x64 isn't.
> I'll simply initialize the variable to true by default.
Or that.
I'll discard this pull request and expect a v2.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread