-----Original Message-----
> Crash encounters a bug like the following:
> ...
> SECTION_SIZE_BITS: 30
> CONFIG_ARM64_VA_BITS: 52
> VA_BITS_ACTUAL: 48
> (calculated) VA_BITS: 48
> PAGE_OFFSET: ffff000000000000
> VA_START: ffff800000000000
> modules: ffff800008000000 - ffff80000fffffff
> vmalloc: ffff800010000000 - ffffffdfdffeffff
> kernel image: ffff800010000000 - ffff800012750000
> vmemmap: ffffffdfffe00000 - ffffffffffffffff
>
> <readmem: ffff800011c53bc8, KVADDR, "nr_irqs", 4, (FOE), b47bdc>
> <read_kdump: addr: ffff800011c53bc8 paddr: eb453bc8 cnt: 4>
> read_netdump: addr: ffff800011c53bc8 paddr: eb453bc8 cnt: 4 offset: 1c73bc8
> irq_stack_ptr:
> type: 1, TYPE_CODE_PTR
> target_typecode: 8, TYPE_CODE_INT
> target_length: 8
> length: 8
> GNU_GET_DATATYPE[thread_union]: returned via gdb_error_hook
> <readmem: ffff000b779c0050, KVADDR, "IRQ stack pointer", 8, (ROE), 3a37bea0>
> <read_kdump: addr: ffff000b779c0050 paddr: fff1000bf79c0050 cnt: 8>
> read_netdump: READ_ERROR: offset not found for paddr: fff1000bf79c0050
> crash: read error: kernel virtual address: ffff000b779c0050 type: "IRQ stack pointer"
> <readmem: ffff000b77a60050, KVADDR, "IRQ stack pointer", 8, (ROE), 3a37bea8>
> <read_kdump: addr: ffff000b77a60050 paddr: fff1000bf7a60050 cnt: 8>
> read_netdump: READ_ERROR: offset not found for paddr: fff1000bf7a60050
> ...
>
> Apparently, for a normal system, the 'paddr: fff1000bf79c0050' is
> unreasonable.
>
> This bug connects 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 phys_offset. If
> using memstart_addr to calculate the real paddr, the unreasonable paddr
> will be got.
>
> Furthermore, in crash utility, PTOV() needs memstart_addr to calculate
> VA from PA, while getting PFN offset in a dumpfile, phys_offset is
> required.
>
> To serve the different purpose, using phys_offset_nominal and
> phys_offset to store them.
>
> Signed-off-by: Pingfan Liu <piliu@xxxxxxxxxx>
> Cc: HAGIO KAZUHITO <k-hagio-ab@xxxxxxx>
> Cc: Lianbo Jiang <lijiang@xxxxxxxxxx>
> Cc: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>
> To: crash-utility@xxxxxxxxxx
> ---
> arm64.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
> defs.h | 17 ++++++++++----
> 2 files changed, 78 insertions(+), 10 deletions(-)
>
> diff --git a/arm64.c b/arm64.c
> index 98138b2..b3b3242 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -23,6 +23,10 @@
> #include <sys/ioctl.h>
>
> #define NOT_IMPLEMENTED(X) error((X), "%s: function not implemented\n", __func__)
> +/*
> + * _PAGE_OFFSET() refers to arch/arm64/include/asm/memory.h
> + */
> +#define _PAGE_OFFSET(va) (-1UL << (va))
>
> static struct machine_specific arm64_machine_specific = { 0 };
> static int arm64_verify_symbol(const char *, ulong, char);
> @@ -691,6 +695,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, " phys_offset_nominal: %lx\n", ms->phys_offset_nominal);
> 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);
> @@ -991,7 +996,17 @@ arm64_calc_physvirt_offset(void)
> ulong physvirt_offset;
> struct syment *sp;
>
> - ms->physvirt_offset = ms->phys_offset - ms->page_offset;
> + /* if flipped but having 'physvirt_offset', ms->physvirt_offset is overwritten in this func */
> + if (machdep->flags & FLIPPED_VM) {
> + /*
> + * source arch/arm64/include/asm/memory.h
> + * #define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
> + * the part "addr & ~PAGE_OFFSET" is done in arm64_VTOP()
> + */
> + ms->physvirt_offset = ms->phys_offset_nominal;
> + } else {
> + ms->physvirt_offset = ms->phys_offset - ms->page_offset;
> + }
Can this be moved after the following "if ((sp = kernel_symbol_search(..."
block? I think it's more natural and readable than the overwriting. i.e.:
if ((sp = kernel_symbol_search("physvirt_offset")) &&
machdep->machspec->kimage_voffset) {
if (READMEM(...) > 0) {
...
return;
}
}
if (machdep->flags & FLIPPED_VM) {
...
>
> if ((sp = kernel_symbol_search("physvirt_offset")) &&
> machdep->machspec->kimage_voffset) {
> @@ -1007,6 +1022,8 @@ arm64_calc_physvirt_offset(void)
> static void
> arm64_calc_phys_offset(void)
> {
> +#define MEMSTART_ADDR_OFFSET _PAGE_OFFSET(48) - _PAGE_OFFSET(52)
> +
> struct machine_specific *ms = machdep->machspec;
> ulong phys_offset;
>
> @@ -1033,7 +1050,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->phys_offset_nominal = htol(string, QUIET, NULL);
> + if (ms->phys_offset_nominal < 0)
> + ms->phys_offset = ms->phys_offset_nominal +
> MEMSTART_ADDR_OFFSET;
> + else
> + ms->phys_offset = ms->phys_offset_nominal;
> free(string);
> return;
> }
If it's /dev/mem (not /proc/kcore), the memstart_addr value is still negative?
It's no problem?
> @@ -1085,7 +1106,18 @@ arm64_calc_phys_offset(void)
> } else if (DISKDUMP_DUMPFILE() && diskdump_phys_base(&phys_offset)) {
> ms->phys_offset = phys_offset;
makedumpfile also set kdump_sub_header.phys_base to NUMBER(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->phys_offset_nominal = phys_offset;
> + if ((long)phys_offset < 0)
> + ms->phys_offset = phys_offset + MEMSTART_ADDR_OFFSET;
> + else
> + ms->phys_offset = phys_offset;
> } else {
> error(WARNING,
> "phys_offset cannot be determined from the dumpfile.\n");
> @@ -1175,6 +1207,23 @@ arm64_init_kernel_pgd(void)
> vt->kernel_pgd[i] = value;
> }
>
> +ulong arm64_PTOV(ulong paddr)
> +{
> + ulong v;
> + struct machine_specific *ms = machdep->machspec;
> +
> + /*
> + * Either older kernel before kernel has 'physvirt_offset' or newer kernel which
> + * removes 'physvirt_offset' has the same formula
> + */
> + if (!(machdep->flags & HAS_PHYSVIRT_OFFSET))
> + v = (paddr - ms->physvirt_offset) | PAGE_OFFSET;
> + else
> + v = paddr - ms->physvirt_offset;
> +
> + return v;
> +}
> +
> ulong
> arm64_VTOP(ulong addr)
> {
> @@ -1185,8 +1234,20 @@ arm64_VTOP(ulong addr)
> return addr - machdep->machspec->kimage_voffset;
> }
>
> - if (addr >= machdep->machspec->page_offset)
> - return addr + machdep->machspec->physvirt_offset;
> + if (addr >= machdep->machspec->page_offset) {
> + ulong paddr;
> +
> + if (!(machdep->flags & FLIPPED_VM) || (machdep->flags & HAS_PHYSVIRT_OFFSET)) {
> + paddr = addr;
> + } else {
> + /*
> + * #define __lm_to_phys(addr) (((addr) & ~PAGE_OFFSET) + PHYS_OFFSET)
> + */
> + paddr = addr & ~ _PAGE_OFFSET(machdep->machspec->CONFIG_ARM64_VA_BITS);
> + }
> + paddr += machdep->machspec->physvirt_offset;
> + return paddr;
Hmm, complex. It should be symmetric to PTOV, why differences?
> + }
> else if (machdep->machspec->kimage_voffset)
> return addr - machdep->machspec->kimage_voffset;
> else /* no randomness */
> diff --git a/defs.h b/defs.h
> index 24d41e7..786186a 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -3086,11 +3086,6 @@ typedef u64 pte_t;
> #define _64BIT_
> #define MACHINE_TYPE "ARM64"
>
> -#define PTOV(X) \
> - ((unsigned long)(X) - (machdep->machspec->physvirt_offset))
> -
> -#define VTOP(X) arm64_VTOP((ulong)(X))
> -
> #define USERSPACE_TOP (machdep->machspec->userspace_top)
> #define PAGE_OFFSET (machdep->machspec->page_offset)
> #define VMALLOC_START (machdep->machspec->vmalloc_start_addr)
> @@ -3100,6 +3095,14 @@ typedef u64 pte_t;
> #define MODULES_VADDR (machdep->machspec->modules_vaddr)
> #define MODULES_END (machdep->machspec->modules_end)
>
> +/*
> + * The formula:
> + * #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
> + */
> +#define PTOV(X) arm64_PTOV((ulong)(X))
> +
> +#define VTOP(X) arm64_VTOP((ulong)(X))
> +
> #define IS_VMALLOC_ADDR(X) arm64_IS_VMALLOC_ADDR((ulong)(X))
>
> #define PAGEBASE(X) (((ulong)(X)) & (ulong)machdep->pagemask)
> @@ -3300,7 +3303,10 @@ struct machine_specific {
> ulong vmemmap_end;
> ulong modules_vaddr;
> ulong modules_end;
> + /* real physical offset */
> ulong phys_offset;
> + /* read from kernel symbol memstart_addr */
> + long phys_offset_nominal;
> ulong __exception_text_start;
> ulong __exception_text_end;
> struct arm64_pt_regs *panic_task_regs;
> @@ -5903,6 +5909,7 @@ void unwind_backtrace(struct bt_info *);
> void arm64_init(int);
> void arm64_dump_machdep_table(ulong);
> ulong arm64_VTOP(ulong);
> +ulong arm64_PTOV(ulong);
> int arm64_IS_VMALLOC_ADDR(ulong);
> ulong arm64_swp_type(ulong);
> ulong arm64_swp_offset(ulong);
> --
> 2.29.2
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
