Re: [XEN PATCH v7 41/51] build,x86: remove the need for build32.mk

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.08.2021 12:50, Anthony PERARD wrote:
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,23 +1,51 @@
>  obj-bin-y += head.o
> +head-objs := cmdline.S reloc.S
>  
> -DEFS_H_DEPS = $(BASEDIR)/$(src)/defs.h $(BASEDIR)/include/xen/stdbool.h
> +nocov-y += $(head-objs:.S=.o)
> +noubsan-y += $(head-objs:.S=.o)
> +targets += $(head-objs:.S=.o)

This working right depends on targets initially getting set with := ,
because of ...

> -CMDLINE_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/$(src)/video.h \
> -            $(BASEDIR)/include/xen/kconfig.h \
> -            $(BASEDIR)/include/generated/autoconf.h
> +head-objs := $(addprefix $(obj)/, $(head-objs))

... this subsequent adjustment to the variable. Might it be more future
proof for start with

head-srcs := cmdline.S reloc.S

and then derive head-objs only here?

> -RELOC_DEPS = $(DEFS_H_DEPS) \
> -          $(BASEDIR)/include/generated/autoconf.h \
> -          $(BASEDIR)/include/xen/kconfig.h \
> -          $(BASEDIR)/include/xen/multiboot.h \
> -          $(BASEDIR)/include/xen/multiboot2.h \
> -          $(BASEDIR)/include/xen/const.h \
> -          $(BASEDIR)/include/public/arch-x86/hvm/start_info.h
> +$(obj)/head.o: $(head-objs)
>  
> -$(obj)/head.o: $(obj)/cmdline.S $(obj)/reloc.S
> +LDFLAGS_DIRECT_OpenBSD = _obsd
> +LDFLAGS_DIRECT_FreeBSD = _fbsd

This is somewhat ugly - it means needing to change things in two places
when config/x86_32.mk would change (e.g. to add another build OS). How
about ...

> +$(head-objs:.S=.lnk): LDFLAGS_DIRECT := -melf_i386$(LDFLAGS_DIRECT_$(XEN_OS))

... instead:

$(head-objs:.S=.lnk): LDFLAGS_DIRECT := $(subst x86_64,i386,$(LDFLAGS_DIRECT))

? Or if deemed still too broad

$(head-objs:.S=.lnk): LDFLAGS_DIRECT := $(subst 
elf_x86_64,elf_i386,$(LDFLAGS_DIRECT))

?

> -$(obj)/cmdline.S: $(src)/cmdline.c $(CMDLINE_DEPS) $(src)/build32.lds
> -     $(MAKE) -f $(BASEDIR)/$(src)/build32.mk -C $(obj) $(@F) 
> CMDLINE_DEPS="$(CMDLINE_DEPS)"
> +CFLAGS_x86_32 := -m32 -march=i686
> +CFLAGS_x86_32 += -fno-strict-aliasing
> +CFLAGS_x86_32 += -std=gnu99
> +CFLAGS_x86_32 += -Wall -Wstrict-prototypes
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wdeclaration-after-statement)
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wno-unused-but-set-variable)
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wno-unused-local-typedefs)
> +$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float
> +CFLAGS_x86_32 += -I$(srctree)/include

I'm afraid I'm not convinced that having to keep this in sync with the
original is in fair balance with the removal of build32.mk.

Jan