[PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
causes a link failure when all the crypto support is in a loadable module
and kexec_file support is built-in:

x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
(.text+0x32e30a): undefined reference to `crypto_alloc_shash'
x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'

Both s390 and x86 have this problem, while ppc64 and riscv have the
correct dependency already. On riscv, the dependency is only used
for the purgatory, not for the kexec_file code itself, which may
be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
possible to enable KEXEC_FILE but then the purgatory code is silently
left out.

Move this into the common Kconfig.kexec file in a way that is
correct everywhere, using the dependency on CRYPTO_SHA256=y only
when the purgatory code is available. This requires reversing the
dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
but the effect remains the same, other than making riscv behave
like the other ones.

On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
should technically not be required but gives better performance. Remove
this dependency here, noting that it was not present in the initial
Kconfig code but was brought in without an explanation in commit
71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").

Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/Kconfig | 4 ++--
 arch/riscv/Kconfig   | 4 +---
 arch/s390/Kconfig    | 4 ++--
 arch/x86/Kconfig     | 4 ++--
 kernel/Kconfig.kexec | 1 +
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d5d5388973ac7..4640cee33f123 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+	def_bool PPC64
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 25474f8c12b79..f571bad2d22d0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
-	depends on CRYPTO=y
-	depends on CRYPTO_SHA256=y
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b0d67ac8695f9..ec77106af4137 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool MODULE_SIG_FORMAT
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 94efde80ebf35..f9975b15ccd57 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+	def_bool X86_64
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
@@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select HAVE_IMA_KEXEC if IMA
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f48..bfc636d64ff2b 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
+	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2

[PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
causes a link failure when all the crypto support is in a loadable module
and kexec_file support is built-in:

x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
(.text+0x32e30a): undefined reference to `crypto_alloc_shash'
x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'

Both s390 and x86 have this problem, while ppc64 and riscv have the
correct dependency already. On riscv, the dependency is only used
for the purgatory, not for the kexec_file code itself, which may
be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
possible to enable KEXEC_FILE but then the purgatory code is silently
left out.

Move this into the common Kconfig.kexec file in a way that is
correct everywhere, using the dependency on CRYPTO_SHA256=y only
when the purgatory code is available. This requires reversing the
dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
but the effect remains the same, other than making riscv behave
like the other ones.

On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
should technically not be required but gives better performance. Remove
this dependency here, noting that it was not present in the initial
Kconfig code but was brought in without an explanation in commit
71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").

Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/Kconfig | 4 ++--
 arch/riscv/Kconfig   | 4 +---
 arch/s390/Kconfig    | 4 ++--
 arch/x86/Kconfig     | 4 ++--
 kernel/Kconfig.kexec | 1 +
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d5d5388973ac7..4640cee33f123 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+	def_bool PPC64
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 25474f8c12b79..f571bad2d22d0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
-	depends on CRYPTO=y
-	depends on CRYPTO_SHA256=y
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b0d67ac8695f9..ec77106af4137 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool MODULE_SIG_FORMAT
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 94efde80ebf35..f9975b15ccd57 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+	def_bool X86_64
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
@@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select HAVE_IMA_KEXEC if IMA
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f48..bfc636d64ff2b 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
+	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
causes a link failure when all the crypto support is in a loadable module
and kexec_file support is built-in:

x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
(.text+0x32e30a): undefined reference to `crypto_alloc_shash'
x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'

Both s390 and x86 have this problem, while ppc64 and riscv have the
correct dependency already. On riscv, the dependency is only used
for the purgatory, not for the kexec_file code itself, which may
be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
possible to enable KEXEC_FILE but then the purgatory code is silently
left out.

Move this into the common Kconfig.kexec file in a way that is
correct everywhere, using the dependency on CRYPTO_SHA256=y only
when the purgatory code is available. This requires reversing the
dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
but the effect remains the same, other than making riscv behave
like the other ones.

On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
should technically not be required but gives better performance. Remove
this dependency here, noting that it was not present in the initial
Kconfig code but was brought in without an explanation in commit
71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").

Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/Kconfig | 4 ++--
 arch/riscv/Kconfig   | 4 +---
 arch/s390/Kconfig    | 4 ++--
 arch/x86/Kconfig     | 4 ++--
 kernel/Kconfig.kexec | 1 +
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d5d5388973ac7..4640cee33f123 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+	def_bool PPC64
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 25474f8c12b79..f571bad2d22d0 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
-	depends on CRYPTO=y
-	depends on CRYPTO_SHA256=y
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b0d67ac8695f9..ec77106af4137 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool MODULE_SIG_FORMAT
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_CRASH_DUMP
 	def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 94efde80ebf35..f9975b15ccd57 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
 	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_FILE
-	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+	def_bool X86_64
 
 config ARCH_SELECTS_KEXEC_FILE
 	def_bool y
@@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
 	select HAVE_IMA_KEXEC if IMA
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-	def_bool KEXEC_FILE
+	def_bool y
 
 config ARCH_SUPPORTS_KEXEC_SIG
 	def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f48..bfc636d64ff2b 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
+	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2

[PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

All other users of crypto code use 'select' instead of 'depends on',
so do the same thing with KEXEC_FILE for consistency.

In practice this makes very little difference as kernels with kexec
support are very likely to also include some other feature that already
selects both crypto and crypto_sha256, but being consistent here helps
for usability as well as to avoid potential circular dependencies.

This reverts the dependency back to what it was originally before commit
74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
new syscall"), which changed changed it with the comment "This should
be safer as "select" is not recursive", but that appears to have been
done in error, as "select" is indeed recursive, and there are no other
dependencies that prevent CRYPTO_SHA256 from being selected here.

Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/Kconfig.kexec | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index bfc636d64ff2b..51f719af10e79 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,7 +36,8 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
-	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
+	select CRYPTO
+	select CRYPTO_SHA256
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2

[PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

All other users of crypto code use 'select' instead of 'depends on',
so do the same thing with KEXEC_FILE for consistency.

In practice this makes very little difference as kernels with kexec
support are very likely to also include some other feature that already
selects both crypto and crypto_sha256, but being consistent here helps
for usability as well as to avoid potential circular dependencies.

This reverts the dependency back to what it was originally before commit
74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
new syscall"), which changed changed it with the comment "This should
be safer as "select" is not recursive", but that appears to have been
done in error, as "select" is indeed recursive, and there are no other
dependencies that prevent CRYPTO_SHA256 from being selected here.

Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/Kconfig.kexec | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index bfc636d64ff2b..51f719af10e79 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,7 +36,8 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
-	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
+	select CRYPTO
+	select CRYPTO_SHA256
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

All other users of crypto code use 'select' instead of 'depends on',
so do the same thing with KEXEC_FILE for consistency.

In practice this makes very little difference as kernels with kexec
support are very likely to also include some other feature that already
selects both crypto and crypto_sha256, but being consistent here helps
for usability as well as to avoid potential circular dependencies.

This reverts the dependency back to what it was originally before commit
74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
new syscall"), which changed changed it with the comment "This should
be safer as "select" is not recursive", but that appears to have been
done in error, as "select" is indeed recursive, and there are no other
dependencies that prevent CRYPTO_SHA256 from being selected here.

Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/Kconfig.kexec | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index bfc636d64ff2b..51f719af10e79 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,7 +36,8 @@ config KEXEC
 config KEXEC_FILE
 	bool "Enable kexec file based system call"
 	depends on ARCH_SUPPORTS_KEXEC_FILE
-	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
+	select CRYPTO
+	select CRYPTO_SHA256
 	select KEXEC_CORE
 	help
 	  This is new version of kexec system call. This system call is
-- 
2.39.2

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5323 bytes --]

On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
> 'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
> causes a link failure when all the crypto support is in a loadable module
> and kexec_file support is built-in:
> 
> x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
> (.text+0x32e30a): undefined reference to `crypto_alloc_shash'
> x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
> x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'
> 
> Both s390 and x86 have this problem, while ppc64 and riscv have the
> correct dependency already. On riscv, the dependency is only used
> for the purgatory, not for the kexec_file code itself, which may
> be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
> possible to enable KEXEC_FILE but then the purgatory code is silently
> left out.
> 
> Move this into the common Kconfig.kexec file in a way that is
> correct everywhere, using the dependency on CRYPTO_SHA256=y only
> when the purgatory code is available. This requires reversing the
> dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
> but the effect remains the same, other than making riscv behave
> like the other ones.
> 
> On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
> should technically not be required but gives better performance. Remove
> this dependency here, noting that it was not present in the initial
> Kconfig code but was brought in without an explanation in commit
> 71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").
> 
> Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/powerpc/Kconfig | 4 ++--

>  arch/riscv/Kconfig   | 4 +---

This doesn't appear to work for rv32. The defconfig build in our
patchwork automation complains:
  /tmp/tmp.Aq21JVRQTx/arch/riscv/purgatory/entry.S:29:2: error: instruction requires the following: RV64I Base Instruction Set

>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y

This being the problem, KEXEC_FILE is 64-bit only.

IIRC I commented on this same thing during the original conversion
patches.

Cheers,
Conor.

>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 5323 bytes --]

