[kvm-unit-tests PATCH 0/2] arm/arm64: Add self-modifying code test case

[kvm-unit-tests PATCH 0/2] arm/arm64: Add self-modifying code test case

[Alexander Spyridakis]

This series implements a self-modifying code test for the arm and arm64 targets.
The first patch adds the new test case, while the second modifies the arch_clean
rule for arm and arm64, to explicitly remove object files and binaries on
'make clean' (fixing and issue while reconfiguring).

The test case is meant for testing the new mttcg implementation, although it was
also verified on KVM targets like Arndale and OMAP5-uevm for ARMv7 and the Juno
board (aarch32 and aarch64 guests) for ARMv8.

Alexander Spyridakis (2):
  arm/arm64: Add self-modifying code test
  arm/arm64 config: Fix arch_clean rule

 arm/self-modifying-test.c    | 109 +++++++++++++++++++++++++++++++++++++++++++
 config/config-arm-common.mak |   2 +
 config/config-arm.mak        |   2 +
 config/config-arm64.mak      |   3 +-
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 arm/self-modifying-test.c

-- 
2.1.4

[Qemu-devel] [kvm-unit-tests PATCH 0/2] arm/arm64: Add self-modifying code test case

[Alexander Spyridakis]

This series implements a self-modifying code test for the arm and arm64 targets.
The first patch adds the new test case, while the second modifies the arch_clean
rule for arm and arm64, to explicitly remove object files and binaries on
'make clean' (fixing and issue while reconfiguring).

The test case is meant for testing the new mttcg implementation, although it was
also verified on KVM targets like Arndale and OMAP5-uevm for ARMv7 and the Juno
board (aarch32 and aarch64 guests) for ARMv8.

Alexander Spyridakis (2):
  arm/arm64: Add self-modifying code test
  arm/arm64 config: Fix arch_clean rule

 arm/self-modifying-test.c    | 109 +++++++++++++++++++++++++++++++++++++++++++
 config/config-arm-common.mak |   2 +
 config/config-arm.mak        |   2 +
 config/config-arm64.mak      |   3 +-
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 arm/self-modifying-test.c

-- 
2.1.4

[kvm-unit-tests PATCH 1/2] arm/arm64: Add self-modifying code test

[Alexander Spyridakis]

Basic torture test that continuously modifies a single instruction
opcode and checks if all modifications were done and run as expected.

Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
---
 arm/self-modifying-test.c    | 109 +++++++++++++++++++++++++++++++++++++++++++
 config/config-arm-common.mak |   2 +
 2 files changed, 111 insertions(+)
 create mode 100644 arm/self-modifying-test.c

