[Buildroot] [PATCH] arch/arc: Explicitly set "max-page-size" for GNU LD
Yann E. MORIN
yann.morin.1998 at free.fr
Thu Dec 26 17:05:04 UTC 2019
Thomas, Alexey, All,
On 2019-12-25 22:36 +0100, Thomas Petazzoni spake thusly:
> On Tue, 24 Dec 2019 17:10:32 +0300
> Alexey Brodkin <Alexey.Brodkin at synopsys.com> wrote:
> > Back in the day we relied on a default value that used to be 8KiB
> > and it worked perfectly fine for ARC's default 8KiB page as well as
> > 4 KiB ones, but not for 16 KiB, see [1] for more details.
> >
> > So that we fixed by setting "max-page-size" if 16KiB pages are in use by
> > commit d024d369b82d2 ("arch/arc: Accommodate 16 KiB MMU pages").
> >
> > But as Yann very rightfully mentioned here [2] we should be setting this
> > thing explicitly for all page sizes because:
> > 1. Defaults might change unexpectedly
> > 2. Explicitly set stuff is better understood
> > 3. We act similarly to all settings but not only addressing some corner cases
> >
> > [1] https://git.buildroot.org/buildroot/commit/?id=d024d369b82d2d3d9d4d75489c19e9488202bca0
> > [2] https://patchwork.ozlabs.org/patch/1212544/#2330647
> >
> > Signed-off-by: Alexey Brodkin <abrodkin at synopsys.com>
> > Cc: Yann E. MORIN <yann.morin.1998 at free.fr>
>
> > -# By default MAXPAGESIZE for ARC is 8192 so for larger MMU pages
> > -# it needs to be overridden.
> > +# 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
> > +endif
> > +
> > +ifeq ($(BR2_ARC_PAGE_SIZE_8K),y)
> > +ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=8192
> > +endif
> > +
> > ifeq ($(BR2_ARC_PAGE_SIZE_16K),y)
> > ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384
> > endif
>
> Since these options are mutually exclusive:
>
> 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)
> 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
> endif
>
> is a bit more compact, so I changed to use this before committing.
Agreed.
> Also, we already have an option called BR2_ARC_PAGE_SIZE, which
> contains 4K, 8K or 16K, so I thought if we could use it here as well. I
> think we can, but I'd like to hear from Yann about this, because it
> uses bash arithmetic, which I'm not sure we're allowed to use:
>
> ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=$(shell echo $$(($(patsubst %K,%,$(BR2_ARC_PAGE_SIZE)) * 1024)))
We can do bash arithmetics, because we are guaranteed we use bash as our
shell, see: https://git.buildroot.org/buildroot/tree/Makefile#n30
But BR2_ARC_PAGE_SIZE is a string, so it is double-quoted, which would
break the above.
I was also contemplating a similar solution, but I was not too happy
with this trick to drop the suffix. I would have prefered something
like:
config BR2_ARC_PAGE_SIZE_KB
int
default 4 if BR2_ARC_PAGE_SIZE_4K
default 8 if BR2_ARC_PAGE_SIZE_8K
default 16 if BR2_ARC_PAGE_SIZE_16K
and then:
ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=$(shell echo $$(($(BR2_ARC_PAGE_SIZE)*1024)))
and in package/uclibc/uclibc.mk:
UCLIBC_ARC_PAGE_SIZE = CONFIG_ARC_PAGE_SIZE_$(BR2_ARC_PAGE_SIZE)K
... and even that intermediate variable could have been dropped
altogether, to directly write:
$(call KCONFIG_ENABLE_OPT,CONFIG_ARC_PAGE_SIZE_$(BR2_ARC_PAGE_SIZE)K,$(@D)/.config)
And that would have been a bit nicer, I believe.
Regards,
Yann E. MORIN.
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
More information about the buildroot
mailing list