Date: Wed, 7 Aug 2024 11:47:48 +1200 From: Tao Liu <ltao(a)redhat.com&gt; Subject: [Crash-utility] Re: [PATCH] arm64: fix the determination of vmemmap and struct_page_size To: qiwu.chen(a)transsion.com Cc: devel(a)lists.crash-utility.osci.io Message-ID: < CAO7dBbXV3coyp14n94Ad1x2HRXt9+nZ8FAy6wai-2y-O50dAcQ(a)mail.gmail.com&gt; Content-Type: text/plain; charset="UTF-8" Hi qiwu, On Mon, Jul 29, 2024 at 2:22 AM <qiwu.chen(a)transsion.com&gt; 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.chen(a)qq.com&gt; > --- > 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.

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