On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
> 'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
> causes a link failure when all the crypto support is in a loadable module
> and kexec_file support is built-in:
> 
> x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
> (.text+0x32e30a): undefined reference to `crypto_alloc_shash'
> x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
> x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'
> 
> Both s390 and x86 have this problem, while ppc64 and riscv have the
> correct dependency already. On riscv, the dependency is only used
> for the purgatory, not for the kexec_file code itself, which may
> be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
> possible to enable KEXEC_FILE but then the purgatory code is silently
> left out.
> 
> Move this into the common Kconfig.kexec file in a way that is
> correct everywhere, using the dependency on CRYPTO_SHA256=y only
> when the purgatory code is available. This requires reversing the
> dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
> but the effect remains the same, other than making riscv behave
> like the other ones.
> 
> On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
> should technically not be required but gives better performance. Remove
> this dependency here, noting that it was not present in the initial
> Kconfig code but was brought in without an explanation in commit
> 71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").
> 
> Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/powerpc/Kconfig | 4 ++--

>  arch/riscv/Kconfig   | 4 +---

This doesn't appear to work for rv32. The defconfig build in our
patchwork automation complains:
  /tmp/tmp.Aq21JVRQTx/arch/riscv/purgatory/entry.S:29:2: error: instruction requires the following: RV64I Base Instruction Set

>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y

This being the problem, KEXEC_FILE is 64-bit only.

IIRC I commented on this same thing during the original conversion
patches.

Cheers,
Conor.

>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5323 bytes --]

On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The cleanup for the CONFIG_KEXEC Kconfig logic accidentally changed the
> 'depends on CRYPTO=y' dependency to a plain 'depends on CRYPTO', which
> causes a link failure when all the crypto support is in a loadable module
> and kexec_file support is built-in:
> 
> x86_64-linux-ld: vmlinux.o: in function `__x64_sys_kexec_file_load':
> (.text+0x32e30a): undefined reference to `crypto_alloc_shash'
> x86_64-linux-ld: (.text+0x32e58e): undefined reference to `crypto_shash_update'
> x86_64-linux-ld: (.text+0x32e6ee): undefined reference to `crypto_shash_final'
> 
> Both s390 and x86 have this problem, while ppc64 and riscv have the
> correct dependency already. On riscv, the dependency is only used
> for the purgatory, not for the kexec_file code itself, which may
> be a bit surprising as it means that with CONFIG_CRYPTO=m, it is
> possible to enable KEXEC_FILE but then the purgatory code is silently
> left out.
> 
> Move this into the common Kconfig.kexec file in a way that is
> correct everywhere, using the dependency on CRYPTO_SHA256=y only
> when the purgatory code is available. This requires reversing the
> dependency between ARCH_SUPPORTS_KEXEC_PURGATORY and KEXEC_FILE,
> but the effect remains the same, other than making riscv behave
> like the other ones.
> 
> On s390, there is an additional dependency on CRYPTO_SHA256_S390, which
> should technically not be required but gives better performance. Remove
> this dependency here, noting that it was not present in the initial
> Kconfig code but was brought in without an explanation in commit
> 71406883fd357 ("s390/kexec_file: Add kexec_file_load system call").
> 
> Fixes: 6af5138083005 ("x86/kexec: refactor for kernel/Kconfig.kexec")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/powerpc/Kconfig | 4 ++--

>  arch/riscv/Kconfig   | 4 +---

This doesn't appear to work for rv32. The defconfig build in our
patchwork automation complains:
  /tmp/tmp.Aq21JVRQTx/arch/riscv/purgatory/entry.S:29:2: error: instruction requires the following: RV64I Base Instruction Set

>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y

This being the problem, KEXEC_FILE is 64-bit only.

IIRC I commented on this same thing during the original conversion
patches.

Cheers,
Conor.

>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:

>> index 25474f8c12b79..f571bad2d22d0 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>>  	select KEXEC_ELF
>>  
>>  config ARCH_SUPPORTS_KEXEC_PURGATORY
>> -	def_bool KEXEC_FILE
>> -	depends on CRYPTO=y
>> -	depends on CRYPTO_SHA256=y
>> +	def_bool y
>
> This being the problem, KEXEC_FILE is 64-bit only.
>
> IIRC I commented on this same thing during the original conversion
> patches.

Does it work with this patch on top?

--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
        select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-       def_bool y
+       def_bool ARCH_SUPPORTS_KEXEC_FILE
 
 config ARCH_SUPPORTS_CRASH_DUMP
        def_bool y


     Arnd

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:

>> index 25474f8c12b79..f571bad2d22d0 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>>  	select KEXEC_ELF
>>  
>>  config ARCH_SUPPORTS_KEXEC_PURGATORY
>> -	def_bool KEXEC_FILE
>> -	depends on CRYPTO=y
>> -	depends on CRYPTO_SHA256=y
>> +	def_bool y
>
> This being the problem, KEXEC_FILE is 64-bit only.
>
> IIRC I commented on this same thing during the original conversion
> patches.

Does it work with this patch on top?

--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
        select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-       def_bool y
+       def_bool ARCH_SUPPORTS_KEXEC_FILE
 
 config ARCH_SUPPORTS_CRASH_DUMP
        def_bool y


     Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:

>> index 25474f8c12b79..f571bad2d22d0 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>>  	select KEXEC_ELF
>>  
>>  config ARCH_SUPPORTS_KEXEC_PURGATORY
>> -	def_bool KEXEC_FILE
>> -	depends on CRYPTO=y
>> -	depends on CRYPTO_SHA256=y
>> +	def_bool y
>
> This being the problem, KEXEC_FILE is 64-bit only.
>
> IIRC I commented on this same thing during the original conversion
> patches.

Does it work with this patch on top?

--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
        select KEXEC_ELF
 
 config ARCH_SUPPORTS_KEXEC_PURGATORY
-       def_bool y
+       def_bool ARCH_SUPPORTS_KEXEC_FILE
 
 config ARCH_SUPPORTS_CRASH_DUMP
        def_bool y


     Arnd

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

On Mon, Oct 23, 2023 at 06:04:06PM +0200, Arnd Bergmann wrote:
> On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> > On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> 
> >> index 25474f8c12b79..f571bad2d22d0 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
> >>  	select KEXEC_ELF
> >>  
> >>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> >> -	def_bool KEXEC_FILE
> >> -	depends on CRYPTO=y
> >> -	depends on CRYPTO_SHA256=y
> >> +	def_bool y
> >
> > This being the problem, KEXEC_FILE is 64-bit only.
> >
> > IIRC I commented on this same thing during the original conversion
> > patches.
> 
> Does it work with this patch on top?

Yah, that modification builds.

rv32 being the redheaded stepchild strikes again :!

Cheers,
Conor.

> 
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>         select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -       def_bool y
> +       def_bool ARCH_SUPPORTS_KEXEC_FILE
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>         def_bool y
> 
> 
>      Arnd

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1190 bytes --]

On Mon, Oct 23, 2023 at 06:04:06PM +0200, Arnd Bergmann wrote:
> On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> > On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> 
> >> index 25474f8c12b79..f571bad2d22d0 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
> >>  	select KEXEC_ELF
> >>  
> >>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> >> -	def_bool KEXEC_FILE
> >> -	depends on CRYPTO=y
> >> -	depends on CRYPTO_SHA256=y
> >> +	def_bool y
> >
> > This being the problem, KEXEC_FILE is 64-bit only.
> >
> > IIRC I commented on this same thing during the original conversion
> > patches.
> 
> Does it work with this patch on top?

Yah, that modification builds.

rv32 being the redheaded stepchild strikes again :!

Cheers,
Conor.

> 
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>         select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -       def_bool y
> +       def_bool ARCH_SUPPORTS_KEXEC_FILE
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>         def_bool y
> 
> 
>      Arnd

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

On Mon, Oct 23, 2023 at 06:04:06PM +0200, Arnd Bergmann wrote:
> On Mon, Oct 23, 2023, at 17:37, Conor Dooley wrote:
> > On Mon, Oct 23, 2023 at 01:01:54PM +0200, Arnd Bergmann wrote:
> 
> >> index 25474f8c12b79..f571bad2d22d0 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
> >>  	select KEXEC_ELF
> >>  
> >>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> >> -	def_bool KEXEC_FILE
> >> -	depends on CRYPTO=y
> >> -	depends on CRYPTO_SHA256=y
> >> +	def_bool y
> >
> > This being the problem, KEXEC_FILE is 64-bit only.
> >
> > IIRC I commented on this same thing during the original conversion
> > patches.
> 
> Does it work with this patch on top?

Yah, that modification builds.

rv32 being the redheaded stepchild strikes again :!

Cheers,
Conor.

> 
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,7 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>         select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -       def_bool y
> +       def_bool ARCH_SUPPORTS_KEXEC_FILE
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>         def_bool y
> 
> 
>      Arnd

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> All other users of crypto code use 'select' instead of 'depends on',
> so do the same thing with KEXEC_FILE for consistency.
> 
> In practice this makes very little difference as kernels with kexec
> support are very likely to also include some other feature that already
> selects both crypto and crypto_sha256, but being consistent here helps
> for usability as well as to avoid potential circular dependencies.
> 
> This reverts the dependency back to what it was originally before commit
> 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> new syscall"), which changed changed it with the comment "This should
                       ~~~~~~~~~~~~~~ typo
> be safer as "select" is not recursive", but that appears to have been
> done in error, as "select" is indeed recursive, and there are no other
> dependencies that prevent CRYPTO_SHA256 from being selected here.
> 
> Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  kernel/Kconfig.kexec | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM,

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index bfc636d64ff2b..51f719af10e79 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,7 +36,8 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> +	select CRYPTO
> +	select CRYPTO_SHA256
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> All other users of crypto code use 'select' instead of 'depends on',
> so do the same thing with KEXEC_FILE for consistency.
> 
> In practice this makes very little difference as kernels with kexec
> support are very likely to also include some other feature that already
> selects both crypto and crypto_sha256, but being consistent here helps
> for usability as well as to avoid potential circular dependencies.
> 
> This reverts the dependency back to what it was originally before commit
> 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> new syscall"), which changed changed it with the comment "This should
                       ~~~~~~~~~~~~~~ typo
> be safer as "select" is not recursive", but that appears to have been
> done in error, as "select" is indeed recursive, and there are no other
> dependencies that prevent CRYPTO_SHA256 from being selected here.
> 
> Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  kernel/Kconfig.kexec | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM,

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index bfc636d64ff2b..51f719af10e79 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,7 +36,8 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> +	select CRYPTO
> +	select CRYPTO_SHA256
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> All other users of crypto code use 'select' instead of 'depends on',
> so do the same thing with KEXEC_FILE for consistency.
> 
> In practice this makes very little difference as kernels with kexec
> support are very likely to also include some other feature that already
> selects both crypto and crypto_sha256, but being consistent here helps
> for usability as well as to avoid potential circular dependencies.
> 
> This reverts the dependency back to what it was originally before commit
> 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> new syscall"), which changed changed it with the comment "This should
                       ~~~~~~~~~~~~~~ typo
> be safer as "select" is not recursive", but that appears to have been
> done in error, as "select" is indeed recursive, and there are no other
> dependencies that prevent CRYPTO_SHA256 from being selected here.
> 
> Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  kernel/Kconfig.kexec | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM,

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index bfc636d64ff2b..51f719af10e79 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,7 +36,8 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> +	select CRYPTO
> +	select CRYPTO_SHA256
>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2
> 

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/24/23 at 11:55am, Baoquan He wrote:
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > All other users of crypto code use 'select' instead of 'depends on',
> > so do the same thing with KEXEC_FILE for consistency.
> > 
> > In practice this makes very little difference as kernels with kexec
> > support are very likely to also include some other feature that already
> > selects both crypto and crypto_sha256, but being consistent here helps
> > for usability as well as to avoid potential circular dependencies.
> > 
> > This reverts the dependency back to what it was originally before commit
> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> > new syscall"), which changed changed it with the comment "This should
>                        ~~~~~~~~~~~~~~ typo
> > be safer as "select" is not recursive", but that appears to have been
> > done in error, as "select" is indeed recursive, and there are no other
> > dependencies that prevent CRYPTO_SHA256 from being selected here.
> > 
> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: linux-crypto@vger.kernel.org
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  kernel/Kconfig.kexec | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> LGTM,
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
didn't get the intention of the entire patchset. I need hold the ack
until I read the patch 1. I have some concerns about patch 1 if I didn't
misunderstand it. Will come back later when patch 1 reviewing is
finished.

> 
> > 
> > diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> > index bfc636d64ff2b..51f719af10e79 100644
> > --- a/kernel/Kconfig.kexec
> > +++ b/kernel/Kconfig.kexec
> > @@ -36,7 +36,8 @@ config KEXEC
> >  config KEXEC_FILE
> >  	bool "Enable kexec file based system call"
> >  	depends on ARCH_SUPPORTS_KEXEC_FILE
> > -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> > +	select CRYPTO
> > +	select CRYPTO_SHA256
> >  	select KEXEC_CORE
> >  	help
> >  	  This is new version of kexec system call. This system call is
> > -- 
> > 2.39.2
> > 
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/24/23 at 11:55am, Baoquan He wrote:
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > All other users of crypto code use 'select' instead of 'depends on',
> > so do the same thing with KEXEC_FILE for consistency.
> > 
> > In practice this makes very little difference as kernels with kexec
> > support are very likely to also include some other feature that already
> > selects both crypto and crypto_sha256, but being consistent here helps
> > for usability as well as to avoid potential circular dependencies.
> > 
> > This reverts the dependency back to what it was originally before commit
> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> > new syscall"), which changed changed it with the comment "This should
>                        ~~~~~~~~~~~~~~ typo
> > be safer as "select" is not recursive", but that appears to have been
> > done in error, as "select" is indeed recursive, and there are no other
> > dependencies that prevent CRYPTO_SHA256 from being selected here.
> > 
> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: linux-crypto@vger.kernel.org
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  kernel/Kconfig.kexec | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> LGTM,
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
didn't get the intention of the entire patchset. I need hold the ack
until I read the patch 1. I have some concerns about patch 1 if I didn't
misunderstand it. Will come back later when patch 1 reviewing is
finished.

> 
> > 
> > diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> > index bfc636d64ff2b..51f719af10e79 100644
> > --- a/kernel/Kconfig.kexec
> > +++ b/kernel/Kconfig.kexec
> > @@ -36,7 +36,8 @@ config KEXEC
> >  config KEXEC_FILE
> >  	bool "Enable kexec file based system call"
> >  	depends on ARCH_SUPPORTS_KEXEC_FILE
> > -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> > +	select CRYPTO
> > +	select CRYPTO_SHA256
> >  	select KEXEC_CORE
> >  	help
> >  	  This is new version of kexec system call. This system call is
> > -- 
> > 2.39.2
> > 
> 

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Baoquan He]

On 10/24/23 at 11:55am, Baoquan He wrote:
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > All other users of crypto code use 'select' instead of 'depends on',
> > so do the same thing with KEXEC_FILE for consistency.
> > 
> > In practice this makes very little difference as kernels with kexec
> > support are very likely to also include some other feature that already
> > selects both crypto and crypto_sha256, but being consistent here helps
> > for usability as well as to avoid potential circular dependencies.
> > 
> > This reverts the dependency back to what it was originally before commit
> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
> > new syscall"), which changed changed it with the comment "This should
>                        ~~~~~~~~~~~~~~ typo
> > be safer as "select" is not recursive", but that appears to have been
> > done in error, as "select" is indeed recursive, and there are no other
> > dependencies that prevent CRYPTO_SHA256 from being selected here.
> > 
> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: linux-crypto@vger.kernel.org
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  kernel/Kconfig.kexec | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> LGTM,
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
didn't get the intention of the entire patchset. I need hold the ack
until I read the patch 1. I have some concerns about patch 1 if I didn't
misunderstand it. Will come back later when patch 1 reviewing is
finished.

> 
> > 
> > diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> > index bfc636d64ff2b..51f719af10e79 100644
> > --- a/kernel/Kconfig.kexec
> > +++ b/kernel/Kconfig.kexec
> > @@ -36,7 +36,8 @@ config KEXEC
> >  config KEXEC_FILE
> >  	bool "Enable kexec file based system call"
> >  	depends on ARCH_SUPPORTS_KEXEC_FILE
> > -	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> > +	select CRYPTO
> > +	select CRYPTO_SHA256
> >  	select KEXEC_CORE
> >  	help
> >  	  This is new version of kexec system call. This system call is
> > -- 
> > 2.39.2
> > 
> 

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 06:10, Baoquan He wrote:
> On 10/24/23 at 11:55am, Baoquan He wrote:
>> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> > 
>> > All other users of crypto code use 'select' instead of 'depends on',
>> > so do the same thing with KEXEC_FILE for consistency.
>> > 
>> > In practice this makes very little difference as kernels with kexec
>> > support are very likely to also include some other feature that already
>> > selects both crypto and crypto_sha256, but being consistent here helps
>> > for usability as well as to avoid potential circular dependencies.
>> > 
>> > This reverts the dependency back to what it was originally before commit
>> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
>> > new syscall"), which changed changed it with the comment "This should
>>                        ~~~~~~~~~~~~~~ typo
>> > be safer as "select" is not recursive", but that appears to have been
>> > done in error, as "select" is indeed recursive, and there are no other
>> > dependencies that prevent CRYPTO_SHA256 from being selected here.
>> > 
>> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
>> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> > Cc: "David S. Miller" <davem@davemloft.net>
>> > Cc: linux-crypto@vger.kernel.org
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> >  kernel/Kconfig.kexec | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> LGTM,
>> 
>> Acked-by: Baoquan He <bhe@redhat.com>
>
> Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
> didn't get the intention of the entire patchset. I need hold the ack
> until I read the patch 1. I have some concerns about patch 1 if I didn't
> misunderstand it. Will come back later when patch 1 reviewing is
> finished.

Sorry about missing you on Cc. If anyone else is looking for the
patch, it's archived at
https://lore.kernel.org/lkml/20231023110308.1202042-1-arnd@kernel.org/

The idea of patch 1 was only to address the build regression on
x86, so I was hoping that part would be uncontroversial. I split
out patch 2/2 since that is intended to actually change the behavior,
hopefully for the better.

I introduced a new regression on riscv that Conor Dooley found, and
that should already be fixed now. It looks like we may need a similar
change on s390

--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools

but I haven't tested that, and I'll wait for your reply then.

    Arnd

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 06:10, Baoquan He wrote:
> On 10/24/23 at 11:55am, Baoquan He wrote:
>> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> > 
>> > All other users of crypto code use 'select' instead of 'depends on',
>> > so do the same thing with KEXEC_FILE for consistency.
>> > 
>> > In practice this makes very little difference as kernels with kexec
>> > support are very likely to also include some other feature that already
>> > selects both crypto and crypto_sha256, but being consistent here helps
>> > for usability as well as to avoid potential circular dependencies.
>> > 
>> > This reverts the dependency back to what it was originally before commit
>> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
>> > new syscall"), which changed changed it with the comment "This should
>>                        ~~~~~~~~~~~~~~ typo
>> > be safer as "select" is not recursive", but that appears to have been
>> > done in error, as "select" is indeed recursive, and there are no other
>> > dependencies that prevent CRYPTO_SHA256 from being selected here.
>> > 
>> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
>> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> > Cc: "David S. Miller" <davem@davemloft.net>
>> > Cc: linux-crypto@vger.kernel.org
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> >  kernel/Kconfig.kexec | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> LGTM,
>> 
>> Acked-by: Baoquan He <bhe@redhat.com>
>
> Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
> didn't get the intention of the entire patchset. I need hold the ack
> until I read the patch 1. I have some concerns about patch 1 if I didn't
> misunderstand it. Will come back later when patch 1 reviewing is
> finished.

Sorry about missing you on Cc. If anyone else is looking for the
patch, it's archived at
https://lore.kernel.org/lkml/20231023110308.1202042-1-arnd@kernel.org/

The idea of patch 1 was only to address the build regression on
x86, so I was hoping that part would be uncontroversial. I split
out patch 2/2 since that is intended to actually change the behavior,
hopefully for the better.

I introduced a new regression on riscv that Conor Dooley found, and
that should already be fixed now. It looks like we may need a similar
change on s390

--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools

but I haven't tested that, and I'll wait for your reply then.

    Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] kexec: select CRYPTO from KEXEC_FILE instead of depending on it

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 06:10, Baoquan He wrote:
> On 10/24/23 at 11:55am, Baoquan He wrote:
>> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> > 
>> > All other users of crypto code use 'select' instead of 'depends on',
>> > so do the same thing with KEXEC_FILE for consistency.
>> > 
>> > In practice this makes very little difference as kernels with kexec
>> > support are very likely to also include some other feature that already
>> > selects both crypto and crypto_sha256, but being consistent here helps
>> > for usability as well as to avoid potential circular dependencies.
>> > 
>> > This reverts the dependency back to what it was originally before commit
>> > 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for
>> > new syscall"), which changed changed it with the comment "This should
>>                        ~~~~~~~~~~~~~~ typo
>> > be safer as "select" is not recursive", but that appears to have been
>> > done in error, as "select" is indeed recursive, and there are no other
>> > dependencies that prevent CRYPTO_SHA256 from being selected here.
>> > 
>> > Fixes: 74ca317c26a3f ("kexec: create a new config option CONFIG_KEXEC_FILE for new syscall")
>> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> > Cc: "David S. Miller" <davem@davemloft.net>
>> > Cc: linux-crypto@vger.kernel.org
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> >  kernel/Kconfig.kexec | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> LGTM,
>> 
>> Acked-by: Baoquan He <bhe@redhat.com>
>
> Sorry, the patch 1/2 is not sent to me and kexec mailing list, so I
> didn't get the intention of the entire patchset. I need hold the ack
> until I read the patch 1. I have some concerns about patch 1 if I didn't
> misunderstand it. Will come back later when patch 1 reviewing is
> finished.

Sorry about missing you on Cc. If anyone else is looking for the
patch, it's archived at
https://lore.kernel.org/lkml/20231023110308.1202042-1-arnd@kernel.org/

The idea of patch 1 was only to address the build regression on
x86, so I was hoping that part would be uncontroversial. I split
out patch 2/2 since that is intended to actually change the behavior,
hopefully for the better.

I introduced a new regression on riscv that Conor Dooley found, and
that should already be fixed now. It looks like we may need a similar
change on s390

--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools

but I haven't tested that, and I'll wait for your reply then.

    Arnd

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Just add people and mailing list to CC since I didn't find this mail in
my box, just drag it via 'b4 am'.

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
......
> ---
>  arch/powerpc/Kconfig | 4 ++--
>  arch/riscv/Kconfig   | 4 +---
>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY

I am not sure if the logic is correct. In theory, kexec_file code
utilizes purgatory to verify the checksum digested during kernel loading
when try to jump to the kernel. That means kexec_file depends on
purgatory, but not contrary?

With these changes, we can achieve the goal to avoid building issue,
whereas the code logic becomes confusing. E.g people could disable
CONFIG_KEXEC_FILE, but still get purgatory code built in which is
totally useless.

Not sure if I think too much over this.

>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Just add people and mailing list to CC since I didn't find this mail in
my box, just drag it via 'b4 am'.

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
......
> ---
>  arch/powerpc/Kconfig | 4 ++--
>  arch/riscv/Kconfig   | 4 +---
>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY

I am not sure if the logic is correct. In theory, kexec_file code
utilizes purgatory to verify the checksum digested during kernel loading
when try to jump to the kernel. That means kexec_file depends on
purgatory, but not contrary?

With these changes, we can achieve the goal to avoid building issue,
whereas the code logic becomes confusing. E.g people could disable
CONFIG_KEXEC_FILE, but still get purgatory code built in which is
totally useless.

Not sure if I think too much over this.

>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2


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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Just add people and mailing list to CC since I didn't find this mail in
my box, just drag it via 'b4 am'.

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
......
> ---
>  arch/powerpc/Kconfig | 4 ++--
>  arch/riscv/Kconfig   | 4 +---
>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY

I am not sure if the logic is correct. In theory, kexec_file code
utilizes purgatory to verify the checksum digested during kernel loading
when try to jump to the kernel. That means kexec_file depends on
purgatory, but not contrary?

With these changes, we can achieve the goal to avoid building issue,
whereas the code logic becomes confusing. E.g people could disable
CONFIG_KEXEC_FILE, but still get purgatory code built in which is
totally useless.

Not sure if I think too much over this.

>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Just add people and mailing list to CC since I didn't find this mail in
my box, just drag it via 'b4 am'.

On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
......
> ---
>  arch/powerpc/Kconfig | 4 ++--
>  arch/riscv/Kconfig   | 4 +---
>  arch/s390/Kconfig    | 4 ++--
>  arch/x86/Kconfig     | 4 ++--
>  kernel/Kconfig.kexec | 1 +
>  5 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d5d5388973ac7..4640cee33f123 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -607,10 +607,10 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
> +	def_bool PPC64
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 25474f8c12b79..f571bad2d22d0 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -687,9 +687,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select KEXEC_ELF
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> -	depends on CRYPTO=y
> -	depends on CRYPTO_SHA256=y
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b0d67ac8695f9..ec77106af4137 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -253,13 +253,13 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool MODULE_SIG_FORMAT
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_CRASH_DUMP
>  	def_bool y
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 94efde80ebf35..f9975b15ccd57 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2073,7 +2073,7 @@ config ARCH_SUPPORTS_KEXEC
>  	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_FILE
> -	def_bool X86_64 && CRYPTO && CRYPTO_SHA256
> +	def_bool X86_64
>  
>  config ARCH_SELECTS_KEXEC_FILE
>  	def_bool y
> @@ -2081,7 +2081,7 @@ config ARCH_SELECTS_KEXEC_FILE
>  	select HAVE_IMA_KEXEC if IMA
>  
>  config ARCH_SUPPORTS_KEXEC_PURGATORY
> -	def_bool KEXEC_FILE
> +	def_bool y
>  
>  config ARCH_SUPPORTS_KEXEC_SIG
>  	def_bool y
> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> index 7aff28ded2f48..bfc636d64ff2b 100644
> --- a/kernel/Kconfig.kexec
> +++ b/kernel/Kconfig.kexec
> @@ -36,6 +36,7 @@ config KEXEC
>  config KEXEC_FILE
>  	bool "Enable kexec file based system call"
>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY

I am not sure if the logic is correct. In theory, kexec_file code
utilizes purgatory to verify the checksum digested during kernel loading
when try to jump to the kernel. That means kexec_file depends on
purgatory, but not contrary?

With these changes, we can achieve the goal to avoid building issue,
whereas the code logic becomes confusing. E.g people could disable
CONFIG_KEXEC_FILE, but still get purgatory code built in which is
totally useless.

Not sure if I think too much over this.

>  	select KEXEC_CORE
>  	help
>  	  This is new version of kexec system call. This system call is
> -- 
> 2.39.2

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> Just add people and mailing list to CC since I didn't find this mail in
> my box, just drag it via 'b4 am'.
>
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> ......

>> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
>> index 7aff28ded2f48..bfc636d64ff2b 100644
>> --- a/kernel/Kconfig.kexec
>> +++ b/kernel/Kconfig.kexec
>> @@ -36,6 +36,7 @@ config KEXEC
>>  config KEXEC_FILE
>>  	bool "Enable kexec file based system call"
>>  	depends on ARCH_SUPPORTS_KEXEC_FILE
>> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> I am not sure if the logic is correct. In theory, kexec_file code
> utilizes purgatory to verify the checksum digested during kernel loading
> when try to jump to the kernel. That means kexec_file depends on
> purgatory, but not contrary?

The expression I wrote is a bit confusing, but I think this just
keeps the existing behavior:

- on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
  to be built-in.
- on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
  or =m.

Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
be written as

depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
           || !ARCH_SUPPORTS_KEXEC_PURGATORY

if you find that clearer. I see that the second patch
actually gets this wrong, it should actually do

select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

> With these changes, we can achieve the goal to avoid building issue,
> whereas the code logic becomes confusing. E.g people could disable
> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> totally useless.
>
> Not sure if I think too much over this.

I see your point here, and I would suggest changing the
CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
the availability of the purgatory code for the arch, rather
than actually controlling the code itself. I already mentioned
this for s390, but riscv would need the same thing on top.

I think the change below should address your concern.

     Arnd

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
        /* Add purgatory to the image */
        kbuf.top_down = true;
        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                                             sizeof(kernel_start), 0);
        if (ret)
                pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
 obj-y += errata/
 obj-$(CONFIG_KVM) += kvm/
 
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
 
 # for cleaning
 subdir- += boot
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..361aa01dbd49 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> Just add people and mailing list to CC since I didn't find this mail in
> my box, just drag it via 'b4 am'.
>
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> ......

>> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
>> index 7aff28ded2f48..bfc636d64ff2b 100644
>> --- a/kernel/Kconfig.kexec
>> +++ b/kernel/Kconfig.kexec
>> @@ -36,6 +36,7 @@ config KEXEC
>>  config KEXEC_FILE
>>  	bool "Enable kexec file based system call"
>>  	depends on ARCH_SUPPORTS_KEXEC_FILE
>> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> I am not sure if the logic is correct. In theory, kexec_file code
> utilizes purgatory to verify the checksum digested during kernel loading
> when try to jump to the kernel. That means kexec_file depends on
> purgatory, but not contrary?

The expression I wrote is a bit confusing, but I think this just
keeps the existing behavior:

- on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
  to be built-in.
- on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
  or =m.

Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
be written as

depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
           || !ARCH_SUPPORTS_KEXEC_PURGATORY

if you find that clearer. I see that the second patch
actually gets this wrong, it should actually do

select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

> With these changes, we can achieve the goal to avoid building issue,
> whereas the code logic becomes confusing. E.g people could disable
> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> totally useless.
>
> Not sure if I think too much over this.

I see your point here, and I would suggest changing the
CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
the availability of the purgatory code for the arch, rather
than actually controlling the code itself. I already mentioned
this for s390, but riscv would need the same thing on top.

I think the change below should address your concern.

     Arnd

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
        /* Add purgatory to the image */
        kbuf.top_down = true;
        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                                             sizeof(kernel_start), 0);
        if (ret)
                pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
 obj-y += errata/
 obj-$(CONFIG_KVM) += kvm/
 
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
 
 # for cleaning
 subdir- += boot
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..361aa01dbd49 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> Just add people and mailing list to CC since I didn't find this mail in
> my box, just drag it via 'b4 am'.
>
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> ......

>> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
>> index 7aff28ded2f48..bfc636d64ff2b 100644
>> --- a/kernel/Kconfig.kexec
>> +++ b/kernel/Kconfig.kexec
>> @@ -36,6 +36,7 @@ config KEXEC
>>  config KEXEC_FILE
>>  	bool "Enable kexec file based system call"
>>  	depends on ARCH_SUPPORTS_KEXEC_FILE
>> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> I am not sure if the logic is correct. In theory, kexec_file code
> utilizes purgatory to verify the checksum digested during kernel loading
> when try to jump to the kernel. That means kexec_file depends on
> purgatory, but not contrary?

The expression I wrote is a bit confusing, but I think this just
keeps the existing behavior:

- on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
  to be built-in.
- on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
  or =m.

Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
be written as

depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
           || !ARCH_SUPPORTS_KEXEC_PURGATORY

if you find that clearer. I see that the second patch
actually gets this wrong, it should actually do

select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

> With these changes, we can achieve the goal to avoid building issue,
> whereas the code logic becomes confusing. E.g people could disable
> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> totally useless.
>
> Not sure if I think too much over this.

I see your point here, and I would suggest changing the
CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
the availability of the purgatory code for the arch, rather
than actually controlling the code itself. I already mentioned
this for s390, but riscv would need the same thing on top.

I think the change below should address your concern.

     Arnd

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
        /* Add purgatory to the image */
        kbuf.top_down = true;
        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                                             sizeof(kernel_start), 0);
        if (ret)
                pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
 obj-y += errata/
 obj-$(CONFIG_KVM) += kvm/
 
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
 
 # for cleaning
 subdir- += boot
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..361aa01dbd49 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools


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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Arnd Bergmann]

On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> Just add people and mailing list to CC since I didn't find this mail in
> my box, just drag it via 'b4 am'.
>
> On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> ......

>> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
>> index 7aff28ded2f48..bfc636d64ff2b 100644
>> --- a/kernel/Kconfig.kexec
>> +++ b/kernel/Kconfig.kexec
>> @@ -36,6 +36,7 @@ config KEXEC
>>  config KEXEC_FILE
>>  	bool "Enable kexec file based system call"
>>  	depends on ARCH_SUPPORTS_KEXEC_FILE
>> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
>
> I am not sure if the logic is correct. In theory, kexec_file code
> utilizes purgatory to verify the checksum digested during kernel loading
> when try to jump to the kernel. That means kexec_file depends on
> purgatory, but not contrary?

The expression I wrote is a bit confusing, but I think this just
keeps the existing behavior:

- on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
  to be built-in.
- on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
  or =m.

Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
be written as

depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
           || !ARCH_SUPPORTS_KEXEC_PURGATORY

if you find that clearer. I see that the second patch
actually gets this wrong, it should actually do

select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

> With these changes, we can achieve the goal to avoid building issue,
> whereas the code logic becomes confusing. E.g people could disable
> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> totally useless.
>
> Not sure if I think too much over this.

I see your point here, and I would suggest changing the
CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
the availability of the purgatory code for the arch, rather
than actually controlling the code itself. I already mentioned
this for s390, but riscv would need the same thing on top.

I think the change below should address your concern.

     Arnd

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
        /* Add purgatory to the image */
        kbuf.top_down = true;
        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                                             sizeof(kernel_start), 0);
        if (ret)
                pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
 obj-y += errata/
 obj-$(CONFIG_KVM) += kvm/
 
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/
 
 # for cleaning
 subdir- += boot
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..361aa01dbd49 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
 obj-$(CONFIG_APPLDATA_BASE)    += appldata/
 obj-y                          += net/
 obj-$(CONFIG_PCI)              += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE)       += purgatory/
 
 # for cleaning
 subdir- += boot tools

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 


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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Eric DeVolder]

[-- Attachment #1: Type: text/plain, Size: 5342 bytes --]

 

    On Wednesday, October 25, 2023 at 04:58:20 AM CDT, Baoquan He <bhe@redhat.com> wrote:  
 
 On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>      bool "Enable kexec file based system call"
> >>      depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>  (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>  to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>  (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>  or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY

Yes, this seems to be clearer to me. Thanks.

> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY

Yeah, makes sense to me.

Hi Eric,

Do you have comment about these?
[eric]: The original goal of the conversion patches was to consolidate, but keep existing behavior.Then I had to break existing behavior a bit by tying CRASH to KEXEC. I was hoping to haveavoided introducing new problems, but looks like there was an escape. At any rate, I thinkthe ideas here are on track and necessary. Unfortunately at the moment I'm not at a placewhere I can test.

> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.
> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                cmdline = modified_cmdline;
>        }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>        /* Add purgatory to the image */
>        kbuf.top_down = true;
>        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>        if (ret)
>                pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */

If so, we don't need the CONFIG_KEXEC_FILE ifdeffery because the
file elf_kexec.c relied on CONFIG_KEXEC_FILE enabling to build in.
We can just remove the "#ifdef CONFIG_KEXEC_FILE..#endif" as x86 does.

>  
>        /* Add the initrd to the image */
>        if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)      += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 

  

[-- Attachment #2: Type: text/html, Size: 9164 bytes --]

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Hi Arnd,

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.

Since no new comment, do you mind spinning v2 to wrap all these up?

Thanks
Baoquan

> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */
>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Hi Arnd,

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.

Since no new comment, do you mind spinning v2 to wrap all these up?

Thanks
Baoquan

> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */
>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 


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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Hi Arnd,

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.

Since no new comment, do you mind spinning v2 to wrap all these up?

Thanks
Baoquan

> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */
>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Baoquan He]

Hi Arnd,

On 10/24/23 at 03:17pm, Arnd Bergmann wrote:
> On Tue, Oct 24, 2023, at 14:44, Baoquan He wrote:
> > Just add people and mailing list to CC since I didn't find this mail in
> > my box, just drag it via 'b4 am'.
> >
> > On 10/23/23 at 01:01pm, Arnd Bergmann wrote:
> > ......
> 
> >> diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
> >> index 7aff28ded2f48..bfc636d64ff2b 100644
> >> --- a/kernel/Kconfig.kexec
> >> +++ b/kernel/Kconfig.kexec
> >> @@ -36,6 +36,7 @@ config KEXEC
> >>  config KEXEC_FILE
> >>  	bool "Enable kexec file based system call"
> >>  	depends on ARCH_SUPPORTS_KEXEC_FILE
> >> +	depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
> >
> > I am not sure if the logic is correct. In theory, kexec_file code
> > utilizes purgatory to verify the checksum digested during kernel loading
> > when try to jump to the kernel. That means kexec_file depends on
> > purgatory, but not contrary?
> 
> The expression I wrote is a bit confusing, but I think this just
> keeps the existing behavior:
> 
> - on architectures that select ARCH_SUPPORTS_KEXEC_PURGATORY
>   (powerpc, riscv, s390 and x86), we also require CRYPTO_SHA256
>   to be built-in.
> - on architectures that do not have ARCH_SUPPORTS_KEXEC_PURGATORY
>   (arm64 and parisc), CRYPTO_SHA256 is not used and can be disabled
>   or =m.
> 
> Since ARCH_SUPPORTS_KEXEC_PURGATORY is a 'bool' symbol, it could
> be written as
> 
> depends on (ARCH_SUPPORTS_KEXEC_PURGATORY && CRYPTO_SHA256=y) \
>            || !ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> if you find that clearer. I see that the second patch
> actually gets this wrong, it should actually do
> 
> select CRYPTO if ARCH_SUPPORTS_KEXEC_PURGATORY
> select CRYPTO_SHA256 if ARCH_SUPPORTS_KEXEC_PURGATORY
> 
> > With these changes, we can achieve the goal to avoid building issue,
> > whereas the code logic becomes confusing. E.g people could disable
> > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > totally useless.
> >
> > Not sure if I think too much over this.
> 
> I see your point here, and I would suggest changing the
> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> the availability of the purgatory code for the arch, rather
> than actually controlling the code itself. I already mentioned
> this for s390, but riscv would need the same thing on top.
> 
> I think the change below should address your concern.

Since no new comment, do you mind spinning v2 to wrap all these up?

Thanks
Baoquan

> 
>      Arnd
> 
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index e60fbd8660c4..3ac341d296db 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                 cmdline = modified_cmdline;
>         }
>  
> -#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
> +#ifdef CONFIG_KEXEC_FILE
>         /* Add purgatory to the image */
>         kbuf.top_down = true;
>         kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> @@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
>                                              sizeof(kernel_start), 0);
>         if (ret)
>                 pr_err("Error update purgatory ret=%d\n", ret);
> -#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
> +#endif /* CONFIG_KEXEC_FILE */
>  
>         /* Add the initrd to the image */
>         if (initrd != NULL) {
> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
> index d25ad1c19f88..ab181d187c23 100644
> --- a/arch/riscv/Kbuild
> +++ b/arch/riscv/Kbuild
> @@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
>  obj-y += errata/
>  obj-$(CONFIG_KVM) += kvm/
>  
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE) += purgatory/
>  
>  # for cleaning
>  subdir- += boot
> diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
> index a5d3503b353c..361aa01dbd49 100644
> --- a/arch/s390/Kbuild
> +++ b/arch/s390/Kbuild
> @@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)        += hypfs/
>  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
>  obj-y                          += net/
>  obj-$(CONFIG_PCI)              += pci/
> -obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
> +obj-$(CONFIG_KEXEC_FILE)       += purgatory/
>  
>  # for cleaning
>  subdir- += boot tools
> 

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Andrew Morton]

On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:

> > > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > > totally useless.
> > >
> > > Not sure if I think too much over this.
> > 
> > I see your point here, and I would suggest changing the
> > CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> > the availability of the purgatory code for the arch, rather
> > than actually controlling the code itself. I already mentioned
> > this for s390, but riscv would need the same thing on top.
> > 
> > I think the change below should address your concern.
> 
> Since no new comment, do you mind spinning v2 to wrap all these up?

This patchset remains in mm-hotfixes-unstable from the previous -rc
cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
not, should I merge v1?  If so, should I now add cc:stable?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Andrew Morton]

On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:

> > > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > > totally useless.
> > >
> > > Not sure if I think too much over this.
> > 
> > I see your point here, and I would suggest changing the
> > CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> > the availability of the purgatory code for the arch, rather
> > than actually controlling the code itself. I already mentioned
> > this for s390, but riscv would need the same thing on top.
> > 
> > I think the change below should address your concern.
> 
> Since no new comment, do you mind spinning v2 to wrap all these up?

This patchset remains in mm-hotfixes-unstable from the previous -rc
cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
not, should I merge v1?  If so, should I now add cc:stable?

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Andrew Morton]

On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:

> > > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > > totally useless.
> > >
> > > Not sure if I think too much over this.
> > 
> > I see your point here, and I would suggest changing the
> > CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> > the availability of the purgatory code for the arch, rather
> > than actually controlling the code itself. I already mentioned
> > this for s390, but riscv would need the same thing on top.
> > 
> > I think the change below should address your concern.
> 
> Since no new comment, do you mind spinning v2 to wrap all these up?

This patchset remains in mm-hotfixes-unstable from the previous -rc
cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
not, should I merge v1?  If so, should I now add cc:stable?

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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Andrew Morton]

On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:

> > > CONFIG_KEXEC_FILE, but still get purgatory code built in which is
> > > totally useless.
> > >
> > > Not sure if I think too much over this.
> > 
> > I see your point here, and I would suggest changing the
> > CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
> > the availability of the purgatory code for the arch, rather
> > than actually controlling the code itself. I already mentioned
> > this for s390, but riscv would need the same thing on top.
> > 
> > I think the change below should address your concern.
> 
> Since no new comment, do you mind spinning v2 to wrap all these up?

This patchset remains in mm-hotfixes-unstable from the previous -rc
cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
not, should I merge v1?  If so, should I now add cc:stable?

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Eric DeVolder]

On 11/30/23 10:56, Andrew Morton wrote:
> On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:
>
>>>> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
>>>> totally useless.
>>>>
>>>> Not sure if I think too much over this.
>>> I see your point here, and I would suggest changing the
>>> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
>>> the availability of the purgatory code for the arch, rather
>>> than actually controlling the code itself. I already mentioned
>>> this for s390, but riscv would need the same thing on top.
>>>
>>> I think the change below should address your concern.
>> Since no new comment, do you mind spinning v2 to wrap all these up?
> This patchset remains in mm-hotfixes-unstable from the previous -rc
> cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
> not, should I merge v1?  If so, should I now add cc:stable?

My apologies, I lost this. I've looked at these changes, and I am in 
favor of these changes.

Furthermore, I ran the following thru the Kconfig regression script, and 
did not find anything!

I believe the following patch represents the current discussion threads 
around Kconfig and KEXEC/CRASH.

Reviewed-by: Eric DeVolder <eric_devolder@yahoo.com>

Tested-by: Eric DeVolder <eric_devolder@yahoo.com>

Thanks!

eric

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..1f11a62809f2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -608,10 +608,10 @@ config ARCH_SUPPORTS_KEXEC
      def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+    def_bool PPC64

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
  obj-y += errata/
  obj-$(CONFIG_KVM) += kvm/

-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a..98857d76e458 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -702,9 +702,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select KEXEC_ELF

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
-    depends on CRYPTO=y
-    depends on CRYPTO_SHA256=y
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
          cmdline = modified_cmdline;
      }

-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
      /* Add purgatory to the image */
      kbuf.top_down = true;
      kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
                           sizeof(kernel_start), 0);
      if (ret)
          pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */

      /* Add the initrd to the image */
      if (initrd != NULL) {
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..f2ce80b65551 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)    += hypfs/
  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
  obj-y                += net/
  obj-$(CONFIG_PCI)        += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot tools
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..d5d8f99d1f25 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -254,13 +254,13 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool MODULE_SIG_FORMAT

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..1566748f16c4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2072,7 +2072,7 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+    def_bool X86_64

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
@@ -2080,7 +2080,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select HAVE_IMA_KEXEC if IMA

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..92120e396008 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
  config KEXEC_FILE
      bool "Enable kexec file based system call"
      depends on ARCH_SUPPORTS_KEXEC_FILE
+    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
      select KEXEC_CORE
      help
        This is new version of kexec system call. This system call is
@@ -94,10 +95,8 @@ config KEXEC_JUMP
  config CRASH_DUMP
      bool "kernel crash dumps"
      depends on ARCH_SUPPORTS_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


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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Eric DeVolder]

On 11/30/23 10:56, Andrew Morton wrote:
> On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:
>
>>>> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
>>>> totally useless.
>>>>
>>>> Not sure if I think too much over this.
>>> I see your point here, and I would suggest changing the
>>> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
>>> the availability of the purgatory code for the arch, rather
>>> than actually controlling the code itself. I already mentioned
>>> this for s390, but riscv would need the same thing on top.
>>>
>>> I think the change below should address your concern.
>> Since no new comment, do you mind spinning v2 to wrap all these up?
> This patchset remains in mm-hotfixes-unstable from the previous -rc
> cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
> not, should I merge v1?  If so, should I now add cc:stable?

My apologies, I lost this. I've looked at these changes, and I am in 
favor of these changes.

Furthermore, I ran the following thru the Kconfig regression script, and 
did not find anything!

I believe the following patch represents the current discussion threads 
around Kconfig and KEXEC/CRASH.

Reviewed-by: Eric DeVolder <eric_devolder@yahoo.com>

Tested-by: Eric DeVolder <eric_devolder@yahoo.com>

Thanks!

eric

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..1f11a62809f2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -608,10 +608,10 @@ config ARCH_SUPPORTS_KEXEC
      def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+    def_bool PPC64

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
  obj-y += errata/
  obj-$(CONFIG_KVM) += kvm/

-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a..98857d76e458 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -702,9 +702,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select KEXEC_ELF

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
-    depends on CRYPTO=y
-    depends on CRYPTO_SHA256=y
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
          cmdline = modified_cmdline;
      }

-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
      /* Add purgatory to the image */
      kbuf.top_down = true;
      kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
                           sizeof(kernel_start), 0);
      if (ret)
          pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */

      /* Add the initrd to the image */
      if (initrd != NULL) {
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..f2ce80b65551 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)    += hypfs/
  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
  obj-y                += net/
  obj-$(CONFIG_PCI)        += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot tools
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..d5d8f99d1f25 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -254,13 +254,13 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool MODULE_SIG_FORMAT

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..1566748f16c4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2072,7 +2072,7 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+    def_bool X86_64

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
@@ -2080,7 +2080,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select HAVE_IMA_KEXEC if IMA

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..92120e396008 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
  config KEXEC_FILE
      bool "Enable kexec file based system call"
      depends on ARCH_SUPPORTS_KEXEC_FILE
+    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
      select KEXEC_CORE
      help
        This is new version of kexec system call. This system call is
@@ -94,10 +95,8 @@ config KEXEC_JUMP
  config CRASH_DUMP
      bool "kernel crash dumps"
      depends on ARCH_SUPPORTS_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

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Eric DeVolder]

On 11/30/23 10:56, Andrew Morton wrote:
> On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:
>
>>>> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
>>>> totally useless.
>>>>
>>>> Not sure if I think too much over this.
>>> I see your point here, and I would suggest changing the
>>> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
>>> the availability of the purgatory code for the arch, rather
>>> than actually controlling the code itself. I already mentioned
>>> this for s390, but riscv would need the same thing on top.
>>>
>>> I think the change below should address your concern.
>> Since no new comment, do you mind spinning v2 to wrap all these up?
> This patchset remains in mm-hotfixes-unstable from the previous -rc
> cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
> not, should I merge v1?  If so, should I now add cc:stable?

My apologies, I lost this. I've looked at these changes, and I am in 
favor of these changes.

Furthermore, I ran the following thru the Kconfig regression script, and 
did not find anything!

I believe the following patch represents the current discussion threads 
around Kconfig and KEXEC/CRASH.

Reviewed-by: Eric DeVolder <eric_devolder@yahoo.com>

Tested-by: Eric DeVolder <eric_devolder@yahoo.com>

Thanks!

eric

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..1f11a62809f2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -608,10 +608,10 @@ config ARCH_SUPPORTS_KEXEC
      def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+    def_bool PPC64

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
  obj-y += errata/
  obj-$(CONFIG_KVM) += kvm/

-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a..98857d76e458 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -702,9 +702,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select KEXEC_ELF

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
-    depends on CRYPTO=y
-    depends on CRYPTO_SHA256=y
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
          cmdline = modified_cmdline;
      }

-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
      /* Add purgatory to the image */
      kbuf.top_down = true;
      kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
                           sizeof(kernel_start), 0);
      if (ret)
          pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */

      /* Add the initrd to the image */
      if (initrd != NULL) {
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..f2ce80b65551 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)    += hypfs/
  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
  obj-y                += net/
  obj-$(CONFIG_PCI)        += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot tools
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..d5d8f99d1f25 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -254,13 +254,13 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool MODULE_SIG_FORMAT

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..1566748f16c4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2072,7 +2072,7 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+    def_bool X86_64

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
@@ -2080,7 +2080,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select HAVE_IMA_KEXEC if IMA

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..92120e396008 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
  config KEXEC_FILE
      bool "Enable kexec file based system call"
      depends on ARCH_SUPPORTS_KEXEC_FILE
+    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
      select KEXEC_CORE
      help
        This is new version of kexec system call. This system call is
@@ -94,10 +95,8 @@ config KEXEC_JUMP
  config CRASH_DUMP
      bool "kernel crash dumps"
      depends on ARCH_SUPPORTS_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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/2] kexec: fix KEXEC_FILE dependencies

[Eric DeVolder]

On 11/30/23 10:56, Andrew Morton wrote:
> On Thu, 2 Nov 2023 16:03:18 +0800 Baoquan He <bhe@redhat.com> wrote:
>
>>>> CONFIG_KEXEC_FILE, but still get purgatory code built in which is
>>>> totally useless.
>>>>
>>>> Not sure if I think too much over this.
>>> I see your point here, and I would suggest changing the
>>> CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY symbol to just indicate
>>> the availability of the purgatory code for the arch, rather
>>> than actually controlling the code itself. I already mentioned
>>> this for s390, but riscv would need the same thing on top.
>>>
>>> I think the change below should address your concern.
>> Since no new comment, do you mind spinning v2 to wrap all these up?
> This patchset remains in mm-hotfixes-unstable from the previous -rc
> cycle.  Eric, do you have any comments?  Arnd, do you plan on a v2?  If
> not, should I merge v1?  If so, should I now add cc:stable?

My apologies, I lost this. I've looked at these changes, and I am in 
favor of these changes.

Furthermore, I ran the following thru the Kconfig regression script, and 
did not find anything!

I believe the following patch represents the current discussion threads 
around Kconfig and KEXEC/CRASH.

Reviewed-by: Eric DeVolder <eric_devolder@yahoo.com>

Tested-by: Eric DeVolder <eric_devolder@yahoo.com>

Thanks!

eric

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..1f11a62809f2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -608,10 +608,10 @@ config ARCH_SUPPORTS_KEXEC
      def_bool PPC_BOOK3S || PPC_E500 || (44x && !SMP)

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool PPC64 && CRYPTO=y && CRYPTO_SHA256=y
+    def_bool PPC64

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild
index d25ad1c19f88..ab181d187c23 100644
--- a/arch/riscv/Kbuild
+++ b/arch/riscv/Kbuild
@@ -5,7 +5,7 @@ obj-$(CONFIG_BUILTIN_DTB) += boot/dts/
  obj-y += errata/
  obj-$(CONFIG_KVM) += kvm/

-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a..98857d76e458 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -702,9 +702,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select KEXEC_ELF

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
-    depends on CRYPTO=y
-    depends on CRYPTO_SHA256=y
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index e60fbd8660c4..3ac341d296db 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -266,7 +266,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
          cmdline = modified_cmdline;
      }

-#ifdef CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY
+#ifdef CONFIG_KEXEC_FILE
      /* Add purgatory to the image */
      kbuf.top_down = true;
      kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
@@ -280,7 +280,7 @@ static void *elf_kexec_load(struct kimage *image, 
char *kernel_buf,
                           sizeof(kernel_start), 0);
      if (ret)
          pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY */
+#endif /* CONFIG_KEXEC_FILE */

      /* Add the initrd to the image */
      if (initrd != NULL) {
diff --git a/arch/s390/Kbuild b/arch/s390/Kbuild
index a5d3503b353c..f2ce80b65551 100644
--- a/arch/s390/Kbuild
+++ b/arch/s390/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_S390_HYPFS)    += hypfs/
  obj-$(CONFIG_APPLDATA_BASE)    += appldata/
  obj-y                += net/
  obj-$(CONFIG_PCI)        += pci/
-obj-$(CONFIG_ARCH_SUPPORTS_KEXEC_PURGATORY) += purgatory/
+obj-$(CONFIG_KEXEC_FILE) += purgatory/

  # for cleaning
  subdir- += boot tools
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 3bec98d20283..d5d8f99d1f25 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -254,13 +254,13 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool MODULE_SIG_FORMAT

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_CRASH_DUMP
      def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..1566748f16c4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2072,7 +2072,7 @@ config ARCH_SUPPORTS_KEXEC
      def_bool y

  config ARCH_SUPPORTS_KEXEC_FILE
-    def_bool X86_64 && CRYPTO && CRYPTO_SHA256
+    def_bool X86_64

  config ARCH_SELECTS_KEXEC_FILE
      def_bool y
@@ -2080,7 +2080,7 @@ config ARCH_SELECTS_KEXEC_FILE
      select HAVE_IMA_KEXEC if IMA

  config ARCH_SUPPORTS_KEXEC_PURGATORY
-    def_bool KEXEC_FILE
+    def_bool y

  config ARCH_SUPPORTS_KEXEC_SIG
      def_bool y
diff --git a/kernel/Kconfig.kexec b/kernel/Kconfig.kexec
index 7aff28ded2f4..92120e396008 100644
--- a/kernel/Kconfig.kexec
+++ b/kernel/Kconfig.kexec
@@ -36,6 +36,7 @@ config KEXEC
  config KEXEC_FILE
      bool "Enable kexec file based system call"
      depends on ARCH_SUPPORTS_KEXEC_FILE
+    depends on CRYPTO_SHA256=y || !ARCH_SUPPORTS_KEXEC_PURGATORY
      select KEXEC_CORE
      help
        This is new version of kexec system call. This system call is
@@ -94,10 +95,8 @@ config KEXEC_JUMP
  config CRASH_DUMP
      bool "kernel crash dumps"
      depends on ARCH_SUPPORTS_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