[PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-19 Thread Xianting Tian
Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for different kernel
version as below. For pagetable levels, it sets sv57 by default and falls
back to setting sv48 at boot time if sv57 is not supported by the hardware.

For ram base, the default value is 0x8020 for qemu riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

 * Linux Kernel 5.18 ~
 *  PGTABLE_LEVELS = 5
 *  PAGE_OFFSET = 0xff60
 * Linux Kernel 5.17 ~
 *  PGTABLE_LEVELS = 4
 *  PAGE_OFFSET = 0xaf80
 * Linux Kernel 4.19 ~
 *  PGTABLE_LEVELS = 3
 *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and version to version,
it is preferable to export them via vmcoreinfo than to change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
 arch/riscv/kernel/Makefile |  1 +
 arch/riscv/kernel/crash_core.c | 23 +++
 2 files changed, 24 insertions(+)
 create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)+= kgdb.o
 obj-$(CONFIG_KEXEC_CORE)   += kexec_relocate.o crash_save_regs.o 
machine_kexec.o
 obj-$(CONFIG_KEXEC_FILE)   += elf_kexec.o machine_kexec_file.o
 obj-$(CONFIG_CRASH_DUMP)   += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)   += crash_core.o
 
 obj-$(CONFIG_JUMP_LABEL)   += jump_label.o
 
diff --git a/arch/riscv/kernel/crash_core.c b/arch/riscv/kernel/crash_core.c
new file mode 100644
index ..3e889d0ed7bd
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+#endif
+
+   if (IS_ENABLED(CONFIG_64BIT))
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);
+}
-- 
2.17.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-19 Thread Baoquan He
On 10/19/22 at 06:36pm, Xianting Tian wrote:
> Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, VMALLOC,
> VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.
> 
> Default pagetable levels and PAGE_OFFSET aren't same for different kernel
> version as below. For pagetable levels, it sets sv57 by default and falls
> back to setting sv48 at boot time if sv57 is not supported by the hardware.
> 
> For ram base, the default value is 0x8020 for qemu riscv64 env and,
> for example, is 0x20 on the XuanTie 910 CPU.
> 
>  * Linux Kernel 5.18 ~
>  *  PGTABLE_LEVELS = 5
>  *  PAGE_OFFSET = 0xff60
>  * Linux Kernel 5.17 ~
>  *  PGTABLE_LEVELS = 4
>  *  PAGE_OFFSET = 0xaf80
>  * Linux Kernel 4.19 ~
>  *  PGTABLE_LEVELS = 3
>  *  PAGE_OFFSET = 0xffe0
> 
> Since these configurations change from time to time and version to version,
> it is preferable to export them via vmcoreinfo than to change the crash's
> code frequently, it can simplify the development of crash tool.
> 
> Signed-off-by: Xianting Tian 
> ---
>  arch/riscv/kernel/Makefile |  1 +
>  arch/riscv/kernel/crash_core.c | 23 +++
>  2 files changed, 24 insertions(+)
>  create mode 100644 arch/riscv/kernel/crash_core.c
> 
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index db6e4b1294ba..4cf303a779ab 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)  += kgdb.o
>  obj-$(CONFIG_KEXEC_CORE) += kexec_relocate.o crash_save_regs.o 
> machine_kexec.o
>  obj-$(CONFIG_KEXEC_FILE) += elf_kexec.o machine_kexec_file.o
>  obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> +obj-$(CONFIG_CRASH_CORE) += crash_core.o
>  
>  obj-$(CONFIG_JUMP_LABEL) += jump_label.o
>  
> diff --git a/arch/riscv/kernel/crash_core.c b/arch/riscv/kernel/crash_core.c
> new file mode 100644
> index ..3e889d0ed7bd
> --- /dev/null
> +++ b/arch/riscv/kernel/crash_core.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include 
> +#include 
> +
> +void arch_crash_save_vmcoreinfo(void)
> +{
> + VMCOREINFO_NUMBER(VA_BITS);
> + VMCOREINFO_NUMBER(phys_ram_base);
> +
> + vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
> + vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> + vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
> + vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
> + vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
> +#ifdef CONFIG_64BIT
> + vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
> + vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
> +#endif
> +
> + if (IS_ENABLED(CONFIG_64BIT))
> + vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
> KERNEL_LINK_ADDR);

Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
ifdeffery scope, with that you can save one line of "IS_ENABLED(CONFIG_64BIT)".


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-19 Thread Xianting Tian


在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for different kernel
version as below. For pagetable levels, it sets sv57 by default and falls
back to setting sv48 at boot time if sv57 is not supported by the hardware.

For ram base, the default value is 0x8020 for qemu riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

  * Linux Kernel 5.18 ~
  *  PGTABLE_LEVELS = 5
  *  PAGE_OFFSET = 0xff60
  * Linux Kernel 5.17 ~
  *  PGTABLE_LEVELS = 4
  *  PAGE_OFFSET = 0xaf80
  * Linux Kernel 4.19 ~
  *  PGTABLE_LEVELS = 3
  *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and version to version,
it is preferable to export them via vmcoreinfo than to change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
  arch/riscv/kernel/Makefile |  1 +
  arch/riscv/kernel/crash_core.c | 23 +++
  2 files changed, 24 insertions(+)
  create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)+= kgdb.o
  obj-$(CONFIG_KEXEC_CORE)  += kexec_relocate.o crash_save_regs.o 
machine_kexec.o
  obj-$(CONFIG_KEXEC_FILE)  += elf_kexec.o machine_kexec_file.o
  obj-$(CONFIG_CRASH_DUMP)  += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)   += crash_core.o
  
  obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o
  
diff --git a/arch/riscv/kernel/crash_core.c b/arch/riscv/kernel/crash_core.c

