[v6,2/2] arm64/crash_core: Export TCR_EL1.T1SZ in vmcoreinfo

Commit Message

Bhupesh Sharma May 13, 2020, 6:52 p.m. UTC

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 <[e-mail]>
Cc: Mark Rutland <[e-mail]>
Cc: Will Deacon <[e-mail]>
Cc: Steve Capper <[e-mail]>
Cc: Catalin Marinas <[e-mail]>
Cc: Ard Biesheuvel <[e-mail]>
Cc: Dave Anderson <[e-mail]>
Cc: Kazuhito Hagio <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Tested-by: John Donnelly <[e-mail]>
Signed-off-by: Bhupesh Sharma <[e-mail]>
---
 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(+)

Comments

Kamlakant Patel June 3, 2020, 11:20 a.m. UTC | #1

Hi Bhupesh,

> -----Original Message-----
> From: kexec <[e-mail]> On Behalf Of Bhupesh
> Sharma
> Sent: Thursday, May 14, 2020 12:23 AM
> To: [e-mail]; [e-mail]
> Cc: Mark Rutland <[e-mail]>; Kazuhito Hagio <k-
> [e-mail]>; Steve Capper <[e-mail]>; Catalin
> Marinas <[e-mail]>; [e-mail]; Ard Biesheuvel
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; James Morse <[e-mail]>; Dave
> Anderson <[e-mail]>; [e-mail]; Will Deacon
> <[e-mail]>
> 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 <[e-mail]>
> Cc: Mark Rutland <[e-mail]>
> Cc: Will Deacon <[e-mail]>
> Cc: Steve Capper <[e-mail]>
> Cc: Catalin Marinas <[e-mail]>
> Cc: Ard Biesheuvel <[e-mail]>
> Cc: Dave Anderson <[e-mail]>
> Cc: Kazuhito Hagio <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Tested-by: John Donnelly <[e-mail]>
> Signed-off-by: Bhupesh Sharma <[e-mail]>
> ---
>  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
> [e-mail]
> 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=

Bhupesh Sharma June 3, 2020, 8:34 p.m. UTC | #2

Hi Kamlakant,

Many thanks for having a look at the patchset.

On Wed, Jun 3, 2020 at 4:50 PM Kamlakant Patel <[e-mail]> wrote:
>
> Hi Bhupesh,
>
> > -----Original Message-----
> > From: kexec <[e-mail]> On Behalf Of Bhupesh
> > Sharma
> > Sent: Thursday, May 14, 2020 12:23 AM
> > To: [e-mail]; [e-mail]
> > Cc: Mark Rutland <[e-mail]>; Kazuhito Hagio <k-
> > [e-mail]>; Steve Capper <[e-mail]>; Catalin
> > Marinas <[e-mail]>; [e-mail]; Ard Biesheuvel
> > <[e-mail]>; [e-mail]; linux-
> > [e-mail]; James Morse <[e-mail]>; Dave
> > Anderson <[e-mail]>; [e-mail]; Will Deacon
> > <[e-mail]>
> > 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 <[e-mail]>
> > Cc: Mark Rutland <[e-mail]>
> > Cc: Will Deacon <[e-mail]>
> > Cc: Steve Capper <[e-mail]>
> > Cc: Catalin Marinas <[e-mail]>
> > Cc: Ard Biesheuvel <[e-mail]>
> > Cc: Dave Anderson <[e-mail]>
> > Cc: Kazuhito Hagio <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > Cc: [e-mail]
> > Tested-by: John Donnelly <[e-mail]>
> > Signed-off-by: Bhupesh Sharma <[e-mail]>
> > ---
> >  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/[e-mail]/

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
> > [e-mail]
> > 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=
>

Kamlakant Patel June 4, 2020, 4:49 a.m. UTC | #3

Hi Bhupesh,

> -----Original Message-----
> From: Bhupesh Sharma <[e-mail]>
> Sent: Thursday, June 4, 2020 2:05 AM
> To: Kamlakant Patel <[e-mail]>
> Cc: [e-mail]; [e-mail]; Mark Rutland
> <[e-mail]>; Kazuhito Hagio <[e-mail]>; Steve
> Capper <[e-mail]>; Catalin Marinas
> <[e-mail]>; Ard Biesheuvel <[e-mail]>;
> [e-mail]; [e-mail]; James Morse
> <[e-mail]>; Dave Anderson <[e-mail]>;
> [e-mail]; Will Deacon <[e-mail]>; Ganapatrao Kulkarni
> <[e-mail]>
> 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 <[e-mail]>
> wrote:
> >
> > Hi Bhupesh,
> >
> > > -----Original Message-----
> > > From: kexec <[e-mail]> On Behalf Of Bhupesh
> > > Sharma
> > > Sent: Thursday, May 14, 2020 12:23 AM
> > > To: [e-mail]; [e-mail]
> > > Cc: Mark Rutland <[e-mail]>; Kazuhito Hagio <k-
> > > [e-mail]>; Steve Capper <[e-mail]>; Catalin
> > > Marinas <[e-mail]>; [e-mail]; Ard
> > > Biesheuvel <[e-mail]>; [e-mail];
> > > linux- [e-mail]; James Morse <[e-mail]>;
> > > Dave Anderson <[e-mail]>; [e-mail]; Will
> > > Deacon <[e-mail]>
> > > 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 <[e-mail]>
> > > Cc: Mark Rutland <[e-mail]>
> > > Cc: Will Deacon <[e-mail]>
> > > Cc: Steve Capper <[e-mail]>
> > > Cc: Catalin Marinas <[e-mail]>
> > > Cc: Ard Biesheuvel <[e-mail]>
> > > Cc: Dave Anderson <[e-mail]>
> > > Cc: Kazuhito Hagio <[e-mail]>
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > Tested-by: John Donnelly <[e-mail]>
> > > Signed-off-by: Bhupesh Sharma <[e-mail]>
> > > ---
> > >  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 <[e-mail]>

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
> > > [e-mail]
> > > 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=
> >

Will Deacon June 4, 2020, 7:19 a.m. UTC | #4

On Thu, Jun 04, 2020 at 02:04:58AM +0530, Bhupesh Sharma wrote:
> On Wed, Jun 3, 2020 at 4:50 PM Kamlakant Patel <[e-mail]> 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

Amit Daniel Kachhap July 1, 2020, 8:04 a.m. UTC | #5

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 <[e-mail]>
> Cc: Mark Rutland <[e-mail]>
> Cc: Will Deacon <[e-mail]>
> Cc: Steve Capper <[e-mail]>
> Cc: Catalin Marinas <[e-mail]>
> Cc: Ard Biesheuvel <[e-mail]>
> Cc: Dave Anderson <[e-mail]>
> Cc: Kazuhito Hagio <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Tested-by: John Donnelly <[e-mail]>
> Signed-off-by: Bhupesh Sharma <[e-mail]>

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 <[e-mail]>

Also the code changes/documentation looks fine to me with a minor 
comments below,

Reviewed-by: Amit Daniel Kachhap <[e-mail]>

> ---
>   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() ?
>

James Morse July 1, 2020, 11:59 a.m. UTC | #6

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 <[e-mail]>

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

Patch

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() ?