[Crash-utility] [PATCHv7 1/4] crash-utility/arm64: introduce a dedicated field to record the mem layout changes

[lijiang]

Hi, Pingfan
Thank you for the update.

On Fri, Jun 4, 2021 at 10:39 AM Pingfan Liu <piliu at redhat.com> wrote:
>
> At present, we have the following important changes for arm64 memory
> layout:
>
> -1. redesigned ARM64 kernel virtual memory layout and associated KASLR
>     support that was introduced in Linux 4.6. And NEW_VMEMMAP is used to
>     flag it.
> -2. memory layout flipped just right before introducing 52-bits kernel.
> -3. introducing of vabits_actual and phyvirt_offset in kernel
> -4. removing phyvirt_offset.
>
> These changes have effects on PTOV()/VTOP() formula. So introducing a
> dedicate field mmlayout_flags to record it.
>
> Among above, 2 and 3 are introduced closely, and are not distinguished
> in current implement. And this patch also keep this practice and use
> vabits_actual as a hint to flag mem flipped.
>
> Signed-off-by: Pingfan Liu <piliu at redhat.com>
> Cc: HAGIO KAZUHITO <k-hagio-ab at nec.com>
> Cc: Lianbo Jiang <lijiang at redhat.com>
> Cc: Bhupesh Sharma <bhupesh.sharma at linaro.org>
> To: crash-utility at redhat.com
> ---
>  arm64.c | 11 +++++++++++
>  defs.h  |  1 +
>  2 files changed, 12 insertions(+)
>
> diff --git a/arm64.c b/arm64.c
> index 8934961..eb88ced 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -88,6 +88,10 @@ static int arm64_is_uvaddr(ulong, struct task_context *);
>  static void arm64_calc_KERNELPACMASK(void);
>
>

I tested this patchset with the old vmcore and the latest vmcore, and
it passed. But I still have
some questions:

> +/* arm64 kernel layout experiences changes, using these flags to distinguish them */
> +#define MMLAYOUT_FLAGS_FLIP    0x1
[1] For this micro, is it possible to check the "vabits_actual" and
"TCR_EL1_T1SZ" in order to
achieve a similar purpose?

if (kernel_symbol_exists("vabits_actual") || ...
read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)"))

> +#define MMLAYOUT_FLAGS_HAS_PHYSVIRT_OFFSET     0x2
[2] For this one, can it be replaced with the following ways instead
of adding a new micro definition?

if (kernel_symbol_exists("physvirt_offset"))

Thanks.
Lianbo



> +
>  /*
>   * Do all necessary machine-specific setup here. This is called several times
>   * during initialization.
> @@ -994,6 +998,7 @@ arm64_calc_physvirt_offset(void)
>                 if (READMEM(pc->mfd, &physvirt_offset, sizeof(physvirt_offset),
>                         sp->value, sp->value -
>                         machdep->machspec->kimage_voffset) > 0) {
> +                               machdep->machspec->mmlayout_flags |= MMLAYOUT_FLAGS_HAS_PHYSVIRT_OFFSET;
>                                 ms->physvirt_offset = physvirt_offset;
>                 }
>         }
> @@ -3923,6 +3928,7 @@ arm64_calc_VA_BITS(void)
>         if (kernel_symbol_exists("vabits_actual")) {
>                 if (pc->flags & PROC_KCORE) {
>                         vabits_actual = symbol_value_from_proc_kallsyms("vabits_actual");
> +                       machdep->machspec->mmlayout_flags |= MMLAYOUT_FLAGS_FLIP;
>                         if ((vabits_actual != BADVAL) && (READMEM(pc->mfd, &value, sizeof(ulong),
>                             vabits_actual, KCORE_USE_VADDR) > 0)) {
>                                 if (CRASHDEBUG(1))
> @@ -3953,6 +3959,11 @@ arm64_calc_VA_BITS(void)
>                                 machdep->machspec->VA_BITS_ACTUAL = value;
>                                 machdep->machspec->VA_BITS = value;
>                                 machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> +                               /*
> +                                * The mm flip commit is introduced before 52-bits VA, which is before the
> +                                * commit to export NUMBER(TCR_EL1_T1SZ)
> +                                */
> +                               machdep->machspec->mmlayout_flags |= MMLAYOUT_FLAGS_FLIP;
>                         } else if (machdep->machspec->VA_BITS_ACTUAL) {
>                                 machdep->machspec->VA_BITS = machdep->machspec->VA_BITS_ACTUAL;
>                                 machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> diff --git a/defs.h b/defs.h
> index 396d61a..22b3cbd 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -3290,6 +3290,7 @@ struct arm64_pt_regs {
>  struct machine_specific {
>         ulong flags;
>         ulong userspace_top;
> +       ulong mmlayout_flags;
>         ulong page_offset;
>         ulong vmalloc_start_addr;
>         ulong vmalloc_end;
> --
> 2.29.2
>