[PATCH 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[PATCH 0/5] elf2dmp: improve Win2022, Win11 and large dumps

[Viktor Prutyanov]

Windows Server 2022 and Windows 11 require more careful kernel PE image
search and handling of PDB than previous Windows versions.
Also, improve support of large ELF dump files, dumps with unaligned
memory ranges and with big number of ranges.

Viktor Prutyanov (5):
  elf2dmp: replace PE export name check with PDB name check
  elf2dmp: introduce physical block alignment
  elf2dmp: introduce merging of physical memory runs
  elf2dmp: use Linux mmap with MAP_NORESERVE when possible
  elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

 contrib/elf2dmp/addrspace.c |  31 +++++++-
 contrib/elf2dmp/addrspace.h |   1 +
 contrib/elf2dmp/main.c      | 154 ++++++++++++++++++++----------------
 contrib/elf2dmp/pdb.c       |  10 +--
 contrib/elf2dmp/qemu_elf.c  |  66 +++++++++++++---
 contrib/elf2dmp/qemu_elf.h  |   4 +
 6 files changed, 175 insertions(+), 91 deletions(-)

-- 
2.21.0

[PATCH 1/5] elf2dmp: replace PE export name check with PDB name check

[Viktor Prutyanov]

PE export name check introduced in d399d6b179 isn't reliable enough,
because a page with the export directory may be not present for some
reason. On the other hand, elf2dmp retrieves the PDB name in any case.
It can be also used to check that a PE image is the kernel image. So,
check PDB name when searching for Windows kernel image.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2165917

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/main.c | 93 +++++++++++++++---------------------------
 1 file changed, 33 insertions(+), 60 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 6d4d18501a..bb6744c0cd 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -411,89 +411,64 @@ static int write_dump(struct pa_space *ps,
     return fclose(dmp_file);
 }
 
-static bool pe_check_export_name(uint64_t base, void *start_addr,
-        struct va_space *vs)
-{
-    IMAGE_EXPORT_DIRECTORY export_dir;
-    const char *pe_name;
-
-    if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_EXPORT_DIRECTORY,
-                &export_dir, sizeof(export_dir), vs)) {
-        return false;
-    }
-
-    pe_name = va_space_resolve(vs, base + export_dir.Name);
-    if (!pe_name) {
-        return false;
-    }
-
-    return !strcmp(pe_name, PE_NAME);
-}
-
-static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,
-        char *hash, struct va_space *vs)
+static bool pe_check_pdb_name(uint64_t base, void *start_addr,
+        struct va_space *vs, OMFSignatureRSDS *rsds)
 {
     const char sign_rsds[4] = "RSDS";
     IMAGE_DEBUG_DIRECTORY debug_dir;
-    OMFSignatureRSDS rsds;
-    char *pdb_name;
-    size_t pdb_name_sz;
-    size_t i;
+    char pdb_name[sizeof(PDB_NAME)];
 
     if (pe_get_data_dir_entry(base, start_addr, IMAGE_FILE_DEBUG_DIRECTORY,
                 &debug_dir, sizeof(debug_dir), vs)) {
         eprintf("Failed to get Debug Directory\n");
-        return 1;
+        return false;
     }
 
     if (debug_dir.Type != IMAGE_DEBUG_TYPE_CODEVIEW) {
-        return 1;
+        eprintf("Debug Directory type is not CodeView\n");
+        return false;
     }
 
     if (va_space_rw(vs,
                 base + debug_dir.AddressOfRawData,
-                &rsds, sizeof(rsds), 0)) {
-        return 1;
+                rsds, sizeof(*rsds), 0)) {
+        eprintf("Failed to resolve OMFSignatureRSDS\n");
+        return false;
     }
 
