2022-08-02 12:59:49

by Xianting Tian

[permalink] [raw]
Subject: [PATCH V5 5/6] riscv: crash_core: Export kernel vm layout, phys_ram_base

These infos are needed by the kdump crash tool. Since these values change
from time to time, it is preferable to export them via vmcoreinfo than to
change the crash's code frequently.

Signed-off-by: Xianting Tian <[email protected]>
---
.../admin-guide/kdump/vmcoreinfo.rst | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
index 8419019b6a88..6b76284a503c 100644
--- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
+++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
@@ -595,3 +595,34 @@ X2TLB
-----

Indicates whether the crashed kernel enabled SH extended mode.
+
+RISCV64
+=======
+
+VA_BITS
+-------
+
+The maximum number of bits for virtual addresses. Used to compute the
+virtual memory ranges.
+
+PAGE_OFFSET
+-----------
+
+Indicates the virtual kernel start address of direct-mapped RAM region.
+
+phys_ram_base
+-------------
+
+Indicates the start physical RAM address.
+
+MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END
+-----------------------------------------------------------------------------
+KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
+----------------------------------------------------------------------
+
+Used to get the correct ranges:
+ MODULES_VADDR ~ MODULES_END : Kernel module space.
+ VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
+ VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
+ KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
+ KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.
--
2.17.1



2022-08-09 21:23:22

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V5 5/6] riscv: crash_core: Export kernel vm layout, phys_ram_base

On 02/08/2022 13:18, Xianting Tian wrote:
> riscv: crash_core: Export kernel vm layout, phys_ram_base

Can you please just use RISC-V: for the whole series, my OCD
hates the mix haha.

> These infos are needed by the kdump crash tool. Since these values change
> from time to time, it is preferable to export them via vmcoreinfo than to
> change the crash's code frequently.

This commit description doesn't seem to match the patches at all.
I don't see any exporting happening here at all - this is documenting
the export. Maybe I am just misunderstanding, but this commit message
just doesn't seem to match the change. Secondly, should the subject not
be something like "docs: admin-guide: add riscv crash kernel yada yada"?
Maybe the current subject lime that explains the lack of a review from
the docs maintainer?

Thanks,
Conor.

>
> Signed-off-by: Xianting Tian <[email protected]>
> ---
> .../admin-guide/kdump/vmcoreinfo.rst | 31 +++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> index 8419019b6a88..6b76284a503c 100644
> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> @@ -595,3 +595,34 @@ X2TLB
> -----
>
> Indicates whether the crashed kernel enabled SH extended mode.
> +
> +RISCV64
> +=======
> +
> +VA_BITS
> +-------
> +
> +The maximum number of bits for virtual addresses. Used to compute the
> +virtual memory ranges.
> +
> +PAGE_OFFSET
> +-----------
> +
> +Indicates the virtual kernel start address of direct-mapped RAM region.
> +
> +phys_ram_base
> +-------------
> +
> +Indicates the start physical RAM address.
> +
> +MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END
> +-----------------------------------------------------------------------------
> +KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
> +----------------------------------------------------------------------
> +
> +Used to get the correct ranges:
> + MODULES_VADDR ~ MODULES_END : Kernel module space.
> + VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
> + VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
> + KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
> + KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.

2022-08-10 12:30:26

by Xianting Tian

[permalink] [raw]
Subject: Re: [PATCH V5 5/6] riscv: crash_core: Export kernel vm layout, phys_ram_base


在 2022/8/10 上午5:20, [email protected] 写道:
> On 02/08/2022 13:18, Xianting Tian wrote:
>> riscv: crash_core: Export kernel vm layout, phys_ram_base
> Can you please just use RISC-V: for the whole series, my OCD
> hates the mix haha.

Sorry, I missed it,  you ever pointed out this poblem in the comments of
V3, I will definitely fix the issue in V6.

