Re: [Crash-utility] Fix for the determination of the ARM64 page size

Hi Dave and yueyi, I read what you mean, above your suggestions, I made changes for patch v2.

Best regards, Qiwu -----Original Message----- From: Yueyi Li <liyueyi(a)live.com&gt; Sent: Friday, November 15, 2019 12:17 AM To: Discussion list for crash utility usage, maintenance and development <crash-utility(a)redhat.com>; 陈启武 <chenqiwu(a)xiaomi.com&gt; Subject: [External Mail]Re: [Crash-utility] 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(a)redhat.com 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(a)redhat.com&gt; >> Sent: Wednesday, November 13, 2019 11:28 PM >> To: 陈启武 <chenqiwu(a)xiaomi.com&gt; >> Cc: Discussion list for crash utility usage, maintenance and >> development <crash-utility(a)redhat.com&gt; >> 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(a)redhat.com&gt; >>> Sent: Tuesday, November 12, 2019 11:34 PM >>> To: 陈启武 <chenqiwu(a)xiaomi.com&gt; >>> Cc: Discussion list for crash utility usage, maintenance and >>> development <crash-utility(a)redhat.com&gt; >>> 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(a)linaro.org&gt; >>> 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(a)linaro.org&gt; >>> 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(a)redhat.com > 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!******/#