-    printf("CodeView signature is \'%.4s\'\n", rsds.Signature);
-
-    if (memcmp(&rsds.Signature, sign_rsds, sizeof(sign_rsds))) {
-        return 1;
+    if (memcmp(&rsds->Signature, sign_rsds, sizeof(sign_rsds))) {
+        eprintf("CodeView signature is \'%.4s\', \'%s\' expected\n",
+                rsds->Signature, sign_rsds);
+        return false;
     }
 
-    pdb_name_sz = debug_dir.SizeOfData - sizeof(rsds);
-    pdb_name = malloc(pdb_name_sz);
-    if (!pdb_name) {
-        return 1;
+    if (debug_dir.SizeOfData - sizeof(*rsds) != sizeof(PDB_NAME)) {
+        eprintf("PDB name size doesn't match\n");
+        return false;
     }
 
     if (va_space_rw(vs, base + debug_dir.AddressOfRawData +
-                offsetof(OMFSignatureRSDS, name), pdb_name, pdb_name_sz, 0)) {
-        free(pdb_name);
-        return 1;
+                offsetof(OMFSignatureRSDS, name), pdb_name, sizeof(PDB_NAME),
+                0)) {
+        eprintf("Failed to resolve PDB name\n");
+        return false;
     }
 
     printf("PDB name is \'%s\', \'%s\' expected\n", pdb_name, PDB_NAME);
 
-    if (strcmp(pdb_name, PDB_NAME)) {
-        eprintf("Unexpected PDB name, it seems the kernel isn't found\n");
-        free(pdb_name);
-        return 1;
-    }
-
-    free(pdb_name);
+    return !strcmp(pdb_name, PDB_NAME);
+}
 
-    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds.guid.a, rsds.guid.b,
-            rsds.guid.c, rsds.guid.d[0], rsds.guid.d[1]);
+static void pe_get_pdb_symstore_hash(OMFSignatureRSDS *rsds, char *hash)
+{
+    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds->guid.a, rsds->guid.b,
+            rsds->guid.c, rsds->guid.d[0], rsds->guid.d[1]);
     hash += 20;
-    for (i = 0; i < 6; i++, hash += 2) {
-        sprintf(hash, "%.02x", rsds.guid.e[i]);
+    for (unsigned int i = 0; i < 6; i++, hash += 2) {
+        sprintf(hash, "%.02x", rsds->guid.e[i]);
     }
 
-    sprintf(hash, "%.01x", rsds.age);
-
-    return 0;
+    sprintf(hash, "%.01x", rsds->age);
 }
 
 int main(int argc, char *argv[])
@@ -515,6 +490,7 @@ int main(int argc, char *argv[])
     KDDEBUGGER_DATA64 *kdbg;
     uint64_t KdVersionBlock;
     bool kernel_found = false;
+    OMFSignatureRSDS rsds;
 
     if (argc != 3) {
         eprintf("usage:\n\t%s elf_file dmp_file\n", argv[0]);
@@ -562,7 +538,8 @@ int main(int argc, char *argv[])
         }
 
         if (*(uint16_t *)nt_start_addr == 0x5a4d) { /* MZ */
-            if (pe_check_export_name(KernBase, nt_start_addr, &vs)) {
+            printf("Checking candidate KernBase = 0x%016"PRIx64"\n", KernBase);
+            if (pe_check_pdb_name(KernBase, nt_start_addr, &vs, &rsds)) {
                 kernel_found = true;
                 break;
             }
@@ -578,11 +555,7 @@ int main(int argc, char *argv[])
     printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
             (char *)nt_start_addr);
 
-    if (pe_get_pdb_symstore_hash(KernBase, nt_start_addr, pdb_hash, &vs)) {
-        eprintf("Failed to get PDB symbol store hash\n");
-        err = 1;
-        goto out_ps;
-    }
+    pe_get_pdb_symstore_hash(&rsds, pdb_hash);
 
     sprintf(pdb_url, "%s%s/%s/%s", SYM_URL_BASE, PDB_NAME, pdb_hash, PDB_NAME);
     printf("PDB URL is %s\n", pdb_url);
-- 
2.21.0

[PATCH 2/5] elf2dmp: introduce physical block alignment

[Viktor Prutyanov]

Physical memory ranges may not be aligned to page size in QEMU ELF, but
DMP can only contain page-aligned runs. So, align them.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/addrspace.c | 31 +++++++++++++++++++++++++++++--
 contrib/elf2dmp/addrspace.h |  1 +
 contrib/elf2dmp/main.c      |  5 +++--
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 0b04cba00e..64b5d680ad 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -14,7 +14,7 @@ static struct pa_block *pa_space_find_block(struct pa_space *ps, uint64_t pa)
 
     for (i = 0; i < ps->block_nr; i++) {
         if (ps->block[i].paddr <= pa &&
-                pa <= ps->block[i].paddr + ps->block[i].size) {
+                pa < ps->block[i].paddr + ps->block[i].size) {
             return ps->block + i;
         }
     }
@@ -33,6 +33,30 @@ static uint8_t *pa_space_resolve(struct pa_space *ps, uint64_t pa)
     return block->addr + (pa - block->paddr);
 }
 
