Re: [PATCH] Makefile: Remove version from /usr/share/makedumpfile

2022-10-26 Thread 萩尾 一仁
On 2022/10/24 11:25, HAGIO KAZUHITO(萩尾 一仁) wrote:
> On 2022/10/21 19:24, Leonidas Spyropoulos wrote:
>> Version specific paths doesn't make sense at
>> /usr/share/makedumpfile. This assumes you will have only one version
>> installed which on a normal system it makes sense and devs can always
>> specify different DESTDIR per versions.
>>
>> Fixes: #10
>>
>> Signed-off-by: Leonidas Spyropoulos 
> 
> Thanks for the patch.
> 
> I agree.
> 
> The patch [1] introduced the directory with ${VERSION}, but makedumpfile
> has backward compatibility and the directory does not have any data that
> has version restraint, so I don't see any reason.  Also I didn't find any
> discussion in the list archive.
> 
> I will merge this a few days later if no objection.

Applied.
https://github.com/makedumpfile/makedumpfile/commit/f1d84a5d69d81bc7a89aefae504be88df1e50693

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


Re: [PATCH v3 0/2] arm64: kdump: Function supplement and performance optimization

2022-10-26 Thread john . p . donnelly

On 10/14/22 11:29 AM, Catalin Marinas wrote:

On Thu, Oct 13, 2022 at 06:46:35PM +0800, Baoquan He wrote:

On 10/06/22 at 09:55am, john.p.donne...@oracle.com wrote:

What is the progress of this series ?

Without this patch set we are seeing  larger crashkernel=896M failures on
Arm  with Linux-6.0.rc7.  This larger value is needed for
iSCSI booted systems with certain network adapters.


This change is located in arch/arm64 folder, I have pinged arm64
maintainer to consider merging this patchset. Not sure if they are
still thinking, or ignore this.

Hi Catalin, Will,

Ping again!

Do you have plan to accept this patchset? It's very important for
crashkernel setting on arm64 with a simple and default syntax.


I'll look at it once the merging window closes. I saw discussions on
this thread and I ignored it until you all agreed ;).




Hi,

Do you have a timeline for this ?  This crashkernel > 4G for Arm item 
has been lingering for 3 years. I think it is time for it to be 
incorporated.



Thanks,

John.





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


Re: [PATCH v12 7/7] x86/crash: Add x86 crash hotplug support

2022-10-26 Thread David Hildenbrand

On 26.10.22 16:48, Baoquan He wrote:

On 10/25/22 at 12:31pm, Borislav Petkov wrote:

On Thu, Oct 13, 2022 at 10:57:28AM +0800, Baoquan He wrote:

The concern to range number mainly is on Virt guest systems.


And why would virt emulate 1K hotpluggable DIMM slots and not emulate a
real machine?


IIRC, ACPI only allows for 256 slots. PPC dlpar might provide more.



Well, currently, mem hotpug is an important feature on virt system to
dynamically increase/shrink memory on the system. If only emulating real
machine, it won't be different than bare metal system.

IIRC, the ballon driver or virtio-mem feature can add memory board, e.g
1G, block size is 128M, 8 blocks added. When shrinking this 1G memory
later, it will take best effort way to hot remove memory. Means if any
memory block is occupied, it will be kept there. Finally we could only
remove every second blocks, 4 blocks altogether. Then the left
un-removed blocks will produce 4 separate memory regions. Like this, a
virt guest could have many memory regions in kernel after memory
being added/removed.

If I am wrong, Please correct me, David.


Yes, virtio-mem (but also PPC dlpar) can result in many individual 
memory blocks with holes in between after hotunplug. Hotplug OTOH, 
usually tries to "plug" these holes and reduce the total number of 
memory blocks. It might be rare that our range will be heavily 
fragmented after unplug, but it's certainly possible.


[...]



Yes, now assume we have a HPE SGI system and it has memory hotplug
capacity. The system itself has already got memory regions more than
1024. Then when we hot add extra memory board, we want to include the
newly added memory regions into elfcorehdr so that it will be dumped out
in kdump kernel.

That's why I earlier suggested 2048 for number of memory regions.


The more the better, unless "it hurts". Assuming a single memory block 
is 128 MiB, that would be 256 GiB.


