RE: [PATCH v4 3/4] makedumpfile/arm64: Add support for ARMv8.2-LVA (52-bit kernel VA support)

2019-12-05 Thread Kazuhito Hagio
> -Original Message-
> > > > +/*
> > > > + * The linear kernel range starts at the bottom of the virtual address
> > > > + * space. Testing the top bit for the start of the region is a
> > > > + * sufficient check and avoids having to worry about the tag.
> > > > + */
> > > > +#define is_linear_addr(addr)   (!(((unsigned long)addr) & (1UL << 
> > > > (vabits_actual - 1
> > >
> > > Does this check cover 5.3 or earlier kernels?
> > > There is no case that vabits_actual is zero?
> 
> We can set vabits_actual as va_bits for older kernels. That shouldn't
> be a big change.
> Will add it in v5. See more below ...
> 
> > As you know, 14c127c957c1 ("arm64: mm: Flip kernel VA space") changed
> > the check for linear address:
> >
> > -#define __is_lm_address(addr)  (!!((addr) & BIT(VA_BITS - 1)))
> > +#define __is_lm_address(addr)  (!((addr) & BIT(VA_BITS - 1)))
> >
> > so if we use the same check as kernel has, I think we will need the
> > former one to support earlier kernels.
> 
> See above, we can use va_bits where vabits_actual is not present.

Yes, but it is not the problem that I wanted to say here.

The problem is that, even if we set vabits_actual to va_bits, we cannot
determine whether an address is in linear map range with just one macro
for 5.3 and 5.4 kernels.

Because the bit value to be checked by the macro changed:

5.3 VA_BITS=48
  linear map : 0x8000 to 0x
5.4 VA_BITS=48
  linear map : 0x to 0x7fff

or I missed something?

Thanks,
Kazu

> 
> > > > +
> > > >  static unsigned long long
> > > >  __pa(unsigned long vaddr)
> > > >  {
> > > > if (kimage_voffset == NOT_FOUND_NUMBER ||
> > > > -   (vaddr >= PAGE_OFFSET))
> > > > -   return (vaddr - PAGE_OFFSET + info->phys_base);
> > > > +   is_linear_addr(vaddr))
> > > > +   return (vaddr + info->phys_base - PAGE_OFFSET);
> > > > else
> > > > return (vaddr - kimage_voffset);
> > > >  }
> > > > @@ -253,6 +261,7 @@ static int calculate_plat_config(void)
> > > > (PAGESIZE() == SZ_64K && va_bits == 42)) {
> > > > pgtable_level = 2;
> > > > } else if ((PAGESIZE() == SZ_64K && va_bits == 48) ||
> > > > +   (PAGESIZE() == SZ_64K && va_bits == 52) ||
> > > > (PAGESIZE() == SZ_4K && va_bits == 39) ||
> > > > (PAGESIZE() == SZ_16K && va_bits == 47)) {
> > > > pgtable_level = 3;
> > > > @@ -287,6 +296,16 @@ get_phys_base_arm64(void)
> > > > return TRUE;
> > > > }
> > > >
> > > > +   /* If both vabits_actual and va_bits are now initialized, always
> > > > +* prefer vabits_actual over va_bits to calculate PAGE_OFFSET
> > > > +* value.
> > > > +*/
> > > > +   if (vabits_actual && va_bits && vabits_actual != va_bits) {
> > > > +   info->page_offset = (-(1UL << vabits_actual));
> > > > +   DEBUG_MSG("page_offset: %lx (via vabits_actual)\n",
> > > > +   info->page_offset);
> > > > +   }
> > > > +
> > >
> > > Is this for --mem-usage?
> > > If so, let's drop from this patch and think about it later because
> > > some additional base functions will be needed for the option, I think.
> 
> Ok.
> 
> > > > if (get_num_pt_loads() && PAGE_OFFSET) {
> > > > for (i = 0;
> > > > get_pt_load(i, _start, NULL, _start, NULL);
> > > > @@ -406,6 +425,73 @@ get_stext_symbol(void)
> > > > return(found ? kallsym : FALSE);
> > > >  }
> > > >
> > > > +static int
> > > > +get_va_bits_from_stext_arm64(void)
> > > > +{
> > > > +   ulong _stext;
> > > > +
> > > > +   _stext = get_stext_symbol();
> > > > +   if (!_stext) {
> > > > +   ERRMSG("Can't get the symbol of _stext.\n");
> > > > +   return FALSE;
> > > > +   }
> > > > +
> > > > +   /* Derive va_bits as per arch/arm64/Kconfig. Note that this is a
> > > > +* best case approximation at the moment, as there can be
> > > > +* inconsistencies in this calculation (for e.g., for
> > > > +* 52-bit kernel VA case, even the 48th bit might be set in
> > > > +* the _stext symbol).
> > > > +*
> > > > +* So, we need to rely on the actual VA_BITS symbol in the
> > > > +* vmcoreinfo for a accurate value.
> > > > +*
> > > > +* TODO: Improve this further once there is a closure with arm64
> > > > +* kernel maintainers on the same.
> > > > +*/
> > > > +   if ((_stext & PAGE_OFFSET_52) == PAGE_OFFSET_52) {
> > > > +   va_bits = 52;
> > > > +   } else if ((_stext & PAGE_OFFSET_48) == PAGE_OFFSET_48) {
> > > > +   va_bits = 48;
> > > > +   } else if ((_stext & PAGE_OFFSET_47) == PAGE_OFFSET_47) {
> > > > +   va_bits = 47;
> > > > +   } else if ((_stext & PAGE_OFFSET_42) == PAGE_OFFSET_42) {
> > > > +   va_bits = 42;
> > > > +   } else if ((_stext & PAGE_OFFSET_39) == PAGE_OFFSET_39) {
> > > > +   va_bits = 39;
> > > > + 

Re: [PATCH v4 3/4] makedumpfile/arm64: Add support for ARMv8.2-LVA (52-bit kernel VA support)

2019-12-05 Thread Bhupesh Sharma
Hi Kazu,

On Thu, Dec 5, 2019 at 9:00 PM Kazuhito Hagio  wrote:
>
> > -Original Message-
> > > -Original Message-
> > > With ARMv8.2-LVA architecture extension availability, arm64 hardware
> > > which supports this extension can support upto 52-bit virtual
> > > addresses. It is specially useful for having a 52-bit user-space virtual
> > > address space while the kernel can still retain 48-bit/52-bit virtual
> > > addressing.
> > >
> > > Since at the moment we enable the support of this extension in the
> > > kernel via a CONFIG flag (CONFIG_ARM64_VA_BITS_52), so there are
> > > no clear mechanisms in user-space to determine this CONFIG
> > > flag value and use it to determine the kernel-space VA address range
> > > values.
> > >
> > > 'makedumpfile' can instead use 'TCR_EL1.T1SZ' value from vmcoreinfo
> > > which indicates the size offset of the memory region addressed by
> > > TTBR1_EL1 (and hence can be used for determining the
> > > vabits_actual value).
> > >
> > > The user-space computation for determining whether an address lies in
> > > the linear map range is the same as we have in kernel-space:
> > >
> > >   #define __is_lm_address(addr) (!(((u64)addr) & BIT(vabits_actual - 
> > > 1)))
> > >
> > > I have sent a kernel patch upstream to add 'TCR_EL1.T1SZ' to
> > > vmcoreinfo for arm64 (see [0]).
> > >
> > > This patch is in accordance with ARMv8 Architecture Reference Manual
> > > version D.a
> > >
> > > Note that with these changes the '--mem-usage' option will not work
> > > properly for arm64 (a subsequent patch in this series will address the
> > > same) and there is a discussion on-going with the arm64 maintainers to
> > > find a way-out for the same (via standard kernel symbols like _stext).
> > >
> > > [0].http://lists.infradead.org/pipermail/kexec/2019-November/023962.html
> > >
> > > Cc: Kazuhito Hagio 
> > > Cc: John Donnelly 
> > > Cc: kexec@lists.infradead.org
> > > Signed-off-by: Bhupesh Sharma 
> > > ---
> > >  arch/arm64.c   | 148 
> > > +
> > >  makedumpfile.c |   2 +
> > >  makedumpfile.h |   3 +-
> > >  3 files changed, 122 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/arch/arm64.c b/arch/arm64.c
> > > index ecb19139e178..094d73b8a60f 100644
> > > --- a/arch/arm64.c
> > > +++ b/arch/arm64.c
> > > @@ -47,6 +47,7 @@ typedef struct {
> > >  static int lpa_52_bit_support_available;
> > >  static int pgtable_level;
> > >  static int va_bits;
> > > +static int vabits_actual;
> > >  static unsigned long kimage_voffset;
> > >
> > >  #define SZ_4K  4096
> > > @@ -218,12 +219,19 @@ pmd_page_paddr(pmd_t pmd)
> > >  #define pte_index(vaddr)   (((vaddr) >> PAGESHIFT()) & 
> > > (PTRS_PER_PTE - 1))
> > >  #define pte_offset(dir, vaddr) (pmd_page_paddr((*dir)) + 
> > > pte_index(vaddr) * sizeof(pte_t))
> > >
> > > +/*
> > > + * The linear kernel range starts at the bottom of the virtual address
> > > + * space. Testing the top bit for the start of the region is a
> > > + * sufficient check and avoids having to worry about the tag.
> > > + */
> > > +#define is_linear_addr(addr)   (!(((unsigned long)addr) & (1UL << 
> > > (vabits_actual - 1
> >
> > Does this check cover 5.3 or earlier kernels?
> > There is no case that vabits_actual is zero?

We can set vabits_actual as va_bits for older kernels. That shouldn't
be a big change.
Will add it in v5. See more below ...

> As you know, 14c127c957c1 ("arm64: mm: Flip kernel VA space") changed
> the check for linear address:
>
> -#define __is_lm_address(addr)  (!!((addr) & BIT(VA_BITS - 1)))
> +#define __is_lm_address(addr)  (!((addr) & BIT(VA_BITS - 1)))
>
> so if we use the same check as kernel has, I think we will need the
> former one to support earlier kernels.

See above, we can use va_bits where vabits_actual is not present.

> > > +
> > >  static unsigned long long
> > >  __pa(unsigned long vaddr)
> > >  {
> > > if (kimage_voffset == NOT_FOUND_NUMBER ||
> > > -   (vaddr >= PAGE_OFFSET))
> > > -   return (vaddr - PAGE_OFFSET + info->phys_base);
> > > +   is_linear_addr(vaddr))
> > > +   return (vaddr + info->phys_base - PAGE_OFFSET);
> > > else
> > > return (vaddr - kimage_voffset);
> > >  }
> > > @@ -253,6 +261,7 @@ static int calculate_plat_config(void)
> > > (PAGESIZE() == SZ_64K && va_bits == 42)) {
> > > pgtable_level = 2;
> > > } else if ((PAGESIZE() == SZ_64K && va_bits == 48) ||
> > > +   (PAGESIZE() == SZ_64K && va_bits == 52) ||
> > > (PAGESIZE() == SZ_4K && va_bits == 39) ||
> > > (PAGESIZE() == SZ_16K && va_bits == 47)) {
> > > pgtable_level = 3;
> > > @@ -287,6 +296,16 @@ get_phys_base_arm64(void)
> > > return TRUE;
> > > }
> > >
> > > +   /* If both vabits_actual and va_bits are now initialized, always
> > > +* 

RE: [PATCH v4 3/4] makedumpfile/arm64: Add support for ARMv8.2-LVA (52-bit kernel VA support)

2019-12-05 Thread Kazuhito Hagio
> -Original Message-
> > -Original Message-
> > With ARMv8.2-LVA architecture extension availability, arm64 hardware
> > which supports this extension can support upto 52-bit virtual
> > addresses. It is specially useful for having a 52-bit user-space virtual
> > address space while the kernel can still retain 48-bit/52-bit virtual
> > addressing.
> >
> > Since at the moment we enable the support of this extension in the
> > kernel via a CONFIG flag (CONFIG_ARM64_VA_BITS_52), so there are
> > no clear mechanisms in user-space to determine this CONFIG
> > flag value and use it to determine the kernel-space VA address range
> > values.
> >
> > 'makedumpfile' can instead use 'TCR_EL1.T1SZ' value from vmcoreinfo
> > which indicates the size offset of the memory region addressed by
> > TTBR1_EL1 (and hence can be used for determining the
> > vabits_actual value).
> >
> > The user-space computation for determining whether an address lies in
> > the linear map range is the same as we have in kernel-space:
> >
> >   #define __is_lm_address(addr) (!(((u64)addr) & BIT(vabits_actual - 
> > 1)))
> >
> > I have sent a kernel patch upstream to add 'TCR_EL1.T1SZ' to
> > vmcoreinfo for arm64 (see [0]).
> >
> > This patch is in accordance with ARMv8 Architecture Reference Manual
> > version D.a
> >
> > Note that with these changes the '--mem-usage' option will not work
> > properly for arm64 (a subsequent patch in this series will address the
> > same) and there is a discussion on-going with the arm64 maintainers to
> > find a way-out for the same (via standard kernel symbols like _stext).
> >
> > [0].http://lists.infradead.org/pipermail/kexec/2019-November/023962.html
> >
> > Cc: Kazuhito Hagio 
> > Cc: John Donnelly 
> > Cc: kexec@lists.infradead.org
> > Signed-off-by: Bhupesh Sharma 
> > ---
> >  arch/arm64.c   | 148 
> > +
> >  makedumpfile.c |   2 +
> >  makedumpfile.h |   3 +-
> >  3 files changed, 122 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/arm64.c b/arch/arm64.c
> > index ecb19139e178..094d73b8a60f 100644
> > --- a/arch/arm64.c
> > +++ b/arch/arm64.c
> > @@ -47,6 +47,7 @@ typedef struct {
> >  static int lpa_52_bit_support_available;
> >  static int pgtable_level;
> >  static int va_bits;
> > +static int vabits_actual;
> >  static unsigned long kimage_voffset;
> >
> >  #define SZ_4K  4096
> > @@ -218,12 +219,19 @@ pmd_page_paddr(pmd_t pmd)
> >  #define pte_index(vaddr)   (((vaddr) >> PAGESHIFT()) & 
> > (PTRS_PER_PTE - 1))
> >  #define pte_offset(dir, vaddr) (pmd_page_paddr((*dir)) + 
> > pte_index(vaddr) * sizeof(pte_t))
> >
> > +/*
> > + * The linear kernel range starts at the bottom of the virtual address
> > + * space. Testing the top bit for the start of the region is a
> > + * sufficient check and avoids having to worry about the tag.
> > + */
> > +#define is_linear_addr(addr)   (!(((unsigned long)addr) & (1UL << 
> > (vabits_actual - 1
> 
> Does this check cover 5.3 or earlier kernels?
> There is no case that vabits_actual is zero?

As you know, 14c127c957c1 ("arm64: mm: Flip kernel VA space") changed
the check for linear address:

-#define __is_lm_address(addr)  (!!((addr) & BIT(VA_BITS - 1)))
+#define __is_lm_address(addr)  (!((addr) & BIT(VA_BITS - 1)))

so if we use the same check as kernel has, I think we will need the
former one to support earlier kernels.

> 
> > +
> >  static unsigned long long
> >  __pa(unsigned long vaddr)
> >  {
> > if (kimage_voffset == NOT_FOUND_NUMBER ||
> > -   (vaddr >= PAGE_OFFSET))
> > -   return (vaddr - PAGE_OFFSET + info->phys_base);
> > +   is_linear_addr(vaddr))
> > +   return (vaddr + info->phys_base - PAGE_OFFSET);
> > else
> > return (vaddr - kimage_voffset);
> >  }
> > @@ -253,6 +261,7 @@ static int calculate_plat_config(void)
> > (PAGESIZE() == SZ_64K && va_bits == 42)) {
> > pgtable_level = 2;
> > } else if ((PAGESIZE() == SZ_64K && va_bits == 48) ||
> > +   (PAGESIZE() == SZ_64K && va_bits == 52) ||
> > (PAGESIZE() == SZ_4K && va_bits == 39) ||
> > (PAGESIZE() == SZ_16K && va_bits == 47)) {
> > pgtable_level = 3;
> > @@ -287,6 +296,16 @@ get_phys_base_arm64(void)
> > return TRUE;
> > }
> >
> > +   /* If both vabits_actual and va_bits are now initialized, always
> > +* prefer vabits_actual over va_bits to calculate PAGE_OFFSET
> > +* value.
> > +*/
> > +   if (vabits_actual && va_bits && vabits_actual != va_bits) {
> > +   info->page_offset = (-(1UL << vabits_actual));
> > +   DEBUG_MSG("page_offset: %lx (via vabits_actual)\n",
> > +   info->page_offset);
> > +   }
> > +
> 
> Is this for --mem-usage?
> If so, let's drop from this patch and think about it later because
> some 

