[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v8 26/47] build,x86: remove the need for build32.mk
On Tue, Dec 21, 2021 at 02:33:18PM +0100, Jan Beulich wrote: > On 25.11.2021 14:39, Anthony PERARD wrote: > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -171,6 +171,10 @@ export LEX = $(if $(FLEX),$(FLEX),flex) > > # Default file for 'make defconfig'. > > export KBUILD_DEFCONFIG := $(ARCH)_defconfig > > > > +# Copy CFLAGS generated by "Config.mk" so they can be reused later without > > +# reparsing Config.mk by e.g. arch/x86/boot/. > > +export XEN_COMMON_CFLAGS := $(CFLAGS) > > For my own understanding (it's hard to check being half way through the > series): At this point there are no further adjustments expected to > CFLAGS? I'm not sure what you mean. The comment should be explicit. CFLAGS is going to be adjusted after the copy, for the benefit of all the 64bit code (when building x86_64). The "renamed" to XEN_CFLAGS to be useable by the rest of the tree. The name "XEN_COMMON_CFLAGS" might not be the right one, it is only common to everything in xen.git, so same flags that the toolstack would work with. > > --- a/xen/arch/x86/boot/Makefile > > +++ b/xen/arch/x86/boot/Makefile > > @@ -1,25 +1,45 @@ > > obj-bin-y += head.o > > +head-objs := cmdline.S reloc.S > > Is "-objs" really a suitable part of the name for a list of *.S? Maybe not. I think this "x-objs" naming is used in Linux to build modules with more than one object. But I guess here we have an object with more than one source. Maybe "head-srcs" is better, even if it's compiled code made ready to include into a .S source file. > > -DEFS_H_DEPS = $(abs_srctree)/$(src)/defs.h > > $(abs_srctree)/include/xen/stdbool.h > > +nocov-y += $(head-objs:.S=.o) > > +noubsan-y += $(head-objs:.S=.o) > > +targets += $(head-objs:.S=.o) > > > > -CMDLINE_DEPS = $(DEFS_H_DEPS) $(abs_srctree)/$(src)/video.h \ > > - $(BASEDIR)/include/xen/kconfig.h \ > > - $(BASEDIR)/include/generated/autoconf.h > > +head-objs := $(addprefix $(obj)/, $(head-objs)) > > > > -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 > > +$(head-objs:.S=.lnk): LDFLAGS_DIRECT := $(subst > > x86_64,i386,$(LDFLAGS_DIRECT)) > > Considering there's just a single use of LDFLAGS_DIRECT below, wouldn't > it make sense to avoid overriding the variable and doing the $(subst ...) > right at the use site instead? Yes, that might be ok to do. > > -$(obj)/cmdline.S: $(src)/cmdline.c $(CMDLINE_DEPS) $(src)/build32.lds > > - $(MAKE) -f $(abs_srctree)/$(src)/build32.mk -C $(obj) $(@F) > > CMDLINE_DEPS="$(CMDLINE_DEPS)" > > +CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_COMMON_CFLAGS)) > > I can't seem to be able to spot -march=i686 in the old code. Or wait - > Is this duplicating what config/x86_32.mk has? Yes. > > +$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS)) > > +CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float > > You did inherit -Werror and -fno-builtin from $(XEN_COMMON_CFLAGS) > already, so I don't think you need to specify these again? No, because I didn't want to change the CFLAGS used to build the 32bits binaries in this patch. So XEN_COMMON_CFLAGS just hold the CFLAGS produced by "Config.mk" for the x86_32 arch. So XEN_COMMON_CFLAGS is different from XEN_CFLAGS. If we want to use the same CFLAGS to build head.o, then I think it would be better to do in it's own patch. I can probably do it before or after this patch as XEN_CFLAGS is already available. Thanks, -- Anthony PERARD
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |