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

On Tue, Aug 15, 2023 at 6:34 PM Song Shuai <suagrfillet(a)gmail.com&gt; 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(a)gmail.com&gt; > 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.