Re: [PATCH v4] arm: Add Kconfig entry to select CONFIG_DTB_FILE

[Julien Grall <julien@xen.org>]

Hi Michal,

On 10/03/2021 06:58, Michal Orzel wrote:
> Currently in order to link existing DTB into Xen image
> we need to either specify option CONFIG_DTB_FILE on the
> command line or manually add it into .config.
> Add Kconfig entry: CONFIG_DTB_FILE to be able to
> provide the path to DTB we want to embed into Xen image.
> If no path provided - the dtb will not be embedded.
> 
> Remove the line: AFLAGS-y += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
> as it is not needed since Kconfig will define it in a header
> with all the other config options.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
>   xen/arch/arm/Makefile | 5 ++---
>   xen/common/Kconfig    | 8 ++++++++
>   2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 16e6523e2c..46e6a95fec 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -68,9 +68,8 @@ extra-y += $(TARGET_SUBARCH)/head.o
>   
>   #obj-bin-y += ....o
>   
> -ifdef CONFIG_DTB_FILE
> +ifneq ($(CONFIG_DTB_FILE),"")
>   obj-y += dtb.o
> -AFLAGS-y += -DCONFIG_DTB_FILE=\"$(CONFIG_DTB_FILE)\"
>   endif
>   
>   ALL_OBJS := $(TARGET_SUBARCH)/head.o $(ALL_OBJS)
> @@ -137,7 +136,7 @@ asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>   xen.lds: xen.lds.S
>   	$(CPP) -P $(a_flags) -MQ $@ -o $@ $<
>   
> -dtb.o: $(CONFIG_DTB_FILE)
> +dtb.o: $(patsubst "%",%,$(CONFIG_DTB_FILE))
>   
>   .PHONY: clean
>   clean::
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index eb953d171e..a27836bf47 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -400,6 +400,14 @@ config DOM0_MEM
>   
>   	  Leave empty if you are not sure what to specify.
>   
> +config DTB_FILE
> +	string "Absolute path to device tree blob"
> +	depends on HAS_DEVICE_TREE
> +	---help---
> +	  When using a bootloader that has no device tree support or when there
> +	  is no bootloader at all, use this option to specify the absolute path
> +	  to a device tree that will be linked directly inside Xen binary.

With this approach, CONFIG_DTB_FILE will always be defined. This means 
that Xen will always be compiled to use the "embedded" DTB. When the 
string is "", it will be garbagge.

So I think we need a second config to that indicates whether the string 
is empty or not.

Interestingly, your first version of patch didn't expose the problem 
because CONFIG_DTB_FILE would not be defined if the CONFIG_LINK_DTB is 
not selected. Although, it would still happily build if CONFIG_DTB_FILE 
is "".

Cheers,

-- 
Julien Grall