Re: [Crash-utility] [PATCH] read physical memory layout from device tree (32-bit ARM)

Sorry for patch mistakes... with multiple physical base addresses? CONFIG_PATCH_ARM_PHYS_VIRT, but it seems to depend Exactly!, i've found another __virt_to_phys() in our kernel, which works in same way as vtop_sparse() in my patch. -a" Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align NOTE 0x0000d4 0x00000000 0x00000000 0x00850 0x00850 0 LOAD 0x000924 0xc0000000 0x20000000 0xf800000 0xf800000 RWE 0 LOAD 0xf800924 0xe0000000 0x40000000 0x00000 0x00000 RWE 0 LOAD 0xf800924 0xe1000000 0x41000000 0x2b000000 0x2b000000 RWE 0 LOAD 0x3a800924 0x24800000 0x84800000 0x2b000000 0x2b000000 RWE 0 Yes it reflects memory regions of board, but not precisely(I don't know why). As you can see third PT_LOAD has PhysAddr more for 0x1000000 than needed, may be previous PT_LOAD with 0x40000000 and size 0 must be considered. come from? It is just binary file with information about hardware which used by kernel during boot. Also there is information about memory regions. "--machdep phys_base=<addr:size,addr:size,addr:size>" I think you are right, it will be more simple way than always carrying file with information of memory regions. So now I understood that issue is in another virt_to_phys() for my board, I also found another sparse virt_to_phys() in arch/arm/mach-realview/include/mach/memory.h and finally I think problem is that crash doesn't support "not flat" virt_to_phys() cases. Of course it is very rare case, but maybe it will be useful to handle such situations... ----- Original Message ----- Hello Arseniy, Just so the 32-bit ARM developers on this list can understand what this is all about, here is the description from the bugzilla you filed: Description of problem: crash failed reading 'memory_section' symbol with sparse mem. How reproducible: I have board with sparse memory. When i'm trying to load coredump of this board, i get the following message: crash: read error: kernel virtual address: dc98f400 type: "memory section" This happens because 'arm_kvtop' translates virtual to physical just using offset, while mentioned board has the following physical memory layout(from, size): 0x20000000, 0xf800000 0x40000000, 0x2c000000 0x84800000, 0x2b000000 So when crash substracts offset from 0xdc98f400 it gets 0x3c98f400, which is out of physical addresses of coredump program segments. I've prepared simple patch which parses device tree in order to read these memory regions and use them instead of offset substraction during virtual to physical address translation. So the problem is not with the CONFIG_SPARSEMEM per se, but rather with the the crash utility's VTOP() macro for 32-bit ARM, which is this: #define PTOV(X) \ ((unsigned long)(X)-(machdep->machspec->phys_base)+(machdep->kvbase)) #define VTOP(X) \ ((unsigned long)(X)-(machdep->kvbase)+(machdep->machspec->phys_base)) and where the machdep->machspec->phys_base value is determined during initialization like so: (1) on live systems, by parsing /proc/iomem (2) in ELF kdump dumpfiles, from parsing the PT_LOAD segments, and selects the lowest "PhysAddr" value. (3) in compressed kdump dumpfiles, from the dumpile header (4) or the user must pass the offset with "--machdep phys_base=<addr>" The crash utility's VTOP() is a direct reflection of how the 32-bit ARM kernel does it, at least if the kernel is configured with CONFIG_FLATMEM: static inline phys_addr_t __virt_to_phys(unsigned long x) { return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET; } Looking at the kernel sources, I can't understand exactly how it works with multiple physical base addresses? There is another __virt_to_phys() that is constrained to CONFIG_PATCH_ARM_PHYS_VIRT, but it seems to depend on !SPARSEMEM? Can you explain how the kernel does it? So anyway, the problem is that crash does not support 32-bit ARM kernels configured with CONFIG_SPARSEMEM which have *multiple* phys_base values. Your patch adds a "--dtbmem <file>" option which reads the physical memory layout from a "provided device tree". It parses that file, and creates an array of structures containing a physical-base-address/size pairs for each memory segment. I don't know what "provided device tree" file means. Where does the file come from? Regardless of that, I have to believe that this cannot be accomplished in a much simpler manner. Your dumpfile is an ELF vmcore. Can you show me the output of "readelf -a" on your vmcore? It should reflect the physical address ranges you showed above: I guessing that the PT_LOAD segments will precisely reflect the regions above. If that's true, why can't the information be gathered from the vmcore header as it does now? Worse case, it seems that you should be able to create something like a "--machdep phys_base=<addr:size,addr:size,addr:size>" option string that describes the multiple physical bases/sizes. In any case, I have some initial comments about the patch. First, can you please post your patches as attachments to your email? It seems that it has been line-wrapped somewhere along the line. I started fixing each complaint, but I give up: $ patch -p1 --dry-run < dtb.patch patching file Makefile patch: **** malformed patch at line 39: help.c task.c \ $ vi +39 $dl/dtc.patch < fix line 39 > $ patch -p1 --dry-run < dtb.patch patching file Makefile patch: **** malformed patch at line 43: \ $ vi +43 $dl/dtc.patch < fix line 43 > $ patch -p1 --dry-run < dtb.patch patching file Makefile Hunk #1 succeeded at 69 with fuzz 2 (offset -1 lines). patching file arm.c patch: **** malformed patch at line 77: PMD_TYPE_TABLE 1 #define PMD_TYPE_SECT_LPAE 1 $ Your patch replaces the two VTOP() calls in arm.c with a new vtop_sparse() function if this feature has been used: + if (dyn_mems == NULL) + *paddr = VTOP(kvaddr); + else + *paddr = vtop_sparse(kvaddr); But the patch does not handle the cases where VTOP() is used by generic crash code outside of arm.c. And it doesn't handle PTOV() at all. So what would have to be done is to change the ARM VTOP() and PTOV() macros in "defs.h" so that all callers will do the right thing. The Makefile has declared mem_dtb.c and mem_dtb.o, but then compiles it based upon "parse.o"?: + ramdump.c vmware_vmss.c mem_dtb.c + ramdump.o vmware_vmss.o mem_dtb.o +parse.o: ${GENERIC_HFILES} mem_dtb.c + ${CC} -c ${CRASH_CFLAGS} mem_dtb.c ${WARNING_OPTIONS} ${WARNING_ERROR} + I note this at the end of the new mem_dtb.c file: +#ifdef ARM ... [ cut ] ... +} +#else +int read_dtb_sparse_mem(const char *dtb_file_name) { + error(INFO, "Sparse mem supported only for arm!\n"); + return 0; +} +#endif -- Kind of misleading -- it's not a problem with sparsemem, but rather this device tree file business. But again, I appreciate the work you put into this, but please let's try to avoid this additional file requirement. Thanks, Dave unhandled\n", -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility End of Crash-utility Digest, Vol 118, Issue 3 *********************************************