diff mbox series

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

Message ID 20220125110103.3527686-24-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements, now with out-of-tree build! | expand

Commit Message

Anthony PERARD Jan. 25, 2022, 11 a.m. UTC 
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>
---

Notes:
    v9:
    - rename XEN_COMMON_CFLAGS to XEN_TREEWIDE_CFLAGS
    - rename $(head-objs) to $(head-srcs)
    - substitute LDFLAGS_DIRECT in %.lnk rule, rather than changing the value
      of it.
    
    v8:
    - avoid the need to list CFLAGS from Config.mk a second time by
      introducing XEN_COMMON_CFLAGS, and using it in boot/
    - improve LDFLAGS_DIRECT, by just substitute x64 to i368 from x86
      LDFLAGS_DIRECT. And thus avoid copying the logic from Config.mk.

 xen/Makefile                 |  4 +++
 xen/arch/x86/boot/Makefile   | 49 ++++++++++++++++++++++++------------
 xen/arch/x86/boot/build32.mk | 40 -----------------------------
 3 files changed, 37 insertions(+), 56 deletions(-)
 delete mode 100644 xen/arch/x86/boot/build32.mk

Comments

Jan Beulich March 3, 2022, 10:29 a.m. UTC | #1 
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
Anthony PERARD March 3, 2022, 3:31 p.m. UTC | #2 
On Thu, Mar 03, 2022 at 11:29:36AM +0100, Jan Beulich wrote:
> 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?

Yes.

Thanks,
diff mbox series

Patch

diff --git a/xen/Makefile b/xen/Makefile
index dc25fa443b82..8baa260b93a7 100644
--- 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_TREEWIDE_CFLAGS := $(CFLAGS)
+
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 CLANG_FLAGS :=
diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 1ac8cb435e0e..ca8001c72b23 100644
--- 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 :=
+$(head-srcs:.S=.o): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
 
-$(obj)/reloc.S: $(src)/reloc.c $(RELOC_DEPS) $(src)/build32.lds
-	$(MAKE) -f $(abs_srctree)/$(src)/build32.mk -C $(obj) $(@F) RELOC_DEPS="$(RELOC_DEPS)"
+$(head-srcs): %.S: %.bin
+	(od -v -t x $< | tr -s ' ' | awk 'NR > 1 {print s} {s=$$0}' | \
+	sed 's/ /,0x/g' | sed 's/,0x$$//' | sed 's/^[0-9]*,/ .long /') >$@
+
+# Drop .got.plt during conversion to plain binary format.
+# Please check build32.lds for more details.
+%.bin: %.lnk
+	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | \
+		while read idx name sz rest; do \
+			case "$$name" in \
+			.got.plt) \
+				test $$sz != 0c || continue; \
+				echo "Error: non-empty $$name: 0x$$sz" >&2; \
+				exit $$(expr $$idx + 1);; \
+			esac; \
+		done
+	$(OBJCOPY) -O binary -R .got.plt $< $@
+
+%.lnk: %.o $(src)/build32.lds
+	$(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
 
 clean-files := cmdline.S reloc.S *.lnk *.bin
diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk
deleted file mode 100644
index e90680cd9f52..000000000000
--- a/xen/arch/x86/boot/build32.mk
+++ /dev/null
@@ -1,40 +0,0 @@ 
-override XEN_TARGET_ARCH=x86_32
-CFLAGS =
-include $(XEN_ROOT)/Config.mk
-
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-
-CFLAGS += -Werror -fno-builtin -g0 -msoft-float
-CFLAGS += -I$(BASEDIR)/include
-CFLAGS := $(filter-out -flto,$(CFLAGS)) 
-
-# NB. awk invocation is a portable alternative to 'head -n -1'
-%.S: %.bin
-	(od -v -t x $< | tr -s ' ' | awk 'NR > 1 {print s} {s=$$0}' | \
-	sed 's/ /,0x/g' | sed 's/,0x$$//' | sed 's/^[0-9]*,/ .long /') >$@
-
-# Drop .got.plt during conversion to plain binary format.
-# Please check build32.lds for more details.
-%.bin: %.lnk
-	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | \
-		while read idx name sz rest; do \
-			case "$$name" in \
-			.got.plt) \
-				test $$sz != 0c || continue; \
-				echo "Error: non-empty $$name: 0x$$sz" >&2; \
-				exit $$(expr $$idx + 1);; \
-			esac; \
-		done
-	$(OBJCOPY) -O binary -R .got.plt $< $@
-
-%.lnk: %.o build32.lds
-	$(LD) $(LDFLAGS_DIRECT) -N -T build32.lds -o $@ $<
-
-%.o: %.c
-	$(CC) $(CFLAGS) -c -fpic $< -o $@
-
-cmdline.o: cmdline.c $(CMDLINE_DEPS)
-
-reloc.o: reloc.c $(RELOC_DEPS)
-
-.PRECIOUS: %.bin %.lnk