new file mode 100644
index ..3e889d0ed7bd
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+#endif
+
+   if (IS_ENABLED(CONFIG_64BIT))
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);

Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
ifdeffery scope, with that you can save one line of "IS_ENABLED(CONFIG_64BIT)".
I followed the rule in print_vm_layout() of arch/riscv/mm/init.c, which 
used IS_ENABLED when print the value of KERNEL_LINK_ADDR.


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-19 Thread Baoquan He
On 10/20/22 at 10:17am, Xianting Tian wrote:
> 
> 在 2022/10/20 上午10:08, Baoquan He 写道:
> > On 10/19/22 at 06:36pm, Xianting Tian wrote:
> > > Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, 
> > > VMALLOC,
> > > VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.
> > > 
> > > Default pagetable levels and PAGE_OFFSET aren't same for different kernel
> > > version as below. For pagetable levels, it sets sv57 by default and falls
> > > back to setting sv48 at boot time if sv57 is not supported by the 
> > > hardware.
> > > 
> > > For ram base, the default value is 0x8020 for qemu riscv64 env and,
> > > for example, is 0x20 on the XuanTie 910 CPU.
> > > 
> > >   * Linux Kernel 5.18 ~
> > >   *  PGTABLE_LEVELS = 5
> > >   *  PAGE_OFFSET = 0xff60
> > >   * Linux Kernel 5.17 ~
> > >   *  PGTABLE_LEVELS = 4
> > >   *  PAGE_OFFSET = 0xaf80
> > >   * Linux Kernel 4.19 ~
> > >   *  PGTABLE_LEVELS = 3
> > >   *  PAGE_OFFSET = 0xffe0
> > > 
> > > Since these configurations change from time to time and version to 
> > > version,
> > > it is preferable to export them via vmcoreinfo than to change the crash's
> > > code frequently, it can simplify the development of crash tool.
> > > 
> > > Signed-off-by: Xianting Tian 
> > > ---
> > >   arch/riscv/kernel/Makefile |  1 +
> > >   arch/riscv/kernel/crash_core.c | 23 +++
> > >   2 files changed, 24 insertions(+)
> > >   create mode 100644 arch/riscv/kernel/crash_core.c
> > > 
> > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > > index db6e4b1294ba..4cf303a779ab 100644
> > > --- a/arch/riscv/kernel/Makefile
> > > +++ b/arch/riscv/kernel/Makefile
> > > @@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)  += kgdb.o
> > >   obj-$(CONFIG_KEXEC_CORE)+= kexec_relocate.o crash_save_regs.o 
> > > machine_kexec.o
> > >   obj-$(CONFIG_KEXEC_FILE)+= elf_kexec.o machine_kexec_file.o
> > >   obj-$(CONFIG_CRASH_DUMP)+= crash_dump.o
> > > +obj-$(CONFIG_CRASH_CORE) += crash_core.o
> > >   obj-$(CONFIG_JUMP_LABEL)+= jump_label.o
> > > diff --git a/arch/riscv/kernel/crash_core.c 
> > > b/arch/riscv/kernel/crash_core.c
> > > new file mode 100644
> > > index ..3e889d0ed7bd
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/crash_core.c
> > > @@ -0,0 +1,23 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +void arch_crash_save_vmcoreinfo(void)
> > > +{
> > > + VMCOREINFO_NUMBER(VA_BITS);
> > > + VMCOREINFO_NUMBER(phys_ram_base);
> > > +
> > > + vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
> > > + vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> > > + vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
> > > + vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
> > > + vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
> > > +#ifdef CONFIG_64BIT
> > > + vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
> > > + vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
> > > +#endif
> > > +
> > > + if (IS_ENABLED(CONFIG_64BIT))
> > > + vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
> > > KERNEL_LINK_ADDR);
> > Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
> > ifdeffery scope, with that you can save one line of 
> > "IS_ENABLED(CONFIG_64BIT)".
> I followed the rule in print_vm_layout() of arch/riscv/mm/init.c, which used
> IS_ENABLED when print the value of KERNEL_LINK_ADDR.
> 

I see. There's PAGE_OFFSET in the middle. Thanks.

print_ml("lowmem", (unsigned long)PAGE_OFFSET,
(unsigned long)high_memory)

So now, do you think if it's necessary to have another
IS_ENABLED(CONFIG_64BIT) in the current arch_crash_save_vmcoreinfo()?


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-19 Thread Xianting Tian


在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for different kernel
version as below. For pagetable levels, it sets sv57 by default and falls
back to setting sv48 at boot time if sv57 is not supported by the hardware.

For ram base, the default value is 0x8020 for qemu riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

   * Linux Kernel 5.18 ~
   *  PGTABLE_LEVELS = 5
   *  PAGE_OFFSET = 0xff60
   * Linux Kernel 5.17 ~
   *  PGTABLE_LEVELS = 4
   *  PAGE_OFFSET = 0xaf80
   * Linux Kernel 4.19 ~
   *  PGTABLE_LEVELS = 3
   *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and version to version,
it is preferable to export them via vmcoreinfo than to change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
   arch/riscv/kernel/Makefile |  1 +
   arch/riscv/kernel/crash_core.c | 23 +++
   2 files changed, 24 insertions(+)
   create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)+= kgdb.o
   obj-$(CONFIG_KEXEC_CORE) += kexec_relocate.o crash_save_regs.o 
machine_kexec.o
   obj-$(CONFIG_KEXEC_FILE) += elf_kexec.o machine_kexec_file.o
   obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)   += crash_core.o
   obj-$(CONFIG_JUMP_LABEL) += jump_label.o
diff --git a/arch/riscv/kernel/crash_core.c b/arch/riscv/kernel/crash_core.c
new file mode 100644
index ..3e889d0ed7bd
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+#endif
+
+   if (IS_ENABLED(CONFIG_64BIT))
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);

Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
ifdeffery scope, with that you can save one line of "IS_ENABLED(CONFIG_64BIT)".

I followed the rule in print_vm_layout() of arch/riscv/mm/init.c, which used
IS_ENABLED when print the value of KERNEL_LINK_ADDR.


I see. There's PAGE_OFFSET in the middle. Thanks.

 print_ml("lowmem", (unsigned long)PAGE_OFFSET,
 (unsigned long)high_memory)

So now, do you think if it's necessary to have another
IS_ENABLED(CONFIG_64BIT) in the current arch_crash_save_vmcoreinfo()?


For which MACRO?  I think current code for PAGE_OFFSET is OK.


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-20 Thread Guo Ren
Tested-by: Guo Ren 

Don't forget the another two kdump fixups [1]
[1] https://lore.kernel.org/all/20221020141603.2856206-3-guo...@kernel.org/

Only the patch without [1]:
crash> help -r
CPU 0: [OFFLINE]

CPU 1:
epc : 80009ff0 ra : 800b789a sp : ff201098bb40
 gp : 815fca60 tp : ff600468 t0 : 3c5b
 t1 :  t2 : 663c s0 : ff201098bc90
 s1 : 81600798 a0 : ff201098bb48 a1 : 
 a2 :  a3 : 0001 a4 : 
 a5 : ff6004690800 a6 :  a7 : 
 s2 : ff201098bb48 s3 : 81093ec8 s4 : 816004ac
 s5 :  s6 : 0007 s7 : 80e7f720
 s8 : 00fff3f0 s9 : 0007 s10: 00b98700
 s11: 0001 t3 : 819a8097 t4 : 819a8097
 t5 : 819a8098 t6 : ff201098b9a8

CPU 2: [OFFLINE]

CPU 3: [OFFLINE]

The patch with [1]:
crash> help -r
CPU 0:
epc : 80003f34 ra : 808caa7c sp : 81403eb0
 gp : 815fcb48 tp : 81413400 t0 : 
 t1 :  t2 :  s0 : 81403ec0
 s1 :  a0 :  a1 : 
 a2 :  a3 :  a4 : 
 a5 :  a6 :  a7 : 
 s2 : 816001c8 s3 : 81600370 s4 : 80c32e18
 s5 : 819d3018 s6 : 810e2110 s7 : 
 s8 :  s9 : 80039eac s10: 
 s11:  t3 :  t4 : 
 t5 :  t6 : 

CPU 1:
epc : 80003f34 ra : 808caa7c sp : ff200068bf30
 gp : 815fcb48 tp : ff600240d400 t0 : 
 t1 :  t2 :  s0 : ff200068bf40
 s1 : 0001 a0 :  a1 : 
 a2 :  a3 :  a4 : 
 a5 :  a6 :  a7 : 
 s2 : 816001c8 s3 : 81600370 s4 : 80c32e18
 s5 : 819d3018 s6 : 810e2110 s7 : 
 s8 :  s9 : 80039ea8 s10: 
 s11:  t3 :  t4 : 
 t5 :  t6 : 