diff --git a/arm/self-modifying-test.c b/arm/self-modifying-test.c
new file mode 100644
index 0000000..cab47a0
--- /dev/null
+++ b/arm/self-modifying-test.c
@@ -0,0 +1,109 @@
+/*
+ * Basic self-modifying code test case
+ *
+ * Copyright (C) 2015 Virtual Open Systems SAS
+ * Author: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <asm/smp.h>
+#include <asm/mmu.h>
+#include <asm/spinlock.h>
+
+#define LOOP_SIZE 100000
+
+int result;
+static cpumask_t smp_test_complete;
+static struct spinlock lock;
+
+void self_modifying_test(void)
+{
+	extern void *smc;
+	static int toggle;
+	int i, cpu = smp_processor_id(), *ptr = (int *)&smc;
+
+	printf("CPU%d starting test\n", cpu);
+
+	for (i = 0; i < LOOP_SIZE; i++) {
+		/* Don't run concurrently with other CPUs*/
+		spin_lock(&lock);
+
+		/* A simple snippet that increments a memory value which
+		   will be modified immediately after. Before running,
+		   invalidate the instruction and data cache for that
+		   specific virtual address */
+#ifdef __arm__
+		asm("mcr	p15, 0, %0, c7, c6, 1\n" /* DCIMVAC */
+			"mcr	p15, 0, %0, c7, c5, 1\n" /* ICIMVAU */
+#else
+		asm("dc	ivac, %0\n"
+			"ic	ivau, %0\n"
+#endif
+			"dsb	ish\n"
+			"isb\n"
+			"smc:\n"
+			"add	%1, %1, #1\n"
+			"str	%1, [%2]\n"
+			:: "r" (ptr), "r" (result), "r" (&result));
+
+		/* Overwrite the previous labelled opcode,
+		   toggle between incrementing by one or two */
+		toggle ^= 1;
+		if (toggle)
+#ifdef __arm__
+			*ptr += 1;
+		else
+			*ptr -= 1;
+#else
+		{
+			*ptr &= ~(1 << 10);
+			*ptr |=  (1 << 11);
+		} else {
+			*ptr |=  (1 << 10);
+			*ptr &= ~(1 << 11);
+		}
+#endif
+
+		spin_unlock(&lock);
+	}
+
+	cpumask_set_cpu(cpu, &smp_test_complete);
+	if (cpu != 0)
+		halt();
+}
+
+int main(int argc, char **argv)
+{
+	int cpu, calc;
+	(void)argc, (void)argv;
+
+	/* Set memory as writeable, on ARMv7 we need to re-enable the MMU */
+#ifdef __arm__
+	mmu_disable();
+	flush_tlb_all();
+	mmu_set_range_ptes(mmu_idmap, PHYS_OFFSET, PHYS_OFFSET, PHYS_END,
+			__pgprot(PTE_WBWA));
+	mmu_enable(mmu_idmap);
+#else
+	mmu_set_range_ptes(mmu_idmap, PHYS_OFFSET, PHYS_OFFSET, PHYS_END,
+			__pgprot(PTE_WBWA));
+	flush_tlb_all();
+#endif
+
+	for_each_present_cpu(cpu) {
+		if (cpu == 0)
+			continue;
+		smp_boot_secondary(cpu, self_modifying_test);
+	}
+
+	self_modifying_test();
+
+	while (!cpumask_full(&smp_test_complete))
+		cpu_relax();
+
+	calc = LOOP_SIZE * nr_cpus + (LOOP_SIZE * nr_cpus / 2);
+	report("Result: %d - Expected: %d\n", result == calc, result, calc);
+
+	return report_summary();
+}
diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 698555d..74a73e4 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -10,6 +10,7 @@ ifeq ($(LOADADDR),)
 endif
 
 tests-common = \
+	$(TEST_DIR)/self-modifying-test.flat \
 	$(TEST_DIR)/selftest.flat \
 	$(TEST_DIR)/spinlock-test.flat
 
@@ -70,3 +71,4 @@ test_cases: $(generated_files) $(tests-common) $(tests)
 
 $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
 $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
+$(TEST_DIR)/self-modifying-test.elf: $(cstart.o) $(TEST_DIR)/self-modifying-test.o
-- 
2.1.4

[Qemu-devel] [kvm-unit-tests PATCH 1/2] arm/arm64: Add self-modifying code test

[Alexander Spyridakis]

Basic torture test that continuously modifies a single instruction
opcode and checks if all modifications were done and run as expected.

Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
---
 arm/self-modifying-test.c    | 109 +++++++++++++++++++++++++++++++++++++++++++
 config/config-arm-common.mak |   2 +
 2 files changed, 111 insertions(+)
 create mode 100644 arm/self-modifying-test.c

diff --git a/arm/self-modifying-test.c b/arm/self-modifying-test.c
new file mode 100644
index 0000000..cab47a0
--- /dev/null
+++ b/arm/self-modifying-test.c
@@ -0,0 +1,109 @@
+/*
+ * Basic self-modifying code test case
+ *
+ * Copyright (C) 2015 Virtual Open Systems SAS
+ * Author: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <asm/smp.h>
+#include <asm/mmu.h>
+#include <asm/spinlock.h>
+
+#define LOOP_SIZE 100000
+
+int result;
+static cpumask_t smp_test_complete;
+static struct spinlock lock;
+
+void self_modifying_test(void)
+{
+	extern void *smc;
+	static int toggle;
+	int i, cpu = smp_processor_id(), *ptr = (int *)&smc;
+
+	printf("CPU%d starting test\n", cpu);
+
+	for (i = 0; i < LOOP_SIZE; i++) {
+		/* Don't run concurrently with other CPUs*/
+		spin_lock(&lock);
+
+		/* A simple snippet that increments a memory value which
+		   will be modified immediately after. Before running,
+		   invalidate the instruction and data cache for that
+		   specific virtual address */
+#ifdef __arm__
+		asm("mcr	p15, 0, %0, c7, c6, 1\n" /* DCIMVAC */
+			"mcr	p15, 0, %0, c7, c5, 1\n" /* ICIMVAU */
+#else
+		asm("dc	ivac, %0\n"
+			"ic	ivau, %0\n"
+#endif
+			"dsb	ish\n"
+			"isb\n"
+			"smc:\n"
+			"add	%1, %1, #1\n"
+			"str	%1, [%2]\n"
+			:: "r" (ptr), "r" (result), "r" (&result));
+
+		/* Overwrite the previous labelled opcode,
+		   toggle between incrementing by one or two */
+		toggle ^= 1;
+		if (toggle)
+#ifdef __arm__
+			*ptr += 1;
+		else
+			*ptr -= 1;
+#else
+		{
+			*ptr &= ~(1 << 10);
+			*ptr |=  (1 << 11);
+		} else {
+			*ptr |=  (1 << 10);
+			*ptr &= ~(1 << 11);
+		}
+#endif
+
+		spin_unlock(&lock);
+	}
+
+	cpumask_set_cpu(cpu, &smp_test_complete);
+	if (cpu != 0)
+		halt();
+}
+
+int main(int argc, char **argv)
+{
+	int cpu, calc;
+	(void)argc, (void)argv;
+
+	/* Set memory as writeable, on ARMv7 we need to re-enable the MMU */
+#ifdef __arm__
+	mmu_disable();
+	flush_tlb_all();
+	mmu_set_range_ptes(mmu_idmap, PHYS_OFFSET, PHYS_OFFSET, PHYS_END,
+			__pgprot(PTE_WBWA));
+	mmu_enable(mmu_idmap);
+#else
+	mmu_set_range_ptes(mmu_idmap, PHYS_OFFSET, PHYS_OFFSET, PHYS_END,
+			__pgprot(PTE_WBWA));
+	flush_tlb_all();
+#endif
+
+	for_each_present_cpu(cpu) {
+		if (cpu == 0)
+			continue;
+		smp_boot_secondary(cpu, self_modifying_test);
+	}
+
+	self_modifying_test();
+
+	while (!cpumask_full(&smp_test_complete))
+		cpu_relax();
+
+	calc = LOOP_SIZE * nr_cpus + (LOOP_SIZE * nr_cpus / 2);
+	report("Result: %d - Expected: %d\n", result == calc, result, calc);
+
+	return report_summary();
+}
diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 698555d..74a73e4 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -10,6 +10,7 @@ ifeq ($(LOADADDR),)
 endif
 
 tests-common = \
+	$(TEST_DIR)/self-modifying-test.flat \
 	$(TEST_DIR)/selftest.flat \
 	$(TEST_DIR)/spinlock-test.flat
 
@@ -70,3 +71,4 @@ test_cases: $(generated_files) $(tests-common) $(tests)
 
 $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
 $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
+$(TEST_DIR)/self-modifying-test.elf: $(cstart.o) $(TEST_DIR)/self-modifying-test.o
-- 
2.1.4

[kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

Properly clean any generated object and binary files after a 'make clean',
this fixes an issue when trying to reconfigure between arm and arm64.

Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
---
 config/config-arm.mak   | 2 ++
 config/config-arm64.mak | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/config/config-arm.mak b/config/config-arm.mak
index ae6c2e7..68fab62 100644
--- a/config/config-arm.mak
+++ b/config/config-arm.mak
@@ -21,3 +21,5 @@ tests =
 include config/config-arm-common.mak
 
 arch_clean: arm_clean
+	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
+	$(TEST_DIR)/.*.d lib/arm/.*.d
diff --git a/config/config-arm64.mak b/config/config-arm64.mak
index d61b703..a0bc1b3 100644
--- a/config/config-arm64.mak
+++ b/config/config-arm64.mak
@@ -17,4 +17,5 @@ tests =
 include config/config-arm-common.mak
 
 arch_clean: arm_clean
-	$(RM) lib/arm64/.*.d
+	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
+	$(TEST_DIR)/.*.d lib/arm64/.*.d
-- 
2.1.4

[Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

Properly clean any generated object and binary files after a 'make clean',
this fixes an issue when trying to reconfigure between arm and arm64.

Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
---
 config/config-arm.mak   | 2 ++
 config/config-arm64.mak | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/config/config-arm.mak b/config/config-arm.mak
index ae6c2e7..68fab62 100644
--- a/config/config-arm.mak
+++ b/config/config-arm.mak
@@ -21,3 +21,5 @@ tests =
 include config/config-arm-common.mak
 
 arch_clean: arm_clean
+	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
+	$(TEST_DIR)/.*.d lib/arm/.*.d
diff --git a/config/config-arm64.mak b/config/config-arm64.mak
index d61b703..a0bc1b3 100644
--- a/config/config-arm64.mak
+++ b/config/config-arm64.mak
@@ -17,4 +17,5 @@ tests =
 include config/config-arm-common.mak
 
 arch_clean: arm_clean
-	$(RM) lib/arm64/.*.d
+	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
+	$(TEST_DIR)/.*.d lib/arm64/.*.d
-- 
2.1.4

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Wed, Sep 02, 2015 at 11:25:26AM +0200, Alexander Spyridakis wrote:
> Properly clean any generated object and binary files after a 'make clean',
> this fixes an issue when trying to reconfigure between arm and arm64.

Are you also running configure (with the opposite arch selected) after
'make clean'? If not, then that could be the source of your problems.
Anyway, please describe the issues you're seeing because I don't see
what this patch is doing that isn't already being done. The lines this
patch adds are already there. See the arm_clean target in
config/config-arm-common.mak.

Thanks,
drew

> 
> Signed-off-by: Alexander Spyridakis <a.spyridakis@virtualopensystems.com>
> ---
>  config/config-arm.mak   | 2 ++
>  config/config-arm64.mak | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/config/config-arm.mak b/config/config-arm.mak
> index ae6c2e7..68fab62 100644
> --- a/config/config-arm.mak
> +++ b/config/config-arm.mak
> @@ -21,3 +21,5 @@ tests =
>  include config/config-arm-common.mak
>  
>  arch_clean: arm_clean
> +	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> +	$(TEST_DIR)/.*.d lib/arm/.*.d
> diff --git a/config/config-arm64.mak b/config/config-arm64.mak
> index d61b703..a0bc1b3 100644
> --- a/config/config-arm64.mak
> +++ b/config/config-arm64.mak
> @@ -17,4 +17,5 @@ tests =
>  include config/config-arm-common.mak
>  
>  arch_clean: arm_clean
> -	$(RM) lib/arm64/.*.d
> +	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> +	$(TEST_DIR)/.*.d lib/arm64/.*.d
> -- 
> 2.1.4
> 
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> Are you also running configure (with the opposite arch selected) after
> 'make clean'? If not, then that could be the source of your problems.
> Anyway, please describe the issues you're seeing because I don't see
> what this patch is doing that isn't already being done. The lines this
> patch adds are already there. See the arm_clean target in
> config/config-arm-common.mak.

Steps to reproduce my issue:
> ./configure --arch=arm --cross-prefix=arm-linux-gnueabihf-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make
> arm/selftest.o: error adding symbols: File in wrong format

I would expect that after 'make clean', the object and binary files
are removed. Instead they are not and I have to manually remove them
before rebuilding. Running 'make clean' before and/or after
reconfiguring still produces the same issue for me.

Thanks.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> Are you also running configure (with the opposite arch selected) after
> 'make clean'? If not, then that could be the source of your problems.
> Anyway, please describe the issues you're seeing because I don't see
> what this patch is doing that isn't already being done. The lines this
> patch adds are already there. See the arm_clean target in
> config/config-arm-common.mak.

Steps to reproduce my issue:
> ./configure --arch=arm --cross-prefix=arm-linux-gnueabihf-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make
> arm/selftest.o: error adding symbols: File in wrong format

I would expect that after 'make clean', the object and binary files
are removed. Instead they are not and I have to manually remove them
before rebuilding. Running 'make clean' before and/or after
reconfiguring still produces the same issue for me.

Thanks.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> The lines this
> patch adds are already there. See the arm_clean target in
> config/config-arm-common.mak.

I see your point now. Maybe then "arm_clean" should be changed in
"arch_clean" in config-arm-common.mak?

Regards.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> The lines this
> patch adds are already there. See the arm_clean target in
> config/config-arm-common.mak.

I see your point now. Maybe then "arm_clean" should be changed in
"arch_clean" in config-arm-common.mak?

Regards.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Fri, Sep 04, 2015 at 03:48:35PM +0200, Alexander Spyridakis wrote:
> On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> > Are you also running configure (with the opposite arch selected) after
> > 'make clean'? If not, then that could be the source of your problems.
> > Anyway, please describe the issues you're seeing because I don't see
> > what this patch is doing that isn't already being done. The lines this
> > patch adds are already there. See the arm_clean target in
> > config/config-arm-common.mak.
> 
> Steps to reproduce my issue:
> > ./configure --arch=arm --cross-prefix=arm-linux-gnueabihf-
> > make
> > ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> > make clean && make
> > arm/selftest.o: error adding symbols: File in wrong format

This doesn't reproduce for me. I did the following, and it worked
fine.

make distclean
./configure --arch=arm --cross-prefix=arm-linux-gnu-
make
./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
make clean && make


But anyway I would suggest you do your 'make clean' or even a
'make distclean' *before* running configure the second time.
Otherwise you'll leave old object files around from the previously
configured arch. I.e. you want 'make clean' to apply to the
currently built config, not the new (not yet built) config.

drew

> 
> I would expect that after 'make clean', the object and binary files
> are removed. Instead they are not and I have to manually remove them
> before rebuilding. Running 'make clean' before and/or after
> reconfiguring still produces the same issue for me.
> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Fri, Sep 04, 2015 at 03:53:46PM +0200, Alexander Spyridakis wrote:
> On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> > The lines this
> > patch adds are already there. See the arm_clean target in
> > config/config-arm-common.mak.
> 
> I see your point now. Maybe then "arm_clean" should be changed in
> "arch_clean" in config-arm-common.mak?

No, the dependency chain for 'make clean' is

clean
  arch_clean (this either arm's or arm64's)
    arm_clean (this is for both arm and arm64)

> 
> Regards.
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Fri, Sep 04, 2015 at 03:53:46PM +0200, Alexander Spyridakis wrote:
> On 4 September 2015 at 12:48, Andrew Jones <drjones@redhat.com> wrote:
> > The lines this
> > patch adds are already there. See the arm_clean target in
> > config/config-arm-common.mak.
> 
> I see your point now. Maybe then "arm_clean" should be changed in
> "arch_clean" in config-arm-common.mak?

No, the dependency chain for 'make clean' is

clean
  arch_clean (this either arm's or arm64's)
    arm_clean (this is for both arm and arm64)

> 
> Regards.
> 

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Peter Maydell]

On 4 September 2015 at 15:05, Andrew Jones <drjones@redhat.com> wrote:
> But anyway I would suggest you do your 'make clean' or even a
> 'make distclean' *before* running configure the second time.
> Otherwise you'll leave old object files around from the previously
> configured arch. I.e. you want 'make clean' to apply to the
> currently built config, not the new (not yet built) config.

Separate object directories is usually the best approach if
you regularly want to build multiple configurations IMHO...

thanks
-- PMM

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Peter Maydell]

