[PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

Currently, x86, arm64, riscv and loongarch has been switched to generic
crashkernel reservation. Also use generic interface to simplify crashkernel
reservation for arm32, and fix two bugs by the way.

Jinjie Ruan (3):
  crash: Fix memory reserve dead loop bug in
    reserve_crashkernel_generic()
  ARM: Fix crash kenrel data type bug
  ARM: Use generic interface to simplify crashkernel reservation

 arch/arm/Kconfig                     |  3 ++
 arch/arm/include/asm/crash_reserve.h | 24 +++++++++++
 arch/arm/kernel/setup.c              | 63 +++++-----------------------
 kernel/crash_reserve.c               |  8 +++-
 4 files changed, 44 insertions(+), 54 deletions(-)
 create mode 100644 arch/arm/include/asm/crash_reserve.h

-- 
2.34.1


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

[PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

Currently, x86, arm64, riscv and loongarch has been switched to generic
crashkernel reservation. Also use generic interface to simplify crashkernel
reservation for arm32, and fix two bugs by the way.

Jinjie Ruan (3):
  crash: Fix memory reserve dead loop bug in
    reserve_crashkernel_generic()
  ARM: Fix crash kenrel data type bug
  ARM: Use generic interface to simplify crashkernel reservation

 arch/arm/Kconfig                     |  3 ++
 arch/arm/include/asm/crash_reserve.h | 24 +++++++++++
 arch/arm/kernel/setup.c              | 63 +++++-----------------------
 kernel/crash_reserve.c               |  8 +++-
 4 files changed, 44 insertions(+), 54 deletions(-)
 create mode 100644 arch/arm/include/asm/crash_reserve.h

-- 
2.34.1

[PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()

[Jinjie Ruan]

If the platform do not support memory above 4G, such as 32 bit arch,
and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
crash kernel memory reservation is meaningless and it will cause
dead loop and system stall:

-> reserve_crashkernel_generic() and high is true
 -> memblock_phys_alloc_range() fail and return 0
    -> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
       -> call memblock_phys_alloc_range() again and fail agin.
          -> search_end == CRASH_ADDR_HIGH_MAX satisfy again
	......

However, the current check only considers the case where
CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.

Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 kernel/crash_reserve.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
index 5b2722a93a48..e18fb1bb5d28 100644
--- a/kernel/crash_reserve.c
+++ b/kernel/crash_reserve.c
@@ -390,6 +390,11 @@ void __init reserve_crashkernel_generic(char *cmdline,
 	} else if (high) {
 		search_base = CRASH_ADDR_LOW_MAX;
 		search_end = CRASH_ADDR_HIGH_MAX;
+
+		if (search_base >= search_end) {
+			pr_warn("crashkernel high memory reservation failed.\n");
+			return;
+		}
 	}
 
 retry:
@@ -410,7 +415,8 @@ void __init reserve_crashkernel_generic(char *cmdline,
 		 * low memory, fall back to high memory, the minimum required
 		 * low memory will be reserved later.
 		 */
-		if (!high && search_end == CRASH_ADDR_LOW_MAX) {
+		if (!high && search_end == CRASH_ADDR_LOW_MAX &&
+		    CRASH_ADDR_HIGH_MAX > CRASH_ADDR_LOW_MAX) {
 			search_end = CRASH_ADDR_HIGH_MAX;
 			search_base = CRASH_ADDR_LOW_MAX;
 			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
-- 
2.34.1


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

[PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()

[Jinjie Ruan]

If the platform do not support memory above 4G, such as 32 bit arch,
and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
crash kernel memory reservation is meaningless and it will cause
dead loop and system stall:

-> reserve_crashkernel_generic() and high is true
 -> memblock_phys_alloc_range() fail and return 0
    -> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
       -> call memblock_phys_alloc_range() again and fail agin.
          -> search_end == CRASH_ADDR_HIGH_MAX satisfy again
	......

However, the current check only considers the case where
CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.

Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 kernel/crash_reserve.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/crash_reserve.c b/kernel/crash_reserve.c
index 5b2722a93a48..e18fb1bb5d28 100644
--- a/kernel/crash_reserve.c
+++ b/kernel/crash_reserve.c
@@ -390,6 +390,11 @@ void __init reserve_crashkernel_generic(char *cmdline,
 	} else if (high) {
 		search_base = CRASH_ADDR_LOW_MAX;
 		search_end = CRASH_ADDR_HIGH_MAX;
+
+		if (search_base >= search_end) {
+			pr_warn("crashkernel high memory reservation failed.\n");
+			return;
+		}
 	}
 
 retry:
@@ -410,7 +415,8 @@ void __init reserve_crashkernel_generic(char *cmdline,
 		 * low memory, fall back to high memory, the minimum required
 		 * low memory will be reserved later.
 		 */
-		if (!high && search_end == CRASH_ADDR_LOW_MAX) {
+		if (!high && search_end == CRASH_ADDR_LOW_MAX &&
+		    CRASH_ADDR_HIGH_MAX > CRASH_ADDR_LOW_MAX) {
 			search_end = CRASH_ADDR_HIGH_MAX;
 			search_base = CRASH_ADDR_LOW_MAX;
 			crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
-- 
2.34.1

Re: [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()

[Linus Walleij]

On Mon, Jul 8, 2024 at 3:30 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> If the platform do not support memory above 4G, such as 32 bit arch,
> and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
> crash kernel memory reservation is meaningless and it will cause
> dead loop and system stall:
>
> -> reserve_crashkernel_generic() and high is true
>  -> memblock_phys_alloc_range() fail and return 0
>     -> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
>        -> call memblock_phys_alloc_range() again and fail agin.
>           -> search_end == CRASH_ADDR_HIGH_MAX satisfy again
>         ......
>
> However, the current check only considers the case where
> CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.
>
> Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

Looks good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

Re: [PATCH 1/3] crash: Fix memory reserve dead loop bug in reserve_crashkernel_generic()

[Linus Walleij]

On Mon, Jul 8, 2024 at 3:30 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> If the platform do not support memory above 4G, such as 32 bit arch,
> and CRASH_ADDR_LOW_MAX is equal to CRASH_ADDR_HIGH_MAX, the high
> crash kernel memory reservation is meaningless and it will cause
> dead loop and system stall:
>
> -> reserve_crashkernel_generic() and high is true
>  -> memblock_phys_alloc_range() fail and return 0
>     -> search_end = CRASH_ADDR_LOW_MAX(same as CRASH_ADDR_HIGH_MAX)
>        -> call memblock_phys_alloc_range() again and fail agin.
>           -> search_end == CRASH_ADDR_HIGH_MAX satisfy again
>         ......
>
> However, the current check only considers the case where
> CRASH_ADDR_HIGH_MAX is greater than CRASH_ADDR_LOW_MAX. Fix it.
>
> Fixes: 0ab97169aa05 ("crash_core: add generic function to do reservation")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

Looks good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/3] ARM: Fix crash kenrel data type bug

[Jinjie Ruan]

On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
is ok as below:
	Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)

The above info is confusing, because the System memory is as below:
	# cat /proc/iomem | grep Sys
	60000000-9fffffff : System RAM

The cause is that the crash_size is parsed and printed with "unsigned long
long" data type which is 8 bytes but used with "phys_addr_t" which is
4 bytes in memblock_phys_alloc_range().

Fixes: 9d17f3372306 ("ARM: 9190/1: kdump: add invalid input check for 'crashkernel=0'")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/kernel/setup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index e6a857bf0ce6..59e1a13b5cf6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1012,6 +1012,7 @@ static void __init reserve_crashkernel(void)
 				&crash_size, &crash_base,
 				NULL, NULL);
 	/* invalid value specified or crashkernel=0 */
+	crash_size = (phys_addr_t)crash_size;
 	if (ret || !crash_size)
 		return;
 
-- 
2.34.1


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

[PATCH 2/3] ARM: Fix crash kenrel data type bug

[Jinjie Ruan]

On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
is ok as below:
	Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)

The above info is confusing, because the System memory is as below:
	# cat /proc/iomem | grep Sys
	60000000-9fffffff : System RAM

The cause is that the crash_size is parsed and printed with "unsigned long
long" data type which is 8 bytes but used with "phys_addr_t" which is
4 bytes in memblock_phys_alloc_range().

Fixes: 9d17f3372306 ("ARM: 9190/1: kdump: add invalid input check for 'crashkernel=0'")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/kernel/setup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index e6a857bf0ce6..59e1a13b5cf6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1012,6 +1012,7 @@ static void __init reserve_crashkernel(void)
 				&crash_size, &crash_base,
 				NULL, NULL);
 	/* invalid value specified or crashkernel=0 */
+	crash_size = (phys_addr_t)crash_size;
 	if (ret || !crash_size)
 		return;
 
-- 
2.34.1

Re: [PATCH 2/3] ARM: Fix crash kenrel data type bug

[Linus Walleij]

On Mon, Jul 8, 2024 at 3:29 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
> is ok as below:
>         Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)
>
> The above info is confusing, because the System memory is as below:
>         # cat /proc/iomem | grep Sys
>         60000000-9fffffff : System RAM
>
> The cause is that the crash_size is parsed and printed with "unsigned long
> long" data type which is 8 bytes but used with "phys_addr_t" which is
> 4 bytes in memblock_phys_alloc_range().

Is that the whole explanation?

>         /* invalid value specified or crashkernel=0 */
> +       crash_size = (phys_addr_t)crash_size;
>         if (ret || !crash_size)
>                 return;

How did crash_size get assigned before you added this oneliner?

Yours,
Linus Walleij

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

Re: [PATCH 2/3] ARM: Fix crash kenrel data type bug

[Linus Walleij]

On Mon, Jul 8, 2024 at 3:29 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> On QEMU vexpress-a9 with 1GB memory, the crash kernel "crashkernel=4G"
> is ok as below:
>         Reserving 4096MB of memory at 2432MB for crashkernel (System RAM: 1024MB)
>
> The above info is confusing, because the System memory is as below:
>         # cat /proc/iomem | grep Sys
>         60000000-9fffffff : System RAM
>
> The cause is that the crash_size is parsed and printed with "unsigned long
> long" data type which is 8 bytes but used with "phys_addr_t" which is
> 4 bytes in memblock_phys_alloc_range().

Is that the whole explanation?

>         /* invalid value specified or crashkernel=0 */
> +       crash_size = (phys_addr_t)crash_size;
>         if (ret || !crash_size)
>                 return;

How did crash_size get assigned before you added this oneliner?

Yours,
Linus Walleij

[PATCH 3/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

Currently, x86, arm64, riscv and loongarch has been switched to generic
crashkernel reservation. With the help of function parse_crashkernel()
and generic reserve_crashkernel_generic(), arm32 crashkernel reservation
can also be simplified by steps:

1) Add a new header file <asm/crash_reserve.h>, and define CRASH_ALIGN,
   CRASH_ADDR_LOW_MAX, CRASH_ADDR_HIGH_MAX in it;

2) Add arch_reserve_crashkernel() to call parse_crashkernel() and
   reserve_crashkernel_generic();

3) Add ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION Kconfig in
   arch/arm/Kconfig.

The old reserve_crashkernel() can be removed.

Following test cases have been performed as expected on QEMU vexpress-a9
(1GB system memory):

1) crashkernel=4G,high				// invalid
2) crashkernel=1G,high				// invalid
3) crashkernel=1G,high crashkernel=0M,low	// invalid
4) crashkernel=256M,high			// invalid
5) crashkernel=256M,low				// invalid
6) crashkernel=256M crashkernel=256M,high	// high is ignored, ok
7) crashkernel=256M crashkernel=256M,low	// low is ignored, ok
8) crashkernel=256M,high crashkernel=256M,low	// invalid
9) crashkernel=256M,high crashkernel=4G,low	// invalid
10) crashkernel=256M				// ok
11) crashkernel=512M				// ok
12) crashkernel=256M@0x88000000			// ok
13) crashkernel=256M@0x78000000			// ok
14) crashkernel=512M@0x78000000			// ok

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/Kconfig                     |  3 ++
 arch/arm/include/asm/crash_reserve.h | 24 +++++++++++
 arch/arm/kernel/setup.c              | 62 ++++------------------------
 3 files changed, 36 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm/include/asm/crash_reserve.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 68990e1645d5..02e620e185c0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1596,6 +1596,9 @@ config ATAGS_PROC
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
 