+static void pa_block_align(struct pa_block *b)
+{
+    uint64_t low_align = ((b->paddr - 1) | ELF2DMP_PAGE_MASK) + 1 - b->paddr;
+    uint64_t high_align = (b->paddr + b->size) & ELF2DMP_PAGE_MASK;
+
+    if (low_align == 0 && high_align == 0) {
+        return;
+    }
+
+    if (low_align + high_align < b->size) {
+        printf("Block 0x%"PRIx64"+:0x%"PRIx64" will be aligned to "
+                "0x%"PRIx64"+:0x%"PRIx64"\n", b->paddr, b->size,
+                b->paddr + low_align, b->size - low_align - high_align);
+        b->size -= low_align + high_align;
+    } else {
+        printf("Block 0x%"PRIx64"+:0x%"PRIx64" is too small to align\n",
+                b->paddr, b->size);
+        b->size = 0;
+    }
+
+    b->addr += low_align;
+    b->paddr += low_align;
+}
+
 int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
 {
     Elf64_Half phdr_nr = elf_getphdrnum(qemu_elf->map);
@@ -60,10 +84,13 @@ int pa_space_create(struct pa_space *ps, QEMU_Elf *qemu_elf)
                 .paddr = phdr[i].p_paddr,
                 .size = phdr[i].p_filesz,
             };
-            block_i++;
+            pa_block_align(&ps->block[block_i]);
+            block_i = ps->block[block_i].size ? (block_i + 1) : block_i;
         }
     }
 
+    ps->block_nr = block_i;
+
     return 0;
 }
 
diff --git a/contrib/elf2dmp/addrspace.h b/contrib/elf2dmp/addrspace.h
index 00b44c1218..039c70c5b0 100644
--- a/contrib/elf2dmp/addrspace.h
+++ b/contrib/elf2dmp/addrspace.h
@@ -12,6 +12,7 @@
 
 #define ELF2DMP_PAGE_BITS 12
 #define ELF2DMP_PAGE_SIZE (1ULL << ELF2DMP_PAGE_BITS)
+#define ELF2DMP_PAGE_MASK (ELF2DMP_PAGE_SIZE - 1)
 #define ELF2DMP_PFN_MASK (~(ELF2DMP_PAGE_SIZE - 1))
 
 #define INVALID_PA  UINT64_MAX
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index bb6744c0cd..b7e3930164 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -400,9 +400,10 @@ static int write_dump(struct pa_space *ps,
     for (i = 0; i < ps->block_nr; i++) {
         struct pa_block *b = &ps->block[i];
 
-        printf("Writing block #%zu/%zu to file...\n", i, ps->block_nr);
+        printf("Writing block #%zu/%zu of %"PRIu64" bytes to file...\n", i,
+                ps->block_nr, b->size);
         if (fwrite(b->addr, b->size, 1, dmp_file) != 1) {
-            eprintf("Failed to write dump header\n");
+            eprintf("Failed to write block\n");
             fclose(dmp_file);
             return 1;
         }
-- 
2.21.0

[PATCH 3/5] elf2dmp: introduce merging of physical memory runs

[Viktor Prutyanov]

DMP supports 42 physical memory runs at most. So, merge adjacent
physical memory ranges from QEMU ELF when possible to minimize total
number of runs.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/main.c | 56 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index b7e3930164..9ef5cfcd23 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -20,6 +20,7 @@
 #define PE_NAME     "ntoskrnl.exe"
 
 #define INITIAL_MXCSR   0x1f80
+#define MAX_NUMBER_OF_RUNS  42
 
 typedef struct idt_desc {
     uint16_t offset1;   /* offset bits 0..15 */
@@ -234,6 +235,42 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
     return 1;
 }
 
+static void try_merge_runs(struct pa_space *ps,
+        WinDumpPhyMemDesc64 *PhysicalMemoryBlock)
+{
+    unsigned int merge_cnt = 0, run_idx = 0;
+
+    PhysicalMemoryBlock->NumberOfRuns = 0;
+
+    for (unsigned int idx = 0; idx < ps->block_nr; idx++) {
+        struct pa_block *blk = ps->block + idx;
+        struct pa_block *next = blk + 1;
+
+        PhysicalMemoryBlock->NumberOfPages += blk->size / ELF2DMP_PAGE_SIZE;
+
+        if (idx + 1 != ps->block_nr && blk->paddr + blk->size == next->paddr) {
+            printf("Block #%u 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be "
+                    "merged\n", idx, blk->paddr, blk->size, merge_cnt);
+            merge_cnt++;
+        } else {
+            struct pa_block *first_merged = blk - merge_cnt;
+
+            printf("Block #%u 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be "
+                    "merged to 0x%"PRIx64"+:0x%"PRIx64" and saved as run #%u\n",
+                    idx, blk->paddr, blk->size, merge_cnt, first_merged->paddr,
+                    blk->paddr + blk->size - first_merged->paddr, run_idx);
+            PhysicalMemoryBlock->Run[run_idx] = (WinDumpPhyMemRun64) {
+                .BasePage = first_merged->paddr / ELF2DMP_PAGE_SIZE,
+                .PageCount = (blk->paddr + blk->size - first_merged->paddr) /
+                        ELF2DMP_PAGE_SIZE,
+            };
+            PhysicalMemoryBlock->NumberOfRuns++;
+            run_idx++;
+            merge_cnt = 0;
+        }
+    }
+}
+
 static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
         struct va_space *vs, uint64_t KdDebuggerDataBlock,
         KDDEBUGGER_DATA64 *kdbg, uint64_t KdVersionBlock, int nr_cpus)
@@ -244,7 +281,6 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
             KUSD_OFFSET_PRODUCT_TYPE);
     DBGKD_GET_VERSION64 kvb;
     WinDumpHeader64 h;
