diff mbox

[kvm-unit-tests] x86: Makefile refine

Message ID 1462464222-2872-1-git-send-email-richard.weiyang@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Yang May 5, 2016, 4:03 p.m. UTC
In x86 Makefile, each elf target has a rule for its dependent, which shares
the same pattern. We could let makefile to handle this job instead of
writing a specific rule for each elf target. By doing so, the makefile rule
looks clear and would be easy for adding new cases.

This patch does several cleanup:
1. add $(cstart.o) in *.elf dependent
2. remove all those elf rules which share the same pattern
3. remove the *.o dependent in the elf rule who has extra dependent
4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
5. remove elf rules in Makefile.i386

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

---
This is based on latest master branch and I just tested this on x86_64
arch.

---
 x86/Makefile.common |   69 +++------------------------------------------------
 x86/Makefile.i386   |    4 ---
 x86/Makefile.x86_64 |    2 ++
 3 files changed, 6 insertions(+), 69 deletions(-)

Comments

Andrew Jones May 6, 2016, 9:44 a.m. UTC | #1
On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
> In x86 Makefile, each elf target has a rule for its dependent, which shares
> the same pattern. We could let makefile to handle this job instead of
> writing a specific rule for each elf target. By doing so, the makefile rule
> looks clear and would be easy for adding new cases.
> 
> This patch does several cleanup:
> 1. add $(cstart.o) in *.elf dependent
> 2. remove all those elf rules which share the same pattern
> 3. remove the *.o dependent in the elf rule who has extra dependent
> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
> 5. remove elf rules in Makefile.i386