Usually, on big systems, the memory block size is 2 GiB. So 4 TiB.

--
Thanks,

David / dhildenb


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


Re: [PATCH v12 7/7] x86/crash: Add x86 crash hotplug support

2022-10-26 Thread Baoquan He
On 10/25/22 at 12:31pm, Borislav Petkov wrote:
> On Thu, Oct 13, 2022 at 10:57:28AM +0800, Baoquan He wrote:
> > The concern to range number mainly is on Virt guest systems.
> 
> And why would virt emulate 1K hotpluggable DIMM slots and not emulate a
> real machine?

Well, currently, mem hotpug is an important feature on virt system to
dynamically increase/shrink memory on the system. If only emulating real
machine, it won't be different than bare metal system.

IIRC, the ballon driver or virtio-mem feature can add memory board, e.g
1G, block size is 128M, 8 blocks added. When shrinking this 1G memory
later, it will take best effort way to hot remove memory. Means if any
memory block is occupied, it will be kept there. Finally we could only
remove every second blocks, 4 blocks altogether. Then the left
un-removed blocks will produce 4 separate memory regions. Like this, a
virt guest could have many memory regions in kernel after memory
being added/removed.

If I am wrong, Please correct me, David.

> 
> > On baremetal system, basically only very high end server support
> > memory hotplug. I ever visited customer's lab and saw one server,
> > it owns 8 slots, on each slot a box containing about 20 cpus and 2T
> > memory at most can be plugged in at one time. So people won't make too
> > many slots for hotplugging since it's too expensive.
> 
> There you have it - the persuading argument.
> 
> > I checked user space kexec code, the maximum memory range number is
> > honored to x86_64 because of a HPE SGI system. After that, nobody
> > complains about it. Please see below user space kexec-tools commit in
> > https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git
> > 
> > The memory ranges may be not all made by different DIMM slots, could be
> > firmware reservatoin, e.g efi/BIOS diggged out physical memory,
>   ^
> 
> I don't know what that means.
> 
> If it is firmware crap, you want to exclude that from kdump anyway.

Yes, now assume we have a HPE SGI system and it has memory hotplug
capacity. The system itself has already got memory regions more than
1024. Then when we hot add extra memory board, we want to include the
newly added memory regions into elfcorehdr so that it will be dumped out
in kdump kernel.

That's why I earlier suggested 2048 for number of memory regions.

> 
> > Now CONFIG_NR_CPUS has the maximum number as 8192. And user space
> > kexec-tools has maximum memory range number as 2048. We can take
> > the current 8192 + 2048  = 10K as default value conservatively. Or
> > take 8192 + 2048 * 2 = 12K which has two times of maximum memory range
> > bumber in kexec-tools. What do you think?
> 
> I still think that we should stick to reality and support what is
> possible not what is potentially and theoretically there.

Yes, agree. We should try to get a number which satisfies needs in
reality.

For Kconfig CRASH_MAX_MEMORY_RANGES in this patch, I have three items to
suggest:

1) the name is not good, it doesn't reflect the fact that it's the
number of program headers of elfcorehdr which includes the cpu note 
numbers and memory region numers.

2) default cpu number, I suggest 512 or 1024. The biggest number I
ever saw in reality is 384. On virt system, it won't be too big. Below
is abstracted from arch/x86/Kconfig. A smaller one is also OK, we can
enlarge it when people really have a super machine and run into the
problem.

   config NR_CPUS_DEFAULT
   int
   depends on X86_64
   default 8192 if  MAXSMP
   default   64 if  SMP
   default1 if !SMP

3) For memory regions, I would suggest 2048. Likewise, smaller value is
also fine, we can enlarge it when a real system run into this.

I made a draft here for reference, with my undertanding. Please feel
free to change it.

+config CRASH_ELF_CORE_PHDRS_NUM
+   depends on CRASH_DUMP && KEXEC_FILE && (HOTPLUG_CPU || MEMORY_HOTPLUG)
+   int
+   default 3072
+   help
+ For the kexec_file_load path, specify the default number of
+ phdr for the vmcore. E.g the memory regions represented by the
+ 'System RAM' entries in /proc/iomem, the cpu notes of each
+ present cpu stored in /sys/devices/system/cpu/cpuX/crash_notes.

