On Tue, Jun 18, 2024 at 01:49:22AM -0700, Breno Leitao wrote:
> Hello,
>
> I am setting the following warning when loading a kdump kernel in
> ACPI-based aarch64 box, running upstream and linux-next (stack below
> against 5f703ce5c981).
>
> virt_to_phys used for non-linear address: 00000000cf9a4e41 (0xffffffffff5e0000)
> WARNING: CPU: 35 PID: 2279 at arch/arm64/mm/physaddr.c:15 __virt_to_phys (arch/arm64/mm/physaddr.c:17)
> Modules linked in: sunrpc(E) ipmi_ssif(E) nvidia_cspmu(E) arm_cspmu_module(E) arm_smmuv3_pmu(E) coresight_trbe(E) mlx5_ib(E) ib_uverbs(E) ipmi_devintf(E) ipmi_msghandler(E) coresight_stm(E) coresight_tmc(E) coresight_funnel(E) stm_core(E) coresight_etm4x(E) coresight(E) cppc_cpufreq(E) sch_fq_codel(E) drm(E) backlight(E) drm_panel_orientation_quirks(E) crct10dif_ce(E) sm3_ce(E) sm3(E) sha3_ce(E) sha512_ce(E) sha512_arm64(E) xhci_pci(E) xhci_hcd(E) spi_tegra210_quad(E) nbd(E) acpi_power_meter(E) loop(E) efivarfs(E) autofs4(E)
> Hardware name: Quanta Java Island EVT 29F0EMAZ046/Java Island, BIOS F0EJ3A01 06/03/2024
> pstate: 63400009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> pc : __virt_to_phys (arch/arm64/mm/physaddr.c:17)
> lr : __virt_to_phys (arch/arm64/mm/physaddr.c:17)
> sp : ffff8000be1cf9f0
> x29: ffff8000be1cf9f0 x28: dfff800000000000 x27: ffff800083ca9630
> x26: ffff800083ca9000 x25: 1ffff000107952c6 x24: ffff0000ff1b6800
> x23: 0000000000000000 x22: 0000000000001687 x21: ffff0001ecd46800
> x20: ffff000103524000 x19: ffffffffff5e0000 x18: ffffffffffffffff
> x17: ffff8000802d07d4 x16: 2930303030653566 x15: 0000000000000001
> x14: 1ffff00017c39ed8 x13: 0000000000000000 x12: 0000000000000000
> x11: ffff700017c39ed9 x10: 0000000000000002 x9 : e7a4565c7b34a200
> x8 : ffff800083c50000 x7 : 0000000000000001 x6 : 0000000000000001
> x5 : ffff8000be1cf6a0 x4 : ffff800084a5ff40 x3 : ffff800081580960
> x2 : 0000000000000001 x1 : ffff800083253340 x0 : 000000000000004f
> Call trace:
> __virt_to_phys (arch/arm64/mm/physaddr.c:17)
> of_kexec_alloc_and_setup_fdt (drivers/of/kexec.c:305)
> load_other_segments (arch/arm64/kernel/machine_kexec_file.c:162)
> image_load (arch/arm64/kernel/kexec_image.c:103)
> __arm64_sys_kexec_file_load (kernel/kexec_file.c:73 kernel/kexec_file.c:257 kernel/kexec_file.c:296 kernel/kexec_file.c:374 kernel/kexec_file.c:332 kernel/kexec_file.c:332)
> invoke_syscall (arch/arm64/kernel/syscall.c:? arch/arm64/kernel/syscall.c:48)
> el0_svc_common (./include/linux/thread_info.h:127 arch/arm64/kernel/syscall.c:141)
> do_el0_svc (arch/arm64/kernel/syscall.c:153)
> el0_svc (arch/arm64/kernel/entry-common.c:165 arch/arm64/kernel/entry-common.c:178 arch/arm64/kernel/entry-common.c:713)
> el0t_64_sync_handler (arch/arm64/kernel/entry-common.c:781)
> el0t_64_sync (arch/arm64/kernel/entry.S:598)
> irq event stamp: 49964
> hardirqs last enabled at (49963): console_unlock (kernel/printk/printk.c:? kernel/printk/printk.c:2746 kernel/printk/printk.c:3065)
> hardirqs last disabled at (49964): el1_dbg (arch/arm64/kernel/entry-common.c:371 arch/arm64/kernel/entry-common.c:471)
> softirqs last enabled at (49942): handle_softirqs (./arch/arm64/include/asm/preempt.h:13 kernel/softirq.c:401 kernel/softirq.c:582)
> softirqs last disabled at (49937): __do_softirq (kernel/softirq.c:589)
> ---[ end trace 0000000000000000 ]---
>
>
> This is happening on the following code:
>
> /* Remove memory reservation for the current device tree. */
> ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),
> fdt_totalsize(initial_boot_params));
>
>
> Where `initial_boot_params` contains a copy of the fdt allocated at init
> time, and as I've analyzed, it is a virtual address and can be
> translated by the MMU.
AFAICT, on arm64 'initial_boot_params' will be a pointer to the FDT in
the fixmap that we mapped in setup_machine_fdt(). So that's not a linear
address, and the warning is legitimate -- the use of __pa() here will
give a bogus result and we could remove a memory reservation for an
unrelated address.
It looks like this is a regression due to commit:
ac10be5cdbfa8521 ("arm64: Use common of_kexec_alloc_and_setup_fdt()")
... as at that point arm64 had a fixmap'd fdt, and prior to that commit
we didn't try to use __pa() on the fixmap pointer.
It looks like no-one's tested kexec_file_load() with
CONFIG_DEBUG_VIRTUAL since then.
> Since __pa() is a macro to __virt_to_phys():
>
> #define __pa(x) __virt_to_phys((unsigned long)(x))
>
> I am curious why this address is special that makes __virt_to_phys()
> unhappy.
It's a fixmap address rather than a linear-map address. Confusingly
'virt' generally means the linear map rather than any virtual address,
and usually virt_to_*() only work on linear map addresses.
I'm not sure how to fix this; the major reason we fixmap the FDT is so
that it can be anywhere in memory (and e.g. may not be in the linear map
at all), so we can't always generate a linear map VA.
We could stash the PA at boot time, and pass this as an argument to
of_kexec_alloc_and_setup_fdt().
Rob, any thoughts? I couldn't see a neat way of doing this, but maybe we
could initialise an initial_boot_params_phys at setup time, with a bit
of churn to early_init_dt_verify() and friends?
Mark.
>
> Have you seen this before? Any tip on how to debug this further?
>
> Thanks