CPU 2:
epc : 80003f34 ra : 808caa7c sp : ff2000693f30
 gp : 815fcb48 tp : ff600240e900 t0 : 
 t1 :  t2 :  s0 : ff2000693f40
 s1 : 0002 a0 :  a1 : 
 a2 :  a3 :  a4 : 
 a5 :  a6 :  a7 : 
 s2 : 816001c8 s3 : 81600370 s4 : 80c32e18
 s5 : 819d3018 s6 : 810e2110 s7 : 
 s8 :  s9 : 80039eb0 s10: 
 s11:  t3 :  t4 : 
 t5 :  t6 : 

CPU 3:
epc : 8000a1e4 ra : 800b7bba sp : ff20109bbb40
 gp : 815fcb48 tp : ff600373aa00 t0 : 3c5b
 t1 :  t2 : 663c s0 : ff20109bbc90
 s1 : 816007a0 a0 : ff20109bbb48 a1 : 
 a2 :  a3 : 0001 a4 : 
 a5 : ff6002c61c00 a6 :  a7 : 
 s2 : ff20109bbb48 s3 : 810941a8 s4 : 816004b4
 s5 :  s6 : 0007 s7 : 80e7f7a0
 s8 : 00fff3f0 s9 : 0007 s10: 00b98700
 s11: 0001 t3 : 819a8097 t4 : 819a8097
 t5 : 819a8098 t6 : ff20109bb9a8

On Wed, Oct 19, 2022 at 6:36 PM Xianting Tian
 wrote:
>
> Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, VMALLOC,
> VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for vmcore.
>
> Default pagetable levels and PAGE_OFFSET aren't same for different kernel
> version as below. For pagetable levels, it sets sv57 by default and falls
> back to setting sv48 at boot time if sv57 is not supported by the hardware.
>
> For ram base, the default value is 0x8020 for qemu riscv64 env and,
> for example, is 0x20 on the XuanTie 910 CPU.
>
>  * Linux Kernel 5.18 ~
>  *  PGTABLE_LEVELS = 5
>  *  PAGE_OFFSET = 0xff60
>  * Linux Kernel 5.17 ~
>  *  PGTABLE_LEVELS = 4
>  *  PAGE_OFFSET = 0xaf80
>  * Linux Kernel 4.19 ~
>  *  PGTABLE_LEVELS = 3
>  *  PAGE_OFFSET = 

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Xianting Tian

Hi Palmer, Conor

Is this version OK for you? Do you plan to apply this patch set? thanks

在 2022/10/20 下午12:40, Xianting Tian 写道:


在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:
Add arch_crash_save_vmcoreinfo(), which exports VM layout(MODULES, 
VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram base for 
vmcore.


Default pagetable levels and PAGE_OFFSET aren't same for different 
kernel
version as below. For pagetable levels, it sets sv57 by default 
and falls
back to setting sv48 at boot time if sv57 is not supported by the 
hardware.


For ram base, the default value is 0x8020 for qemu riscv64 env 
and,

for example, is 0x20 on the XuanTie 910 CPU.

   * Linux Kernel 5.18 ~
   *  PGTABLE_LEVELS = 5
   *  PAGE_OFFSET = 0xff60
   * Linux Kernel 5.17 ~
   *  PGTABLE_LEVELS = 4
   *  PAGE_OFFSET = 0xaf80
   * Linux Kernel 4.19 ~
   *  PGTABLE_LEVELS = 3
   *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and version to 
version,
it is preferable to export them via vmcoreinfo than to change the 
crash's

code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
   arch/riscv/kernel/Makefile |  1 +
   arch/riscv/kernel/crash_core.c | 23 +++
   2 files changed, 24 insertions(+)
   create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)    += kgdb.o
   obj-$(CONFIG_KEXEC_CORE)    += kexec_relocate.o 
crash_save_regs.o machine_kexec.o

   obj-$(CONFIG_KEXEC_FILE)    += elf_kexec.o machine_kexec_file.o
   obj-$(CONFIG_CRASH_DUMP)    += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)    += crash_core.o
   obj-$(CONFIG_JUMP_LABEL)    += jump_label.o
diff --git a/arch/riscv/kernel/crash_core.c 
b/arch/riscv/kernel/crash_core.c

new file mode 100644
index ..3e889d0ed7bd
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+void arch_crash_save_vmcoreinfo(void)
+{
+    VMCOREINFO_NUMBER(VA_BITS);
+    VMCOREINFO_NUMBER(phys_ram_base);
+
+    vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", 
PAGE_OFFSET);
+ vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", 
VMALLOC_START);
+    vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", 
VMALLOC_END);
+ vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", 
VMEMMAP_START);
+    vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", 
VMEMMAP_END);

+#ifdef CONFIG_64BIT
+ vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", 
MODULES_VADDR);
+    vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", 
MODULES_END);

+#endif
+
+    if (IS_ENABLED(CONFIG_64BIT))
+ vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);

Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
ifdeffery scope, with that you can save one line of 
"IS_ENABLED(CONFIG_64BIT)".
I followed the rule in print_vm_layout() of arch/riscv/mm/init.c, 
which used

IS_ENABLED when print the value of KERNEL_LINK_ADDR.


I see. There's PAGE_OFFSET in the middle. Thanks.

 print_ml("lowmem", (unsigned long)PAGE_OFFSET,
 (unsigned long)high_memory)

So now, do you think if it's necessary to have another
IS_ENABLED(CONFIG_64BIT) in the current arch_crash_save_vmcoreinfo()?


For which MACRO?  I think current code for PAGE_OFFSET is OK.



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Conor Dooley
On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:
> Hi Palmer, Conor
> 
> Is this version OK for you?

The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
odd & I notice Baoquan brought it up too. I didn't (and won't) give you
a reviewed by on these patches because I don't understand the area well
enough. The general nitpickery seems to be sorted though.

Thanks,
Conor.