Thanks


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


[PATCH V5 2/2] Documentation: kdump: describe VMCOREINFO export for RISCV64

2022-10-26 Thread Xianting Tian
The following interrelated definitions and ranges are needed by the kdump
crash tool, which are exported by "arch/riscv/kernel/crash_core.c":
VA_BITS,
PAGE_OFFSET,
phys_ram_base,
KERNEL_LINK_ADDR,
MODULES_VADDR ~ MODULES_END,
VMALLOC_START ~ VMALLOC_END,
VMEMMAP_START ~ VMEMMAP_END,

Document these RISCV64 exports above.

Reviewed-by: Bagas Sanjaya 
Signed-off-by: Xianting Tian 
---
 .../admin-guide/kdump/vmcoreinfo.rst  | 29 +++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst 
b/Documentation/admin-guide/kdump/vmcoreinfo.rst
index 6726f439958c..86fd88492870 100644
--- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
+++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
@@ -595,3 +595,32 @@ 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 the 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|KERNEL_LINK_ADDR
+--
+
+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 space, used for struct page array.
+  * KERNEL_LINK_ADDR : start address of Kernel link and BPF
-- 
2.17.1


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


[PATCH V5 0/2] Support VMCOREINFO export for RISCV64

2022-10-26 Thread Xianting Tian
As disscussed in below patch set, the patch of 'describe VMCOREINFO export in 
Documentation'
need to update according to Bagas's comments. 
https://lore.kernel.org/linux-riscv/22aaf52e-8cc8-4d11-99cb-88de4d113...@kernel.org/

As others patches in above patch set already applied, so this patch set only 
contains below two
patches.

--
Changes:
   Fix commit message in patch 2: use "Document these RISCV64 exports above" 
instead of
   "This patch just add the description of VMCOREINFO export for RISCV64."
V1 -> V2:
   Remove unnecessary overline above header text in patch 2.
V2 -> V3:
   Fix commit message in patch 1,2; 
   Use 'space' instead of 'region' for vmemmap description in patch 2.
V3 -> V4:
   Remove unnecessary kernel space export:
   KASAN_SHADOW_START ~ KASAN_SHADOW_END,
   ADDRESS_SPACE_END
V4 -> V5:
   Remove IS_ENABLED() judgement for KERNEL_LINK_ADDR in patch 1.

Xianting Tian (2):
  RISC-V: Add arch_crash_save_vmcoreinfo support
  Documentation: kdump: describe VMCOREINFO export for RISCV64

 .../admin-guide/kdump/vmcoreinfo.rst  | 29 +++
 arch/riscv/kernel/Makefile|  1 +
 arch/riscv/kernel/crash_core.c| 21 ++
 3 files changed, 51 insertions(+)
 create mode 100644 arch/riscv/kernel/crash_core.c

-- 
2.17.1


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


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

2022-10-26 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 | 21 +
 2 files changed, 22 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 ..b351a3c01355
--- /dev/null
+++ b/arch/riscv/kernel/crash_core.c
@@ -0,0 +1,21 @@
+// 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
+   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-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)    += 

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 

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 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 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 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 

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 v12 3/7] crash: add generic infrastructure for crash hotplug support

2022-10-26 Thread Sourabh Jain

Hello Baoquan,

On 24/10/22 14:40, Baoquan He wrote:

Hi Eric, Sourabh,

On 10/07/22 at 02:14pm, Eric DeVolder wrote:


On 10/3/22 12:51, Sourabh Jain wrote:

Hello Eric,

On 10/09/22 02:35, Eric DeVolder wrote:

..