-    size_t i;
 
     QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= ELF2DMP_PAGE_SIZE);
     QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= ELF2DMP_PAGE_SIZE);
@@ -282,13 +318,17 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
         .RequiredDumpSpace = sizeof(h),
     };
 
-    for (i = 0; i < ps->block_nr; i++) {
-        h.PhysicalMemoryBlock.NumberOfPages +=
-                ps->block[i].size / ELF2DMP_PAGE_SIZE;
-        h.PhysicalMemoryBlock.Run[i] = (WinDumpPhyMemRun64) {
-            .BasePage = ps->block[i].paddr / ELF2DMP_PAGE_SIZE,
-            .PageCount = ps->block[i].size / ELF2DMP_PAGE_SIZE,
-        };
+    if (h.PhysicalMemoryBlock.NumberOfRuns <= MAX_NUMBER_OF_RUNS) {
+        for (unsigned int idx = 0; idx < ps->block_nr; idx++) {
+            h.PhysicalMemoryBlock.NumberOfPages +=
+                    ps->block[idx].size / ELF2DMP_PAGE_SIZE;
+            h.PhysicalMemoryBlock.Run[idx] = (WinDumpPhyMemRun64) {
+                .BasePage = ps->block[idx].paddr / ELF2DMP_PAGE_SIZE,
+                .PageCount = ps->block[idx].size / ELF2DMP_PAGE_SIZE,
+            };
+        }
+    } else {
+        try_merge_runs(ps, &h.PhysicalMemoryBlock);
     }
 
     h.RequiredDumpSpace +=
-- 
2.21.0

[PATCH 4/5] elf2dmp: use Linux mmap with MAP_NORESERVE when possible

[Viktor Prutyanov]

Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and
MAP_PRIVATE. This leads to premature physical memory allocation of dump
file size on Linux hosts and may fail. On Linux, mapping the file with
MAP_NORESERVE limits the allocation by available memory.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++-------
 contrib/elf2dmp/qemu_elf.h |  4 +++
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index ebda60dcb8..94a8c3ad15 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe)
     return true;
 }
 
