On Tue, Aug 15, 2023 at 6:34 PM Song Shuai <suagrfillet@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@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