| Message ID | 20220717200407.1400318-1-thomas.petazzoni@bootlin.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arch: remove support for BR2_ARC_PAGE_SIZE_4K | expand |
Hi Thomas, Please let me take a look at this one. The point is we do use 4 KiB page for building for the ARCv3 processors, see https://github.com/foss-for-synopsys-dwc-arc-processors/buildroot/blob/arc64/arch/Config.in.arc#L133. And so far I haven't seen any problems with either uClibc or glibc. And since I don't immediately see any changes in Binutils for ARCv3 which touch "common-page-size", I'd like to figure out what goes wrong here. I'll get back to you soon with some explanation of what we see here. -Alexey
Hello Alexey, On Sun, 17 Jul 2022 21:13:08 +0000 Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Please let me take a look at this one. > > The point is we do use 4 KiB page for building for the ARCv3 processors, > see https://github.com/foss-for-synopsys-dwc-arc-processors/buildroot/blob/arc64/arch/Config.in.arc#L133. > And so far I haven't seen any problems with either uClibc or glibc. > > And since I don't immediately see any changes in Binutils for ARCv3 which touch > "common-page-size", I'd like to figure out what goes wrong here. > > I'll get back to you soon with some explanation of what we see here. Thanks for your feedback. Will definitely wait to hear from you before merging this patch. Thanks! Thomas
Hi Thomas, Claudiu, > On Sun, 17 Jul 2022 21:13:08 +0000 > Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > > Please let me take a look at this one. > > > > The point is we do use 4 KiB page for building for the ARCv3 processors, > > see https://urldefense.com/v3/__https://github.com/foss-for-synopsys-dwc-arc-processors/buildroot/blob/arc64/arch/Config.in.arc*L133__;Iw!!A4F2R9G_pg!cxR1QodBjZ2_XZsfTSe3ThcFYm_X7BN7ERYq6smx9IqcZZUrUzXD2gEF1R_oFrKNb0SrguAeGg-kt6hFvkHhbcQpTs3QXHvksOs$ . > > And so far I haven't seen any problems with either uClibc or glibc. > > > > And since I don't immediately see any changes in Binutils for ARCv3 which touch > > "common-page-size", I'd like to figure out what goes wrong here. > > > > I'll get back to you soon with some explanation of what we see here. > > Thanks for your feedback. Will definitely wait to hear from you before > merging this patch. So that seems to be a good catch indeed. 1. In older GNU LD such a check doesn't exist and so LD never craps out. And here I mean the latest ARC Binutils ("arc-2020.09-release"), which are based on upstream v2.35. Thus if building with 4KiB pages but ARC Binutils, the problem is not seen. I'm sure even built that way binaries work perfectly fine as I do remember trying it not that long ago myself. BTW that pre-defined "common-page-size" comes from GCC actually, see https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/arc/linux.h;h=0f1ff055be757ad1483910fc4918e7fef7fe4d5e;hb=1ea978e3066ac565a1ec28a96a4d61eaf38e2726#l48 2. And in case of building for ARCv3 processors we don't mess with "page-size" stuff in the "LINK_SPEC", see https://github.com/foss-for-synopsys-dwc-arc-processors/gcc/blob/arc64/gcc/config/arc64/linux32.h#L26 That said I'd like to get our GCC & Binutils maintainer input on what needs to be done with these "page-size" options in the "LINK_SPEC". Maybe with use of up-to-date Binutils those default values are no longer needed? Or we may keep default "max" value and then not explicitly set outside "common" value will be adjusted as needed automatically. See https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/ldelf.c;h=bfa0d54753ad8a7de02b4bae999119588ef982c6;hb=refs/heads/master#l90 -Alexey
Hello Alexey, On Tue, 19 Jul 2022 18:20:25 +0000 Alexey Brodkin via buildroot <buildroot@buildroot.org> wrote: > So that seems to be a good catch indeed. > > 1. In older GNU LD such a check doesn't exist and so LD never craps out. > And here I mean the latest ARC Binutils ("arc-2020.09-release"), which are based > on upstream v2.35. Thus if building with 4KiB pages but ARC Binutils, > the problem is not seen. I'm sure even built that way binaries work perfectly > fine as I do remember trying it not that long ago myself. > > BTW that pre-defined "common-page-size" comes from GCC actually, see > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/arc/linux.h;h=0f1ff055be757ad1483910fc4918e7fef7fe4d5e;hb=1ea978e3066ac565a1ec28a96a4d61eaf38e2726#l48 OK. > 2. And in case of building for ARCv3 processors we don't mess with "page-size" > stuff in the "LINK_SPEC", see https://github.com/foss-for-synopsys-dwc-arc-processors/gcc/blob/arc64/gcc/config/arc64/linux32.h#L26 OK, but I don't really see the relationship. > That said I'd like to get our GCC & Binutils maintainer input on what needs > to be done with these "page-size" options in the "LINK_SPEC". > > Maybe with use of up-to-date Binutils those default values are no longer needed? Why no longer needed? Of course is you change the page size, you need to pass some options to the linker so that it can place sections and define load addresses (and other related stuff) accordingly. > Or we may keep default "max" value and then not explicitly set outside "common" > value will be adjusted as needed automatically. I don't understand what you mean here. Today, we are setting max-page-size, and that doesn't work because the common-page-size value is higher. As I mentioned in my initial patch, passing -Wl,-z,max-page-size=4096 (already done) and -Wl,-z,common-page-size=4096 would most likely fix the error, but I wasn't sure it was the correct thing to do. Do you have more feedback from your GCC and binutils maintainer perhaps? Thanks a lot! Thomas
Hello, Re-adding Buildroot mailing list in Cc. On Thu, 28 Jul 2022 20:25:47 +0000 Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > I don't understand what you mean here. Today, we are setting > > max-page-size, and that doesn't work because the common-page-size value > > is higher. As I mentioned in my initial patch, passing > > -Wl,-z,max-page-size=4096 (already done) and > > -Wl,-z,common-page-size=4096 would most likely fix the error, but I > > wasn't sure it was the correct thing to do. > > So, right, suggested change of explicit setup of "common-page-size=max-page-size" > is a good fix as it is compatible with older Binutils (before the change > referred above, i.e. before 2.38). Could you provide the patch? Keep in mind that this block of code: # Explicitly set LD's "max-page-size" instead of relying on some defaults ifeq ($(BR2_ARC_PAGE_SIZE_4K)$(BR2_ARM64_PAGE_SIZE_4K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=4096 else ifeq ($(BR2_ARC_PAGE_SIZE_8K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=8192 else ifeq ($(BR2_ARC_PAGE_SIZE_16K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384 else ifeq ($(BR2_ARM64_PAGE_SIZE_64K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=65536 endif in arch/arch.mk, has been recently modified to be used for configurable page sizes on ARM64. So, if we decide to add -Wl,-z,common-page-size=XYZ, it also needs to work for ARM64 (I think it should, but I just want to warn that this code is no longer ARC-specific). Thanks! Thomas
Hello Alexey, On Wed, 3 Aug 2022 19:22:29 +0200 Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > So, right, suggested change of explicit setup of "common-page-size=max-page-size" > > is a good fix as it is compatible with older Binutils (before the change > > referred above, i.e. before 2.38). > > Could you provide the patch? > > Keep in mind that this block of code: > > # Explicitly set LD's "max-page-size" instead of relying on some defaults > ifeq ($(BR2_ARC_PAGE_SIZE_4K)$(BR2_ARM64_PAGE_SIZE_4K),y) > ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=4096 > else ifeq ($(BR2_ARC_PAGE_SIZE_8K),y) > ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=8192 > else ifeq ($(BR2_ARC_PAGE_SIZE_16K),y) > ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384 > else ifeq ($(BR2_ARM64_PAGE_SIZE_64K),y) > ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=65536 > endif > > in arch/arch.mk, has been recently modified to be used for configurable > page sizes on ARM64. So, if we decide to add > -Wl,-z,common-page-size=XYZ, it also needs to work for ARM64 (I think > it should, but I just want to warn that this code is no longer > ARC-specific). Do you think you will be able to provide a patch to fix support for 4K page size support on ARC in Buildroot? It is not really a major feature of Buildroot, and it's causing some build failures in our autobuilders, so if no fix is provided, we'll probably end up dropping support, as proposed by my patch posted on July 17. Thanks! Thomas
diff --git a/Config.in.legacy b/Config.in.legacy index 956da10a9f..1590a9fa25 100644 --- a/Config.in.legacy +++ b/Config.in.legacy @@ -146,6 +146,13 @@ endif comment "Legacy options removed in 2022.08" +config BR2_ARC_PAGE_SIZE_4K + bool "Support for 4KB pages on ARC removed" + select BR2_LEGACY + help + The support to use 4KB pages on the ARC architecture has + been removed, as it was broken with both glibc and uClibc. + config BR2_PACKAGE_PHP_EXT_WDDX bool "php wddx removed" select BR2_LEGACY diff --git a/arch/Config.in.arc b/arch/Config.in.arc index 388d3496bc..58103ad171 100644 --- a/arch/Config.in.arc +++ b/arch/Config.in.arc @@ -103,10 +103,6 @@ choice size matching the hardware configuration. Otherwise user-space applications will fail at runtime. -config BR2_ARC_PAGE_SIZE_4K - bool "4KB" - depends on !BR2_arc750d - config BR2_ARC_PAGE_SIZE_8K bool "8KB" help @@ -121,7 +117,6 @@ endchoice config BR2_ARC_PAGE_SIZE string - default "4K" if BR2_ARC_PAGE_SIZE_4K default "8K" if BR2_ARC_PAGE_SIZE_8K default "16K" if BR2_ARC_PAGE_SIZE_16K diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc index 32b818b0e0..697c0ff524 100644 --- a/arch/arch.mk.arc +++ b/arch/arch.mk.arc @@ -6,9 +6,7 @@ ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic endif # Explicitly set LD's "max-page-size" instead of relying on some defaults -ifeq ($(BR2_ARC_PAGE_SIZE_4K),y) -ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=4096 -else ifeq ($(BR2_ARC_PAGE_SIZE_8K),y) +ifeq ($(BR2_ARC_PAGE_SIZE_8K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=8192 else ifeq ($(BR2_ARC_PAGE_SIZE_16K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384 diff --git a/linux/linux.mk b/linux/linux.mk index 322ccabbd9..f012689a9e 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -362,10 +362,6 @@ define LINUX_KCONFIG_FIXUP_CMDS $(call KCONFIG_ENABLE_OPT,CONFIG_AEABI)) $(if $(BR2_powerpc)$(BR2_powerpc64)$(BR2_powerpc64le), $(call KCONFIG_ENABLE_OPT,CONFIG_PPC_DISABLE_WERROR)) - $(if $(BR2_ARC_PAGE_SIZE_4K), - $(call KCONFIG_ENABLE_OPT,CONFIG_ARC_PAGE_SIZE_4K) - $(call KCONFIG_DISABLE_OPT,CONFIG_ARC_PAGE_SIZE_8K) - $(call KCONFIG_DISABLE_OPT,CONFIG_ARC_PAGE_SIZE_16K)) $(if $(BR2_ARC_PAGE_SIZE_8K), $(call KCONFIG_DISABLE_OPT,CONFIG_ARC_PAGE_SIZE_4K) $(call KCONFIG_ENABLE_OPT,CONFIG_ARC_PAGE_SIZE_8K)
BR2_ARC_PAGE_SIZE_4K=y is broken with both glibc and uClibc. The link of the C library fails with: ld: common page size (0x2000) > maximum page size (0x1000) This is due to BR2_ARC_PAGE_SIZE_4K=y settings -Wl,-z,max-page-size=4096, but binutils apparently defaulting to common-page-size=8192. While this could potentially be fixed by passing -Wl,-z,common-page-size=4096, these failures indicates that nobody has tested 4KB page sizes on ARC in the context of Buildroot, so it is more sensible to drop support for this feature. Should anybody need this, and have the ability to ensure that it works, we would certainly be able to re-introduce the feature. Fixes: http://autobuild.buildroot.net/results/c8b2f331c98453670cd982558144c4fd84674a3d/ (uclibc) http://autobuild.buildroot.net/results/3a22f7aac38145b26c549254b819f87329e7a77e/ (glibc) Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- Config.in.legacy | 7 +++++++ arch/Config.in.arc | 5 ----- arch/arch.mk.arc | 4 +--- linux/linux.mk | 4 ---- 4 files changed, 8 insertions(+), 12 deletions(-)