Changes since v1:
----------------
- v1 was sent out as a single patch which can be seen here:
http://lists.infradead.org/pipermail/kexec/2019-February/022411.html
- v2 breaks the single patch into two independent patches:
[PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
[PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)
This patchset primarily fixes the regression reported in user-space
utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
with the availability of 52-bit address space feature in underlying
kernel. These regressions have been reported both on CPUs which don't
support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
and also on prototype platforms (like ARMv8 FVP simulator model) which
support ARMv8.2 extensions and are running newer kernels.
The reason for these regressions is that right now user-space tools
have no direct access to these values (since these are not exported
from the kernel) and hence need to rely on a best-guess method of
determining value of 'PTRS_PER_PGD' and 'MAX_PHYSMEM_BITS' supported
by underlying kernel.
Exporting these values via vmcoreinfo will help user-land in such cases.
In addition, as per suggestion from makedumpfile maintainer (Kazu)
during v1 review, it makes more sense to append 'MAX_PHYSMEM_BITS' to
vmcoreinfo in the core code itself rather than in arm64 arch-specific
code, so that the user-space code for other archs can also benefit from
this addition to the vmcoreinfo and use it as a standard way of
determining 'SECTIONS_SHIFT' value in user-land.
Cc: Mark Rutland <[email protected]>
Cc: James Morse <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Boris Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Dave Anderson <[email protected]>
Cc: Kazuhito Hagio <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Bhupesh Sharma (2):
arm64, vmcoreinfo : Append 'PTRS_PER_PGD' to vmcoreinfo
crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
arch/arm64/kernel/crash_core.c | 1 +
kernel/crash_core.c | 1 +
2 files changed, 2 insertions(+)
--
2.7.4
With ARMv8.2-LVA architecture extension availability, arm64 hardware
which supports this extension can support a virtual address-space upto
52-bits.
Since at the moment we enable the support of this extension in kernel
via CONFIG flags, e.g.
- User-space 52-bit LVA via CONFIG_ARM64_USER_VA_BITS_52
so, there is no clear mechanism in the user-space right now to
determine these CONFIG flag values and hence determine the maximum
virtual address space supported by the underlying kernel.
User-space tools like 'makedumpfile' therefore are broken currently
as they have no proper method to calculate the 'PTRS_PER_PGD' value
which is required to perform a page table walk to determine the
physical address of a corresponding virtual address found in
kcore/vmcoreinfo.
If one appends 'PTRS_PER_PGD' number to vmcoreinfo for arm64,
it can be used in user-space to determine the maximum virtual address
supported by underlying kernel.
A reference 'makedumpfile' implementation which uses this approach to
determining the maximum physical address is available in [0].
[0]. https://github.com/bhupesh-sharma/makedumpfile/blob/52-bit-va-support-via-vmcore-upstream-v3/arch/arm64.c#L459
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: James Morse <[email protected]>
Cc: Dave Anderson <[email protected]>
Cc: Kazuhito Hagio <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Suggested-by: Steve Capper <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/kernel/crash_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
index ca4c3e12d8c5..123a42c56b8e 100644
--- a/arch/arm64/kernel/crash_core.c
+++ b/arch/arm64/kernel/crash_core.c
@@ -10,6 +10,7 @@
void arch_crash_save_vmcoreinfo(void)
{
VMCOREINFO_NUMBER(VA_BITS);
+ VMCOREINFO_NUMBER(PTRS_PER_PGD);
/* Please note VMCOREINFO_NUMBER() uses "%d", not "%x" */
vmcoreinfo_append_str("NUMBER(kimage_voffset)=0x%llx\n",
kimage_voffset);
--
2.7.4
Right now user-space tools like 'makedumpfile' and 'crash' need to rely
on a best-guess method of determining value of 'MAX_PHYSMEM_BITS'
supported by underlying kernel.
This value is used in user-space code to calculate the bit-space
required to store a section for SPARESMEM (similar to the existing
calculation method used in the kernel implementation):
#define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
Now, regressions have been reported in user-space utilities
like 'makedumpfile' and 'crash' on arm64, with the recently added
kernel support for 52-bit physical address space, as there is
no clear method of determining this value in user-space
(other than reading kernel CONFIG flags).
As per suggestion from makedumpfile maintainer (Kazu), it makes more
sense to append 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself
rather than in arch-specific code, so that the user-space code for other
archs can also benefit from this addition to the vmcoreinfo and use it
as a standard way of determining 'SECTIONS_SHIFT' value in user-land.
A reference 'makedumpfile' implementation which reads the
'MAX_PHYSMEM_BITS' value from vmcoreinfo in a arch-independent fashion
is available here:
[0]. https://github.com/bhupesh-sharma/makedumpfile/blob/remove-max-phys-mem-bit-v1/arch/ppc64.c#L471
Cc: Boris Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: James Morse <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Dave Anderson <[email protected]>
Cc: Kazuhito Hagio <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Bhupesh Sharma <[email protected]>
---
kernel/crash_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 093c9f917ed0..44b90368e183 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -467,6 +467,7 @@ static int __init crash_save_vmcoreinfo_init(void)
#define PAGE_OFFLINE_MAPCOUNT_VALUE (~PG_offline)
VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
#endif
+ VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
arch_crash_save_vmcoreinfo();
update_vmcoreinfo_note();
--
2.7.4
Hi Bhupesh,
On 03/10/19 at 03:34pm, Bhupesh Sharma wrote:
> Changes since v1:
> ----------------
> - v1 was sent out as a single patch which can be seen here:
> http://lists.infradead.org/pipermail/kexec/2019-February/022411.html
>
> - v2 breaks the single patch into two independent patches:
> [PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
> [PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)
>
> This patchset primarily fixes the regression reported in user-space
> utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
> with the availability of 52-bit address space feature in underlying
> kernel. These regressions have been reported both on CPUs which don't
> support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
> and also on prototype platforms (like ARMv8 FVP simulator model) which
> support ARMv8.2 extensions and are running newer kernels.
>
> The reason for these regressions is that right now user-space tools
> have no direct access to these values (since these are not exported
> from the kernel) and hence need to rely on a best-guess method of
> determining value of 'PTRS_PER_PGD' and 'MAX_PHYSMEM_BITS' supported
> by underlying kernel.
>
> Exporting these values via vmcoreinfo will help user-land in such cases.
> In addition, as per suggestion from makedumpfile maintainer (Kazu)
> during v1 review, it makes more sense to append 'MAX_PHYSMEM_BITS' to
> vmcoreinfo in the core code itself rather than in arm64 arch-specific
> code, so that the user-space code for other archs can also benefit from
> this addition to the vmcoreinfo and use it as a standard way of
> determining 'SECTIONS_SHIFT' value in user-land.
>
> Cc: Mark Rutland <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Boris Petkov <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Dave Anderson <[email protected]>
> Cc: Kazuhito Hagio <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Bhupesh Sharma (2):
> arm64, vmcoreinfo : Append 'PTRS_PER_PGD' to vmcoreinfo
> crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
>
> arch/arm64/kernel/crash_core.c | 1 +
> kernel/crash_core.c | 1 +
> 2 files changed, 2 insertions(+)
>
Lianbo's document patch has been merged, would you mind to add vmcoreinfo doc
patch as well in your series?
Thanks
Dave
Hi Dave,
On 03/11/2019 02:35 PM, Dave Young wrote:
> Hi Bhupesh,
> On 03/10/19 at 03:34pm, Bhupesh Sharma wrote:
>> Changes since v1:
>> ----------------
>> - v1 was sent out as a single patch which can be seen here:
>> http://lists.infradead.org/pipermail/kexec/2019-February/022411.html
>>
>> - v2 breaks the single patch into two independent patches:
>> [PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
>> [PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)
>>
>> This patchset primarily fixes the regression reported in user-space
>> utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
>> with the availability of 52-bit address space feature in underlying
>> kernel. These regressions have been reported both on CPUs which don't
>> support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
>> and also on prototype platforms (like ARMv8 FVP simulator model) which
>> support ARMv8.2 extensions and are running newer kernels.
>>
>> The reason for these regressions is that right now user-space tools
>> have no direct access to these values (since these are not exported
>> from the kernel) and hence need to rely on a best-guess method of
>> determining value of 'PTRS_PER_PGD' and 'MAX_PHYSMEM_BITS' supported
>> by underlying kernel.
>>
>> Exporting these values via vmcoreinfo will help user-land in such cases.
>> In addition, as per suggestion from makedumpfile maintainer (Kazu)
>> during v1 review, it makes more sense to append 'MAX_PHYSMEM_BITS' to
>> vmcoreinfo in the core code itself rather than in arm64 arch-specific
>> code, so that the user-space code for other archs can also benefit from
>> this addition to the vmcoreinfo and use it as a standard way of
>> determining 'SECTIONS_SHIFT' value in user-land.
>>
>> Cc: Mark Rutland <[email protected]>
>> Cc: James Morse <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Boris Petkov <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Dave Anderson <[email protected]>
>> Cc: Kazuhito Hagio <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>>
>> Bhupesh Sharma (2):
>> arm64, vmcoreinfo : Append 'PTRS_PER_PGD' to vmcoreinfo
>> crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
>>
>> arch/arm64/kernel/crash_core.c | 1 +
>> kernel/crash_core.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>
> Lianbo's document patch has been merged, would you mind to add vmcoreinfo doc
> patch as well in your series?
Thanks for the inputs. Will add it to the v3.
Let's wait for other comments/reviews, before I spin a version 3.
Regards,
Bhupesh
Hi Bhupesh,
-----Original Message-----
> Right now user-space tools like 'makedumpfile' and 'crash' need to rely
> on a best-guess method of determining value of 'MAX_PHYSMEM_BITS'
> supported by underlying kernel.
>
> This value is used in user-space code to calculate the bit-space
> required to store a section for SPARESMEM (similar to the existing
> calculation method used in the kernel implementation):
>
> #define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
>
> Now, regressions have been reported in user-space utilities
> like 'makedumpfile' and 'crash' on arm64, with the recently added
> kernel support for 52-bit physical address space, as there is
> no clear method of determining this value in user-space
> (other than reading kernel CONFIG flags).
>
> As per suggestion from makedumpfile maintainer (Kazu), it makes more
> sense to append 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself
> rather than in arch-specific code, so that the user-space code for other
> archs can also benefit from this addition to the vmcoreinfo and use it
> as a standard way of determining 'SECTIONS_SHIFT' value in user-land.
>
> A reference 'makedumpfile' implementation which reads the
> 'MAX_PHYSMEM_BITS' value from vmcoreinfo in a arch-independent fashion
> is available here:
>
> [0]. https://github.com/bhupesh-sharma/makedumpfile/blob/remove-max-phys-mem-bit-v1/arch/ppc64.c#L471
>
> Cc: Boris Petkov <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Dave Anderson <[email protected]>
> Cc: Kazuhito Hagio <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> kernel/crash_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 093c9f917ed0..44b90368e183 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -467,6 +467,7 @@ static int __init crash_save_vmcoreinfo_init(void)
> #define PAGE_OFFLINE_MAPCOUNT_VALUE (~PG_offline)
> VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
> #endif
> + VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
Some architectures define MAX_PHYSMEM_BITS only with CONFIG_SPARSEMEM,
so we need to move this to the #ifdef section that exports some
mem_section things.
Thanks!
Kazu
>
> arch_crash_save_vmcoreinfo();
> update_vmcoreinfo_note();
> --
> 2.7.4
>
Hi Kazu,
On 03/13/2019 01:17 AM, Kazuhito Hagio wrote:
> Hi Bhupesh,
>
> -----Original Message-----
>> Right now user-space tools like 'makedumpfile' and 'crash' need to rely
>> on a best-guess method of determining value of 'MAX_PHYSMEM_BITS'
>> supported by underlying kernel.
>>
>> This value is used in user-space code to calculate the bit-space
>> required to store a section for SPARESMEM (similar to the existing
>> calculation method used in the kernel implementation):
>>
>> #define SECTIONS_SHIFT (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
>>
>> Now, regressions have been reported in user-space utilities
>> like 'makedumpfile' and 'crash' on arm64, with the recently added
>> kernel support for 52-bit physical address space, as there is
>> no clear method of determining this value in user-space
>> (other than reading kernel CONFIG flags).
>>
>> As per suggestion from makedumpfile maintainer (Kazu), it makes more
>> sense to append 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself
>> rather than in arch-specific code, so that the user-space code for other
>> archs can also benefit from this addition to the vmcoreinfo and use it
>> as a standard way of determining 'SECTIONS_SHIFT' value in user-land.
>>
>> A reference 'makedumpfile' implementation which reads the
>> 'MAX_PHYSMEM_BITS' value from vmcoreinfo in a arch-independent fashion
>> is available here:
>>
>> [0]. https://github.com/bhupesh-sharma/makedumpfile/blob/remove-max-phys-mem-bit-v1/arch/ppc64.c#L471
>>
>> Cc: Boris Petkov <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: James Morse <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Dave Anderson <[email protected]>
>> Cc: Kazuhito Hagio <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Bhupesh Sharma <[email protected]>
>> ---
>> kernel/crash_core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 093c9f917ed0..44b90368e183 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -467,6 +467,7 @@ static int __init crash_save_vmcoreinfo_init(void)
>> #define PAGE_OFFLINE_MAPCOUNT_VALUE (~PG_offline)
>> VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>> #endif
>> + VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
>
> Some architectures define MAX_PHYSMEM_BITS only with CONFIG_SPARSEMEM,
> so we need to move this to the #ifdef section that exports some
> mem_section things.
>
> Thanks!
> Kazu
Sorry for the late response, I wanted to make sure I check almost all
archs to understand if a proposal would work for all.
As per my current understanding, we can protect the export of
'MAX_PHYSMEM_BITS' via a #ifdef section against CONFIG_SPARSEMEM, and it
should work for all archs. Here are some arguments to support the same,
would request maintainers of various archs (in Cc) to correct me if I am
missing something here:
1. SPARSEMEM is dependent upon on (!SELECT_MEMORY_MODEL &&
ARCH_SPARSEMEM_ENABLE) || SPARSEMEM_MANUAL:
config SPARSEMEM
def_bool y
depends on (!SELECT_MEMORY_MODEL && ARCH_SPARSEMEM_ENABLE) ||
SPARSEMEM_MANUAL
2. For a couple of archs, this option is already turned on by default in
their respective defconfigs:
$ grep -nrw "CONFIG_SPARSEMEM_MANUAL" *
arch/ia64/configs/gensparse_defconfig:18:CONFIG_SPARSEMEM_MANUAL=y
arch/powerpc/configs/ppc64e_defconfig:30:CONFIG_SPARSEMEM_MANUAL=y
3. Note that other archs use ARCH_SPARSEMEM_DEFAULT to define if
CONFIG_SPARSEMEM_MANUAL is set by default:
choice
prompt "Memory model"
..
default SPARSEMEM_MANUAL if ARCH_SPARSEMEM_DEFAULT
3a.
$ grep -nrw -A 2 "ARCH_SPARSEMEM_DEFAULT" *
arch/s390/Kconfig:621:config ARCH_SPARSEMEM_DEFAULT
arch/s390/Kconfig-622- def_bool y
--
arch/x86/Kconfig:1623:config ARCH_SPARSEMEM_DEFAULT
arch/x86/Kconfig-1624- def_bool y
arch/x86/Kconfig-1625- depends on X86_64
--
arch/powerpc/Kconfig:614:config ARCH_SPARSEMEM_DEFAULT
arch/powerpc/Kconfig-615- def_bool y
arch/powerpc/Kconfig-616- depends on PPC_BOOK3S_64
--
arch/arm64/Kconfig:850:config ARCH_SPARSEMEM_DEFAULT
arch/arm64/Kconfig-851- def_bool ARCH_SPARSEMEM_ENABLE
--
arch/sh/mm/Kconfig:138:config ARCH_SPARSEMEM_DEFAULT
arch/sh/mm/Kconfig-139- def_bool y
--
arch/sparc/Kconfig:315:config ARCH_SPARSEMEM_DEFAULT
arch/sparc/Kconfig-316- def_bool y if SPARC64
--
arch/arm/Kconfig:1591:config ARCH_SPARSEMEM_DEFAULT
arch/arm/Kconfig-1592- def_bool ARCH_SPARSEMEM_ENABLE
Since most archs (except MIPS) set
CONFIG_ARCH_SPARSEMEM_DEFAULT/CONFIG_ARCH_SPARSEMEM_ENABLE to y in the
default configurations, so even though they don't protect
'MAX_PHYSMEM_BITS' define in CONFIG_SPARSEMEM ifdef sections, we still
would be ok protecting the 'MAX_PHYSMEM_BITS' vmcoreinfo export inside a
CONFIG_SPARSEMEM ifdef section.
Thanks for your inputs, I will include this change in the v3.
Regards,
Bhupesh