1. Patchew
  2. Xen
  3. View series

[PATCH] build: centralize / unify asm-offsets generation

Jan Beulich posted 1 patch 3 years ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/fa340caa-4ee1-a302-fbf1-1df400493d6b@suse.com
[PATCH] build: centralize / unify asm-offsets generation
Posted by Jan Beulich 3 years ago 
Except for an additional prereq Arm and x86 have the same needs here,
and Arm can also benefit from the recent x86 side improvement. Recurse
into arch/*/ only for a phony include target (doing nothing on Arm),
and handle asm-offsets itself entirely locally to xen/Makefile.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/.gitignore
+++ b/.gitignore
@@ -318,7 +318,6 @@
 xen/arch/x86/efi/check.efi
 xen/arch/x86/efi/mkreloc
 xen/arch/*/xen.lds
-xen/arch/*/asm-offsets.s
 xen/arch/*/efi/boot.c
 xen/arch/*/efi/compat.c
 xen/arch/*/efi/ebmalloc.c
@@ -325,6 +324,7 @@
 xen/arch/*/efi/efi.h
 xen/arch/*/efi/pe.c
 xen/arch/*/efi/runtime.c
+xen/asm-offsets.s
 xen/common/config_data.S
 xen/common/config.gz
 xen/include/headers*.chk
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -341,7 +341,7 @@ _clean: delete-unfresh-files
 	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
 		-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
-	rm -f include/asm-*/asm-offsets.h
+	rm -f asm-offsets.s include/asm-*/asm-offsets.h
 	rm -f .banner
 
 .PHONY: _distclean
@@ -362,7 +362,7 @@ $(TARGET): delete-unfresh-files
 		done; \
 		true
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C include
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) asm-offsets.s
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/asm-$(TARGET_ARCH)/asm-offsets.h
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
 
@@ -399,7 +399,11 @@ include/xen/compile.h: include/xen/compi
 	@sed -rf tools/process-banner.sed < .banner >> $@.new
 	@mv -f $@.new $@
 
