All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
To: Grygorii Strashko <grygorii_strashko@epam.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>,
	"Michal Orzel" <michal.orzel@amd.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Anthony PERARD" <anthony.perard@vates.tech>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jason Andryuk" <jason.andryuk@amd.com>
Subject: Re: [XEN][PATCH] common/libfdt: optimize usage
Date: Mon, 17 Nov 2025 15:34:10 +0100	[thread overview]
Message-ID: <DEB1LPKJSO4H.23SQXSLYDAW1K@amd.com> (raw)
In-Reply-To: <20251114180130.346755-1-grygorii_strashko@epam.com>

On Fri Nov 14, 2025 at 7:01 PM CET, 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

This is a nice idea, and the less we compile of this nonsense library the
better.

>
> 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.

We likely want a DEVICE_TREE_WRITER Kconfig, or some such. That would compile
in all code that performs DTB writes and gate the DTB overlay. Not having it
restricts libfdt to fdt.c and fdt_ro.c, and having it adds fdt_rw, and allows
the others depending on EFI and/or overlay settings.

But the helpers themselves are not terribly well organized, so I'd say it's
not strictly required for now.

>
>  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

Only on UEFI? That's surprising. Why does this entry point need more than
creating a DTB for a domU in dom0less?

>  	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

Out of curiosity, what's the relationship between those libfdt files and the
helpers? What do they use them for?

It's not like LIBFDT_WIP or LIBFDT_SW have very insighful names (through no
fault of your own. The original file organization is dubious at best), so why
not just compile those files conditionally on DOMAIN_BUILD_HELPERS instead?

Something like...

>  
>  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

... turning these two into:

  obj-libfdt-$(CONFIG_DOMAIN_BUILD_HELPERS) += fdt_sw.o fdt_wip.o

Likewise for fdt_empty_tree.o and fdt_overlay.o for ARM_EFI and OVERLAY_DTB
respectively.

> +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)

Cheers,
Alejandro


  parent reply	other threads:[~2025-11-17 14:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 18:01 [XEN][PATCH] common/libfdt: optimize usage Grygorii Strashko
2025-11-17  9:31 ` Jan Beulich
2025-11-17  9:34   ` Jan Beulich
2025-11-21 10:59     ` Grygorii Strashko
2025-11-21 11:28       ` Jan Beulich
2025-11-18 19:43   ` Grygorii Strashko
2025-11-17 14:34 ` Alejandro Vallejo [this message]
2025-11-21 11:01 ` Grygorii Strashko
2025-12-03 13:12 ` Andrew Cooper
2025-12-03 14:12   ` Nicola Vetrini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DEB1LPKJSO4H.23SQXSLYDAW1K@amd.com \
    --to=alejandro.garciavallejo@amd.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=bertrand.marquis@arm.com \
    --cc=grygorii_strashko@epam.com \
    --cc=jason.andryuk@amd.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.