+config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
+	def_bool CRASH_RESERVE
+
 config AUTO_ZRELADDR
 	bool "Auto calculation of the decompressed kernel image address" if !ARCH_MULTIPLATFORM
 	default !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100)
diff --git a/arch/arm/include/asm/crash_reserve.h b/arch/arm/include/asm/crash_reserve.h
new file mode 100644
index 000000000000..85c9298bd3b7
--- /dev/null
+++ b/arch/arm/include/asm/crash_reserve.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ARM_CRASH_RESERVE_H
+#define _ARM_CRASH_RESERVE_H
+
+/*
+ * The crash region must be aligned to 128MB to avoid
+ * zImage relocating below the reserved region.
+ */
+#define CRASH_ALIGN			(128 << 20)
+
+#define CRASH_ADDR_LOW_MAX		crash_addr_low_max()
+#define CRASH_ADDR_HIGH_MAX		memblock_end_of_DRAM()
+
+static inline unsigned long crash_addr_low_max(void)
+{
+	unsigned long long crash_max = idmap_to_phys((u32)~0);
+	unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
+
+	return (crash_max > lowmem_max) ? lowmem_max : crash_max;
+}
+
+
+#define HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
+#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 59e1a13b5cf6..3a8b6f08f4ec 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -979,13 +979,6 @@ static int __init init_machine_late(void)
 }
 late_initcall(init_machine_late);
 
