[PATCH 0/2] powerpc: tweak the kernel options for CRASH_DUMP and RELOCATABLE

[Kevin Hao]

Hi,

The first patch fixes a build error when CRASH_DUMP=y && ADVANCED_OPTIONS=n
for ppc32. The second does some cleanup for RELOCATABLE option.

Kevin Hao (2):
powerpc: fix the dependency issue for CRASH_DUMP
powerpc: merge the RELOCATABLE config entries for ppc32 and ppc64

arch/powerpc/Kconfig | 68 +++++++++++++++---------------------
arch/powerpc/configs/ppc64_defconfig | 1 +
2 files changed, 29 insertions(+), 40 deletions(-)

[Kevin Hao]

In the current code, the RELOCATABLE will be forcedly enabled when
enabling CRASH_DUMP. But for ppc32, the RELOCABLE also depend on
ADVANCED_OPTIONS and select NONSTATIC_KERNEL. This will cause build
error when CRASH_DUMP=y && ADVANCED_OPTIONS=n. Even there is no such
issue for ppc64, but select is only for non-visible symbols and for
symbols with no dependencies. As for a symbol like RELOCATABLE, it is
definitely not suitable to select it. So choose to depend on it.

Also enable the RELOCATABLE explicitly for the defconfigs which has
CRASH_DUMP enabled.

Signed-off-by: Kevin Hao <***@gmail.com>
---
arch/powerpc/Kconfig | 3 +--
arch/powerpc/configs/ppc64_defconfig | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 190cc48abc0c..d6bbf4f6f869 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -429,8 +429,7 @@ config KEXEC

config CRASH_DUMP
bool "Build a kdump crash kernel"
- depends on PPC64 || 6xx || FSL_BOOKE || (44x && !SMP)
- select RELOCATABLE if (PPC64 && !COMPILE_TEST) || 44x || FSL_BOOKE
+ depends on 6xx || ((PPC64 || FSL_BOOKE || (44x && !SMP)) && RELOCATABLE)
help
Build a kernel suitable for use as a kdump capture kernel.
The same kernel binary can be used as production kernel and dump
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index aad501ae3834..01f7b63f2df0 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -46,6 +46,7 @@ CONFIG_BINFMT_MISC=m
CONFIG_PPC_TRANSACTIONAL_MEM=y
CONFIG_KEXEC=y
CONFIG_CRASH_DUMP=y
+CONFIG_RELOCATABLE=y
CONFIG_IRQ_ALL_CPUS=y
CONFIG_MEMORY_HOTREMOVE=y
CONFIG_KSM=y

[Scott Wood]

Post by Kevin Hao
In the current code, the RELOCATABLE will be forcedly enabled when
enabling CRASH_DUMP. But for ppc32, the RELOCABLE also depend on
ADVANCED_OPTIONS and select NONSTATIC_KERNEL. This will cause build
error when CRASH_DUMP=y && ADVANCED_OPTIONS=n. Even there is no such
issue for ppc64, but select is only for non-visible symbols and for
symbols with no dependencies. As for a symbol like RELOCATABLE, it is
definitely not suitable to select it. So choose to depend on it.

Why is it "definitely not suitable to select it", provided the
ADVANCED_OPTIONS dependency is removed, and the FLATMEM dependency is
moved to places that select RELOCATABLE? It seems wrong that the user
should have to enable ADVANCED_OPTIONS to even see the option to build a
crash kernel.

-Scott

[Kevin Hao]

Post by Scott Wood

Post by Kevin Hao
In the current code, the RELOCATABLE will be forcedly enabled when
enabling CRASH_DUMP. But for ppc32, the RELOCABLE also depend on
ADVANCED_OPTIONS and select NONSTATIC_KERNEL. This will cause build
error when CRASH_DUMP=y && ADVANCED_OPTIONS=n. Even there is no such
issue for ppc64, but select is only for non-visible symbols and for
symbols with no dependencies. As for a symbol like RELOCATABLE, it is
definitely not suitable to select it. So choose to depend on it.

Why is it "definitely not suitable to select it", provided the
ADVANCED_OPTIONS dependency is removed, and the FLATMEM dependency is
moved to places that select RELOCATABLE?

Even with this change, the definition of RELOCATABLE still be something like
this:
config RELOCATABLE
bool "Build a relocatable kernel"
depends on (PPC64 && !COMPILE_TEST) || 44x || FSL_BOOKE
select NONSTATIC_KERNEL

Quoted form Documentation/kbuild/kconfig-language.txt:
select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.

So it is always error prone to select a kernel option like this.

Post by Scott Wood
It seems wrong that the user
should have to enable ADVANCED_OPTIONS to even see the option to build a
crash kernel.

Yes, it seems ridiculous. But this is fixed in the patch 2.

Thanks,
Kevin

[Scott Wood]

Post by Kevin Hao

Post by Scott Wood

Post by Kevin Hao
In the current code, the RELOCATABLE will be forcedly enabled when
enabling CRASH_DUMP. But for ppc32, the RELOCABLE also depend on
ADVANCED_OPTIONS and select NONSTATIC_KERNEL. This will cause build
error when CRASH_DUMP=y && ADVANCED_OPTIONS=n. Even there is no such
issue for ppc64, but select is only for non-visible symbols and for
symbols with no dependencies. As for a symbol like RELOCATABLE, it is
definitely not suitable to select it. So choose to depend on it.

