On Sun, Sep 22, 2024 at 5:47 AM Stephen Boyd <sboyd@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxx/
> [2] https://lore.kernel.org/all/20240910094459.352572-1-masahiroy@xxxxxxxxxx/
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