-#ifdef CONFIG_CRASH_RESERVE
-/*
- * The crash region must be aligned to 128MB to avoid
- * zImage relocating below the reserved region.
- */
-#define CRASH_ALIGN	(128 << 20)
-
 static inline unsigned long long get_total_mem(void)
 {
 	unsigned long total;
@@ -994,61 +987,27 @@ static inline unsigned long long get_total_mem(void)
 	return total << PAGE_SHIFT;
 }
 
-/**
- * reserve_crashkernel() - reserves memory are for crash kernel
- *
- * This function reserves memory area given in "crashkernel=" kernel command
- * line parameter. The memory reserved is used by a dump capture kernel when
- * primary kernel is crashing.
- */
-static void __init reserve_crashkernel(void)
+static void __init arch_reserve_crashkernel(void)
 {
 	unsigned long long crash_size, crash_base;
+	unsigned long long low_size = 0;
 	unsigned long long total_mem;
+	bool high = false;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_CRASH_RESERVE))
+		return;
+
 	total_mem = get_total_mem();
 	ret = parse_crashkernel(boot_command_line, total_mem,
 				&crash_size, &crash_base,
-				NULL, NULL);
+				&low_size, &high);
 	/* invalid value specified or crashkernel=0 */
 	crash_size = (phys_addr_t)crash_size;
 	if (ret || !crash_size)
 		return;
 
