[kvm-unit-tests PATCH 0/2] arm/powerpc: Makefile cleanup

[kvm-unit-tests PATCH 0/2] arm/powerpc: Makefile cleanup

[Andrew Jones]

Cleanup makefiles like 'x86: Makefile refine' does.

Andrew Jones (2):
  arm/arm64: Makefile cleanup
  powerpc/ppc64: Makefile cleanup

 arm/Makefile.common     |  8 ++++----
 powerpc/Makefile.common | 13 ++++---------
 2 files changed, 8 insertions(+), 13 deletions(-)

-- 
2.4.11

[kvm-unit-tests PATCH 1/2] arm/arm64: Makefile cleanup

[Andrew Jones]

This does the same cleanup as 'x86: Makefile refine'

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/Makefile.common | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arm/Makefile.common b/arm/Makefile.common
index 9a2d61fc88a27..51d2cf1690cae 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -24,6 +24,9 @@ CFLAGS += -Wextra
 CFLAGS += -O2
 CFLAGS += -I lib -I lib/libfdt
 
+# We want to keep intermediate files: %.elf and %.o
+.PRECIOUS: %.elf %.o
+
 asm-offsets = lib/$(ARCH)/asm-offsets.h
 include scripts/asm-offsets.mak
 
@@ -48,7 +51,7 @@ start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) )))
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
 %.elf: LDFLAGS = $(CFLAGS) -nostdlib
-%.elf: %.o $(FLATLIBS) arm/flat.lds
+%.elf: %.o $(FLATLIBS) arm/flat.lds $(cstart.o)
 	$(CC) $(LDFLAGS) -o $@ \
 		-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
 		$(filter %.o, $^) $(FLATLIBS)
@@ -69,7 +72,4 @@ generated_files = $(asm-offsets)
 
 test_cases: $(generated_files) $(tests-common) $(tests)
 