On 4 September 2015 at 15:05, Andrew Jones <drjones@redhat.com> wrote:
> But anyway I would suggest you do your 'make clean' or even a
> 'make distclean' *before* running configure the second time.
> Otherwise you'll leave old object files around from the previously
> configured arch. I.e. you want 'make clean' to apply to the
> currently built config, not the new (not yet built) config.

Separate object directories is usually the best approach if
you regularly want to build multiple configurations IMHO...

thanks
-- PMM

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> This doesn't reproduce for me. I did the following, and it worked
> fine.
>
> make distclean
> ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make

This is very troubling then as with the same commands the binaries are
still there. I need to check what's wrong with my setup, although I
managed to have the same issue on a newly created docker container...

Thanks for trying to replicate, I will investigate more.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> This doesn't reproduce for me. I did the following, and it worked
> fine.
>
> make distclean
> ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make

This is very troubling then as with the same commands the binaries are
still there. I need to check what's wrong with my setup, although I
managed to have the same issue on a newly created docker container...

Thanks for trying to replicate, I will investigate more.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> This doesn't reproduce for me. I did the following, and it worked
> fine.
>
> make distclean
> ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make

Ok I think I found the issue:

config-arm-common.mak:
>arm_clean: libfdt_clean asm_offsets_clean
>    $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
>          $(TEST_DIR)/.*.d lib/arm/.*.d

