On Thu, Dec 08, 2022 at 02:23:55AM +0000, HAGIO KAZUHITO(萩尾 一仁) wrote:
> 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.
> 
Makes sense. I only started passing -m vabits_actual=N for 5.10+ kernel dumps.

> 
> +     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;
>          }
>   
This works and looks simple. I have resent the patch.

PS: I am not on the list. so please keep me in to/cc while replying.

Thanks,
Pavan

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

Reply via email to