-int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
+static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename)
 {
+#ifdef CONFIG_LINUX
+    struct stat st;
+
+    printf("Using Linux's mmap\n");
+
+    qe->fd = open(filename, O_RDONLY, 0);
+    if (qe->fd == -1) {
+        eprintf("Failed to open ELF dump file \'%s\'\n", filename);
+        return 1;
+    }
+
+    if (fstat(qe->fd, &st)) {
+        eprintf("Failed to get size of ELF dump file\n");
+        close(qe->fd);
+        return 1;
+    }
+    qe->size = st.st_size;
+
+    qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE,
+            MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0);
+    if (qe->map == MAP_FAILED) {
+        eprintf("Failed to map ELF file\n");
+        close(qe->fd);
+        return 1;
+    }
+#else
     GError *gerr = NULL;
-    int err = 0;
+
+    printf("Using GLib's mmap\n");
 
     qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
     if (gerr) {
@@ -179,29 +206,44 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
 
     qe->map = g_mapped_file_get_contents(qe->gmf);
     qe->size = g_mapped_file_get_length(qe->gmf);
+#endif
+
+    return 0;
+}
+
+static void QEMU_Elf_unmap(QEMU_Elf *qe)
+{
+#ifdef CONFIG_LINUX
+    munmap(qe->map, qe->size);
+    close(qe->fd);
+#else
+    g_mapped_file_unref(qe->gmf);
+#endif
+}
+
+int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
+{
+    if (QEMU_Elf_map(qe, filename)) {
+        return 1;
+    }
 
     if (!check_ehdr(qe)) {
         eprintf("Input file has the wrong format\n");
-        err = 1;
-        goto out_unmap;
+        QEMU_Elf_unmap(qe);
+        return 1;
     }
 
     if (init_states(qe)) {
         eprintf("Failed to extract QEMU CPU states\n");
-        err = 1;
-        goto out_unmap;
+        QEMU_Elf_unmap(qe);
+        return 1;
     }
 
     return 0;
-
-out_unmap:
-    g_mapped_file_unref(qe->gmf);
-
-    return err;
 }
 
 void QEMU_Elf_exit(QEMU_Elf *qe)
 {
     exit_states(qe);
-    g_mapped_file_unref(qe->gmf);
+    QEMU_Elf_unmap(qe);
 }
diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
index b2f0d9cbc9..2a71beca8e 100644
--- a/contrib/elf2dmp/qemu_elf.h
+++ b/contrib/elf2dmp/qemu_elf.h
@@ -32,7 +32,11 @@ typedef struct QEMUCPUState {
 int is_system(QEMUCPUState *s);
 
 typedef struct QEMU_Elf {
+#ifdef CONFIG_POSIX
+    int fd;
+#else
     GMappedFile *gmf;
+#endif
     size_t size;
     void *map;
     QEMUCPUState **state;
-- 
2.21.0

[PATCH 5/5] elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

[Viktor Prutyanov]

PDB for Windows 11 kernel has slightly different structure compared to
previous versions. Since elf2dmp don't use the other fields, copy only
'segments' field from PDB_STREAM_INDEXES.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
---
 contrib/elf2dmp/pdb.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index adcfa7e154..2ef873b13e 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -188,15 +188,11 @@ static int pdb_init_symbols(struct pdb_reader *r)
 
     r->symbols = symbols;
 
-    if (symbols->stream_index_size != sizeof(PDB_STREAM_INDEXES)) {
-        err = 1;
-        goto out_symbols;
-    }
-
-    memcpy(sidx, (const char *)symbols + sizeof(PDB_SYMBOLS) +
+    sidx->segments = *(uint16_t *)((const char *)symbols + sizeof(PDB_SYMBOLS) +
             symbols->module_size + symbols->offset_size +
             symbols->hash_size + symbols->srcmodule_size +
-            symbols->pdbimport_size + symbols->unknown2_size, sizeof(*sidx));
+            symbols->pdbimport_size + symbols->unknown2_size +
+            offsetof(PDB_STREAM_INDEXES, segments));
 
     /* Read global symbol table */
     r->modimage = pdb_ds_read_file(r, symbols->gsym_file);
-- 
2.21.0

Re: [PATCH 4/5] elf2dmp: use Linux mmap with MAP_NORESERVE when possible

[Akihiko Odaki]

On 2023/09/14 7:46, Viktor Prutyanov wrote:
> Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and
> MAP_PRIVATE. This leads to premature physical memory allocation of dump
> file size on Linux hosts and may fail. On Linux, mapping the file with
> MAP_NORESERVE limits the allocation by available memory.
> 
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> ---
>   contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++-------
>   contrib/elf2dmp/qemu_elf.h |  4 +++
>   2 files changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> index ebda60dcb8..94a8c3ad15 100644
> --- a/contrib/elf2dmp/qemu_elf.c
> +++ b/contrib/elf2dmp/qemu_elf.c
> @@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe)
>       return true;
>   }
>   
> -int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
> +static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename)
>   {
> +#ifdef CONFIG_LINUX

Here CONFIG_LINUX is used while qemu_elf.h uses CONFIG_POSIX.
I also wonder if GLib implementation is really necessary.

> +    struct stat st;
> +
> +    printf("Using Linux's mmap\n");
> +
> +    qe->fd = open(filename, O_RDONLY, 0);
> +    if (qe->fd == -1) {
> +        eprintf("Failed to open ELF dump file \'%s\'\n", filename);
> +        return 1;
> +    }
> +
> +    if (fstat(qe->fd, &st)) {
> +        eprintf("Failed to get size of ELF dump file\n");
> +        close(qe->fd);
> +        return 1;
> +    }
> +    qe->size = st.st_size;
> +
> +    qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE,
> +            MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0);