> 
> 在 2022/10/20 下午12:40, Xianting Tian 写道:
> > 
> > 在 2022/10/20 上午11:05, Baoquan He 写道:
> > > On 10/20/22 at 10:17am, Xianting Tian wrote:
> > > > 在 2022/10/20 上午10:08, Baoquan He 写道:
> > > > > On 10/19/22 at 06:36pm, Xianting Tian wrote:
> > > > > > Add arch_crash_save_vmcoreinfo(), which exports VM
> > > > > > layout(MODULES, VMALLOC,
> > > > > > VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
> > > > > > base for vmcore.
> > > > > > 
> > > > > > Default pagetable levels and PAGE_OFFSET aren't same for
> > > > > > different kernel
> > > > > > version as below. For pagetable levels, it sets sv57 by
> > > > > > default and falls
> > > > > > back to setting sv48 at boot time if sv57 is not
> > > > > > supported by the hardware.
> > > > > > 
> > > > > > For ram base, the default value is 0x8020 for qemu
> > > > > > riscv64 env and,
> > > > > > for example, is 0x20 on the XuanTie 910 CPU.
> > > > > > 
> > > > > >    * Linux Kernel 5.18 ~
> > > > > >    *  PGTABLE_LEVELS = 5
> > > > > >    *  PAGE_OFFSET = 0xff60
> > > > > >    * Linux Kernel 5.17 ~
> > > > > >    *  PGTABLE_LEVELS = 4
> > > > > >    *  PAGE_OFFSET = 0xaf80
> > > > > >    * Linux Kernel 4.19 ~
> > > > > >    *  PGTABLE_LEVELS = 3
> > > > > >    *  PAGE_OFFSET = 0xffe0
> > > > > > 
> > > > > > Since these configurations change from time to time and
> > > > > > version to version,
> > > > > > it is preferable to export them via vmcoreinfo than to
> > > > > > change the crash's
> > > > > > code frequently, it can simplify the development of crash tool.
> > > > > > 
> > > > > > Signed-off-by: Xianting Tian 
> > > > > > ---
> > > > > >    arch/riscv/kernel/Makefile |  1 +
> > > > > >    arch/riscv/kernel/crash_core.c | 23 +++
> > > > > >    2 files changed, 24 insertions(+)
> > > > > >    create mode 100644 arch/riscv/kernel/crash_core.c
> > > > > > 
> > > > > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > > > > > index db6e4b1294ba..4cf303a779ab 100644
> > > > > > --- a/arch/riscv/kernel/Makefile
> > > > > > +++ b/arch/riscv/kernel/Makefile
> > > > > > @@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)    += kgdb.o
> > > > > >    obj-$(CONFIG_KEXEC_CORE)    += kexec_relocate.o
> > > > > > crash_save_regs.o machine_kexec.o
> > > > > >    obj-$(CONFIG_KEXEC_FILE)    += elf_kexec.o machine_kexec_file.o
> > > > > >    obj-$(CONFIG_CRASH_DUMP)    += crash_dump.o
> > > > > > +obj-$(CONFIG_CRASH_CORE)    += crash_core.o
> > > > > >    obj-$(CONFIG_JUMP_LABEL)    += jump_label.o
> > > > > > diff --git a/arch/riscv/kernel/crash_core.c
> > > > > > b/arch/riscv/kernel/crash_core.c
> > > > > > new file mode 100644
> > > > > > index ..3e889d0ed7bd
> > > > > > --- /dev/null
> > > > > > +++ b/arch/riscv/kernel/crash_core.c
> > > > > > @@ -0,0 +1,23 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +
> > > > > > +#include 
> > > > > > +#include 
> > > > > > +
> > > > > > +void arch_crash_save_vmcoreinfo(void)
> > > > > > +{
> > > > > > +    VMCOREINFO_NUMBER(VA_BITS);
> > > > > > +    VMCOREINFO_NUMBER(phys_ram_base);
> > > > > > +
> > > > > > +   
> > > > > > vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n",
> > > > > > PAGE_OFFSET);
> > > > > > + vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n",
> > > > > > VMALLOC_START);
> > > > > > +   
> > > > > > vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n",
> > > > > > VMALLOC_END);
> > > > > > + vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n",
> > > > > > VMEMMAP_START);
> > > > > > +   
> > > > > > vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n",
> > > > > > VMEMMAP_END);
> > > > > > +#ifdef CONFIG_64BIT
> > > > > > + vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n",
> > > > > > MODULES_VADDR);
> > > > > > +   
> > > > > > vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n",
> > > > > > MODULES_END);
> > > > > > +#endif
> > > > > > +
> > > > > > +    if (IS_ENABLED(CONFIG_64BIT))
> > > > > > +
> > > > > > vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n",
> > > > > > KERNEL_LINK_ADDR);
> > > > > Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
> > > > > ifdeffery scope, with that you can save one line of
> > > > > "IS_ENABLED(CONFIG_64BIT)".
> > > > I followed the rule in print_vm_layout() of
> > > > arch/riscv/mm/init.c, which used
> > > > IS_ENABLED when print the value of KERNEL_LINK_ADDR.
> > > > 
> > > I see. There's PAGE_OFFSET in the middle. Thanks.
> > > 
> > >  print_ml("lowmem", (unsigned long)PAGE_

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Xianting Tian


在 2022/10/26 下午5:25, Conor Dooley 写道:

On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:

Hi Palmer, Conor

Is this version OK for you?

The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
odd & I notice Baoquan brought it up too. I didn't (and won't) give you
a reviewed by on these patches because I don't understand the area well
enough. The general nitpickery seems to be sorted though.


I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for 
CONFIG_64BIT and !CONFIG_64BIT.


Maybe we can remove IS_ENABLED(CONFIG_64BIT)

arch/riscv/include/asm/pgtable.h
#define ADDRESS_SPACE_END   (UL(-1))
#ifdef CONFIG_64BIT
/* Leave 2GB for kernel and BPF at the end of the address space */
#define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
#else
#define KERNEL_LINK_ADDR    PAGE_OFFSET
#endif

arch/riscv/include/asm/page.h
#ifdef CONFIG_64BIT
#ifdef CONFIG_MMU
#define PAGE_OFFSET kernel_map.page_offset
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif
/*
 * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address 
space so

 * define the PAGE_OFFSET value for SV39.
 */
#define PAGE_OFFSET_L4  _AC(0xaf80, UL)
#define PAGE_OFFSET_L3  _AC(0xffd8, UL)
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif /* CONFIG_64BIT */



Thanks,
Conor.


在 2022/10/20 下午12:40, Xianting Tian 写道:

在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM
layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for
different kernel
version as below. For pagetable levels, it sets sv57 by
default and falls
back to setting sv48 at boot time if sv57 is not
supported by the hardware.

For ram base, the default value is 0x8020 for qemu
riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

    * Linux Kernel 5.18 ~
    *  PGTABLE_LEVELS = 5
    *  PAGE_OFFSET = 0xff60
    * Linux Kernel 5.17 ~
    *  PGTABLE_LEVELS = 4
    *  PAGE_OFFSET = 0xaf80
    * Linux Kernel 4.19 ~
    *  PGTABLE_LEVELS = 3
    *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and
version to version,
it is preferable to export them via vmcoreinfo than to
change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
    arch/riscv/kernel/Makefile |  1 +
    arch/riscv/kernel/crash_core.c | 23 +++
    2 files changed, 24 insertions(+)
    create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)    += kgdb.o
    obj-$(CONFIG_KEXEC_CORE)    += kexec_relocate.o
crash_save_regs.o machine_kexec.o
    obj-$(CONFIG_KEXEC_FILE)    += elf_kexec.o machine_kexec_file.o
    obj-$(CONFIG_CRASH_DUMP)    += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)    += crash_core.o
    obj-$(CONFIG_JUMP_LABEL)    += jump_label.o
diff --git a/arch/riscv/kernel/crash_core.c
b/arch/riscv/kernel/crash_core.c
new file mode 100644
index ..3e889d0ed7bd
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+void arch_crash_save_vmcoreinfo(void)
+{
+    VMCOREINFO_NUMBER(VA_BITS);
+    VMCOREINFO_NUMBER(phys_ram_base);
+
+
vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n",
PAGE_OFFSET);
+ vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n",
VMALLOC_START);
+
vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n",
VMALLOC_END);
+ vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n",
VMEMMAP_START);
+
vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n",
VMEMMAP_END);
+#ifdef CONFIG_64BIT
+ vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n",
MODULES_VADDR);
+
vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n",
MODULES_END);
+#endif
+
+    if (IS_ENABLED(CONFIG_64BIT))
+
vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n",
KERNEL_LINK_ADDR);