Why is it "definitely not suitable to select it", provided the
ADVANCED_OPTIONS dependency is removed, and the FLATMEM dependency is
moved to places that select RELOCATABLE?

Even with this change, the definition of RELOCATABLE still be something like
config RELOCATABLE
bool "Build a relocatable kernel"
depends on (PPC64 && !COMPILE_TEST) || 44x || FSL_BOOKE
select NONSTATIC_KERNEL

That matches the cases where CRASH_DUMP selects RELOCATABLE.

Post by Kevin Hao
select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.
So it is always error prone to select a kernel option like this.

Yes, but these days Kbuild does warn about selecting a symbol with unmet
dependencies, which IIRC wasn't the case when that was written.

Post by Kevin Hao

Post by Scott Wood
It seems wrong that the user
should have to enable ADVANCED_OPTIONS to even see the option to build a
crash kernel.

Yes, it seems ridiculous. But this is fixed in the patch 2.

OK... Still non-obvious, but at least not *as* bad.

-Scott

[Kevin Hao]

It makes no sense to keep two separate RELOCATABLE config entries for
ppc32 and ppc64 respectively. Merge them into one and move it to
a common place. The dependency on ADVANCED_OPTIONS for ppc32 seems
unnecessary, also drop it.

Signed-off-by: Kevin Hao <***@gmail.com>
---
arch/powerpc/Kconfig | 65 ++++++++++++++++++++++------------------------------
1 file changed, 27 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d6bbf4f6f869..4080a14707bb 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -427,6 +427,33 @@ config KEXEC
interface is strongly in flux, so no good recommendation can be
made.

+config RELOCATABLE
+ bool "Build a relocatable kernel"
+ depends on (PPC64 && !COMPILE_TEST) || (FLATMEM && (44x || FSL_BOOKE))
+ select NONSTATIC_KERNEL
+ help
+ This builds a kernel image that is capable of running at the
+ location the kernel is loaded at. For ppc32, there is no any
+ alignment restrictions, and this feature is a superset of
+ DYNAMIC_MEMSTART and hence overrides it. For ppc64, we should use
+ 16k-aligned base address. The kernel is linked as a
+ position-independent executable (PIE) and contains dynamic relocations
+ which are processed early in the bootup process.
+
+ One use is for the kexec on panic case where the recovery kernel
+ must live at a different physical address than the primary
+ kernel.
+
+ Note: If CONFIG_RELOCATABLE=y, then the kernel runs from the address
+ it has been loaded at and the compile time physical addresses
+ CONFIG_PHYSICAL_START is ignored. However CONFIG_PHYSICAL_START
+ setting can still be useful to bootwrappers that need to know the
+ load address of the kernel (eg. u-boot/mkimage).
+
+config RELOCATABLE_PPC32
+ def_bool y
+ depends on PPC32 && RELOCATABLE
+
config CRASH_DUMP
bool "Build a kdump crash kernel"
depends on 6xx || ((PPC64 || FSL_BOOKE || (44x && !SMP)) && RELOCATABLE)
@@ -926,29 +953,6 @@ config DYNAMIC_MEMSTART

This option is overridden by CONFIG_RELOCATABLE

-config RELOCATABLE
- bool "Build a relocatable kernel"
- depends on ADVANCED_OPTIONS && FLATMEM && (44x || FSL_BOOKE)
- select NONSTATIC_KERNEL
- help
- This builds a kernel image that is capable of running at the
- location the kernel is loaded at, without any alignment restrictions.
- This feature is a superset of DYNAMIC_MEMSTART and hence overrides it.
-
- One use is for the kexec on panic case where the recovery kernel
- must live at a different physical address than the primary
- kernel.
-
- Note: If CONFIG_RELOCATABLE=y, then the kernel runs from the address
- it has been loaded at and the compile time physical addresses
- CONFIG_PHYSICAL_START is ignored. However CONFIG_PHYSICAL_START
- setting can still be useful to bootwrappers that need to know the
- load address of the kernel (eg. u-boot/mkimage).
-
-config RELOCATABLE_PPC32
- def_bool y
- depends on PPC32 && RELOCATABLE
-
config PAGE_OFFSET_BOOL
bool "Set custom page offset address"
depends on ADVANCED_OPTIONS
@@ -1034,21 +1038,6 @@ config PIN_TLB
endmenu

if PPC64
-config RELOCATABLE
- bool "Build a relocatable kernel"
- depends on !COMPILE_TEST
- select NONSTATIC_KERNEL
- help
- This builds a kernel image that is capable of running anywhere
- in the RMA (real memory area) at any 16k-aligned base address.
- The kernel is linked as a position-independent executable (PIE)
- and contains dynamic relocations which are processed early
- in the bootup process.
-
- One use is for the kexec on panic case where the recovery kernel
- must live at a different physical address than the primary
- kernel.
-
# This value must have zeroes in the bottom 60 bits otherwise lots will break
config PAGE_OFFSET
hex