* [kvm-unit-tests PATCH] x86: Makefile refine
@ 2016-05-05 16:03 Wei Yang
2016-05-06 9:44 ` Andrew Jones
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2016-05-05 16:03 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, Wei Yang
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(-)
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-05 16:03 [kvm-unit-tests PATCH] x86: Makefile refine Wei Yang
@ 2016-05-06 9:44 ` Andrew Jones
2016-05-07 22:59 ` Wei Yang
2016-05-08 13:12 ` Wei Yang
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Jones @ 2016-05-06 9:44 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-06 9:44 ` Andrew Jones
@ 2016-05-07 22:59 ` Wei Yang
2016-05-09 13:24 ` Andrew Jones
2016-05-08 13:12 ` Wei Yang
1 sibling, 1 reply; 7+ messages in thread
From: Wei Yang @ 2016-05-07 22:59 UTC (permalink / raw)
To: Andrew Jones; +Cc: Wei Yang, pbonzini, kvm
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
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-06 9:44 ` Andrew Jones
2016-05-07 22:59 ` Wei Yang
@ 2016-05-08 13:12 ` Wei Yang
2016-05-09 13:28 ` Andrew Jones
1 sibling, 1 reply; 7+ messages in thread
From: Wei Yang @ 2016-05-08 13:12 UTC (permalink / raw)
To: Andrew Jones; +Cc: Wei Yang, pbonzini, kvm
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
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-07 22:59 ` Wei Yang
@ 2016-05-09 13:24 ` Andrew Jones
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2016-05-09 13:24 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-08 13:12 ` Wei Yang
@ 2016-05-09 13:28 ` Andrew Jones
2016-05-09 22:22 ` Wei Yang
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Jones @ 2016-05-09 13:28 UTC (permalink / raw)
To: Wei Yang; +Cc: pbonzini, kvm
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: Makefile refine
2016-05-09 13:28 ` Andrew Jones
@ 2016-05-09 22:22 ` Wei Yang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2016-05-09 22:22 UTC (permalink / raw)
To: Andrew Jones; +Cc: Wei Yang, pbonzini, kvm
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
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-05-09 22:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-05 16:03 [kvm-unit-tests PATCH] x86: Makefile refine Wei Yang
2016-05-06 9:44 ` Andrew Jones
2016-05-07 22:59 ` Wei Yang
2016-05-09 13:24 ` Andrew Jones
2016-05-08 13:12 ` Wei Yang
2016-05-09 13:28 ` Andrew Jones
2016-05-09 22:22 ` Wei Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox