Re: [PATCH 1/2 v5] x86/kdump: always reserve the low 1MiB when the crashkernel option is specified

[lijiang]

å 2019å10æ25æ 11:39, Eric W. Biederman åé:
> lijiang <lijiang@xxxxxxxxxx> writes:
> 
>>  * Returns the length of the argument (regardless of if it was
>>  * truncated to fit in the buffer), or -1 on not found.
>>  */
>> static int
>> __cmdline_find_option(const char *cmdline, int max_cmdline_size,
>>                       const char *option, char *buffer, int bufsize)
>>
>>
>> According to the above code comment, it should be better like this:
>>
>> +       if (cmdline_find_option(boot_command_line, "crashkernel",
>> +                               NULL, 0) > 0) {
>>
>> After i test, i will post again.
>>
> 
> This seems reasonable as we are dealing with x86 only code.
> 
When we compile the non-x86 kernel, that could cause the the compile error
because the cmdline_find_option() won't be defined on non-x86 architecture.
So i will define a weak function in the kernel/kexec_core.c like this:
+
+void __init __weak kexec_reserve_low_1MiB(void)
+{}

and implement the kexec_reserve_low_1MiB() in the arch/x86/kernel/machine_kexec_64.c.

+/*
+ * When the crashkernel option is specified, only use the low
+ * 1MiB for the real mode trampoline.
+ */
+void __init kexec_reserve_low_1MiB(void)
+{
+       if (cmdline_find_option(boot_command_line, "crashkernel",
+                               NULL, 0) > 0) {
+               memblock_reserve(0, 1<<20);
+               pr_info("Reserving the low 1MiB of memory for crashkernel\n");
+       }
+} 

That will solve the compile error on the non-x86 kernel, and it also works well on
the x86 kernel.

BTW: i pasted the code at the end, please refer to it.

> It wound be nice if someone could generalize cmdline_find_option to be
> arch independent so that crash_core.c:parse_crashkernel could use it.

Good point, that could be done in the future.

> I don't think for this patchset, but it looks like an overdue cleanup.
> 
> We run the risk with parse_crashkernel using strstr and this using
> another algorithm of having different kernel command line parsers
> giving different results and disagreeing if "crashkernel=" is present
> or not on the kernel command line.
> 
Indeed, but sometimes, the crashkernel has a complicated syntax, maybe
that could be a reason.

Thanks.
Lianbo

> Eric
> 
> 

---
 arch/x86/kernel/machine_kexec_64.c | 15 +++++++++++++++
 arch/x86/realmode/init.c           |  2 ++
 include/linux/kexec.h              |  2 ++
 kernel/kexec_core.c                |  3 +++
 4 files changed, 22 insertions(+)

diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 5dcd438ad8f2..42d7c15c45f1 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -17,6 +17,7 @@
 #include <linux/suspend.h>
 #include <linux/vmalloc.h>
 #include <linux/efi.h>
+#include <linux/memblock.h>
 
 #include <asm/init.h>
 #include <asm/pgtable.h>
@@ -27,6 +28,7 @@
 #include <asm/kexec-bzimage64.h>
 #include <asm/setup.h>
 #include <asm/set_memory.h>
+#include <asm/cmdline.h>
 
 #ifdef CONFIG_ACPI
 /*
@@ -687,3 +689,16 @@ void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
 	 */
 	set_memory_encrypted((unsigned long)vaddr, pages);
 }
+
+/*
+ * When the crashkernel option is specified, only use the low
+ * 1MiB for the real mode trampoline.
+ */
+void __init kexec_reserve_low_1MiB(void)
+{
+	if (cmdline_find_option(boot_command_line, "crashkernel",
+				NULL, 0) > 0) {
+		memblock_reserve(0, 1<<20);
+		pr_info("Reserving the low 1MiB of memory for crashkernel\n");
+	}
+}
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index 7dce39c8c034..064cc79a015d 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -3,6 +3,7 @@
 #include <linux/slab.h>
 #include <linux/memblock.h>
 #include <linux/mem_encrypt.h>
+#include <linux/kexec.h>
 
 #include <asm/set_memory.h>
 #include <asm/pgtable.h>
@@ -34,6 +35,7 @@ void __init reserve_real_mode(void)
 
 	memblock_reserve(mem, size);
 	set_real_mode_mem(mem);
+	kexec_reserve_low_1MiB();
 }
 
 static void __init setup_real_mode(void)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 1776eb2e43a4..988bf2de51a7 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -306,6 +306,7 @@ extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
 int kexec_crash_loaded(void);
+void __init kexec_reserve_low_1MiB(void);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 extern int kimage_crash_copy_vmcoreinfo(struct kimage *image);
 
@@ -397,6 +398,7 @@ static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
 static inline int kexec_crash_loaded(void) { return 0; }
+static inline void __init kexec_reserve_low_1MiB(void) { }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 15d70a90b50d..8856047bcdc8 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1213,3 +1213,6 @@ void __weak arch_kexec_protect_crashkres(void)
 
 void __weak arch_kexec_unprotect_crashkres(void)
 {}
+
+void __init __weak kexec_reserve_low_1MiB(void)
+{}
-- 
2.17.1