+static void handle_hotplug_event(unsigned int hp_action, unsigned int cpu)
+{
+    /* Obtain lock while changing crash information */
+    mutex_lock(_mutex);
+
+    /* Check kdump is loaded */
+    if (kexec_crash_image) {
+    struct kimage *image = kexec_crash_image;
+
+    if (hp_action == KEXEC_CRASH_HP_ADD_CPU ||
+    hp_action == KEXEC_CRASH_HP_REMOVE_CPU)
+    pr_debug("crash hp: hp_action %u, cpu %u\n", hp_action, cpu);
+    else
+    pr_debug("crash hp: hp_action %u\n", hp_action);
+
+    /*
+ * When the struct kimage is allocated, it is wiped to zero, so
+ * the elfcorehdr_index_valid defaults to false. Find the
+ * segment containing the elfcorehdr, if not already found.
+ * This works for both the kexec_load and kexec_file_load paths.
+ */
+    if (!image->elfcorehdr_index_valid) {
+    unsigned char *ptr;
+    unsigned long mem, memsz;
+    unsigned int n;
+
+    for (n = 0; n < image->nr_segments; n++) {
+    mem = image->segment[n].mem;
+    memsz = image->segment[n].memsz;
+    ptr = arch_map_crash_pages(mem, memsz);
+    if (ptr) {
+    /* The segment containing elfcorehdr */
+    if (memcmp(ptr, ELFMAG, SELFMAG) == 0) {
+    image->elfcorehdr_index = (int)n;
+    image->elfcorehdr_index_valid = true;
+    }
+    }
+    arch_unmap_crash_pages((void **));
+    }
+    }
+
+    if (!image->elfcorehdr_index_valid) {
+    pr_err("crash hp: unable to locate elfcorehdr segment");
+    goto out;
+    }
+
+    /* Needed in order for the segments to be updated */
+    arch_kexec_unprotect_crashkres();
+
+    /* Flag to differentiate between normal load and hotplug */
+    image->hotplug_event = true;
+
+    /* Now invoke arch-specific update handler */
+    arch_crash_handle_hotplug_event(image, hp_action);
+
+    /* No longer handling a hotplug event */
+    image->hotplug_event = false;
+
+    /* Change back to read-only */
+    arch_kexec_protect_crashkres();
+    }
+
+out:
+    /* Release lock now that update complete */
+    mutex_unlock(_mutex);
+}
+
+static int crash_memhp_notifier(struct notifier_block *nb, unsigned long val, 
void *v)
+{
+    switch (val) {
+    case MEM_ONLINE:
+    handle_hotplug_event(KEXEC_CRASH_HP_ADD_MEMORY, 0);
+    break;
+
+    case MEM_OFFLINE:
+    handle_hotplug_event(KEXEC_CRASH_HP_REMOVE_MEMORY, 0);
+    break;
+    }
+    return NOTIFY_OK;

Can we pass v (memory_notify) argument to arch_crash_handle_hotplug_event 
function
via handle_hotplug_event?

Because the way memory hotplug is handled on PowerPC, it is hard to update the 
elfcorehdr
without memory_notify args.

On PowePC memblock data structure is used to prepare elfcorehdr for kdump. 
Since the notifier
used for memory hotplug crash handler get initiated before the memblock data 
structure update
happens (as depicted below), the newly prepared elfcorehdr still holds the old 
memory regions.
So if the system crash with obsolete elfcorehdr, makedumpfile failed to collect 
vmcore.

Sequence of actions done on PowerPC to server the memory hotplug:

   Initiate memory hot remove
    |
    v
   offline pages
    |
    v
   initiate memory notify call chain
   for MEM_OFFLINE event.
   (same is used for crash update)
    |
    v
   prepare new elfcorehdr for kdump using
   memblock data structure
    |
    v
   update memblock data structure

How passing memory_notify to arch crash hotplug handler will help?

memory_notify holds the start PFN and page count, with that we can get
the base address and size of hot unplugged memory and can use the same
to avoid hot unplugged memeory region to get added in the elfcorehdr..

Thanks,
Sourabh Jain


Sourabh, let's see what Baoquan thinks.

Baoquan, are you OK with this request? I once had these parameters to the
crash hotplug handler and since they were unused at the time, you asked
that I remove them, which I did.

Sorry to miss this mail. I thought both of you were talking about
somthing, and didn't notice this question to me.

I think there are two ways to solve the issue Sourabh raised:
1) make handle_hotplug_event() get and pass down the memory_notify as
Sourabh said, or the hp_action, mem_start|size as Eric suggested. I
have to admit I haven't carefully checked which one is better.

2) let the current code as is since it's aiming at x86 only. Later
Sourabh can modify code according to his need on ppc. This can give