[Crash-utility] [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()

[lijiang]

On Tue, Aug 15, 2023 at 6:34 PM Song Shuai <suagrfillet at gmail.com> wrote:

>
> 在 2023/8/15 16:42, HAGIO KAZUHITO(萩尾 一仁) 写道:
> > On 2023/08/15 15:44, lijiang wrote:
> >> On Tue, Aug 15, 2023 at 10:40 AM Song Shuai <suagrfillet at gmail.com>
> wrote:
> >>
> >>>
> >>> 在 2023/8/14 16:27, lijiang 写道:
> >>>>       +static void
> >>>>       +riscv64_get_va_kernel_pa_offset(struct machine_specific *ms)
> >>>>       +{
> >>>>       +       unsigned long kernel_version =
> riscv64_get_kernel_version();
> >>>>       +
> >>>>       +       /*
> >>>>       +        * Since Linux v6.4 phys_base is not the physical start
> of
> >>>>       the kernel,
> >>>>       +        * trying to use "va_kernel_pa_offset" to determine the
> >>>>       offset between
> >>>>       +        * kernel virtual and physical addresses.
> >>>>       +        */
> >>>>       +       if (kernel_version >= LINUX(6,4,0)) {
> >>>>
> >>>> Is it possible to detect the existence of the symbol
> >>>> 'linear_mapping_va_to_pa' or 'linear_mapping_pa_to_va' to decide
> reading
> >>>> the value of 'va_kernel_pa_offset'? For example:
> >>>> kernel_symbol_exists()/symbol_exists()
> >>>>
> >>>> if (kernel_symbol_exists("linear_mapping_va_to_pa") ||
> >>>> kernel_symbol_exists("linear_mapping_pa_to_va")) {
> >>>>        string = pc->read_vmcoreinfo("NUMBER(va_kernel_pa_offset)");
> >>>>        ...
> >>>> }
> >>>
> >>> The `linear_mapping_va_to_pa` and `linear_mapping_pa_to_va` symbols
> will
> >>> only be exported when the debug option -- CONFIG_DEBUG_VIRTUAL is
> >>> enabled, otherwise they will expanded as macros. As the kernel Makefile
> >>
> >>
> >> That is really problematic. If so, I tend to read the vmcoreinfo
> directly
> >> as below. I haven't tested it , just an idea.
> >>
> >> string = pc->read_vmcoreinfo("NUMBER(va_kernel_pa_offset)");
> >> if (string) {
> >>       ms->va_kernel_pa_offset = htol(string, QUIET, NULL);
> >>       free(string);
> >>       if (CRASHDEBUG(1))
> >>           fprintf(fp, "NUMBER(va_kernel_pa_offset): %lx\n",
> >> ms->va_kernel_pa_offset);
> >> } else
> >>       ms->va_kernel_pa_offset = ms->kernel_link_addr - ms->phys_base;
> >>
> >> But that depends on how you and Kazu think about it.
> >
> > I was also thinking about a similar way like this:
> >
> >     if ((string = pc->read_vmcoreinfo(...
> >        ms->va_kernel_pa_offset = ...
> >     else if (kernel_vesrion >= LINUX(6,4,0))
> >        error()
> >     else
> >        ms->va_kernel_pa_offset = ...
> > > but personally I don't suppose the kernel commit 3335068f8721 is likely
> > to be backported to an old kernel, so the current patch is also fine to
> > me.  We can fix it when needed.  I'd defer to riscv folks here.
> >
> I agree.
>
> Additionally, this commit (riscv: Use PUD/P4D/PGD pages for the linear
> mapping) was blamed for and fixed by a number of well-known problems
> (such as hibernation, UEFI boot, etc.), so IMO, it's hard to backport it
> to an older kernel.
>
>
Thanks for the confirmation, Kazu and Song.

I have no other issues, so: Ack.

Thanks.
Lianbo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20230815/bbb9c7e3/attachment.htm>