On Wed, Aug 7, 2024 at 11:31 AM <devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
Date: Wed, 7 Aug 2024 11:47:48 +1200
From: Tao Liu <ltao@xxxxxxxxxx>
Subject: Re: [PATCH] arm64: fix the determination of
vmemmap and struct_page_size
To: qiwu.chen@xxxxxxxxxxxxx
Cc: devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
Message-ID:
<CAO7dBbXV3coyp14n94Ad1x2HRXt9+nZ8FAy6wai-2y-O50dAcQ@xxxxxxxxxxxxxx>
Content-Type: text/plain; charset="UTF-8"
Hi qiwu,
On Mon, Jul 29, 2024 at 2:22 AM <qiwu.chen@xxxxxxxxxxxxx> 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@xxxxxx>
> ---
> 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):
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@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
