Re: [XEN PATCH v8 27/47] build: grab common EFI source files in arch specific dir

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.11.2021 14:39, Anthony PERARD wrote:
> Rather than preparing the efi source file, we will make the symbolic
> link as needed from the build location.
> 
> The `ln` command is run every time to allow to update the link in case
> the source tree change location.

Btw, since symlinks aren't being liked, would there be a way to make
things work using vpath?

> This patch also introduce "efi_common.mk" which allow to reuse the
> common make instructions without having to duplicate them into each
> arch.
> 
> And now that we have a list of common source file, we can start to
> remove the links to the source files on clean.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> 
> Notes:
>     v8:
>     - use symbolic link instead of making a copy of the source
>     - introduce efi_common.mk
>     - remove links to source file on clean
>     - use -iquote for "efi.h" headers in common/efi
> 
>  xen/Makefile                 |  5 -----
>  xen/arch/arm/efi/Makefile    |  4 ++--
>  xen/arch/x86/Makefile        |  1 +
>  xen/arch/x86/efi/Makefile    |  5 +----
>  xen/common/efi/efi_common.mk | 12 ++++++++++++

Could I talk you into avoiding _ when - is suitable, which is the case not
only for (non-exported) make variables, but also file names?

> --- a/xen/arch/arm/efi/Makefile
> +++ b/xen/arch/arm/efi/Makefile
> @@ -1,4 +1,4 @@
> -CFLAGS-y += -fshort-wchar
> +include $(srctree)/common/efi/efi_common.mk
>  
> -obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o
> +obj-y += $(EFIOBJ-y)
>  obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index e8151bf4b111..eabd8d3919a4 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -79,6 +79,7 @@ endif
>  
>  # Allows "clean" to descend into boot/
>  subdir- += boot
> +subdir- += efi

This renders the comment stale - please generalize it.

Also, any reason a similar adjustment isn't needed for Arm? Perhaps
this could even move into xen/Makefile:

subdir- += $(wildcard efi/)

> --- /dev/null
> +++ b/xen/common/efi/efi_common.mk
> @@ -0,0 +1,12 @@
> +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o
> +EFIOBJ-$(CONFIG_COMPAT) += compat.o
> +
> +CFLAGS-y += -fshort-wchar
> +CFLAGS-y += -iquote $(srctree)/common/efi
> +
> +$(obj)/%.c: $(abs_srctree)/common/efi/%.c FORCE
> +     $(Q)ln -nfs $< $@

Like was the case before, I think it would be better if the links were
relative ones, at least when srctree == objtree (but ideally always).

> +clean-files += $(patsubst %.o,%.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-))

Nit: Please be consistent (at least within a single line) about blanks
following commas.

Jan