>
>> These infos are needed by the kdump crash tool. Since these values change
>> from time to time, it is preferable to export them via vmcoreinfo than to
>> change the crash's code frequently.
> This commit description doesn't seem to match the patches at all.
> I don't see any exporting happening here at all - this is documenting
The real export code is in 2/6 patch, yes this is the document just
describe the exporting contents
> the export. Maybe I am just misunderstanding, but this commit message
> just doesn't seem to match the change. Secondly, should the subject not
> be something like "docs: admin-guide: add riscv crash kernel yada yada"?
the commit log for the file
'Documentation/admin-guide/kdump/vmcoreinfo.rst' is not unified in the
commit history of the file. I agree with you 'docs: admin-guide: xxx' is
better,  will fix it V6.
> Maybe the current subject lime that explains the lack of a review from
> the docs maintainer?
yes, it is possible :)
>
> Thanks,
> Conor.
For your comments for other patches, I will fix it in v6, thanks
>
>> Signed-off-by: Xianting Tian <[email protected]>
>> ---
>> .../admin-guide/kdump/vmcoreinfo.rst | 31 +++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>> index 8419019b6a88..6b76284a503c 100644
>> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
>> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>> @@ -595,3 +595,34 @@ X2TLB
>> -----
>>
>> Indicates whether the crashed kernel enabled SH extended mode.
>> +
>> +RISCV64
>> +=======
>> +
>> +VA_BITS
>> +-------
>> +
>> +The maximum number of bits for virtual addresses. Used to compute the
>> +virtual memory ranges.
>> +
>> +PAGE_OFFSET
>> +-----------
>> +
>> +Indicates the virtual kernel start address of direct-mapped RAM region.
>> +
>> +phys_ram_base
>> +-------------
>> +
>> +Indicates the start physical RAM address.
>> +
>> +MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END
>> +-----------------------------------------------------------------------------
>> +KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
>> +----------------------------------------------------------------------
>> +
>> +Used to get the correct ranges:
>> + MODULES_VADDR ~ MODULES_END : Kernel module space.
>> + VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
>> + VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
>> + KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
>> + KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.

2022-08-11 03:45:12

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: [PATCH V5 5/6] riscv: crash_core: Export kernel vm layout, phys_ram_base

On Tue, Aug 02, 2022 at 08:18:17PM +0800, Xianting Tian wrote:
> These infos are needed by the kdump crash tool. Since these values change
> from time to time, it is preferable to export them via vmcoreinfo than to
> change the crash's code frequently.
>

I have to agree with Conor.Dooley, that this patch is misleading (I see
documentation instead of real export). So IMO, the patch subject should
be "Documentation: kdump: describe VMCOREINFO export for RISCV64".

For MODULES_VADDR and friends, the doc can be improved, like:

diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
index 6b76284a503ca5..6694acc32c3588 100644
--- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
+++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
@@ -615,14 +615,13 @@ phys_ram_base

Indicates the start physical RAM address.

-MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END
------------------------------------------------------------------------------
-KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
-----------------------------------------------------------------------
+MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END|KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
+----------------------------------------------------------------------------------------------------------------------------------------------------

Used to get the correct ranges:
- MODULES_VADDR ~ MODULES_END : Kernel module space.
- VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
- VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
- KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
- KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.
+
+ * MODULES_VADDR ~ MODULES_END : Kernel module space.
+ * VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
+ * VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
+ * KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
+ * KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.

Thanks.

--
An old man doll... just what I always wanted! - Clara

2022-08-11 03:59:37

by Xianting Tian

[permalink] [raw]
Subject: Re: [PATCH V5 5/6] riscv: crash_core: Export kernel vm layout, phys_ram_base


在 2022/8/11 上午11:06, Bagas Sanjaya 写道:
> On Tue, Aug 02, 2022 at 08:18:17PM +0800, Xianting Tian wrote:
>> These infos are needed by the kdump crash tool. Since these values change
>> from time to time, it is preferable to export them via vmcoreinfo than to
>> change the crash's code frequently.
>>
> I have to agree with Conor.Dooley, that this patch is misleading (I see
> documentation instead of real export). So IMO, the patch subject should
> be "Documentation: kdump: describe VMCOREINFO export for RISCV64".
>
> For MODULES_VADDR and friends, the doc can be improved, like:
>
> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> index 6b76284a503ca5..6694acc32c3588 100644
> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> @@ -615,14 +615,13 @@ phys_ram_base
>
> Indicates the start physical RAM address.
>
> -MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END
> ------------------------------------------------------------------------------
> -KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
> -----------------------------------------------------------------------
> +MODULES_VADDR|MODULES_END|VMALLOC_START|VMALLOC_END|VMEMMAP_START|VMEMMAP_END|KASAN_SHADOW_START|KASAN_SHADOW_END|KERNEL_LINK_ADDR|ADDRESS_SPACE_END
> +----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Used to get the correct ranges:
> - MODULES_VADDR ~ MODULES_END : Kernel module space.
> - VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
> - VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
> - KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
> - KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.
> +
> + * MODULES_VADDR ~ MODULES_END : Kernel module space.
> + * VMALLOC_START ~ VMALLOC_END : vmalloc() / ioremap() space.
> + * VMEMMAP_START ~ VMEMMAP_END : vmemmap region, used for struct page array.
> + * KASAN_SHADOW_START ~ KASAN_SHADOW_END : kasan shadow space.
> + * KERNEL_LINK_ADDR ~ ADDRESS_SPACE_END : Kernel link and BPF space.
Thanks for the comment, I will fix it in V6
>
> Thanks.
>