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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 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@xxxxxxxxx> 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,
Kazu





says:

obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o

Actually, it's hard to extract some explicit infomation from the commit
3335068f8721 and use them as the first `if` condition, so the kernel
version checking was the final choice.


The kernel version checking is not very good, although it has some similar
code in crash-utility. Once the related kernel code is backported to the
old kernel(such as a stable branch), crash won't work well on these vmcore.
As you mentioned, the kernel version checking is the last resort if there
is no better way.

Thanks.
Lianbo



I saw the commit 3335068f8721 exported two symbols:

+phys_addr_t linear_mapping_va_to_pa(unsigned long x)
+{
+       BUG_ON(!kernel_map.va_pa_offset);
+
+       return ((unsigned long)(x) - kernel_map.va_pa_offset);
+}
+EXPORT_SYMBOL(linear_mapping_va_to_pa);
+
+void *linear_mapping_pa_to_va(unsigned long x)
+{
+       BUG_ON(!kernel_map.va_pa_offset);
+
+       return ((void *)((unsigned long)(x) + kernel_map.va_pa_offset));
+}
+EXPORT_SYMBOL(linear_mapping_pa_to_va);
>

--
Thanks
Song Shuai

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




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux