[Crash-utility] patch to add vmss memory regions support
Dyno (Hongjun) Fu
hfu at vmware.com
Tue Mar 24 22:07:42 UTC 2015
Dave,
updated the patch and please review. thanks.
- the page_size/page_shift problem.
- change type cast to union.
- the read_vmware_vmss() regression.
rgds,
Dyno
On 3/24/15 11:52 AM, Dave Anderson wrote:
> Dyno,
>
> I haven't really begun to review the patch yet, but when I was checking
> read_vmware_vmss() for the incorrect PAGE_SHIFT usage, I notice that this
> change is also incorrect:
>
> - if (fread(bufptr, 1 , cnt, vmss.dfp) != cnt)
> - return READ_ERROR;
> + cnt = fread(bufptr, 1, cnt, vmss.dfp);
>
> return cnt;
> }
>
> The readmem() function keys on READ_ERROR, SEEK_ERROR and PAGE_EXCLUDED,
> and if none of those are returned, it presumes the read was successful.
> See the READMEM() macro/call in the readmem() function.
>
> Dave
>
>
>
> ----- Original Message -----
>>
>> ----- Original Message -----
>>> Dave,
>>> patch attached. thanks.
>>> rgds,
>>> Dyno
>>
>> OK thanks, but now can you re-work the patch so that crash builds
>> cleanly with "make warn"? Here on an x86_64 with gcc-4.7.2:
>>
>> $ make warn
>> TARGET: X86_64
>> CRASH: 7.1.1rc12
>> GDB: 7.6
>>
>> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 build_data.c -Wall -O2
>> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
>> -Wformat-security
>> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 vmware_vmss.c -Wall -O2
>> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
>> -Wformat-security
>> vmware_vmss.c: In function ‘vmware_vmss_init’:
>> vmware_vmss.c:218:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmware_vmss.c:222:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmware_vmss.c:225:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmware_vmss.c:228:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmware_vmss.c:231:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> ...
>>
>> And then for a quick sanity check, I noted that the patch won't even
>> compile at all on an ARM64 machine:
>>
>> $ make warn
>> TARGET: ARM64
>> CRASH: 7.1.0
>> GDB: 7.6
>>
>> cc -c -g -DARM64 -DGDB_7_6 build_data.c -Wall -O2 -Wstrict-prototypes
>> -Wmissing-prototypes -fstack-protector -Wformat-security
>> cc -c -g -DARM64 -DGDB_7_6 netdump.c -Wall -O2 -Wstrict-prototypes
>> -Wmissing-prototypes -fstack-protector -Wformat-security
>> cc -c -g -DARM64 -DGDB_7_6 vmware_vmss.c -Wall -O2 -Wstrict-prototypes
>> -Wmissing-prototypes -fstack-protector -Wformat-security
>> vmware_vmss.c: In function ‘vmware_vmss_init’:
>> vmware_vmss.c:218:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmss.regionscount = *(uint32_t*)val;
>> ^
>> vmware_vmss.c:222:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmss.regions[idx[0]].startpagenum = *(uint32_t*)val;
>> ^
>> vmware_vmss.c:225:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmss.regions[idx[0]].startppn = *(uint32_t*)val;
>> ^
>> vmware_vmss.c:228:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmss.regions[idx[0]].size = *(uint32_t*)val;
>> ^
>> vmware_vmss.c:231:7: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules [-Wstrict-aliasing]
>> vmss.alignmask = *(uint32_t*)val;
>> ^
>> vmware_vmss.c: In function ‘vmware_vmss_page_size’:
>> vmware_vmss.c:277:9: error: ‘PAGE_SIZE’ undeclared (first use in this
>> function)
>> return PAGE_SIZE;
>> ^
>> vmware_vmss.c:277:9: note: each undeclared identifier is reported only once
>> for each function it appears in
>> vmware_vmss.c: In function ‘read_vmware_vmss’:
>> vmware_vmss.c:287:37: error: ‘PAGE_SHIFT’ undeclared (first use in this
>> function)
>> uint32_t ppn = (uint32_t) (pos >> PAGE_SHIFT);
>> ^
>> vmware_vmss.c: In function ‘vmware_vmss_page_size’:
>> vmware_vmss.c:278:1: warning: control reaches end of non-void function
>> [-Wreturn-type]
>> }
>> ^
>> make[4]: *** [vmware_vmss.o] Error 1
>> make[3]: *** [gdb] Error 2
>> make[2]: *** [rebuild] Error 2
>> make[1]: *** [gdb_merge] Error 2
>> make: *** [warn] Error 2
>> $
>>
>> And that's because PAGE_SIZE is only #define'd in defs.h for x86_64, and
>> and PAGE_SHIFT is only #define'd in defs.h for x86_64 and ppc32.
>>
>> There are machdep->pagesize and machdep->pageshift that get assigned for
>> each architecture. So you could use machdep->pageshift in your patch
>> to read_vmware_vmss().
>>
>>
>> But then there's this:
>>
>> uint vmware_vmss_page_size(void)
>> {
>> - return 4096;
>> + return PAGE_SIZE;
>> }
>>
>> which is pretty much the wrong way to do it both before and after.
>> I'm not sure how you want to handle it.
>>
>> Dave
>>
>>
>>
>>
>>
>>
>>
>>> On 3/24/15 10:00 AM, Dyno Hongjun Fu wrote:
>>>> ---------- Forwarded message ----------
>>>> From: Dave Anderson <anderson at redhat.com>
>>>> Date: Tue, Mar 24, 2015 at 7:17 AM
>>>> Subject: Re: [Crash-utility] patch to add vmss memory regions support
>>>> To: "Discussion list for crash utility usage, maintenance and
>>>> development" <crash-utility at redhat.com>
>>>>
>>>>
>>>>
>>>> Dyno,
>>>>
>>>> Can you re-post this path so that it applies cleanly to the
>>>> current git repository? This is what happens:
>>>>
>>>> $ git clone
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_crash-2Dutility_crash.git&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=CMSBEaJrUe0l1I8xeRN7TmIsyloBPUmxxxRDdOkWC_0&s=urNZ6YRJ7FSXVWnWZBOKvR8FnqMpawcqdLe_2jNTvo0&e=
>>>> Cloning into 'crash'...
>>>> remote: Counting objects: 811, done.
>>>> remote: Total 811 (delta 0), reused 0 (delta 0), pack-reused 811
>>>> Receiving objects: 100% (811/811), 1.87 MiB | 1.04 MiB/s, done.
>>>> Resolving deltas: 100% (532/532), done.
>>>> $ cd crash
>>>> $ patch -p1 < ../vmware_memory_region_support.patch
>>>> patching file vmware_vmss.c
>>>> Reversed (or previously applied) patch detected! Assume -R? [n] n
>>>> Apply anyway? [n] y
>>>> Hunk #1 succeeded at 42 with fuzz 2 (offset 2 lines).
>>>> Hunk #2 succeeded at 141 (offset 2 lines).
>>>> Hunk #3 succeeded at 164 (offset 2 lines).
>>>> Hunk #4 succeeded at 192 (offset 2 lines).
>>>> Hunk #5 succeeded at 217 (offset 2 lines).
>>>> Hunk #6 FAILED at 256.
>>>> Hunk #7 succeeded at 276 (offset 2 lines).
>>>> 1 out of 7 hunks FAILED -- saving rejects to file vmware_vmss.c.rej
>>>> patching file vmware_vmss.h
>>>> $
>>>>
>>>> Thanks,
>>>> Dave
>>>>
>>>>
>>>> ----- Original Message -----
>>>>> Dave,
>>>>>
>>>>> attached patch is to add vmss memroy regions support.
>>>>>
>>>>> There might be holes in the memory address saved for PCI etc.
>>>>> In such case memory dump is divided into regions. Currently
>>>>> up to 3 regions are supported.
>>>>>
>>>>> Memory dump larger than 3GB will have the first hole.
>>>>> My dropbox space used up, I cannot attach a 4GB memory dump.
>>>>> and here is how it looks like in the vmss meta data dump.
>>>>>
>>>>> 3GB:
>>>>> ===
>>>>> - Group: memory pos=0x1f6f6 size=0xc000090c
>>>>> ------------------------------------
>>>>> align_mask[0, 0] => 0x00ffff
>>>>> regionsCount => 0x000000
>>>>> Memory[0, 0] => BLOCK, pos=0x20000, size=0xc0000000
>>>>>
>>>>> 4GB:
>>>>> ===
>>>>> - Group: memory pos=0x1f6f6 size=0x10000090c
>>>>> -----------------------------------
>>>>> align_mask[0, 0] => 0x00ffff
>>>>> regionsCount => 0x000002
>>>>> regionPageNum[0] => 0x000000
>>>>> regionPPN[0] => 0x000000
>>>>> regionSize[0] => 0x0c0000
>>>>> regionPageNum[1] => 0x0c0000
>>>>> regionPPN[1] => 0x100000
>>>>> regionSize[1] => 0x040000
>>>>> Memory[0, 0] => BLOCK, pos=0x20000, size=0x100000000
>>>>>
>>>>> rgds,
>>>>> Dyno
>>>>>
>>>>> --
>>>>> Crash-utility mailing list
>>>>> Crash-utility at redhat.com
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailman_listinfo_crash-2Dutility&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=CMSBEaJrUe0l1I8xeRN7TmIsyloBPUmxxxRDdOkWC_0&s=jsnuoM4kieLx44ivbcbd1s3TI9JGIOUsTTtZ4j5ytZw&e=
>>>> --
>>>> Crash-utility mailing list
>>>> Crash-utility at redhat.com
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailman_listinfo_crash-2Dutility&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=CMSBEaJrUe0l1I8xeRN7TmIsyloBPUmxxxRDdOkWC_0&s=jsnuoM4kieLx44ivbcbd1s3TI9JGIOUsTTtZ4j5ytZw&e=
>>>>
>>>>
>>> --
>>> Crash-utility mailing list
>>> Crash-utility at redhat.com
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailman_listinfo_crash-2Dutility&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=55WeomeY5CnqBNROsRJVFP_OOG8qG9F9ll8NKL52w8c&s=GQsVt0YEwaAqS48y4NCMQiwKZu9RDz0hJWKzU1-dtqE&e=
>> --
>> Crash-utility mailing list
>> Crash-utility at redhat.com
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailman_listinfo_crash-2Dutility&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=55WeomeY5CnqBNROsRJVFP_OOG8qG9F9ll8NKL52w8c&s=GQsVt0YEwaAqS48y4NCMQiwKZu9RDz0hJWKzU1-dtqE&e=
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.redhat.com_mailman_listinfo_crash-2Dutility&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=g2Vka_25x09RSowRkQw8pA&m=55WeomeY5CnqBNROsRJVFP_OOG8qG9F9ll8NKL52w8c&s=GQsVt0YEwaAqS48y4NCMQiwKZu9RDz0hJWKzU1-dtqE&e=
-------------- next part --------------
commit b22765ac59bff1661e08c748954b3a3138922b2a
Author: Dyno (Hongjun) Fu <hfu at vmware.com>
Date: Tue Mar 24 13:47:46 2015 -0700
add vmss memory regions support
There might be holes in the guest os memory address saved for PCI
etc. The memory dump will be divided into regions to skip these
holes.
On vSphere 5.5, RHEL 6.5 memory dump larger than 3GB is such a
case.
diff --git a/vmware_vmss.c b/vmware_vmss.c
index aeb1c1d..b961d09 100644
--- a/vmware_vmss.c
+++ b/vmware_vmss.c
@@ -21,6 +21,10 @@
#define LOGPRX "vmw: "
+/* VMware only supports X86/X86_64 virtual machines. */
+#define VMW_PAGE_SIZE (4096)
+#define VMW_PAGE_SHIFT (12)
+
static vmssdata vmss = { 0 };
int
@@ -139,8 +143,7 @@ vmware_vmss_init(char *filename, FILE *ofp)
break;
}
name[nameLen] = 0;
- DEBUG_PARSE_PRINT((vmss.ofp, LOGPRX"\t Item %20s",
- name));
+ DEBUG_PARSE_PRINT((vmss.ofp, LOGPRX"\t Item %20s", name));
nindx = TAG_NINDX(tag);
if (nindx > 3) {
@@ -163,6 +166,8 @@ vmware_vmss_init(char *filename, FILE *ofp)
int compressed = IS_BLOCK_COMPRESSED_TAG(tag);
uint16_t padsize;
+ assert(strcmp(name, "Memory") == 0);
+
if (fread(&nbytes, sizeof(nbytes), 1, vmss.dfp) != 1) {
fprintf(vmss.ofp, LOGPRX"Cannot read block size.\n");
break;
@@ -188,46 +193,50 @@ vmware_vmss_init(char *filename, FILE *ofp)
}
/* The things that we really care about...*/
- if (strcmp(grps[i].name, "memory") == 0 &&
- strcmp(name, "Memory") == 0) {
- vmss.memoffset = blockpos;
- vmss.memsize = nbytesinmem;
- }
+ vmss.memoffset = blockpos;
+ vmss.memsize = nbytesinmem;
DEBUG_PARSE_PRINT((vmss.ofp, "\t=> %sBLOCK: position=%#llx size=%#llx memsize=%#llx\n",
compressed ? "COMPRESSED " : "",
(ulonglong)blockpos, (ulonglong)nbytes, (ulonglong)nbytesinmem));
+ assert (!compressed);
} else {
- uint8_t val[TAG_VALSIZE_MASK];
+ union {
+ uint8_t val[TAG_VALSIZE_MASK];
+ uint32_t val32;
+ } u;
unsigned k;
unsigned valsize = TAG_VALSIZE(tag);
uint64_t blockpos = ftell(vmss.dfp);
DEBUG_PARSE_PRINT((vmss.ofp, "\t=> position=%#llx size=%#x: ", (ulonglong)blockpos, valsize));
- if (fread(val, sizeof(val[0]), valsize, vmss.dfp) != valsize) {
+ if (fread(u.val, sizeof(u.val[0]), valsize, vmss.dfp) != valsize) {
fprintf(vmss.ofp, LOGPRX"Cannot read item.\n");
break;
}
for (k = 0; k < valsize; k++) {
/* Assume Little Endian */
- DEBUG_PARSE_PRINT((vmss.ofp, "%02X", val[valsize - k - 1]));
+ DEBUG_PARSE_PRINT((vmss.ofp, "%02X", u.val[valsize - k - 1]));
}
if (strcmp(grps[i].name, "memory") == 0) {
if (strcmp(name, "regionsCount") == 0) {
- vmss.regionscount = (uint32_t) *val;
- if (vmss.regionscount != 0) {
- fprintf(vmss.ofp, LOGPRX"regionsCount=%d (!= 0) NOT TESTED!",
- vmss.regionscount);
- }
+ vmss.regionscount = u.val32;
+ assert(vmss.regionscount <= MAX_REGIONS);
+ }
+ if (strcmp(name, "regionPageNum") == 0) {
+ vmss.regions[idx[0]].startpagenum = u.val32;
+ }
+ if (strcmp(name, "regionPPN") == 0) {
+ vmss.regions[idx[0]].startppn = u.val32;
+ }
+ if (strcmp(name, "regionSize") == 0) {
+ vmss.regions[idx[0]].size = u.val32;
}
if (strcmp(name, "align_mask") == 0) {
- vmss.alignmask = (uint32_t) *val;
- if (vmss.alignmask != 0xff) {
- fprintf(vmss.ofp, LOGPRX"align_mask=%d (!= 0xff) NOT TESTED!",
- vmss.regionscount);
- }
+ vmss.alignmask = u.val32;
+ assert(vmss.alignmask == 0xFFFF);
}
}
@@ -272,26 +281,35 @@ vmware_vmss_init(char *filename, FILE *ofp)
uint vmware_vmss_page_size(void)
{
- return 4096;
+ return VMW_PAGE_SIZE;
}
int
read_vmware_vmss(int fd, void *bufptr, int cnt, ulong addr, physaddr_t paddr)
{
- uint64_t pos = vmss.memoffset + paddr;
+ uint64_t pos = paddr;
+
+ if (vmss.regionscount > 0) {
+ /* Memory is divided into regions and there are holes between them. */
+ uint32_t ppn = (uint32_t) (pos >> VMW_PAGE_SHIFT);
+ int i;
+
+ for (i = 0; i < vmss.regionscount; i++) {
+ if (ppn < vmss.regions[i].startppn)
+ break;
- if (pos + cnt > vmss.memoffset + vmss.memsize) {
- cnt -= ((pos + cnt) - (vmss.memoffset + vmss.memsize));
- if (cnt < 0) {
- error(INFO, LOGPRX"Read beyond the end of file! paddr=%#lx\n",
- paddr);
+ /* skip holes. */
+ pos -= ((vmss.regions[i].startppn - vmss.regions[i].startpagenum)
+ << VMW_PAGE_SHIFT);
}
}
+ assert(pos + cnt <= vmss.memsize);
+ pos += vmss.memoffset;
if (fseek(vmss.dfp, pos, SEEK_SET) != 0)
return SEEK_ERROR;
- if (fread(bufptr, 1 , cnt, vmss.dfp) != cnt)
+ if (fread(bufptr, 1, cnt, vmss.dfp) != cnt)
return READ_ERROR;
return cnt;
diff --git a/vmware_vmss.h b/vmware_vmss.h
index dcbde2d..3f46188 100644
--- a/vmware_vmss.h
+++ b/vmware_vmss.h
@@ -82,6 +82,14 @@ struct cptgroupdesc {
};
typedef struct cptgroupdesc cptgroupdesc;
+struct memregion {
+ uint32_t startpagenum;
+ uint32_t startppn;
+ uint32_t size;
+};
+typedef struct memregion memregion;
+
+#define MAX_REGIONS 3
struct vmssdata {
int32_t cpt64bit;
FILE *dfp;
@@ -89,6 +97,7 @@ struct vmssdata {
/* about the memory */
uint32_t alignmask;
uint32_t regionscount;
+ memregion regions[MAX_REGIONS];
uint64_t memoffset;
uint64_t memsize;
};
More information about the Crash-utility
mailing list