It should be possible to close the file immediately after mmap().

Re: [PATCH 3/5] elf2dmp: introduce merging of physical memory runs

[Akihiko Odaki]

On 2023/09/14 7:46, Viktor Prutyanov wrote:
> DMP supports 42 physical memory runs at most. So, merge adjacent
> physical memory ranges from QEMU ELF when possible to minimize total
> number of runs.
> 
> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
> ---
>   contrib/elf2dmp/main.c | 56 ++++++++++++++++++++++++++++++++++++------
>   1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index b7e3930164..9ef5cfcd23 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -20,6 +20,7 @@
>   #define PE_NAME     "ntoskrnl.exe"
>   
>   #define INITIAL_MXCSR   0x1f80
> +#define MAX_NUMBER_OF_RUNS  42
>   
>   typedef struct idt_desc {
>       uint16_t offset1;   /* offset bits 0..15 */
> @@ -234,6 +235,42 @@ static int fix_dtb(struct va_space *vs, QEMU_Elf *qe)
>       return 1;
>   }
>   
> +static void try_merge_runs(struct pa_space *ps,
> +        WinDumpPhyMemDesc64 *PhysicalMemoryBlock)
> +{
> +    unsigned int merge_cnt = 0, run_idx = 0;
> +
> +    PhysicalMemoryBlock->NumberOfRuns = 0;
> +
> +    for (unsigned int idx = 0; idx < ps->block_nr; idx++) {
> +        struct pa_block *blk = ps->block + idx;
> +        struct pa_block *next = blk + 1;
> +
> +        PhysicalMemoryBlock->NumberOfPages += blk->size / ELF2DMP_PAGE_SIZE;
> +
> +        if (idx + 1 != ps->block_nr && blk->paddr + blk->size == next->paddr) {
> +            printf("Block #%u 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be "
> +                    "merged\n", idx, blk->paddr, blk->size, merge_cnt);
> +            merge_cnt++;
> +        } else {
> +            struct pa_block *first_merged = blk - merge_cnt;
> +
> +            printf("Block #%u 0x%"PRIx64"+:0x%"PRIx64" and %u previous will be "
> +                    "merged to 0x%"PRIx64"+:0x%"PRIx64" and saved as run #%u\n",
> +                    idx, blk->paddr, blk->size, merge_cnt, first_merged->paddr,
> +                    blk->paddr + blk->size - first_merged->paddr, run_idx);
> +            PhysicalMemoryBlock->Run[run_idx] = (WinDumpPhyMemRun64) {
> +                .BasePage = first_merged->paddr / ELF2DMP_PAGE_SIZE,
> +                .PageCount = (blk->paddr + blk->size - first_merged->paddr) /
> +                        ELF2DMP_PAGE_SIZE,
> +            };
> +            PhysicalMemoryBlock->NumberOfRuns++;
> +            run_idx++;
> +            merge_cnt = 0;
> +        }
> +    }
> +}
> +
>   static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
>           struct va_space *vs, uint64_t KdDebuggerDataBlock,
>           KDDEBUGGER_DATA64 *kdbg, uint64_t KdVersionBlock, int nr_cpus)
> @@ -244,7 +281,6 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
>               KUSD_OFFSET_PRODUCT_TYPE);
>       DBGKD_GET_VERSION64 kvb;
>       WinDumpHeader64 h;
> -    size_t i;
>   
>       QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= ELF2DMP_PAGE_SIZE);
>       QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= ELF2DMP_PAGE_SIZE);
> @@ -282,13 +318,17 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
>           .RequiredDumpSpace = sizeof(h),
>       };
>   
> -    for (i = 0; i < ps->block_nr; i++) {
> -        h.PhysicalMemoryBlock.NumberOfPages +=
> -                ps->block[i].size / ELF2DMP_PAGE_SIZE;
> -        h.PhysicalMemoryBlock.Run[i] = (WinDumpPhyMemRun64) {
> -            .BasePage = ps->block[i].paddr / ELF2DMP_PAGE_SIZE,
> -            .PageCount = ps->block[i].size / ELF2DMP_PAGE_SIZE,
> -        };
> +    if (h.PhysicalMemoryBlock.NumberOfRuns <= MAX_NUMBER_OF_RUNS) {
> +        for (unsigned int idx = 0; idx < ps->block_nr; idx++) {

I suggest keep it size_t since that's the type of ps->block_nr. It's 
somewhat annoying typing something long like "unsigned int" too.

Re: [PATCH 5/5] elf2dmp: rework PDB_STREAM_INDEXES::segments obtaining

[Akihiko Odaki]

On 2023/09/14 7:46, Viktor Prutyanov wrote:
> PDB for Windows 11 kernel has slightly different structure compared to
> previous versions. Since elf2dmp don't use the other fields, copy only
> 'segments' field from PDB_STREAM_INDEXES.

I suggest replacing the sidx member of struct pdb_reader with a single 
uint16_t to save some space and prevent accidentally introducing 
references to other members.

Re: [PATCH 4/5] elf2dmp: use Linux mmap with MAP_NORESERVE when possible

[Viktor Prutyanov]

> On 2023/09/14 7:46, Viktor Prutyanov wrote:
> 
>> Glib's g_mapped_file_new maps file with PROT_READ|PROT_WRITE and
>> MAP_PRIVATE. This leads to premature physical memory allocation of dump
>> file size on Linux hosts and may fail. On Linux, mapping the file with
>> MAP_NORESERVE limits the allocation by available memory.
>>
>> Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
>> ---
>> contrib/elf2dmp/qemu_elf.c | 66 +++++++++++++++++++++++++++++++-------
>> contrib/elf2dmp/qemu_elf.h | 4 +++
>> 2 files changed, 58 insertions(+), 12 deletions(-)
>>
>> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
>> index ebda60dcb8..94a8c3ad15 100644
>> --- a/contrib/elf2dmp/qemu_elf.c
>> +++ b/contrib/elf2dmp/qemu_elf.c
>> @@ -165,10 +165,37 @@ static bool check_ehdr(QEMU_Elf *qe)
>> return true;
>> }
>>
>> -int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
>> +static int QEMU_Elf_map(QEMU_Elf *qe, const char *filename)
>> {
>> +#ifdef CONFIG_LINUX
> 
> Here CONFIG_LINUX is used while qemu_elf.h uses CONFIG_POSIX.

Thank you, the right one is CONFIG_LINUX.

> I also wonder if GLib implementation is really necessary.

GLib implementation is for non-Linux OS. Some of them have mmap,
but don't have MAP_NORESERVE. Some other, such as Windows, don't have
POSIX calls at all.

> 
>> + struct stat st;
>> +
>> + printf("Using Linux's mmap\n");
>> +
>> + qe->fd = open(filename, O_RDONLY, 0);
>> + if (qe->fd == -1) {
>> + eprintf("Failed to open ELF dump file \'%s\'\n", filename);
>> + return 1;
>> + }
>> +
>> + if (fstat(qe->fd, &st)) {
>> + eprintf("Failed to get size of ELF dump file\n");
>> + close(qe->fd);
>> + return 1;
>> + }
>> + qe->size = st.st_size;
>> +
>> + qe->map = mmap(NULL, qe->size, PROT_READ | PROT_WRITE,
>> + MAP_PRIVATE | MAP_NORESERVE, qe->fd, 0);
> 
> It should be possible to close the file immediately after mmap().