Wondering why you don't put KERNEL_LINK_ADDR exporting into the above
ifdeffery scope, with that you can save one line of
"IS_ENABLED(CONFIG_64BIT)".

I followed the rule in print_vm_layout() of
arch/riscv/mm/init.c, which used
IS_ENABLED when print the value of KERNEL_LINK_ADDR.


I see. There's PAGE_OFFSET in the middle. Thanks.

  print_ml("lowmem", (unsigned long)PAGE_OFFSET,
  (unsigned long)high_memory)

So now, do you think if it's necessary to have another
IS_ENABLED(CONFIG_64BIT) in the current arch_crash_save_vmcoreinfo()?

For which MACRO? 

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Baoquan He
Hi Xianting, 

On 10/26/22 at 05:44pm, Xianting Tian wrote:
> 
> 在 2022/10/26 下午5:25, Conor Dooley 写道:
> > On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:
> > > Hi Palmer, Conor
> > > 
> > > Is this version OK for you?
> > The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
> > odd & I notice Baoquan brought it up too. I didn't (and won't) give you
> > a reviewed by on these patches because I don't understand the area well
> > enough. The general nitpickery seems to be sorted though.
> 
> I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
> CONFIG_64BIT and !CONFIG_64BIT.

This series looks good to me. My only minor concern is if we can make
the arch_crash_save_vmcoreinfo() as below. I don't understand why we
have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
between two adjacent code blocks. Not sure if we are saying the same
thing.

+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);
+#endif
+}

> 
> Maybe we can remove IS_ENABLED(CONFIG_64BIT)
> 
> arch/riscv/include/asm/pgtable.h
> #define ADDRESS_SPACE_END   (UL(-1))
> #ifdef CONFIG_64BIT
> /* Leave 2GB for kernel and BPF at the end of the address space */
> #define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
> #else
> #define KERNEL_LINK_ADDR    PAGE_OFFSET
> #endif
> 
> arch/riscv/include/asm/page.h
> #ifdef CONFIG_64BIT
> #ifdef CONFIG_MMU
> #define PAGE_OFFSET kernel_map.page_offset
> #else
> #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> #endif
> /*
>  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space so
>  * define the PAGE_OFFSET value for SV39.
>  */
> #define PAGE_OFFSET_L4  _AC(0xaf80, UL)
> #define PAGE_OFFSET_L3  _AC(0xffd8, UL)
> #else
> #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> #endif /* CONFIG_64BIT */
> 
> > 
> > Thanks,
> > Conor.
> > 
> > > 在 2022/10/20 下午12:40, Xianting Tian 写道:
> > > > 在 2022/10/20 上午11:05, Baoquan He 写道:
> > > > > On 10/20/22 at 10:17am, Xianting Tian wrote:
> > > > > > 在 2022/10/20 上午10:08, Baoquan He 写道:
> > > > > > > On 10/19/22 at 06:36pm, Xianting Tian wrote:
> > > > > > > > Add arch_crash_save_vmcoreinfo(), which exports VM
> > > > > > > > layout(MODULES, VMALLOC,
> > > > > > > > VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
> > > > > > > > base for vmcore.
> > > > > > > > 
> > > > > > > > Default pagetable levels and PAGE_OFFSET aren't same for
> > > > > > > > different kernel
> > > > > > > > version as below. For pagetable levels, it sets sv57 by
> > > > > > > > default and falls
> > > > > > > > back to setting sv48 at boot time if sv57 is not
> > > > > > > > supported by the hardware.
> > > > > > > > 
> > > > > > > > For ram base, the default value is 0x8020 for qemu
> > > > > > > > riscv64 env and,
> > > > > > > > for example, is 0x20 on the XuanTie 910 CPU.
> > > > > > > > 
> > > > > > > >     * Linux Kernel 5.18 ~
> > > > > > > >     *  PGTABLE_LEVELS = 5
> > > > > > > >     *  PAGE_OFFSET = 0xff60
> > > > > > > >     * Linux Kernel 5.17 ~
> > > > > > > >     *  PGTABLE_LEVELS = 4
> > > > > > > >     *  PAGE_OFFSET = 0xaf80
> > > > > > > >     * Linux Kernel 4.19 ~
> > > > > > > >     *  PGTABLE_LEVELS = 3
> > > > > > > >     *  PAGE_OFFSET = 0xffe0
> > > > > > > > 
> > > > > > > > Since these configurations change from time to time and
> > > > > > > > version to version,
> > > > > > > > it is preferable to export them via vmcoreinfo than to
> > > > > > > > change the crash's
> > > > > > > > code frequently, it can simplify the development of crash tool.
> > > > > > > > 
> > > > > > > > Signed-off-by: Xianting Tian 
> > > > > > > > ---
> > > > > > > >     arch/riscv/kernel/Makefile |  1 +
> > > > > > > >     arch/riscv/kernel/crash_core.c | 23 +++
> > > > > > > >     2 files changed, 24 insertions(+)
> > > > > > > >     create mode 100644 arch/riscv/kernel/crash_core.c
> > > > > > > > 
> > > > > > > > diff --git a/arch/riscv/kernel/Makefile 
> > > > > > > > b/arch/riscv/kernel/Makefile
> > > > > > > > index db6e4b1294ba..4cf303a779ab 100644
> > > > > > > > --- a/arch/riscv/kernel/Makefile
> > > > > > > 

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Conor Dooley
On Wed, Oct 26, 2022 at 08:05:41PM +0800, Baoquan He wrote:
> Hi Xianting, 
> 
> On 10/26/22 at 05:44pm, Xianting Tian wrote:
> > 
> > 在 2022/10/26 下午5:25, Conor Dooley 写道:
> > > On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:
> > > > Hi Palmer, Conor
> > > > 
> > > > Is this version OK for you?
> > > The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
> > > odd & I notice Baoquan brought it up too. I didn't (and won't) give you
> > > a reviewed by on these patches because I don't understand the area well
> > > enough. The general nitpickery seems to be sorted though.
> > 
> > I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
> > CONFIG_64BIT and !CONFIG_64BIT.
> 
> This series looks good to me. My only minor concern is if we can make
> the arch_crash_save_vmcoreinfo() as below. I don't understand why we
> have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
> between two adjacent code blocks. Not sure if we are saying the same
> thing.

I think we can just go and drop the IS_ENABLED(). From looking at it
last time, one bit is compileable (but not usable) for !64BIT and the
other isn't hence the IS_ENABLED(). I think it would make sense to drop
the IS_ENABLED() - I don't think we're too likely to hit some compile
testing edge cases that IS_ENABLED() would help with & only having one
makes the code look a lot less odd and a lot more intentional.

