----- Original Message ----- > On Tue, May 24, 2016 at 01:59:06PM -0400, Dave Anderson wrote: > > > > ----- Original Message ----- > > > Yet some issues, but ... > > > > > > > Hi Takahiro, > > > > Here are my general comments on my testing of the v2 patch, followed > > by a few comments in the patch itself. > > Thank you for your quick review/testing, again. > > > First, the combination of the new memory map layout and KASLR is somewhat > > confusing. I am testing your patch on a 4.6.0-0.rc7.git2.1.fc25 kernel > > that has this configuration: > > Right. I use the "kaslr kernel" or "kaslr-enabled kernel" in two > meanings (almost interchangeably): > - kernel with a new memory map, that is v4.6 or later > - kernel that has ability of KASLR (configured with CONFIG_RANDOMIZE_RAM) > > When we talked offline, you mentioned the possibility of backporting > KASLR to older kernel, so I hesitated to use "v4.6 or later". > But I think that we'd better use "v4.6 or later" for a clarification > for now. Actually I was thinking more along the lines of the new vmemmap stuff being backported. But it looks like that would be impossible without also bringing along "kimage_voffset". > > config-arm64:# CONFIG_RANDOMIZE_BASE is not set > > > > So KASLR doesn't really enter into the picture. But when bringing > > up the crash session, it shows the "kaslr kernel" WARNING: > > > > # ./crash > > > > crash 7.1.5++ > > Copyright (C) 2002-2016 Red Hat, Inc. > > Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation > > Copyright (C) 1999-2006 Hewlett-Packard Co > > Copyright (C) 2005, 2006, 2011, 2012 Fujitsu Limited > > Copyright (C) 2006, 2007 VA Linux Systems Japan K.K. > > Copyright (C) 2005, 2011 NEC Corporation > > Copyright (C) 1999, 2002, 2007 Silicon Graphics, Inc. > > Copyright (C) 1999, 2000, 2001, 2002 Mission Critical Linux, Inc. > > This program is free software, covered by the GNU General Public License, > > and you are welcome to change it and/or distribute copies of it under > > certain conditions. Enter "help copying" to see the conditions. > > This program has absolutely no warranty. Enter "help warranty" for > > details. > > > > WARNING: kimage_voffset not identified for kaslr kernel > > GNU gdb (GDB) 7.6 > > Copyright (C) 2013 Free Software Foundation, Inc. > > License GPLv3+: GNU GPL version 3 or later > > <http://gnu.org/licenses/gpl.html> > > This is free software: you are free to change and redistribute it. > > There is NO WARRANTY, to the extent permitted by law. Type "show > > copying" > > and "show warranty" for details. > > This GDB was configured as "aarch64-unknown-linux-gnu"... > > > > KERNEL: > > /usr/lib/debug/lib/modules/4.6.0-0.rc7.git2.1.fc25.aarch64/vmlinux > > DUMPFILE: /dev/crash > > CPUS: 8 > > DATE: Tue May 24 10:08:08 2016 > > UPTIME: 11 days, 18:32:41 > > LOAD AVERAGE: 0.17, 0.09, 0.12 > > TASKS: 197 > > NODENAME: apm-mustang-ev3-36.khw.lab.eng.bos.redhat.com > > RELEASE: 4.6.0-0.rc7.git2.1.fc25.aarch64 > > VERSION: #1 SMP Thu May 12 13:28:43 UTC 2016 > > MACHINE: aarch64 (unknown Mhz) > > MEMORY: 16 GB > > PID: 7556 > > COMMAND: "crash" > > TASK: fffffe00beb45400 [THREAD_INFO: fffffe00beb98000] > > CPU: 7 > > STATE: TASK_RUNNING (ACTIVE) > > > > crash> > > > > Why show that WARNING in this case? I understant that it's not > > stashed during early initialization: > > If we don't know kimage_voffset, we can't analyze the contents of > memory (live or kdump). I understand. But clearly that's not the case with my system above that does not have CONFIG_RANDOMIZE_BASE configured, so the warning message is nonsensical/confusing. That's why I was speculating about perhaps having kdump export a VMCOREINFO_CONFIG(RANDOMIZE_BASE) so that it is readily apparent. > > > crash> help -m > > flags: 104000c5 > > (KSYMS_START|VM_L2_64K|VMEMMAP|IRQ_STACKS|MACHDEP_BT_TEXT|NEW_VMEMMAP) > > ... [ cut ] ... > > memory map layout: new <--- BTW, this is > > redundant/not-a-member, and we've got NEW_VMEMMAP > > VA_BITS: 42 > > userspace_top: 0000040000000000 > > page_offset: fffffe0000000000 > > vmalloc_start_addr: fffffc0008000000 > > vmalloc_end: fffffdff5ffeffff > > modules_vaddr: fffffc0000000000 > > modules_end: fffffc0007ffffff > > vmemmap_vaddr: fffffdff80000000 > > vmemmap_end: fffffdffffffffff > > kimage_text: fffffc0008080000 > > kimage_end: fffffc0009070000 > > kimage_voffset: 0000000000000000 <-- available if kernel is > > not randomized > > phys_offset: 4000000000 > > ... > > > > But it can be read: > > > > crash> px kimage_voffset > > kimage_voffset = $1 = 0xfffffbc008000000 > > crash> > > > > SO because it wasn't determined during session initialization, > > it falls into the "no randomness" section of arm64_VTOP(): > > Exactly. > > > ulong > > arm64_VTOP(ulong addr) > > { > > if (!(machdep->flags & NEW_VMEMMAP) || > > (addr >= machdep->machspec->page_offset)) { > > return machdep->machspec->phys_offset > > + (addr - machdep->machspec->page_offset); > > } else { > > if (machdep->machspec->kimage_voffset) > > return addr - machdep->machspec->kimage_voffset; > > else /* no randomness */ > > return machdep->machspec->phys_offset > > + (addr - > > machdep->machspec->vmalloc_start_addr); > > } > > } > > > > That works, but if "kimage_offset" can be read normally later on, perhaps it > > should update "machdep->machspec->kimage_voffset" at that time? > > It is possible, but the check in arm64_VTOP() would be still necessary. Agreed... > > > There are some discrepancies with respect to the calculated addresses > > and what is seen by looking at the reported memory map in the kernel log: > > > > [ 0.000000] Virtual kernel memory layout: > > [ 0.000000] modules : 0xfffffc0000000000 - 0xfffffc0008000000 ( > > 128 MB) > > [ 0.000000] vmalloc : 0xfffffc0008000000 - 0xfffffdfedfff0000 ( > > 2043 GB) > > [ 0.000000] .text : 0xfffffc0008080000 - 0xfffffc0008890000 ( > > 8256 KB) > > .rodata : 0xfffffc0008890000 - 0xfffffc0008c10000 ( > > 3584 KB) > > .init : 0xfffffc0008c10000 - 0xfffffc0008d50000 ( > > 1280 KB) > > .data : 0xfffffc0008d50000 - 0xfffffc0008eaac00 ( > > 1387 KB) > > [ 0.000000] vmemmap : 0xfffffdfee0000000 - 0xfffffdffe0000000 ( > > 4 GB maximum) > > 0xfffffdfee0000000 - 0xfffffdfee1000000 ( > > 16 MB actual) > > [ 0.000000] fixed : 0xfffffdfffe7d0000 - 0xfffffdfffec00000 ( > > 4288 KB) > > [ 0.000000] PCI I/O : 0xfffffdfffee00000 - 0xfffffdffffe00000 ( > > 16 MB) > > [ 0.000000] memory : 0xfffffe0000000000 - 0xfffffe0400000000 ( > > 16384 MB) > > > > Comparing it to the calculated values: > > > > VA_BITS: 42 > > userspace_top: 0000040000000000 > > page_offset: fffffe0000000000 OK > > vmalloc_start_addr: fffffc0008000000 OK > > vmalloc_end: fffffdff5ffeffff ? (seems wrong) > > modules_vaddr: fffffc0000000000 OK > > modules_end: fffffc0007ffffff OK (-1) > > vmemmap_vaddr: fffffdff80000000 ? (seems wrong) > > vmemmap_end: fffffdffffffffff ? (seems wrong) > > kimage_text: fffffc0008080000 OK > > kimage_end: fffffc0009070000 OK > > kimage_voffset: 0000000000000000 > > phys_offset: 4000000000 > > > > Starting with vmalloc_start_addr/vmalloc_end, if I run "kmem -v" to dump > > the > > vmalloc allocations, there are some allocations that are below the > > 0xfffffc0008000000 start value shown in the log and by your calcuation. > > I'm not clear on what that means?: > > > > crash> kmem -v > > VMAP_AREA VM_STRUCT ADDRESS RANGE > > SIZE > > fffffe03daefd900 fffffe03daefd880 fffffc0000c10000 - fffffc0000c30000 > > 131072 > > fffffe00bf2c3a00 fffffe00bf2c3980 fffffc0000c90000 - fffffc0000cb0000 > > 131072 > > fffffe00bf2c7400 fffffe00bf2c7380 fffffc0000d50000 - fffffc0000d70000 > > 131072 > > fffffe03dc76aa00 fffffe03dc76a980 fffffc0000d90000 - fffffc0000db0000 > > 131072 > > fffffe03dc76be00 fffffe03dc76bd80 fffffc0000dd0000 - fffffc0000ee0000 > > 1114112 > > fffffe03d90e7900 fffffe03d90e7b80 fffffc0000f40000 - fffffc0000fb0000 > > 458752 > > fffffe00bec12f00 fffffe00bec12b00 fffffc0000fe0000 - fffffc0001000000 > > 131072 > > fffffe00bf3a1100 fffffe00bf3a1080 fffffc0001020000 - fffffc0001050000 > > 196608 > > fffffe00bf3a3280 fffffe00bf3a3200 fffffc0001070000 - fffffc0001090000 > > 131072 > > fffffe03ddf7fc00 fffffe03ddf7a880 fffffc0001230000 - fffffc0001250000 > > 131072 > > fffffe03fd508900 fffffe03fd508880 fffffc0008000000 - fffffc0008020000 > > 131072 > > fffffe03fd509580 fffffe03fd508d00 fffffc0008020000 - fffffc0008040000 > > 131072 > > fffffe03fd508a00 fffffe03fd508980 fffffc0008040000 - fffffc0008070000 > > 196608 > > ... [ cut ] ... > > fffffe03d7235000 fffffe03d7233480 fffffc0014b30000 - fffffc0014b50000 > > 131072 > > fffffe00b921e700 fffffe00b921df00 fffffc0014b70000 - fffffc0014b90000 > > 131072 > > fffffe00b851eb00 fffffe00b8512900 fffffdfedfcf0000 - fffffdfedfe70000 > > 1572864 > > fffffe03dca4ef00 fffffe03dca4ef80 fffffdfedfe70000 - fffffdfedfff0000 > > 1572864 > > crash> > > > > Although the end value of fffffdfedfff0000 does match up with the value in > > the > > kernel log, your calculation is 2GB higher at fffffdff5ffeffff: > > > > crash> eval fffffdff5ffeffff - 0xfffffdfedfff0000 > > hexadecimal: 7fffffff > > decimal: 2147483647 > > octal: 17777777777 > > binary: > > 0000000000000000000000000000000001111111111111111111111111111111 > > crash> eval 7fffffff + 1 > > hexadecimal: 80000000 (2GB) > > decimal: 2147483648 > > octal: 20000000000 > > binary: > > 0000000000000000000000000000000010000000000000000000000000000000 > > crash> > > > > With respect to the vmemmap range, vmmemap address in the log file is the > > one > > that is displayed by "kmem -p": > > > > crash> kmem -p > > PAGE PHYSICAL MAPPING INDEX CNT FLAGS > > fffffdfee0000000 4000000000 fffffe03fd441950 2f 1 1002c > > referenced,uptodate,lru,mappedtodisk > > fffffdfee0000040 4000010000 fffffe03fd441950 30 1 1002c > > referenced,uptodate,lru,mappedtodisk > > fffffdfee0000080 4000020000 fffffe03fd441950 31 1 1002c > > referenced,uptodate,lru,mappedtodisk > > fffffdfee00000c0 4000030000 fffffe03fd441950 32 1 1002c > > referenced,uptodate,lru,mappedtodisk > > ... > > fffffdfee0ffff00 43fffc0000 0 0 1 > > 4000000000000400 reserved > > fffffdfee0ffff40 43fffd0000 0 0 1 > > 4000000000000400 reserved > > fffffdfee0ffff80 43fffe0000 0 0 1 > > 4000000000000400 reserved > > fffffdfee0ffffc0 43ffff0000 0 0 1 > > 4000000000000400 reserved > > crash> > > > > And "kmem" alone recognizes it: > > > > crash> kmem fffffdfee0000000 > > PAGE PHYSICAL MAPPING INDEX CNT FLAGS > > fffffdfee0000000 4000000000 fffffe03fd441950 2f 1 1002c > > referenced,uptodate,lru,mappedtodisk > > crash> > > > > But your calculated vmemmap_vaddr above doesn't seem right: > > > > crash> kmem fffffdff80000000 > > kmem: WARNING: cannot make virtual-to-physical translation: > > fffffdff80000000 > > fffffdff80000000: kernel virtual address not found in mem map > > crash> > > > > Do your numbers match up on a dump that actually has CONFIG_RANDOMIZE_BASE? > > In my environment, the numbers above match up on a dump whether > CONFIG_RANDOMIZE_BASE is enabled or not. > > Are you using a mainline kernel (final v4.6, not -rcX)? Good point. When I configured my arm64 test system, I installed the latest Fedora kernel available at the time (4.6.0-0.rc7.git2.1.fc25), but it is based upon linux-4.5.tar.xz. I see now that there is a kernel-4.6.0-1.fc25 available that is based upon linux-4.6.tar.xz. I'll update the system. > > Anyway, onto the patch... > > > > > > > > changes in v2: > > > * Fixed build warnings > > > * Moved ARM64_NEW_VMEMMAP to machdep->flags > > > * Show additional kaslr-related parameters in arm64_dump_machdep_table() > > > * Handle a VMCOREINFO, "NUMBER(kimage_voffset)" > > > > > > ===8<=== > > > >From 080a54ec232ac48ef2d8123cbedcf0f1fe27e391 Mon Sep 17 00:00:00 2001 > > > From: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx> > > > Date: Mon, 16 May 2016 17:31:55 +0900 > > > Subject: [PATCH v2] arm64: fix kernel memory map handling for > > > kaslr-enabled > > > kernel > > > > > > In kernel v4.6, Kernel ASLR (KASLR) is supported on arm64, and the start > > > address of the kernel image can be randomized if CONFIG_RANDOMIZE_BASE is > > > enabled. > > > Even worse, the kernel image is no more mapped in the linear mapping, but > > > in vmalloc area (i.e. below PAGE_OFFSET). > > > > > > Now, according to the kernel's memory.h, converting a virtual address to > > > a physical address should be done like below: > > > > > > phys_addr_t __x = (phys_addr_t)(x); \ > > > __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \ > > > (__x - kimage_voffset); }) > > > > > > Please note that PHYS_OFFSET is no more equal to the start address of > > > the first usable memory block in SYSTEM RAM due to the fact mentioned > > > above. > > > > > > This patch addresses this change and allows the crash utility to access > > > memory contents with correct addresses. > > > *However*, it is still rough-edged and we need more refinements. > > > > > > 1-1) On a live system, crash with this patch won't work because > > > we currently have no way to know kimage_voffset. > > > > > > 1-2) For a core dump file, we can do simply: > > > $ crash <vmlinux> <vmcore> > > > as long as the file has "NUMBER(kimage_voffset)" > > > (RELOC_AUTO|KASLR is automatically set.) > > > > > > I'm planning to add this enhancement in my next version of kexec/kdump > > > patch, i.e. v17. > > > > > > 2) My current change breaks the backward compatibility. > > > Crash won't work with v4.5 or early kernel partly because I changed > > > the calculation of VA_BITS. (I think that the existing code is wrong.) > > > So far I don't have enough time to look into this issue as I'm > > > focusing > > > on KASLR support. > > > > OK, but you still haven't shown how the current VA_BITS determination is > > not a correct representation of CONFIG_ARM64_VA_BITS. > > At least, the current VA_BITS doesn't work with my calculations, which > were derived directly from v4.6's "asm/memory.h". What are your kernel's PAGE_OFFSET value and CONFIG_ARM64_VA_BITS value? > > > > 3) Backtracing a 'panic'ed task fails: > > > crash> bt > > > PID: 999 TASK: ffffffe960081800 CPU: 0 COMMAND: "sh" > > > bt: WARNING: corrupt prstatus? pstate=0x80000000, but no user frame > > > found > > > bt: WARNING: cannot determine starting stack frame for task > > > ffffffe960081800 > > > > > > frame->pc indicates that it is a kernel address, so obviously something > > > is wrong. I'm diggin into it. > > > > Can you add a debug statement that displays frame->pc, frame->sp, and > > frame->fp? > > I've already identified the cause. > pt_regs->pstate was set mistakenly from "sprsr_el1" in panic(). > But I have a difficulty here to fix the problem as we have no direct way > to retrieve a value of the *current* PSTATE. Interesting. I don't know what you mean by "sprsr_el1" in panic(), but I just got a report from my github "issues", where the user injected a panic() call into a kernel module and got the same error as above because of the pstate value. I asked him to put this hack into arm64.c: --- a/arm64.c +++ b/arm64.c @@ -1711,11 +1711,14 @@ arm64_get_dumpfile_stackframe(struct bt_info *bt, struct arm64_stackframe *frame error(WARNING, "corrupt prstatus? pstate=0x%lx, but no user frame found\n", ptregs->pstate); + if (INSTACK(frame->sp, bt) && INSTACK(frame->fp, bt)) + goto try_kernel; bt->flags |= BT_REGS_NOT_FOUND; return FALSE; } bt->flags |= BT_USER_SPACE; } else { +try_kernel: frame->sp = ptregs->sp; frame->fp = ptregs->regs[29]; } Can you give that a shot? > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx> > > > --- > > > arm64.c | 217 > > > ++++++++++++++++++++++++++++++++++++++++++++++++-------------- > > > defs.h | 24 +++++-- > > > main.c | 7 +- > > > symbols.c | 10 +-- > > > 4 files changed, 196 insertions(+), 62 deletions(-) > > > > > > diff --git a/arm64.c b/arm64.c > > > index 34c8c59..6b97093 100644 > > > --- a/arm64.c > > > +++ b/arm64.c > > > @@ -72,6 +72,21 @@ static int arm64_get_crash_notes(void); > > > static void arm64_calc_VA_BITS(void); > > > static int arm64_is_uvaddr(ulong, struct task_context *); > > > > > > +ulong > > > +arm64_VTOP(ulong addr) > > > +{ > > > + if (!(machdep->flags & NEW_VMEMMAP) || > > > + (addr >= machdep->machspec->page_offset)) { > > > + return machdep->machspec->phys_offset > > > + + (addr - machdep->machspec->page_offset); > > > + } else { > > > + if (machdep->machspec->kimage_voffset) > > > + return addr - machdep->machspec->kimage_voffset; > > > + else /* no randomness */ > > > + return machdep->machspec->phys_offset > > > + + (addr - machdep->machspec->vmalloc_start_addr); > > > + } > > > +} > > > > > > /* > > > * Do all necessary machine-specific setup here. This is called several > > > times > > > @@ -81,6 +96,7 @@ void > > > arm64_init(int when) > > > { > > > ulong value; > > > + char *string; > > > struct machine_specific *ms; > > > > > > #if defined(__x86_64__) > > > @@ -102,9 +118,34 @@ arm64_init(int when) > > > if (machdep->cmdline_args[0]) > > > arm64_parse_cmdline_args(); > > > machdep->flags |= MACHDEP_BT_TEXT; > > > + > > > + ms = machdep->machspec; > > > + if (!ms->kimage_voffset && > > > + (string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) { > > > + ms->kimage_voffset = htol(string, QUIET, NULL); > > > + free(string); > > > + } > > > > It doesn't make sense to call pc->read_vmcoreinfo() if !DUMPFILE(). > > Do you think so? > By default, pc->read_vmcoreinfo is set to no_vmcoreinfo. > So it doesn't hurt anything. Right, just a suggestion... > > > + > > > + if (ms->kimage_voffset) { > > > + machdep->flags |= NEW_VMEMMAP; > > > + > > > + /* if not specified in command line */ > > > + if (!kt->relocate && !(kt->flags2 & (RELOC_AUTO|KASLR))) > > > + kt->flags2 |= (RELOC_AUTO|KASLR); > > > + } > > > + > > > > If CONFIG_RANDOMIZE_BASE is NOT set, should RELOC_AUTO|KASLR be set? They > > don't get set on my live system. > > Not necessarily, but the current implementation of derive_kaslr_offset() > just works even in this case. Ahah -- cool! > (This is the reason that I don't think we need a VMCOREINFO for > CONFIG_RANDOMIZE_BASE.) Well, as long as there's a way to avoid that unnecessary/confusing warning message for non-KASLR new-vmemmap kernels. I also wonder whether makedumpfile could use it? > > > break; > > > > > > case PRE_GDB: > > > + /* This check is somewhat redundant */ > > > + if (kernel_symbol_exists("kimage_voffset")) { > > > + machdep->flags |= NEW_VMEMMAP; > > > + > > > + if (!machdep->machspec->kimage_voffset) > > > + error(WARNING, > > > + "kimage_voffset not identified for kaslr kernel\n"); > > > + } > > > + > > > > As mentioned in the general discussion above, the WARNING above makes > > no sense if CONFIG_RANDOMIZE_BASE is not configured. > > What about if I change the message "for v4.6 or later kernel"? But again, I do not want the message at all if it's not applicable. Worse case, you could set a flag, and then if/when the crash session ultimately craps out if it can't figure things out, then the message could be displayed post-mortem. > > I'm now thinking that CONFIG_RANDOMIZE_BASE (and maybe others like KASAN?) > > should be passed in the dumpfile with VMCOREINFO_CONFIG(). > > Regarding KASAN, we only need to check for an existence of "kasan_init" > symbol as you recommended before :) Nice. > > > > > if (!machdep->pagesize) { > > > /* > > > * Kerneldoc Documentation/arm64/booting.txt describes > > > @@ -160,16 +201,35 @@ arm64_init(int when) > > > machdep->pagemask = ~((ulonglong)machdep->pageoffset); > > > > > > arm64_calc_VA_BITS(); > > > - machdep->machspec->page_offset = ARM64_PAGE_OFFSET; > > > + ms = machdep->machspec; > > > + ms->page_offset = ARM64_PAGE_OFFSET; > > > + /* FIXME: idmap for NEW_VMEMMAP */ > > > machdep->identity_map_base = ARM64_PAGE_OFFSET; > > > - machdep->machspec->userspace_top = ARM64_USERSPACE_TOP; > > > - machdep->machspec->modules_vaddr = ARM64_MODULES_VADDR; > > > - machdep->machspec->modules_end = ARM64_MODULES_END; > > > - machdep->machspec->vmalloc_start_addr = ARM64_VMALLOC_START; > > > - machdep->machspec->vmalloc_end = ARM64_VMALLOC_END; > > > - machdep->kvbase = ARM64_VMALLOC_START; > > > - machdep->machspec->vmemmap_vaddr = ARM64_VMEMMAP_VADDR; > > > - machdep->machspec->vmemmap_end = ARM64_VMEMMAP_END; > > > + machdep->kvbase = ARM64_VA_START; > > > + ms->userspace_top = ARM64_USERSPACE_TOP; > > > + if (machdep->flags & NEW_VMEMMAP) { > > > + struct syment *sp; > > > + > > > + sp = kernel_symbol_search("_text"); > > > + ms->kimage_text = (sp ? sp->value : 0); > > > + sp = kernel_symbol_search("_end"); > > > + ms->kimage_end = (sp ? sp->value : 0); > > > + > > > + ms->modules_vaddr = ARM64_VA_START; > > > + if (kernel_symbol_exists("kasan_init")) > > > + ms->modules_vaddr += ARM64_KASAN_SHADOW_SIZE; > > > + ms->modules_end = ms->modules_vaddr > > > + + ARM64_MODULES_VSIZE -1; > > > + > > > + ms->vmalloc_start_addr = ms->modules_end + 1; > > > + } else { > > > + ms->modules_vaddr = ARM64_PAGE_OFFSET - MEGABYTES(64); > > > + ms->modules_end = ARM64_PAGE_OFFSET - 1; > > > + ms->vmalloc_start_addr = ARM64_VA_START; > > > + } > > > + ms->vmalloc_end = ARM64_VMALLOC_END; > > > + ms->vmemmap_vaddr = ARM64_VMEMMAP_VADDR; > > > + ms->vmemmap_end = ARM64_VMEMMAP_END; > > > > > > switch (machdep->pagesize) > > > { > > > @@ -232,8 +292,6 @@ arm64_init(int when) > > > machdep->stacksize = ARM64_STACK_SIZE; > > > machdep->flags |= VMEMMAP; > > > > > > - arm64_calc_phys_offset(); > > > - > > > machdep->uvtop = arm64_uvtop; > > > machdep->kvtop = arm64_kvtop; > > > machdep->is_kvaddr = generic_is_kvaddr; > > > @@ -262,6 +320,10 @@ arm64_init(int when) > > > machdep->dumpfile_init = NULL; > > > machdep->verify_line_number = NULL; > > > machdep->init_kernel_pgd = arm64_init_kernel_pgd; > > > + > > > + /* use machdep parameters */ > > > + arm64_calc_phys_offset(); > > > + > > > > This doesn't make any sense to me. I understand you've done it because > > you are now doing a readmem() in arm64_calc_phys_offset(). But readmem() > > calls are never done this early. And in fact, on the live system the > > readmem() in arm64_calc_phys_offset() fails like so (running "crash -d4"): > > > > <readmem: fffffc0008d70bb0, KVADDR, "memstart_addr", 8, (ROE|Q), > > 3ffcc2842c0> > > <read_memory_device: addr: fffffc0008d70bb0 paddr: d70bb0 cnt: 8> > > crash: read error: kernel virtual address: fffffc0008d70bb0 type: > > "memstart_addr" > > > > It fails above because it needs the phys_base in order to do the readmem(). > > During > > runtime it has it: > > This is because we don't know kimage_voffset on a live system. > (and so I mentioned this issue in my commit log.) Right -- which is a good reason for NOT using readmem() at that point in time. I can also envision a scenario where readmem() may inadvertently "work" (i.e. depending upon where phys_base really is), and then return something bogus from an unintended location. > As far as kdump case is concerned, readmem() at this point just works > for any address in kernel image. This is my intentional use of this function. I understand, but you're subverting the subsequent dumpfile-specific calls later in the function. It is precisely *their* responsibility to return the value of phys_offset. > > crash> set debug 4 > > debug: 4 > > crash> rd memstart_addr > > <addr: fffffc0008d70bb0 count: 1 flag: 490 (KVADDR)> > > <readmem: fffffc0008d70bb0, KVADDR, "64-bit KVADDR", 8, (FOE), > > 3ffcbfbfc28> > > <read_memory_device: addr: fffffc0008d70bb0 paddr: 4000d70bb0 cnt: 8> > > fffffc0008d70bb0: 0000004000000000 ....@... > > crash> > > > > So without kimage_voffset, that readmem(), how would that readmem() > > possibly work? > > You need the *contents* of memstart_addr in order to *read* memstart_addr. > > > > Now, if it kimage_voffset were passed into the vmcore, then I guess you > > could > > read it. But why do it that way? That's what diskdump_get_phys_base() and > > kdump_get_phys_base() are supposed to do. > > Because I believe that this is the *common* place to do it either for > diskdump, or kdump ( or even a live system if we know kimage_voffset.) Well, as I have shown, it won't work on my live system because of the chicken-and-egg scenario. Let's worry about live system analysis after we get kdump straightened out. > > > > > break; > > > > > > case POST_GDB: > > > @@ -409,6 +471,8 @@ arm64_dump_machdep_table(ulong arg) > > > fprintf(fp, "%sIRQ_STACKS", others++ ? "|" : ""); > > > if (machdep->flags & MACHDEP_BT_TEXT) > > > fprintf(fp, "%sMACHDEP_BT_TEXT", others++ ? "|" : ""); > > > + if (machdep->flags & NEW_VMEMMAP) > > > + fprintf(fp, "%sNEW_VMEMMAP", others++ ? "|" : ""); > > > fprintf(fp, ")\n"); > > > > > > fprintf(fp, " kvbase: %lx\n", machdep->kvbase); > > > @@ -494,6 +558,8 @@ arm64_dump_machdep_table(ulong arg) > > > ms = machdep->machspec; > > > > > > fprintf(fp, " machspec: %lx\n", (ulong)ms); > > > + fprintf(fp, " memory map layout: %s\n", > > > + (machdep->flags & NEW_VMEMMAP) ? "new" : "old"); > > > > As before, these internal "help" structure dumps just show existing > > structure fields, > > and NEW_VMMEMAP is already shown in the flags. > > I put it as useful information for those people who might have got > any concerns about those odd numbers/kernel layout. > But if you don't like it, I will remove it. > > > > > > fprintf(fp, " VA_BITS: %ld\n", ms->VA_BITS); > > > fprintf(fp, " userspace_top: %016lx\n", ms->userspace_top); > > > fprintf(fp, " page_offset: %016lx\n", ms->page_offset); > > > @@ -503,6 +569,11 @@ arm64_dump_machdep_table(ulong arg) > > > fprintf(fp, " modules_end: %016lx\n", ms->modules_end); > > > fprintf(fp, " vmemmap_vaddr: %016lx\n", ms->vmemmap_vaddr); > > > fprintf(fp, " vmemmap_end: %016lx\n", ms->vmemmap_end); > > > + if (machdep->flags & NEW_VMEMMAP) { > > > + fprintf(fp, " kimage_text: %016lx\n", ms->kimage_text); > > > + fprintf(fp, " kimage_end: %016lx\n", ms->kimage_end); > > > + fprintf(fp, " kimage_voffset: %016lx\n", ms->kimage_voffset); > > > + } > > > fprintf(fp, " phys_offset: %lx\n", ms->phys_offset); > > > fprintf(fp, "__exception_text_start: %lx\n", > > > ms->__exception_text_start); > > > fprintf(fp, " __exception_text_end: %lx\n", ms->__exception_text_end); > > > @@ -543,6 +614,42 @@ arm64_dump_machdep_table(ulong arg) > > > } > > > } > > > > > > +static int > > > +arm64_parse_machdep_arg_l(char *argstring, char *param, ulong *value) > > > +{ > > > + int len; > > > + int megabytes = FALSE; > > > + char *p; > > > + > > > + len = strlen(param); > > > + if (!STRNEQ(argstring, param) || (argstring[len] != '=')) > > > + return FALSE; > > > + > > > + if ((LASTCHAR(argstring) == 'm') || > > > + (LASTCHAR(argstring) == 'M')) { > > > + LASTCHAR(argstring) = NULLCHAR; > > > + megabytes = TRUE; > > > + } > > > + > > > + p = argstring + len + 1; > > > + if (strlen(p)) { > > > + int flags = RETURN_ON_ERROR | QUIET; > > > + int err = 0; > > > + > > > + if (megabytes) { > > > + *value = dtol(p, flags, &err); > > > + if (!err) > > > + *value = MEGABYTES(*value); > > > + } else { > > > + *value = htol(p, flags, &err); > > > + } > > > + > > > + if (!err) > > > + return TRUE; > > > + } > > > + > > > + return FALSE; > > > +} > > > > > > /* > > > * Parse machine dependent command line arguments. > > > @@ -554,11 +661,10 @@ arm64_dump_machdep_table(ulong arg) > > > static void > > > arm64_parse_cmdline_args(void) > > > { > > > - int index, i, c, err; > > > + int index, i, c; > > > char *arglist[MAXARGS]; > > > char buf[BUFSIZE]; > > > char *p; > > > - ulong value = 0; > > > > > > for (index = 0; index < MAX_MACHDEP_ARGS; index++) { > > > if (!machdep->cmdline_args[index]) > > > @@ -580,39 +686,23 @@ arm64_parse_cmdline_args(void) > > > c = parse_line(buf, arglist); > > > > > > for (i = 0; i < c; i++) { > > > - err = 0; > > > - > > > - if (STRNEQ(arglist[i], "phys_offset=")) { > > > - int megabytes = FALSE; > > > - int flags = RETURN_ON_ERROR | QUIET; > > > - > > > - if ((LASTCHAR(arglist[i]) == 'm') || > > > - (LASTCHAR(arglist[i]) == 'M')) { > > > - LASTCHAR(arglist[i]) = NULLCHAR; > > > - megabytes = TRUE; > > > - } > > > - > > > - p = arglist[i] + strlen("phys_offset="); > > > - if (strlen(p)) { > > > - if (megabytes) > > > - value = dtol(p, flags, &err); > > > - else > > > - value = htol(p, flags, &err); > > > - } > > > - > > > - if (!err) { > > > - if (megabytes) > > > - value = MEGABYTES(value); > > > - > > > - machdep->machspec->phys_offset = value; > > > - > > > - error(NOTE, > > > - "setting phys_offset to: 0x%lx\n\n", > > > - machdep->machspec->phys_offset); > > > + if (arm64_parse_machdep_arg_l(arglist[i], > > > + "phys_offset", > > > + &machdep->machspec->phys_offset)) { > > > + error(NOTE, > > > + "setting phys_offset to: 0x%lx\n\n", > > > + machdep->machspec->phys_offset); > > > + > > > + machdep->flags |= PHYS_OFFSET; > > > + continue; > > > + } else if (arm64_parse_machdep_arg_l(arglist[i], > > > + "kimage_voffset", > > > + &machdep->machspec->kimage_voffset)) { > > > + error(NOTE, > > > + "setting kimage_voffset to: 0x%lx\n\n", > > > + machdep->machspec->kimage_voffset); > > > > > > - machdep->flags |= PHYS_OFFSET; > > > - continue; > > > - } > > > + continue; > > > } > > > > > > error(WARNING, "ignoring --machdep option: %s\n", > > > @@ -627,10 +717,20 @@ arm64_calc_phys_offset(void) > > > { > > > struct machine_specific *ms = machdep->machspec; > > > ulong phys_offset; > > > + struct syment *sp; > > > + ulong value; > > > > > > if (machdep->flags & PHYS_OFFSET) /* --machdep override */ > > > return; > > > > > > + sp = kernel_symbol_search("memstart_addr"); > > > + if (sp && readmem(sp->value, KVADDR, (char *)&value, sizeof(value), > > > + "memstart_addr", QUIET|RETURN_ON_ERROR)) { > > > + ms->phys_offset = value; > > > + machdep->flags |= PHYS_OFFSET; > > > + return; > > > + } > > > + > > > > Also, I may be missing something, but the PHYS_OFFSET flag means that it > > was > > passed in via --machdep, and is only used in this function to avoid any > > further > > attempts to figure out what it is. Here you are setting the flag for no > > particular > > reason that I can see. Why? > > Well, I don't remember very well. If not necessary, I will remove it. OK, I guarantee it's useless in this case... > > But again, doing a readmem() call this early is setting a bad precedent -- > > they have > > *always* been delayed until after gdb_session_init() in main_loop(). And > > the work > > of determining phys_base should be done by the dumpfile call-out functions > > that > > already exist. > > > > > > > /* > > > * Next determine suitable value for phys_offset. User can override > > > this > > > * by passing valid '--machdep phys_offset=<addr>' option. > > > @@ -2377,6 +2477,11 @@ arm64_IS_VMALLOC_ADDR(ulong vaddr) > > > { > > > struct machine_specific *ms = machdep->machspec; > > > > > > + if ((machdep->flags & NEW_VMEMMAP) && > > > + (vaddr >= machdep->machspec->kimage_text) && > > > + (vaddr <= machdep->machspec->kimage_end)) > > > + return FALSE; > > > + > > > return ((vaddr >= ms->vmalloc_start_addr && vaddr <= > > > ms->vmalloc_end) || > > > ((machdep->flags & VMEMMAP) && > > > (vaddr >= ms->vmemmap_vaddr && vaddr <= > > > ms->vmemmap_end)) > > > || > > > @@ -2407,7 +2512,11 @@ arm64_calc_VA_BITS(void) > > > > > > for (bitval = highest_bit_long(value); bitval; bitval--) { > > > if ((value & (1UL << bitval)) == 0) { > > > +#if 1 > > > + machdep->machspec->VA_BITS = bitval + 1; > > > +#else /* FIXME: bug? */ > > > machdep->machspec->VA_BITS = bitval + 2; > > > +#endif > > > break; > > > } > > > } > > > > Again, a non-starter until the above is resolved -- but you know that > > already... > > Just a reminder for now. > > > > > > @@ -2459,10 +2568,22 @@ arm64_calc_virtual_memory_ranges(void) > > > break; > > > } > > > > > > - vmemmap_size = ALIGN((1UL << (ms->VA_BITS - machdep->pageshift)) * > > > SIZE(page), PUD_SIZE); > > > + if (machdep->flags & NEW_VMEMMAP) > > > +#define STRUCT_PAGE_MAX_SHIFT 6 > > > + vmemmap_size = 1UL << (ms->VA_BITS - machdep->pageshift - 1 > > > + + STRUCT_PAGE_MAX_SHIFT); > > > + else > > > + vmemmap_size = ALIGN((1UL << (ms->VA_BITS - machdep->pageshift)) * > > > SIZE(page), PUD_SIZE); > > > + > > > vmalloc_end = (ms->page_offset - PUD_SIZE - vmemmap_size - SZ_64K); > > > - vmemmap_start = vmalloc_end + SZ_64K; > > > - vmemmap_end = vmemmap_start + vmemmap_size; > > > + > > > + if (machdep->flags & NEW_VMEMMAP) { > > > + vmemmap_start = ms->page_offset - vmemmap_size; > > > + vmemmap_end = ms->page_offset; > > > + } else { > > > + vmemmap_start = vmalloc_end + SZ_64K; > > > + vmemmap_end = vmemmap_start + vmemmap_size; > > > + } > > > > > > ms->vmalloc_end = vmalloc_end - 1; > > > ms->vmemmap_vaddr = vmemmap_start; > > > diff --git a/defs.h b/defs.h > > > index a09fa9a..2bbb55b 100644 > > > --- a/defs.h > > > +++ b/defs.h > > > @@ -2844,8 +2844,8 @@ typedef u64 pte_t; > > > > > > #define PTOV(X) \ > > > ((unsigned > > > long)(X)-(machdep->machspec->phys_offset)+(machdep->machspec->page_offset)) > > > -#define VTOP(X) \ > > > - ((unsigned > > > long)(X)-(machdep->machspec->page_offset)+(machdep->machspec->phys_offset)) > > > + > > > +#define VTOP(X) arm64_VTOP((ulong)(X)) > > > > > > #define USERSPACE_TOP (machdep->machspec->userspace_top) > > > #define PAGE_OFFSET (machdep->machspec->page_offset) > > > @@ -2938,18 +2938,23 @@ typedef signed int s32; > > > #define VM_L3_4K (0x10) > > > #define KDUMP_ENABLED (0x20) > > > #define IRQ_STACKS (0x40) > > > +#define NEW_VMEMMAP (0x80) > > > > > > /* > > > * sources: Documentation/arm64/memory.txt > > > * arch/arm64/include/asm/memory.h > > > * arch/arm64/include/asm/pgtable.h > > > */ > > > - > > > -#define ARM64_PAGE_OFFSET ((0xffffffffffffffffUL) << > > > (machdep->machspec->VA_BITS - 1)) > > > +#define ARM64_VA_START ((0xffffffffffffffffUL) \ > > > + << machdep->machspec->VA_BITS) > > > +#define ARM64_PAGE_OFFSET ((0xffffffffffffffffUL) \ > > > + << (machdep->machspec->VA_BITS - 1)) > > > #define ARM64_USERSPACE_TOP ((1UL) << machdep->machspec->VA_BITS) > > > -#define ARM64_MODULES_VADDR (ARM64_PAGE_OFFSET - MEGABYTES(64)) > > > -#define ARM64_MODULES_END (ARM64_PAGE_OFFSET - 1) > > > -#define ARM64_VMALLOC_START ((0xffffffffffffffffUL) << > > > machdep->machspec->VA_BITS) > > > + > > > +/* only used for v4.6 or later */ > > > +#define ARM64_MODULES_VSIZE MEGABYTES(128) > > > +#define ARM64_KASAN_SHADOW_SIZE (1UL << (machdep->machspec->VA_BITS - > > > 3)) > > > + > > > /* > > > * The following 3 definitions are the original values, but are obsolete > > > * for 3.17 and later kernels because they are now build-time > > > calculations. > > > @@ -3028,6 +3033,10 @@ struct machine_specific { > > > ulong kernel_flags; > > > ulong irq_stack_size; > > > ulong *irq_stacks; > > > + /* only needed for kaslr-enabled kernel */ > > > + ulong kimage_voffset; > > > + ulong kimage_text; > > > + ulong kimage_end; > > > }; > > > > The comment is wrong -- the kimage_text and kimage_end values are needed > > for > > NEW_VMEMMAP kernels that are NOT configured with CONFIG_RANDOMIZE_BASE. > > Unless you consider my live system "kaslr-enabled"? > > I will change it to "for v4.6 or later". > > I have limited (or no) time to investigate a live-system case, and so if > you have any suggestions or your own patch, it is very much appreciated. Yep... but I will build upon your solid foundation. If I see anything interesting/different after my live system upgrade, I'll let you know. Thanks, Dave -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
