Re: [XEN PATCH v9 23/30] build,x86: remove the need for build32.mk

From: Jan Beulich <jbeulich@suse.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH v9 23/30] build,x86: remove the need for build32.mk
Date: Thu, 3 Mar 2022 11:29:36 +0100	[thread overview]
Message-ID: <2d0bed19-bf52-e934-0a5a-c424ff6e4db0@suse.com> (raw)
In-Reply-To: <20220125110103.3527686-24-anthony.perard@citrix.com>

On 25.01.2022 12:00, Anthony PERARD wrote:
> Rework "arch/x86/boot/Makefile" to allow it to build both file
> "cmdline.S" and "reloc.S" without "build32.mk".
> 
> These will now use the main rules for "%.o: %.c", and thus generate a
> dependency file. (We will not need to track the dependency manually
> anymore.)
> 
> But for that, we need to override the main CFLAGS to do a 32bit build.
> We introduce XEN_TREEWIDE_CFLAGS which can be reused in boot/Makefile,
> and avoid the need to reparse Config.mk with a different value for
> XEN_TARGET_ARCH. From this new $(XEN_TREEWIDE_CFLAGS), we only need to
> change -m64 to have the 32bit flags. Then those are applied only to
> "cmdline.o" and "reloc.o".
> 
> Specifically apply the rule "%.S: %.bin" to both cmdline.S and reloc.S
> to avoid make trying to regenerate other %.S files with it.
> 
> There is no change expected to the resulting "cmdline.S" and
> "reloc.S", only the *.o file changes as their symbol for FILE goes
> from "cmdline.c" to "arch/x86//cmdline.c". (No idea why "boot" is
> missing from the string.) (I've only check with GCC, not clang.)
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one question, just to be sure I understand things right:

> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,25 +1,42 @@
>  obj-bin-y += head.o
> +head-srcs := cmdline.S reloc.S
>  
> -DEFS_H_DEPS = $(abs_srctree)/$(src)/defs.h $(abs_srctree)/include/xen/stdbool.h
> +nocov-y += $(head-srcs:.S=.o)
> +noubsan-y += $(head-srcs:.S=.o)
> +targets += $(head-srcs:.S=.o)
>  
> -CMDLINE_DEPS = $(DEFS_H_DEPS) $(abs_srctree)/$(src)/video.h \
> -	       $(BASEDIR)/include/xen/kconfig.h \
> -	       $(BASEDIR)/include/generated/autoconf.h
> +head-srcs := $(addprefix $(obj)/, $(head-srcs))
>  
> -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-srcs)
>  
> -$(obj)/head.o: $(obj)/cmdline.S $(obj)/reloc.S
> +CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
> +$(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
>  
> -$(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)"
> +# override for 32bit binaries
> +$(head-srcs:.S=.o): CFLAGS_stack_boundary :=

You overriding CFLAGS_stack_boundary but not object_label_flags is
merely because the latter has no (unwanted) effect on the compilation?

Jan