-include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
+asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
+	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
+	$(call move-if-changed,$@.new,$@)
+
+include/asm-$(TARGET_ARCH)/asm-offsets.h: asm-offsets.s
 	@(set -e; \
 	  echo "/*"; \
 	  echo " * DO NOT MODIFY."; \
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -131,8 +131,8 @@ $(TARGET)-syms: prelink.o xen.lds
 		>$(@D)/$(@F).map
 	rm -f $(@D)/.$(@F).[0-9]*
 
-asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
-	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
+.PHONY: include
+include:
 
 xen.lds: xen.lds.S
 	$(CPP) -P $(a_flags) -MQ $@ -o $@ $<
@@ -141,6 +141,6 @@ dtb.o: $(CONFIG_DTB_FILE)
 
 .PHONY: clean
 clean::
-	rm -f asm-offsets.s xen.lds
+	rm -f xen.lds
 	rm -f $(BASEDIR)/.xen-syms.[0-9]*
 	rm -f $(TARGET).efi
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -244,9 +244,8 @@ endif
 efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
 efi/buildid.o efi/relocs-dummy.o: ;
 
-asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
-	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
-	$(call move-if-changed,$@.new,$@)
+.PHONY: include
+include: $(BASEDIR)/include/asm-x86/asm-macros.h
 
 asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
 
@@ -276,7 +275,7 @@ hweight.o: CFLAGS-y += $(foreach reg,cx
 
 .PHONY: clean
 clean::
-	rm -f asm-offsets.s *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
+	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
 	rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
 	rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
 	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
Re: [PATCH] build: centralize / unify asm-offsets generation
Posted by Julien Grall 3 years ago 
Hi Jan,

On 01/04/2021 09:33, Jan Beulich wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -341,7 +341,7 @@ _clean: delete-unfresh-files
>   	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
>   		-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
>   	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
> -	rm -f include/asm-*/asm-offsets.h
> +	rm -f asm-offsets.s include/asm-*/asm-offsets.h
>   	rm -f .banner
>   
>   .PHONY: _distclean
> @@ -362,7 +362,7 @@ $(TARGET): delete-unfresh-files
>   		done; \
>   		true
>   	$(MAKE) -f $(BASEDIR)/Rules.mk -C include
> -	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) asm-offsets.s
> +	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include
>   	$(MAKE) -f $(BASEDIR)/Rules.mk include/asm-$(TARGET_ARCH)/asm-offsets.h
>   	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
>   
> @@ -399,7 +399,11 @@ include/xen/compile.h: include/xen/compi
>   	@sed -rf tools/process-banner.sed < .banner >> $@.new
>   	@mv -f $@.new $@
>   
> -include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
> +asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
> +	$(call move-if-changed,$@.new,$@)
> +
> +include/asm-$(TARGET_ARCH)/asm-offsets.h: asm-offsets.s
>   	@(set -e; \
>   	  echo "/*"; \
>   	  echo " * DO NOT MODIFY."; \
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -131,8 +131,8 @@ $(TARGET)-syms: prelink.o xen.lds
>   		>$(@D)/$(@F).map
>   	rm -f $(@D)/.$(@F).[0-9]*
>   
> -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
> -	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
> +.PHONY: include
> +include:

 From a generic PoV, this target is meant to generate arch-specific 
include. Is that correct?

Cheers,

-- 
Julien Grall
Re: [PATCH] build: centralize / unify asm-offsets generation
Posted by Jan Beulich 3 years ago 
On 01.04.2021 17:05, Julien Grall wrote:
> On 01/04/2021 09:33, Jan Beulich wrote:
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -131,8 +131,8 @@ $(TARGET)-syms: prelink.o xen.lds
>>   		>$(@D)/$(@F).map
>>   	rm -f $(@D)/.$(@F).[0-9]*
>>   
>> -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>> -	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
>> +.PHONY: include
>> +include:
> 
>  From a generic PoV, this target is meant to generate arch-specific 
> include. Is that correct?

Yes, like is happening on x86 already.

Jan
Ping: [PATCH] build: centralize / unify asm-offsets generation
Posted by Jan Beulich 3 years ago 
On 01.04.2021 10:33, Jan Beulich wrote:
> Except for an additional prereq Arm and x86 have the same needs here,
> and Arm can also benefit from the recent x86 side improvement. Recurse
> into arch/*/ only for a phony include target (doing nothing on Arm),
> and handle asm-offsets itself entirely locally to xen/Makefile.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments / acks, anyone?

Thanks, Jan

> --- a/.gitignore
> +++ b/.gitignore
> @@ -318,7 +318,6 @@
>  xen/arch/x86/efi/check.efi
>  xen/arch/x86/efi/mkreloc
>  xen/arch/*/xen.lds
> -xen/arch/*/asm-offsets.s
>  xen/arch/*/efi/boot.c
>  xen/arch/*/efi/compat.c
>  xen/arch/*/efi/ebmalloc.c
> @@ -325,6 +324,7 @@
>  xen/arch/*/efi/efi.h
>  xen/arch/*/efi/pe.c
>  xen/arch/*/efi/runtime.c
> +xen/asm-offsets.s
>  xen/common/config_data.S
>  xen/common/config.gz
>  xen/include/headers*.chk
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -341,7 +341,7 @@ _clean: delete-unfresh-files
>  	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
>  		-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
>  	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
> -	rm -f include/asm-*/asm-offsets.h
> +	rm -f asm-offsets.s include/asm-*/asm-offsets.h
>  	rm -f .banner
>  
>  .PHONY: _distclean
> @@ -362,7 +362,7 @@ $(TARGET): delete-unfresh-files
>  		done; \
>  		true
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C include
> -	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) asm-offsets.s
> +	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include
>  	$(MAKE) -f $(BASEDIR)/Rules.mk include/asm-$(TARGET_ARCH)/asm-offsets.h
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
>  
> @@ -399,7 +399,11 @@ include/xen/compile.h: include/xen/compi
>  	@sed -rf tools/process-banner.sed < .banner >> $@.new
>  	@mv -f $@.new $@
>  
> -include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
> +asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
> +	$(call move-if-changed,$@.new,$@)
> +
> +include/asm-$(TARGET_ARCH)/asm-offsets.h: asm-offsets.s
>  	@(set -e; \
>  	  echo "/*"; \
>  	  echo " * DO NOT MODIFY."; \
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -131,8 +131,8 @@ $(TARGET)-syms: prelink.o xen.lds
>  		>$(@D)/$(@F).map
>  	rm -f $(@D)/.$(@F).[0-9]*
>  
> -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
> -	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
> +.PHONY: include
> +include:
>  
>  xen.lds: xen.lds.S
>  	$(CPP) -P $(a_flags) -MQ $@ -o $@ $<
> @@ -141,6 +141,6 @@ dtb.o: $(CONFIG_DTB_FILE)
>  
>  .PHONY: clean
>  clean::
> -	rm -f asm-offsets.s xen.lds
> +	rm -f xen.lds
>  	rm -f $(BASEDIR)/.xen-syms.[0-9]*
>  	rm -f $(TARGET).efi
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -244,9 +244,8 @@ endif
>  efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o
>  efi/buildid.o efi/relocs-dummy.o: ;
>  
> -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
> -	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
> -	$(call move-if-changed,$@.new,$@)
> +.PHONY: include
> +include: $(BASEDIR)/include/asm-x86/asm-macros.h
>  
>  asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
>  
> @@ -276,7 +275,7 @@ hweight.o: CFLAGS-y += $(foreach reg,cx
>  
>  .PHONY: clean
>  clean::
> -	rm -f asm-offsets.s *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
> +	rm -f *.lds *.new boot/*.o boot/*~ boot/core boot/mkelf32
>  	rm -f asm-macros.i $(BASEDIR)/include/asm-x86/asm-macros.*
>  	rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d $(BASEDIR)/.xen.elf32
>  	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/mkreloc
>
Re: [PATCH] build: centralize / unify asm-offsets generation
Posted by Roger Pau Monné 3 years ago 
On Thu, Apr 01, 2021 at 10:33:47AM +0200, Jan Beulich wrote:
> Except for an additional prereq Arm and x86 have the same needs here,
> and Arm can also benefit from the recent x86 side improvement. Recurse
> into arch/*/ only for a phony include target (doing nothing on Arm),
> and handle asm-offsets itself entirely locally to xen/Makefile.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/.gitignore
> +++ b/.gitignore
> @@ -318,7 +318,6 @@
>  xen/arch/x86/efi/check.efi
>  xen/arch/x86/efi/mkreloc
>  xen/arch/*/xen.lds
> -xen/arch/*/asm-offsets.s
>  xen/arch/*/efi/boot.c
>  xen/arch/*/efi/compat.c
>  xen/arch/*/efi/ebmalloc.c
> @@ -325,6 +324,7 @@
>  xen/arch/*/efi/efi.h
>  xen/arch/*/efi/pe.c
>  xen/arch/*/efi/runtime.c
> +xen/asm-offsets.s
>  xen/common/config_data.S
>  xen/common/config.gz
>  xen/include/headers*.chk
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -341,7 +341,7 @@ _clean: delete-unfresh-files
>  	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
>  		-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
>  	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
> -	rm -f include/asm-*/asm-offsets.h
> +	rm -f asm-offsets.s include/asm-*/asm-offsets.h
>  	rm -f .banner
>  
>  .PHONY: _distclean
> @@ -362,7 +362,7 @@ $(TARGET): delete-unfresh-files
>  		done; \
>  		true
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C include
> -	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) asm-offsets.s
> +	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include
>  	$(MAKE) -f $(BASEDIR)/Rules.mk include/asm-$(TARGET_ARCH)/asm-offsets.h
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
>  
> @@ -399,7 +399,11 @@ include/xen/compile.h: include/xen/compi
>  	@sed -rf tools/process-banner.sed < .banner >> $@.new
>  	@mv -f $@.new $@
>  
> -include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
> +asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
> +	$(call move-if-changed,$@.new,$@)

Won't it be more natural to keep the .s file in arch/$(TARGET_ARCH)?

Thanks, Roger.
Re: [PATCH] build: centralize / unify asm-offsets generation
Posted by Jan Beulich 3 years ago 
On 20.04.2021 17:29, Roger Pau Monné wrote:
> On Thu, Apr 01, 2021 at 10:33:47AM +0200, Jan Beulich wrote:
>> @@ -399,7 +399,11 @@ include/xen/compile.h: include/xen/compi
>>  	@sed -rf tools/process-banner.sed < .banner >> $@.new
>>  	@mv -f $@.new $@
>>  
>> -include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
>> +asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
>> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
>> +	$(call move-if-changed,$@.new,$@)
> 
> Won't it be more natural to keep the .s file in arch/$(TARGET_ARCH)?

Yes and no: Yes as far as the actual file location is concerned.
No when considering where it gets generated: I generally consider
it risky to generate files outside of the directory where make
currently runs. There may be good reasons for certain exceptions,
but personally I don't see this case as good enough a reason.

Somewhat related - if doing as you suggest, which Makefile's
clean: rule should clean up that file in your opinion?

Nevertheless, if there's general agreement that keeping the file
there is better, I'd make the change and simply ignore my unhappy
feelings about it.

Jan

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.