Apologies for the delayed update. Its been quite some time since I
posted the last version (v5), but I have been really caught up in some
other critical issues.
Changes since v5:
----------------
- v5 can be viewed here:
http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
- Addressed review comments from James Morse and Boris.
- Added Tested-by received from John on v5 patchset.
- Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
applied.
Changes since v4:
----------------
- v4 can be seen here:
http://lists.infradead.org/pipermail/kexec/2019-November/023961.html
- Addressed comments from Dave and added patches for documenting
new variables appended to vmcoreinfo documentation.
- Added testing report shared by Akashi for PATCH 2/5.
Changes since v3:
----------------
- v3 can be seen here:
http://lists.infradead.org/pipermail/kexec/2019-March/022590.html
- Addressed comments from James and exported TCR_EL1.T1SZ in vmcoreinfo
instead of PTRS_PER_PGD.
- Added a new patch (via [PATCH 3/3]), which fixes a simple typo in
'Documentation/arm64/memory.rst'
Changes since v2:
----------------
- v2 can be seen here:
http://lists.infradead.org/pipermail/kexec/2019-March/022531.html
- Protected 'MAX_PHYSMEM_BITS' vmcoreinfo variable under CONFIG_SPARSEMEM
ifdef sections, as suggested by Kazu.
- Updated vmcoreinfo documentation to add description about
'MAX_PHYSMEM_BITS' variable (via [PATCH 3/3]).
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 'vabits_actual' 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),
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: Boris Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: James Morse <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Steve Capper <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ard Biesheuvel <[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: John Donnelly <[email protected]>
Cc: [email protected]
Cc: Amit Kachhap <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Bhupesh Sharma (2):
crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
Documentation/admin-guide/kdump/vmcoreinfo.rst | 16 ++++++++++++++++
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/kernel/crash_core.c | 10 ++++++++++
kernel/crash_core.c | 1 +
4 files changed, 28 insertions(+)
--
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:
While at it also update vmcoreinfo documentation for 'MAX_PHYSMEM_BITS'
variable being added to vmcoreinfo.
'MAX_PHYSMEM_BITS' defines the maximum supported physical address
space memory.
Cc: Boris Petkov <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: James Morse <[email protected]>
Cc: Mark Rutland <[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]
Tested-by: John Donnelly <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/admin-guide/kdump/vmcoreinfo.rst | 5 +++++
kernel/crash_core.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
index e4ee8b2db604..2a632020f809 100644
--- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
+++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
@@ -93,6 +93,11 @@ It exists in the sparse memory mapping model, and it is also somewhat
similar to the mem_map variable, both of them are used to translate an
address.
+MAX_PHYSMEM_BITS
+----------------
+
+Defines the maximum supported physical address space memory.
+
page
----
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 9f1557b98468..18175687133a 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -413,6 +413,7 @@ static int __init crash_save_vmcoreinfo_init(void)
VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
VMCOREINFO_STRUCT_SIZE(mem_section);
VMCOREINFO_OFFSET(mem_section, section_mem_map);
+ VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
#endif
VMCOREINFO_STRUCT_SIZE(page);
VMCOREINFO_STRUCT_SIZE(pglist_data);
--
2.7.4
vabits_actual variable on arm64 indicates the actual VA space size,
and allows a single binary to support both 48-bit and 52-bit VA
spaces.
If the ARMv8.2-LVA optional feature is present, and we are running
with a 64KB page size; then it is possible to use 52-bits of address
space for both userspace and kernel addresses. However, any kernel
binary that supports 52-bit must also be able to fall back to 48-bit
at early boot time if the hardware feature is not present.
Since TCR_EL1.T1SZ indicates the size offset of the memory region
addressed by TTBR1_EL1 (and hence can be used for determining the
vabits_actual value) it makes more sense to export the same in
vmcoreinfo rather than vabits_actual variable, as the name of the
variable can change in future kernel versions, but the architectural
constructs like TCR_EL1.T1SZ can be used better to indicate intended
specific fields to user-space.
User-space utilities like makedumpfile and crash-utility, need to
read this value from vmcoreinfo for determining if a virtual
address lies in the linear map range.
While at it also add documentation for TCR_EL1.T1SZ variable being
added to vmcoreinfo.
It indicates the size offset of the memory region addressed by TTBR1_EL1
Cc: James Morse <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Steve Capper <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Dave Anderson <[email protected]>
Cc: Kazuhito Hagio <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Tested-by: John Donnelly <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/admin-guide/kdump/vmcoreinfo.rst | 11 +++++++++++
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/kernel/crash_core.c | 10 ++++++++++
3 files changed, 22 insertions(+)
diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
index 2a632020f809..2baad0bfb09d 100644
--- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
+++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
@@ -404,6 +404,17 @@ KERNELPACMASK
The mask to extract the Pointer Authentication Code from a kernel virtual
address.
+TCR_EL1.T1SZ
+------------
+
+Indicates the size offset of the memory region addressed by TTBR1_EL1.
+The region size is 2^(64-T1SZ) bytes.
+
+TTBR1_EL1 is the table base address register specified by ARMv8-A
+architecture which is used to lookup the page-tables for the Virtual
+addresses in the higher VA range (refer to ARMv8 ARM document for
+more details).
+
arm
===
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 6bf5e650da78..a1861af97ac9 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -216,6 +216,7 @@
#define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
#define TCR_TxSZ_WIDTH 6
#define TCR_T0SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T0SZ_OFFSET)
+#define TCR_T1SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T1SZ_OFFSET)
#define TCR_EPD0_SHIFT 7
#define TCR_EPD0_MASK (UL(1) << TCR_EPD0_SHIFT)
diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
index 1f646b07e3e9..314391a156ee 100644
--- a/arch/arm64/kernel/crash_core.c
+++ b/arch/arm64/kernel/crash_core.c
@@ -7,6 +7,14 @@
#include <linux/crash_core.h>
#include <asm/cpufeature.h>
#include <asm/memory.h>
+#include <asm/pgtable-hwdef.h>
+
+static inline u64 get_tcr_el1_t1sz(void);
+
+static inline u64 get_tcr_el1_t1sz(void)
+{
+ return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET;
+}
void arch_crash_save_vmcoreinfo(void)
{
@@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
kimage_voffset);
vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
PHYS_OFFSET);
+ vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
+ get_tcr_el1_t1sz());
vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
system_supports_address_auth() ?
--
2.7.4
Hello,
On Thu, May 14, 2020 at 12:22 AM Bhupesh Sharma <[email protected]> wrote:
>
> Apologies for the delayed update. Its been quite some time since I
> posted the last version (v5), but I have been really caught up in some
> other critical issues.
>
> Changes since v5:
> ----------------
> - v5 can be viewed here:
> http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
> - Addressed review comments from James Morse and Boris.
> - Added Tested-by received from John on v5 patchset.
> - Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
> patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
> applied.
>
> Changes since v4:
> ----------------
> - v4 can be seen here:
> http://lists.infradead.org/pipermail/kexec/2019-November/023961.html
> - Addressed comments from Dave and added patches for documenting
> new variables appended to vmcoreinfo documentation.
> - Added testing report shared by Akashi for PATCH 2/5.
>
> Changes since v3:
> ----------------
> - v3 can be seen here:
> http://lists.infradead.org/pipermail/kexec/2019-March/022590.html
> - Addressed comments from James and exported TCR_EL1.T1SZ in vmcoreinfo
> instead of PTRS_PER_PGD.
> - Added a new patch (via [PATCH 3/3]), which fixes a simple typo in
> 'Documentation/arm64/memory.rst'
>
> Changes since v2:
> ----------------
> - v2 can be seen here:
> http://lists.infradead.org/pipermail/kexec/2019-March/022531.html
> - Protected 'MAX_PHYSMEM_BITS' vmcoreinfo variable under CONFIG_SPARSEMEM
> ifdef sections, as suggested by Kazu.
> - Updated vmcoreinfo documentation to add description about
> 'MAX_PHYSMEM_BITS' variable (via [PATCH 3/3]).
>
> 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 'vabits_actual' 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),
> 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: Boris Petkov <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: James Morse <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Steve Capper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ard Biesheuvel <[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: John Donnelly <[email protected]>
> Cc: [email protected]
> Cc: Amit Kachhap <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Bhupesh Sharma (2):
> crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
> arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
>
> Documentation/admin-guide/kdump/vmcoreinfo.rst | 16 ++++++++++++++++
> arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> arch/arm64/kernel/crash_core.c | 10 ++++++++++
> kernel/crash_core.c | 1 +
> 4 files changed, 28 insertions(+)
Ping. @James Morse , Others
Please share if you have some comments regarding this patchset.
Thanks,
Bhupesh
Hi Bhupesh,
> -----Original Message-----
> From: kexec <[email protected]> On Behalf Of Bhupesh
> Sharma
> Sent: Thursday, May 14, 2020 12:23 AM
> To: [email protected]; [email protected]
> Cc: Mark Rutland <[email protected]>; Kazuhito Hagio <k-
> [email protected]>; Steve Capper <[email protected]>; Catalin
> Marinas <[email protected]>; [email protected]; Ard Biesheuvel
> <[email protected]>; [email protected]; linux-
> [email protected]; James Morse <[email protected]>; Dave
> Anderson <[email protected]>; [email protected]; Will Deacon
> <[email protected]>
> Subject: [PATCH v6 2/2] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
>
> vabits_actual variable on arm64 indicates the actual VA space size, and allows a
> single binary to support both 48-bit and 52-bit VA spaces.
>
> If the ARMv8.2-LVA optional feature is present, and we are running with a 64KB
> page size; then it is possible to use 52-bits of address space for both userspace
> and kernel addresses. However, any kernel binary that supports 52-bit must also
> be able to fall back to 48-bit at early boot time if the hardware feature is not
> present.
>
> Since TCR_EL1.T1SZ indicates the size offset of the memory region addressed by
> TTBR1_EL1 (and hence can be used for determining the vabits_actual value) it
> makes more sense to export the same in vmcoreinfo rather than vabits_actual
> variable, as the name of the variable can change in future kernel versions, but
> the architectural constructs like TCR_EL1.T1SZ can be used better to indicate
> intended specific fields to user-space.
>
> User-space utilities like makedumpfile and crash-utility, need to read this value
> from vmcoreinfo for determining if a virtual address lies in the linear map range.
>
> While at it also add documentation for TCR_EL1.T1SZ variable being added to
> vmcoreinfo.
>
> It indicates the size offset of the memory region addressed by TTBR1_EL1
>
> Cc: James Morse <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Steve Capper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Dave Anderson <[email protected]>
> Cc: Kazuhito Hagio <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Tested-by: John Donnelly <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/admin-guide/kdump/vmcoreinfo.rst | 11 +++++++++++
> arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> arch/arm64/kernel/crash_core.c | 10 ++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> index 2a632020f809..2baad0bfb09d 100644
> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> @@ -404,6 +404,17 @@ KERNELPACMASK
> The mask to extract the Pointer Authentication Code from a kernel virtual
> address.
>
> +TCR_EL1.T1SZ
> +------------
> +
> +Indicates the size offset of the memory region addressed by TTBR1_EL1.
> +The region size is 2^(64-T1SZ) bytes.
> +
> +TTBR1_EL1 is the table base address register specified by ARMv8-A
> +architecture which is used to lookup the page-tables for the Virtual
> +addresses in the higher VA range (refer to ARMv8 ARM document for more
> +details).
> +
> arm
> ===
>
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h
> b/arch/arm64/include/asm/pgtable-hwdef.h
> index 6bf5e650da78..a1861af97ac9 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -216,6 +216,7 @@
> #define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
> #define TCR_TxSZ_WIDTH 6
> #define TCR_T0SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> TCR_T0SZ_OFFSET)
> +#define TCR_T1SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> TCR_T1SZ_OFFSET)
>
> #define TCR_EPD0_SHIFT 7
> #define TCR_EPD0_MASK (UL(1) << TCR_EPD0_SHIFT)
> diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
> index 1f646b07e3e9..314391a156ee 100644
> --- a/arch/arm64/kernel/crash_core.c
> +++ b/arch/arm64/kernel/crash_core.c
> @@ -7,6 +7,14 @@
> #include <linux/crash_core.h>
> #include <asm/cpufeature.h>
> #include <asm/memory.h>
> +#include <asm/pgtable-hwdef.h>
> +
> +static inline u64 get_tcr_el1_t1sz(void);
> +
> +static inline u64 get_tcr_el1_t1sz(void) {
> + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET; }
>
> void arch_crash_save_vmcoreinfo(void)
> {
> @@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
> kimage_voffset);
> vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> PHYS_OFFSET);
> + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> + get_tcr_el1_t1sz());
I tested this patch on top of upstream kernel v5.7 and I am getting "crash: cannot determine VA_BITS_ACTUAL" error with crash tool.
I looked into crash-utility source and it is expecting tcr_el1_t1sz not TCR_EL1_T1SZ.
Could you please check.
Thanks,
Kamlakant Patel
> vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
>
> system_supports_address_auth() ?
> --
> 2.7.4
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__lists.infradead.org_mailman_listinfo_kexec&d=DwICAg&c=nKjWec2b6R0m
> OyPaz7xtfQ&r=XecQZQJWhG6-
> mN8sWxffFOgUXg4irGP3Sjuy6RxdacQ&m=oeLdIVaWScimdfEc4dNhRI0tT24IgzG
> 7LkpAE5P11JQ&s=LLjHpz349DuDtORX4xywCxzbGUOagoq4JXosStycqI4&e=
Hi Kamlakant,
Many thanks for having a look at the patchset.
On Wed, Jun 3, 2020 at 4:50 PM Kamlakant Patel <[email protected]> wrote:
>
> Hi Bhupesh,
>
> > -----Original Message-----
> > From: kexec <[email protected]> On Behalf Of Bhupesh
> > Sharma
> > Sent: Thursday, May 14, 2020 12:23 AM
> > To: [email protected]; [email protected]
> > Cc: Mark Rutland <[email protected]>; Kazuhito Hagio <k-
> > [email protected]>; Steve Capper <[email protected]>; Catalin
> > Marinas <[email protected]>; [email protected]; Ard Biesheuvel
> > <[email protected]>; [email protected]; linux-
> > [email protected]; James Morse <[email protected]>; Dave
> > Anderson <[email protected]>; [email protected]; Will Deacon
> > <[email protected]>
> > Subject: [PATCH v6 2/2] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
> >
> > vabits_actual variable on arm64 indicates the actual VA space size, and allows a
> > single binary to support both 48-bit and 52-bit VA spaces.
> >
> > If the ARMv8.2-LVA optional feature is present, and we are running with a 64KB
> > page size; then it is possible to use 52-bits of address space for both userspace
> > and kernel addresses. However, any kernel binary that supports 52-bit must also
> > be able to fall back to 48-bit at early boot time if the hardware feature is not
> > present.
> >
> > Since TCR_EL1.T1SZ indicates the size offset of the memory region addressed by
> > TTBR1_EL1 (and hence can be used for determining the vabits_actual value) it
> > makes more sense to export the same in vmcoreinfo rather than vabits_actual
> > variable, as the name of the variable can change in future kernel versions, but
> > the architectural constructs like TCR_EL1.T1SZ can be used better to indicate
> > intended specific fields to user-space.
> >
> > User-space utilities like makedumpfile and crash-utility, need to read this value
> > from vmcoreinfo for determining if a virtual address lies in the linear map range.
> >
> > While at it also add documentation for TCR_EL1.T1SZ variable being added to
> > vmcoreinfo.
> >
> > It indicates the size offset of the memory region addressed by TTBR1_EL1
> >
> > Cc: James Morse <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Steve Capper <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Ard Biesheuvel <[email protected]>
> > Cc: Dave Anderson <[email protected]>
> > Cc: Kazuhito Hagio <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Tested-by: John Donnelly <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > Documentation/admin-guide/kdump/vmcoreinfo.rst | 11 +++++++++++
> > arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> > arch/arm64/kernel/crash_core.c | 10 ++++++++++
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > index 2a632020f809..2baad0bfb09d 100644
> > --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > @@ -404,6 +404,17 @@ KERNELPACMASK
> > The mask to extract the Pointer Authentication Code from a kernel virtual
> > address.
> >
> > +TCR_EL1.T1SZ
> > +------------
> > +
> > +Indicates the size offset of the memory region addressed by TTBR1_EL1.
> > +The region size is 2^(64-T1SZ) bytes.
> > +
> > +TTBR1_EL1 is the table base address register specified by ARMv8-A
> > +architecture which is used to lookup the page-tables for the Virtual
> > +addresses in the higher VA range (refer to ARMv8 ARM document for more
> > +details).
> > +
> > arm
> > ===
> >
> > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h
> > b/arch/arm64/include/asm/pgtable-hwdef.h
> > index 6bf5e650da78..a1861af97ac9 100644
> > --- a/arch/arm64/include/asm/pgtable-hwdef.h
> > +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> > @@ -216,6 +216,7 @@
> > #define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
> > #define TCR_TxSZ_WIDTH 6
> > #define TCR_T0SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> > TCR_T0SZ_OFFSET)
> > +#define TCR_T1SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> > TCR_T1SZ_OFFSET)
> >
> > #define TCR_EPD0_SHIFT 7
> > #define TCR_EPD0_MASK (UL(1) << TCR_EPD0_SHIFT)
> > diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
> > index 1f646b07e3e9..314391a156ee 100644
> > --- a/arch/arm64/kernel/crash_core.c
> > +++ b/arch/arm64/kernel/crash_core.c
> > @@ -7,6 +7,14 @@
> > #include <linux/crash_core.h>
> > #include <asm/cpufeature.h>
> > #include <asm/memory.h>
> > +#include <asm/pgtable-hwdef.h>
> > +
> > +static inline u64 get_tcr_el1_t1sz(void);
> > +
> > +static inline u64 get_tcr_el1_t1sz(void) {
> > + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET; }
> >
> > void arch_crash_save_vmcoreinfo(void)
> > {
> > @@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
> > kimage_voffset);
> > vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> > PHYS_OFFSET);
> > + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> > + get_tcr_el1_t1sz());
> I tested this patch on top of upstream kernel v5.7 and I am getting "crash: cannot determine VA_BITS_ACTUAL" error with crash tool.
> I looked into crash-utility source and it is expecting tcr_el1_t1sz not TCR_EL1_T1SZ.
> Could you please check.
Indeed. As per James comments on the v5 (see [1]) where he suggested
converting ttcr_el1_t1sz into TCR_EL1_T1SZ, I made the change in v6
accordingly.
This time I haven't sent out the v6 userspace changes
(makedumpfile/crash-utility) upstream first, since we are waiting for
kernel changes to be accepted first, as we have seen in the past that
while the userspace patches have been accepted, the kernel patches
required a respin cycle, thus leading to inconsistencies, as you also
pointed out with crash-utility.
If you want, for your local testing, I can share my github branch
where I have kept the crash-utility v6 patchset ready. Please let me
know.
[1]. https://lore.kernel.org/linuxppc-dev/[email protected]/
Thanks,
Bhupesh
>
> Thanks,
> Kamlakant Patel
> > vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> > vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
> >
> > system_supports_address_auth() ?
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > kexec mailing list
> > [email protected]
> > https://urldefense.proofpoint.com/v2/url?u=http-
> > 3A__lists.infradead.org_mailman_listinfo_kexec&d=DwICAg&c=nKjWec2b6R0m
> > OyPaz7xtfQ&r=XecQZQJWhG6-
> > mN8sWxffFOgUXg4irGP3Sjuy6RxdacQ&m=oeLdIVaWScimdfEc4dNhRI0tT24IgzG
> > 7LkpAE5P11JQ&s=LLjHpz349DuDtORX4xywCxzbGUOagoq4JXosStycqI4&e=
>
Hi Bhupesh,
> -----Original Message-----
> From: Bhupesh Sharma <[email protected]>
> Sent: Thursday, June 4, 2020 2:05 AM
> To: Kamlakant Patel <[email protected]>
> Cc: [email protected]; [email protected]; Mark Rutland
> <[email protected]>; Kazuhito Hagio <[email protected]>; Steve
> Capper <[email protected]>; Catalin Marinas
> <[email protected]>; Ard Biesheuvel <[email protected]>;
> [email protected]; [email protected]; James Morse
> <[email protected]>; Dave Anderson <[email protected]>;
> [email protected]; Will Deacon <[email protected]>; Ganapatrao Kulkarni
> <[email protected]>
> Subject: [EXT] Re: [PATCH v6 2/2] arm64/crash_core: Export TCR_EL1.T1SZ in
> vmcoreinfo
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Kamlakant,
>
> Many thanks for having a look at the patchset.
>
> On Wed, Jun 3, 2020 at 4:50 PM Kamlakant Patel <[email protected]>
> wrote:
> >
> > Hi Bhupesh,
> >
> > > -----Original Message-----
> > > From: kexec <[email protected]> On Behalf Of Bhupesh
> > > Sharma
> > > Sent: Thursday, May 14, 2020 12:23 AM
> > > To: [email protected]; [email protected]
> > > Cc: Mark Rutland <[email protected]>; Kazuhito Hagio <k-
> > > [email protected]>; Steve Capper <[email protected]>; Catalin
> > > Marinas <[email protected]>; [email protected]; Ard
> > > Biesheuvel <[email protected]>; [email protected];
> > > linux- [email protected]; James Morse <[email protected]>;
> > > Dave Anderson <[email protected]>; [email protected]; Will
> > > Deacon <[email protected]>
> > > Subject: [PATCH v6 2/2] arm64/crash_core: Export TCR_EL1.T1SZ in
> > > vmcoreinfo
> > >
> > > vabits_actual variable on arm64 indicates the actual VA space size,
> > > and allows a single binary to support both 48-bit and 52-bit VA spaces.
> > >
> > > If the ARMv8.2-LVA optional feature is present, and we are running
> > > with a 64KB page size; then it is possible to use 52-bits of address
> > > space for both userspace and kernel addresses. However, any kernel
> > > binary that supports 52-bit must also be able to fall back to 48-bit
> > > at early boot time if the hardware feature is not present.
> > >
> > > Since TCR_EL1.T1SZ indicates the size offset of the memory region
> > > addressed by
> > > TTBR1_EL1 (and hence can be used for determining the vabits_actual
> > > value) it makes more sense to export the same in vmcoreinfo rather
> > > than vabits_actual variable, as the name of the variable can change
> > > in future kernel versions, but the architectural constructs like
> > > TCR_EL1.T1SZ can be used better to indicate intended specific fields to user-
> space.
> > >
> > > User-space utilities like makedumpfile and crash-utility, need to
> > > read this value from vmcoreinfo for determining if a virtual address lies in the
> linear map range.
> > >
> > > While at it also add documentation for TCR_EL1.T1SZ variable being
> > > added to vmcoreinfo.
> > >
> > > It indicates the size offset of the memory region addressed by
> > > TTBR1_EL1
> > >
> > > Cc: James Morse <[email protected]>
> > > Cc: Mark Rutland <[email protected]>
> > > Cc: Will Deacon <[email protected]>
> > > Cc: Steve Capper <[email protected]>
> > > Cc: Catalin Marinas <[email protected]>
> > > Cc: Ard Biesheuvel <[email protected]>
> > > Cc: Dave Anderson <[email protected]>
> > > Cc: Kazuhito Hagio <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Tested-by: John Donnelly <[email protected]>
> > > Signed-off-by: Bhupesh Sharma <[email protected]>
> > > ---
> > > Documentation/admin-guide/kdump/vmcoreinfo.rst | 11 +++++++++++
> > > arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> > > arch/arm64/kernel/crash_core.c | 10 ++++++++++
> > > 3 files changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > > b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > > index 2a632020f809..2baad0bfb09d 100644
> > > --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > > +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> > > @@ -404,6 +404,17 @@ KERNELPACMASK
> > > The mask to extract the Pointer Authentication Code from a kernel
> > > virtual address.
> > >
> > > +TCR_EL1.T1SZ
> > > +------------
> > > +
> > > +Indicates the size offset of the memory region addressed by TTBR1_EL1.
> > > +The region size is 2^(64-T1SZ) bytes.
> > > +
> > > +TTBR1_EL1 is the table base address register specified by ARMv8-A
> > > +architecture which is used to lookup the page-tables for the
> > > +Virtual addresses in the higher VA range (refer to ARMv8 ARM
> > > +document for more details).
> > > +
> > > arm
> > > ===
> > >
> > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h
> > > b/arch/arm64/include/asm/pgtable-hwdef.h
> > > index 6bf5e650da78..a1861af97ac9 100644
> > > --- a/arch/arm64/include/asm/pgtable-hwdef.h
> > > +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> > > @@ -216,6 +216,7 @@
> > > #define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
> > > #define TCR_TxSZ_WIDTH 6
> > > #define TCR_T0SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> > > TCR_T0SZ_OFFSET)
> > > +#define TCR_T1SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) <<
> > > TCR_T1SZ_OFFSET)
> > >
> > > #define TCR_EPD0_SHIFT 7
> > > #define TCR_EPD0_MASK (UL(1) << TCR_EPD0_SHIFT)
> > > diff --git a/arch/arm64/kernel/crash_core.c
> > > b/arch/arm64/kernel/crash_core.c index 1f646b07e3e9..314391a156ee
> > > 100644
> > > --- a/arch/arm64/kernel/crash_core.c
> > > +++ b/arch/arm64/kernel/crash_core.c
> > > @@ -7,6 +7,14 @@
> > > #include <linux/crash_core.h>
> > > #include <asm/cpufeature.h>
> > > #include <asm/memory.h>
> > > +#include <asm/pgtable-hwdef.h>
> > > +
> > > +static inline u64 get_tcr_el1_t1sz(void);
> > > +
> > > +static inline u64 get_tcr_el1_t1sz(void) {
> > > + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >>
> > > +TCR_T1SZ_OFFSET; }
> > >
> > > void arch_crash_save_vmcoreinfo(void) { @@ -16,6 +24,8 @@ void
> > > arch_crash_save_vmcoreinfo(void)
> > > kimage_voffset);
> > > vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> > > PHYS_OFFSET);
> > > + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> > > + get_tcr_el1_t1sz());
> > I tested this patch on top of upstream kernel v5.7 and I am getting "crash:
> cannot determine VA_BITS_ACTUAL" error with crash tool.
> > I looked into crash-utility source and it is expecting tcr_el1_t1sz not
> TCR_EL1_T1SZ.
> > Could you please check.
>
> Indeed. As per James comments on the v5 (see [1]) where he suggested
> converting ttcr_el1_t1sz into TCR_EL1_T1SZ, I made the change in v6
> accordingly.
>
> This time I haven't sent out the v6 userspace changes
> (makedumpfile/crash-utility) upstream first, since we are waiting for kernel
> changes to be accepted first, as we have seen in the past that while the
> userspace patches have been accepted, the kernel patches required a respin
> cycle, thus leading to inconsistencies, as you also pointed out with crash-utility.
>
> If you want, for your local testing, I can share my github branch where I have
> kept the crash-utility v6 patchset ready. Please let me know.
>
> [1]. https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_linuxppc-2Ddev_63d6e63c-2D7218-2Dd2dd-2D8767-
> 2D4464be83603f-
> 40arm.com_&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=XecQZQJWhG6-
> mN8sWxffFOgUXg4irGP3Sjuy6RxdacQ&m=ijR8vEafG_QGTKYX2oI-
> SvfFsY4pPou6tvtrnxRoloo&s=zJmo3qbm2XfnKbrUqJPNN5o6PJqER9OzltwgS4aTa
> -k&e=
Thanks for clarifying.
I made userspace changes accordingly and tested and it works fine. We will be wait for your userspace patch.
Tested-by: Kamlakant Patel <[email protected]>
Thanks,
Kamlakant Patel
>
> Thanks,
> Bhupesh
>
>
> >
> > Thanks,
> > Kamlakant Patel
> > > vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> > > vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
> > >
> > > system_supports_address_auth() ?
> > > --
> > > 2.7.4
> > >
> > >
> > > _______________________________________________
> > > kexec mailing list
> > > [email protected]
> > > https://urldefense.proofpoint.com/v2/url?u=http-
> > > 3A__lists.infradead.org_mailman_listinfo_kexec&d=DwICAg&c=nKjWec2b6R
> > > 0m
> > > OyPaz7xtfQ&r=XecQZQJWhG6-
> > >
> mN8sWxffFOgUXg4irGP3Sjuy6RxdacQ&m=oeLdIVaWScimdfEc4dNhRI0tT24IgzG
> > > 7LkpAE5P11JQ&s=LLjHpz349DuDtORX4xywCxzbGUOagoq4JXosStycqI4&e=
> >
On Thu, Jun 04, 2020 at 02:04:58AM +0530, Bhupesh Sharma wrote:
> On Wed, Jun 3, 2020 at 4:50 PM Kamlakant Patel <[email protected]> wrote:
> > > diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
> > > index 1f646b07e3e9..314391a156ee 100644
> > > --- a/arch/arm64/kernel/crash_core.c
> > > +++ b/arch/arm64/kernel/crash_core.c
> > > @@ -7,6 +7,14 @@
> > > #include <linux/crash_core.h>
> > > #include <asm/cpufeature.h>
> > > #include <asm/memory.h>
> > > +#include <asm/pgtable-hwdef.h>
> > > +
> > > +static inline u64 get_tcr_el1_t1sz(void);
> > > +
> > > +static inline u64 get_tcr_el1_t1sz(void) {
> > > + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET; }
> > >
> > > void arch_crash_save_vmcoreinfo(void)
> > > {
> > > @@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
> > > kimage_voffset);
> > > vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> > > PHYS_OFFSET);
> > > + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> > > + get_tcr_el1_t1sz());
> > I tested this patch on top of upstream kernel v5.7 and I am getting "crash: cannot determine VA_BITS_ACTUAL" error with crash tool.
> > I looked into crash-utility source and it is expecting tcr_el1_t1sz not TCR_EL1_T1SZ.
> > Could you please check.
>
> Indeed. As per James comments on the v5 (see [1]) where he suggested
> converting ttcr_el1_t1sz into TCR_EL1_T1SZ, I made the change in v6
> accordingly.
>
> This time I haven't sent out the v6 userspace changes
> (makedumpfile/crash-utility) upstream first, since we are waiting for
> kernel changes to be accepted first, as we have seen in the past that
> while the userspace patches have been accepted, the kernel patches
> required a respin cycle, thus leading to inconsistencies, as you also
> pointed out with crash-utility.
Yes, and that';s the right way to do it. Userspace can't rely on the
stability of a kernel interface if it's no in the upstream kernel!
So doing with the ALL CAPS names is the right thing to do.
Will
Hello Catalin, Will,
On Tue, Jun 2, 2020 at 10:54 AM Bhupesh Sharma <[email protected]> wrote:
>
> Hello,
>
> On Thu, May 14, 2020 at 12:22 AM Bhupesh Sharma <[email protected]> wrote:
> >
> > Apologies for the delayed update. Its been quite some time since I
> > posted the last version (v5), but I have been really caught up in some
> > other critical issues.
> >
> > Changes since v5:
> > ----------------
> > - v5 can be viewed here:
> > http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
> > - Addressed review comments from James Morse and Boris.
> > - Added Tested-by received from John on v5 patchset.
> > - Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
> > patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
> > applied.
> >
> > Changes since v4:
> > ----------------
> > - v4 can be seen here:
> > http://lists.infradead.org/pipermail/kexec/2019-November/023961.html
> > - Addressed comments from Dave and added patches for documenting
> > new variables appended to vmcoreinfo documentation.
> > - Added testing report shared by Akashi for PATCH 2/5.
> >
> > Changes since v3:
> > ----------------
> > - v3 can be seen here:
> > http://lists.infradead.org/pipermail/kexec/2019-March/022590.html
> > - Addressed comments from James and exported TCR_EL1.T1SZ in vmcoreinfo
> > instead of PTRS_PER_PGD.
> > - Added a new patch (via [PATCH 3/3]), which fixes a simple typo in
> > 'Documentation/arm64/memory.rst'
> >
> > Changes since v2:
> > ----------------
> > - v2 can be seen here:
> > http://lists.infradead.org/pipermail/kexec/2019-March/022531.html
> > - Protected 'MAX_PHYSMEM_BITS' vmcoreinfo variable under CONFIG_SPARSEMEM
> > ifdef sections, as suggested by Kazu.
> > - Updated vmcoreinfo documentation to add description about
> > 'MAX_PHYSMEM_BITS' variable (via [PATCH 3/3]).
> >
> > 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 'vabits_actual' 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),
> > 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: Boris Petkov <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: James Morse <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Steve Capper <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Cc: Ard Biesheuvel <[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: John Donnelly <[email protected]>
> > Cc: [email protected]
> > Cc: Amit Kachhap <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Bhupesh Sharma (2):
> > crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
> > arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
> >
> > Documentation/admin-guide/kdump/vmcoreinfo.rst | 16 ++++++++++++++++
> > arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> > arch/arm64/kernel/crash_core.c | 10 ++++++++++
> > kernel/crash_core.c | 1 +
> > 4 files changed, 28 insertions(+)
>
> Ping. @James Morse , Others
>
> Please share if you have some comments regarding this patchset.
Ping. I think we have two Tested-by available from Oracle and Marvell
folks on this patchset and no further review-comments.
Can you please help review/pick this patchset via the arm64 tree?
User-space utilities like makedumpfile and crash have been broken
since 52-bit VA addressing was enabled on arm64 kernel, so distros are
obliged to carry downstream-only fixes for these user-space utilities
to make them work with the kernel which support 52-bit VA addressing
on arm64.
Thanks,
Bhupesh
Hi Bhupesh,
On 5/14/20 12:22 AM, Bhupesh Sharma wrote:
> vabits_actual variable on arm64 indicates the actual VA space size,
> and allows a single binary to support both 48-bit and 52-bit VA
> spaces.
>
> If the ARMv8.2-LVA optional feature is present, and we are running
> with a 64KB page size; then it is possible to use 52-bits of address
> space for both userspace and kernel addresses. However, any kernel
> binary that supports 52-bit must also be able to fall back to 48-bit
> at early boot time if the hardware feature is not present.
>
> Since TCR_EL1.T1SZ indicates the size offset of the memory region
> addressed by TTBR1_EL1 (and hence can be used for determining the
> vabits_actual value) it makes more sense to export the same in
> vmcoreinfo rather than vabits_actual variable, as the name of the
> variable can change in future kernel versions, but the architectural
> constructs like TCR_EL1.T1SZ can be used better to indicate intended
> specific fields to user-space.
>
> User-space utilities like makedumpfile and crash-utility, need to
> read this value from vmcoreinfo for determining if a virtual
> address lies in the linear map range.
>
> While at it also add documentation for TCR_EL1.T1SZ variable being
> added to vmcoreinfo.
>
> It indicates the size offset of the memory region addressed by TTBR1_EL1
>
> Cc: James Morse <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Steve Capper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Dave Anderson <[email protected]>
> Cc: Kazuhito Hagio <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Tested-by: John Donnelly <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
I tested this for both 48 and 52 VA. The dump log looks fine with the
crash tool link provided by you so,
Tested-by: Amit Daniel Kachhap <[email protected]>
Also the code changes/documentation looks fine to me with a minor
comments below,
Reviewed-by: Amit Daniel Kachhap <[email protected]>
> ---
> Documentation/admin-guide/kdump/vmcoreinfo.rst | 11 +++++++++++
> arch/arm64/include/asm/pgtable-hwdef.h | 1 +
> arch/arm64/kernel/crash_core.c | 10 ++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> index 2a632020f809..2baad0bfb09d 100644
> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> @@ -404,6 +404,17 @@ KERNELPACMASK
> The mask to extract the Pointer Authentication Code from a kernel virtual
> address.
>
> +TCR_EL1.T1SZ
> +------------
> +
> +Indicates the size offset of the memory region addressed by TTBR1_EL1.
> +The region size is 2^(64-T1SZ) bytes.
> +
> +TTBR1_EL1 is the table base address register specified by ARMv8-A
> +architecture which is used to lookup the page-tables for the Virtual
> +addresses in the higher VA range (refer to ARMv8 ARM document for
> +more details).
> +
> arm
> ===
>
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index 6bf5e650da78..a1861af97ac9 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -216,6 +216,7 @@
> #define TCR_TxSZ(x) (TCR_T0SZ(x) | TCR_T1SZ(x))
> #define TCR_TxSZ_WIDTH 6
> #define TCR_T0SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T0SZ_OFFSET)
> +#define TCR_T1SZ_MASK (((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T1SZ_OFFSET)
>
> #define TCR_EPD0_SHIFT 7
> #define TCR_EPD0_MASK (UL(1) << TCR_EPD0_SHIFT)
> diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
> index 1f646b07e3e9..314391a156ee 100644
> --- a/arch/arm64/kernel/crash_core.c
> +++ b/arch/arm64/kernel/crash_core.c
> @@ -7,6 +7,14 @@
> #include <linux/crash_core.h>
> #include <asm/cpufeature.h>
> #include <asm/memory.h>
> +#include <asm/pgtable-hwdef.h>
Nit: May be you forgot to include <asm/sysreg.h> here as suggested by
James in v5.
Cheers,
Amit
> +
> +static inline u64 get_tcr_el1_t1sz(void);
> +
> +static inline u64 get_tcr_el1_t1sz(void)
> +{
> + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET;
> +}
>
> void arch_crash_save_vmcoreinfo(void)
> {
> @@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
> kimage_voffset);
> vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> PHYS_OFFSET);
> + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> + get_tcr_el1_t1sz());
> vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
> system_supports_address_auth() ?
>
Hi Bhupesh,
On 13/05/2020 19:52, Bhupesh Sharma wrote:
> vabits_actual variable on arm64 indicates the actual VA space size,
> and allows a single binary to support both 48-bit and 52-bit VA
> spaces.
I'd prefer the commit message not to refer to this 'vabits_actual' thing at all. By the
time a git-archaeologist comes to read this, it may be long gone.
Ideally this would refer to: TCR_EL1.TxSZ, which controls the VA space size,
and can be configured by a single kernel image to support either 48-bit or 52-bit VA space.
> If the ARMv8.2-LVA optional feature is present, and we are running
> with a 64KB page size; then it is possible to use 52-bits of address
> space for both userspace and kernel addresses. However, any kernel
> binary that supports 52-bit must also be able to fall back to 48-bit
> at early boot time if the hardware feature is not present.
>
> Since TCR_EL1.T1SZ indicates the size offset of the memory region
> addressed by TTBR1_EL1 (and hence can be used for determining the
> vabits_actual value) it makes more sense to export the same in
> vmcoreinfo rather than vabits_actual variable, as the name of the
> variable can change in future kernel versions, but the architectural
> constructs like TCR_EL1.T1SZ can be used better to indicate intended
> specific fields to user-space.
>
> User-space utilities like makedumpfile and crash-utility, need to
> read this value from vmcoreinfo for determining if a virtual
> address lies in the linear map range.
>
> While at it also add documentation for TCR_EL1.T1SZ variable being
> added to vmcoreinfo.
>
> It indicates the size offset of the memory region addressed by TTBR1_EL1
> diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/crash_core.c
> index 1f646b07e3e9..314391a156ee 100644
> --- a/arch/arm64/kernel/crash_core.c
> +++ b/arch/arm64/kernel/crash_core.c
> @@ -7,6 +7,14 @@
> #include <linux/crash_core.h>
> #include <asm/cpufeature.h>
> #include <asm/memory.h>
> +#include <asm/pgtable-hwdef.h>
> +
> +static inline u64 get_tcr_el1_t1sz(void);
> +
> +static inline u64 get_tcr_el1_t1sz(void)
> +{
> + return (read_sysreg(tcr_el1) & TCR_T1SZ_MASK) >> TCR_T1SZ_OFFSET;
> +}
>
> void arch_crash_save_vmcoreinfo(void)
> {
> @@ -16,6 +24,8 @@ void arch_crash_save_vmcoreinfo(void)
> kimage_voffset);
> vmcoreinfo_append_str("NUMBER(PHYS_OFFSET)=0x%llx\n",
> PHYS_OFFSET);
> + vmcoreinfo_append_str("NUMBER(TCR_EL1_T1SZ)=0x%llx\n",
> + get_tcr_el1_t1sz());
> vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
> system_supports_address_auth() ?
(I think second guessing the kernel memory map is a sisyphean effort), but this register
isn't going to disappear, or change its meaning!:
Reviewed-by: James Morse <[email protected]>
You may need to re-post this to get the maintainer's attention as its normally safe to
assume patches posted before rc1 no longer apply. (in this case, this one does)
Thanks,
James
On Thu, May 14, 2020 at 12:22:36AM +0530, Bhupesh Sharma wrote:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 9f1557b98468..18175687133a 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -413,6 +413,7 @@ static int __init crash_save_vmcoreinfo_init(void)
> VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
> VMCOREINFO_STRUCT_SIZE(mem_section);
> VMCOREINFO_OFFSET(mem_section, section_mem_map);
> + VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
> #endif
> VMCOREINFO_STRUCT_SIZE(page);
> VMCOREINFO_STRUCT_SIZE(pglist_data);
I can queue this patch via the arm64 tree (together with the second one)
but I'd like an ack from the kernel/crash_core.c maintainers. They don't
seem to have been cc'ed either (only the kexec list).
Thanks.
--
Catalin
Hi Catalin,
On 07/02/20 at 12:00pm, Catalin Marinas wrote:
> On Thu, May 14, 2020 at 12:22:36AM +0530, Bhupesh Sharma wrote:
> > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> > index 9f1557b98468..18175687133a 100644
> > --- a/kernel/crash_core.c
> > +++ b/kernel/crash_core.c
> > @@ -413,6 +413,7 @@ static int __init crash_save_vmcoreinfo_init(void)
> > VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
> > VMCOREINFO_STRUCT_SIZE(mem_section);
> > VMCOREINFO_OFFSET(mem_section, section_mem_map);
> > + VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
> > #endif
> > VMCOREINFO_STRUCT_SIZE(page);
> > VMCOREINFO_STRUCT_SIZE(pglist_data);
>
> I can queue this patch via the arm64 tree (together with the second one)
> but I'd like an ack from the kernel/crash_core.c maintainers. They don't
> seem to have been cc'ed either (only the kexec list).
For the VMCOREINFO part, I'm fine with the changes, but since I do not
understand the arm64 pieces so I would like to leave to arm64 people to
review. If arm64 bits are good enough, feel free to add:
Acked-by: Dave Young <[email protected]>
Thanks
Dave
On Thu, Jul 02, 2020 at 08:08:55PM +0800, Dave Young wrote:
> Hi Catalin,
> On 07/02/20 at 12:00pm, Catalin Marinas wrote:
> > On Thu, May 14, 2020 at 12:22:36AM +0530, Bhupesh Sharma wrote:
> > > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> > > index 9f1557b98468..18175687133a 100644
> > > --- a/kernel/crash_core.c
> > > +++ b/kernel/crash_core.c
> > > @@ -413,6 +413,7 @@ static int __init crash_save_vmcoreinfo_init(void)
> > > VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
> > > VMCOREINFO_STRUCT_SIZE(mem_section);
> > > VMCOREINFO_OFFSET(mem_section, section_mem_map);
> > > + VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
> > > #endif
> > > VMCOREINFO_STRUCT_SIZE(page);
> > > VMCOREINFO_STRUCT_SIZE(pglist_data);
> >
> > I can queue this patch via the arm64 tree (together with the second one)
> > but I'd like an ack from the kernel/crash_core.c maintainers. They don't
> > seem to have been cc'ed either (only the kexec list).
>
> For the VMCOREINFO part, I'm fine with the changes, but since I do not
> understand the arm64 pieces so I would like to leave to arm64 people to
> review. If arm64 bits are good enough, feel free to add:
>
> Acked-by: Dave Young <[email protected]>
Thanks.
--
Catalin
On Thu, 14 May 2020 00:22:35 +0530, Bhupesh Sharma wrote:
> Apologies for the delayed update. Its been quite some time since I
> posted the last version (v5), but I have been really caught up in some
> other critical issues.
>
> Changes since v5:
> ----------------
> - v5 can be viewed here:
> http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
> - Addressed review comments from James Morse and Boris.
> - Added Tested-by received from John on v5 patchset.
> - Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
> patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
> applied.
>
> [...]
Applied to arm64 (for-next/vmcoreinfo), thanks!
[1/2] crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
https://git.kernel.org/arm64/c/1d50e5d0c505
[2/2] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
https://git.kernel.org/arm64/c/bbdbc11804ff
--
Catalin
On Thu, Jul 2, 2020 at 10:45 PM Catalin Marinas <[email protected]> wrote:
>
> On Thu, 14 May 2020 00:22:35 +0530, Bhupesh Sharma wrote:
> > Apologies for the delayed update. Its been quite some time since I
> > posted the last version (v5), but I have been really caught up in some
> > other critical issues.
> >
> > Changes since v5:
> > ----------------
> > - v5 can be viewed here:
> > http://lists.infradead.org/pipermail/kexec/2019-November/024055.html
> > - Addressed review comments from James Morse and Boris.
> > - Added Tested-by received from John on v5 patchset.
> > - Rebased against arm64 (for-next/ptr-auth) branch which has Amit's
> > patchset for ARMv8.3-A Pointer Authentication feature vmcoreinfo
> > applied.
> >
> > [...]
>
> Applied to arm64 (for-next/vmcoreinfo), thanks!
>
> [1/2] crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
> https://git.kernel.org/arm64/c/1d50e5d0c505
> [2/2] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo
> https://git.kernel.org/arm64/c/bbdbc11804ff
Thanks Catalin for pulling in the changes.
Dave and James, many thanks for reviewing the same as well.
Regards,
Bhupesh