Re: [XEN][PATCH] common/libfdt: optimize usage

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-12-03 14:12, Andrew Cooper wrote:
> On 14/11/2025 6:01 pm, Grygorii Strashko wrote:
> > From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> >
> > Now all libfdt features are built-it unconditionally, but...
> >
> > X86: The libfdt is used on x86 only to parse Hyperlaunch/dom0less Xen
> > nodes, so full libfdt is not needed in this case and minimal, RO
> > configuration can be used.
> >
> > ARM - situation is more complicated:
> > 1) ARM reads Host DT (fdt.c RO)
> > 2) ARM reads passthrough DT (RO)
> > 3) ARM generates dom0/hwdom DT from Host DT (there is a mix of WIP and 
> > SW APIs)
> > 4) ARM generates domU DT (there is a mix of WIP and SW APIs)
> > 4) With EFI enabled - ARM needs RW API and fdt_empty_tree
> > 5) With CONFIG_OVERLAY_DTB - ARM needs RW and fdt_overlay API
> >
> > Hence, add possibility for optimizing libfdt usage by introducing 
> > separate
> > Kconfig options for each libfdt feature and select them where needed.
> >
> > Following libfdt modules are not used after this change:
> >  Makefile.libfdt
> >  fdt_addresses.c
> >  fdt_strerror.c
> >  fdt_check.c
> >
> > Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> > ---
> > Not sure about using DOMAIN_BUILD_HELPERS for selecting
> > LIBFDT features, as DOMAIN_BUILD_HELPERS doesn't exactly
> > says that domain's DT will be generated when selected.
> >
> >  xen/arch/arm/Kconfig       |  4 ++++
> >  xen/common/Kconfig         |  4 ++++
> >  xen/common/libfdt/Kconfig  | 14 ++++++++++++++
> >  xen/common/libfdt/Makefile | 12 +++++++++---
> >  4 files changed, 31 insertions(+), 3 deletions(-)
> >  create mode 100644 xen/common/libfdt/Kconfig
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index cf6af68299f6..f10cd3d7effc 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -111,6 +111,8 @@ config ARM_EFI
> >         bool "UEFI boot service support"
> >         depends on ARM_64 && !MPU
> >         default y
> > +       select LIBFDT_RW
> > +       select LIBFDT_EMPTY_TREE
> >         help
> >           This option provides support for boot services through
> >           UEFI firmware. A UEFI stub is provided to allow Xen to
> > @@ -149,6 +151,8 @@ config HAS_ITS
> >  config OVERLAY_DTB
> >         bool "DTB overlay support (UNSUPPORTED)" if UNSUPPORTED
> >         depends on SYSCTL
> > +       select LIBFDT_RW
> > +       select LIBFDT_OVERLAY
> >         help
> >           Dynamic addition/removal of Xen device tree nodes using a dtbo.
> >
> > diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> > index 401d5046f6f5..256aff269c3b 100644
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -28,6 +28,8 @@ config DOM0LESS_BOOT
> >
> >  config DOMAIN_BUILD_HELPERS
> >         bool
> > +       select LIBFDT_WIP
> > +       select LIBFDT_SW
> >
> >  config GRANT_TABLE
> >         bool "Grant table support" if EXPERT
> > @@ -680,4 +682,6 @@ config PM_STATS
> >           Enable collection of performance management statistics to aid in
> >  	  analyzing and tuning power/performance characteristics of the 
> > system
> > +source "common/libfdt/Kconfig"
> > +
> >  endmenu
> > diff --git a/xen/common/libfdt/Kconfig b/xen/common/libfdt/Kconfig
> > new file mode 100644
> > index 000000000000..3abd904b2969
> > --- /dev/null
> > +++ b/xen/common/libfdt/Kconfig
> > @@ -0,0 +1,14 @@
> > +config LIBFDT_WIP
> > +       bool
> > +
> > +config LIBFDT_SW
> > +    bool
> > +
> > +config LIBFDT_RW
> > +    bool
> > +
> > +config LIBFDT_EMPTY_TREE
> > +    bool
> > +
> > +config LIBFDT_OVERLAY
> > +    bool
> > diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
> > index 6ce679f98f47..c832d1849a5c 100644
> > --- a/xen/common/libfdt/Makefile
> > +++ b/xen/common/libfdt/Makefile
> > @@ -1,7 +1,13 @@
> > -include $(src)/Makefile.libfdt
> >
> >  SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
> >
> > +obj-libfdt-y := fdt.o fdt_ro.o
> > +obj-libfdt-$(CONFIG_LIBFDT_WIP) += fdt_wip.o
> > +obj-libfdt-$(CONFIG_LIBFDT_SW) += fdt_sw.o
> > +obj-libfdt-$(CONFIG_LIBFDT_RW) += fdt_rw.o
> > +obj-libfdt-$(CONFIG_LIBFDT_EMPTY_TREE) += fdt_empty_tree.o
> > +obj-libfdt-$(CONFIG_LIBFDT_OVERLAY) += fdt_overlay.o
> > +
> >  # For CONFIG_OVERLAY_DTB, libfdt functionalities will be needed 
> > during runtime.
> >  ifneq ($(CONFIG_OVERLAY_DTB),y)
> >  OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section 
> > .$(s)=.init.$(s))
> > @@ -15,7 +21,7 @@ CFLAGS-y += -I$(srctree)/include/xen/libfdt/
> >  $(obj)/libfdt.o: $(obj)/libfdt-temp.o FORCE
> >         $(call if_changed,objcopy)
> >
> > -$(obj)/libfdt-temp.o: $(addprefix $(obj)/,$(LIBFDT_OBJS)) FORCE
> > +$(obj)/libfdt-temp.o: $(addprefix $(obj)/,$(obj-libfdt-y)) FORCE
> >         $(call if_changed,ld)
> >
> > -targets += libfdt-temp.o $(LIBFDT_OBJS)
> > +targets += libfdt-temp.o $(obj-libfdt-y)
> Pulling together several aspects.
>
> Now that we have xen/lib, library stuff should be in it, including this
> libfdt.  I suggest moving it to xen/lib/fdt.
>
> The build system problems are created by using non-standard rules to
> bodge the init-ness.  For livepatches, we have `init_or_livepatch` as
> friends to do conditional init-ness.  I'd suggest a similar approach 
> here.
> You might want a prompt-less CONFIG_LIBFDT_NONINIT which can be 
> selected
> by CONFIG_DTB_OVERLAY, because that's going to be better than trying to
> make an implication directly about DTB_OVERLAY.
>
>
> As to other issues hinted at:
>
> * Init coverage.  The only reason we don't have init coverage is 
> because
> of the overly-restrictive SPECIAL_DATA_SECTIONS check, and while it
> serves a purpose, it does a lot of harm too.  It should be disabled by
> things like CONFIG_COVERAGE so that we can retrieve coverage of boot
> time paths, and because data placement is not interesting for these
> types of builds.
>
> * -f{function,data}-sections and --gc-sections.  This gets dead
> code/data elimination with better granularity than the translation 
> unit,
> and removes the need for interior ifdefary to achieve the same 
> savings. 
> We already have -f*-sections for livepatching, so this is really just
> using --gc-sections and will probably net a good win in one fell swoop.
>
> ~Andrew
I didn't check, but moving libfdt code might entail doing some 
replacements in ECLAIR deviations to keep guidelines clean, since there 
are a few assumptions about paths there and in 
"docs/misra/exclude-list.json", which is used also by ECLAIR.
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253