3:27 a.m.
Thank you for the update, Song.
On Mon, Aug 14, 2023 at 9:54 AM <crash-utility-request(a)redhat.com> wrote:
Date: Fri, 4 Aug 2023 17:15:59 +0800
From: Song Shuai <suagrfillet(a)gmail.com>
To: xianting.tian(a)linux.alibaba.com, mick(a)ics.forth.gr,
heinrich.schuchardt(a)canonical.com, guoren(a)kernel.org,
k-hagio-ab(a)nec.com, yixun.lan(a)gmail.com, lijiang(a)redhat.com
Cc: linux-riscv(a)lists.infradead.org, kexec(a)lists.infradead.org,
crash-utility(a)redhat.com, Song Shuai <suagrfillet(a)gmail.com>
Subject: [Crash-utility] [Crash-utility PATCH V2] RISCV64: Use
va_kernel_pa_offset in VTOP()
Message-ID: <20230804091559.3005820-1-suagrfillet(a)gmail.com>
Content-Type: text/plain; charset="US-ASCII"; x-default=true
Since RISC-V Linux v6.4, the commit 3335068f8721 ("riscv: Use
PUD/P4D/PGD pages for the linear mapping") changes phys_ram_base from
the physical start of the kernel to the actual start of the DRAM.
The Crash's VTOP() still uses phys_ram_base and kernel_map.virt_addr
to translate kernel virtual address, that made Crash boot failed with
Linux v6.4 and later version.
Let Linux export kernel_map.va_kernel_pa_offset in v6.5 and backported
v6.4.0 stable, so Crash can use "va_kernel_pa_offset" to translate the
kernel virtual address in VTOP() correctly.
Signed-off-by: Song Shuai <suagrfillet(a)gmail.com>
---
Changes since V1:
- remove unnecessary first kernel version check as Kazu suggested
- amend the commit-msg as Alex suggested
---
defs.h | 4 ++--
riscv64.c | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/defs.h b/defs.h
index 5ee60f1..c07e6d7 100644
--- a/defs.h
+++ b/defs.h
@@ -3662,8 +3662,7 @@ typedef signed int s32;
ulong _X = X;
\
(THIS_KERNEL_VERSION >= LINUX(5,13,0) &&
\
(_X) >= machdep->machspec->kernel_link_addr) ?
\
- (((unsigned
long)(_X)-(machdep->machspec->kernel_link_addr)) + \
- machdep->machspec->phys_base):
\
+ ((unsigned
long)(_X)-(machdep->machspec->va_kernel_pa_offset)): \
(((unsigned long)(_X)-(machdep->kvbase)) +
\
machdep->machspec->phys_base);
\
})
@@ -7021,6 +7020,7 @@ struct machine_specific {
ulong modules_vaddr;
ulong modules_end;
ulong kernel_link_addr;
+ ulong va_kernel_pa_offset;
ulong _page_present;
ulong _page_read;
diff --git a/riscv64.c b/riscv64.c
index 6b9a688..7b5dd3d 100644
--- a/riscv64.c
+++ b/riscv64.c
@@ -418,6 +418,28 @@ error:
error(FATAL, "cannot get vm layout\n");
}
+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)");
...
}
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.
Lianbo
+ char *string;
+ if ((string =
pc->read_vmcoreinfo("NUMBER(va_kernel_pa_offset)"))) {
+ ms->va_kernel_pa_offset = htol(string, QUIET,
NULL);
+ free(string);
+ } else
+ error(FATAL, "cannot read va_kernel_pa_offset\n");
+ }
+ else
+ ms->va_kernel_pa_offset = ms->kernel_link_addr -
ms->phys_base;
+}
+
static int
riscv64_is_kvaddr(ulong vaddr)
{
@@ -1352,6 +1374,7 @@ riscv64_init(int when)
riscv64_get_struct_page_size(machdep->machspec);
riscv64_get_va_bits(machdep->machspec);
riscv64_get_va_range(machdep->machspec);
+ riscv64_get_va_kernel_pa_offset(machdep->machspec);
pt_level_alloc(&machdep->pgd, "cannot malloc pgd
space.");
pt_level_alloc(&machdep->machspec->p4d, "cannot malloc
p4d
space.");
--
2.20.1
9:40 p.m.
New subject: [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()
在 2023/8/14 16:27, lijiang 写道:
Thank you for the update, Song.
On Mon, Aug 14, 2023 at 9:54 AM <crash-utility-request(a)redhat.com
<mailto:crash-utility-request@redhat.com>> wrote:
Date: Fri, 4 Aug 2023 17:15:59 +0800
From: Song Shuai <suagrfillet(a)gmail.com <mailto:suagrfillet@gmail.com>>
To: xianting.tian(a)linux.alibaba.com
<mailto:xianting.tian@linux.alibaba.com>, mick(a)ics.forth.gr
<mailto:mick@ics.forth.gr>,
heinrich.schuchardt(a)canonical.com
<mailto:heinrich.schuchardt@canonical.com>, guoren(a)kernel.org
<mailto:guoren@kernel.org>,
k-hagio-ab(a)nec.com <mailto:k-hagio-ab@nec.com>, yixun.lan(a)gmail.com
<mailto:yixun.lan@gmail.com>, lijiang(a)redhat.com
<mailto:lijiang@redhat.com>
Cc: linux-riscv(a)lists.infradead.org
<mailto:linux-riscv@lists.infradead.org>, kexec(a)lists.infradead.org
<mailto:kexec@lists.infradead.org>,
crash-utility(a)redhat.com <mailto:crash-utility@redhat.com>, Song
Shuai <suagrfillet(a)gmail.com <mailto:suagrfillet@gmail.com>>
Subject: [Crash-utility] [Crash-utility PATCH V2] RISCV64: Use
va_kernel_pa_offset in VTOP()
Message-ID: <20230804091559.3005820-1-suagrfillet(a)gmail.com
<mailto:20230804091559.3005820-1-suagrfillet@gmail.com>>
Content-Type: text/plain; charset="US-ASCII"; x-default=true
Since RISC-V Linux v6.4, the commit 3335068f8721 ("riscv: Use
PUD/P4D/PGD pages for the linear mapping") changes phys_ram_base from
the physical start of the kernel to the actual start of the DRAM.
The Crash's VTOP() still uses phys_ram_base and kernel_map.virt_addr
to translate kernel virtual address, that made Crash boot failed with
Linux v6.4 and later version.
Let Linux export kernel_map.va_kernel_pa_offset in v6.5 and backported
v6.4.0 stable, so Crash can use "va_kernel_pa_offset" to translate the
kernel virtual address in VTOP() correctly.
Signed-off-by: Song Shuai <suagrfillet(a)gmail.com
<mailto:suagrfillet@gmail.com>>
---
Changes since V1:
- remove unnecessary first kernel version check as Kazu suggested
- amend the commit-msg as Alex suggested
---
defs.h | 4 ++--
riscv64.c | 23 +++++++++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/defs.h b/defs.h
index 5ee60f1..c07e6d7 100644
--- a/defs.h
+++ b/defs.h
@@ -3662,8 +3662,7 @@ typedef signed int s32;
ulong _X = X;
\
(THIS_KERNEL_VERSION >= LINUX(5,13,0) &&
\
(_X) >= machdep->machspec->kernel_link_addr) ?
\
- (((unsigned
long)(_X)-(machdep->machspec->kernel_link_addr)) + \
- machdep->machspec->phys_base):
\
+ ((unsigned
long)(_X)-(machdep->machspec->va_kernel_pa_offset)): \
(((unsigned long)(_X)-(machdep->kvbase)) +
\
machdep->machspec->phys_base);
\
})
@@ -7021,6 +7020,7 @@ struct machine_specific {
ulong modules_vaddr;
ulong modules_end;
ulong kernel_link_addr;
+ ulong va_kernel_pa_offset;
ulong _page_present;
ulong _page_read;
diff --git a/riscv64.c b/riscv64.c
index 6b9a688..7b5dd3d 100644
--- a/riscv64.c
+++ b/riscv64.c
@@ -418,6 +418,28 @@ error:
error(FATAL, "cannot get vm layout\n");
}
+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
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.
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.
Lianbo
+ char *string;
+ if ((string =
pc->read_vmcoreinfo("NUMBER(va_kernel_pa_offset)"))) {
+ ms->va_kernel_pa_offset = htol(string,
QUIET, NULL);
+ free(string);
+ } else
+ error(FATAL, "cannot read
va_kernel_pa_offset\n");
+ }
+ else
+ ms->va_kernel_pa_offset = ms->kernel_link_addr -
ms->phys_base;
+}
+
static int
riscv64_is_kvaddr(ulong vaddr)
{
@@ -1352,6 +1374,7 @@ riscv64_init(int when)
riscv64_get_struct_page_size(machdep->machspec);
riscv64_get_va_bits(machdep->machspec);
riscv64_get_va_range(machdep->machspec);
+ riscv64_get_va_kernel_pa_offset(machdep->machspec);
pt_level_alloc(&machdep->pgd, "cannot malloc pgd
space.");
pt_level_alloc(&machdep->machspec->p4d, "cannot
malloc p4d space.");
--
2.20.1
--
Thanks
Song Shuai
1:44 a.m.
New subject: [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()
On Tue, Aug 15, 2023 at 10:40 AM Song Shuai <suagrfillet(a)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.
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);
3:42 a.m.
New subject: [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()
On 2023/08/15 15:44, lijiang wrote:
On Tue, Aug 15, 2023 at 10:40 AM Song Shuai
<suagrfillet(a)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.
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);
>>
5:34 a.m.
New subject: [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()
在 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> 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
7:22 a.m.
New subject: [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> 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>
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
7:25 p.m.
New subject: [PATCH V2] RISCV64: Use va_kernel_pa_offset in VTOP()
On 2023/08/15 21:22, lijiang wrote:
On Tue, Aug 15, 2023 at 6:34 PM Song Shuai
<suagrfillet(a)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(a)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, applied.
https://github.com/crash-utility/crash/commit/ff963b795b3f93b9d1a3cc5ec02...
Thanks,
Kazu
320
days inactive
322
days old
devel@lists.crash-utility.osci.io
6 comments
3 participants
participants (3)
-
HAGIO KAZUHITO(萩尾 一仁) -
lijiang -
Song Shuai