Re: [Crash-utility] [PATCH] arm64: store phy_offset and memstart_addr separately

[piliu]

On 3/23/21 3:04 PM, lijiang wrote:

Hi, Pingfan
Thanks for the fix.
在 2021年03月18日 09:36, Pingfan Liu 写道:

With kernel commit 7bc1a0f9e176 ("arm64: mm: use single quantity to
represent the PA to VA translation"), memstart_addr can be negative,
which makes it different from real phy_offset.

In crash utility, PTOV() needs memstart_addr, while getting PFN offset
in a dumpfile, phy_offset is required. So storing them separately for
different purpose.

Signed-off-by: Pingfan Liu <pi...@redhat.com>
Cc: Lianbo Jiang <liji...@redhat.com>
To: crash-utility@redhat.com
---
  arm64.c | 22 +++++++++++++++++++---
  defs.h  |  1 +
  2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arm64.c b/arm64.c
index 37aed07..e560d07 100644
--- a/arm64.c
+++ b/arm64.c
@@ -687,6 +687,7 @@ arm64_dump_machdep_table(ulong arg)
                fprintf(fp, "        kimage_voffset: %016lx\n", 
ms->kimage_voffset);
        }
        fprintf(fp, "           phys_offset: %lx\n", ms->phys_offset);
+       fprintf(fp, "         memstart_addr: %lx\n", ms->memstart_addr);
        fprintf(fp, "__exception_text_start: %lx\n", 
ms->__exception_text_start);
        fprintf(fp, "  __exception_text_end: %lx\n", ms->__exception_text_end);
        fprintf(fp, " __irqentry_text_start: %lx\n", ms->__irqentry_text_start);
@@ -987,7 +988,7 @@ arm64_calc_physvirt_offset(void)
        ulong physvirt_offset;
        struct syment *sp;

- ms->physvirt_offset = ms->phys_offset - ms->page_offset;

+       ms->physvirt_offset = ms->memstart_addr - ms->page_offset;

if ((sp = kernel_symbol_search("physvirt_offset")) &&

                        machdep->machspec->kimage_voffset) {
@@ -1028,7 +1029,11 @@ arm64_calc_phys_offset(void)
                    ms->kimage_voffset && (sp = 
kernel_symbol_search("memstart_addr"))) {
                        if (pc->flags & PROC_KCORE) {
                                if ((string = 
pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
-                                       ms->phys_offset = htol(string, QUIET, 
NULL);
+                                       ms->memstart_addr = htol(string, QUIET, 
NULL);
+                                       if (ms->memstart_addr < 0)
+                                               ms->phys_offset = 
ms->memstart_addr + 0xffff000000000000 - 0xfff0000000000000;

Is that possible to explicitly use the following ways instead of the magic 
number? For example:

((UL(1) << (52))) - ((UL(1) << (48))) or ((UL(1) << (VA_BITS))) something?

Yes. I will update with V2.

Thanks,
Pingfan

Thanks.
Lianbo

+                                       else
+                                               ms->phys_offset = 
ms->memstart_addr;
                                        free(string);
                                        return;
                                }
@@ -1080,7 +1085,18 @@ arm64_calc_phys_offset(void)
        } else if (DISKDUMP_DUMPFILE() && diskdump_phys_base(&phys_offset)) {
                ms->phys_offset = phys_offset;
        } else if (KDUMP_DUMPFILE() && arm64_kdump_phys_base(&phys_offset)) {
-               ms->phys_offset = phys_offset;
+               /*
+                * When running a 52bits kernel on 48bits hardware. Kernel 
plays a trick:
+                * if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52) && (vabits_actual != 
52))
+                 *       memstart_addr -= _PAGE_OFFSET(48) - _PAGE_OFFSET(52);
+                *
+                * In crash, this should be detected to get a real physical 
start address.
+                */
+               ms->memstart_addr = phys_offset;
+               if ((long)phys_offset < 0)
+                       ms->phys_offset = phys_offset + 0xffff000000000000 - 
0xfff0000000000000;
+               else
+                       ms->phys_offset = phys_offset;
        } else {
                error(WARNING,
                        "phys_offset cannot be determined from the 
dumpfile.\n");
diff --git a/defs.h b/defs.h
index 35b983a..64f2bcb 100644
--- a/defs.h
+++ b/defs.h
@@ -3290,6 +3290,7 @@ struct machine_specific {
        ulong modules_vaddr;
        ulong modules_end;
        ulong phys_offset;
+       long memstart_addr;
        ulong __exception_text_start;
        ulong __exception_text_end;
        struct arm64_pt_regs *panic_task_regs;

--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility