[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAQEx52BYMYfNu+xj8sNmdtH9XfPapdhJDrsbDo43aD3Dg@mail.gmail.com>
Date: Sun, 22 Sep 2024 17:14:12 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Stephen Boyd <sboyd@...nel.org>
Cc: Andrea della Porta <andrea.porta@...e.com>, Andrew Lunn <andrew@...n.ch>, Arnd Bergmann <arnd@...db.de>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Catalin Marinas <catalin.marinas@....com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
Conor Dooley <conor+dt@...nel.org>, "David S. Miller" <davem@...emloft.net>,
Derek Kiernan <derek.kiernan@....com>, Dragan Cvetic <dragan.cvetic@....com>,
Eric Dumazet <edumazet@...gle.com>, Florian Fainelli <florian.fainelli@...adcom.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jakub Kicinski <kuba@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Lee Jones <lee@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>, Michael Turquette <mturquette@...libre.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>, Stefan Wahren <wahrenst@....net>,
Will Deacon <will@...nel.org>, devicetree@...r.kernel.org, linux-arch@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
netdev@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH 05/11] vmlinux.lds.h: Preserve DTB sections from being
discarded after init
On Sun, Sep 22, 2024 at 5:47 AM Stephen Boyd <sboyd@...nel.org> wrote:
>
> Quoting Andrea della Porta (2024-09-03 05:29:18)
> > On 12:46 Fri 30 Aug , Stephen Boyd wrote:
> > > Quoting Andrea della Porta (2024-08-20 07:36:07)
> > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > > > index ad6afc5c4918..3ae9097774b0 100644
> > > > --- a/include/asm-generic/vmlinux.lds.h
> > > > +++ b/include/asm-generic/vmlinux.lds.h
> > >
> > > It would be nice to keep the initdata properties when this isn't used
> > > after init as well. Perhaps we need another macro and/or filename to
> > > indicate that the DTB{O} can be thrown away after init/module init.
> >
> > We can certainly add some more filename extension that would place the
> > relevant data in a droppable section.
> > Throwing away the dtb/o after init is like the actual KERNEL_DTB macro that
> > is adding teh data to section .init.data, but this would mean t would be
> > useful only at very early init stage, just like for CONFIG_OF_UNITTEST.
> > Throwing after module init could be more difficult though, I think,
> > for example we're not sure when to discard the section in case of deferred
> > modules probe.
> >
>
> This patch can fix a modpost warning seen in linux-next because I have
> added DT overlays from KUnit tests while kbuild has properly marked the
> overlay as initdata that is discarded. See [1] for details. In KUnit I
> doubt this really matters because most everything runs from __init code
> (even if it isn't marked that way).
>
> I'm thinking that we need to make dtbo Makefile target put the blob in
> the rodata section so it doesn't get thrown away and leave the builtin
> DTB as part of init.rodata. Did you already do that? I see the kbuild
> tree has removed the commit that caused the warning, but I suspect this
> may still be a problem. See [2] for the next series where overlays
> applied in the test happen from driver probe so __ref is added.
>
> If we simply copy the wrap command and make it so that overlays always
> go to the .rodata section we should be good. Maybe there's some way to
> figure out what is being wrapped so we don't have to copy the whole
> thing.
>
> Finally, it's unfortunate that the DTBO is copied when an overlay is
> applied. We'll waste memory after this patch, so of_overlay_fdt_apply()
> could be taught to reuse the blob passed in instead of copying it.
>
> -----8<----
> diff --git a/scripts/Makefile.dtbs b/scripts/Makefile.dtbs
> index 55998b878e54..070e08082cd3 100644
> --- a/scripts/Makefile.dtbs
> +++ b/scripts/Makefile.dtbs
> @@ -51,11 +51,25 @@ quiet_cmd_wrap_S_dtb = WRAP $@
> echo '.balign STRUCT_ALIGNMENT'; \
> } > $@
>
> +quiet_cmd_wrap_S_dtbo = WRAP $@
> + cmd_wrap_S_dtbo = { \
> + symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*)); \
> + echo '\#include <asm-generic/vmlinux.lds.h>'; \
> + echo '.section .rodata,"a"'; \
> + echo '.balign STRUCT_ALIGNMENT'; \
> + echo ".global $${symbase}_begin"; \
> + echo "$${symbase}_begin:"; \
> + echo '.incbin "$<" '; \
> + echo ".global $${symbase}_end"; \
> + echo "$${symbase}_end:"; \
> + echo '.balign STRUCT_ALIGNMENT'; \
> + } > $@
> +
> $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
> $(call if_changed,wrap_S_dtb)
>
> $(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE
> - $(call if_changed,wrap_S_dtb)
> + $(call if_changed,wrap_S_dtbo)
>
> # Schema check
> # ---------------------------------------------------------------------------
>
> [1] https://lore.kernel.org/all/20240909112728.30a9bd35@canb.auug.org.au/
> [2] https://lore.kernel.org/all/20240910094459.352572-1-masahiroy@kernel.org/
Rather, I'd modify my patch as follows:
--- a/scripts/Makefile.dtbs
+++ b/scripts/Makefile.dtbs
@@ -34,12 +34,14 @@ $(obj)/dtbs-list: $(dtb-y) FORCE
# Assembly file to wrap dtb(o)
# ---------------------------------------------------------------------------
+builtin-dtb-section = $(if $(filter arch/%, $(obj)),.dtb.init.rodata,.rodata)
+
# Generate an assembly file to wrap the output of the device tree compiler
quiet_cmd_wrap_S_dtb = WRAP $@
cmd_wrap_S_dtb = { \
symbase=__$(patsubst .%,%,$(suffix $<))_$(subst -,_,$(notdir $*)); \
echo '\#include <asm-generic/vmlinux.lds.h>'; \
- echo '.section .dtb.init.rodata,"a"'; \
+ echo '.section $(builtin-dtb-section),"a"'; \
echo '.balign STRUCT_ALIGNMENT'; \
echo ".global $${symbase}_begin"; \
echo "$${symbase}_begin:"; \
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists