| Message ID | 20220818145616.3156379-6-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: clean up after multiplatform changes | expand |
Hi Arnd, On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Some options like CONFIG_DEBUG_UNCOMPRESS and CONFIG_CMDLINE_FORCE are > fundamentally incompatible with portable kernels but are currently allowed > in all configurations. Other options like XIP_KERNEL are essentially > useless after the completion of the multiplatform conversion. > > Repurpose the existing CONFIG_ARCH_MULTIPLATFORM option to decide > whether the resulting kernel image is meant to be portable or not, > and using this to guard all of the known incompatible options. > > This is similar to how the RISC-V kernel handles the CONFIG_NONPORTABLE > option (with the opposite polarity). > > A few references to CONFIG_ARCH_MULTIPLATFORM were left behind by > earlier clanups and have to be removed now up. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for your patch, which is now commit 84fc863606239d8b ("ARM: make ARCH_MULTIPLATFORM user-visible") in soc/for-next. > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -320,7 +320,19 @@ config ARCH_MMAP_RND_BITS_MAX > default 16 > > config ARCH_MULTIPLATFORM > - def_bool MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) > + bool "Require kernel to be portable to multiple machines" if EXPERT > + depends on MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) > + default y > + help > + In general, all Arm machines can be supported in a single > + kernel image, covering either Armv4/v5 or Armv6/v7. > + > + However, some configuration options require hardcoding machine > + specific physical addresses or enable errata workarounds that may > + break other machines. > + > + Selecting N here allows using those options, including > + DEBUG_UNCOMPRESS, XIP_KERNEL and ZBOOT_ROM. If unsure, say Y. > > menu "Platform selection" > depends on MMU > @@ -1609,6 +1621,7 @@ config CMDLINE_EXTEND > > config CMDLINE_FORCE > bool "Always use the default kernel command string" > + depends on !ARCH_MULTIPLATFORM This change broke half of the boards in my collective. Dropping this dependency again fixes the issue for me. On older platforms that boot an image with an appended DTB, or where the boot loader has no support for updating chosen/bootargs, I rely on CMDLINE_FORCE. Note that the CMDLINE choice depends on CONFIG_ATAGS=y, although my systems do not use ATAGS at all. Arm64, loongarch, microblaze, nios2, powerpc, and riscv do not have such a limitation, so perhaps that should be lifted on arm, too? I do see the rationale behind this change, and agree that a fixed command line can make the kernel unbootable on other platforms. However, a common command line is not guaranteed to cause that. E.g. all Renesas boards use the same chosen/bootargs in upstream DTS, which works fine if your DHCP server hands out proper nfsroot parameters (note that mine, running on OpenWRT, doesn't, hence my use of CMDLINE_FORCE ;-). > help > Always use the default kernel command string, even if the boot > loader passes other arguments to the kernel. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Arnd et al, On Tue, Sep 27, 2022 at 2:31 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > Some options like CONFIG_DEBUG_UNCOMPRESS and CONFIG_CMDLINE_FORCE are > > fundamentally incompatible with portable kernels but are currently allowed > > in all configurations. Other options like XIP_KERNEL are essentially > > useless after the completion of the multiplatform conversion. > > > > Repurpose the existing CONFIG_ARCH_MULTIPLATFORM option to decide > > whether the resulting kernel image is meant to be portable or not, > > and using this to guard all of the known incompatible options. > > > > This is similar to how the RISC-V kernel handles the CONFIG_NONPORTABLE > > option (with the opposite polarity). > > > > A few references to CONFIG_ARCH_MULTIPLATFORM were left behind by > > earlier clanups and have to be removed now up. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Thanks for your patch, which is now commit 84fc863606239d8b ("ARM: make > ARCH_MULTIPLATFORM user-visible") in soc/for-next. > > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -320,7 +320,19 @@ config ARCH_MMAP_RND_BITS_MAX > > default 16 > > > > config ARCH_MULTIPLATFORM > > - def_bool MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) > > + bool "Require kernel to be portable to multiple machines" if EXPERT > > + depends on MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) > > + default y > > + help > > + In general, all Arm machines can be supported in a single > > + kernel image, covering either Armv4/v5 or Armv6/v7. > > + > > + However, some configuration options require hardcoding machine > > + specific physical addresses or enable errata workarounds that may > > + break other machines. > > + > > + Selecting N here allows using those options, including > > + DEBUG_UNCOMPRESS, XIP_KERNEL and ZBOOT_ROM. If unsure, say Y. > > > > menu "Platform selection" > > depends on MMU > > @@ -1609,6 +1621,7 @@ config CMDLINE_EXTEND > > > > config CMDLINE_FORCE > > bool "Always use the default kernel command string" > > + depends on !ARCH_MULTIPLATFORM > > This change broke half of the boards in my collective. > Dropping this dependency again fixes the issue for me. > > On older platforms that boot an image with an appended DTB, or where > the boot loader has no support for updating chosen/bootargs, I rely on > CMDLINE_FORCE. > > Note that the CMDLINE choice depends on CONFIG_ATAGS=y, although > my systems do not use ATAGS at all. Arm64, loongarch, microblaze, > nios2, powerpc, and riscv do not have such a limitation, so perhaps > that should be lifted on arm, too? "[PATCH] ARM: Drop CMDLINE_* dependency on ATAGS" https://lore.kernel.org/r/09f0619e8038654d01588d9ad3a023485b2bd77f.1664285209.git.geert+renesas@glider.be > I do see the rationale behind this change, and agree that a fixed > command line can make the kernel unbootable on other platforms. > However, a common command line is not guaranteed to cause that. > E.g. all Renesas boards use the same chosen/bootargs in upstream DTS, > which works fine if your DHCP server hands out proper nfsroot > parameters (note that mine, running on OpenWRT, doesn't, hence my use > of CMDLINE_FORCE ;-). "[PATCH] ARM: Drop CMDLINE_FORCE dependency on !ARCH_MULTIPLATFORM" https://lore.kernel.org/r/c557b149780faa2299700585afc9d270ede7f78b.1664285062.git.geert+renesas@glider.be Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Arnd, On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Some options like CONFIG_DEBUG_UNCOMPRESS and CONFIG_CMDLINE_FORCE are > fundamentally incompatible with portable kernels but are currently allowed > in all configurations. Other options like XIP_KERNEL are essentially > useless after the completion of the multiplatform conversion. > > Repurpose the existing CONFIG_ARCH_MULTIPLATFORM option to decide > whether the resulting kernel image is meant to be portable or not, > and using this to guard all of the known incompatible options. > > This is similar to how the RISC-V kernel handles the CONFIG_NONPORTABLE > option (with the opposite polarity). > > A few references to CONFIG_ARCH_MULTIPLATFORM were left behind by > earlier clanups and have to be removed now up. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for your patch! > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -1904,6 +1904,7 @@ config DEBUG_UART_8250_PALMCHIP > > config DEBUG_UNCOMPRESS > bool "Enable decompressor debugging via DEBUG_LL output" > + depends on !ARCH_MULTIPLATFORM Shouldn't DEBUG_LL itself depend on !ARCH_MULTIPLATFORM instead? > depends on !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) > depends on DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \ > (!DEBUG_TEGRA_UART || !ZBOOT_ROM) && \ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Sep 27, 2022, at 3:31 PM, Geert Uytterhoeven wrote: > On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: > >> --- a/arch/arm/Kconfig.debug >> +++ b/arch/arm/Kconfig.debug >> @@ -1904,6 +1904,7 @@ config DEBUG_UART_8250_PALMCHIP >> >> config DEBUG_UNCOMPRESS >> bool "Enable decompressor debugging via DEBUG_LL output" >> + depends on !ARCH_MULTIPLATFORM > > Shouldn't DEBUG_LL itself depend on !ARCH_MULTIPLATFORM instead? > That would also be possible, but I prefer to keep limiting only the DEBUG_UNCOMPRESS. The idea of DEBUG_LL is that while it's hardwired to a particular hardware address, it does not actually access this address unless you specify the 'earlyprintk' argument on the command line. We should probably remove the earlyprintk argument from all the bootargs in the dts files though, because that somewhat defeats the purpose. Arnd
Hi Arnd, On Tue, Sep 27, 2022 at 10:19 PM Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Sep 27, 2022, at 3:31 PM, Geert Uytterhoeven wrote: > > On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: > >> --- a/arch/arm/Kconfig.debug > >> +++ b/arch/arm/Kconfig.debug > >> @@ -1904,6 +1904,7 @@ config DEBUG_UART_8250_PALMCHIP > >> > >> config DEBUG_UNCOMPRESS > >> bool "Enable decompressor debugging via DEBUG_LL output" > >> + depends on !ARCH_MULTIPLATFORM > > > > Shouldn't DEBUG_LL itself depend on !ARCH_MULTIPLATFORM instead? > > That would also be possible, but I prefer to keep limiting only > the DEBUG_UNCOMPRESS. The idea of DEBUG_LL is that while it's > hardwired to a particular hardware address, it does not actually > access this address unless you specify the 'earlyprintk' > argument on the command line. ... or unless something goes really wrong, and the kernel tries to inform the user using early_print()? Note that the I/O region for the debug serial port is mapped regardless. Any chance this can cause conflicts? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Sep 28, 2022, at 8:48 AM, Geert Uytterhoeven wrote: > On Tue, Sep 27, 2022 at 10:19 PM Arnd Bergmann <arnd@arndb.de> wrote: >> On Tue, Sep 27, 2022, at 3:31 PM, Geert Uytterhoeven wrote: >> > On Thu, Aug 18, 2022 at 4:56 PM Arnd Bergmann <arnd@kernel.org> wrote: >> >> --- a/arch/arm/Kconfig.debug >> >> +++ b/arch/arm/Kconfig.debug >> >> @@ -1904,6 +1904,7 @@ config DEBUG_UART_8250_PALMCHIP >> >> >> >> config DEBUG_UNCOMPRESS >> >> bool "Enable decompressor debugging via DEBUG_LL output" >> >> + depends on !ARCH_MULTIPLATFORM >> > >> > Shouldn't DEBUG_LL itself depend on !ARCH_MULTIPLATFORM instead? >> >> That would also be possible, but I prefer to keep limiting only >> the DEBUG_UNCOMPRESS. The idea of DEBUG_LL is that while it's >> hardwired to a particular hardware address, it does not actually >> access this address unless you specify the 'earlyprintk' >> argument on the command line. > > ... or unless something goes really wrong, and the kernel tries to > inform the user using early_print()? I don't think this matters either: without DEBUG_LL, you get a non-booting kernel and no diagnostics, while with DEBUG_LL, you might get some diagnostic if you have configured the right debug address, and otherwise the user gets the same as before: a crash without any output ;-) > Note that the I/O region for the debug serial port is mapped regardless. > Any chance this can cause conflicts? Not sure. The early debug mapping should only be used in for the earlyprintk output, but if a platform has a conflicting mapping at the same address, it just never gets used before it gets replaced. Arnd
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 3066ce82cffc..9cbac5bf8b3a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -320,7 +320,19 @@ config ARCH_MMAP_RND_BITS_MAX default 16 config ARCH_MULTIPLATFORM - def_bool MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) + bool "Require kernel to be portable to multiple machines" if EXPERT + depends on MMU && !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) + default y + help + In general, all Arm machines can be supported in a single + kernel image, covering either Armv4/v5 or Armv6/v7. + + However, some configuration options require hardcoding machine + specific physical addresses or enable errata workarounds that may + break other machines. + + Selecting N here allows using those options, including + DEBUG_UNCOMPRESS, XIP_KERNEL and ZBOOT_ROM. If unsure, say Y. menu "Platform selection" depends on MMU @@ -1609,6 +1621,7 @@ config CMDLINE_EXTEND config CMDLINE_FORCE bool "Always use the default kernel command string" + depends on !ARCH_MULTIPLATFORM help Always use the default kernel command string, even if the boot loader passes other arguments to the kernel. diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 655f84ada30f..c345775f035b 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -1904,6 +1904,7 @@ config DEBUG_UART_8250_PALMCHIP config DEBUG_UNCOMPRESS bool "Enable decompressor debugging via DEBUG_LL output" + depends on !ARCH_MULTIPLATFORM depends on !(ARCH_FOOTBRIDGE || ARCH_RPC || ARCH_SA1100) depends on DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \ (!DEBUG_TEGRA_UART || !ZBOOT_ROM) && \ diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c index 02839d8b6202..264827281113 100644 --- a/arch/arm/kernel/devtree.c +++ b/arch/arm/kernel/devtree.c @@ -194,14 +194,12 @@ const struct machine_desc * __init setup_machine_fdt(void *dt_virt) { const struct machine_desc *mdesc, *mdesc_best = NULL; -#if defined(CONFIG_ARCH_MULTIPLATFORM) || defined(CONFIG_ARM_SINGLE_ARMV7M) DT_MACHINE_START(GENERIC_DT, "Generic DT based system") .l2c_aux_val = 0x0, .l2c_aux_mask = ~0x0, MACHINE_END mdesc_best = &__mach_desc_GENERIC_DT; -#endif if (!dt_virt || !early_init_dt_verify(dt_virt)) return NULL; diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile index e83f6492834d..da373a5768ba 100644 --- a/arch/arm/mach-dove/Makefile +++ b/arch/arm/mach-dove/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-orion/include +ccflags-y := -I$(srctree)/arch/arm/plat-orion/include obj-y += common.o obj-$(CONFIG_DOVE_LEGACY) += irq.o mpp.o diff --git a/arch/arm/mach-mv78xx0/Makefile b/arch/arm/mach-mv78xx0/Makefile index a839e960b8c6..50aff70065f2 100644 --- a/arch/arm/mach-mv78xx0/Makefile +++ b/arch/arm/mach-mv78xx0/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-orion/include +ccflags-y := -I$(srctree)/arch/arm/plat-orion/include obj-y += common.o mpp.o irq.o pcie.o obj-$(CONFIG_MACH_DB78X00_BP) += db78x00-bp-setup.o diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index cb106899dd7c..c21733cbb4fa 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-orion/include +ccflags-y := -I$(srctree)/arch/arm/plat-orion/include AFLAGS_coherency_ll.o := -Wa,-march=armv7-a CFLAGS_pmsu.o := -march=armv7-a diff --git a/arch/arm/mach-orion5x/Makefile b/arch/arm/mach-orion5x/Makefile index 1a585a62d5e6..572c3520f7fe 100644 --- a/arch/arm/mach-orion5x/Makefile +++ b/arch/arm/mach-orion5x/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-orion/include +ccflags-y := -I$(srctree)/arch/arm/plat-orion/include obj-y += common.o pci.o irq.o mpp.o obj-$(CONFIG_MACH_DB88F5281) += db88f5281-setup.o