RE: [PATCH v4 3/4] makedumpfile/arm64: Add support for ARMv8.2-LVA (52-bit kernel VA support)

2019-12-04 Thread Kazuhito Hagio
> -Original Message-
> With ARMv8.2-LVA architecture extension availability, arm64 hardware
> which supports this extension can support upto 52-bit virtual
> addresses. It is specially useful for having a 52-bit user-space virtual
> address space while the kernel can still retain 48-bit/52-bit virtual
> addressing.
> 
> Since at the moment we enable the support of this extension in the
> kernel via a CONFIG flag (CONFIG_ARM64_VA_BITS_52), so there are
> no clear mechanisms in user-space to determine this CONFIG
> flag value and use it to determine the kernel-space VA address range
> values.
> 
> 'makedumpfile' can instead use 'TCR_EL1.T1SZ' value from vmcoreinfo
> which indicates the size offset of the memory region addressed by
> TTBR1_EL1 (and hence can be used for determining the
> vabits_actual value).
> 
> The user-space computation for determining whether an address lies in
> the linear map range is the same as we have in kernel-space:
> 
>   #define __is_lm_address(addr)   (!(((u64)addr) & BIT(vabits_actual - 
> 1)))
> 
> I have sent a kernel patch upstream to add 'TCR_EL1.T1SZ' to
> vmcoreinfo for arm64 (see [0]).
> 
> This patch is in accordance with ARMv8 Architecture Reference Manual
> version D.a
> 
> Note that with these changes the '--mem-usage' option will not work
> properly for arm64 (a subsequent patch in this series will address the
> same) and there is a discussion on-going with the arm64 maintainers to
> find a way-out for the same (via standard kernel symbols like _stext).
> 
> [0].http://lists.infradead.org/pipermail/kexec/2019-November/023962.html
> 
> Cc: Kazuhito Hagio 
> Cc: John Donnelly 
> Cc: kexec@lists.infradead.org
> Signed-off-by: Bhupesh Sharma 
> ---
>  arch/arm64.c   | 148 
> +
>  makedumpfile.c |   2 +
>  makedumpfile.h |   3 +-
>  3 files changed, 122 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/arm64.c b/arch/arm64.c
> index ecb19139e178..094d73b8a60f 100644
> --- a/arch/arm64.c
> +++ b/arch/arm64.c
> @@ -47,6 +47,7 @@ typedef struct {
>  static int lpa_52_bit_support_available;
>  static int pgtable_level;
>  static int va_bits;
> +static int vabits_actual;
>  static unsigned long kimage_voffset;
> 
>  #define SZ_4K4096
> @@ -218,12 +219,19 @@ pmd_page_paddr(pmd_t pmd)
>  #define pte_index(vaddr) (((vaddr) >> PAGESHIFT()) & 
> (PTRS_PER_PTE - 1))
>  #define pte_offset(dir, vaddr)   (pmd_page_paddr((*dir)) + 
> pte_index(vaddr) * sizeof(pte_t))
> 
> +/*
> + * The linear kernel range starts at the bottom of the virtual address
> + * space. Testing the top bit for the start of the region is a
> + * sufficient check and avoids having to worry about the tag.
> + */
> +#define is_linear_addr(addr) (!(((unsigned long)addr) & (1UL << 
> (vabits_actual - 1

Does this check cover 5.3 or earlier kernels?
There is no case that vabits_actual is zero?

> +
>  static unsigned long long
>  __pa(unsigned long vaddr)
>  {
>   if (kimage_voffset == NOT_FOUND_NUMBER ||
> - (vaddr >= PAGE_OFFSET))
> - return (vaddr - PAGE_OFFSET + info->phys_base);
> + is_linear_addr(vaddr))
> + return (vaddr + info->phys_base - PAGE_OFFSET);
>   else
>   return (vaddr - kimage_voffset);
>  }
> @@ -253,6 +261,7 @@ static int calculate_plat_config(void)
>   (PAGESIZE() == SZ_64K && va_bits == 42)) {
>   pgtable_level = 2;
>   } else if ((PAGESIZE() == SZ_64K && va_bits == 48) ||
> + (PAGESIZE() == SZ_64K && va_bits == 52) ||
>   (PAGESIZE() == SZ_4K && va_bits == 39) ||
>   (PAGESIZE() == SZ_16K && va_bits == 47)) {
>   pgtable_level = 3;
> @@ -287,6 +296,16 @@ get_phys_base_arm64(void)
>   return TRUE;
>   }
> 
> + /* If both vabits_actual and va_bits are now initialized, always
> +  * prefer vabits_actual over va_bits to calculate PAGE_OFFSET
> +  * value.
> +  */
> + if (vabits_actual && va_bits && vabits_actual != va_bits) {
> + info->page_offset = (-(1UL << vabits_actual));
> + DEBUG_MSG("page_offset: %lx (via vabits_actual)\n",
> + info->page_offset);
> + }
> +

Is this for --mem-usage?
If so, let's drop from this patch and think about it later because
some additional base functions will be needed for the option, I think.

>   if (get_num_pt_loads() && PAGE_OFFSET) {
>   for (i = 0;
>   get_pt_load(i, _start, NULL, _start, NULL);
> @@ -406,6 +425,73 @@ get_stext_symbol(void)
>   return(found ? kallsym : FALSE);
>  }
> 
> +static int
> +get_va_bits_from_stext_arm64(void)
> +{
> + ulong _stext;
> +
> + _stext = get_stext_symbol();
> + if (!_stext) {
> + ERRMSG("Can't get the symbol of _stext.\n");
> + return FALSE;
> +