Re: [Crash-utility] [PATCH] arm64: Add fallback option when vabits_actual is missing
On 2022/12/07 13:25, Pavan Kondeti wrote:
On Wed, Dec 07, 2022 at 02:01:56AM +0000, HAGIO KAZUHITO(萩尾 一仁)
wrote:
> Thanks for the patch.
>
> On 2022/12/06 19:00, Pavankumar Kondeti wrote:
>> After the commit 0d9b1ffefabe ("arm64: mm: make vabits_actual
>> a build time constant if possible") introduced in v5.19
>> Linux kernel, the crash will not find vabits_actual symbol.
>
> "if VA_BITS <= 48" ?
>
Thats right.
>> Add a fallback option to initialize VA_BITS based on the
>> user supplied machdep options.
>>
>> Tested ramdumps loading in both 6.0 and 5.15 kernels.
>
> What if kernels < 5.4? For such old kernels without vabits_actual
> introduced, probably we should not set VA_BITS_ACTUAL..
>
>
ok, so what if the user passes -m vabits_actual=X on an older kernel dumps.
I think it's ok to fail, because they should not use that option
for older kernels than 5.4, it's a wrong usage.
+ if (machdep->machspec->CONFIG_ARM64_VA_BITS) {
+ /* guess */
+ machdep->machspec->VA_BITS_ACTUAL =
machdep->machspec->CONFIG_ARM64_VA_BITS;
This is my concern. Actually I found a 4.18.0 arm64 vmcore with
NUMBER(VA_BITS)=48 in vmcoreinfo, crash fails to start with the patch.
# crash-arm64 vmlinux vmcore
...
crash-arm64: read error: kernel virtual address: fffff6003bcc0050 type: "IRQ stack
pointer"
crash-arm64: read error: kernel virtual address: fffff6003bcf0050 type: "IRQ stack
pointer"
crash-arm64: read error: kernel virtual address: fffff6003fffe400 type: "memory
section root table"
#
Earlier we ignore it, but now we end up using it. should I add a
kernel
version check in fallback() routine?
hmm, currently THIS_KERNEL_VERSION cannot be used at the point.
We need to fill it, but also need vmcoreinfo..
um, please let me know if I misunderstand, I don't have arm64
knowledge very much.
Do ramdumps need "-m vabits_actual=xx" regardless of the
existence of vabits_actual symbol on 5.4 and later?
And it seems that recent arm64 vmcores have NUMBER(TCR_EL1_T1SZ)
in vmcoreinfo, they are covered by arm64_set_va_bits_by_tcr().
so how about adding only this?
--- a/arm64.c
+++ b/arm64.c
@@ -4671,6 +4671,10 @@ arm64_calc_VA_BITS(void)
return;
} else if (arm64_set_va_bits_by_tcr()) {
return;
+ } else if (machdep->machspec->VA_BITS_ACTUAL) {
+ machdep->machspec->VA_BITS =
machdep->machspec->VA_BITS_ACTUAL;
+ machdep->machspec->VA_START =
_VA_START(machdep->machspec->VA_BITS_ACTUAL);
+ return;
}
if (!(sp = symbol_search("swapper_pg_dir")) &&
Thanks,
Kazu