[PATCH v2 4/4] dump/win_dump: add 32-bit guest Windows support

Viktor Prutyanov posted 4 patches 4 months, 1 week ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Viktor Prutyanov <viktor.prutyanov@phystech.edu>
[PATCH v2 4/4] dump/win_dump: add 32-bit guest Windows support
Posted by Viktor Prutyanov 4 months, 1 week ago
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@phystech.edu>
---
 dump/win_dump.c | 231 +++++++++++++++++++++++++++++-------------------
 1 file changed, 139 insertions(+), 92 deletions(-)

diff --git a/dump/win_dump.c b/dump/win_dump.c
index df3b432ca5..d751cd6d36 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -24,18 +24,18 @@
 #include "hw/misc/vmcoreinfo.h"
 #include "win_dump.h"
 
-#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
+#define WIN_DUMP_PTR_SIZE (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)
+#define WIN_DUMP_CTX_SIZE (x64 ? sizeof(WinContext64) : sizeof(WinContext32))
 
 static size_t write_run(uint64_t base_page, uint64_t page_count,
         int fd, Error **errp)
@@ -71,7 +71,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;
@@ -88,22 +88,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, 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");
@@ -111,13 +113,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;
@@ -142,30 +143,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',"
@@ -173,14 +178,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];
@@ -189,7 +197,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;
@@ -214,15 +222,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)
 {
@@ -232,15 +244,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;
@@ -251,9 +263,9 @@ 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,
+        if (cpu_read_ptr(x64, first_cpu,
                 KiProcessorBlock + i * WIN_DUMP_PTR_SIZE,
                 &Prcb)) {
             error_setg(errp, "win-dump: failed to read"
@@ -261,7 +273,7 @@ static void patch_and_save_context(WinDumpHeader64 *h,
             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"
@@ -271,47 +283,79 @@ 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)) {
@@ -329,7 +373,7 @@ 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;
@@ -344,25 +388,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.
@@ -370,13 +417,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));
 
@@ -385,7 +432,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;
@@ -393,20 +440,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:
-- 
2.31.1


Re: [PATCH v2 4/4] dump/win_dump: add 32-bit guest Windows support
Posted by Philippe Mathieu-Daudé 4 months, 1 week ago
On 13/1/22 01:52, Viktor Prutyanov wrote:
> 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@phystech.edu>
> ---
>   dump/win_dump.c | 231 +++++++++++++++++++++++++++++-------------------
>   1 file changed, 139 insertions(+), 92 deletions(-)
> 
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index df3b432ca5..d751cd6d36 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -24,18 +24,18 @@
>   #include "hw/misc/vmcoreinfo.h"
>   #include "win_dump.h"
>   
> -#define WIN_DUMP_PTR_SIZE sizeof(uint64_t)
> +#define WIN_DUMP_PTR_SIZE (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)
> +#define WIN_DUMP_CTX_SIZE (x64 ? sizeof(WinContext64) : sizeof(WinContext32))

I'd feel safer with functions rather than macros. Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>