-$(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
-$(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
-
 $(TEST_DIR)/selftest.o $(cstart.o): $(asm-offsets)
-- 
2.4.11

[kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Andrew Jones]

This does the same cleanup as 'x86: Makefile refine'

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 powerpc/Makefile.common | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 4449aec616853..60d545c98e38c 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
 CFLAGS += -Wa,-mregnames
 CFLAGS += -fpie
 
+# We want to keep intermediate files: %.elf and %.o
+.PRECIOUS: %.elf %.o
+
 asm-offsets = lib/$(ARCH)/asm-offsets.h
 include scripts/asm-offsets.mak
 
@@ -37,7 +40,7 @@ cflatobjs += lib/powerpc/processor.o
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
 %.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
-%.elf: %.o $(FLATLIBS) powerpc/flat.lds
+%.elf: %.o $(FLATLIBS) powerpc/flat.lds $(cstart.o) $(reloc.o)
 	$(LD) $(LDFLAGS) -o $@ \
 	      -T powerpc/flat.lds --build-id=none \
 		$(filter %.o, $^) $(FLATLIBS)
@@ -69,11 +72,3 @@ generated_files = $(asm-offsets)
 test_cases: $(generated_files) $(tests-common) $(tests)
 
 $(cstart.o): $(asm-offsets)
-
-$(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
-
-$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
-
-$(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
-
-$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
-- 
2.4.11

Re: [kvm-unit-tests PATCH 1/2] arm/arm64: Makefile cleanup

[Wei Yang]

On Mon, May 09, 2016 at 04:01:05PM +0200, Andrew Jones wrote:
>This does the same cleanup as 'x86: Makefile refine'
>
>Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

>---
> arm/Makefile.common | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/arm/Makefile.common b/arm/Makefile.common
>index 9a2d61fc88a27..51d2cf1690cae 100644
>--- a/arm/Makefile.common
>+++ b/arm/Makefile.common
>@@ -24,6 +24,9 @@ CFLAGS += -Wextra
> CFLAGS += -O2
> CFLAGS += -I lib -I lib/libfdt
> 
>+# We want to keep intermediate files: %.elf and %.o
>+.PRECIOUS: %.elf %.o
>+
> asm-offsets = lib/$(ARCH)/asm-offsets.h
> include scripts/asm-offsets.mak
> 
>@@ -48,7 +51,7 @@ start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) )))
> 
> FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
> %.elf: LDFLAGS = $(CFLAGS) -nostdlib
>-%.elf: %.o $(FLATLIBS) arm/flat.lds
>+%.elf: %.o $(FLATLIBS) arm/flat.lds $(cstart.o)
> 	$(CC) $(LDFLAGS) -o $@ \
> 		-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
> 		$(filter %.o, $^) $(FLATLIBS)
>@@ -69,7 +72,4 @@ generated_files = $(asm-offsets)
> 
> test_cases: $(generated_files) $(tests-common) $(tests)
> 
>-$(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
>-$(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
>-
> $(TEST_DIR)/selftest.o $(cstart.o): $(asm-offsets)
>-- 
>2.4.11

-- 
Wei Yang
Help you, Help me

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Wei Yang]

On Mon, May 09, 2016 at 04:01:06PM +0200, Andrew Jones wrote:
>This does the same cleanup as 'x86: Makefile refine'
>
>Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

>---
> powerpc/Makefile.common | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
>diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>index 4449aec616853..60d545c98e38c 100644
>--- a/powerpc/Makefile.common
>+++ b/powerpc/Makefile.common
>@@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
> CFLAGS += -Wa,-mregnames
> CFLAGS += -fpie
> 
>+# We want to keep intermediate files: %.elf and %.o
>+.PRECIOUS: %.elf %.o
>+
> asm-offsets = lib/$(ARCH)/asm-offsets.h
> include scripts/asm-offsets.mak
> 
>@@ -37,7 +40,7 @@ cflatobjs += lib/powerpc/processor.o
> FLATLIBS = $(libcflat) $(LIBFDT_archive)
> %.elf: CFLAGS += $(arch_CFLAGS)
> %.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
>-%.elf: %.o $(FLATLIBS) powerpc/flat.lds
>+%.elf: %.o $(FLATLIBS) powerpc/flat.lds $(cstart.o) $(reloc.o)
> 	$(LD) $(LDFLAGS) -o $@ \
> 	      -T powerpc/flat.lds --build-id=none \
> 		$(filter %.o, $^) $(FLATLIBS)
>@@ -69,11 +72,3 @@ generated_files = $(asm-offsets)
> test_cases: $(generated_files) $(tests-common) $(tests)
> 
> $(cstart.o): $(asm-offsets)
>-
>-$(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>-
>-$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>-
>-$(TEST_DIR)/rtas.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/rtas.o
>-
>-$(TEST_DIR)/emulator.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/emulator.o
>-- 
>2.4.11

-- 
Wei Yang
Help you, Help me

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Thomas Huth]

On 09.05.2016 16:01, Andrew Jones wrote:
> This does the same cleanup as 'x86: Makefile refine'
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  powerpc/Makefile.common | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 4449aec616853..60d545c98e38c 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
>  CFLAGS += -Wa,-mregnames
>  CFLAGS += -fpie
>  
> +# We want to keep intermediate files: %.elf and %.o
> +.PRECIOUS: %.elf %.o

If you just want to keep intermediate files, wouldn't it be better to
use .SECONDARY instead of .PRECIOUS? .PRECIOUS seems to contain some
more magic that comes into action when make is killed by a signal... not
sure if we want to have that here?

 Thomas

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Andrew Jones]

On Tue, May 10, 2016 at 08:19:36AM +0200, Thomas Huth wrote:
> On 09.05.2016 16:01, Andrew Jones wrote:
> > This does the same cleanup as 'x86: Makefile refine'
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  powerpc/Makefile.common | 13 ++++---------
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index 4449aec616853..60d545c98e38c 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
> >  CFLAGS += -Wa,-mregnames
> >  CFLAGS += -fpie
> >  
> > +# We want to keep intermediate files: %.elf and %.o
> > +.PRECIOUS: %.elf %.o
> 
> If you just want to keep intermediate files, wouldn't it be better to
> use .SECONDARY instead of .PRECIOUS? .PRECIOUS seems to contain some
> more magic that comes into action when make is killed by a signal... not
> sure if we want to have that here?

I should extend the commit message to explain that. Wei Yang found
(and I confirmed) that .SECONDARY doesn't seem to work[*] - we tried it,
but the intermediate files still got removed. We both agree it would be
the more appropriate choice though. Did you have better luck with it?

Thanks,
drew

[*] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/151351/focus=151536

> 
>  Thomas
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Laurent Vivier]

On 10/05/2016 08:35, Andrew Jones wrote:
> On Tue, May 10, 2016 at 08:19:36AM +0200, Thomas Huth wrote:
>> On 09.05.2016 16:01, Andrew Jones wrote:
>>> This does the same cleanup as 'x86: Makefile refine'
>>>
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> ---
>>>  powerpc/Makefile.common | 13 ++++---------
>>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>>> index 4449aec616853..60d545c98e38c 100644
>>> --- a/powerpc/Makefile.common
>>> +++ b/powerpc/Makefile.common
>>> @@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
>>>  CFLAGS += -Wa,-mregnames
>>>  CFLAGS += -fpie
>>>  
>>> +# We want to keep intermediate files: %.elf and %.o
>>> +.PRECIOUS: %.elf %.o
>>
>> If you just want to keep intermediate files, wouldn't it be better to
>> use .SECONDARY instead of .PRECIOUS? .PRECIOUS seems to contain some
>> more magic that comes into action when make is killed by a signal... not
>> sure if we want to have that here?
> 
> I should extend the commit message to explain that. Wei Yang found
> (and I confirmed) that .SECONDARY doesn't seem to work[*] - we tried it,
> but the intermediate files still got removed. We both agree it would be
> the more appropriate choice though. Did you have better luck with it?

I don't understand why you want to keep intermediate files. If they have
to be kept, they should be primary targets.

It seems for, at least ppc64, we don't need the .PRECIOUS.

Do you have an example?

Laurent

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Thomas Huth]

On 10.05.2016 09:44, Laurent Vivier wrote:
> 
> On 10/05/2016 08:35, Andrew Jones wrote:
>> On Tue, May 10, 2016 at 08:19:36AM +0200, Thomas Huth wrote:
>>> On 09.05.2016 16:01, Andrew Jones wrote:
>>>> This does the same cleanup as 'x86: Makefile refine'
>>>>
>>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>>> ---
>>>>  powerpc/Makefile.common | 13 ++++---------
>>>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>>>> index 4449aec616853..60d545c98e38c 100644
>>>> --- a/powerpc/Makefile.common
>>>> +++ b/powerpc/Makefile.common
>>>> @@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
>>>>  CFLAGS += -Wa,-mregnames
>>>>  CFLAGS += -fpie
>>>>  
>>>> +# We want to keep intermediate files: %.elf and %.o
>>>> +.PRECIOUS: %.elf %.o
>>>
>>> If you just want to keep intermediate files, wouldn't it be better to
>>> use .SECONDARY instead of .PRECIOUS? .PRECIOUS seems to contain some
>>> more magic that comes into action when make is killed by a signal... not
>>> sure if we want to have that here?
>>
>> I should extend the commit message to explain that. Wei Yang found
>> (and I confirmed) that .SECONDARY doesn't seem to work[*] - we tried it,
>> but the intermediate files still got removed. We both agree it would be
>> the more appropriate choice though. Did you have better luck with it?

I also only had a look at the info pages of make, to understand what
PRECIOUS is doing (since I didn't know that command yet), and so I
stumbled upon SECONDARY which sounded like a better match.
But I now gave it a try, and it also does not work here.
According to some comments on stackoverlow.com, SECONDARY does not work
with patterns. This is also indirectly documented in the info page of
make. For PRECIOUS, it says:

"You can also list the target pattern of an implicit rule (such as
 `%.o') as a prerequisite file of the special target `.PRECIOUS' to
 preserve intermediate files created by rules whose target patterns
 match that file's name."

But that paragraph is missing for SECONDARY, i.e. indicating that this
does not work there.

> I don't understand why you want to keep intermediate files. If they have
> to be kept, they should be primary targets.
> 
> It seems for, at least ppc64, we don't need the .PRECIOUS.

They could be useful for debugging purposes, e.g. for running "objdump"
on them... ?

Anyway, I think you can at least remove the "%.elf" from the PRECIOUS
line, since the elf files are our primary target on powerpc and thus do
not have to be listed here.

 Thomas

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Laurent Vivier]

On 10/05/2016 10:42, Thomas Huth wrote:
> On 10.05.2016 09:44, Laurent Vivier wrote:
>>
...
>> I don't understand why you want to keep intermediate files. If they have
>> to be kept, they should be primary targets.
>>
>> It seems for, at least ppc64, we don't need the .PRECIOUS.
> 
> They could be useful for debugging purposes, e.g. for running "objdump"
> on them... ?

You can run objdump on the .elf.

Laurent

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Andrew Jones]