> 
> +void arch_crash_save_vmcoreinfo(void)
> +{
> +   VMCOREINFO_NUMBER(VA_BITS);
> +   VMCOREINFO_NUMBER(phys_ram_base);
> +
> +   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
> +   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
> +   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
> +   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
> +   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
> +#ifdef CONFIG_64BIT
> + vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
> +   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
> +   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
> KERNEL_LINK_ADDR);
> +#endif
> +}
> 
> > 
> > Maybe we can remove IS_ENABLED(CONFIG_64BIT)
> > 
> > arch/riscv/include/asm/pgtable.h
> > #define ADDRESS_SPACE_END   (UL(-1))
> > #ifdef CONFIG_64BIT
> > /* Leave 2GB for kernel and BPF at the end of the address space */
> > #define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
> > #else
> > #define KERNEL_LINK_ADDR    PAGE_OFFSET
> > #endif
> > 
> > arch/riscv/include/asm/page.h
> > #ifdef CONFIG_64BIT
> > #ifdef CONFIG_MMU
> > #define PAGE_OFFSET kernel_map.page_offset
> > #else
> > #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> > #endif
> > /*
> >  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space so
> >  * define the PAGE_OFFSET value for SV39.
> >  */
> > #define PAGE_OFFSET_L4  _AC(0xaf80, UL)
> > #define PAGE_OFFSET_L3  _AC(0xffd8, UL)
> > #else
> > #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> > #endif /* CONFIG_64BIT */
> > 
> > > 
> > > Thanks,
> > > Conor.
> > > 
> > > > 在 2022/10/20 下午12:40, Xianting Tian 写道:
> > > > > 在 2022/10/20 上午11:05, Baoquan He 写道:
> > > > > > On 10/20/22 at 10:17am, Xianting Tian wrote:
> > > > > > > 在 2022/10/20 上午10:08, Baoquan He 写道:
> > > > > > > > On 10/19/22 at 06:36pm, Xianting Tian wrote:
> > > > > > > > > Add arch_crash_save_vmcoreinfo(), which exports VM
> > > > > > > > > layout(MODULES, VMALLOC,
> > > > > > > > > VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
> > > > > > > > > base for vmcore.
> > > > > > > > > 
> > > > > > > > > Default pagetable levels and PAGE_OFFSET aren't same for
> > > > > > > > > different kernel
> > > > > > > > > version as below. For pagetable levels, it sets sv57 by
> > > > > > > > > default and falls
> > > > > > > > > back to setting sv48 at boot time if sv57 is not
> > > > > > > > > supported by the hardware.
> > > > > > > > > 
> > > > > > > > > For ram base, the default value is 0x8020 for qemu
> > > > > > > > > riscv64 env and,
> > > > > > > > > for example, is 0x20 on the XuanTie 910 CPU.
> > > > > > > > > 
> > > > > > > > >     * Linux Kernel 5.18 ~
> > > > > > > > >     *  PGTABLE_LEVELS = 5
> > > > > > > > >     *  PAGE_OFFSET = 0xff60
> > > > > > > > >     * Linux Kernel 5.17 ~
> > > > > > > > >     *  PGTABLE_LEVELS = 4
> > > > > > > > >     *  PAGE_OFFSET = 0xaf80
> > > > > > > > >     * Linux Kernel 4.19 ~
> > > > > > > > >     *  PGTABLE_LEVELS = 3
> > > > > > > > >     *  PAGE_OFFSET = 0xffe0
> > > > > > > > > 
> > > > > > > > > Since these configurations change from time to time and
> > > > > > > > > version to version,
> > > > > > > > > it is preferable to export them via vmcoreinfo than to
> > > >

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Xianting Tian


在 2022/10/26 下午8:05, Baoquan He 写道:

Hi Xianting,

On 10/26/22 at 05:44pm, Xianting Tian wrote:

在 2022/10/26 下午5:25, Conor Dooley 写道:

On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:

Hi Palmer, Conor

Is this version OK for you?

The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
odd & I notice Baoquan brought it up too. I didn't (and won't) give you
a reviewed by on these patches because I don't understand the area well
enough. The general nitpickery seems to be sorted though.

I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
CONFIG_64BIT and !CONFIG_64BIT.

This series looks good to me. My only minor concern is if we can make
the arch_crash_save_vmcoreinfo() as below. I don't understand why we
have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
between two adjacent code blocks. Not sure if we are saying the same
thing.


KERNEL_LINK_ADDR is valid for both CONFIG_64BIT and !CONFIG_64BIT, but  
MODULES_VADDR is only defined when CONFIG_64BIT.


I tend to agree only remove IS_ENABLED, thanks

For riscv:

#ifdef CONFIG_64BIT

/* Leave 2GB for kernel and BPF at the end of the address space */
#define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
#else
#define KERNEL_LINK_ADDR    PAGE_OFFSET
#endif


+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);
+#endif
+}


Maybe we can remove IS_ENABLED(CONFIG_64BIT)

arch/riscv/include/asm/pgtable.h
#define ADDRESS_SPACE_END   (UL(-1))
#ifdef CONFIG_64BIT
/* Leave 2GB for kernel and BPF at the end of the address space */
#define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
#else
#define KERNEL_LINK_ADDR    PAGE_OFFSET
#endif

arch/riscv/include/asm/page.h
#ifdef CONFIG_64BIT
#ifdef CONFIG_MMU
#define PAGE_OFFSET kernel_map.page_offset
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif
/*
  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space so
  * define the PAGE_OFFSET value for SV39.
  */
#define PAGE_OFFSET_L4  _AC(0xaf80, UL)
#define PAGE_OFFSET_L3  _AC(0xffd8, UL)
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif /* CONFIG_64BIT */


Thanks,
Conor.


在 2022/10/20 下午12:40, Xianting Tian 写道:

在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM
layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for
different kernel
version as below. For pagetable levels, it sets sv57 by
default and falls
back to setting sv48 at boot time if sv57 is not
supported by the hardware.

For ram base, the default value is 0x8020 for qemu
riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

     * Linux Kernel 5.18 ~
     *  PGTABLE_LEVELS = 5
     *  PAGE_OFFSET = 0xff60
     * Linux Kernel 5.17 ~
     *  PGTABLE_LEVELS = 4
     *  PAGE_OFFSET = 0xaf80
     * Linux Kernel 4.19 ~
     *  PGTABLE_LEVELS = 3
     *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and
version to version,
it is preferable to export them via vmcoreinfo than to
change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
     arch/riscv/kernel/Makefile |  1 +
     arch/riscv/kernel/crash_core.c | 23 +++
     2 files changed, 24 insertions(+)
     create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)    += kgdb.o
     obj-$(CONFIG_KEXEC_CORE)    += kexec_relocate.o
crash_save_regs.o machine_kexec.o
     obj-$(CONFIG_KEXEC_FILE)    += elf_kexec.o machine_kexec_file.o
     obj-$(CONFIG_CRASH_DUMP)    += crash_dump.o
+obj-$(CONFIG_CRASH_CORE)    += crash_core.o
     obj-$(CONFIG_JUMP_LABEL)    += jump_label.o
diff --git a/arch/riscv/kernel

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-26 Thread Xianting Tian


在 2022/10/26 下午9:47, Conor Dooley 写道:

On Wed, Oct 26, 2022 at 08:05:41PM +0800, Baoquan He wrote:

