* [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.