config-x86-common.mak:
>arch_clean:
>    $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>    $(TEST_DIR)/.*.d lib/x86/.*.d

I think the arm case tries to be too clever and on many systems it
fails (tested on debian:jessie,sid and ubuntu:14.04,15.04). Basically
the expression for the arm case fails to resolve, while using the
simpler x86 way works as expected.

So is the following change acceptable in config-arm-common.mak?
> arm_clean: libfdt_clean asm_offsets_clean
>-       $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
>-             $(TEST_DIR)/.*.d lib/arm/.*.d
>+       $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>+               $(libeabi) $(eabiobjs) $(TEST_DIR)/.*.d lib/arm/.*.d

Thanks.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> This doesn't reproduce for me. I did the following, and it worked
> fine.
>
> make distclean
> ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> make
> ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> make clean && make

Ok I think I found the issue:

config-arm-common.mak:
>arm_clean: libfdt_clean asm_offsets_clean
>    $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
>          $(TEST_DIR)/.*.d lib/arm/.*.d

config-x86-common.mak:
>arch_clean:
>    $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>    $(TEST_DIR)/.*.d lib/x86/.*.d

I think the arm case tries to be too clever and on many systems it
fails (tested on debian:jessie,sid and ubuntu:14.04,15.04). Basically
the expression for the arm case fails to resolve, while using the
simpler x86 way works as expected.

So is the following change acceptable in config-arm-common.mak?
> arm_clean: libfdt_clean asm_offsets_clean
>-       $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
>-             $(TEST_DIR)/.*.d lib/arm/.*.d
>+       $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>+               $(libeabi) $(eabiobjs) $(TEST_DIR)/.*.d lib/arm/.*.d

Thanks.