This is a nice idea for a cleanup, but it seems to have a not so
nice side-effect. When I tested it I see a new final action occurs.
All x86/*.o and x86/*.elf files that we didn't keep the explicit
rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
removed. I'd prefer we don't delete anything unless a 'make clean'
is done. Also, for arm, we need to keep the elf files for objdump
to work.

Is there any way to tell make to remove that 'rm'?

Thanks,
drew

> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> 
> ---
> This is based on latest master branch and I just tested this on x86_64
> arch.
> 
> ---
>  x86/Makefile.common |   69 +++------------------------------------------------
>  x86/Makefile.i386   |    4 ---
>  x86/Makefile.x86_64 |    2 ++
>  3 files changed, 6 insertions(+), 69 deletions(-)
> 
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 298e5f7..842a9e7 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>  
>  FLATLIBS = lib/libcflat.a $(libgcc)
> -%.elf: %.o $(FLATLIBS) x86/flat.lds
> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
>  		$(filter %.o, $^) $(FLATLIBS)
>  
> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
>  
>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
>  
> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
> -
> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
> -
> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
> -
> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
> -
> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
> -
> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
> -
> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
> -
> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
> -
> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
> -
> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
> -
> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
> -
> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
> -
> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
> -
>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
>  
>  $(TEST_DIR)/realmode.o: bits = 32
>  
> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
> -
> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
> -
> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
> -
> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
> -
> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
> -
> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
> -                                $(TEST_DIR)/kvmclock_test.o
> -
> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
> -
> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
> -
> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
> -
> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
> -
> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
> -
> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
> -
> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
> -
> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
> -
> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
> -
> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
> -
> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> -                              $(TEST_DIR)/hyperv_synic.o
> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
>  
> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> -                               $(TEST_DIR)/hyperv_stimer.o
> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
>  
> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
>  
>  arch_clean:
>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
> index 8a4c45c..c4176c4 100644
> --- a/x86/Makefile.i386
> +++ b/x86/Makefile.i386
> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
>  	$(TEST_DIR)/cmpxchg8b.flat
>  
>  include $(TEST_DIR)/Makefile.common
> -
> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> index 6b7ccfb..e166911 100644
> --- a/x86/Makefile.x86_64
> +++ b/x86/Makefile.x86_64
> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
>  tests += $(TEST_DIR)/tscdeadline_latency.flat
>  
>  include $(TEST_DIR)/Makefile.common
> +
> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
> -- 
> 1.7.9.5
> 
> --
> 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
--
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
Wei Yang May 7, 2016, 10:59 p.m. UTC | #2
On Fri, May 06, 2016 at 11:44:07AM +0200, Andrew Jones wrote:
>On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
>> In x86 Makefile, each elf target has a rule for its dependent, which shares
>> the same pattern. We could let makefile to handle this job instead of
>> writing a specific rule for each elf target. By doing so, the makefile rule
>> looks clear and would be easy for adding new cases.
>> 
>> This patch does several cleanup:
>> 1. add $(cstart.o) in *.elf dependent
>> 2. remove all those elf rules which share the same pattern
>> 3. remove the *.o dependent in the elf rule who has extra dependent
>> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
>> 5. remove elf rules in Makefile.i386
>
>This is a nice idea for a cleanup, but it seems to have a not so
>nice side-effect. When I tested it I see a new final action occurs.
>All x86/*.o and x86/*.elf files that we didn't keep the explicit
>rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
>x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
>removed. I'd prefer we don't delete anything unless a 'make clean'
>is done. Also, for arm, we need to keep the elf files for objdump
>to work.

Hi, Drew

Nice to hear from you.

I didn't understand how the side-effect you mentioned happens. If you could
let me know which steps you did and the difference between the original
version and the one after my patch, maybe I could understand what the
side-effect is.

The change in the makefile remove some rules instead of remove elf files, so I
am a little bit confused about your last sentence. Those elf files are there
unless you remove them manually.

>
>Is there any way to tell make to remove that 'rm'?
>
>Thanks,
>drew
>
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> 
>> ---
>> This is based on latest master branch and I just tested this on x86_64
>> arch.
>> 
>> ---
>>  x86/Makefile.common |   69 +++------------------------------------------------
>>  x86/Makefile.i386   |    4 ---
>>  x86/Makefile.x86_64 |    2 ++
>>  3 files changed, 6 insertions(+), 69 deletions(-)
>> 
>> diff --git a/x86/Makefile.common b/x86/Makefile.common
>> index 298e5f7..842a9e7 100644
>> --- a/x86/Makefile.common
>> +++ b/x86/Makefile.common
>> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
>>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>>  
>>  FLATLIBS = lib/libcflat.a $(libgcc)
>> -%.elf: %.o $(FLATLIBS) x86/flat.lds
>> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
>>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
>>  		$(filter %.o, $^) $(FLATLIBS)
>>  
>> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
>>  
>>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
>>  
>> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
>> -
>> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
>> -
>> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
>> -
>> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
>> -
>> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
>> -
>> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
>> -
>> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
>> -
>> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
>> -
>> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
>> -
>> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
>> -
>> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
>> -
>> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
>> -
>> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
>> -
>>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
>>  
>>  $(TEST_DIR)/realmode.o: bits = 32
>>  
>> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
>> -
>> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
>> -
>> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
>> -
>> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
>> -
>> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
>> -
>> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
>> -                                $(TEST_DIR)/kvmclock_test.o
>> -
>> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
>> -
>> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
>> -
>> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
>> -
>> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
>> -
>> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
>> -
>> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
>> -
>> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
>> -
>> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
>> -
>> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
>> -
>> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
>> -
>> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> -                              $(TEST_DIR)/hyperv_synic.o
>> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
>>  
>> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> -                               $(TEST_DIR)/hyperv_stimer.o
>> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
>>  
>> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
>> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
>>  
>>  arch_clean:
>>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
>> index 8a4c45c..c4176c4 100644
>> --- a/x86/Makefile.i386
>> +++ b/x86/Makefile.i386
>> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
>>  	$(TEST_DIR)/cmpxchg8b.flat
>>  
>>  include $(TEST_DIR)/Makefile.common
>> -
>> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
>> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
>> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
>> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
>> index 6b7ccfb..e166911 100644
>> --- a/x86/Makefile.x86_64
>> +++ b/x86/Makefile.x86_64
>> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
>>  tests += $(TEST_DIR)/tscdeadline_latency.flat
>>  
>>  include $(TEST_DIR)/Makefile.common
>> +
>> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
>> -- 
>> 1.7.9.5
>> 
>> --
>> 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
Wei Yang May 8, 2016, 1:12 p.m. UTC | #3
On Fri, May 06, 2016 at 11:44:07AM +0200, Andrew Jones wrote:
>On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
>> In x86 Makefile, each elf target has a rule for its dependent, which shares
>> the same pattern. We could let makefile to handle this job instead of
>> writing a specific rule for each elf target. By doing so, the makefile rule
>> looks clear and would be easy for adding new cases.
>> 
>> This patch does several cleanup:
>> 1. add $(cstart.o) in *.elf dependent
>> 2. remove all those elf rules which share the same pattern
>> 3. remove the *.o dependent in the elf rule who has extra dependent
>> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
>> 5. remove elf rules in Makefile.i386
>
>This is a nice idea for a cleanup, but it seems to have a not so
>nice side-effect. When I tested it I see a new final action occurs.
>All x86/*.o and x86/*.elf files that we didn't keep the explicit
>rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
>x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
>removed. I'd prefer we don't delete anything unless a 'make clean'
>is done. Also, for arm, we need to keep the elf files for objdump
>to work.
>

Ok, I guess I understand what you mean. You mean after run "make", some
intermediate files will be deleted.

By looking at the document,
http://www.gnu.org/software/make/manual/html_node/Special-Targets.html

Two special target looks meet our requirement: .PRECIOUS and .SECONDARY. While
with some testing, it looks .SECONDARY not working well on my machine. So I
choose to use .PRECIOUS: %.elf %.o to fix this.

V2 will be sent.

Thanks for your comment:-)

>Is there any way to tell make to remove that 'rm'?
>
>Thanks,
>drew
>
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> 
>> ---
>> This is based on latest master branch and I just tested this on x86_64
>> arch.
>> 
>> ---
>>  x86/Makefile.common |   69 +++------------------------------------------------
>>  x86/Makefile.i386   |    4 ---
>>  x86/Makefile.x86_64 |    2 ++
>>  3 files changed, 6 insertions(+), 69 deletions(-)
>> 
>> diff --git a/x86/Makefile.common b/x86/Makefile.common
>> index 298e5f7..842a9e7 100644
>> --- a/x86/Makefile.common
>> +++ b/x86/Makefile.common
>> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
>>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>>  
>>  FLATLIBS = lib/libcflat.a $(libgcc)
>> -%.elf: %.o $(FLATLIBS) x86/flat.lds
>> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
>>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
>>  		$(filter %.o, $^) $(FLATLIBS)
>>  
>> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
>>  
>>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
>>  
>> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
>> -
>> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
>> -
>> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
>> -
>> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
>> -
>> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
>> -
>> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
>> -
>> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
>> -
>> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
>> -
>> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
>> -
>> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
>> -
>> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
>> -
>> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
>> -
>> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
>> -
>>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
>>  
>>  $(TEST_DIR)/realmode.o: bits = 32
>>  
>> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
>> -
>> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
>> -
>> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
>> -
>> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
>> -
>> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
>> -
>> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
>> -                                $(TEST_DIR)/kvmclock_test.o
>> -
>> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
>> -
>> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
>> -
>> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
>> -
>> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
>> -
>> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
>> -
>> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
>> -
>> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
>> -
>> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
>> -
>> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
>> -
>> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
>> -
>> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> -                              $(TEST_DIR)/hyperv_synic.o
>> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
>>  
>> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> -                               $(TEST_DIR)/hyperv_stimer.o
>> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
>>  
>> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
>> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
>>  
>>  arch_clean:
>>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
>> index 8a4c45c..c4176c4 100644
>> --- a/x86/Makefile.i386
>> +++ b/x86/Makefile.i386
>> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
>>  	$(TEST_DIR)/cmpxchg8b.flat
>>  
>>  include $(TEST_DIR)/Makefile.common
>> -
>> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
>> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
>> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
>> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
>> index 6b7ccfb..e166911 100644
>> --- a/x86/Makefile.x86_64
>> +++ b/x86/Makefile.x86_64
>> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
>>  tests += $(TEST_DIR)/tscdeadline_latency.flat
>>  
>>  include $(TEST_DIR)/Makefile.common
>> +
>> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
>> -- 
>> 1.7.9.5
>> 
>> --
>> 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
Andrew Jones May 9, 2016, 1:24 p.m. UTC | #4
On Sat, May 07, 2016 at 10:59:01PM +0000, Wei Yang wrote:
> On Fri, May 06, 2016 at 11:44:07AM +0200, Andrew Jones wrote:
> >On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
> >> In x86 Makefile, each elf target has a rule for its dependent, which shares
> >> the same pattern. We could let makefile to handle this job instead of
> >> writing a specific rule for each elf target. By doing so, the makefile rule
> >> looks clear and would be easy for adding new cases.
> >> 
> >> This patch does several cleanup:
> >> 1. add $(cstart.o) in *.elf dependent
> >> 2. remove all those elf rules which share the same pattern
> >> 3. remove the *.o dependent in the elf rule who has extra dependent
> >> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
> >> 5. remove elf rules in Makefile.i386
> >
> >This is a nice idea for a cleanup, but it seems to have a not so
> >nice side-effect. When I tested it I see a new final action occurs.
> >All x86/*.o and x86/*.elf files that we didn't keep the explicit
> >rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
> >x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
> >removed. I'd prefer we don't delete anything unless a 'make clean'
> >is done. Also, for arm, we need to keep the elf files for objdump
> >to work.
> 
> Hi, Drew
> 
> Nice to hear from you.
> 
> I didn't understand how the side-effect you mentioned happens. If you could
> let me know which steps you did and the difference between the original
> version and the one after my patch, maybe I could understand what the
> side-effect is.

I run 'make' :-)

And the side-effect is what I described above.

Just do
 without patch: make >& make.orig
 with patch   : make >& make.new
 then         : diff make.orig make.new

> 
> The change in the makefile remove some rules instead of remove elf files, so I
> am a little bit confused about your last sentence. Those elf files are there
> unless you remove them manually.

Well, I guess not. At least not for me with Fedora 22,
make-4.0-3.1.fc22.x86_64

Thanks,
drew

> 
> >
> >Is there any way to tell make to remove that 'rm'?
> >
> >Thanks,
> >drew
> >
> >> 
> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >> 
> >> ---
> >> This is based on latest master branch and I just tested this on x86_64
> >> arch.
> >> 
> >> ---
> >>  x86/Makefile.common |   69 +++------------------------------------------------
> >>  x86/Makefile.i386   |    4 ---
> >>  x86/Makefile.x86_64 |    2 ++
> >>  3 files changed, 6 insertions(+), 69 deletions(-)
> >> 
> >> diff --git a/x86/Makefile.common b/x86/Makefile.common
> >> index 298e5f7..842a9e7 100644
> >> --- a/x86/Makefile.common
> >> +++ b/x86/Makefile.common
> >> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
> >>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
> >>  
> >>  FLATLIBS = lib/libcflat.a $(libgcc)
> >> -%.elf: %.o $(FLATLIBS) x86/flat.lds
> >> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
> >>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
> >>  		$(filter %.o, $^) $(FLATLIBS)
> >>  
> >> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
> >>  
> >>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
> >>  
> >> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
> >> -
> >> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
> >> -
> >> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
> >> -
> >> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
> >> -
> >> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
> >> -
> >> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
> >> -
> >> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
> >> -
> >> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
> >> -
> >> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
> >> -
> >> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
> >> -
> >> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
> >> -
> >> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
> >> -
> >> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
> >> -
> >>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
> >>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
> >>  
> >>  $(TEST_DIR)/realmode.o: bits = 32
> >>  
> >> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
> >> -
> >> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
> >> -
> >> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
> >> -
> >> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
> >> -
> >> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
> >> -
> >> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
> >> -                                $(TEST_DIR)/kvmclock_test.o
> >> -
> >> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
> >> -
> >> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
> >> -
> >> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
> >> -
> >> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
> >> -
> >> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
> >> -
> >> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
> >> -
> >> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
> >> -
> >> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
> >> -
> >> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
> >> -
> >> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
> >> -
> >> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> >> -                              $(TEST_DIR)/hyperv_synic.o
> >> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
> >>  
> >> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> >> -                               $(TEST_DIR)/hyperv_stimer.o
> >> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
> >>  
> >> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
> >> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
> >>  
> >>  arch_clean:
> >>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
> >> index 8a4c45c..c4176c4 100644
> >> --- a/x86/Makefile.i386
> >> +++ b/x86/Makefile.i386
> >> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
> >>  	$(TEST_DIR)/cmpxchg8b.flat
> >>  
> >>  include $(TEST_DIR)/Makefile.common
> >> -
> >> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
> >> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
> >> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
> >> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> >> index 6b7ccfb..e166911 100644
> >> --- a/x86/Makefile.x86_64
> >> +++ b/x86/Makefile.x86_64
> >> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
> >>  tests += $(TEST_DIR)/tscdeadline_latency.flat
> >>  
> >>  include $(TEST_DIR)/Makefile.common
> >> +
> >> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
> >> -- 
> >> 1.7.9.5
> >> 
> >> --
> >> 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
> 
> -- 
> Wei Yang
> Help you, Help me
> --
> 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
--
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
Andrew Jones May 9, 2016, 1:28 p.m. UTC | #5
On Sun, May 08, 2016 at 01:12:19PM +0000, Wei Yang wrote:
> On Fri, May 06, 2016 at 11:44:07AM +0200, Andrew Jones wrote:
> >On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
> >> In x86 Makefile, each elf target has a rule for its dependent, which shares
> >> the same pattern. We could let makefile to handle this job instead of
> >> writing a specific rule for each elf target. By doing so, the makefile rule
> >> looks clear and would be easy for adding new cases.
> >> 
> >> This patch does several cleanup:
> >> 1. add $(cstart.o) in *.elf dependent
> >> 2. remove all those elf rules which share the same pattern
> >> 3. remove the *.o dependent in the elf rule who has extra dependent
> >> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
> >> 5. remove elf rules in Makefile.i386
> >
> >This is a nice idea for a cleanup, but it seems to have a not so
> >nice side-effect. When I tested it I see a new final action occurs.
> >All x86/*.o and x86/*.elf files that we didn't keep the explicit
> >rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
> >x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
> >removed. I'd prefer we don't delete anything unless a 'make clean'
> >is done. Also, for arm, we need to keep the elf files for objdump
> >to work.
> >
> 
> Ok, I guess I understand what you mean. You mean after run "make", some
> intermediate files will be deleted.

I should read mail in reverse chronological order :-) Looks like you see
the issue now and have a solution.

> 
> By looking at the document,
> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
> 
> Two special target looks meet our requirement: .PRECIOUS and .SECONDARY. While
> with some testing, it looks .SECONDARY not working well on my machine. So I
> choose to use .PRECIOUS: %.elf %.o to fix this.

These special targets do look the like right approach. Too bad
.SECONDARY doesn't work, as that indeed seems like the best choice.

Thanks,
drew

> 
> V2 will be sent.
> 
> Thanks for your comment:-)
> 
> >Is there any way to tell make to remove that 'rm'?
> >
> >Thanks,
> >drew
> >
> >> 
> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> >> 
> >> ---
> >> This is based on latest master branch and I just tested this on x86_64
> >> arch.
> >> 
> >> ---
> >>  x86/Makefile.common |   69 +++------------------------------------------------
> >>  x86/Makefile.i386   |    4 ---
> >>  x86/Makefile.x86_64 |    2 ++
> >>  3 files changed, 6 insertions(+), 69 deletions(-)
> >> 
> >> diff --git a/x86/Makefile.common b/x86/Makefile.common
> >> index 298e5f7..842a9e7 100644
> >> --- a/x86/Makefile.common
> >> +++ b/x86/Makefile.common
> >> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
> >>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
> >>  
> >>  FLATLIBS = lib/libcflat.a $(libgcc)
> >> -%.elf: %.o $(FLATLIBS) x86/flat.lds
> >> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
> >>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
> >>  		$(filter %.o, $^) $(FLATLIBS)
> >>  
> >> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
> >>  
> >>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
> >>  
> >> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
> >> -
> >> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
> >> -
> >> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
> >> -
> >> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
> >> -
> >> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
> >> -
> >> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
> >> -
> >> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
> >> -
> >> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
> >> -
> >> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
> >> -
> >> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
> >> -
> >> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
> >> -
> >> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
> >> -
> >> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
> >> -
> >>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
> >>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
> >>  
> >>  $(TEST_DIR)/realmode.o: bits = 32
> >>  
> >> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
> >> -
> >> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
> >> -
> >> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
> >> -
> >> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
> >> -
> >> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
> >> -
> >> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
> >> -                                $(TEST_DIR)/kvmclock_test.o
> >> -
> >> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
> >> -
> >> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
> >> -
> >> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
> >> -
> >> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
> >> -
> >> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
> >> -
> >> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
> >> -
> >> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
> >> -
> >> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
> >> -
> >> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
> >> -
> >> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
> >> -
> >> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> >> -                              $(TEST_DIR)/hyperv_synic.o
> >> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
> >>  
> >> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
> >> -                               $(TEST_DIR)/hyperv_stimer.o
> >> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
> >>  
> >> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
> >> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
> >>  
> >>  arch_clean:
> >>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
> >> index 8a4c45c..c4176c4 100644
> >> --- a/x86/Makefile.i386
> >> +++ b/x86/Makefile.i386
> >> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
> >>  	$(TEST_DIR)/cmpxchg8b.flat
> >>  
> >>  include $(TEST_DIR)/Makefile.common
> >> -
> >> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
> >> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
> >> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
> >> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
> >> index 6b7ccfb..e166911 100644
> >> --- a/x86/Makefile.x86_64
> >> +++ b/x86/Makefile.x86_64
> >> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
> >>  tests += $(TEST_DIR)/tscdeadline_latency.flat
> >>  
> >>  include $(TEST_DIR)/Makefile.common
> >> +
> >> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
> >> -- 
> >> 1.7.9.5
> >> 
> >> --
> >> 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
> 
> -- 
> Wei Yang
> Help you, Help me
> --
> 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
--
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
Wei Yang May 9, 2016, 10:22 p.m. UTC | #6
On Mon, May 09, 2016 at 03:28:28PM +0200, Andrew Jones wrote:
>On Sun, May 08, 2016 at 01:12:19PM +0000, Wei Yang wrote:
>> On Fri, May 06, 2016 at 11:44:07AM +0200, Andrew Jones wrote:
>> >On Thu, May 05, 2016 at 04:03:42PM +0000, Wei Yang wrote:
>> >> In x86 Makefile, each elf target has a rule for its dependent, which shares
>> >> the same pattern. We could let makefile to handle this job instead of
>> >> writing a specific rule for each elf target. By doing so, the makefile rule
>> >> looks clear and would be easy for adding new cases.
>> >> 
>> >> This patch does several cleanup:
>> >> 1. add $(cstart.o) in *.elf dependent
>> >> 2. remove all those elf rules which share the same pattern
>> >> 3. remove the *.o dependent in the elf rule who has extra dependent
>> >> 4. move the vmx.elf rule to Makefile.x86_64 since this is not a common case
>> >> 5. remove elf rules in Makefile.i386
>> >
>> >This is a nice idea for a cleanup, but it seems to have a not so
>> >nice side-effect. When I tested it I see a new final action occurs.
>> >All x86/*.o and x86/*.elf files that we didn't keep the explicit
>> >rule for (i.e. x86/hyperv_stimer.elf,  x86/hyperv_synic.elf,
>> >x86/kvmclock_test.elf, x86/realmode.elf, x86/vmx.elf) are now
>> >removed. I'd prefer we don't delete anything unless a 'make clean'
>> >is done. Also, for arm, we need to keep the elf files for objdump
>> >to work.
>> >
>> 
>> Ok, I guess I understand what you mean. You mean after run "make", some
>> intermediate files will be deleted.
>
>I should read mail in reverse chronological order :-) Looks like you see
>the issue now and have a solution.
>
>> 
>> By looking at the document,
>> http://www.gnu.org/software/make/manual/html_node/Special-Targets.html
>> 
>> Two special target looks meet our requirement: .PRECIOUS and .SECONDARY. While
>> with some testing, it looks .SECONDARY not working well on my machine. So I
>> choose to use .PRECIOUS: %.elf %.o to fix this.
>
>These special targets do look the like right approach. Too bad
>.SECONDARY doesn't work, as that indeed seems like the best choice.

Yep, we share the same thoughts.

>
>Thanks,
>drew
>
>> 
>> V2 will be sent.
>> 
>> Thanks for your comment:-)
>> 
>> >Is there any way to tell make to remove that 'rm'?
>> >
>> >Thanks,
>> >drew
>> >
>> >> 
>> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> >> 
>> >> ---
>> >> This is based on latest master branch and I just tested this on x86_64
>> >> arch.
>> >> 
>> >> ---
>> >>  x86/Makefile.common |   69 +++------------------------------------------------
>> >>  x86/Makefile.i386   |    4 ---
>> >>  x86/Makefile.x86_64 |    2 ++
>> >>  3 files changed, 6 insertions(+), 69 deletions(-)
>> >> 
>> >> diff --git a/x86/Makefile.common b/x86/Makefile.common
>> >> index 298e5f7..842a9e7 100644
>> >> --- a/x86/Makefile.common
>> >> +++ b/x86/Makefile.common
>> >> @@ -26,7 +26,7 @@ KEEP_FRAME_POINTER := y
>> >>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>> >>  
>> >>  FLATLIBS = lib/libcflat.a $(libgcc)
>> >> -%.elf: %.o $(FLATLIBS) x86/flat.lds
>> >> +%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
>> >>  	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
>> >>  		$(filter %.o, $^) $(FLATLIBS)
>> >>  
>> >> @@ -53,77 +53,16 @@ test_cases: $(tests-common) $(tests)
>> >>  
>> >>  $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
>> >>  
>> >> -$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
>> >> -
>> >> -$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
>> >> -
>> >> -$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
>> >> -
>> >> -$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
>> >> -
>> >> -$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
>> >> -
>> >> -$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
>> >> -
>> >> -$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
>> >> -
>> >> -$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
>> >> -
>> >> -$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
>> >> -
>> >> -$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
>> >> -
>> >> -$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
>> >> -
>> >> -$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
>> >> -
>> >> -$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
>> >> -
>> >>  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>> >>  	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
>> >>  
>> >>  $(TEST_DIR)/realmode.o: bits = 32
>> >>  
>> >> -$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
>> >> -
>> >> -$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
>> >> -
>> >> -$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
>> >> -
>> >> -$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
>> >> -
>> >> -$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
>> >> -
>> >> -$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
>> >> -                                $(TEST_DIR)/kvmclock_test.o
>> >> -
>> >> -$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
>> >> -
>> >> -$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
>> >> -
>> >> -$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
>> >> -
>> >> -$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
>> >> -
>> >> -$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
>> >> -
>> >> -$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
>> >> -
>> >> -$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
>> >> -
>> >> -$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
>> >> -
>> >> -$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
>> >> -
>> >> -$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
>> >> -
>> >> -$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> >> -                              $(TEST_DIR)/hyperv_synic.o
>> >> +$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
>> >>  
>> >> -$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
>> >> -                               $(TEST_DIR)/hyperv_stimer.o
>> >> +$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
>> >>  
>> >> -$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
>> >> +$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
>> >>  
>> >>  arch_clean:
>> >>  	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>> >> diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
>> >> index 8a4c45c..c4176c4 100644
>> >> --- a/x86/Makefile.i386
>> >> +++ b/x86/Makefile.i386
>> >> @@ -9,7 +9,3 @@ tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
>> >>  	$(TEST_DIR)/cmpxchg8b.flat
>> >>  
>> >>  include $(TEST_DIR)/Makefile.common
>> >> -
>> >> -$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
>> >> -$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
>> >> -$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
>> >> diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
>> >> index 6b7ccfb..e166911 100644
>> >> --- a/x86/Makefile.x86_64
>> >> +++ b/x86/Makefile.x86_64
>> >> @@ -16,3 +16,5 @@ tests += $(TEST_DIR)/vmx.flat
>> >>  tests += $(TEST_DIR)/tscdeadline_latency.flat
>> >>  
>> >>  include $(TEST_DIR)/Makefile.common
>> >> +
>> >> +$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o
>> >> -- 
>> >> 1.7.9.5
>> >> 
>> >> --
>> >> 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
>> 
>> -- 
>> Wei Yang
>> Help you, Help me
>> --
>> 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
diff mbox

Patch

diff --git a/x86/Makefile.common b/x86/Makefile.common
index 298e5f7..842a9e7 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -26,7 +26,7 @@  KEEP_FRAME_POINTER := y
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-%.elf: %.o $(FLATLIBS) x86/flat.lds
+%.elf: %.o $(FLATLIBS) x86/flat.lds $(cstart.o)
 	$(CC) $(CFLAGS) -nostdlib -o $@ -Wl,-T,x86/flat.lds \
 		$(filter %.o, $^) $(FLATLIBS)
 
@@ -53,77 +53,16 @@  test_cases: $(tests-common) $(tests)
 
 $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib -I lib/x86
 
-$(TEST_DIR)/access.elf: $(cstart.o) $(TEST_DIR)/access.o
-
-$(TEST_DIR)/hypercall.elf: $(cstart.o) $(TEST_DIR)/hypercall.o
-
-$(TEST_DIR)/sieve.elf: $(cstart.o) $(TEST_DIR)/sieve.o
-
-$(TEST_DIR)/vmexit.elf: $(cstart.o) $(TEST_DIR)/vmexit.o
-
-$(TEST_DIR)/smptest.elf: $(cstart.o) $(TEST_DIR)/smptest.o
-
-$(TEST_DIR)/emulator.elf: $(cstart.o) $(TEST_DIR)/emulator.o
-
-$(TEST_DIR)/port80.elf: $(cstart.o) $(TEST_DIR)/port80.o
-
-$(TEST_DIR)/tsc.elf: $(cstart.o) $(TEST_DIR)/tsc.o
-
-$(TEST_DIR)/tsc_adjust.elf: $(cstart.o) $(TEST_DIR)/tsc_adjust.o
-
-$(TEST_DIR)/apic.elf: $(cstart.o) $(TEST_DIR)/apic.o
-
-$(TEST_DIR)/ioapic.elf: $(cstart.o) $(TEST_DIR)/ioapic.o
-
-$(TEST_DIR)/tscdeadline_latency.elf: $(cstart.o) $(TEST_DIR)/tscdeadline_latency.o
-
-$(TEST_DIR)/init.elf: $(cstart.o) $(TEST_DIR)/init.o
-
 $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
 	$(CC) -m32 -nostdlib -o $@ -Wl,-T,$(TEST_DIR)/realmode.lds $^
 
 $(TEST_DIR)/realmode.o: bits = 32
 
-$(TEST_DIR)/msr.elf: $(cstart.o) $(TEST_DIR)/msr.o
-
-$(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o
-
-$(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
-
-$(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
-
-$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
-
-$(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
-                                $(TEST_DIR)/kvmclock_test.o
-
-$(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
-
-$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
-
-$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
-
-$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
-
-$(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
-
-$(TEST_DIR)/smap.elf: $(cstart.o) $(TEST_DIR)/smap.o
-
-$(TEST_DIR)/pku.elf: $(cstart.o) $(TEST_DIR)/pku.o
-
-$(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
-
-$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
-
-$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
-
-$(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
-                              $(TEST_DIR)/hyperv_synic.o
+$(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
 
-$(TEST_DIR)/hyperv_stimer.elf: $(cstart.o) $(TEST_DIR)/hyperv.o \
-                               $(TEST_DIR)/hyperv_stimer.o
+$(TEST_DIR)/hyperv_synic.elf: $(TEST_DIR)/hyperv.o
 
-$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
+$(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o
 
 arch_clean:
 	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
index 8a4c45c..c4176c4 100644
--- a/x86/Makefile.i386
+++ b/x86/Makefile.i386
@@ -9,7 +9,3 @@  tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
 	$(TEST_DIR)/cmpxchg8b.flat
 
 include $(TEST_DIR)/Makefile.common
-
-$(TEST_DIR)/cmpxchg8b.elf: $(cstart.o) $(TEST_DIR)/cmpxchg8b.o
-$(TEST_DIR)/taskswitch.elf: $(cstart.o) $(TEST_DIR)/taskswitch.o
-$(TEST_DIR)/taskswitch2.elf: $(cstart.o) $(TEST_DIR)/taskswitch2.o
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index 6b7ccfb..e166911 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -16,3 +16,5 @@  tests += $(TEST_DIR)/vmx.flat
 tests += $(TEST_DIR)/tscdeadline_latency.flat
 
 include $(TEST_DIR)/Makefile.common
+
+$(TEST_DIR)/vmx.elf: $(TEST_DIR)/vmx_tests.o