Re: [PATCH 1/2 RESEND v8] x86/kdump: always reserve the low 1M when the crashkernel option is specified
在 2019年10月31日 18:47, Borislav Petkov 写道: > On Thu, Oct 31, 2019 at 05:40:35PM +0800, lijiang wrote: >> Maybe it should be a separate patch to fix the old compile warnings as >> follow. >> And i should put the patch into this series. > > Yes, maybe. > >> commit d2091d1f4f67f1c38293b0e93fdbfefa766940cf (HEAD -> master) >> Author: Lianbo Jiang >> Date: Thu Oct 31 15:48:02 2019 +0800 >> >> kexec: Fix i386 build warnings that missed declaration of struct kimage >> >> Kbuild test robot reported some build warnings, please refer to the >> Link below for details. > > Explain here what the warnings are, why they trigger and how you're > fixing it. How a commit message should look like is also explained in > that document I pointed you at. > OK, looks better('what-why-how'). I will improve the above log. > Refering to some link is not what we do in commit messages. > >> Add a declaration of struct kimage to fix these compile warnings. >> >> Fixes: dd5f726076cc ("kexec: support for kexec on panic using new system >> call") >> Reported-by: kbuild test robot >> Signed-off-by: Lianbo Jiang >> Link: https://lkml.org/lkml/2019/10/30/833 > > *NEVER* use lkml.org or any other external URL for refering to mail > threads but *always* use our own > > lkml.kernel.org/r/ > > redirector. See other tip commits for an example. > It's useful to me. Thanks. >>> You can read >>> >>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html >>> >>> in the meantime, especially section >>> >>> "9) Don't get discouraged - or impatient" >>> >>> while waiting. >> >> OK. Thanks. > > And make sure to read that whole document and also have a look at the > process document > I will read the above document carefully. But some of the rules in the document are still easy to be forgot, maybe need to practice repeatedly. > https://www.kernel.org/doc/html/latest/process/index.html > > so that you can avoid such mistakes in the future. > Good suggestions. Thank you so much. Lianbo > Thx. > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/2 RESEND v8] x86/kdump: always reserve the low 1M when the crashkernel option is specified
On Thu, Oct 31, 2019 at 05:40:35PM +0800, lijiang wrote: > Maybe it should be a separate patch to fix the old compile warnings as follow. > And i should put the patch into this series. Yes, maybe. > commit d2091d1f4f67f1c38293b0e93fdbfefa766940cf (HEAD -> master) > Author: Lianbo Jiang > Date: Thu Oct 31 15:48:02 2019 +0800 > > kexec: Fix i386 build warnings that missed declaration of struct kimage > > Kbuild test robot reported some build warnings, please refer to the > Link below for details. Explain here what the warnings are, why they trigger and how you're fixing it. How a commit message should look like is also explained in that document I pointed you at. Refering to some link is not what we do in commit messages. > Add a declaration of struct kimage to fix these compile warnings. > > Fixes: dd5f726076cc ("kexec: support for kexec on panic using new system > call") > Reported-by: kbuild test robot > Signed-off-by: Lianbo Jiang > Link: https://lkml.org/lkml/2019/10/30/833 *NEVER* use lkml.org or any other external URL for refering to mail threads but *always* use our own lkml.kernel.org/r/ redirector. See other tip commits for an example. > > You can read > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > > > in the meantime, especially section > > > > "9) Don't get discouraged - or impatient" > > > > while waiting. > > OK. Thanks. And make sure to read that whole document and also have a look at the process document https://www.kernel.org/doc/html/latest/process/index.html so that you can avoid such mistakes in the future. Thx. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/2 RESEND v8] x86/kdump: always reserve the low 1M when the crashkernel option is specified
在 2019年10月31日 15:13, Borislav Petkov 写道: > Please do not merge a 0day bot fix with another patch of yours which > does not cause it in the first place. When you look at this patch alone, > what do you think the Reported-by tag means, if anything at all? > Thanks for your suggestions. Maybe it should be a separate patch to fix the old compile warnings as follow. And i should put the patch into this series. commit d2091d1f4f67f1c38293b0e93fdbfefa766940cf (HEAD -> master) Author: Lianbo Jiang Date: Thu Oct 31 15:48:02 2019 +0800 kexec: Fix i386 build warnings that missed declaration of struct kimage Kbuild test robot reported some build warnings, please refer to the Link below for details. Add a declaration of struct kimage to fix these compile warnings. Fixes: dd5f726076cc ("kexec: support for kexec on panic using new system call") Reported-by: kbuild test robot Signed-off-by: Lianbo Jiang Link: https://lkml.org/lkml/2019/10/30/833 diff --git a/arch/x86/include/asm/crash.h b/arch/x86/include/asm/crash.h index 0acf5ee45a21..ef5638f641f2 100644 --- a/arch/x86/include/asm/crash.h +++ b/arch/x86/include/asm/crash.h @@ -2,6 +2,8 @@ #ifndef _ASM_X86_CRASH_H #define _ASM_X86_CRASH_H +struct kimage; + int crash_load_segments(struct kimage *image); int crash_copy_backup_region(struct kimage *image); int crash_setup_memmap_entries(struct kimage *image, > Also, it is not a "RESEND" if you change them. You can call them v8.1 or > whatever to denote that the change is small. > Thanks for your explanation in detail. > Also, do not send v9 or v8.1 or whatever, immediately but wait for other > reviews. OK. Lets wait a week or more. > You have sent these patches 4(!) times in this week alone. How > would you feel if I hammer your inbox with patches on a daily basis? >Probably because the change is small. Anyway, so sorry, it seems inconsiderate. > You can read > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > in the meantime, especially section > > "9) Don't get discouraged - or impatient" > > while waiting. OK. Thanks. Lianbo ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/2 RESEND v8] x86/kdump: always reserve the low 1M when the crashkernel option is specified
On Thu, Oct 31, 2019 at 11:35:16AM +0800, Lianbo Jiang wrote: > Kdump kernel will reuse the first 640k region because the real mode > trampoline has to work in this area. When the vmcore is dumped, the > old memory in this area may be accessed, therefore, kernel has to > copy the contents of the first 640k area to a backup region so that > kdump kernel can read the old memory from the backup area of the > first 640k area, which is done in the purgatory(). > > But, the current handling of copying the first 640k area runs into > problems when SME is enabled, kernel does not properly copy these > old memory to the backup area in the purgatory(), thereby, kdump > kernel reads out the encrypted contents, because the kdump kernel > must access the first kernel's memory with the encryption bit set > when SME is enabled in the first kernel. Please refer to this link: > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204793 > > Finally, it causes the following errors, and the crash tool gets > invalid pointers when parsing the vmcore. > > crash> kmem -s|grep -i invalid > kmem: dma-kmalloc-512: slab:d77680001c00 invalid > freepointer:a6086ac099f0c5a4 > kmem: dma-kmalloc-512: slab:d77680001c00 invalid > freepointer:a6086ac099f0c5a4 > crash> > > To avoid the above errors, when the crashkernel option is specified, > lets reserve the remaining low 1M memory(after reserving real mode > memory) so that the allocated memory does not fall into the low 1M > area, which makes us not to copy the first 640k content to a backup > region in purgatory(). This indicates that it does not need to be > included in crash dumps or used for anything except the processor > trampolines that must live in the low 1M. > > Signed-off-by: Lianbo Jiang > Reported-by: kbuild test robot Please do not merge a 0day bot fix with another patch of yours which does not cause it in the first place. When you look at this patch alone, what do you think the Reported-by tag means, if anything at all? Also, it is not a "RESEND" if you change them. You can call them v8.1 or whatever to denote that the change is small. Also, do not send v9 or v8.1 or whatever, immediately but wait for other reviews. You have sent these patches 4(!) times in this week alone. How would you feel if I hammer your inbox with patches on a daily basis? You can read https://www.kernel.org/doc/html/latest/process/submitting-patches.html in the meantime, especially section "9) Don't get discouraged - or impatient" while waiting. Thx. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/2 RESEND v8] x86/kdump: always reserve the low 1M when the crashkernel option is specified
Kdump kernel will reuse the first 640k region because the real mode trampoline has to work in this area. When the vmcore is dumped, the old memory in this area may be accessed, therefore, kernel has to copy the contents of the first 640k area to a backup region so that kdump kernel can read the old memory from the backup area of the first 640k area, which is done in the purgatory(). But, the current handling of copying the first 640k area runs into problems when SME is enabled, kernel does not properly copy these old memory to the backup area in the purgatory(), thereby, kdump kernel reads out the encrypted contents, because the kdump kernel must access the first kernel's memory with the encryption bit set when SME is enabled in the first kernel. Please refer to this link: Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204793 Finally, it causes the following errors, and the crash tool gets invalid pointers when parsing the vmcore. crash> kmem -s|grep -i invalid kmem: dma-kmalloc-512: slab:d77680001c00 invalid freepointer:a6086ac099f0c5a4 kmem: dma-kmalloc-512: slab:d77680001c00 invalid freepointer:a6086ac099f0c5a4 crash> To avoid the above errors, when the crashkernel option is specified, lets reserve the remaining low 1M memory(after reserving real mode memory) so that the allocated memory does not fall into the low 1M area, which makes us not to copy the first 640k content to a backup region in purgatory(). This indicates that it does not need to be included in crash dumps or used for anything except the processor trampolines that must live in the low 1M. Signed-off-by: Lianbo Jiang Reported-by: kbuild test robot --- arch/x86/include/asm/crash.h | 8 arch/x86/kernel/crash.c | 15 +++ arch/x86/realmode/init.c | 2 ++ 3 files changed, 25 insertions(+) diff --git a/arch/x86/include/asm/crash.h b/arch/x86/include/asm/crash.h index 0acf5ee45a21..88eadd08ad70 100644 --- a/arch/x86/include/asm/crash.h +++ b/arch/x86/include/asm/crash.h @@ -2,10 +2,18 @@ #ifndef _ASM_X86_CRASH_H #define _ASM_X86_CRASH_H +struct kimage; + int crash_load_segments(struct kimage *image); int crash_copy_backup_region(struct kimage *image); int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params); void crash_smp_send_stop(void); +#ifdef CONFIG_KEXEC_CORE +void __init crash_reserve_low_1M(void); +#else +static inline void __init crash_reserve_low_1M(void) { } +#endif + #endif /* _ASM_X86_CRASH_H */ diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index eb651fbde92a..db2301afade5 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -39,6 +40,7 @@ #include #include #include +#include /* Used while preparing memory map entries for second kernel */ struct crash_memmap_data { @@ -68,6 +70,19 @@ static inline void cpu_crash_vmclear_loaded_vmcss(void) rcu_read_unlock(); } +/* + * When the crashkernel option is specified, only use the low + * 1M for the real mode trampoline. + */ +void __init crash_reserve_low_1M(void) +{ + if (cmdline_find_option(boot_command_line, "crashkernel", + NULL, 0) > 0) { + memblock_reserve(0, 1<<20); + pr_info("Reserving the low 1M of memory for crashkernel\n"); + } +} + #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC) static void kdump_nmi_callback(int cpu, struct pt_regs *regs) diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c index 7dce39c8c034..262f83cad355 100644 --- a/arch/x86/realmode/init.c +++ b/arch/x86/realmode/init.c @@ -8,6 +8,7 @@ #include #include #include +#include struct real_mode_header *real_mode_header; u32 *trampoline_cr4_features; @@ -34,6 +35,7 @@ void __init reserve_real_mode(void) memblock_reserve(mem, size); set_real_mode_mem(mem); + crash_reserve_low_1M(); } static void __init setup_real_mode(void) -- 2.17.1 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec