1. Patchew
  2. Xen
  3. View series

[PATCH] x86/build: correctly record dependencies of asm-offsets.s

Jan Beulich posted 1 patch 3 years, 2 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b3b57f6b-3ed9-18f6-2a87-6af3304c6645@suse.com
[PATCH] x86/build: correctly record dependencies of asm-offsets.s
Posted by Jan Beulich 3 years, 2 months ago 
Going through an intermediate *.new file requires telling the compiler
what the real target is, so that the inclusion of the resulting .*.d
file will actually be useful.

Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
Reported-by: Julien Grall <julien@xen.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Already on the original patch I did suggest that perhaps Arm would want
to follow suit. So again - perhaps the rules should be unified by moving
to common code?

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -241,7 +241,7 @@ efi/buildid.o efi/relocs-dummy.o: $(BASE
 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 $<
+	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
 	$(call move-if-changed,$@.new,$@)
 
 asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s
Posted by Julien Grall 3 years, 2 months ago 
Hi Jan,

On 01/02/2021 14:56, Jan Beulich wrote:
> Going through an intermediate *.new file requires telling the compiler
> what the real target is, so that the inclusion of the resulting .*.d
> file will actually be useful.
> 
> Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
> Reported-by: Julien Grall <julien@xen.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Already on the original patch I did suggest that perhaps Arm would want
> to follow suit. So again - perhaps the rules should be unified by moving
> to common code?

Sorry I missed the original patch. The recent changes looks beneficials 
to Arm as well.

> 
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -241,7 +241,7 @@ efi/buildid.o efi/relocs-dummy.o: $(BASE
>   efi/buildid.o efi/relocs-dummy.o: ;
>   
>   asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h

On Arm, only asm-offsets.c is a pre-requisite. May I ask why you need 
the second on x86?

> -	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new $<
> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
>   	$(call move-if-changed,$@.new,$@)
>   
>   asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
> 

Cheers,

-- 
Julien Grall
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s
Posted by Jan Beulich 3 years, 2 months ago 
On 06.02.2021 10:09, Julien Grall wrote:
> On 01/02/2021 14:56, Jan Beulich wrote:
>> Going through an intermediate *.new file requires telling the compiler
>> what the real target is, so that the inclusion of the resulting .*.d
>> file will actually be useful.
>>
>> Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
>> Reported-by: Julien Grall <julien@xen.org>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Already on the original patch I did suggest that perhaps Arm would want
>> to follow suit. So again - perhaps the rules should be unified by moving
>> to common code?
> 
> Sorry I missed the original patch. The recent changes looks beneficials 
> to Arm as well.

Okay, I can make a patch then (for 4.16) to make this common
as far as possible.

>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -241,7 +241,7 @@ efi/buildid.o efi/relocs-dummy.o: $(BASE
>>   efi/buildid.o efi/relocs-dummy.o: ;
>>   
>>   asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
> 
> On Arm, only asm-offsets.c is a pre-requisite. May I ask why you need 
> the second on x86?

Because this header gets used by the file and hence needs
generating up front? But that's nothing Arm needs to worry
about - I intend to allow extra per-arch dependencies, no
matter that common things are to become common.

Jan
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]
Posted by Ian Jackson 3 years, 2 months ago 
Andrew Cooper writes ("Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s"):
> On 01/02/2021 14:56, Jan Beulich wrote:
> > Going through an intermediate *.new file requires telling the compiler
> > what the real target is, so that the inclusion of the resulting .*.d
> > file will actually be useful.
> >
> > Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
> > Reported-by: Julien Grall <julien@xen.org>
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, whatever the
> outcome of the discussion below.

This is a bugfix and does not need a release ack, but FTAOD

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

> > Already on the original patch I did suggest that perhaps Arm would want
> > to follow suit. So again - perhaps the rules should be unified by moving
> > to common code?

Quite so.

Ian.
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]
Posted by Jan Beulich 3 years, 2 months ago 
On 01.02.2021 16:16, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s"):
>> On 01/02/2021 14:56, Jan Beulich wrote:
>>> Going through an intermediate *.new file requires telling the compiler
>>> what the real target is, so that the inclusion of the resulting .*.d
>>> file will actually be useful.
>>>
>>> Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
>>> Reported-by: Julien Grall <julien@xen.org>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, whatever the
>> outcome of the discussion below.
> 
> This is a bugfix and does not need a release ack, but FTAOD

Oh, this used to be different on prior releases once we were
past the full freeze point. Are to intending to allow bug fixes
without release ack until the actual release (minus commit
moratorium periods, of course), or will this change at some
(un?)predictable point?

> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Thanks.

Jan
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]
Posted by Ian Jackson 3 years, 2 months ago 
Jan Beulich writes ("Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]"):
> Oh, this used to be different on prior releases once we were
> past the full freeze point. Are to intending to allow bug fixes
> without release ack until the actual release (minus commit
> moratorium periods, of course), or will this change at some
> (un?)predictable point?

> >    Friday 29th January    Feature freeze
> > 
> >        Patches adding new features should be committed by this date.
> >        Straightforward bugfixes may continue to be accepted by maintainers.
> > 
> >    Friday 12th February **tentatve**   Code freeze
> > 
> >        Bugfixes only, all changes to be approved by the Release Manager.

I will send a proper announcement.

Ian.
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]
Posted by Jan Beulich 3 years, 2 months ago 
On 01.02.2021 16:28, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s [and 1 more messages]"):
>> Oh, this used to be different on prior releases once we were
>> past the full freeze point. Are to intending to allow bug fixes
>> without release ack until the actual release (minus commit
>> moratorium periods, of course), or will this change at some
>> (un?)predictable point?
> 
>>>    Friday 29th January    Feature freeze
>>>
>>>        Patches adding new features should be committed by this date.
>>>        Straightforward bugfixes may continue to be accepted by maintainers.
>>>
>>>    Friday 12th February **tentatve**   Code freeze
>>>
>>>        Bugfixes only, all changes to be approved by the Release Manager.

Oh, looks like I forgot we have three freezes, not two.

> I will send a proper announcement.

Thanks.

Jan
Re: [PATCH] x86/build: correctly record dependencies of asm-offsets.s
Posted by Andrew Cooper 3 years, 2 months ago 
On 01/02/2021 14:56, Jan Beulich wrote:
> Going through an intermediate *.new file requires telling the compiler
> what the real target is, so that the inclusion of the resulting .*.d
> file will actually be useful.
>
> Fixes: 7d2d7a43d014 ("x86/build: limit rebuilding of asm-offsets.h")
> Reported-by: Julien Grall <julien@xen.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>, whatever the
outcome of the discussion below.

> ---
> Already on the original patch I did suggest that perhaps Arm would want
> to follow suit. So again - perhaps the rules should be unified by moving
> to common code?
>
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -241,7 +241,7 @@ efi/buildid.o efi/relocs-dummy.o: $(BASE
>  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 $<
> +	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
>  	$(call move-if-changed,$@.new,$@)
>  
>  asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.