Hi Xianting,

On 10/26/22 at 05:44pm, Xianting Tian wrote:

在 2022/10/26 下午5:25, Conor Dooley 写道:

On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:

Hi Palmer, Conor

Is this version OK for you?

The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
odd & I notice Baoquan brought it up too. I didn't (and won't) give you
a reviewed by on these patches because I don't understand the area well
enough. The general nitpickery seems to be sorted though.

I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
CONFIG_64BIT and !CONFIG_64BIT.

This series looks good to me. My only minor concern is if we can make
the arch_crash_save_vmcoreinfo() as below. I don't understand why we
have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
between two adjacent code blocks. Not sure if we are saying the same
thing.

I think we can just go and drop the IS_ENABLED(). From looking at it
last time, one bit is compileable (but not usable) for !64BIT and the
other isn't hence the IS_ENABLED(). I think it would make sense to drop
the IS_ENABLED() - I don't think we're too likely to hit some compile
testing edge cases that IS_ENABLED() would help with & only having one
makes the code look a lot less odd and a lot more intentional.

thanks, I will send V5 patch for review.



+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);
+#endif
+}


Maybe we can remove IS_ENABLED(CONFIG_64BIT)

arch/riscv/include/asm/pgtable.h
#define ADDRESS_SPACE_END   (UL(-1))
#ifdef CONFIG_64BIT
/* Leave 2GB for kernel and BPF at the end of the address space */
#define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
#else
#define KERNEL_LINK_ADDR    PAGE_OFFSET
#endif

arch/riscv/include/asm/page.h
#ifdef CONFIG_64BIT
#ifdef CONFIG_MMU
#define PAGE_OFFSET kernel_map.page_offset
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif
/*
  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space so
  * define the PAGE_OFFSET value for SV39.
  */
#define PAGE_OFFSET_L4  _AC(0xaf80, UL)
#define PAGE_OFFSET_L3  _AC(0xffd8, UL)
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif /* CONFIG_64BIT */


Thanks,
Conor.


在 2022/10/20 下午12:40, Xianting Tian 写道:

在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM
layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for
different kernel
version as below. For pagetable levels, it sets sv57 by
default and falls
back to setting sv48 at boot time if sv57 is not
supported by the hardware.

For ram base, the default value is 0x8020 for qemu
riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

     * Linux Kernel 5.18 ~
     *  PGTABLE_LEVELS = 5
     *  PAGE_OFFSET = 0xff60
     * Linux Kernel 5.17 ~
     *  PGTABLE_LEVELS = 4
     *  PAGE_OFFSET = 0xaf80
     * Linux Kernel 4.19 ~
     *  PGTABLE_LEVELS = 3
     *  PAGE_OFFSET = 0xffe0

Since these configurations change from time to time and
version to version,
it is preferable to export them via vmcoreinfo than to
change the crash's
code frequently, it can simplify the development of crash tool.

Signed-off-by: Xianting Tian 
---
     arch/riscv/kernel/Makefile |  1 +
     arch/riscv/kernel/crash_core.c | 23 +++
     2 files changed, 24 insertions(+)
     create mode 100644 arch/riscv/kernel/crash_core.c

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index db6e4b1294ba..4cf303a779ab 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_KGDB)    += kgdb.o
     obj-$(CONFIG_KEXEC_CORE)    += kexec_relocate.o
crash_save_regs.o machine_kexec.o
     obj-$(CONFIG_KEXEC_FILE)    += elf_kexec.o machine_kexec_file.o
     obj-$(CONFIG_CRASH_DUMP)    += crash_dump

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-31 Thread Baoquan He
On 10/26/22 at 02:47pm, Conor Dooley wrote:
> On Wed, Oct 26, 2022 at 08:05:41PM +0800, Baoquan He wrote:
> > Hi Xianting, 
> > 
> > On 10/26/22 at 05:44pm, Xianting Tian wrote:
> > > 
> > > 在 2022/10/26 下午5:25, Conor Dooley 写道:
> > > > On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:
> > > > > Hi Palmer, Conor
> > > > > 
> > > > > Is this version OK for you?
> > > > The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
> > > > odd & I notice Baoquan brought it up too. I didn't (and won't) give you
> > > > a reviewed by on these patches because I don't understand the area well
> > > > enough. The general nitpickery seems to be sorted though.
> > > 
> > > I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
> > > CONFIG_64BIT and !CONFIG_64BIT.
> > 
> > This series looks good to me. My only minor concern is if we can make
> > the arch_crash_save_vmcoreinfo() as below. I don't understand why we
> > have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
> > between two adjacent code blocks. Not sure if we are saying the same
> > thing.
> 
> I think we can just go and drop the IS_ENABLED(). From looking at it
> last time, one bit is compileable (but not usable) for !64BIT and the
> other isn't hence the IS_ENABLED(). I think it would make sense to drop
> the IS_ENABLED() - I don't think we're too likely to hit some compile
> testing edge cases that IS_ENABLED() would help with & only having one
> makes the code look a lot less odd and a lot more intentional.

I check risc-v code again, and agree we can drop the IS_ENABLED checking
to export KERNEL_LINK_ADDR anyway. We can surely deduce
KERNEL_LINK_ADDR in userspace e.g makedumpfile/Crash, while it seems no
harm to get it from the vmcoreinfo directly.

As for the difference between "#ifdef CONFIG_64BIT" and
"if (IS_ENABLED(CONFIG_64BIT))", I haven't got what's the Xianting's
point. Below is the IS_ENABLED definition in include/linux/kconfig.h,
it's truly different than #ifdef, while the change we are discussing
here is not related.

/*
 * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
 * 0 otherwise.  Note that CONFIG_FOO=y results in "#define CONFIG_FOO 1" in
 * autoconf.h, while CONFIG_FOO=m results in "#define CONFIG_FOO_MODULE 1".
 */
#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))

