* [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. @ 2019-10-08 17:54 Chris von Recklinghausen 2019-10-09 18:09 ` Chris von Recklinghausen ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Chris von Recklinghausen @ 2019-10-08 17:54 UTC (permalink / raw) To: linux-arm-kernel, steve.capper With the reshuffling of kernel VA space to support 52 bits, the kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are based on VA_START, but VA_START no longer points to the base of the kernel virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on PAGE_OFFSET, so simply remove the arm64 definitions of them. Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") --- arch/arm64/include/asm/pgtable.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index 7576df00eb50..8330810f699e 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) - #ifdef CONFIG_ARM64_PA_BITS_52 #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) #else -- 2.18.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. 2019-10-08 17:54 [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr Chris von Recklinghausen @ 2019-10-09 18:09 ` Chris von Recklinghausen 2019-10-10 12:51 ` Chris von Recklinghausen 2019-10-10 16:12 ` James Morse 2019-10-10 16:55 ` Mark Rutland 2 siblings, 1 reply; 6+ messages in thread From: Chris von Recklinghausen @ 2019-10-09 18:09 UTC (permalink / raw) To: linux-arm-kernel, steve.capper; +Cc: Dave Anderson, Russell King On 10/08/2019 01:54 PM, Chris von Recklinghausen wrote: > With the reshuffling of kernel VA space to support 52 bits, the > kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are > based on VA_START, but VA_START no longer points to the base of the kernel > virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has > default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on > PAGE_OFFSET, so simply remove the arm64 definitions of them. > > Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") > --- > arch/arm64/include/asm/pgtable.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index 7576df00eb50..8330810f699e 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > > #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) > > -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) > -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) > - > #ifdef CONFIG_ARM64_PA_BITS_52 > #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) > #else This patch addresses the breakage of access to unity mapped kernel addresses via /proc/kcore, which is a regression. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. 2019-10-09 18:09 ` Chris von Recklinghausen @ 2019-10-10 12:51 ` Chris von Recklinghausen 2019-10-10 13:05 ` Dave Anderson 0 siblings, 1 reply; 6+ messages in thread From: Chris von Recklinghausen @ 2019-10-10 12:51 UTC (permalink / raw) To: linux-arm-kernel, steve.capper; +Cc: Dave Anderson, Russell King On 10/09/2019 02:09 PM, Chris von Recklinghausen wrote: > On 10/08/2019 01:54 PM, Chris von Recklinghausen wrote: >> With the reshuffling of kernel VA space to support 52 bits, the >> kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are >> based on VA_START, but VA_START no longer points to the base of the kernel >> virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has >> default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on >> PAGE_OFFSET, so simply remove the arm64 definitions of them. >> >> Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") >> --- >> arch/arm64/include/asm/pgtable.h | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index 7576df00eb50..8330810f699e 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, >> >> #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) >> >> -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) >> -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) >> - >> #ifdef CONFIG_ARM64_PA_BITS_52 >> #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) >> #else > This patch addresses the breakage of access to unity mapped kernel > addresses via /proc/kcore, which is a regression. > Description of problem as encountered last March by Dave Anderson with RHEL8 development kernel updated and experiment repeated with 5.4.0-rc2 kernel: For the last few days I've been attempting (unsuccessfully) to get the crash utility running with the test 52-bit VA kernel. During initialization, it can read the mapped kernel text/static-data region OK, but as soon as any reads from the unity-mapped region are attempted, it returns garbage. Interestingly enough, one of the major changes in the kernel patch is that it moves the unity-mapped (aka the "direct linear map") region from the very top of kernel virtual address space to the very beginning. Finally out of frustration, I figured I'd take the crash utility out of the picture, and sanity check /proc/kcore using gdb alone. And as it turns out, it appears that something in the patch has broken /proc/kcore, at least w/respect to unity-mapped addresses. (I haven't attempted to read module, vmalloc or the vmemmap regions, because I need to be able to access unity-mapped region as a prerequisite) Here's my evidence... First, these are the results of a "good" gdb session running with the 5.4.0-rc2 kernel, which I will compare with a session running with the 5.4.0-rc2 test kernel with kc_* change. For a simple test, I'm looking at the data pointed to by the static kernel data symbol "vmcoreinfo_data": static unsigned char *vmcoreinfo_data; It gets initialized via get_zeroed_page() with a page of unity-mapped memory: vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL); It then gets filled in with various ASCII strings of kernel data that get passed in an ELF note in /proc/vmcore in the kdump kernel. The strings are all linefeed-terminated (i.e., not NULL-terminated), and the first string is always the "OSRELEASE=" string. Here is the static data address of "vmcoreinfo_data" in the 5.4.0-rc2 kernel: $ nm -Bn vmlinux | grep vmcoreinfo_data ffff8000116cbad8 B vmcoreinfo_data ... Here's what a gdb session looks like on 5.4.0-rc2 running with the fix, where first the virtual address above is verified: $ gdb vmlinux /proc/kcore ... (gdb) p &vmcoreinfo_data $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data> (gdb) Here's the unity-mapped address that was returned by get_zeroed_page(): (gdb) printf "%lx\n", vmcoreinfo_data ffff009f43670000 (gdb) And since it's just a giant string, it can be viewed like so: (gdb) printf "%s\n", vmcoreinfo_data OSRELEASE=5.4.0-rc2.tmp2+ PAGESIZE=65536 SYMBOL(init_uts_ns)=ffff8000113a56e8 SYMBOL(node_online_map)=ffff80001139d570 SYMBOL(swapper_pg_dir)=ffff800010ef0000 SYMBOL(_stext)=ffff800010081000 SYMBOL(vmap_area_list)=ffff80001141c360 SYMBOL(mem_section)=ffff009f7eedc5c0 LENGTH(mem_section)=1024 SIZE(mem_section)=16 OFFSET(mem_section.section_mem_map)=0 SIZE(page)=64 SIZE(pglist_data)=6720 SIZE(zone)=1728 SIZE(free_area)=88 SIZE(list_head)=16 SIZE(nodemask_t)=8 OFFSET(page.flags)=0 OFFSET(page._refcount)=52 OFFSET(page.mapping)=24 OFFSET(page.lru)=8 OFFSET(page._mapcount)=48 OFFSET(page.private)=40 OFFSET(page.compound_dtor)=16 OFFSET(page.compound_order)=17 OFFSET(page.compound_head)=8 OFFSET(pglist_data.node_zones)=0 OFFSET(pglist_data.nr_zones)=5984 OFFSET(pglist_data.node_start_pfn)=5992 OFFSET(pglist_data.node_spanned_pages)=6008 OFFSET(pglist_data.node_id)=6016 OFFSET(zone.free_area)=192 OFFSET(zone.vm_stat)=1536 OFFSET(zone.spanned_pages)=104 OFFSET(free_area.free_list)=0 OFFSET(list_head.next)=0 OFFSET(list_head.prev)=8 OFFSET(vmap_area.va_start)=0 OFFSET(vmap_area.list)=40 LENGTH(zone.free_area)=14 SYMBOL(log_buf)=ffff8000113ceeb8 SYMBOL(log_buf_len)=ffff8000113ceeb0 --Type <RET> for more, q to quit, c to continue without paging--q Now, let's try the same thing running with the kernel without the fix: $ nm -Bn vmlinux | grep vmcoreinfo_data ffff8000116cbad8 B vmcoreinfo_data ... $ gdb vmlinux /proc/kcore ... (gdb) p &vmcoreinfo_data $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data> (gdb) The static kernel data address above matches -- let's look at what unity-mapped address it got from get_zeroed_page(): (gdb) printf "%lx\n", vmcoreinfo_data ffff009f43670000 (gdb) Looks good -- so let's read it: (gdb) printf "%s\n", vmcoreinfo_data (gdb) At first glance, it appeared maybe the vmcoreinfo string data had not been written yet. But with each string that gets appended, there is an accompanying "vmcoreinfo_size" that gets bumped: (gdb) printf "%ld\n", vmcoreinfo_size 1864 (gdb) Without fix: [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8 Copyright (C) 2018 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-redhat-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from vmlinux...done. [New process 1] Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+ root=/dev/mapper/rhel_ampere--hr3'. #0 0x0000000000000000 in ?? () (gdb) printf "%s\n", vmcoreinfo_data (gdb) printf "%ld\n", vmcoreinfo_size 1864 (gdb) quit With fix: [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8 Copyright (C) 2018 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-redhat-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from vmlinux...done. [New process 1] Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+ root=/dev/mapper/rhel_ampere--hr3'. #0 0x0000000000000000 in ?? () (gdb) printf "%s\n", vmcoreinfo_data OSRELEASE=5.4.0-rc2.tmp2+ PAGESIZE=65536 SYMBOL(init_uts_ns)=ffff8000113a56e8 SYMBOL(node_online_map)=ffff80001139d570 SYMBOL(swapper_pg_dir)=ffff800010ef0000 SYMBOL(_stext)=ffff800010081000 SYMBOL(vmap_area_list)=ffff80001141c360 SYMBOL(mem_section)=ffff009f7eedc5c0 LENGTH(mem_section)=1024 SIZE(mem_section)=16 OFFSET(mem_section.section_mem_map)=0 SIZE(page)=64 SIZE(pglist_data)=6720 SIZE(zone)=1728 SIZE(free_area)=88 SIZE(list_head)=16 SIZE(nodemask_t)=8 OFFSET(page.flags)=0 OFFSET(page._refcount)=52 OFFSET(page.mapping)=24 OFFSET(page.lru)=8 OFFSET(page._mapcount)=48 OFFSET(page.private)=40 OFFSET(page.compound_dtor)=16 OFFSET(page.compound_order)=17 OFFSET(page.compound_head)=8 OFFSET(pglist_data.node_zones)=0 OFFSET(pglist_data.nr_zones)=5984 OFFSET(pglist_data.node_start_pfn)=5992 OFFSET(pglist_data.node_spanned_pages)=6008 OFFSET(pglist_data.node_id)=6016 OFFSET(zone.free_area)=192 OFFSET(zone.vm_stat)=1536 OFFSET(zone.spanned_pages)=104 OFFSET(free_area.free_list)=0 OFFSET(list_head.next)=0 OFFSET(list_head.prev)=8 OFFSET(vmap_area.va_start)=0 OFFSET(vmap_area.list)=40 LENGTH(zone.free_area)=14 SYMBOL(log_buf)=ffff8000113ceeb8 SYMBOL(log_buf_len)=ffff8000113ceeb0 --Type <RET> for more, q to quit, c to continue without paging-- SYMBOL(log_first_idx)=ffff8000115c55ac SYMBOL(clear_idx)=ffff8000115c55b8 SYMBOL(log_next_idx)=ffff8000115c55a8 SIZE(printk_log)=16 OFFSET(printk_log.ts_nsec)=0 OFFSET(printk_log.len)=8 OFFSET(printk_log.text_len)=10 OFFSET(printk_log.dict_len)=12 LENGTH(free_area.free_list)=5 NUMBER(NR_FREE_PAGES)=0 NUMBER(PG_lru)=4 NUMBER(PG_private)=13 NUMBER(PG_swapcache)=10 NUMBER(PG_swapbacked)=19 NUMBER(PG_slab)=9 NUMBER(PG_hwpoison)=22 NUMBER(PG_head_mask)=65536 NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE)=-129 NUMBER(HUGETLB_PAGE_DTOR)=2 NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE)=-257 NUMBER(VA_BITS)=52 NUMBER(kimage_voffset)=0xffff7fff80000000 NUMBER(PHYS_OFFSET)=0x80000000 KERNELOFFSET=0 (gdb) printf "%ld\n", vmcoreinfo_size 1864 (gdb) quit _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. 2019-10-10 12:51 ` Chris von Recklinghausen @ 2019-10-10 13:05 ` Dave Anderson 0 siblings, 0 replies; 6+ messages in thread From: Dave Anderson @ 2019-10-10 13:05 UTC (permalink / raw) To: Chris von Recklinghausen Cc: mark.rutland, Russell King, linux-arm-kernel, steve capper Right -- it's an obvious bug/regression, and an obvious fix. Can we please get this addressed before 5.4 is released? Thanks, Dave ----- Original Message ----- > On 10/09/2019 02:09 PM, Chris von Recklinghausen wrote: > > On 10/08/2019 01:54 PM, Chris von Recklinghausen wrote: > >> With the reshuffling of kernel VA space to support 52 bits, the > >> kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they > >> are > >> based on VA_START, but VA_START no longer points to the base of the kernel > >> virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has > >> default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on > >> PAGE_OFFSET, so simply remove the arm64 definitions of them. > >> > >> Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") > >> --- > >> arch/arm64/include/asm/pgtable.h | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/arch/arm64/include/asm/pgtable.h > >> b/arch/arm64/include/asm/pgtable.h > >> index 7576df00eb50..8330810f699e 100644 > >> --- a/arch/arm64/include/asm/pgtable.h > >> +++ b/arch/arm64/include/asm/pgtable.h > >> @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct > >> vm_area_struct *vma, > >> > >> #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) > >> > >> -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) > >> -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) > >> - > >> #ifdef CONFIG_ARM64_PA_BITS_52 > >> #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & > >> TTBR_BADDR_MASK_52) > >> #else > > This patch addresses the breakage of access to unity mapped kernel > > addresses via /proc/kcore, which is a regression. > > > Description of problem as encountered last March by Dave Anderson with RHEL8 > development kernel updated and experiment repeated with 5.4.0-rc2 kernel: > > For the last few days I've been attempting (unsuccessfully) to get the > crash utility running with the test 52-bit VA kernel. During > initialization, > it can read the mapped kernel text/static-data region OK, but as soon as > any reads from the unity-mapped region are attempted, it returns garbage. > Interestingly enough, one of the major changes in the kernel patch is that > it moves the unity-mapped (aka the "direct linear map") region from the > very top of kernel virtual address space to the very beginning. > > Finally out of frustration, I figured I'd take the crash utility out of > the picture, and sanity check /proc/kcore using gdb alone. And as it turns > out, it appears that something in the patch has broken /proc/kcore, at > least w/respect to unity-mapped addresses. (I haven't attempted to read > module, vmalloc or the vmemmap regions, because I need to be able to > access unity-mapped region as a prerequisite) > > Here's my evidence... > > First, these are the results of a "good" gdb session running with the > 5.4.0-rc2 kernel, which I will compare with a session running with the > 5.4.0-rc2 test kernel with kc_* change. For a simple test, I'm > looking at the data pointed to by the static kernel data symbol > "vmcoreinfo_data": > > static unsigned char *vmcoreinfo_data; > > It gets initialized via get_zeroed_page() with a page of unity-mapped > memory: > > vmcoreinfo_data = (unsigned char *)get_zeroed_page(GFP_KERNEL); > > It then gets filled in with various ASCII strings of kernel data > that get passed in an ELF note in /proc/vmcore in the kdump kernel. > The strings are all linefeed-terminated (i.e., not NULL-terminated), > and the first string is always the "OSRELEASE=" string. > > Here is the static data address of "vmcoreinfo_data" in the 5.4.0-rc2 > kernel: > > $ nm -Bn vmlinux | grep vmcoreinfo_data > ffff8000116cbad8 B vmcoreinfo_data > ... > > Here's what a gdb session looks like on 5.4.0-rc2 running with the fix, > where > first the virtual address above is verified: > > $ gdb vmlinux /proc/kcore > ... > (gdb) p &vmcoreinfo_data > $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data> > (gdb) > > Here's the unity-mapped address that was returned by get_zeroed_page(): > > (gdb) printf "%lx\n", vmcoreinfo_data > ffff009f43670000 > (gdb) > > And since it's just a giant string, it can be viewed like so: > > (gdb) printf "%s\n", vmcoreinfo_data > OSRELEASE=5.4.0-rc2.tmp2+ > PAGESIZE=65536 > SYMBOL(init_uts_ns)=ffff8000113a56e8 > SYMBOL(node_online_map)=ffff80001139d570 > SYMBOL(swapper_pg_dir)=ffff800010ef0000 > SYMBOL(_stext)=ffff800010081000 > SYMBOL(vmap_area_list)=ffff80001141c360 > SYMBOL(mem_section)=ffff009f7eedc5c0 > LENGTH(mem_section)=1024 > SIZE(mem_section)=16 > OFFSET(mem_section.section_mem_map)=0 > SIZE(page)=64 > SIZE(pglist_data)=6720 > SIZE(zone)=1728 > SIZE(free_area)=88 > SIZE(list_head)=16 > SIZE(nodemask_t)=8 > OFFSET(page.flags)=0 > OFFSET(page._refcount)=52 > OFFSET(page.mapping)=24 > OFFSET(page.lru)=8 > OFFSET(page._mapcount)=48 > OFFSET(page.private)=40 > OFFSET(page.compound_dtor)=16 > OFFSET(page.compound_order)=17 > OFFSET(page.compound_head)=8 > OFFSET(pglist_data.node_zones)=0 > OFFSET(pglist_data.nr_zones)=5984 > OFFSET(pglist_data.node_start_pfn)=5992 > OFFSET(pglist_data.node_spanned_pages)=6008 > OFFSET(pglist_data.node_id)=6016 > OFFSET(zone.free_area)=192 > OFFSET(zone.vm_stat)=1536 > OFFSET(zone.spanned_pages)=104 > OFFSET(free_area.free_list)=0 > OFFSET(list_head.next)=0 > OFFSET(list_head.prev)=8 > OFFSET(vmap_area.va_start)=0 > OFFSET(vmap_area.list)=40 > LENGTH(zone.free_area)=14 > SYMBOL(log_buf)=ffff8000113ceeb8 > SYMBOL(log_buf_len)=ffff8000113ceeb0 > --Type <RET> for more, q to quit, c to continue without paging--q > > > Now, let's try the same thing running with the kernel without the fix: > > $ nm -Bn vmlinux | grep vmcoreinfo_data > ffff8000116cbad8 B vmcoreinfo_data > ... > $ gdb vmlinux /proc/kcore > ... > (gdb) p &vmcoreinfo_data > $1 = (unsigned char **) 0xffff8000116cbad8 <vmcoreinfo_data> > (gdb) > > The static kernel data address above matches -- let's look at what > unity-mapped > address it got from get_zeroed_page(): > > (gdb) printf "%lx\n", vmcoreinfo_data > ffff009f43670000 > (gdb) > > Looks good -- so let's read it: > > (gdb) printf "%s\n", vmcoreinfo_data > > (gdb) > > At first glance, it appeared maybe the vmcoreinfo string data had not been > written yet. But with each string that gets appended, there is an > accompanying > "vmcoreinfo_size" that gets bumped: > > (gdb) printf "%ld\n", vmcoreinfo_size > 1864 > (gdb) > > Without fix: > > [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore > GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8 > Copyright (C) 2018 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-redhat-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <http://www.gnu.org/software/gdb/bugs/>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > > For help, type "help". > Type "apropos word" to search for commands related to "word"... > Reading symbols from vmlinux...done. > [New process 1] > Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+ > root=/dev/mapper/rhel_ampere--hr3'. > #0 0x0000000000000000 in ?? () > (gdb) printf "%s\n", vmcoreinfo_data > > (gdb) printf "%ld\n", vmcoreinfo_size > 1864 > (gdb) quit > > With fix: > > [root@ampere-hr330a-01 linux-tmp]# gdb vmlinux /proc/kcore > GNU gdb (GDB) Red Hat Enterprise Linux 8.2-6.el8 > Copyright (C) 2018 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-redhat-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <http://www.gnu.org/software/gdb/bugs/>. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>. > > For help, type "help". > Type "apropos word" to search for commands related to "word"... > Reading symbols from vmlinux...done. > [New process 1] > Core was generated by `BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.4.0-rc2.tmp2+ > root=/dev/mapper/rhel_ampere--hr3'. > #0 0x0000000000000000 in ?? () > (gdb) printf "%s\n", vmcoreinfo_data > OSRELEASE=5.4.0-rc2.tmp2+ > PAGESIZE=65536 > SYMBOL(init_uts_ns)=ffff8000113a56e8 > SYMBOL(node_online_map)=ffff80001139d570 > SYMBOL(swapper_pg_dir)=ffff800010ef0000 > SYMBOL(_stext)=ffff800010081000 > SYMBOL(vmap_area_list)=ffff80001141c360 > SYMBOL(mem_section)=ffff009f7eedc5c0 > LENGTH(mem_section)=1024 > SIZE(mem_section)=16 > OFFSET(mem_section.section_mem_map)=0 > SIZE(page)=64 > SIZE(pglist_data)=6720 > SIZE(zone)=1728 > SIZE(free_area)=88 > SIZE(list_head)=16 > SIZE(nodemask_t)=8 > OFFSET(page.flags)=0 > OFFSET(page._refcount)=52 > OFFSET(page.mapping)=24 > OFFSET(page.lru)=8 > OFFSET(page._mapcount)=48 > OFFSET(page.private)=40 > OFFSET(page.compound_dtor)=16 > OFFSET(page.compound_order)=17 > OFFSET(page.compound_head)=8 > OFFSET(pglist_data.node_zones)=0 > OFFSET(pglist_data.nr_zones)=5984 > OFFSET(pglist_data.node_start_pfn)=5992 > OFFSET(pglist_data.node_spanned_pages)=6008 > OFFSET(pglist_data.node_id)=6016 > OFFSET(zone.free_area)=192 > OFFSET(zone.vm_stat)=1536 > OFFSET(zone.spanned_pages)=104 > OFFSET(free_area.free_list)=0 > OFFSET(list_head.next)=0 > OFFSET(list_head.prev)=8 > OFFSET(vmap_area.va_start)=0 > OFFSET(vmap_area.list)=40 > LENGTH(zone.free_area)=14 > SYMBOL(log_buf)=ffff8000113ceeb8 > SYMBOL(log_buf_len)=ffff8000113ceeb0 > --Type <RET> for more, q to quit, c to continue without paging-- > SYMBOL(log_first_idx)=ffff8000115c55ac > SYMBOL(clear_idx)=ffff8000115c55b8 > SYMBOL(log_next_idx)=ffff8000115c55a8 > SIZE(printk_log)=16 > OFFSET(printk_log.ts_nsec)=0 > OFFSET(printk_log.len)=8 > OFFSET(printk_log.text_len)=10 > OFFSET(printk_log.dict_len)=12 > LENGTH(free_area.free_list)=5 > NUMBER(NR_FREE_PAGES)=0 > NUMBER(PG_lru)=4 > NUMBER(PG_private)=13 > NUMBER(PG_swapcache)=10 > NUMBER(PG_swapbacked)=19 > NUMBER(PG_slab)=9 > NUMBER(PG_hwpoison)=22 > NUMBER(PG_head_mask)=65536 > NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE)=-129 > NUMBER(HUGETLB_PAGE_DTOR)=2 > NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE)=-257 > NUMBER(VA_BITS)=52 > NUMBER(kimage_voffset)=0xffff7fff80000000 > NUMBER(PHYS_OFFSET)=0x80000000 > KERNELOFFSET=0 > > (gdb) printf "%ld\n", vmcoreinfo_size > 1864 > (gdb) quit > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. 2019-10-08 17:54 [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr Chris von Recklinghausen 2019-10-09 18:09 ` Chris von Recklinghausen @ 2019-10-10 16:12 ` James Morse 2019-10-10 16:55 ` Mark Rutland 2 siblings, 0 replies; 6+ messages in thread From: James Morse @ 2019-10-10 16:12 UTC (permalink / raw) To: Chris von Recklinghausen, linux-arm-kernel Cc: Mark Rutland, steve.capper, Catalin Marinas, Russell King, anderson, will Hi Chris, (CC: +arm64 maintainers ... and the folk further down the thread) ./scripts/get_maintainer.pl will give you a hint at who to CC on patches. On 08/10/2019 18:54, Chris von Recklinghausen wrote: > With the reshuffling of kernel VA space to support 52 bits, the > kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are > based on VA_START, git grep VA_START in /arch/arm64 brings back nothing. These are defined based on PAGE_END, but that is a renamed version of VA_START due to commit 77ad4ce69321a ("arm64: memory: rename VA_START to PAGE_END"). > but VA_START no longer points to the base of the kernel > virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has > default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on > PAGE_OFFSET, so simply remove the arm64 definitions of them. So far this just sounds like we are doing something unnecessary... your subsequent mail says its a regression in v5.4. It does look like kc_offset_to_vaddr() should map values in/out of the full TTBR1 range. It was previously using VA_START. These macros were added on arm64 by commit 03875ad52fdd ("arm64: add kc_offset_to_vaddr and kc_vaddr_to_offset macro"), which notes the default version didn't work because the kernel image was below PAGE_OFFSET. (This sounds like a fix for when we moved the kernel from the linear-map, but I thought that was done by a7f8de168ace as part of kaslr, which was merged later) It looks like the core code versions will now work, but only because PAGE_OFFSET is now 'the start of the TTBR1 address space', (the macro formerly known as VA_START). Based on what we did in commit 233947ef16a1 ("arm64: memory: fix flipped VA space fallout"), we're okay with PAGE_OFFSET meaning both. Could you summarise what is broken in the commit message, and how this changed fixes it? Getting the word 'fix' in the subject will make this look less like cleanup. Thanks, James > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index 7576df00eb50..8330810f699e 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > > #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) > > -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) > -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) > - > #ifdef CONFIG_ARM64_PA_BITS_52 > #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) > #else > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr. 2019-10-08 17:54 [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr Chris von Recklinghausen 2019-10-09 18:09 ` Chris von Recklinghausen 2019-10-10 16:12 ` James Morse @ 2019-10-10 16:55 ` Mark Rutland 2 siblings, 0 replies; 6+ messages in thread From: Mark Rutland @ 2019-10-10 16:55 UTC (permalink / raw) To: Chris von Recklinghausen Cc: will, james.morse, catalin.marinas, linux-arm-kernel, steve.capper Hi, [Roping in the arm64 maintainers] On Tue, Oct 08, 2019 at 01:54:34PM -0400, Chris von Recklinghausen wrote: > With the reshuffling of kernel VA space to support 52 bits, the > kc_vaddr_to_offset and kc_offset_to_vaddr macros are broken, since they are > based on VA_START, but VA_START no longer points to the base of the kernel > virtual address space, (PAGE_OFFSET does now). fs/proc/kcore.c already has > default definitions of kc_vaddr_to_offset and kc_offset_to_vaddr based on > PAGE_OFFSET, so simply remove the arm64 definitions of them. > > Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") I think this patch is correct, but it's missing a Signed-off-by. Can you please provide that here? As James said in another reply, the commit message should describe the intended semantic, what broke, and how this patch fixes the issue. As VA_START got renamed (and no longer exists), it's also a bit unclear. How about: | arm64: fix kcore macros 52-bit va fallout | | We export the entire kernel address space (i.e. the whole of the TTBR1 | address range) via /proc/kcore. The kc_vaddr_to_offset() and | kc_offset_to_vaddr() macros are intended to convert between a kernel | virtual address and its offset relative to the start of the TTBR1 | address space. | | Prior to commit: | | 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") | | ... the offset was calculated relative to VA_START, which at the time | was the start of the TTBR1 address space. At this time, PAGE_OFFSET | pointed to the high half of the TTBR1 address space where arm64's | linear map lived. | | That commit swapped the position of VA_START and PAGE_OFFSET, but | failed to update kc_vaddr_to_offset() or kc_offset_to_vaddr(), so | since then the two macros behave incorrectly. | | Note that VA_START was subsequently renamed to PAGE_END in commit: | | 77ad4ce69321abbe ("arm64: memory: rename VA_START to PAGE_END") | | As the generic implementations of the two macros calculate the offset | relative to PAGE_OFFSET (which is now the start of the TTBR1 address | space), we can delete the arm64 implementation and use those. | | Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space") With that wording (and your Signed-off-by): Reviewed-by: Mark Rutland <mark.rutland@arm.com> Thanks, Mark. > --- > arch/arm64/include/asm/pgtable.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index 7576df00eb50..8330810f699e 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -876,9 +876,6 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > > #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0) > > -#define kc_vaddr_to_offset(v) ((v) & ~PAGE_END) > -#define kc_offset_to_vaddr(o) ((o) | PAGE_END) > - > #ifdef CONFIG_ARM64_PA_BITS_52 > #define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) > #else > -- > 2.18.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-10 16:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-08 17:54 [PATCH] arm64: remove arm64 definitions of kc_vaddr_to_offset and kc_offset_to_vaddr Chris von Recklinghausen 2019-10-09 18:09 ` Chris von Recklinghausen 2019-10-10 12:51 ` Chris von Recklinghausen 2019-10-10 13:05 ` Dave Anderson 2019-10-10 16:12 ` James Morse 2019-10-10 16:55 ` Mark Rutland
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.