----- Original Message ----- > Hi Dave and yueyi, > I read what you mean, above your suggestions, I made changes for patch v2. Looks good -- queued for crash-7.2.8: https://github.com/crash-utility/crash/commit/babd7ae62d4e8fd6f93fd30b88040d9376522aa3 Thanks, Dave > > Best regards, > Qiwu > > > -----Original Message----- > From: Yueyi Li <liyueyi@xxxxxxxx> > Sent: Friday, November 15, 2019 12:17 AM > To: Discussion list for crash utility usage, maintenance and development > <crash-utility@xxxxxxxxxx>; 陈启武 <chenqiwu@xxxxxxxxxx> > Subject: [External Mail]Re: Fix for the determination of the > ARM64 page size > > > > On 2019/11/14 22:14, Dave Anderson wrote: > > > > > > ----- Original Message ----- > >> Hi Dave, > >> Since linux 4.4 and later kernels will always be able to determine > >> the page size by reading the kernel flags (if there is no > >> vmcoreinfo), I agree checking for (THIS_KERNEL_VERSION < LINUX(4,4,0)) is > >> more reasonable. > Hi Qiwu, > > Do you noticed this line? > > 182 if (!machdep->pagesize && > 183 kernel_symbol_exists("swapper_pg_dir") && > 184 kernel_symbol_exists("idmap_pg_dir")) { > > That means "pagesize" can not be read from VMCOREINFO and kernel image > header, so the kernel version must be earlier than Linux 4.4 if this code > section be executed. So, just change it back should be OK. > > Besides, I can only see your message by Dave quoted. Could you please add > mailing list crash-uility@xxxxxxxxxx to 'CC' list, or just sending mail to > mailing list for any discussion? > > Thanks, > Yueyi > > > > Did you finish reading my response from yesterday? > > > > There is no reason to check for (THIS_KERNEL_VERSION < LINUX(4,4,0)), > > because the code section will *only* be executed if the kernel is earlier > > than Linux 4.4. > > > > Again: the "else" section is dead code because it can never be executed: > > > > + if (THIS_KERNEL_VERSION < LINUX(4,16,0)) { > > + value = symbol_value("swapper_pg_dir") - > > + symbol_value("idmap_pg_dir"); > > + } else { > > + if (kernel_symbol_exists("tramp_pg_dir")) > > + value = > > symbol_value("tramp_pg_dir"); > > + else if > > (kernel_symbol_exists("reserved_ttbr0")) > > + value = > > symbol_value("reserved_ttbr0"); > > + else > > + value = > > + symbol_value("swapper_pg_dir"); > > + > > + value -= symbol_value("idmap_pg_dir"); > > + } > > > > You can just use "swapper_pg_dir" and "idmap_pg_dir". > > > > Dave > > > > > > > > > > > >> > >> Best regards, > >> Qiwu > >> > >> -----Original Message----- > >> From: Dave Anderson <anderson@xxxxxxxxxx> > >> Sent: Wednesday, November 13, 2019 11:28 PM > >> To: 陈启武 <chenqiwu@xxxxxxxxxx> > >> Cc: Discussion list for crash utility usage, maintenance and > >> development <crash-utility@xxxxxxxxxx> > >> Subject: Re: [External Mail]Re: Fix for the determination of the > >> ARM64 page size > >> > >> > >> > >> ----- Original Message ----- > >>> Hi Dave, > >>> I find the bug from an ELF format arm64 ramdump (not vmcoreinfo) > >>> with linux 3.18. > >>> As we know, given the page size flags entry was introduced on Linux > >>> 4.4 -rc1 and later versions, so the PAGE_SIZE cannot be determinated > >>> by the following steps for ELF format > >>> arm64 ramdump files with previous Linux 4.4 versions: > >>> (1) checking the vmcoreinfo data, and > >>> (2) checking the kernel image header for the flags field. > >>> > >>> If we ignore the following two steps, could the PAGE_SIZE be > >>> determinated by the third step for previous Linux 4.16 versions? > >>> I think the answer is no, because the symbols order from lowest to > >>> highest value is idmap_pg_dir -> swapper_pg_dir -> reserved_ttbr0 -> > >>> tramp_pg_dir. > >>> idmap_pg_dir = .; > >>> . += IDMAP_DIR_SIZE; > >>> swapper_pg_dir = .; > >>> . += SWAPPER_DIR_SIZE; > >>> > >>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN > >>> reserved_ttbr0 = .; > >>> . += RESERVED_TTBR0_SIZE; > >>> #endif > >>> > >>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 > >>> tramp_pg_dir = .; > >>> . += PAGE_SIZE; > >>> #endif > >>> > >>> For Linux 4.16 and later kernels with commit > >>> 1e1b8c04fa3451e2b7190930adae43c95f0fae31 have changed the symbols > >>> order, from lowest to highest value is idmap_pg_dir -> tramp_pg_dir > >>> -> > >>> reserved_ttbr0 -> swapper_pg_dir. > >>> idmap_pg_dir = .; > >>> . += IDMAP_DIR_SIZE; > >>> > >>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 > >>> tramp_pg_dir = .; > >>> . += PAGE_SIZE; > >>> #endif > >>> > >>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN > >>> reserved_ttbr0 = .; > >>> . += RESERVED_TTBR0_SIZE; > >>> #endif > >>> swapper_pg_dir = .; > >>> . += PAGE_SIZE; > >>> swapper_pg_end = .; > >>> > >>> so we must consider the case on previous Linux 4.16 kernels, > >>> especially for previous Linux 4.4 kernels without commit > >>> 9d372c9fab34cd8803141871195141995f85c7f7. > >> > >> But we really only need to consider kernels that are earlier than > >> Linux 4.4, because Linux 4.4 and later kernels will always be able to > >> determine the page size by reading the kernel flags (if there is no > >> vmcoreinfo). So the code below that you are patching will only be > >> executed if: > >> > >> (1) there is no vmcoreinfo, and > >> (2) no kernel flags (in kernels earlier than Linux 4.4): > >> > >> That being the case, I don't see how it would ever be possible for the > >> "else" > >> section below to ever be executed: > >> > >> + if (THIS_KERNEL_VERSION < LINUX(4,16,0)) { > >> + value = symbol_value("swapper_pg_dir") - > >> + symbol_value("idmap_pg_dir"); > >> + } else { > >> + if (kernel_symbol_exists("tramp_pg_dir")) > >> + value = > >> symbol_value("tramp_pg_dir"); > >> + else if > >> (kernel_symbol_exists("reserved_ttbr0")) > >> + value = > >> symbol_value("reserved_ttbr0"); > >> + else > >> + value = > >> + symbol_value("swapper_pg_dir"); > >> + > >> + value -= symbol_value("idmap_pg_dir"); > >> + } > >> > >> I was going to suggest checking for (THIS_KERNEL_VERSION < > >> LINUX(4,4,0)), but I don't think that's even necessary given that the > >> code sequence above will > >> *only* be executed if the kernel is Linux 4.4 or earlier. So the "else" > >> section has become dead code. > >> > >> Dave > >> > >> > >>> Best regards, > >>> Qiwu > >>> > >>> > >>> -----Original Message----- > >>> From: Dave Anderson <anderson@xxxxxxxxxx> > >>> Sent: Tuesday, November 12, 2019 11:34 PM > >>> To: 陈启武 <chenqiwu@xxxxxxxxxx> > >>> Cc: Discussion list for crash utility usage, maintenance and > >>> development <crash-utility@xxxxxxxxxx> > >>> Subject: [External Mail]Re: Fix for the determination of the ARM64 > >>> page size > >>> > >>> > >>> ----- Original Message ----- > >>>> Hi Dave, > >>>> There is a bug for the determination of the ARM64 page size happen > >>>> on kernel 3.18 crash kdump. > >>> > >>> If it is a kdump, there should be a PAGESIZE vmcoreinfo entry. > >>> As far as I can tell, the PAGE_SIZE has always been exported as the > >>> second item for all architectures here: > >>> > >>> static int __init crash_save_vmcoreinfo_init(void) > >>> { > >>> VMCOREINFO_OSRELEASE(init_uts_ns.name.release); > >>> VMCOREINFO_PAGESIZE(PAGE_SIZE); > >>> ... > >>> > >>> What does "help -D" show for the vmcoreinfo data on your dumpfile? > >>> > >>> > >>>> The crash session failed immediately with the error message "crash: > >>>> cannot determine page size" since the page size cannot be > >>>> determinted by kernel image header flags field or subtraction of > >>>> symbol values address. > >>>> ffffffc0024df000 A idmap_pg_dir > >>>> ffffffc0024e2000 A swapper_pg_dir > >>>> ffffffc0024e4000 A tramp_pg_dir > >>>> so value = symbol_value("tramp_pg_dir") - > >>>> symbol_value("idmap_pg_dir") = 5 > >>>> * PAGE_SIZE, the vaule result is determined by the order of symbol > >>>> address: > >>>> [kernel-3.18/arch/arm64/kernel/vmlinux.lds.S] > >>>> BSS_SECTION(0, 0, 0) > >>>> > >>>> . = ALIGN(PAGE_SIZE); > >>>> idmap_pg_dir = .; > >>>> . += IDMAP_DIR_SIZE; > >>>> swapper_pg_dir = .; > >>>> . += SWAPPER_DIR_SIZE; > >>>> > >>>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN > >>>> reserved_ttbr0 = .; > >>>> . += RESERVED_TTBR0_SIZE; > >>>> #endif > >>>> > >>>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 > >>>> tramp_pg_dir = .; > >>>> . += PAGE_SIZE; > >>>> #endif > >>>> > >>>> For Linux 4.16 and later kernels have changed the order of symbol > >>>> definition due to containing the commit > >>>> 1e1b8c04fa3451e2b7190930adae43c95f0fae31, > >>>> So crash utility upstream commit > >>>> 764e2d09978bb3f87dfaff4c6a59d4a5cc00f277 to fix it, but it ignore > >>>> the determination of the ARM64 page size on previous Linux 4.16 kernels. > >>>> > >>>> So I recommend this patch to fix it. > >>> > >>> I have several old arm64 dumpfiles, with kernel versions 3.19, 4.2, > >>> 4.4, 4.5, 4.7, > >>> 4.9 and 4.14. However, none of them reach your patch because the > >>> code section that you are patching is only used as a third option after: > >>> > >>> (1) checking the vmcoreinfo data, and > >>> (2) checking the kernel image header for the flags field. > >>> > >>> In Linux 4.4, this patch added the page size to the kernel image header: > >>> > >>> commit 9d372c9fab34cd8803141871195141995f85c7f7 > >>> Author: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > >>> Date: Mon Oct 19 14:19:36 2015 +0100 > >>> > >>> arm64: Add page size to the kernel image header > >>> > >>> This patch adds the page size to the arm64 kernel image header > >>> so that one can infer the PAGESIZE used by the kernel. This will > >>> be helpful to diagnose failures to boot the kernel with page size > >>> not supported by the CPU. > >>> > >>> And later on in Linux 4.6, "_kernel_flags_le" was replaced by > >>> "_kernel_flags_le_lo32" and "_kernel_flags_le_hi32": > >>> > >>> commit 6ad1fe5d9077a1ab40bf74b61994d2e770b00b14 > >>> Author: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > >>> Date: Sat Dec 26 13:48:02 2015 +0100 > >>> > >>> arm64: avoid R_AARCH64_ABS64 relocations for Image header > >>> fields > >>> > >>> Unfortunately, the current way of using the linker to emit build > >>> time > >>> constants into the Image header will no longer work once we switch > >>> to > >>> the use of PIE executables. The reason is that such constants are > >>> emitted > >>> into the binary using R_AARCH64_ABS64 relocations, which are > >>> resolved > >>> at > >>> runtime, not at build time, and the places targeted by those > >>> relocations > >>> will contain zeroes before that. > >>> > >>> So refactor the endian swapping linker script constant generation > >>> code > >>> so > >>> that it emits the upper and lower 32-bit words separately. > >>> > >>> Anyway, given that the page size flags entry was introduced in Linux > >>> 4.4, I don't believe your patch checking for LINUX(4,16,0) is correct: > >>> > >>> +if (THIS_KERNEL_VERSION < LINUX(4,16,0)) { value = > >>> +symbol_value("swapper_pg_dir") - symbol_value("idmap_pg_dir"); } > >>> +else { > >>> > >>> Do you agree? > >>> > >>> Dave > >>> > >>> #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于 > >>> 全部 > >>> 或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ > >>> This e-mail and its attachments contain confidential information > >>> from XIAOMI, which is intended only for the person or entity whose > >>> address is listed above. Any use of the information contained herein > >>> in any way (including, but not limited to, total or partial > >>> disclosure, reproduction, or dissemination) by persons other than > >>> the intended > >>> recipient(s) is prohibited. If you receive this e-mail in error, > >>> please notify the sender by phone or email immediately and delete > >>> it!******/# > >>> > >> > >> #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全 > >> 部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ > >> This e-mail and its attachments contain confidential information from > >> XIAOMI, which is intended only for the person or entity whose address > >> is listed above. Any use of the information contained herein in any > >> way (including, but not limited to, total or partial disclosure, > >> reproduction, or dissemination) by persons other than the intended > >> recipient(s) is prohibited. If you receive this e-mail in error, > >> please notify the sender by phone or email immediately and delete > >> it!******/# > >> > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/crash-utility > > > #/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ > This e-mail and its attachments contain confidential information from > XIAOMI, which is intended only for the person or entity whose address is > listed above. Any use of the information contained herein in any way > (including, but not limited to, total or partial disclosure, reproduction, > or dissemination) by persons other than the intended recipient(s) is > prohibited. If you receive this e-mail in error, please notify the sender by > phone or email immediately and delete it!******/# > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
