On Thu, Oct 14, 2021 at 10:32:05AM +0200, Jan Beulich wrote: > 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 ...
They are because Rules.mk initialise those variables that way. > > -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? I'll give it a try. > > -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)) > > ? Sounds good, I'll do that. > > -$(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. Why would this needs to be kept in sync with the original? I would actually like to get rid of Config.mk, I don't see the point in sharing CFLAGS between a kernel and userspace binaries. Also, I hope one day that we can have the hypervisor in its own repo, and thus they won't be any Config.mk to keep in sync with. The goal of this patch is less about removing build32.mk, and more about using the rest of the build system infrastructure to build those few 32bits objects, and been able to use the automatic dependencies generation and other things without having to duplicate them. It probably make the build a bit faster as there is two less invocation of $(MAKE) (which parse Config.mk). As for a possible change: instead of having those x86_32 flags in arch/x86/boot/, I could have them in arch/x86/arch.mk. Linux does something like that were it prepare REALMODE_CFLAGS. I know it's a bit annoying to have another list x86_32 cflags, but I don't see how we can extract them from Config.mk (in a Makefile where we want to use both x86_32 and x86_64 flags). Thanks, -- Anthony PERARD