> 
> > 
> > +void arch_crash_save_vmcoreinfo(void)
> > +{
> > +   VMCOREINFO_NUMBER(VA_BITS);
> > +   VMCOREINFO_NUMBER(phys_ram_base);
> > +
> > +   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
> > +   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", 
> > VMALLOC_START);
> > +   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
> > +   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", 
> > VMEMMAP_START);
> > +   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
> > +#ifdef CONFIG_64BIT
> > +   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
> > +   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
> > +   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
> > KERNEL_LINK_ADDR);
> > +#endif
> > +}
> > 
> > > 
> > > Maybe we can remove IS_ENABLED(CONFIG_64BIT)
> > > 
> > > arch/riscv/include/asm/pgtable.h
> > > #define ADDRESS_SPACE_END   (UL(-1))
> > > #ifdef CONFIG_64BIT
> > > /* Leave 2GB for kernel and BPF at the end of the address space */
> > > #define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
> > > #else
> > > #define KERNEL_LINK_ADDR    PAGE_OFFSET
> > > #endif
> > > 
> > > arch/riscv/include/asm/page.h
> > > #ifdef CONFIG_64BIT
> > > #ifdef CONFIG_MMU
> > > #define PAGE_OFFSET kernel_map.page_offset
> > > #else
> > > #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> > > #endif
> > > /*
> > >  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space 
> > > so
> > >  * define the PAGE_OFFSET value for SV39.
> > >  */
> > > #define PAGE_OFFSET_L4  _AC(0xaf80, UL)
> > > #define PAGE_OFFSET_L3  _AC(0xffd8, UL)
> > > #else
> > > #define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
> > > #endif /* CONFIG_64BIT */
> > > 
> > > > 
> > > > Thanks,
> > > > Conor.
> > > > 
> > > > > 在 2022/10/20 下午12:40, Xianting Tian 写道:
> > > > > > 在 2022/10/20 上午11:05, Baoquan He 写道:
> > > > > > > On 10/20/22 at 10:17am, Xianting Tian wrote:
> > > > > > > > 在 2022/10/20 上午10:08, Baoquan He 写道:
> > > > > > > > > On 10/19/22 at 06:36pm, Xianting Tian wrote:
> > > > > > > > > > Add arch_crash_save_vmcoreinfo(), which exports VM
> > > > > > > > > > layout(MODULES, VMALLOC,
> > > > > > > > > > VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
> > > > > > > > > > base for vmcore.
> > > > > > > > > > 
> > > > > > > > > > Default pagetable levels and PAGE_OFFSET aren't same for
> > > > > > > > > > different kernel
> > > > > > > > 

Re: [PATCH V4 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support

2022-10-31 Thread Xianting Tian


在 2022/10/31 下午4:57, Baoquan He 写道:

On 10/26/22 at 02:47pm, Conor Dooley wrote:

On Wed, Oct 26, 2022 at 08:05:41PM +0800, Baoquan He wrote:

Hi Xianting,

On 10/26/22 at 05:44pm, Xianting Tian wrote:

在 2022/10/26 下午5:25, Conor Dooley 写道:

On Wed, Oct 26, 2022 at 05:08:11PM +0800, Xianting Tian wrote:

Hi Palmer, Conor

Is this version OK for you?

The weird ifdef/IS_ENABLED thing was the only comment I had. It's a bit
odd & I notice Baoquan brought it up too. I didn't (and won't) give you
a reviewed by on these patches because I don't understand the area well
enough. The general nitpickery seems to be sorted though.

I checked the KERNEL_LINK_ADDR definition of riscv,  it is valid for
CONFIG_64BIT and !CONFIG_64BIT.

This series looks good to me. My only minor concern is if we can make
the arch_crash_save_vmcoreinfo() as below. I don't understand why we
have to have the CONFIG_64BIT ifdeffery and the IS_ENABLED(CONFIG_64BIT)
between two adjacent code blocks. Not sure if we are saying the same
thing.

I think we can just go and drop the IS_ENABLED(). From looking at it
last time, one bit is compileable (but not usable) for !64BIT and the
other isn't hence the IS_ENABLED(). I think it would make sense to drop
the IS_ENABLED() - I don't think we're too likely to hit some compile
testing edge cases that IS_ENABLED() would help with & only having one
makes the code look a lot less odd and a lot more intentional.

I check risc-v code again, and agree we can drop the IS_ENABLED checking
to export KERNEL_LINK_ADDR anyway. We can surely deduce
KERNEL_LINK_ADDR in userspace e.g makedumpfile/Crash, while it seems no
harm to get it from the vmcoreinfo directly.


thanks Baoquan for the confirm,

please help give the Revied-by and promote the patch set to be applied 
to Linux 6.1 :)




As for the difference between "#ifdef CONFIG_64BIT" and
"if (IS_ENABLED(CONFIG_64BIT))", I haven't got what's the Xianting's
point. Below is the IS_ENABLED definition in include/linux/kconfig.h,
it's truly different than #ifdef, while the change we are discussing
here is not related.

/*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
  * 0 otherwise.  Note that CONFIG_FOO=y results in "#define CONFIG_FOO 1" in
  * autoconf.h, while CONFIG_FOO=m results in "#define CONFIG_FOO_MODULE 1".
  */
#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))


+void arch_crash_save_vmcoreinfo(void)
+{
+   VMCOREINFO_NUMBER(VA_BITS);
+   VMCOREINFO_NUMBER(phys_ram_base);
+
+   vmcoreinfo_append_str("NUMBER(PAGE_OFFSET)=0x%lx\n", PAGE_OFFSET);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_START)=0x%lx\n", VMALLOC_START);
+   vmcoreinfo_append_str("NUMBER(VMALLOC_END)=0x%lx\n", VMALLOC_END);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_START)=0x%lx\n", VMEMMAP_START);
+   vmcoreinfo_append_str("NUMBER(VMEMMAP_END)=0x%lx\n", VMEMMAP_END);
+#ifdef CONFIG_64BIT
+   vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR);
+   vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END);
+   vmcoreinfo_append_str("NUMBER(KERNEL_LINK_ADDR)=0x%lx\n", 
KERNEL_LINK_ADDR);
+#endif
+}


Maybe we can remove IS_ENABLED(CONFIG_64BIT)

arch/riscv/include/asm/pgtable.h
#define ADDRESS_SPACE_END   (UL(-1))
#ifdef CONFIG_64BIT
/* Leave 2GB for kernel and BPF at the end of the address space */
#define KERNEL_LINK_ADDR    (ADDRESS_SPACE_END - SZ_2G + 1)
#else
#define KERNEL_LINK_ADDR    PAGE_OFFSET
#endif

arch/riscv/include/asm/page.h
#ifdef CONFIG_64BIT
#ifdef CONFIG_MMU
#define PAGE_OFFSET kernel_map.page_offset
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif
/*
  * By default, CONFIG_PAGE_OFFSET value corresponds to SV48 address space so
  * define the PAGE_OFFSET value for SV39.
  */
#define PAGE_OFFSET_L4  _AC(0xaf80, UL)
#define PAGE_OFFSET_L3  _AC(0xffd8, UL)
#else
#define PAGE_OFFSET _AC(CONFIG_PAGE_OFFSET, UL)
#endif /* CONFIG_64BIT */


Thanks,
Conor.


在 2022/10/20 下午12:40, Xianting Tian 写道:

在 2022/10/20 上午11:05, Baoquan He 写道:

On 10/20/22 at 10:17am, Xianting Tian wrote:

在 2022/10/20 上午10:08, Baoquan He 写道:

On 10/19/22 at 06:36pm, Xianting Tian wrote:

Add arch_crash_save_vmcoreinfo(), which exports VM
layout(MODULES, VMALLOC,
VMEMMAP ranges and KERNEL_LINK_ADDR), va bits and ram
base for vmcore.

Default pagetable levels and PAGE_OFFSET aren't same for
different kernel
version as below. For pagetable levels, it sets sv57 by
default and falls
back to setting sv48 at boot time if sv57 is not
supported by the hardware.

For ram base, the default value is 0x8020 for qemu
riscv64 env and,
for example, is 0x20 on the XuanTie 910 CPU.

     * Linux Kernel 5.18 ~
     *  PGTABLE_LEVELS = 5
     *  PAGE_OFFSET = 0xff60
     * Linux Kernel 5.17 ~
     *  PGTABLE_LEVELS = 4
     *  PAGE_OFFSET = 0xaf80
     * Linux Ke