2023-11-23 07:37:20

by Baoquan He

[permalink] [raw]
Subject: [PATCH 0/3] kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP

Ignat reported a potential config regression was introduced by
commit 89cde455915f ("kexec: consolidate kexec and crash options
into kernel/Kconfig.kexec"). Please click below link for more details:

https://lore.kernel.org/all/CALrw=nHpRQQaQTP_jZfREgrQEMpS8jBF8JQCv4ygqXycE-StaA@mail.gmail.com/T/#u

The patch 1 fix the regression by removing incorrect CONFIG_KEXEC
ifdeffery scope adding in arm's <asm/kexec.h>, then dropping the
select of KEXEC for CRASH_DUMP. This is tested and passed a cross
comiping of arm.

Patch 2 is to fix a build failure when I tested patch 1 on x86_64, the
wrong CONFIG_KEXEC iddeffery is replaced with CONFIG_KEXEC_CORE. Test
passed on x86_64.

Patch 3 is to fix an unnecessary 'select KEXEC' in s390 ARCH. Removing
the select won't impact anything. Test passed on a ibm-z system.

Baoquan He (3):
kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP
drivers/base/cpu: crash data showing should depends on KEXEC_CORE
s390/Kconfig: drop select of KEXEC

arch/arm/include/asm/kexec.h | 4 ----
arch/s390/Kconfig | 1 -
drivers/base/cpu.c | 6 +++---
kernel/Kconfig.kexec | 1 -
4 files changed, 3 insertions(+), 9 deletions(-)

--
2.41.0


2023-11-23 07:37:23

by Baoquan He

[permalink] [raw]
Subject: [PATCH 1/3] kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP

Ignat Korchagin complained that a potential config regression was
introduced by commit 89cde455915f ("kexec: consolidate kexec and
crash options into kernel/Kconfig.kexec"). Before the commit,
CONFIG_CRASH_DUMP has no dependency on CONFIG_KEXEC. After the commit,
CRASH_DUMP selects KEXEC. That enforces system to have CONFIG_KEXEC=y
as long as CONFIG_CRASH_DUMP=Y which people may not want.

In Ignat's case, he sets CONFIG_CRASH_DUMP=y, CONFIG_KEXEC_FILE=y and
CONFIG_KEXEC=n because kexec_load interface could have security issue if
kernel/initrd has no chance to be signed and verified.

CRASH_DUMP has select of KEXEC because Eric, author of above commit,
met a LKP report of build failure when posting patch of earlier version.
Please see below link to get detail of the LKP report:

https://lore.kernel.org/all/[email protected]/T/#u

In fact, that LKP report is triggered because arm's <asm/kexec.h> is
wrapped in CONFIG_KEXEC ifdeffery scope. That is wrong. CONFIG_KEXEC
controls the enabling/disabling of kexec_load interface, but not kexec
feature. Removing the wrongly added CONFIG_KEXEC ifdeffery scope in
<asm/kexec.h> of arm allows us to drop the select KEXEC for CRASH_DUMP.

Signed-off-by: Baoquan He <[email protected]>
---
arch/arm/include/asm/kexec.h | 4 ----
kernel/Kconfig.kexec | 1 -
2 files changed, 5 deletions(-)

diff --git a/arch/arm/include/asm/kexec.h b/arch/arm/include/asm/kexec.h
index e62832dcba76..a8287e7ab9d4 100644
--- a/arch/arm/include/asm/kexec.h
+++ b/arch/arm/include/asm/kexec.h
@@ -2,8 +2,6 @@
#ifndef _ARM_KEXEC_H
#define _ARM_KEXEC_H

-#ifdef CONFIG_KEXEC
-
/* Maximum physical address we can use pages from */
#define KEXEC_SOURCE_MEMORY_LIMIT (-1UL)
/* Maximum address we can reach in physical address mode */
@@ -82,6 +80,4 @@ static inline struct page *boot_pfn_to_page(unsigned long boot_pfn)

#endif /* __ASSEMBLY__ */

-#endif /* CONFIG_KEXEC */
-
#endif /* _ARM_KEXEC_H */
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..1cc3b1c595d7 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -97,7 +97,6 @@ config CRASH_DUMP
depends on ARCH_SUPPORTS_KEXEC
select CRASH_CORE
select KEXEC_CORE
- select KEXEC
help
Generate crash dump after being started by kexec.
This should be normally only set in special crash dump kernels
--
2.41.0

2023-11-23 07:37:33

by Baoquan He

[permalink] [raw]
Subject: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

When below kernel config items are set, compiling error are triggered.

CONFIG_CRASH_CORE=y
CONFIG_KEXEC_CORE=y
CONFIG_CRASH_DUMP=y
CONFIG_CRASH_HOTPLUG=y

------------------------------------------------------
drivers/base/cpu.c: In function ‘crash_hotplug_show’:
drivers/base/cpu.c:309:40: error: implicit declaration of function ‘crash_hotplug_cpu_support’; did you mean ‘crash_hotplug_show’? [-Werror=implicit-function-declaration]
309 | return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| crash_hotplug_show
cc1: some warnings being treated as errors
------------------------------------------------------

CONFIG_KEXEC is used to enable kexec_load interface, the
crash_notes/crash_notes_size/crash_hotplug showing depends on
CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.

Fix it now.

Signed-off-by: Baoquan He <[email protected]>
---
drivers/base/cpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 9ea22e165acd..548491de818e 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -144,7 +144,7 @@ static DEVICE_ATTR(release, S_IWUSR, NULL, cpu_release_store);
#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
#endif /* CONFIG_HOTPLUG_CPU */

-#ifdef CONFIG_KEXEC
+#ifdef CONFIG_KEXEC_CORE
#include <linux/kexec.h>

static ssize_t crash_notes_show(struct device *dev,
@@ -189,14 +189,14 @@ static const struct attribute_group crash_note_cpu_attr_group = {
#endif

static const struct attribute_group *common_cpu_attr_groups[] = {
-#ifdef CONFIG_KEXEC
+#ifdef CONFIG_KEXEC_CORE
&crash_note_cpu_attr_group,
#endif
NULL
};

static const struct attribute_group *hotplugable_cpu_attr_groups[] = {
-#ifdef CONFIG_KEXEC
+#ifdef CONFIG_KEXEC_CORE
&crash_note_cpu_attr_group,
#endif
NULL
--
2.41.0

2023-11-23 07:37:34

by Baoquan He

[permalink] [raw]
Subject: [PATCH 3/3] s390/Kconfig: drop select of KEXEC

No proof is found to require that S390 architecture has to select
KEXEC. At least from my testing at below, dropping select of KEXEC won't
impact anything.

===testing 1===
CONFIG_CRASH_CORE=y
CONFIG_KEXEC_CORE=y
CONFIG_CRASH_DUMP=y
===

===testing 2===
CONFIG_CRASH_CORE=y
CONFIG_KEXEC_CORE=y
CONFIG_KEXEC_FILE=y
CONFIG_CRASH_DUMP=y
===

So drop the select of KEXEC now.

Signed-off-by: Baoquan He <[email protected]>
---
arch/s390/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..1aec2e692dca 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -217,7 +217,6 @@ config S390
select HAVE_VIRT_CPU_ACCOUNTING_IDLE
select IOMMU_HELPER if PCI
select IOMMU_SUPPORT if PCI
- select KEXEC
select MMU_GATHER_MERGE_VMAS
select MMU_GATHER_NO_GATHER
select MMU_GATHER_RCU_TABLE_FREE
--
2.41.0

2023-11-23 08:24:03

by Ignat Korchagin

[permalink] [raw]
Subject: Re: [PATCH 1/3] kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP

On Thu, Nov 23, 2023 at 7:37 AM Baoquan He <[email protected]> wrote:
>
> Ignat Korchagin complained that a potential config regression was
> introduced by commit 89cde455915f ("kexec: consolidate kexec and
> crash options into kernel/Kconfig.kexec"). Before the commit,
> CONFIG_CRASH_DUMP has no dependency on CONFIG_KEXEC. After the commit,
> CRASH_DUMP selects KEXEC. That enforces system to have CONFIG_KEXEC=y
> as long as CONFIG_CRASH_DUMP=Y which people may not want.
>
> In Ignat's case, he sets CONFIG_CRASH_DUMP=y, CONFIG_KEXEC_FILE=y and
> CONFIG_KEXEC=n because kexec_load interface could have security issue if
> kernel/initrd has no chance to be signed and verified.
>
> CRASH_DUMP has select of KEXEC because Eric, author of above commit,
> met a LKP report of build failure when posting patch of earlier version.
> Please see below link to get detail of the LKP report:
>
> https://lore.kernel.org/all/[email protected]/T/#u
>
> In fact, that LKP report is triggered because arm's <asm/kexec.h> is
> wrapped in CONFIG_KEXEC ifdeffery scope. That is wrong. CONFIG_KEXEC
> controls the enabling/disabling of kexec_load interface, but not kexec
> feature. Removing the wrongly added CONFIG_KEXEC ifdeffery scope in
> <asm/kexec.h> of arm allows us to drop the select KEXEC for CRASH_DUMP.

Hm... With the patch, when cross compiling for arm and
CONFIG_KEXEC_CORE=y
# CONFIG_KEXEC is not set
CONFIG_CRASH_DUMP=y

I get the following linker error at the end:

CALL scripts/checksyscalls.sh
UPD include/generated/utsversion.h
CC init/version-timestamp.o
LD .tmp_vmlinux.kallsyms1
arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `kimage_free':
kexec_core.c:(.text+0xf5c): undefined reference to `machine_kexec_cleanup'
arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `__crash_kexec':
kexec_core.c:(.text+0x15bc): undefined reference to `machine_crash_shutdown'
arm-linux-gnueabi-ld: kexec_core.c:(.text+0x15c4): undefined reference
to `machine_kexec'
arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `kernel_kexec':
kexec_core.c:(.text+0x1a04): undefined reference to `machine_kexec'
make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
make[1]: *** [/home/ignat/git/linux-upstream/Makefile:1154: vmlinux] Error 2
make: *** [Makefile:234: __sub-make] Error 2

> Signed-off-by: Baoquan He <[email protected]>
> ---
> arch/arm/include/asm/kexec.h | 4 ----
> kernel/Kconfig.kexec | 1 -
> 2 files changed, 5 deletions(-)
>
> diff --git a/arch/arm/include/asm/kexec.h b/arch/arm/include/asm/kexec.h
> index e62832dcba76..a8287e7ab9d4 100644
> --- a/arch/arm/include/asm/kexec.h
> +++ b/arch/arm/include/asm/kexec.h
> @@ -2,8 +2,6 @@
> #ifndef _ARM_KEXEC_H
> #define _ARM_KEXEC_H
>
> -#ifdef CONFIG_KEXEC
> -
> /* Maximum physical address we can use pages from */
> #define KEXEC_SOURCE_MEMORY_LIMIT (-1UL)
> /* Maximum address we can reach in physical address mode */
> @@ -82,6 +80,4 @@ static inline struct page *boot_pfn_to_page(unsigned long boot_pfn)
>
> #endif /* __ASSEMBLY__ */
>
> -#endif /* CONFIG_KEXEC */
> -
> #endif /* _ARM_KEXEC_H */
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f4..1cc3b1c595d7 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -97,7 +97,6 @@ config CRASH_DUMP
> depends on ARCH_SUPPORTS_KEXEC
> select CRASH_CORE
> select KEXEC_CORE
> - select KEXEC
> help
> Generate crash dump after being started by kexec.
> This should be normally only set in special crash dump kernels
> --
> 2.41.0
>

2023-11-23 08:34:40

by Ignat Korchagin

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On Thu, Nov 23, 2023 at 7:37 AM Baoquan He <[email protected]> wrote:
>
> When below kernel config items are set, compiling error are triggered.
>
> CONFIG_CRASH_CORE=y
> CONFIG_KEXEC_CORE=y
> CONFIG_CRASH_DUMP=y
> CONFIG_CRASH_HOTPLUG=y
>
> ------------------------------------------------------
> drivers/base/cpu.c: In function ‘crash_hotplug_show’:
> drivers/base/cpu.c:309:40: error: implicit declaration of function ‘crash_hotplug_cpu_support’; did you mean ‘crash_hotplug_show’? [-Werror=implicit-function-declaration]
> 309 | return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> | crash_hotplug_show
> cc1: some warnings being treated as errors
> ------------------------------------------------------
>
> CONFIG_KEXEC is used to enable kexec_load interface, the
> crash_notes/crash_notes_size/crash_hotplug showing depends on
> CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
>
> Fix it now.

Can we add Fixes/CC stable, so it gets eventually backported into 6.6?

> Signed-off-by: Baoquan He <[email protected]>
> ---
> drivers/base/cpu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 9ea22e165acd..548491de818e 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -144,7 +144,7 @@ static DEVICE_ATTR(release, S_IWUSR, NULL, cpu_release_store);
> #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
> #endif /* CONFIG_HOTPLUG_CPU */
>
> -#ifdef CONFIG_KEXEC
> +#ifdef CONFIG_KEXEC_CORE
> #include <linux/kexec.h>
>
> static ssize_t crash_notes_show(struct device *dev,
> @@ -189,14 +189,14 @@ static const struct attribute_group crash_note_cpu_attr_group = {
> #endif
>
> static const struct attribute_group *common_cpu_attr_groups[] = {
> -#ifdef CONFIG_KEXEC
> +#ifdef CONFIG_KEXEC_CORE
> &crash_note_cpu_attr_group,
> #endif
> NULL
> };
>
> static const struct attribute_group *hotplugable_cpu_attr_groups[] = {
> -#ifdef CONFIG_KEXEC
> +#ifdef CONFIG_KEXEC_CORE
> &crash_note_cpu_attr_group,
> #endif
> NULL
> --
> 2.41.0
>

2023-11-23 11:08:18

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 1/3] kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP

On 11/23/23 at 08:23am, Ignat Korchagin wrote:
> On Thu, Nov 23, 2023 at 7:37 AM Baoquan He <[email protected]> wrote:
> >
> > Ignat Korchagin complained that a potential config regression was
> > introduced by commit 89cde455915f ("kexec: consolidate kexec and
> > crash options into kernel/Kconfig.kexec"). Before the commit,
> > CONFIG_CRASH_DUMP has no dependency on CONFIG_KEXEC. After the commit,
> > CRASH_DUMP selects KEXEC. That enforces system to have CONFIG_KEXEC=y
> > as long as CONFIG_CRASH_DUMP=Y which people may not want.
> >
> > In Ignat's case, he sets CONFIG_CRASH_DUMP=y, CONFIG_KEXEC_FILE=y and
> > CONFIG_KEXEC=n because kexec_load interface could have security issue if
> > kernel/initrd has no chance to be signed and verified.
> >
> > CRASH_DUMP has select of KEXEC because Eric, author of above commit,
> > met a LKP report of build failure when posting patch of earlier version.
> > Please see below link to get detail of the LKP report:
> >
> > https://lore.kernel.org/all/[email protected]/T/#u
> >
> > In fact, that LKP report is triggered because arm's <asm/kexec.h> is
> > wrapped in CONFIG_KEXEC ifdeffery scope. That is wrong. CONFIG_KEXEC
> > controls the enabling/disabling of kexec_load interface, but not kexec
> > feature. Removing the wrongly added CONFIG_KEXEC ifdeffery scope in
> > <asm/kexec.h> of arm allows us to drop the select KEXEC for CRASH_DUMP.
>
> Hm... With the patch, when cross compiling for arm and
> CONFIG_KEXEC_CORE=y
> # CONFIG_KEXEC is not set
> CONFIG_CRASH_DUMP=y
>
> I get the following linker error at the end:
>
> CALL scripts/checksyscalls.sh
> UPD include/generated/utsversion.h
> CC init/version-timestamp.o
> LD .tmp_vmlinux.kallsyms1
> arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `kimage_free':
> kexec_core.c:(.text+0xf5c): undefined reference to `machine_kexec_cleanup'
> arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `__crash_kexec':
> kexec_core.c:(.text+0x15bc): undefined reference to `machine_crash_shutdown'
> arm-linux-gnueabi-ld: kexec_core.c:(.text+0x15c4): undefined reference
> to `machine_kexec'
> arm-linux-gnueabi-ld: kernel/kexec_core.o: in function `kernel_kexec':
> kexec_core.c:(.text+0x1a04): undefined reference to `machine_kexec'
> make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
> make[1]: *** [/home/ignat/git/linux-upstream/Makefile:1154: vmlinux] Error 2
> make: *** [Makefile:234: __sub-make] Error 2

Oops, I forgot this part. This should fix the link error.

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index d53f56d6f840..771264d4726a 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -59,7 +59,7 @@ obj-$(CONFIG_FUNCTION_TRACER) += entry-ftrace.o
obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o patch.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o patch.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o
-obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o
+obj-$(CONFIG_KEXEC_CORE) += machine_kexec.o relocate_kernel.o
# Main staffs in KPROBES are in arch/arm/probes/ .
obj-$(CONFIG_KPROBES) += patch.o insn.o
obj-$(CONFIG_OABI_COMPAT) += sys_oabi-compat.o

2023-11-23 11:16:08

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On 11/23/23 at 08:34am, Ignat Korchagin wrote:
> On Thu, Nov 23, 2023 at 7:37 AM Baoquan He <[email protected]> wrote:
> >
> > When below kernel config items are set, compiling error are triggered.
> >
> > CONFIG_CRASH_CORE=y
> > CONFIG_KEXEC_CORE=y
> > CONFIG_CRASH_DUMP=y
> > CONFIG_CRASH_HOTPLUG=y
> >
> > ------------------------------------------------------
> > drivers/base/cpu.c: In function ‘crash_hotplug_show’:
> > drivers/base/cpu.c:309:40: error: implicit declaration of function ‘crash_hotplug_cpu_support’; did you mean ‘crash_hotplug_show’? [-Werror=implicit-function-declaration]
> > 309 | return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > | crash_hotplug_show
> > cc1: some warnings being treated as errors
> > ------------------------------------------------------
> >
> > CONFIG_KEXEC is used to enable kexec_load interface, the
> > crash_notes/crash_notes_size/crash_hotplug showing depends on
> > CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
> >
> > Fix it now.
>
> Can we add Fixes/CC stable, so it gets eventually backported into 6.6?

Makes sense. Will add it in v2 since I need respin to add the missing
stuff in patch 1. Thanks.

>
> > Signed-off-by: Baoquan He <[email protected]>
> > ---
> > drivers/base/cpu.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> > index 9ea22e165acd..548491de818e 100644
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -144,7 +144,7 @@ static DEVICE_ATTR(release, S_IWUSR, NULL, cpu_release_store);
> > #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
> > #endif /* CONFIG_HOTPLUG_CPU */
> >
> > -#ifdef CONFIG_KEXEC
> > +#ifdef CONFIG_KEXEC_CORE
> > #include <linux/kexec.h>
> >
> > static ssize_t crash_notes_show(struct device *dev,
> > @@ -189,14 +189,14 @@ static const struct attribute_group crash_note_cpu_attr_group = {
> > #endif
> >
> > static const struct attribute_group *common_cpu_attr_groups[] = {
> > -#ifdef CONFIG_KEXEC
> > +#ifdef CONFIG_KEXEC_CORE
> > &crash_note_cpu_attr_group,
> > #endif
> > NULL
> > };
> >
> > static const struct attribute_group *hotplugable_cpu_attr_groups[] = {
> > -#ifdef CONFIG_KEXEC
> > +#ifdef CONFIG_KEXEC_CORE
> > &crash_note_cpu_attr_group,
> > #endif
> > NULL
> > --
> > 2.41.0
> >
>

2023-11-23 13:43:59

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH 3/3] s390/Kconfig: drop select of KEXEC

On Thu, Nov 23, 2023 at 03:36:52PM +0800, Baoquan He wrote:

Hi Baoquan,

> No proof is found to require that S390 architecture has to select
> KEXEC. At least from my testing at below, dropping select of KEXEC won't
> impact anything.

It does impact the outcome of defconfigs.
Namely, CONFIG_KEXEC is not set with this patch.

> ===testing 1===
> CONFIG_CRASH_CORE=y
> CONFIG_KEXEC_CORE=y
> CONFIG_CRASH_DUMP=y
> ===
>
> ===testing 2===
> CONFIG_CRASH_CORE=y
> CONFIG_KEXEC_CORE=y
> CONFIG_KEXEC_FILE=y
> CONFIG_CRASH_DUMP=y
> ===

Unfortunately, I do not quite realize what these testings were
and what is the difference between the two.

> So drop the select of KEXEC now.

I suggest dropping this patch. Once the previous two are upstream
we would remove 'select KEXEC' from Kconfig together with defconfig
updates.

> Signed-off-by: Baoquan He <[email protected]>
> ---
> arch/s390/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 3bec98d20283..1aec2e692dca 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -217,7 +217,6 @@ config S390
> select HAVE_VIRT_CPU_ACCOUNTING_IDLE
> select IOMMU_HELPER if PCI
> select IOMMU_SUPPORT if PCI
> - select KEXEC
> select MMU_GATHER_MERGE_VMAS
> select MMU_GATHER_NO_GATHER
> select MMU_GATHER_RCU_TABLE_FREE
> --

Thanks!

2023-11-23 14:13:58

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On Thu, Nov 23, 2023 at 03:36:51PM +0800, Baoquan He wrote:
> When below kernel config items are set, compiling error are triggered.
>
> CONFIG_CRASH_CORE=y
> CONFIG_KEXEC_CORE=y
> CONFIG_CRASH_DUMP=y
> CONFIG_CRASH_HOTPLUG=y
>
> ------------------------------------------------------
> drivers/base/cpu.c: In function ‘crash_hotplug_show’:
> drivers/base/cpu.c:309:40: error: implicit declaration of function ‘crash_hotplug_cpu_support’; did you mean ‘crash_hotplug_show’? [-Werror=implicit-function-declaration]
> 309 | return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> | crash_hotplug_show
> cc1: some warnings being treated as errors
> ------------------------------------------------------
>
> CONFIG_KEXEC is used to enable kexec_load interface, the
> crash_notes/crash_notes_size/crash_hotplug showing depends on
> CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
>
> Fix it now.

If this error introduced with the prevous patch?
If so, the patches need to be swapped I guess.

> Signed-off-by: Baoquan He <[email protected]>

2023-11-23 14:27:55

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On 11/23/23 at 03:13pm, Alexander Gordeev wrote:
> On Thu, Nov 23, 2023 at 03:36:51PM +0800, Baoquan He wrote:
> > When below kernel config items are set, compiling error are triggered.
> >
> > CONFIG_CRASH_CORE=y
> > CONFIG_KEXEC_CORE=y
> > CONFIG_CRASH_DUMP=y
> > CONFIG_CRASH_HOTPLUG=y
> >
> > ------------------------------------------------------
> > drivers/base/cpu.c: In function ‘crash_hotplug_show’:
> > drivers/base/cpu.c:309:40: error: implicit declaration of function ‘crash_hotplug_cpu_support’; did you mean ‘crash_hotplug_show’? [-Werror=implicit-function-declaration]
> > 309 | return sysfs_emit(buf, "%d\n", crash_hotplug_cpu_support());
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > | crash_hotplug_show
> > cc1: some warnings being treated as errors
> > ------------------------------------------------------
> >
> > CONFIG_KEXEC is used to enable kexec_load interface, the
> > crash_notes/crash_notes_size/crash_hotplug showing depends on
> > CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
> >
> > Fix it now.
>
> If this error introduced with the prevous patch?
> If so, the patches need to be swapped I guess.

From the phenomenon, yes. In fact it's the patch 1 which exposes
the wrong ifdeffery. I can shift the order in v2. Thanks.

>
> > Signed-off-by: Baoquan He <[email protected]>
>

2023-11-23 14:32:27

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 3/3] s390/Kconfig: drop select of KEXEC

On 11/23/23 at 02:43pm, Alexander Gordeev wrote:
> On Thu, Nov 23, 2023 at 03:36:52PM +0800, Baoquan He wrote:
>
> Hi Baoquan,
>
> > No proof is found to require that S390 architecture has to select
> > KEXEC. At least from my testing at below, dropping select of KEXEC won't
> > impact anything.
>
> It does impact the outcome of defconfigs.
> Namely, CONFIG_KEXEC is not set with this patch.

Right, CONFIG_KEXEC won't be set defaultly with this patch applied.

>
> > ===testing 1===
> > CONFIG_CRASH_CORE=y
> > CONFIG_KEXEC_CORE=y
> > CONFIG_CRASH_DUMP=y
> > ===
> >
> > ===testing 2===
> > CONFIG_CRASH_CORE=y
> > CONFIG_KEXEC_CORE=y
> > CONFIG_KEXEC_FILE=y
> > CONFIG_CRASH_DUMP=y
> > ===
>
> Unfortunately, I do not quite realize what these testings were
> and what is the difference between the two.

Both these two testings have CONFIG_KEXEC=n, and building all passed.
I wound't present their difference, but two cases where no CONFIG_KEXEC
is set and no dependency on CONFIG_KEXEC is seen.

>
> > So drop the select of KEXEC now.
>
> I suggest dropping this patch. Once the previous two are upstream
> we would remove 'select KEXEC' from Kconfig together with defconfig
> updates.

I see your concern, will drop this one in v2. Thanks for checking these.

>
> > Signed-off-by: Baoquan He <[email protected]>
> > ---
> > arch/s390/Kconfig | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 3bec98d20283..1aec2e692dca 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -217,7 +217,6 @@ config S390
> > select HAVE_VIRT_CPU_ACCOUNTING_IDLE
> > select IOMMU_HELPER if PCI
> > select IOMMU_SUPPORT if PCI
> > - select KEXEC
> > select MMU_GATHER_MERGE_VMAS
> > select MMU_GATHER_NO_GATHER
> > select MMU_GATHER_RCU_TABLE_FREE
> > --
>
> Thanks!
>

2023-11-24 16:44:53

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On Thu, 23 Nov 2023 19:15:43 +0800 Baoquan He <[email protected]> wrote:

> > > CONFIG_KEXEC is used to enable kexec_load interface, the
> > > crash_notes/crash_notes_size/crash_hotplug showing depends on
> > > CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
> > >
> > > Fix it now.
> >
> > Can we add Fixes/CC stable, so it gets eventually backported into 6.6?
>
> Makes sense. Will add it in v2 since I need respin to add the missing
> stuff in patch 1. Thanks.

Please avoid mixing cc:stable patches and this-merge-window fixes in
the same series as next-merge-window material. Because I'll just have
to separate them out anyway, causing what-I-merged to unnecessarily
differ from what-you-sent.


2023-11-25 02:30:48

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 2/3] drivers/base/cpu: crash data showing should depends on KEXEC_CORE

On 11/24/23 at 08:44am, Andrew Morton wrote:
> On Thu, 23 Nov 2023 19:15:43 +0800 Baoquan He <[email protected]> wrote:
>
> > > > CONFIG_KEXEC is used to enable kexec_load interface, the
> > > > crash_notes/crash_notes_size/crash_hotplug showing depends on
> > > > CONFIG_KEXEC is incorrect. It should depend on KEXEC_CORE instead.
> > > >
> > > > Fix it now.
> > >
> > > Can we add Fixes/CC stable, so it gets eventually backported into 6.6?
> >
> > Makes sense. Will add it in v2 since I need respin to add the missing
> > stuff in patch 1. Thanks.
>
> Please avoid mixing cc:stable patches and this-merge-window fixes in
> the same series as next-merge-window material. Because I'll just have
> to separate them out anyway, causing what-I-merged to unnecessarily
> differ from what-you-sent.

Got it, will send them separately because the issue that this patch is fixing
has been there for very long time. Thanks for noticing.

2023-11-27 03:29:34

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 0/3] kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP

On 11/25/23 at 06:07pm, Eric DeVolder wrote:
>
> On 11/23/23 01:36, Baoquan He wrote:
> > Ignat reported a potential config regression was introduced by
> > commit 89cde455915f ("kexec: consolidate kexec and crash options
> > into kernel/Kconfig.kexec"). Please click below link for more details:
> >
> > https://lore.kernel.org/all/CALrw=nHpRQQaQTP_jZfREgrQEMpS8jBF8JQCv4ygqXycE-StaA@mail.gmail.com/T/#u
> >
> > The patch 1 fix the regression by removing incorrect CONFIG_KEXEC
> > ifdeffery scope adding in arm's <asm/kexec.h>, then dropping the
> > select of KEXEC for CRASH_DUMP. This is tested and passed a cross
> > comiping of arm.
> >
> > Patch 2 is to fix a build failure when I tested patch 1 on x86_64, the
> > wrong CONFIG_KEXEC iddeffery is replaced with CONFIG_KEXEC_CORE. Test
> > passed on x86_64.
> >
> > Patch 3 is to fix an unnecessary 'select KEXEC' in s390 ARCH. Removing
> > the select won't impact anything. Test passed on a ibm-z system.
>
> I apologize for my delay in responding, I did not have a computer with me
> during my holiday travel.
>
> I was able to re-run my Kconfig test script with this patch series (now that
> I'm running this on private resources, it takes half a day 8( ). The script
> only performs comparisons of the .config before (LHSB) and after (RHSB) the
> patch series; it does NOT do any building. At any rate, what that revealed
> was only differences in s390. That means that all other arches do not have
> any unintended side effects. The differences with patch3 applied look like:
>
> FAIL: allnoconfig arch/s390/configs/kasan.config
> LHSB {'CONFIG_CRASH_CORE': 'y', 'CONFIG_KEXEC_CORE': 'y', 'CONFIG_KEXEC':
> 'y'}
> RHSB {'CONFIG_KEXEC': 'n'}
>
> The 'allnoconfig' and 'olddefconfig' targets failed for all s390 defconfigs.
> The LHSB is the pre-patch values, and the RHSB is the post-patch values. So
> this states that CRASH_CORE and KEXEC_CORE were set previously, but now they
> are not. KEXEC obviously is being turned off intentionally.
>
> Hope this helps some.

Thanks, Eric. Alexander has pointed out this in patch 3 reviewing
comment. I misunderstood the select KEXEC in s390 arch where they
intend to do. As I replied to Alexander, I will drop patch 3.

>
>
> > Baoquan He (3):
> > kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP
> > drivers/base/cpu: crash data showing should depends on KEXEC_CORE
> > s390/Kconfig: drop select of KEXEC
> >
> > arch/arm/include/asm/kexec.h | 4 ----
> > arch/s390/Kconfig | 1 -
> > drivers/base/cpu.c | 6 +++---
> > kernel/Kconfig.kexec | 1 -
> > 4 files changed, 3 insertions(+), 9 deletions(-)
> >
>