On 11/17/06, Vivek Goyal <[EMAIL PROTECTED]> wrote: > On Thu, Nov 16, 2006 at 03:32:01PM +0900, Magnus Damm wrote: > > Hi everyone, > > > > I'm wondering if we are exporting more information than we actually need > > in our vmcore files. I think it would be nice to remove hardcoded kernel > > constants in the kexec-tools package if possible. > > > > Here is the program header output of readelf -a on a x86_64 vmcore from > > 2.6.19-rc5 (with saved_max_pfn fix): > > > > Program Headers: > > Type Offset VirtAddr PhysAddr > > FileSiz MemSiz Flags Align > > NOTE 0x0000000000000158 0x0000000000000000 0x0000000000000000 > > 0x00000000000002c8 0x00000000000002c8 0 > > LOAD 0x0000000000000420 0xffffffff80200000 0x0000000000200000 <- K > > 0x00000000002a2000 0x00000000002a2000 RWE 0 > > LOAD 0x00000000002a2420 0xffff810000000000 0x0000000000000000 <- R > > 0x00000000000a0000 0x00000000000a0000 RWE 0 > > LOAD 0x0000000000342420 0xffff810000100000 0x0000000000100000 <- R > > 0x0000000001f00000 0x0000000001f00000 RWE 0 > > LOAD 0x0000000002242420 0xffff810006000000 0x0000000006000000 <- R > > 0x00000000397f0000 0x00000000397f0000 RWE 0 > > > > The PT_LOAD program headers above are marked with K for kernel and R for > > physical RAM. The plan is that the kernel segment should be replaced > > with a hypervisor segment under Xen, but that is not what this email is > > about. > > > > To be able to use gdb on a vmcore file we need the PT_LOAD segment for > > the kernel - marked with K above. This is true for relocatable kernels > > and crash too. > > > > For gdb we need VirtAddr and maybe PhysAddr too. It should be possible > > for kexec-tools to determine VirtAddr from the running kernel > > using /proc/kcore or /proc/kallsyms. PhysAddr can be determined > > from /proc/iomem or maybe /proc/config.gz as fallback. > > > > Say we cannot determine one of VirtAddr, PhysAddr or MemSiz for the > > kernel segment. In that case I think we should skip creating a PT_LOAD > > program header for the kernel. That should only be the case on old > > kernels, and in that case the utility used to analyze the vmcore can > > fallback on getting these values from vmlinux. This means that gdb will > > fail but maybe that is ok on old kernels? > > I think it is not ok to break the compatibility with gdb for older kernels. > Why should we do that?
Well, I'm not saying that we really should, but I'm more asking that if we have a feature that no one is using and it involves a lot of hardcoded values then I think we should get rid of it. > Header generation is very arch specific details so rest of the discussion > mainly is in the context of x86_64 until and unless specified. > > We are determining kernel physical address from /proc/iomem, which is > always available both on old kernels and newer kernels. > > We are determining kernel virtual address from /proc/kcore. Now I am cooking > up and testing a patch which will allow you parse kcore even on older > kernels. So we will not break there too. Great, thank you. It should be possible to get the kernel size from /proc/iomem too if I'm not mistaken. > I think capability to open the dump using gdb is an important aspect. Few > days back itself people had shown interest in using gdb for kdump. So we > should not break this compatibility just to get rid of some hardcoding > in kexec-tools. > > We should get rid of hard-coding by replacing them with more generic > mechanism without breaking gdb. For example, determining the kernel virtual > address from /proc/kcore instead of hardcoding first 40MB is in kernel > test region is one step in that direction. I totally agree. I hope that the load of patches that I just sent makes it easier to unify the high level behaviour of the kexec-tool. > > > > The PT_LOAD program headers for RAM are easy when it comes to PhysAddr > > and MemSiz. For VirtAddr it seems more complicated today, PAGE_OFFSET > > and MAXMEM are both hardcoded in crashdump-x86.h for i386, and > > PAGE_OFFSET in crashdump-x86_64.h for x86_64. Do we really need VirtAddr > > for RAM program headers? > > > > Yes we do need. gdb only looks at virtual addresses and it does not parse > physical addresses as it does not have capability to traverse page tables. I didn't realize that gdb used the ram segments too - I thought that only the kernel segment was of interest for a gdb user. That's why I was interested in removing them... > > If we need VirtAddr for RAM - which utility is relying that then and > > does it have to? Can't we just set VirtAddr to -1 and be done with it? > > We don't have virtual address space for all physical RAM on i386 today > > anyway, and -1 is how we handle HIGHMEM on i386 already. Using a > > hardcoded PAGE_OFFSET on x86 does not work well with the VMSPLIT kernel > > config options, so I'd like to get rid of it all together if possible. > > > > For i386, we can't put virtual addresses for physical memory beyond 896MB > as we don't know at kdump load time where kernel has mapped that memory. > vmalloc regions are allocated as per need. Similarly high memory is mapped > by kernel as per needs. That's why gdb can do limited debugging in i386 > where it can not debug the modules. > > I agree that currently it works well for 3:1 split. If we want to make it > work on other configurations, then yes we need to find out a more generic > way instead of using hardcodings. (Say exporting required info through > some interfaces). But that does not mean that simply drop the hardcodings > and even break what works today. No, breaking things just for the fun of it may not be such a good idea. =) What about getting the PAGE_OFFSET by looking up symbols in /proc/kallsyms (or parsing /proc/kcore) and getting MAXMEM from lowmem information in /proc/meminfo? Or maybe we could parse /proc/config.gz... > > If we can agree on trying not to hardcode things then I believe that we > > can remove the following from crashdump-x86.h and crashdump-x86_64.h: > > > > PAGE_OFFSET, MAXMEM and __VMALLOC_RESERVE. Maybe __START_KERNEL_map and > > KERNEL_TEXT_SIZE. For x86_64 __pa() can be removed if I'm right, on i386 > > we still need it unless we can come up with a better way to implement > > get_crash_notes(). > > > > How does that sound? I think that hardcoding as little as possible > > should lead to a more stable and maintainable piece of software. > > > > Again, getting rid of hardcodings is good but not at the cost of existing > working functionality. So please, don drop the hardcodings without providing > an alternate more generic way to determine the required info. Yeah. I agree. Just wanted to check if we had unused code in the tree. Many thanks for your detailed answer! / magnus _______________________________________________ fastboot mailing list fastboot@lists.osdl.org https://lists.osdl.org/mailman/listinfo/fastboot
