On Wed, Aug 7, 2024 at 11:31 AM <devel-requ...@lists.crash-utility.osci.io>
wrote:
> Date: Wed, 7 Aug 2024 11:47:48 +1200
> From: Tao Liu <l...@redhat.com>
> Subject: [Crash-utility] Re: [PATCH] arm64: fix the determination of
> vmemmap and struct_page_size
> To: qiwu.c...@transsion.com
> Cc: devel@lists.crash-utility.osci.io
> Message-ID:
> <
> cao7dbbxv3coyp14n94ad1x2hrxt9+nz8fay6wai-2y-o50d...@mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> Hi qiwu,
>
> On Mon, Jul 29, 2024 at 2:22 AM <qiwu.c...@transsion.com> wrote:
> >
> > Currently, the vmemmap ptr addr is determined by the vmcoreinfo of
> "SYMBOL(vmemmap)", which leads to an invalid vmemmap addr showed by "help
> -m" for dump files without the vmcoreinfo. The value of vmemmap_end is
> simply set to -1 for available VA_BITS_ACTUAL case in
> arm64_calc_virtual_memory_ranges(), and the struct_page_size value is 0.
> > crash> help -m |grep vmem
> > vmemmap_vaddr: fffffffeffe00000
> > vmemmap_end: ffffffffffffffff
> > vmemmap: 0000000000000000
> > crash> help -m |grep struct_page_size
> > struct_page_size: 0
> >
> > Introduce arm64_get_vmemmap_page_ptr() to fix the determination of
> vmemmap ptr addr, and fix the determination of vmemmap_end and
> struct_page_size in arm64_calc_virtual_memory_ranges().
> > crash> help -m |grep vmem
> > vmemmap_vaddr: fffffffeffe00000
> > vmemmap_end: ffffffffffe00000
> > vmemmap: fffffffefee00000
> > crash> help -m |grep struct_page_size
> > struct_page_size: 64
> >
> > Signed-off-by: qiwu.chen <qiwu.c...@qq.com>
> > ---
> > arm64.c | 29 +++++++++++++++++++++++------
> > 1 file changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/arm64.c b/arm64.c
> > index 78e6609..ef495ec 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -159,7 +159,6 @@ arm64_vmemmap_is_page_ptr(ulong addr, physaddr_t
> *phys)
> > ulong size = SIZE(page);
> > ulong pfn, nr;
> >
> > -
> > if (IS_SPARSEMEM() && (machdep->flags & VMEMMAP) &&
> > (addr >= VMEMMAP_VADDR && addr <= VMEMMAP_END) &&
> > !((addr - VMEMMAP_VADDR) % size)) {
> > @@ -175,6 +174,25 @@ arm64_vmemmap_is_page_ptr(ulong addr, physaddr_t
> *phys)
> > return FALSE;
> > }
> >
> > +static void arm64_get_vmemmap_page_ptr(void)
> > +{
> > + struct machine_specific *ms = machdep->machspec;
> > +
> > + /* If vmemmap exists, it means kernel enabled
> CONFIG_SPARSEMEM_VMEMMAP */
> > + if (arm64_get_vmcoreinfo(&ms->vmemmap, "SYMBOL(vmemmap)",
> NUM_HEX))
> > + goto out;
> > +
> > + /* The global symbol of vmemmap is removed since kernel commit
> 7bc1a0f9e1765 */
> > + if (!kernel_symbol_exists("vmemmap"))
> > + ms->vmemmap = ms->vmemmap_vaddr - ((ms->phys_offset >>
> machdep->pageshift) * ms->struct_page_size);
> > + else
> > + ms->vmemmap = symbol_value("vmemmap");
> > +
>
> I would prefer to exchange the if else sequence, to be:
>
> /* The global symbol of vmemmap is removed since kernel commit
> 7bc1a0f9e1765 */
> if (kernel_symbol_exists("vmemmap"))
> ms->vmemmap = symbol_value("vmemmap");
> else
> ms->vmemmap = ms->vmemmap_vaddr - ((ms->phys_offset >>
> machdep->pageshift) * ms->struct_page_size);
>
> This would be more readable, if a symbol exists, then read the symbol,
> otherwise calc it out.
>
> Other than that, the patch LGTM. Ack the rest of the patch.
>
Applied(with above change):
https://github.com/crash-utility/crash/commit/5cd1c6ace5fe41f3e007669d9bc549e168f8441e
Thanks
Lianbo
>
> Thanks,
> Tao Liu
>
> > +out:
> > + if (ms->vmemmap)
> > + machdep->is_page_ptr = arm64_vmemmap_is_page_ptr;
> > +}
> > +
> > /*
> > * Do all necessary machine-specific setup here. This is called several
> times
> > * during initialization.
> > @@ -443,10 +461,6 @@ arm64_init(int when)
> >
> > machdep->stacksize = ARM64_STACK_SIZE;
> > machdep->flags |= VMEMMAP;
> > - /* If vmemmap exists, it means kernel enabled
> CONFIG_SPARSEMEM_VMEMMAP */
> > - if (arm64_get_vmcoreinfo(&ms->vmemmap,
> "SYMBOL(vmemmap)", NUM_HEX))
> > - machdep->is_page_ptr = arm64_vmemmap_is_page_ptr;
> > -
> > machdep->uvtop = arm64_uvtop;
> > machdep->is_uvaddr = arm64_is_uvaddr;
> > machdep->eframe_search = arm64_eframe_search;
> > @@ -498,6 +512,7 @@ arm64_init(int when)
> > if (!ms->struct_page_size)
> > arm64_calc_virtual_memory_ranges();
> >
> > + arm64_get_vmemmap_page_ptr();
> > arm64_get_section_size_bits();
> >
> > if (!machdep->max_physmem_bits) {
> > @@ -4841,6 +4856,7 @@ arm64_calc_virtual_memory_ranges(void)
> > return;
> >
> > STRUCT_SIZE_INIT(page, "page");
> > + ms->struct_page_size = SIZE(page);
> >
> > switch (machdep->flags &
> (VM_L2_64K|VM_L3_64K|VM_L3_4K|VM_L4_4K))
> > {
> > @@ -4868,7 +4884,8 @@ arm64_calc_virtual_memory_ranges(void)
> > vmemmap_start = (-vmemmap_size - MEGABYTES(2));
> > ms->vmalloc_end = vmalloc_end - 1;
> > ms->vmemmap_vaddr = vmemmap_start;
> > - ms->vmemmap_end = -1;
> > + ms->vmemmap_end = vmemmap_start + vmemmap_size;
> > +
> > return;
> > }
> >
> > --
> > 2.25.1
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
