On Mon, Oct 25, 2021 at 4:22 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
Hi Lianbo,
-----Original Message-----
> On Mon, Oct 18, 2021 at 3:35 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab(a)nec.com> wrote:
> >
> > -----Original Message-----
> > > On Thu, Oct 14, 2021 at 11:17 AM HAGIO KAZUHITO(萩尾 一仁)
> > > <k-hagio-ab(a)nec.com> wrote:
> > > >
> > > > Hi Ankur,
> > > >
> > > > -----Original Message-----
> > > > > >> assign page_offset and kvbase based on VA_BITS
passed
> > > > > >>
> > > > >
> > > > > >Thank you for the patch, Ankur.
> > > > > >Can you help to describe the reason in detail? And
what happened?
> > > > >
> > > > >
> > > > >
> > > > > Hi Lianbo, Raw ramdump without vmcoreinfo doesn't work
here.
> > > > > read_vmcoreinfo("NUMBER(BA_BITS)") will not work on
raw ramdump.
> > > > > getting below error
> > > > >
> > >
> > > Thank you for the information, Ankur.
> > >
> > > > > crash_64: vmlinux and /var/tmp/ramdump_elf_XUtCMT do not match!
> > > > > Usage:
> > > > > crash [OPTION]... NAMELIST MEMORY-IMAGE[@address
<
https://github.com/address> ] (dumpfile form)
> > > > > crash [OPTION]... [NAMELIST] (live system form)
> > > > > Enter "crash_64 -h" for details.
> > > > >
> > > > > Recent change
>
https://github.com/crash-utility/crash/commit/167d37e347fe35c6f7db826e853...
> > > is
> > > > > causing this issue, before this change VA_BITS_ACTUAL was used
which is passed from cmd line.
> > > >
> > > > ok, I see, but I'm not sure why your patch uses the
(pc->flags2 & SNAP) condition.
> > >
> > > This seems not very reasonable, the SNAP flag has a specific use.
> > >
> > > > The calculation used VA_BITS_ACTUAL (== VA_BITS for ramdump) before
the change,
> > > > so how about the following patch?
> > > >./crash
> > > > diff --git a/arm64.c b/arm64.c
> > > > index 7069312671cf..da78acbb013f 100644
> > > > --- a/arm64.c
> > > > +++ b/arm64.c
> > > > @@ -4031,6 +4031,9 @@ arm64_calc_VA_BITS(void)
> > > > error(FATAL, "cannot determine
VA_BITS_ACTUAL\n");
> > > > }
> > > >
> > > > + if (!machdep->machspec->CONFIG_ARM64_VA_BITS)
/* guess */
> > > > + machdep->machspec->CONFIG_ARM64_VA_BITS
= machedp->machspec->VA_BITS;
> > > > +
> > >
> > > Looks good.
> >
> > Thanks, but I'm rethinking about their meanings.
> >
> > The VA_BITS is a macro replaced with CONFIG_ARM64_VA_BITS in the kernel
> > and used to calculate PAGE_OFFSET.
> >
> > #define VA_BITS (CONFIG_ARM64_VA_BITS)
> > #define _PAGE_OFFSET(va) (-(UL(1) << (va)))
> > #define PAGE_OFFSET (_PAGE_OFFSET(VA_BITS))
> >
> > It looks like we should set ms->CONFIG_ARM64_VA_BITS only for the value
> > got from the kernel config or vmcoreinfo. On the other hand, ms->VA_BITS
> > can be calculated from other information in crash.
> >
> Sounds good to me.
>
> > So should we use ms->VA_BITS for ARM64_FLIP_PAGE_OFFSET like this?
> >
> > diff --git a/arm64.c b/arm64.c
> > index 7069312671cf..f33b6c28a3bc 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -4031,6 +4031,9 @@ arm64_calc_VA_BITS(void)
> > error(FATAL, "cannot determine
VA_BITS_ACTUAL\n");
> > }
> >
> > + if (!machdep->machspec->CONFIG_ARM64_VA_BITS)
> > + machdep->machspec->VA_BITS =
machdep->machspec->CONFIG_ARM64_VA_BITS;
>
> If the above judgment condition is true, the value of VA_BITS is
> always assigned to zero? Could you please describe more details?
Ugh, sorry my bad. I actually meant:
if (machdep->machspec->CONFIG_ARM64_VA_BITS)
machdep->machspec->VA_BITS =
machdep->machspec->CONFIG_ARM64_VA_BITS;
By this, ARM64_FLIP_PAGE_OFFSET with VA_BITS uses CONFIG_ARM64_VA_BITS
if vmcoreinfo is available. If it's not available, ARM64_FLIP_PAGE_OFFSET
uses VA_BITS guessed as equal to VA_BITS_ACTUAL.
OK. Thank you for the explanation, Kazu.
BTW: I tried to reproduce the rawdump issue, but I can not reproduce
it on my machine. Do you
happen to know how to reproduce this issue?
Or can you share some steps?Ankur.
Thanks.
Lianbo
And I found that ARM64_FLIP_PAGE_OFFSET_ACTUAL is not used actually.
I think we can remove it.
Will send a patch for testing later.
Thanks,
Kazu
>
> Thanks.
> Lianbo
> > +
> > /*
> > * The mm flip commit is introduced before 52-bits VA, which is
before the
> > * commit to export NUMBER(TCR_EL1_T1SZ)
> > diff --git a/defs.h b/defs.h
> > index cbd45e52f9da..c4438b357259 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -3239,7 +3239,7 @@ typedef signed int s32;
> > #define ARM64_PAGE_OFFSET ((0xffffffffffffffffUL) \
> > <<
(machdep->machspec->VA_BITS - 1))
> > /* kernels >= v5.4 the kernel VA space is flipped */
> > -#define ARM64_FLIP_PAGE_OFFSET (-(1UL) <<
machdep->machspec->CONFIG_ARM64_VA_BITS)
> > +#define ARM64_FLIP_PAGE_OFFSET (-(1UL) <<
machdep->machspec->VA_BITS)
> > #define ARM64_FLIP_PAGE_OFFSET_ACTUAL ((0xffffffffffffffffUL) \
> > - ((1UL) <<
machdep->machspec->VA_BITS_ACTUAL) + 1)
> >
> >
> > >
> > > > /*
> > > > * The mm flip commit is introduced before 52-bits
VA, which is before the
> > > > * commit to export NUMBER(TCR_EL1_T1SZ)
> > > >
> > >
> > > BTW: Can we also try to get its value from kernel config? But this
> > > relies on kernel config: CONFIG_IKCONFIG.
> >
> > The IKCONFIG_AVAIL flag is set by IKCFG_INIT here:
> >
> > read_in_kernel_config(IKCFG_INIT);
> > kernel_init();
> > machdep_init(POST_GDB);
> >
> > The get_kernel_config() cannot be used in machdep_init(PRE_GDB).
> >
> > Thanks,
> > Kazu
> >
> >
> > >
> > > diff --git a/arm64.c b/arm64.c
> > > index 7069312..79f9929 100644
> > > --- a/arm64.c
> > > +++ b/arm64.c
> > > @@ -3989,6 +3989,10 @@ arm64_calc_VA_BITS(void)
> > > value = atol(string);
> > > free(string);
> > > machdep->machspec->CONFIG_ARM64_VA_BITS = value;
> > > + } else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
> > > + if ((get_kernel_config("CONFIG_ARM64_VA_BITS",
> > > + &string)) == IKCONFIG_STR)
> > > + machdep->machspec->CONFIG_ARM64_VA_BITS =
atol(string);
> > > }
> > >
> > > if (kernel_symbol_exists("vabits_actual")) {
> > >
> > > Thanks.
> > > Lianbo
> > > > > 3987 static void
> > > > > 3988 arm64_calc_VA_BITS(void)
> > > > > 3989 {
> > > > > 3990 int bitval;
> > > > > 3991 struct syment *sp;
> > > > > 3992 ulong vabits_actual, value;
> > > > > 3993 char *string;
> > > > > 3994
> > > > > 3995 if ((string =
pc->read_vmcoreinfo("NUMBER(VA_BITS)"))) {
> > > > > 3996 value = atol(string);
> > > > > 3997 free(string);
> > > > > 3998 machdep->machspec->CONFIG_ARM64_VA_BITS =
value;
> > > > > 3999 }
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >Thanks.
> > > > > >Lianbo
> > > > >
> > > > > >> Change-Id:
I525f3c7fd91e1f06e909c2f4c1749c44c068baea
> > > > > >> Signed-off-by: Ankur Bansal
<er.ankurbansal(a)gmail.com <mailto:er.ankurbansal@gmail.com>
> >
> > > > > >> ---
> > > > > >> arm64.c | 15 +++++++++++----
> > > > > >> 1 file changed, 11 insertions(+), 4 deletions(-)
> > > > > >>
> > > > > >> diff --git a/arm64.c b/arm64.c
> > > > > >> index 7069312..2dc77f7 100644
> > > > > >> --- a/arm64.c
> > > > > >> +++ b/arm64.c
> > > > > >> @@ -220,10 +220,17 @@ arm64_init(int when)
> > > > > >>
> > > > > >> /* vabits_actual introduced after
mm flip, so it should be flipped layout
> */
> > > > > >> if (ms->VA_BITS_ACTUAL) {
> > > > > >> - ms->page_offset =
ARM64_FLIP_PAGE_OFFSET;
> > > > > >> - /* useless on arm64 */
> > > > > >> -
machdep->identity_map_base = ARM64_FLIP_PAGE_OFFSET;
> > > > > >> - machdep->kvbase =
ARM64_FLIP_PAGE_OFFSET;
> > > > > >> + if ((pc->flags2 &
SNAP)) {
> > > > > >> +
ms->page_offset = ARM64_FLIP_PAGE_OFFSET;
> > > > > >> + /* useless on
arm64 */
> > > > > >> +
machdep->identity_map_base = ARM64_FLIP_PAGE_OFFSET;
> > > > > >> +
machdep->kvbase = ARM64_FLIP_PAGE_OFFSET;
> > > > > >> + }
> > > > > >> + else{
> > > > > >> +
ms->page_offset = ARM64_FLIP_PAGE_OFFSET_ACTUAL;
> > > > > >> +
machdep->identity_map_base =
> ARM64_FLIP_PAGE_OFFSET_ACTUAL;
> > > > > >> +
machdep->kvbase = ARM64_FLIP_PAGE_OFFSET_ACTUAL;
> > > > > >> + }
> > > > > >> ms->userspace_top =
ARM64_USERSPACE_TOP_ACTUAL;
> > > > > >> } else {
> > > > > >> ms->page_offset =
ARM64_PAGE_OFFSET;
> > > > > >> --
> > > > > >> 2.7.4
> > > > >
> > > > >
> > > > >
> > > > > ------------------------------
> > > > >
> > > > > --
> > > > > Crash-utility mailing list
> > > > > Crash-utility(a)redhat.com
<mailto:Crash-utility@redhat.com>
> > > > >
https://listman.redhat.com/mailman/listinfo/crash-utility
> > > > >
> > > > > End of Crash-utility Digest, Vol 192, Issue 28
> > > > > **********************************************
> > > > >
> > > > >
> > > > >
> > > >