On Tue, May 10, 2016 at 10:42:05AM +0200, Thomas Huth wrote:
> On 10.05.2016 09:44, Laurent Vivier wrote:
> > 
> > On 10/05/2016 08:35, Andrew Jones wrote:
> >> On Tue, May 10, 2016 at 08:19:36AM +0200, Thomas Huth wrote:
> >>> On 09.05.2016 16:01, Andrew Jones wrote:
> >>>> This does the same cleanup as 'x86: Makefile refine'
> >>>>
> >>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
> >>>> ---
> >>>>  powerpc/Makefile.common | 13 ++++---------
> >>>>  1 file changed, 4 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> >>>> index 4449aec616853..60d545c98e38c 100644
> >>>> --- a/powerpc/Makefile.common
> >>>> +++ b/powerpc/Makefile.common
> >>>> @@ -22,6 +22,9 @@ CFLAGS += -I lib -I lib/libfdt
> >>>>  CFLAGS += -Wa,-mregnames
> >>>>  CFLAGS += -fpie
> >>>>  
> >>>> +# We want to keep intermediate files: %.elf and %.o
> >>>> +.PRECIOUS: %.elf %.o
> >>>
> >>> If you just want to keep intermediate files, wouldn't it be better to
> >>> use .SECONDARY instead of .PRECIOUS? .PRECIOUS seems to contain some
> >>> more magic that comes into action when make is killed by a signal... not
> >>> sure if we want to have that here?
> >>
> >> I should extend the commit message to explain that. Wei Yang found
> >> (and I confirmed) that .SECONDARY doesn't seem to work[*] - we tried it,
> >> but the intermediate files still got removed. We both agree it would be
> >> the more appropriate choice though. Did you have better luck with it?
> 
> I also only had a look at the info pages of make, to understand what
> PRECIOUS is doing (since I didn't know that command yet), and so I
> stumbled upon SECONDARY which sounded like a better match.
> But I now gave it a try, and it also does not work here.
> According to some comments on stackoverlow.com, SECONDARY does not work
> with patterns. This is also indirectly documented in the info page of
> make. For PRECIOUS, it says:
> 
> "You can also list the target pattern of an implicit rule (such as
>  `%.o') as a prerequisite file of the special target `.PRECIOUS' to
>  preserve intermediate files created by rules whose target patterns
>  match that file's name."
> 
> But that paragraph is missing for SECONDARY, i.e. indicating that this
> does not work there.

Ah, yes. I agree with your interpretation of the [lack of] documentation
:-)

> 
> > I don't understand why you want to keep intermediate files. If they have
> > to be kept, they should be primary targets.
> > 
> > It seems for, at least ppc64, we don't need the .PRECIOUS.
> 
> They could be useful for debugging purposes, e.g. for running "objdump"
> on them... ?
> 
> Anyway, I think you can at least remove the "%.elf" from the PRECIOUS
> line, since the elf files are our primary target on powerpc and thus do
> not have to be listed here.

Indeed. I copy-pasted from the arm patch with my brain turned off.

I'll send a v2 of this series documenting in a comment the reason
PRECIOUS over SECONDARY was chosen, and drop elf from the ppc patch.

Thanks,
drew

> 
>  Thomas
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 2/2] powerpc/ppc64: Makefile cleanup

[Thomas Huth]

On 10.05.2016 10:55, Laurent Vivier wrote:
> 
> 
> On 10/05/2016 10:42, Thomas Huth wrote:
>> On 10.05.2016 09:44, Laurent Vivier wrote:
>>>
> ...
>>> I don't understand why you want to keep intermediate files. If they have
>>> to be kept, they should be primary targets.
>>>
>>> It seems for, at least ppc64, we don't need the .PRECIOUS.
>>
>> They could be useful for debugging purposes, e.g. for running "objdump"
>> on them... ?
> 
> You can run objdump on the .elf.

That should work most of the time ... but sometimes it's more convenient
to look at the code in the .o files only, so that you do not have to
struggle with the disassembly code of all other parts as well.

Anyway, I just tried the patch with the .PRECIOUS line disabled, and I
got a very strange behavior: The first time when I run make after a
"make clean", it deletes all *.o files in the powerpc folder. But when I
run "make" again the second time, the *.o files are not deleted anymore!
That's very confusing ... not sure if we want to have that behavior, so
maybe let's better keep that .PRECIOUS line?

 Thomas