| Message ID | 20240129135033.157195-1-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [linux-next,1/3] x86, crash: don't nest CONFIG_CRASH_DUMP ifdef inside CONFIG_KEXEC_CODE ifdef scope | expand |
From: Baoquan He <bhe@redhat.com> Sent: Monday, January 29, 2024 5:51 AM > > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > arch/x86/xen/enlighten_hvm.c. Did some words get left out in the above sentence? It mentions the Xen case, but not the Hyper-V case. I'm not sure what you intended. > > Although the nesting works well too since CONFIG_CRASH_DUMP has > dependency on CONFIG_KEXEC_CORE, it may cause confuse because there s/confusion/confuse/ > are places where it's not nested, and people may think it need be nested s/need be/needs to be/ > even though it doesn't have to. > > Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of > CONFIG_KEXEC_CODE ifdeffery scope. > > And also fix a building error Nathan reported as below by replacing > CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. > > ==== > $ curl -LSso .config https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.x86_64 > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- > olddefconfig all > ... > x86_64-linux-ld: arch/x86/xen/mmu_pv.o: in function > `paddr_vmcoreinfo_note': > mmu_pv.c:(.text+0x3af3): undefined reference to `vmcoreinfo_note' > ==== > > Link: https://lore.kernel.org/all/SN6PR02MB4157931105FA68D72E3D3DB8D47B2@SN6PR02MB4157.namprd02.prod.outlook.com/T/#u > Link: https://lore.kernel.org/all/20240126045551.GA126645@dev-arch.thelio-3990X/T/#u > Signed-off-by: Baoquan He <bhe@redhat.com> Modulo the commit message nits, LGTM. Reviewed-by: Michael Kelley <mhklinux@outlook.com> > --- > arch/x86/kernel/cpu/mshyperv.c | 10 ++++++---- > arch/x86/kernel/reboot.c | 2 +- > arch/x86/xen/enlighten_hvm.c | 4 ++-- > arch/x86/xen/mmu_pv.c | 2 +- > 4 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > b/arch/x86/kernel/cpu/mshyperv.c > index f8163a59026b..2e8cd5a4ae85 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -209,6 +209,7 @@ static void hv_machine_shutdown(void) > if (kexec_in_progress) > hyperv_cleanup(); > } > +#endif /* CONFIG_KEXEC_CORE */ > > #ifdef CONFIG_CRASH_DUMP > static void hv_machine_crash_shutdown(struct pt_regs *regs) > @@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct > pt_regs *regs) > /* Disable the hypercall page when there is only 1 active CPU. */ > hyperv_cleanup(); > } > -#endif > -#endif /* CONFIG_KEXEC_CORE */ > +#endif /* CONFIG_CRASH_DUMP */ > #endif /* CONFIG_HYPERV */ > > static uint32_t __init ms_hyperv_platform(void) > @@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void) > no_timer_check = 1; > #endif > > -#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE) > +#if IS_ENABLED(CONFIG_HYPERV) > +#if defined(CONFIG_KEXEC_CORE) > machine_ops.shutdown = hv_machine_shutdown; > -#ifdef CONFIG_CRASH_DUMP > +#endif > +#if defined(CONFIG_CRASH_DUMP) > machine_ops.crash_shutdown = hv_machine_crash_shutdown; > #endif > #endif > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c > index 1287b0d5962f..f3130f762784 100644 > --- a/arch/x86/kernel/reboot.c > +++ b/arch/x86/kernel/reboot.c > @@ -826,7 +826,7 @@ void machine_halt(void) > machine_ops.halt(); > } > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_CRASH_DUMP > void machine_crash_shutdown(struct pt_regs *regs) > { > machine_ops.crash_shutdown(regs); > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c > index 09e3db7ff990..0b367c1e086d 100644 > --- a/arch/x86/xen/enlighten_hvm.c > +++ b/arch/x86/xen/enlighten_hvm.c > @@ -148,6 +148,7 @@ static void xen_hvm_shutdown(void) > if (kexec_in_progress) > xen_reboot(SHUTDOWN_soft_reset); > } > +#endif > > #ifdef CONFIG_CRASH_DUMP > static void xen_hvm_crash_shutdown(struct pt_regs *regs) > @@ -156,7 +157,6 @@ static void xen_hvm_crash_shutdown(struct pt_regs > *regs) > xen_reboot(SHUTDOWN_soft_reset); > } > #endif > -#endif > > static int xen_cpu_up_prepare_hvm(unsigned int cpu) > { > @@ -238,10 +238,10 @@ static void __init xen_hvm_guest_init(void) > > #ifdef CONFIG_KEXEC_CORE > machine_ops.shutdown = xen_hvm_shutdown; > +#endif > #ifdef CONFIG_CRASH_DUMP > machine_ops.crash_shutdown = xen_hvm_crash_shutdown; > #endif > -#endif > } > > static __init int xen_parse_nopv(char *arg) > diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c > index 218773cfb009..e21974f2cf2d 100644 > --- a/arch/x86/xen/mmu_pv.c > +++ b/arch/x86/xen/mmu_pv.c > @@ -2520,7 +2520,7 @@ int xen_remap_pfn(struct vm_area_struct *vma, > unsigned long addr, > } > EXPORT_SYMBOL_GPL(xen_remap_pfn); > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_VMCORE_INFO > phys_addr_t paddr_vmcoreinfo_note(void) > { > if (xen_pv_domain()) > -- > 2.41.0
On 01/29/24 at 06:27pm, Michael Kelley wrote: > From: Baoquan He <bhe@redhat.com> Sent: Monday, January 29, 2024 5:51 AM > > > > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > > arch/x86/xen/enlighten_hvm.c. > > Did some words get left out in the above sentence? It mentions the Xen > case, but not the Hyper-V case. I'm not sure what you intended. Thanks a lot for your careful reviewing. Yeah, I tried to list all affected file names, seems my vim editor threw away some words. And I forgot mentioning the change in reboot.c. I adjusted log as below according to your comments, do you think it's OK now? === Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside CONFIG_KEXEC_CODE ifdef scope in some XEN, HyperV codes. Although the nesting works well too since CONFIG_CRASH_DUMP has dependency on CONFIG_KEXEC_CORE, it may cause confusion because there are places where it's not nested, and people may think it needs be nested even though it doesn't have to. Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of CONFIG_KEXEC_CODE ifdeffery scope. And also put function machine_crash_shutdown() definition inside CONFIG_CRASH_DUMP ifdef scope instead of CONFIG_KEXEC_CORE ifdef. And also fix a building error Nathan reported as below by replacing CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. ...... === Thanks Baoquan
From: Baoquan He <bhe@redhat.com> > > On 01/29/24 at 06:27pm, Michael Kelley wrote: > > From: Baoquan He <bhe@redhat.com> Sent: Monday, January 29, 2024 > 5:51 AM > > > > > > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > > > arch/x86/xen/enlighten_hvm.c. > > > > Did some words get left out in the above sentence? It mentions the Xen > > case, but not the Hyper-V case. I'm not sure what you intended. > > Thanks a lot for your careful reviewing. > > Yeah, I tried to list all affected file names, seems my vim editor threw > away some words. And I forgot mentioning the change in reboot.c. > > I adjusted log as below according to your comments, do you think it's OK > now? Yes -- looks like everything is included and clear up my confusion. But I still have two small nits per below. :-) Michael > > === > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > CONFIG_KEXEC_CODE ifdef scope in some XEN, HyperV codes. s/Hyper-V/HyperV/ > > Although the nesting works well too since CONFIG_CRASH_DUMP has > dependency on CONFIG_KEXEC_CORE, it may cause confusion because there > are places where it's not nested, and people may think it needs be nested s/needs to be/needs be/ > even though it doesn't have to. > > Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of > CONFIG_KEXEC_CODE ifdeffery scope. > > And also put function machine_crash_shutdown() definition inside > CONFIG_CRASH_DUMP ifdef scope instead of CONFIG_KEXEC_CORE ifdef. > > And also fix a building error Nathan reported as below by replacing > CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. > ...... > === > > Thanks > Baoquan
Michael pointed out that the CONFIG_CRASH_DUMP ifdef is nested inside
CONFIG_KEXEC_CODE ifdef scope in some XEN, Hyper-V codes.
Although the nesting works well too since CONFIG_CRASH_DUMP has
dependency on CONFIG_KEXEC_CORE, it may cause confusion because there
are places where it's not nested, and people may think it needs to be
nested even though it doesn't have to.
Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of
CONFIG_KEXEC_CODE ifdeffery scope.
And also put function machine_crash_shutdown() definition inside
CONFIG_CRASH_DUMP ifdef scope instead of CONFIG_KEXEC_CORE ifdef.
And also fix a building error Nathan reported as below by replacing
CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef.
====
$ curl -LSso .config https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.x86_64
$ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- olddefconfig all
...
x86_64-linux-ld: arch/x86/xen/mmu_pv.o: in function `paddr_vmcoreinfo_note':
mmu_pv.c:(.text+0x3af3): undefined reference to `vmcoreinfo_note'
====
Link: https://lore.kernel.org/all/SN6PR02MB4157931105FA68D72E3D3DB8D47B2@SN6PR02MB4157.namprd02.prod.outlook.com/T/#u
Link: https://lore.kernel.org/all/20240126045551.GA126645@dev-arch.thelio-3990X/T/#u
Signed-off-by: Baoquan He <bhe@redhat.com>
---
v1->v2:
- Add missing words and fix typos in patch log pointed out by Michael.
arch/x86/kernel/cpu/mshyperv.c | 10 ++++++----
arch/x86/kernel/reboot.c | 2 +-
arch/x86/xen/enlighten_hvm.c | 4 ++--
arch/x86/xen/mmu_pv.c | 2 +-
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index f8163a59026b..2e8cd5a4ae85 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -209,6 +209,7 @@ static void hv_machine_shutdown(void)
if (kexec_in_progress)
hyperv_cleanup();
}
+#endif /* CONFIG_KEXEC_CORE */
#ifdef CONFIG_CRASH_DUMP
static void hv_machine_crash_shutdown(struct pt_regs *regs)
@@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs)
/* Disable the hypercall page when there is only 1 active CPU. */
hyperv_cleanup();
}
-#endif
-#endif /* CONFIG_KEXEC_CORE */
+#endif /* CONFIG_CRASH_DUMP */
#endif /* CONFIG_HYPERV */
static uint32_t __init ms_hyperv_platform(void)
@@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void)
no_timer_check = 1;
#endif
-#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE)
+#if IS_ENABLED(CONFIG_HYPERV)
+#if defined(CONFIG_KEXEC_CORE)
machine_ops.shutdown = hv_machine_shutdown;
-#ifdef CONFIG_CRASH_DUMP
+#endif
+#if defined(CONFIG_CRASH_DUMP)
machine_ops.crash_shutdown = hv_machine_crash_shutdown;
#endif
#endif
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1287b0d5962f..f3130f762784 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -826,7 +826,7 @@ void machine_halt(void)
machine_ops.halt();
}
-#ifdef CONFIG_KEXEC_CORE
+#ifdef CONFIG_CRASH_DUMP
void machine_crash_shutdown(struct pt_regs *regs)
{
machine_ops.crash_shutdown(regs);
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index 09e3db7ff990..0b367c1e086d 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -148,6 +148,7 @@ static void xen_hvm_shutdown(void)
if (kexec_in_progress)
xen_reboot(SHUTDOWN_soft_reset);
}
+#endif
#ifdef CONFIG_CRASH_DUMP
static void xen_hvm_crash_shutdown(struct pt_regs *regs)
@@ -156,7 +157,6 @@ static void xen_hvm_crash_shutdown(struct pt_regs *regs)
xen_reboot(SHUTDOWN_soft_reset);
}
#endif
-#endif
static int xen_cpu_up_prepare_hvm(unsigned int cpu)
{
@@ -238,10 +238,10 @@ static void __init xen_hvm_guest_init(void)
#ifdef CONFIG_KEXEC_CORE
machine_ops.shutdown = xen_hvm_shutdown;
+#endif
#ifdef CONFIG_CRASH_DUMP
machine_ops.crash_shutdown = xen_hvm_crash_shutdown;
#endif
-#endif
}
static __init int xen_parse_nopv(char *arg)
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 218773cfb009..e21974f2cf2d 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2520,7 +2520,7 @@ int xen_remap_pfn(struct vm_area_struct *vma, unsigned long addr,
}
EXPORT_SYMBOL_GPL(xen_remap_pfn);
-#ifdef CONFIG_KEXEC_CORE
+#ifdef CONFIG_VMCORE_INFO
phys_addr_t paddr_vmcoreinfo_note(void)
{
if (xen_pv_domain())
On 01/30/24 at 01:39am, Michael Kelley wrote: > From: Baoquan He <bhe@redhat.com> > > > > On 01/29/24 at 06:27pm, Michael Kelley wrote: > > > From: Baoquan He <bhe@redhat.com> Sent: Monday, January 29, 2024 > > 5:51 AM > > > > > > > > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > > > > arch/x86/xen/enlighten_hvm.c. > > > > > > Did some words get left out in the above sentence? It mentions the Xen > > > case, but not the Hyper-V case. I'm not sure what you intended. > > > > Thanks a lot for your careful reviewing. > > > > Yeah, I tried to list all affected file names, seems my vim editor threw > > away some words. And I forgot mentioning the change in reboot.c. > > > > I adjusted log as below according to your comments, do you think it's OK > > now? > > Yes -- looks like everything is included and clear up my confusion. But > I still have two small nits per below. :-) Right, I will grabbed them into v2. Thanks again. > > > > > === > > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > > CONFIG_KEXEC_CODE ifdef scope in some XEN, HyperV codes. > > s/Hyper-V/HyperV/ > > > > > Although the nesting works well too since CONFIG_CRASH_DUMP has > > dependency on CONFIG_KEXEC_CORE, it may cause confusion because there > > are places where it's not nested, and people may think it needs be nested > > s/needs to be/needs be/ > > > even though it doesn't have to. > > > > Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of > > CONFIG_KEXEC_CODE ifdeffery scope. > > > > And also put function machine_crash_shutdown() definition inside > > CONFIG_CRASH_DUMP ifdef scope instead of CONFIG_KEXEC_CORE ifdef. > > > > And also fix a building error Nathan reported as below by replacing > > CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. > > ...... > > === > > > > Thanks > > Baoquan >
From: Baoquan He <bhe@redhat.com> Sent: Monday, January 29, 2024 7:00 PM > > Michael pointed out that the CONFIG_CRASH_DUMP ifdef is nested inside > CONFIG_KEXEC_CODE ifdef scope in some XEN, Hyper-V codes. > > Although the nesting works well too since CONFIG_CRASH_DUMP has > dependency on CONFIG_KEXEC_CORE, it may cause confusion because there > are places where it's not nested, and people may think it needs to be > nested even though it doesn't have to. > > Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of > CONFIG_KEXEC_CODE ifdeffery scope. > > And also put function machine_crash_shutdown() definition inside > CONFIG_CRASH_DUMP ifdef scope instead of CONFIG_KEXEC_CORE ifdef. > > And also fix a building error Nathan reported as below by replacing > CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. > > ==== > $ curl -LSso .config https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.x86_64 > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- > olddefconfig all > ... > x86_64-linux-ld: arch/x86/xen/mmu_pv.o: in function > `paddr_vmcoreinfo_note': > mmu_pv.c:(.text+0x3af3): undefined reference to `vmcoreinfo_note' > ==== > > Link: https://lore.kernel.org/all/SN6PR02MB4157931105FA68D72E3D3DB8D47B2@SN6PR02MB4157.namprd02.prod.outlook.com/T/#u > Link: https://lore.kernel.org/all/20240126045551.GA126645@dev-arch.thelio-3990X/T/#u > Signed-off-by: Baoquan He <bhe@redhat.com> > --- > v1->v2: > - Add missing words and fix typos in patch log pointed out by Michael. > > arch/x86/kernel/cpu/mshyperv.c | 10 ++++++---- > arch/x86/kernel/reboot.c | 2 +- > arch/x86/xen/enlighten_hvm.c | 4 ++-- > arch/x86/xen/mmu_pv.c | 2 +- > 4 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mshyperv.c > b/arch/x86/kernel/cpu/mshyperv.c > index f8163a59026b..2e8cd5a4ae85 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -209,6 +209,7 @@ static void hv_machine_shutdown(void) > if (kexec_in_progress) > hyperv_cleanup(); > } > +#endif /* CONFIG_KEXEC_CORE */ > > #ifdef CONFIG_CRASH_DUMP > static void hv_machine_crash_shutdown(struct pt_regs *regs) > @@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct > pt_regs *regs) > /* Disable the hypercall page when there is only 1 active CPU. */ > hyperv_cleanup(); > } > -#endif > -#endif /* CONFIG_KEXEC_CORE */ > +#endif /* CONFIG_CRASH_DUMP */ > #endif /* CONFIG_HYPERV */ > > static uint32_t __init ms_hyperv_platform(void) > @@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void) > no_timer_check = 1; > #endif > > -#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE) > +#if IS_ENABLED(CONFIG_HYPERV) > +#if defined(CONFIG_KEXEC_CORE) > machine_ops.shutdown = hv_machine_shutdown; > -#ifdef CONFIG_CRASH_DUMP > +#endif > +#if defined(CONFIG_CRASH_DUMP) > machine_ops.crash_shutdown = hv_machine_crash_shutdown; > #endif > #endif > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c > index 1287b0d5962f..f3130f762784 100644 > --- a/arch/x86/kernel/reboot.c > +++ b/arch/x86/kernel/reboot.c > @@ -826,7 +826,7 @@ void machine_halt(void) > machine_ops.halt(); > } > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_CRASH_DUMP > void machine_crash_shutdown(struct pt_regs *regs) > { > machine_ops.crash_shutdown(regs); > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c > index 09e3db7ff990..0b367c1e086d 100644 > --- a/arch/x86/xen/enlighten_hvm.c > +++ b/arch/x86/xen/enlighten_hvm.c > @@ -148,6 +148,7 @@ static void xen_hvm_shutdown(void) > if (kexec_in_progress) > xen_reboot(SHUTDOWN_soft_reset); > } > +#endif > > #ifdef CONFIG_CRASH_DUMP > static void xen_hvm_crash_shutdown(struct pt_regs *regs) > @@ -156,7 +157,6 @@ static void xen_hvm_crash_shutdown(struct pt_regs > *regs) > xen_reboot(SHUTDOWN_soft_reset); > } > #endif > -#endif > > static int xen_cpu_up_prepare_hvm(unsigned int cpu) > { > @@ -238,10 +238,10 @@ static void __init xen_hvm_guest_init(void) > > #ifdef CONFIG_KEXEC_CORE > machine_ops.shutdown = xen_hvm_shutdown; > +#endif > #ifdef CONFIG_CRASH_DUMP > machine_ops.crash_shutdown = xen_hvm_crash_shutdown; > #endif > -#endif > } > > static __init int xen_parse_nopv(char *arg) > diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c > index 218773cfb009..e21974f2cf2d 100644 > --- a/arch/x86/xen/mmu_pv.c > +++ b/arch/x86/xen/mmu_pv.c > @@ -2520,7 +2520,7 @@ int xen_remap_pfn(struct vm_area_struct *vma, > unsigned long addr, > } > EXPORT_SYMBOL_GPL(xen_remap_pfn); > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_VMCORE_INFO > phys_addr_t paddr_vmcoreinfo_note(void) > { > if (xen_pv_domain()) > -- > 2.41.0 Reviewed-by: Michael Kelley <mhklinux@outlook.com>
This series resolves the build issues I was seeing. Please feel free to carry Tested-by: Nathan Chancellor <nathan@kernel.org> # build forward if there are any more revisions without drastic changes. On Mon, Jan 29, 2024 at 09:50:31PM +0800, Baoquan He wrote: > Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside > arch/x86/xen/enlighten_hvm.c. > > Although the nesting works well too since CONFIG_CRASH_DUMP has > dependency on CONFIG_KEXEC_CORE, it may cause confuse because there > are places where it's not nested, and people may think it need be nested > even though it doesn't have to. > > Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of > CONFIG_KEXEC_CODE ifdeffery scope. > > And also fix a building error Nathan reported as below by replacing > CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. > > ==== > $ curl -LSso .config https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.x86_64 > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- olddefconfig all > ... > x86_64-linux-ld: arch/x86/xen/mmu_pv.o: in function `paddr_vmcoreinfo_note': > mmu_pv.c:(.text+0x3af3): undefined reference to `vmcoreinfo_note' > ==== > > Link: https://lore.kernel.org/all/SN6PR02MB4157931105FA68D72E3D3DB8D47B2@SN6PR02MB4157.namprd02.prod.outlook.com/T/#u > Link: https://lore.kernel.org/all/20240126045551.GA126645@dev-arch.thelio-3990X/T/#u > Signed-off-by: Baoquan He <bhe@redhat.com> > --- > arch/x86/kernel/cpu/mshyperv.c | 10 ++++++---- > arch/x86/kernel/reboot.c | 2 +- > arch/x86/xen/enlighten_hvm.c | 4 ++-- > arch/x86/xen/mmu_pv.c | 2 +- > 4 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c > index f8163a59026b..2e8cd5a4ae85 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -209,6 +209,7 @@ static void hv_machine_shutdown(void) > if (kexec_in_progress) > hyperv_cleanup(); > } > +#endif /* CONFIG_KEXEC_CORE */ > > #ifdef CONFIG_CRASH_DUMP > static void hv_machine_crash_shutdown(struct pt_regs *regs) > @@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs) > /* Disable the hypercall page when there is only 1 active CPU. */ > hyperv_cleanup(); > } > -#endif > -#endif /* CONFIG_KEXEC_CORE */ > +#endif /* CONFIG_CRASH_DUMP */ > #endif /* CONFIG_HYPERV */ > > static uint32_t __init ms_hyperv_platform(void) > @@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void) > no_timer_check = 1; > #endif > > -#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE) > +#if IS_ENABLED(CONFIG_HYPERV) > +#if defined(CONFIG_KEXEC_CORE) > machine_ops.shutdown = hv_machine_shutdown; > -#ifdef CONFIG_CRASH_DUMP > +#endif > +#if defined(CONFIG_CRASH_DUMP) > machine_ops.crash_shutdown = hv_machine_crash_shutdown; > #endif > #endif > diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c > index 1287b0d5962f..f3130f762784 100644 > --- a/arch/x86/kernel/reboot.c > +++ b/arch/x86/kernel/reboot.c > @@ -826,7 +826,7 @@ void machine_halt(void) > machine_ops.halt(); > } > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_CRASH_DUMP > void machine_crash_shutdown(struct pt_regs *regs) > { > machine_ops.crash_shutdown(regs); > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c > index 09e3db7ff990..0b367c1e086d 100644 > --- a/arch/x86/xen/enlighten_hvm.c > +++ b/arch/x86/xen/enlighten_hvm.c > @@ -148,6 +148,7 @@ static void xen_hvm_shutdown(void) > if (kexec_in_progress) > xen_reboot(SHUTDOWN_soft_reset); > } > +#endif > > #ifdef CONFIG_CRASH_DUMP > static void xen_hvm_crash_shutdown(struct pt_regs *regs) > @@ -156,7 +157,6 @@ static void xen_hvm_crash_shutdown(struct pt_regs *regs) > xen_reboot(SHUTDOWN_soft_reset); > } > #endif > -#endif > > static int xen_cpu_up_prepare_hvm(unsigned int cpu) > { > @@ -238,10 +238,10 @@ static void __init xen_hvm_guest_init(void) > > #ifdef CONFIG_KEXEC_CORE > machine_ops.shutdown = xen_hvm_shutdown; > +#endif > #ifdef CONFIG_CRASH_DUMP > machine_ops.crash_shutdown = xen_hvm_crash_shutdown; > #endif > -#endif > } > > static __init int xen_parse_nopv(char *arg) > diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c > index 218773cfb009..e21974f2cf2d 100644 > --- a/arch/x86/xen/mmu_pv.c > +++ b/arch/x86/xen/mmu_pv.c > @@ -2520,7 +2520,7 @@ int xen_remap_pfn(struct vm_area_struct *vma, unsigned long addr, > } > EXPORT_SYMBOL_GPL(xen_remap_pfn); > > -#ifdef CONFIG_KEXEC_CORE > +#ifdef CONFIG_VMCORE_INFO > phys_addr_t paddr_vmcoreinfo_note(void) > { > if (xen_pv_domain()) > -- > 2.41.0 >
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index f8163a59026b..2e8cd5a4ae85 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -209,6 +209,7 @@ static void hv_machine_shutdown(void) if (kexec_in_progress) hyperv_cleanup(); } +#endif /* CONFIG_KEXEC_CORE */ #ifdef CONFIG_CRASH_DUMP static void hv_machine_crash_shutdown(struct pt_regs *regs) @@ -222,8 +223,7 @@ static void hv_machine_crash_shutdown(struct pt_regs *regs) /* Disable the hypercall page when there is only 1 active CPU. */ hyperv_cleanup(); } -#endif -#endif /* CONFIG_KEXEC_CORE */ +#endif /* CONFIG_CRASH_DUMP */ #endif /* CONFIG_HYPERV */ static uint32_t __init ms_hyperv_platform(void) @@ -497,9 +497,11 @@ static void __init ms_hyperv_init_platform(void) no_timer_check = 1; #endif -#if IS_ENABLED(CONFIG_HYPERV) && defined(CONFIG_KEXEC_CORE) +#if IS_ENABLED(CONFIG_HYPERV) +#if defined(CONFIG_KEXEC_CORE) machine_ops.shutdown = hv_machine_shutdown; -#ifdef CONFIG_CRASH_DUMP +#endif +#if defined(CONFIG_CRASH_DUMP) machine_ops.crash_shutdown = hv_machine_crash_shutdown; #endif #endif diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index 1287b0d5962f..f3130f762784 100644 --- a/arch/x86/kernel/reboot.c +++ b/arch/x86/kernel/reboot.c @@ -826,7 +826,7 @@ void machine_halt(void) machine_ops.halt(); } -#ifdef CONFIG_KEXEC_CORE +#ifdef CONFIG_CRASH_DUMP void machine_crash_shutdown(struct pt_regs *regs) { machine_ops.crash_shutdown(regs); diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c index 09e3db7ff990..0b367c1e086d 100644 --- a/arch/x86/xen/enlighten_hvm.c +++ b/arch/x86/xen/enlighten_hvm.c @@ -148,6 +148,7 @@ static void xen_hvm_shutdown(void) if (kexec_in_progress) xen_reboot(SHUTDOWN_soft_reset); } +#endif #ifdef CONFIG_CRASH_DUMP static void xen_hvm_crash_shutdown(struct pt_regs *regs) @@ -156,7 +157,6 @@ static void xen_hvm_crash_shutdown(struct pt_regs *regs) xen_reboot(SHUTDOWN_soft_reset); } #endif -#endif static int xen_cpu_up_prepare_hvm(unsigned int cpu) { @@ -238,10 +238,10 @@ static void __init xen_hvm_guest_init(void) #ifdef CONFIG_KEXEC_CORE machine_ops.shutdown = xen_hvm_shutdown; +#endif #ifdef CONFIG_CRASH_DUMP machine_ops.crash_shutdown = xen_hvm_crash_shutdown; #endif -#endif } static __init int xen_parse_nopv(char *arg) diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c index 218773cfb009..e21974f2cf2d 100644 --- a/arch/x86/xen/mmu_pv.c +++ b/arch/x86/xen/mmu_pv.c @@ -2520,7 +2520,7 @@ int xen_remap_pfn(struct vm_area_struct *vma, unsigned long addr, } EXPORT_SYMBOL_GPL(xen_remap_pfn); -#ifdef CONFIG_KEXEC_CORE +#ifdef CONFIG_VMCORE_INFO phys_addr_t paddr_vmcoreinfo_note(void) { if (xen_pv_domain())
Michael pointed out that the #ifdef CONFIG_CRASH_DUMP is nested inside arch/x86/xen/enlighten_hvm.c. Although the nesting works well too since CONFIG_CRASH_DUMP has dependency on CONFIG_KEXEC_CORE, it may cause confuse because there are places where it's not nested, and people may think it need be nested even though it doesn't have to. Fix that by moving CONFIG_CRASH_DUMP ifdeffery of codes out of CONFIG_KEXEC_CODE ifdeffery scope. And also fix a building error Nathan reported as below by replacing CONFIG_KEXEC_CORE ifdef with CONFIG_VMCORE_INFO ifdef. ==== $ curl -LSso .config https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.x86_64 $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- olddefconfig all ... x86_64-linux-ld: arch/x86/xen/mmu_pv.o: in function `paddr_vmcoreinfo_note': mmu_pv.c:(.text+0x3af3): undefined reference to `vmcoreinfo_note' ==== Link: https://lore.kernel.org/all/SN6PR02MB4157931105FA68D72E3D3DB8D47B2@SN6PR02MB4157.namprd02.prod.outlook.com/T/#u Link: https://lore.kernel.org/all/20240126045551.GA126645@dev-arch.thelio-3990X/T/#u Signed-off-by: Baoquan He <bhe@redhat.com> --- arch/x86/kernel/cpu/mshyperv.c | 10 ++++++---- arch/x86/kernel/reboot.c | 2 +- arch/x86/xen/enlighten_hvm.c | 4 ++-- arch/x86/xen/mmu_pv.c | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-)