-	if (crash_base <= 0) {
-		unsigned long long crash_max = idmap_to_phys((u32)~0);
-		unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
-		if (crash_max > lowmem_max)
-			crash_max = lowmem_max;
-
-		crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
-						       CRASH_ALIGN, crash_max);
-		if (!crash_base) {
-			pr_err("crashkernel reservation failed - No suitable area found.\n");
-			return;
-		}
-	} else {
-		unsigned long long crash_max = crash_base + crash_size;
-		unsigned long long start;
-
-		start = memblock_phys_alloc_range(crash_size, SECTION_SIZE,
-						  crash_base, crash_max);
-		if (!start) {
-			pr_err("crashkernel reservation failed - memory is in use.\n");
-			return;
-		}
-	}
-
-	pr_info("Reserving %ldMB of memory at %ldMB for crashkernel (System RAM: %ldMB)\n",
-		(unsigned long)(crash_size >> 20),
-		(unsigned long)(crash_base >> 20),
-		(unsigned long)(total_mem >> 20));
-
-	/* The crashk resource must always be located in normal mem */
-	crashk_res.start = crash_base;
-	crashk_res.end = crash_base + crash_size - 1;
-	insert_resource(&iomem_resource, &crashk_res);
+	reserve_crashkernel_generic(boot_command_line, crash_size, crash_base, low_size, high);
 
 	if (arm_has_idmap_alias()) {
 		/*
@@ -1065,9 +1024,6 @@ static void __init reserve_crashkernel(void)
 		insert_resource(&iomem_resource, &crashk_boot_res);
 	}
 }
-#else
-static inline void reserve_crashkernel(void) {}
-#endif /* CONFIG_CRASH_RESERVE*/
 
 void __init hyp_mode_check(void)
 {
@@ -1190,7 +1146,7 @@ void __init setup_arch(char **cmdline_p)
 	if (!is_smp())
 		hyp_mode_check();
 
-	reserve_crashkernel();
+	arch_reserve_crashkernel();
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
-- 
2.34.1


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

[PATCH 3/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

Currently, x86, arm64, riscv and loongarch has been switched to generic
crashkernel reservation. With the help of function parse_crashkernel()
and generic reserve_crashkernel_generic(), arm32 crashkernel reservation
can also be simplified by steps:

1) Add a new header file <asm/crash_reserve.h>, and define CRASH_ALIGN,
   CRASH_ADDR_LOW_MAX, CRASH_ADDR_HIGH_MAX in it;

2) Add arch_reserve_crashkernel() to call parse_crashkernel() and
   reserve_crashkernel_generic();

3) Add ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION Kconfig in
   arch/arm/Kconfig.

The old reserve_crashkernel() can be removed.

Following test cases have been performed as expected on QEMU vexpress-a9
(1GB system memory):

1) crashkernel=4G,high				// invalid
2) crashkernel=1G,high				// invalid
3) crashkernel=1G,high crashkernel=0M,low	// invalid
4) crashkernel=256M,high			// invalid
5) crashkernel=256M,low				// invalid
6) crashkernel=256M crashkernel=256M,high	// high is ignored, ok
7) crashkernel=256M crashkernel=256M,low	// low is ignored, ok
8) crashkernel=256M,high crashkernel=256M,low	// invalid
9) crashkernel=256M,high crashkernel=4G,low	// invalid
10) crashkernel=256M				// ok
11) crashkernel=512M				// ok
12) crashkernel=256M@0x88000000			// ok
13) crashkernel=256M@0x78000000			// ok
14) crashkernel=512M@0x78000000			// ok

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/Kconfig                     |  3 ++
 arch/arm/include/asm/crash_reserve.h | 24 +++++++++++
 arch/arm/kernel/setup.c              | 62 ++++------------------------
 3 files changed, 36 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm/include/asm/crash_reserve.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 68990e1645d5..02e620e185c0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1596,6 +1596,9 @@ config ATAGS_PROC
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
 
+config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION
+	def_bool CRASH_RESERVE
+
 config AUTO_ZRELADDR
 	bool "Auto calculation of the decompressed kernel image address" if !ARCH_MULTIPLATFORM
 	default !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100)
diff --git a/arch/arm/include/asm/crash_reserve.h b/arch/arm/include/asm/crash_reserve.h
new file mode 100644
index 000000000000..85c9298bd3b7
--- /dev/null
+++ b/arch/arm/include/asm/crash_reserve.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ARM_CRASH_RESERVE_H
+#define _ARM_CRASH_RESERVE_H
+
+/*
+ * The crash region must be aligned to 128MB to avoid
+ * zImage relocating below the reserved region.
+ */
+#define CRASH_ALIGN			(128 << 20)
+
+#define CRASH_ADDR_LOW_MAX		crash_addr_low_max()
+#define CRASH_ADDR_HIGH_MAX		memblock_end_of_DRAM()
+
+static inline unsigned long crash_addr_low_max(void)
+{
+	unsigned long long crash_max = idmap_to_phys((u32)~0);
+	unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
+
+	return (crash_max > lowmem_max) ? lowmem_max : crash_max;
+}
+
+
+#define HAVE_ARCH_ADD_CRASH_RES_TO_IOMEM_EARLY
+#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 59e1a13b5cf6..3a8b6f08f4ec 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -979,13 +979,6 @@ static int __init init_machine_late(void)
 }
 late_initcall(init_machine_late);
 
-#ifdef CONFIG_CRASH_RESERVE
-/*
- * The crash region must be aligned to 128MB to avoid
- * zImage relocating below the reserved region.
- */
-#define CRASH_ALIGN	(128 << 20)
-
 static inline unsigned long long get_total_mem(void)
 {
 	unsigned long total;
@@ -994,61 +987,27 @@ static inline unsigned long long get_total_mem(void)
 	return total << PAGE_SHIFT;
 }
 
