Ouh, yes please do it. This is the right change. Thanks Kazu. --Alexey On Thu, Mar 7, 2024 at 12:12 AM HAGIO KAZUHITO(萩尾 一仁) <[email protected]> wrote:
> On 2024/03/05 10:44, Alexey Makhalov wrote: > > There are several versions of debug.guest format. Current version of > > the code is able to parse only version 4. > > > > Improve parser to support other known versions. Split data structures > > on sub-structures and introduce a helper functions to calculate a gap > > between them based on the version number. Implement additional data > > structure (struct mainmeminfo_old) and logic specifically for original > > (version 1) format support. > > > > Signed-off-by: Alexey Makhalov <[email protected]> > > This warning is emitted: > > $ make clean ; make warn > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DZSTD -DGDB_10_2 vmware_guestdump.c > -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector > -Wformat-security > vmware_guestdump.c: In function 'is_vmware_guestdump': > vmware_guestdump.c:290:1: warning: label 'unrecognized' defined but not > used [-Wunused-label] > unrecognized: > ^~~~~~~~~~~~ > > Is it OK to just remove this label and the following code? > > --- a/vmware_guestdump.c > +++ b/vmware_guestdump.c > @@ -286,11 +286,6 @@ is_vmware_guestdump(char *filename) > vmss.memoffset = 0; > vmss.num_vcpus = hdr.num_vcpus; > return TRUE; > - > -unrecognized: > - if (CRASHDEBUG(1)) > - error(INFO, LOGPRX"Unrecognized debug.guest file.\n"); > - return FALSE; > } > > int > > If so, we can do it when applying, > > Acked-by: Kazuhito Hagio <[email protected]> > > Thanks, > Kazu > > > --- > > vmware_guestdump.c | 316 ++++++++++++++++++++++++++++++++------------- > > 1 file changed, 229 insertions(+), 87 deletions(-) > > > > diff --git a/vmware_guestdump.c b/vmware_guestdump.c > > index 5be26c8..5c7ee4d 100644 > > --- a/vmware_guestdump.c > > +++ b/vmware_guestdump.c > > @@ -2,6 +2,8 @@ > > * vmware_guestdump.c > > * > > * Copyright (c) 2020 VMware, Inc. > > + * Copyright (c) 2024 Broadcom. All Rights Reserved. The term "Broadcom" > > + * refers to Broadcom Inc. and/or its subsidiaries. > > * > > * This program is free software; you can redistribute it and/or modify > > * it under the terms of the GNU General Public License as published by > > @@ -13,7 +15,7 @@ > > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > * GNU General Public License for more details. > > * > > - * Author: Alexey Makhalov <[email protected]> > > + * Author: Alexey Makhalov <[email protected]> > > */ > > > > #include "defs.h" > > @@ -21,20 +23,31 @@ > > > > #define LOGPRX "vmw: " > > > > -#define GUESTDUMP_VERSION 4 > > -#define GUESTDUMP_MAGIC1 1 > > -#define GUESTDUMP_MAGIC2 0 > > - > > +/* > > + * debug.guest file layout > > + * 00000000: guest dump header, it includes: > > + * 1. Version (4 bytes) \ > > + * 2. Number of Virtual CPUs (4 bytes) } - struct > guestdumpheader > > + * 3. Reserved gap > > + * 4. Main Memory information - struct mainmeminfo{,_old} > > + * (use get_vcpus_offset() to get total size of guestdumpheader) > > + * vcpus_offset: ---------\ > > + * 1. struct vcpu_state1 \ > > + * 2. reserved gap } num_vcpus times > > + * 3. struct vcpu_state2 / > > + * 4. 4KB of reserved data / > > + * --------/ > > + * > > + */ > > struct guestdumpheader { > > uint32_t version; > > uint32_t num_vcpus; > > - uint8_t magic1; > > - uint8_t reserved1; > > - uint32_t cpu_vendor; > > - uint64_t magic2; > > +} __attribute__((packed)) hdr; > > + > > +struct mainmeminfo { > > uint64_t last_addr; > > uint64_t memsize_in_pages; > > - uint32_t reserved2; > > + uint32_t reserved1; > > uint32_t mem_holes; > > struct memhole { > > uint64_t ppn; > > @@ -42,14 +55,36 @@ struct guestdumpheader { > > } holes[2]; > > } __attribute__((packed)); > > > > -struct vcpu_state { > > +/* Used by version 1 only */ > > +struct mainmeminfo_old { > > + uint64_t last_addr; > > + uint32_t memsize_in_pages; > > + uint32_t reserved1; > > + uint32_t mem_holes; > > + struct memhole1 { > > + uint32_t ppn; > > + uint32_t pages; > > + } holes[2]; > > + /* There are additional fields, see get_vcpus_offset() > calculation. */ > > +} __attribute__((packed)); > > + > > +/* First half of vcpu_state */ > > +struct vcpu_state1 { > > uint32_t cr0; > > uint64_t cr2; > > uint64_t cr3; > > uint64_t cr4; > > uint64_t reserved1[10]; > > uint64_t idt_base; > > - uint16_t reserved2[21]; > > +} __attribute__((packed)); > > + > > +/* > > + * Unused fields between vcpu_state1 and vcpu_state2 swill be skipped. > > + * See get_vcpu_gapsize() calculation. > > + */ > > + > > +/* Second half of vcpu_state */ > > +struct vcpu_state2 { > > struct x86_64_pt_regs { > > uint64_t r15; > > uint64_t r14; > > @@ -76,9 +111,41 @@ struct vcpu_state { > > uint8_t reserved3[65]; > > } __attribute__((packed)); > > > > +/* > > + * Returns the size of the guest dump header. > > + */ > > +static inline long > > +get_vcpus_offset(uint32_t version, int mem_holes) > > +{ > > + switch (version) { > > + case 1: /* ESXi 6.7 and older */ > > + return sizeof(struct guestdumpheader) + 13 + > sizeof(struct mainmeminfo_old) + > > + (mem_holes == -1 ? 0 : 8 * mem_holes + 4); > > + case 3: /* ESXi 6.8 */ > > + return sizeof(struct guestdumpheader) + 14 + > sizeof(struct mainmeminfo); > > + case 4: /* ESXi 7.0 */ > > + case 5: /* ESXi 8.0 */ > > + return sizeof(struct guestdumpheader) + 14 + > sizeof(struct mainmeminfo); > > + case 6: /* ESXi 8.0u2 */ > > + return sizeof(struct guestdumpheader) + 15 + > sizeof(struct mainmeminfo); > > + > > + } > > + return 0; > > +} > > + > > +/* > > + * Returns the size of reserved (unused) fields in the middle of > vcpu_state structure. > > + */ > > +static inline long > > +get_vcpu_gapsize(uint32_t version) > > +{ > > + if (version < 4) > > + return 45; > > + return 42; > > +} > > > > /* > > - * vmware_guestdump is extension to vmware_vmss with ability to debug > > + * vmware_guestdump is an extension to the vmware_vmss with ability to > debug > > * debug.guest and debug.vmem files. > > * > > * debug.guest.gz and debug.vmem.gz can be obtained using following > > @@ -86,73 +153,136 @@ struct vcpu_state { > > * monitor.mini-suspend_on_panic = TRUE > > * monitor.suspend_on_triplefault = TRUE > > * > > - * guestdump (debug.guest) is simplified version of *.vmss which does > > - * not contain full VM state, but minimal guest state, such as memory > > + * guestdump (debug.guest) is a simplified version of the *.vmss which > does > > + * not contain a full VM state, but minimal guest state, such as a > memory > > * layout and CPUs state, needed for debugger. is_vmware_guestdump() > > * and vmware_guestdump_init() functions parse guestdump header and > > - * populate vmss data structure (from vmware_vmss.c). As result, all > > + * populate vmss data structure (from vmware_vmss.c). In result, all > > * handlers (except mempry_dump) from vmware_vmss.c can be reused. > > * > > - * debug.guest does not have dedicated header magic or signature for > > - * its format. To probe debug.guest we need to perform header fields > > - * and file size validity. In addition, check for the filename > > - * extension, which must be ".guest". > > + * debug.guest does not have a dedicated header magic or file format > signature > > + * To probe debug.guest we need to perform series of validations. In > addition, > > + * we check for the filename extension, which must be ".guest". > > */ > > - > > int > > is_vmware_guestdump(char *filename) > > { > > - struct guestdumpheader hdr; > > + struct mainmeminfo mmi; > > + long vcpus_offset; > > FILE *fp; > > - uint64_t filesize, holes_sum = 0; > > + uint64_t filesize, expected_filesize, holes_sum = 0; > > int i; > > > > if (strcmp(filename + strlen(filename) - 6, ".guest")) > > return FALSE; > > > > - if ((fp = fopen(filename, "r")) == NULL) { > > + if ((fp = fopen(filename, "r")) == NULL) { > > error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n", > > - filename, errno, strerror(errno)); > > + filename, errno, strerror(errno)); > > return FALSE; > > - } > > + } > > > > if (fread(&hdr, sizeof(struct guestdumpheader), 1, fp) != 1) { > > error(INFO, LOGPRX"Failed to read '%s' from file '%s': > [Error %d] %s\n", > > - "guestdumpheader", filename, errno, strerror(errno)); > > + "guestdumpheader", filename, errno, > strerror(errno)); > > + fclose(fp); > > + return FALSE; > > + } > > + > > + vcpus_offset = get_vcpus_offset(hdr.version, -1 /* Unknown yet, > adjust it later */); > > + > > + if (!vcpus_offset) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Not supported version %d\n", > hdr.version); > > fclose(fp); > > return FALSE; > > } > > > > + if (hdr.version == 1) { > > + struct mainmeminfo_old tmp; > > + if (fseek(fp, vcpus_offset - sizeof(struct > mainmeminfo_old), SEEK_SET) == -1) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Failed to fseek '%s': > [Error %d] %s\n", > > + filename, errno, > strerror(errno)); > > + fclose(fp); > > + return FALSE; > > + } > > + > > + if (fread(&tmp, sizeof(struct mainmeminfo_old), 1, fp) != > 1) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Failed to read '%s' > from file '%s': [Error %d] %s\n", > > + "mainmeminfo_old", > filename, errno, strerror(errno)); > > + fclose(fp); > > + return FALSE; > > + } > > + mmi.last_addr = tmp.last_addr; > > + mmi.memsize_in_pages = tmp.memsize_in_pages; > > + mmi.mem_holes = tmp.mem_holes; > > + mmi.holes[0].ppn = tmp.holes[0].ppn; > > + mmi.holes[0].pages = tmp.holes[0].pages; > > + mmi.holes[1].ppn = tmp.holes[1].ppn; > > + mmi.holes[1].pages = tmp.holes[1].pages; > > + /* vcpu_offset adjustment for mem_holes is required only > for version 1. */ > > + vcpus_offset = get_vcpus_offset(hdr.version, > mmi.mem_holes); > > + } else { > > + if (fseek(fp, vcpus_offset - sizeof(struct mainmeminfo), > SEEK_SET) == -1) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Failed to fseek '%s': > [Error %d] %s\n", > > + filename, errno, > strerror(errno)); > > + fclose(fp); > > + return FALSE; > > + } > > + > > + if (fread(&mmi, sizeof(struct mainmeminfo), 1, fp) != 1) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Failed to read '%s' > from file '%s': [Error %d] %s\n", > > + "mainmeminfo", filename, > errno, strerror(errno)); > > + fclose(fp); > > + return FALSE; > > + } > > + } > > if (fseek(fp, 0L, SEEK_END) == -1) { > > - error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", > > - filename, errno, strerror(errno)); > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Failed to fseek '%s': [Error > %d] %s\n", > > + filename, errno, strerror(errno)); > > fclose(fp); > > return FALSE; > > } > > filesize = ftell(fp); > > fclose(fp); > > > > - if (hdr.mem_holes > 2) > > - goto unrecognized; > > + if (mmi.mem_holes > 2) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Unexpected mmi.mem_holes value > %d\n", > > + mmi.mem_holes); > > + return FALSE; > > + } > > > > - for (i = 0; i < hdr.mem_holes; i++) { > > + for (i = 0; i < mmi.mem_holes; i++) { > > /* hole start page */ > > - vmss.regions[i].startpagenum = hdr.holes[i].ppn; > > + vmss.regions[i].startpagenum = mmi.holes[i].ppn; > > /* hole end page */ > > - vmss.regions[i].startppn = hdr.holes[i].ppn + > hdr.holes[i].pages; > > - holes_sum += hdr.holes[i].pages; > > + vmss.regions[i].startppn = mmi.holes[i].ppn + > mmi.holes[i].pages; > > + holes_sum += mmi.holes[i].pages; > > + } > > + > > + if ((mmi.last_addr + 1) != ((mmi.memsize_in_pages + holes_sum) << > VMW_PAGE_SHIFT)) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Memory size check failed\n"); > > + return FALSE; > > } > > > > - if (hdr.version != GUESTDUMP_VERSION || > > - hdr.magic1 != GUESTDUMP_MAGIC1 || > > - hdr.magic2 != GUESTDUMP_MAGIC2 || > > - (hdr.last_addr + 1) != ((hdr.memsize_in_pages + holes_sum) << > VMW_PAGE_SHIFT) || > > - filesize != sizeof(struct guestdumpheader) + > > - hdr.num_vcpus * (sizeof (struct vcpu_state) + VMW_PAGE_SIZE)) > > - goto unrecognized; > > + expected_filesize = vcpus_offset + hdr.num_vcpus * (sizeof(struct > vcpu_state1) + > > + get_vcpu_gapsize(hdr.version) + sizeof(struct vcpu_state2) > + VMW_PAGE_SIZE); > > + if (filesize != expected_filesize) { > > + if (CRASHDEBUG(1)) > > + error(INFO, LOGPRX"Incorrect file size: %d != > %d\n", > > + filesize, expected_filesize); > > + return FALSE; > > + } > > > > - vmss.memsize = hdr.memsize_in_pages << VMW_PAGE_SHIFT; > > - vmss.regionscount = hdr.mem_holes + 1; > > + vmss.memsize = mmi.memsize_in_pages << VMW_PAGE_SHIFT; > > + vmss.regionscount = mmi.mem_holes + 1; > > vmss.memoffset = 0; > > vmss.num_vcpus = hdr.num_vcpus; > > return TRUE; > > @@ -169,7 +299,8 @@ vmware_guestdump_init(char *filename, FILE *ofp) > > FILE *fp = NULL; > > int i, result = TRUE; > > char *vmem_filename = NULL; > > - struct vcpu_state vs; > > + struct vcpu_state1 vs1; > > + struct vcpu_state2 vs2; > > char *p; > > > > if (!machine_type("X86") && !machine_type("X86_64")) { > > @@ -180,14 +311,14 @@ vmware_guestdump_init(char *filename, FILE *ofp) > > goto exit; > > } > > > > - if ((fp = fopen(filename, "r")) == NULL) { > > + if ((fp = fopen(filename, "r")) == NULL) { > > error(INFO, LOGPRX"Failed to open '%s': [Error %d] %s\n", > > filename, errno, strerror(errno)); > > result = FALSE; > > goto exit; > > - } > > + } > > > > - if (fseek(fp, sizeof(struct guestdumpheader), SEEK_SET) == -1) { > > + if (fseek(fp, get_vcpus_offset(hdr.version, vmss.regionscount - > 1), SEEK_SET) == -1) { > > error(INFO, LOGPRX"Failed to fseek '%s': [Error %d] %s\n", > > filename, errno, strerror(errno)); > > result = FALSE; > > @@ -203,7 +334,19 @@ vmware_guestdump_init(char *filename, FILE *ofp) > > } > > > > for (i = 0; i < vmss.num_vcpus; i++) { > > - if (fread(&vs, sizeof(struct vcpu_state), 1, fp) != 1) { > > + if (fread(&vs1, sizeof(struct vcpu_state1), 1, fp) != 1) { > > + error(INFO, LOGPRX"Failed to read '%s' from file > '%s': [Error %d] %s\n", > > + "vcpu_state", filename, errno, > strerror(errno)); > > + result = FALSE; > > + goto exit; > > + } > > + if (fseek(fp, get_vcpu_gapsize(hdr.version), SEEK_CUR) == > -1) { > > + error(INFO, LOGPRX"Failed to read '%s' from file > '%s': [Error %d] %s\n", > > + "vcpu_state", filename, errno, > strerror(errno)); > > + result = FALSE; > > + goto exit; > > + } > > + if (fread(&vs2, sizeof(struct vcpu_state2), 1, fp) != 1) { > > error(INFO, LOGPRX"Failed to read '%s' from file > '%s': [Error %d] %s\n", > > "vcpu_state", filename, errno, > strerror(errno)); > > result = FALSE; > > @@ -217,29 +360,29 @@ vmware_guestdump_init(char *filename, FILE *ofp) > > } > > vmss.vcpu_regs[i] = 0; > > > > - vmss.regs64[i]->rax = vs.regs64.rax; > > - vmss.regs64[i]->rcx = vs.regs64.rcx; > > - vmss.regs64[i]->rdx = vs.regs64.rdx; > > - vmss.regs64[i]->rbx = vs.regs64.rbx; > > - vmss.regs64[i]->rbp = vs.regs64.rbp; > > - vmss.regs64[i]->rsp = vs.regs64.rsp; > > - vmss.regs64[i]->rsi = vs.regs64.rsi; > > - vmss.regs64[i]->rdi = vs.regs64.rdi; > > - vmss.regs64[i]->r8 = vs.regs64.r8; > > - vmss.regs64[i]->r9 = vs.regs64.r9; > > - vmss.regs64[i]->r10 = vs.regs64.r10; > > - vmss.regs64[i]->r11 = vs.regs64.r11; > > - vmss.regs64[i]->r12 = vs.regs64.r12; > > - vmss.regs64[i]->r13 = vs.regs64.r13; > > - vmss.regs64[i]->r14 = vs.regs64.r14; > > - vmss.regs64[i]->r15 = vs.regs64.r15; > > - vmss.regs64[i]->idtr = vs.idt_base; > > - vmss.regs64[i]->cr[0] = vs.cr0; > > - vmss.regs64[i]->cr[2] = vs.cr2; > > - vmss.regs64[i]->cr[3] = vs.cr3; > > - vmss.regs64[i]->cr[4] = vs.cr4; > > - vmss.regs64[i]->rip = vs.regs64.rip; > > - vmss.regs64[i]->rflags = vs.regs64.eflags; > > + vmss.regs64[i]->rax = vs2.regs64.rax; > > + vmss.regs64[i]->rcx = vs2.regs64.rcx; > > + vmss.regs64[i]->rdx = vs2.regs64.rdx; > > + vmss.regs64[i]->rbx = vs2.regs64.rbx; > > + vmss.regs64[i]->rbp = vs2.regs64.rbp; > > + vmss.regs64[i]->rsp = vs2.regs64.rsp; > > + vmss.regs64[i]->rsi = vs2.regs64.rsi; > > + vmss.regs64[i]->rdi = vs2.regs64.rdi; > > + vmss.regs64[i]->r8 = vs2.regs64.r8; > > + vmss.regs64[i]->r9 = vs2.regs64.r9; > > + vmss.regs64[i]->r10 = vs2.regs64.r10; > > + vmss.regs64[i]->r11 = vs2.regs64.r11; > > + vmss.regs64[i]->r12 = vs2.regs64.r12; > > + vmss.regs64[i]->r13 = vs2.regs64.r13; > > + vmss.regs64[i]->r14 = vs2.regs64.r14; > > + vmss.regs64[i]->r15 = vs2.regs64.r15; > > + vmss.regs64[i]->idtr = vs1.idt_base; > > + vmss.regs64[i]->cr[0] = vs1.cr0; > > + vmss.regs64[i]->cr[2] = vs1.cr2; > > + vmss.regs64[i]->cr[3] = vs1.cr3; > > + vmss.regs64[i]->cr[4] = vs1.cr4; > > + vmss.regs64[i]->rip = vs2.regs64.rip; > > + vmss.regs64[i]->rflags = vs2.regs64.eflags; > > > > vmss.vcpu_regs[i] = REGS_PRESENT_ALL; > > } > > @@ -268,9 +411,9 @@ vmware_guestdump_init(char *filename, FILE *ofp) > > fprintf(ofp, LOGPRX"vmem file: %s\n\n", vmem_filename); > > > > if (CRASHDEBUG(1)) { > > - vmware_guestdump_memory_dump(ofp); > > - dump_registers_for_vmss_dump(); > > - } > > + vmware_guestdump_memory_dump(ofp); > > + dump_registers_for_vmss_dump(); > > + } > > > > exit: > > if (fp) > > @@ -296,24 +439,23 @@ exit: > > int > > vmware_guestdump_memory_dump(FILE *ofp) > > { > > + uint64_t holes_sum = 0; > > + unsigned i; > > + > > fprintf(ofp, "vmware_guestdump:\n"); > > fprintf(ofp, " Header: version=%d num_vcpus=%llu\n", > > - GUESTDUMP_VERSION, (ulonglong)vmss.num_vcpus); > > + hdr.version, (ulonglong)vmss.num_vcpus); > > fprintf(ofp, "Total memory: %llu\n", (ulonglong)vmss.memsize); > > > > - if (vmss.regionscount > 1) { > > - uint64_t holes_sum = 0; > > - unsigned i; > > > > - fprintf(ofp, "Memory regions[%d]:\n", vmss.regionscount); > > - fprintf(ofp, " [0x%016x-", 0); > > - for (i = 0; i < vmss.regionscount - 1; i++) { > > - fprintf(ofp, "0x%016llx]\n", > (ulonglong)vmss.regions[i].startpagenum << VMW_PAGE_SHIFT); > > - fprintf(ofp, " [0x%016llx-", > (ulonglong)vmss.regions[i].startppn << VMW_PAGE_SHIFT); > > - holes_sum += vmss.regions[i].startppn - > vmss.regions[i].startpagenum; > > - } > > - fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.memsize + > (holes_sum << VMW_PAGE_SHIFT)); > > + fprintf(ofp, "Memory regions[%d]:\n", vmss.regionscount); > > + fprintf(ofp, " [0x%016x-", 0); > > + for (i = 0; i < vmss.regionscount - 1; i++) { > > + fprintf(ofp, "0x%016llx]\n", > (ulonglong)vmss.regions[i].startpagenum << VMW_PAGE_SHIFT); > > + fprintf(ofp, " [0x%016llx-", > (ulonglong)vmss.regions[i].startppn << VMW_PAGE_SHIFT); > > + holes_sum += vmss.regions[i].startppn - > vmss.regions[i].startpagenum; > > } > > + fprintf(ofp, "0x%016llx]\n", (ulonglong)vmss.memsize + (holes_sum > << VMW_PAGE_SHIFT)); > > > > return TRUE; > > }
-- Crash-utility mailing list -- [email protected] To unsubscribe send an email to [email protected] https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
