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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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