-/**
- * reserve_crashkernel() - reserves memory are for crash kernel
- *
- * This function reserves memory area given in "crashkernel=" kernel command
- * line parameter. The memory reserved is used by a dump capture kernel when
- * primary kernel is crashing.
- */
-static void __init reserve_crashkernel(void)
+static void __init arch_reserve_crashkernel(void)
 {
 	unsigned long long crash_size, crash_base;
+	unsigned long long low_size = 0;
 	unsigned long long total_mem;
+	bool high = false;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_CRASH_RESERVE))
+		return;
+
 	total_mem = get_total_mem();
 	ret = parse_crashkernel(boot_command_line, total_mem,
 				&crash_size, &crash_base,
-				NULL, NULL);
+				&low_size, &high);
 	/* invalid value specified or crashkernel=0 */
 	crash_size = (phys_addr_t)crash_size;
 	if (ret || !crash_size)
 		return;
 
-	if (crash_base <= 0) {
-		unsigned long long crash_max = idmap_to_phys((u32)~0);
-		unsigned long long lowmem_max = __pa(high_memory - 1) + 1;
-		if (crash_max > lowmem_max)
-			crash_max = lowmem_max;
-
-		crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
-						       CRASH_ALIGN, crash_max);
-		if (!crash_base) {
-			pr_err("crashkernel reservation failed - No suitable area found.\n");
-			return;
-		}
-	} else {
-		unsigned long long crash_max = crash_base + crash_size;
-		unsigned long long start;
-
-		start = memblock_phys_alloc_range(crash_size, SECTION_SIZE,
-						  crash_base, crash_max);
-		if (!start) {
-			pr_err("crashkernel reservation failed - memory is in use.\n");
-			return;
-		}
-	}
-
-	pr_info("Reserving %ldMB of memory at %ldMB for crashkernel (System RAM: %ldMB)\n",
-		(unsigned long)(crash_size >> 20),
-		(unsigned long)(crash_base >> 20),
-		(unsigned long)(total_mem >> 20));
-
-	/* The crashk resource must always be located in normal mem */
-	crashk_res.start = crash_base;
-	crashk_res.end = crash_base + crash_size - 1;
-	insert_resource(&iomem_resource, &crashk_res);
+	reserve_crashkernel_generic(boot_command_line, crash_size, crash_base, low_size, high);
 
 	if (arm_has_idmap_alias()) {
 		/*
@@ -1065,9 +1024,6 @@ static void __init reserve_crashkernel(void)
 		insert_resource(&iomem_resource, &crashk_boot_res);
 	}
 }
-#else
-static inline void reserve_crashkernel(void) {}
-#endif /* CONFIG_CRASH_RESERVE*/
 
 void __init hyp_mode_check(void)
 {
@@ -1190,7 +1146,7 @@ void __init setup_arch(char **cmdline_p)
 	if (!is_smp())
 		hyp_mode_check();
 
-	reserve_crashkernel();
+	arch_reserve_crashkernel();
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
-- 
2.34.1

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> Currently, x86, arm64, riscv and loongarch has been switched to generic
> crashkernel reservation. Also use generic interface to simplify crashkernel
> reservation for arm32, and fix two bugs by the way.

I am not sure if this is a good idea. I added the generic reservation
itnerfaces for ARCH which support crashkernel=,high|low and normal
crashkernel reservation, with this, the code can be simplified a lot.
However, arm32 doesn't support crashkernel=,high, I am not sure if it's
worth taking the change, most importantly, if it will cause
misunderstanding or misoperation.

Thanks
Baoquan


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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> Currently, x86, arm64, riscv and loongarch has been switched to generic
> crashkernel reservation. Also use generic interface to simplify crashkernel
> reservation for arm32, and fix two bugs by the way.

I am not sure if this is a good idea. I added the generic reservation
itnerfaces for ARCH which support crashkernel=,high|low and normal
crashkernel reservation, with this, the code can be simplified a lot.
However, arm32 doesn't support crashkernel=,high, I am not sure if it's
worth taking the change, most importantly, if it will cause
misunderstanding or misoperation.

Thanks
Baoquan

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
> 
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.

Yes, arm32 doesn't support crashkernel=,high.

However, a little enhancement to the generic code (please see the first
patch), the generic reservation interfaces can also be applicable to
architectures that do not support "high" such as arm32, and it can also
simplify the code (please see the third patch).

> 
> Thanks
> Baoquan
> 
> 

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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
> 
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.

Yes, arm32 doesn't support crashkernel=,high.

However, a little enhancement to the generic code (please see the first
patch), the generic reservation interfaces can also be applicable to
architectures that do not support "high" such as arm32, and it can also
simplify the code (please see the third patch).

> 
> Thanks
> Baoquan
> 
> 

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 17:29, Baoquan He wrote:
> > On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >> crashkernel reservation. Also use generic interface to simplify crashkernel
> >> reservation for arm32, and fix two bugs by the way.
> > 
> > I am not sure if this is a good idea. I added the generic reservation
> > itnerfaces for ARCH which support crashkernel=,high|low and normal
> > crashkernel reservation, with this, the code can be simplified a lot.
> > However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> > worth taking the change, most importantly, if it will cause
> > misunderstanding or misoperation.
> 
> Yes, arm32 doesn't support crashkernel=,high.
> 
> However, a little enhancement to the generic code (please see the first
> patch), the generic reservation interfaces can also be applicable to
> architectures that do not support "high" such as arm32, and it can also
> simplify the code (please see the third patch).

Yeah, I can see the code is simplified. When you specified
'crashkernel=xM,high', do you think what should be warn out? Because
it's an unsupported syntax on arm32, we should do something to print out
appropriate message.


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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 17:29, Baoquan He wrote:
> > On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >> crashkernel reservation. Also use generic interface to simplify crashkernel
> >> reservation for arm32, and fix two bugs by the way.
> > 
> > I am not sure if this is a good idea. I added the generic reservation
> > itnerfaces for ARCH which support crashkernel=,high|low and normal
> > crashkernel reservation, with this, the code can be simplified a lot.
> > However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> > worth taking the change, most importantly, if it will cause
> > misunderstanding or misoperation.
> 
> Yes, arm32 doesn't support crashkernel=,high.
> 
> However, a little enhancement to the generic code (please see the first
> patch), the generic reservation interfaces can also be applicable to
> architectures that do not support "high" such as arm32, and it can also
> simplify the code (please see the third patch).

Yeah, I can see the code is simplified. When you specified
'crashkernel=xM,high', do you think what should be warn out? Because
it's an unsupported syntax on arm32, we should do something to print out
appropriate message.

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 18:39, Baoquan He wrote:
> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 17:29, Baoquan He wrote:
>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>> reservation for arm32, and fix two bugs by the way.
>>>
>>> I am not sure if this is a good idea. I added the generic reservation
>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>> crashkernel reservation, with this, the code can be simplified a lot.
>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>> worth taking the change, most importantly, if it will cause
>>> misunderstanding or misoperation.
>>
>> Yes, arm32 doesn't support crashkernel=,high.
>>
>> However, a little enhancement to the generic code (please see the first
>> patch), the generic reservation interfaces can also be applicable to
>> architectures that do not support "high" such as arm32, and it can also
>> simplify the code (please see the third patch).
> 
> Yeah, I can see the code is simplified. When you specified
> 'crashkernel=xM,high', do you think what should be warn out? Because
> it's an unsupported syntax on arm32, we should do something to print out
> appropriate message.

Yes, you are right! In this patch it will print "crashkernel high memory
reservation failed." message and out for arm32 if you specify
'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
"CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
out for other similar architecture.


> 
> 

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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 18:39, Baoquan He wrote:
> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 17:29, Baoquan He wrote:
>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>> reservation for arm32, and fix two bugs by the way.
>>>
>>> I am not sure if this is a good idea. I added the generic reservation
>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>> crashkernel reservation, with this, the code can be simplified a lot.
>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>> worth taking the change, most importantly, if it will cause
>>> misunderstanding or misoperation.
>>
>> Yes, arm32 doesn't support crashkernel=,high.
>>
>> However, a little enhancement to the generic code (please see the first
>> patch), the generic reservation interfaces can also be applicable to
>> architectures that do not support "high" such as arm32, and it can also
>> simplify the code (please see the third patch).
> 
> Yeah, I can see the code is simplified. When you specified
> 'crashkernel=xM,high', do you think what should be warn out? Because
> it's an unsupported syntax on arm32, we should do something to print out
> appropriate message.

Yes, you are right! In this patch it will print "crashkernel high memory
reservation failed." message and out for arm32 if you specify
'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
"CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
out for other similar architecture.


> 
> 

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 18:39, Baoquan He wrote:
> > On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 17:29, Baoquan He wrote:
> >>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>> reservation for arm32, and fix two bugs by the way.
> >>>
> >>> I am not sure if this is a good idea. I added the generic reservation
> >>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>> crashkernel reservation, with this, the code can be simplified a lot.
> >>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>> worth taking the change, most importantly, if it will cause
> >>> misunderstanding or misoperation.
> >>
> >> Yes, arm32 doesn't support crashkernel=,high.
> >>
> >> However, a little enhancement to the generic code (please see the first
> >> patch), the generic reservation interfaces can also be applicable to
> >> architectures that do not support "high" such as arm32, and it can also
> >> simplify the code (please see the third patch).
> > 
> > Yeah, I can see the code is simplified. When you specified
> > 'crashkernel=xM,high', do you think what should be warn out? Because
> > it's an unsupported syntax on arm32, we should do something to print out
> > appropriate message.
> 
> Yes, you are right! In this patch it will print "crashkernel high memory
> reservation failed." message and out for arm32 if you specify

That message may mislead people to believe crashkernel=,high is
supported but reservation is failed, then a bug need be filed for this?
We may expect a message telling this syntax is not supported on this
ARCH.

> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> out for other similar architecture.
> 
> 
> > 
> > 
> 


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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 18:39, Baoquan He wrote:
> > On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 17:29, Baoquan He wrote:
> >>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>> reservation for arm32, and fix two bugs by the way.
> >>>
> >>> I am not sure if this is a good idea. I added the generic reservation
> >>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>> crashkernel reservation, with this, the code can be simplified a lot.
> >>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>> worth taking the change, most importantly, if it will cause
> >>> misunderstanding or misoperation.
> >>
> >> Yes, arm32 doesn't support crashkernel=,high.
> >>
> >> However, a little enhancement to the generic code (please see the first
> >> patch), the generic reservation interfaces can also be applicable to
> >> architectures that do not support "high" such as arm32, and it can also
> >> simplify the code (please see the third patch).
> > 
> > Yeah, I can see the code is simplified. When you specified
> > 'crashkernel=xM,high', do you think what should be warn out? Because
> > it's an unsupported syntax on arm32, we should do something to print out
> > appropriate message.
> 
> Yes, you are right! In this patch it will print "crashkernel high memory
> reservation failed." message and out for arm32 if you specify

That message may mislead people to believe crashkernel=,high is
supported but reservation is failed, then a bug need be filed for this?
We may expect a message telling this syntax is not supported on this
ARCH.

> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> out for other similar architecture.
> 
> 
> > 
> > 
> 

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 22:06, Baoquan He wrote:
> On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 18:39, Baoquan He wrote:
>>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>>>
>>>>
>>>> On 2024/7/9 17:29, Baoquan He wrote:
>>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>>>> reservation for arm32, and fix two bugs by the way.
>>>>>
>>>>> I am not sure if this is a good idea. I added the generic reservation
>>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>>>> crashkernel reservation, with this, the code can be simplified a lot.
>>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>>>> worth taking the change, most importantly, if it will cause
>>>>> misunderstanding or misoperation.
>>>>
>>>> Yes, arm32 doesn't support crashkernel=,high.
>>>>
>>>> However, a little enhancement to the generic code (please see the first
>>>> patch), the generic reservation interfaces can also be applicable to
>>>> architectures that do not support "high" such as arm32, and it can also
>>>> simplify the code (please see the third patch).
>>>
>>> Yeah, I can see the code is simplified. When you specified
>>> 'crashkernel=xM,high', do you think what should be warn out? Because
>>> it's an unsupported syntax on arm32, we should do something to print out
>>> appropriate message.
>>
>> Yes, you are right! In this patch it will print "crashkernel high memory
>> reservation failed." message and out for arm32 if you specify
> 
> That message may mislead people to believe crashkernel=,high is
> supported but reservation is failed, then a bug need be filed for this?
> We may expect a message telling this syntax is not supported on this
> ARCH.

"CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
not support "crashkernel=,high", I wonder if this is generic for similar
architecture. If so, the first patch can print such as
"crashkernel=,high is not supported on this ARCH" message.

> 
>> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
>> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
>> out for other similar architecture.
>>
>>
>>>
>>>
>>
> 
> 

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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 22:06, Baoquan He wrote:
> On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
>>
>>
>> On 2024/7/9 18:39, Baoquan He wrote:
>>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
>>>>
>>>>
>>>> On 2024/7/9 17:29, Baoquan He wrote:
>>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
>>>>>> reservation for arm32, and fix two bugs by the way.
>>>>>
>>>>> I am not sure if this is a good idea. I added the generic reservation
>>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
>>>>> crashkernel reservation, with this, the code can be simplified a lot.
>>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
>>>>> worth taking the change, most importantly, if it will cause
>>>>> misunderstanding or misoperation.
>>>>
>>>> Yes, arm32 doesn't support crashkernel=,high.
>>>>
>>>> However, a little enhancement to the generic code (please see the first
>>>> patch), the generic reservation interfaces can also be applicable to
>>>> architectures that do not support "high" such as arm32, and it can also
>>>> simplify the code (please see the third patch).
>>>
>>> Yeah, I can see the code is simplified. When you specified
>>> 'crashkernel=xM,high', do you think what should be warn out? Because
>>> it's an unsupported syntax on arm32, we should do something to print out
>>> appropriate message.
>>
>> Yes, you are right! In this patch it will print "crashkernel high memory
>> reservation failed." message and out for arm32 if you specify
> 
> That message may mislead people to believe crashkernel=,high is
> supported but reservation is failed, then a bug need be filed for this?
> We may expect a message telling this syntax is not supported on this
> ARCH.

"CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
not support "crashkernel=,high", I wonder if this is generic for similar
architecture. If so, the first patch can print such as
"crashkernel=,high is not supported on this ARCH" message.

> 
>> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
>> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
>> out for other similar architecture.
>>
>>
>>>
>>>
>>
> 
> 

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/10/24 at 09:52am, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 22:06, Baoquan He wrote:
> > On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 18:39, Baoquan He wrote:
> >>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>>>
> >>>>
> >>>> On 2024/7/9 17:29, Baoquan He wrote:
> >>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>>>> reservation for arm32, and fix two bugs by the way.
> >>>>>
> >>>>> I am not sure if this is a good idea. I added the generic reservation
> >>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>>>> crashkernel reservation, with this, the code can be simplified a lot.
> >>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>>>> worth taking the change, most importantly, if it will cause
> >>>>> misunderstanding or misoperation.
> >>>>
> >>>> Yes, arm32 doesn't support crashkernel=,high.
> >>>>
> >>>> However, a little enhancement to the generic code (please see the first
> >>>> patch), the generic reservation interfaces can also be applicable to
> >>>> architectures that do not support "high" such as arm32, and it can also
> >>>> simplify the code (please see the third patch).
> >>>
> >>> Yeah, I can see the code is simplified. When you specified
> >>> 'crashkernel=xM,high', do you think what should be warn out? Because
> >>> it's an unsupported syntax on arm32, we should do something to print out
> >>> appropriate message.
> >>
> >> Yes, you are right! In this patch it will print "crashkernel high memory
> >> reservation failed." message and out for arm32 if you specify
> > 
> > That message may mislead people to believe crashkernel=,high is
> > supported but reservation is failed, then a bug need be filed for this?
> > We may expect a message telling this syntax is not supported on this
> > ARCH.
> 
> "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
> not support "crashkernel=,high", I wonder if this is generic for similar

Imagine you are a testing engineer or a distros user, how do you know
if "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" when you test
'crashkernel=,high' and see the failure message?

> architecture. If so, the first patch can print such as
> "crashkernel=,high is not supported on this ARCH" message.

Please consider conprehensively if this is doable, you can paste
draft code here to prove it.

> 
> > 
> >> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> >> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> >> out for other similar architecture.
> >>
> >>
> >>>
> >>>
> >>
> > 
> > 
> 


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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Baoquan He]

On 07/10/24 at 09:52am, Jinjie Ruan wrote:
> 
> 
> On 2024/7/9 22:06, Baoquan He wrote:
> > On 07/09/24 at 07:06pm, Jinjie Ruan wrote:
> >>
> >>
> >> On 2024/7/9 18:39, Baoquan He wrote:
> >>> On 07/09/24 at 05:50pm, Jinjie Ruan wrote:
> >>>>
> >>>>
> >>>> On 2024/7/9 17:29, Baoquan He wrote:
> >>>>> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
> >>>>>> Currently, x86, arm64, riscv and loongarch has been switched to generic
> >>>>>> crashkernel reservation. Also use generic interface to simplify crashkernel
> >>>>>> reservation for arm32, and fix two bugs by the way.
> >>>>>
> >>>>> I am not sure if this is a good idea. I added the generic reservation
> >>>>> itnerfaces for ARCH which support crashkernel=,high|low and normal
> >>>>> crashkernel reservation, with this, the code can be simplified a lot.
> >>>>> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> >>>>> worth taking the change, most importantly, if it will cause
> >>>>> misunderstanding or misoperation.
> >>>>
> >>>> Yes, arm32 doesn't support crashkernel=,high.
> >>>>
> >>>> However, a little enhancement to the generic code (please see the first
> >>>> patch), the generic reservation interfaces can also be applicable to
> >>>> architectures that do not support "high" such as arm32, and it can also
> >>>> simplify the code (please see the third patch).
> >>>
> >>> Yeah, I can see the code is simplified. When you specified
> >>> 'crashkernel=xM,high', do you think what should be warn out? Because
> >>> it's an unsupported syntax on arm32, we should do something to print out
> >>> appropriate message.
> >>
> >> Yes, you are right! In this patch it will print "crashkernel high memory
> >> reservation failed." message and out for arm32 if you specify
> > 
> > That message may mislead people to believe crashkernel=,high is
> > supported but reservation is failed, then a bug need be filed for this?
> > We may expect a message telling this syntax is not supported on this
> > ARCH.
> 
> "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" indicate that the arm32 does
> not support "crashkernel=,high", I wonder if this is generic for similar

Imagine you are a testing engineer or a distros user, how do you know
if "CRASH_ADDR_LOW_MAX >= CRASH_ADDR_HIGH_MAX" when you test
'crashkernel=,high' and see the failure message?

> architecture. If so, the first patch can print such as
> "crashkernel=,high is not supported on this ARCH" message.

Please consider conprehensively if this is doable, you can paste
draft code here to prove it.

> 
> > 
> >> 'crashkernel=xM,high because "CRASH_ADDR_LOW_MAX" and
> >> "CRASH_ADDR_HIGH_MAX" is identical for arm32. And it should also warn
> >> out for other similar architecture.
> >>
> >>
> >>>
> >>>
> >>
> > 
> > 
> 

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
> 
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.

It seems that x86_32 doesn't support crashkernel=,high and use the
generic interface，and also have the first patch bug. I’ll resend the
first patch and explain it.

> 
> Thanks
> Baoquan
> 
> 

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

Re: [PATCH 0/3] ARM: Use generic interface to simplify crashkernel reservation

[Jinjie Ruan]

On 2024/7/9 17:29, Baoquan He wrote:
> On 07/08/24 at 09:33pm, Jinjie Ruan wrote:
>> Currently, x86, arm64, riscv and loongarch has been switched to generic
>> crashkernel reservation. Also use generic interface to simplify crashkernel
>> reservation for arm32, and fix two bugs by the way.
> 
> I am not sure if this is a good idea. I added the generic reservation
> itnerfaces for ARCH which support crashkernel=,high|low and normal
> crashkernel reservation, with this, the code can be simplified a lot.
> However, arm32 doesn't support crashkernel=,high, I am not sure if it's
> worth taking the change, most importantly, if it will cause
> misunderstanding or misoperation.

It seems that x86_32 doesn't support crashkernel=,high and use the
generic interface，and also have the first patch bug. I’ll resend the
first patch and explain it.

> 
> Thanks
> Baoquan
> 
>