x86: fold linker script pre-processing rules
diff mbox series

Message ID fd03b80e-ff1a-f2c7-20db-4604ad4d0b2f@suse.com
State New
Headers show
Series
  • x86: fold linker script pre-processing rules
Related show

Commit Message

Jan Beulich Jan. 30, 2020, 2:44 p.m. UTC
There's no need to have twice almost the same rule. Simply add the extra
-DEFI to AFLAGS for the EFI variant, and specify both targets for the
then single rule.

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

Comments

Andrew Cooper Jan. 30, 2020, 2:47 p.m. UTC | #1
On 30/01/2020 14:44, Jan Beulich wrote:
> There's no need to have twice almost the same rule. Simply add the extra
> -DEFI to AFLAGS for the EFI variant, and specify both targets for the
> then single rule.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -241,15 +241,12 @@ $(BASEDIR)/include/generated/config.h: F
>  	echo '#endif' >>$@.new
>  	$(call move-if-changed,$@.new,$@)
>  
> -xen.lds: xen.lds.S
> +xen.lds efi.lds: xen.lds.S
>  	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
>  	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
>  	mv -f .$(@F).d.new .$(@F).d
>  
> -efi.lds: xen.lds.S
> -	$(CC) -P -E -Ui386 -DEFI $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
> -	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
> -	mv -f .$(@F).d.new .$(@F).d
> +efi.lds: AFLAGS += -DEFI

I think it would be more natural to read with this line at the top,
ahead of the rule:

efi.lds: AFLAGS += -DEFI
xen.lds efi.lds: xen.lds.S
        ...

Either way, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich Jan. 30, 2020, 3:22 p.m. UTC | #2
On 30.01.2020 15:47, Andrew Cooper wrote:
> On 30/01/2020 14:44, Jan Beulich wrote:
>> There's no need to have twice almost the same rule. Simply add the extra
>> -DEFI to AFLAGS for the EFI variant, and specify both targets for the
>> then single rule.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -241,15 +241,12 @@ $(BASEDIR)/include/generated/config.h: F
>>  	echo '#endif' >>$@.new
>>  	$(call move-if-changed,$@.new,$@)
>>  
>> -xen.lds: xen.lds.S
>> +xen.lds efi.lds: xen.lds.S
>>  	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
>>  	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
>>  	mv -f .$(@F).d.new .$(@F).d
>>  
>> -efi.lds: xen.lds.S
>> -	$(CC) -P -E -Ui386 -DEFI $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
>> -	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
>> -	mv -f .$(@F).d.new .$(@F).d
>> +efi.lds: AFLAGS += -DEFI
> 
> I think it would be more natural to read with this line at the top,
> ahead of the rule:
> 
> efi.lds: AFLAGS += -DEFI
> xen.lds efi.lds: xen.lds.S
>         ...

In fact I first wanted to do it this way, then thought the EFI
special case shouldn't come earlier than the general one. But
since you ask for it ...

> Either way, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan
Wei Liu Jan. 30, 2020, 4:41 p.m. UTC | #3
On Thu, Jan 30, 2020 at 03:44:53PM +0100, Jan Beulich wrote:
> There's no need to have twice almost the same rule. Simply add the extra
> -DEFI to AFLAGS for the EFI variant, and specify both targets for the
> then single rule.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wl@xen.org>

Patch
diff mbox series

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -241,15 +241,12 @@  $(BASEDIR)/include/generated/config.h: F
 	echo '#endif' >>$@.new
 	$(call move-if-changed,$@.new,$@)
 
-xen.lds: xen.lds.S
+xen.lds efi.lds: xen.lds.S
 	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
 	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
 	mv -f .$(@F).d.new .$(@F).d
 
-efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 -DEFI $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
-	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
-	mv -f .$(@F).d.new .$(@F).d
+efi.lds: AFLAGS += -DEFI
 
 boot/mkelf32: boot/mkelf32.c
 	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<