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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 2025-12-03 14:12, Andrew Cooper wrote:
> On 14/11/2025 6:01 pm, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>> 
>> 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@epam.com>
>> ---
>> 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