[kvm-unit-tests PATCH 0/2] one fix, one resend

[kvm-unit-tests PATCH 0/2] one fix, one resend

[Andrew Jones]

Need to match code to intent in spinlock-test, and
resending the 'make TEST=' patch, because I find it quite
useful and like it. Hey, it was my birthday a couple weeks
ago - give it to me for a present?

Andrew Jones (2):
  arm/arm64: spinlock-test fixup
  arm/arm64: allow building a single test

 arm/spinlock-test.c          | 16 ++++++++++++----
 arm/unittests.cfg            |  6 ++++++
 config/config-arm-common.mak |  6 ++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.4.3

[kvm-unit-tests PATCH 1/2] arm/arm64: spinlock-test fixup

[Andrew Jones]

I hadn't plan to post this for kvm-unit-tests, but Paolo picked it
up. That's fine, but now we should add its header. Also, Paolo
tweaked the original to "use atomics by default", but it actually
was using non-atomic by default. Switch to atomics, and rename
the parameter from 'bad' (which is a bit ambiguous) to 'non-atomic'.
And, let's give it its own place in unittests.cfg, but leave it
disabled for now.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/spinlock-test.c | 16 ++++++++++++----
 arm/unittests.cfg   |  6 ++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arm/spinlock-test.c b/arm/spinlock-test.c
index 1dcf2b144a134..1f91edf4116a4 100644
--- a/arm/spinlock-test.c
+++ b/arm/spinlock-test.c
@@ -1,3 +1,11 @@
+/*
+ * Port of the spinlock torture test from virtualopensystems
+ * tcg_baremetal_tests
+ *
+ * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
 #include <libcflat.h>
 #include <asm/smp.h>
 #include <asm/cpumask.h>
@@ -69,12 +77,12 @@ int main(int argc, char **argv)
 {
 	int cpu;
 
-	if (argc && strcmp(argv[0], "bad") != 0) {
-		lock_ops.lock = gcc_builtin_lock;
-		lock_ops.unlock = gcc_builtin_unlock;
-	} else {
+	if (argc && strcmp(argv[0], "non-atomic") != 0) {
 		lock_ops.lock = none_lock;
 		lock_ops.unlock = none_unlock;
+	} else {
+		lock_ops.lock = gcc_builtin_lock;
+		lock_ops.unlock = gcc_builtin_unlock;
 	}
 
 	for_each_present_cpu(cpu) {
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index ee655b2678a4e..9993d133bc394 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -35,3 +35,9 @@ file = selftest.flat
 smp = $(getconf _NPROCESSORS_CONF)
 extra_params = -append 'smp'
 groups = selftest
+
+# Test spinlocks
+#[smp::spinlocks]
+#file = spinlock-test.flat
+#smp = $(getconf _NPROCESSORS_CONF)
+#groups = smp
-- 
2.4.3

Re: [kvm-unit-tests PATCH 1/2] arm/arm64: spinlock-test fixup

[Paolo Bonzini]

On 06/07/2015 15:31, Andrew Jones wrote:
> +	if (argc && strcmp(argv[0], "non-atomic") != 0) {

You want == here, don't you?

The reason I switched was that the non-atomic test didn't work for me.
I have now debugged it, and it needs this:

@@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var)
 }
 static void none_lock(int *lock_var)
 {
-       while (*lock_var != 0);
-       *lock_var = 1;
+       while (*(volatile int *)lock_var != 0);
+       *(volatile int *)lock_var = 1;
 }
 static void none_unlock(int *lock_var)
 {
-       *lock_var = 0;
+       *(volatile int *)lock_var = 0;
 }

 static int global_a, global_b;

Otherwise the none_lock function does not reload lock_var.

Paolo

>  		lock_ops.lock = none_lock;
>  		lock_ops.unlock = none_unlock;
> +	} else {
> +		lock_ops.lock = gcc_builtin_lock;
> +		lock_ops.unlock = gcc_builtin_unlock;
>  	}

Re: [kvm-unit-tests PATCH 1/2] arm/arm64: spinlock-test fixup

[Andrew Jones]

On Mon, Jul 06, 2015 at 07:51:40PM +0200, Paolo Bonzini wrote:
> 
> 
> On 06/07/2015 15:31, Andrew Jones wrote:
> > +	if (argc && strcmp(argv[0], "non-atomic") != 0) {
> 
> You want == here, don't you?

Ah, now I see that your default was correct before. Sorry that
I didn't looked closely enough to see that you flipped that logic.

> 
> The reason I switched was that the non-atomic test didn't work for me.
> I have now debugged it, and it needs this:
> 
> @@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var)
>  }
>  static void none_lock(int *lock_var)
>  {
> -       while (*lock_var != 0);
> -       *lock_var = 1;
> +       while (*(volatile int *)lock_var != 0);
> +       *(volatile int *)lock_var = 1;
>  }
>  static void none_unlock(int *lock_var)
>  {
> -       *lock_var = 0;
> +       *(volatile int *)lock_var = 0;
>  }
> 
>  static int global_a, global_b;
> 
> Otherwise the none_lock function does not reload lock_var.

Good call. I'll send a v2 of this patch that puts this in.

Thanks,
drew

> 
> Paolo
> 
> >  		lock_ops.lock = none_lock;
> >  		lock_ops.unlock = none_unlock;
> > +	} else {
> > +		lock_ops.lock = gcc_builtin_lock;
> > +		lock_ops.unlock = gcc_builtin_unlock;
> >  	}
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 1/2] arm/arm64: spinlock-test fixup

[Paolo Bonzini]

On 07/07/2015 13:24, Andrew Jones wrote:
>> > The reason I switched was that the non-atomic test didn't work for me.
>> > I have now debugged it, and it needs this:
>> > 
>> > @@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var)
>> >  }
>> >  static void none_lock(int *lock_var)
>> >  {
>> > -       while (*lock_var != 0);
>> > -       *lock_var = 1;
>> > +       while (*(volatile int *)lock_var != 0);
>> > +       *(volatile int *)lock_var = 1;
>> >  }
>> >  static void none_unlock(int *lock_var)
>> >  {
>> > -       *lock_var = 0;
>> > +       *(volatile int *)lock_var = 0;
>> >  }
>> > 
>> >  static int global_a, global_b;
>> > 
>> > Otherwise the none_lock function does not reload lock_var.
> Good call. I'll send a v2 of this patch that puts this in.

No need to, I have already applied this patch, and my fixup on top.

Paolo

[kvm-unit-tests PATCH 2/2] arm/arm64: allow building a single test

[Andrew Jones]

This is mostly useful for building new tests that don't yet (and
may never) have entries in the makefiles (config-arm*.mak). Of course
it can be used to build tests that do have entries as well, in order
to avoid building all tests, if the plan is to run just the one.

Just do 'make TEST=some-test' to use it, where "some-test" matches
the name of the source file, i.e. arm/some-test.c

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 config/config-arm-common.mak | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 0674daaa476d7..e18c3ea678f09 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -13,6 +13,11 @@ tests-common = \
 	$(TEST_DIR)/selftest.flat \
 	$(TEST_DIR)/spinlock-test.flat
 
+ifneq ($(TEST),)
+	tests = $(TEST_DIR)/$(TEST).flat
+	tests-common =
+endif
+
 all: test_cases
 
 ##################################################################
@@ -70,5 +75,6 @@ generated_files = $(asm-offsets)
 
 test_cases: $(generated_files) $(tests-common) $(tests)
 
+$(TEST_DIR)/$(TEST).elf: $(cstart.o) $(TEST_DIR)/$(TEST).o
 $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
 $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
-- 
2.4.3