Re: [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Mon, Sep 07, 2015 at 03:35:19PM +0200, Alexander Spyridakis wrote:
> On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> > This doesn't reproduce for me. I did the following, and it worked
> > fine.
> >
> > make distclean
> > ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> > make
> > ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> > make clean && make
> 
> Ok I think I found the issue:
> 
> config-arm-common.mak:
> >arm_clean: libfdt_clean asm_offsets_clean
> >    $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
> >          $(TEST_DIR)/.*.d lib/arm/.*.d
> 
> config-x86-common.mak:
> >arch_clean:
> >    $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >    $(TEST_DIR)/.*.d lib/x86/.*.d
> 
> I think the arm case tries to be too clever and on many systems it
> fails (tested on debian:jessie,sid and ubuntu:14.04,15.04). Basically
> the expression for the arm case fails to resolve, while using the
> simpler x86 way works as expected.
> 
> So is the following change acceptable in config-arm-common.mak?
> > arm_clean: libfdt_clean asm_offsets_clean
> >-       $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
> >-             $(TEST_DIR)/.*.d lib/arm/.*.d
> >+       $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >+               $(libeabi) $(eabiobjs) $(TEST_DIR)/.*.d lib/arm/.*.d

Ah, it's a dash vs. bash thing. Either we need to change all bashisms
in the makefiles, or, since kvm-unit-tests already depends on bash for
its scripts, then we might as well just tell make to use it too.  This
patch will fix it

diff --git a/Makefile b/Makefile
index 0d5933474cd8c..3e60b4f8e4a57 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,6 @@
 
+SHELL := /bin/bash
+
 ifeq ($(wildcard config.mak),)
 $(error run ./configure first. See ./configure -h)
 endif

I'll probably submit the patch in a second.

Thanks for hunting down the problem!

drew

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Andrew Jones]

On Mon, Sep 07, 2015 at 03:35:19PM +0200, Alexander Spyridakis wrote:
> On 4 September 2015 at 16:05, Andrew Jones <drjones@redhat.com> wrote:
> > This doesn't reproduce for me. I did the following, and it worked
> > fine.
> >
> > make distclean
> > ./configure --arch=arm --cross-prefix=arm-linux-gnu-
> > make
> > ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu-
> > make clean && make
> 
> Ok I think I found the issue:
> 
> config-arm-common.mak:
> >arm_clean: libfdt_clean asm_offsets_clean
> >    $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
> >          $(TEST_DIR)/.*.d lib/arm/.*.d
> 
> config-x86-common.mak:
> >arch_clean:
> >    $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >    $(TEST_DIR)/.*.d lib/x86/.*.d
> 
> I think the arm case tries to be too clever and on many systems it
> fails (tested on debian:jessie,sid and ubuntu:14.04,15.04). Basically
> the expression for the arm case fails to resolve, while using the
> simpler x86 way works as expected.
> 
> So is the following change acceptable in config-arm-common.mak?
> > arm_clean: libfdt_clean asm_offsets_clean
> >-       $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
> >-             $(TEST_DIR)/.*.d lib/arm/.*.d
> >+       $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> >+               $(libeabi) $(eabiobjs) $(TEST_DIR)/.*.d lib/arm/.*.d

Ah, it's a dash vs. bash thing. Either we need to change all bashisms
in the makefiles, or, since kvm-unit-tests already depends on bash for
its scripts, then we might as well just tell make to use it too.  This
patch will fix it

diff --git a/Makefile b/Makefile
index 0d5933474cd8c..3e60b4f8e4a57 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,6 @@
 
+SHELL := /bin/bash
+
 ifeq ($(wildcard config.mak),)
 $(error run ./configure first. See ./configure -h)
 endif

I'll probably submit the patch in a second.

Thanks for hunting down the problem!

drew

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 7 September 2015 at 16:37, Andrew Jones <drjones@redhat.com> wrote:
> +SHELL := /bin/bash

This indeed fixed the issue.

Thanks.

Re: [Qemu-devel] [kvm-unit-tests PATCH 2/2] arm/arm64 config: Fix arch_clean rule

[Alexander Spyridakis]

On 7 September 2015 at 16:37, Andrew Jones <drjones@redhat.com> wrote:
> +SHELL := /bin/bash

This indeed fixed the issue.

Thanks.