From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53683) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yrkma-0003Br-7n for qemu-devel@nongnu.org; Mon, 11 May 2015 06:17:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YrkmW-0002PB-0K for qemu-devel@nongnu.org; Mon, 11 May 2015 06:17:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38182) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YrkmV-0002Oa-PT for qemu-devel@nongnu.org; Mon, 11 May 2015 06:17:51 -0400 Date: Mon, 11 May 2015 12:17:40 +0200 From: Andrew Jones Message-ID: <20150511101740.GA2923@localhost.localdomain> References: <1430998302-31502-1-git-send-email-a.spyridakis@virtualopensystems.com> <1430998302-31502-3-git-send-email-a.spyridakis@virtualopensystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1430998302-31502-3-git-send-email-a.spyridakis@virtualopensystems.com> Subject: Re: [Qemu-devel] [PATCH 2/2] atomic-test: Add spinlock test case List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Spyridakis Cc: mttcg@greensocs.com, jani.kokkonen@huawei.com, tech@virtualopensystems.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org On Thu, May 07, 2015 at 01:31:42PM +0200, Alexander Spyridakis wrote: > Sample spinlock test case with the option to implement the spinlock > by means of GCC atomic instructions or unsafe memory operations. > Additionally, printf is wrapped around a spinlock to avoid concurrent > access to the serial device. > > Suggested-by: Jani Kokkonen > Suggested-by: Claudio Fontana > Signed-off-by: Alexander Spyridakis > --- > tests/atomic-test/Makefile | 3 +++ > tests/atomic-test/helpers.c | 21 +++++++++++++++++++++ > tests/atomic-test/helpers.h | 12 ++++++++++++ > tests/atomic-test/main.c | 35 ++++++++++++++++++++++++++++++++++- > tests/atomic-test/printf.c | 5 +++++ > 5 files changed, 75 insertions(+), 1 deletion(-) > > diff --git a/tests/atomic-test/Makefile b/tests/atomic-test/Makefile > index 094e01a..d1e992d 100644 > --- a/tests/atomic-test/Makefile > +++ b/tests/atomic-test/Makefile > @@ -12,6 +12,9 @@ LD_SCRIPT = link.ld.S > LIBS = $(shell $(CC) $(CCFLAGS) -print-libgcc-file-name) > CPPFLAGS += -gdwarf-2 -fno-stack-protector -nostdinc -fno-builtin > > +# Set to ATOMIC to implement spinlock test with real atomic instructions > +TEST = ATOMIC > + > # > # Target specific variables > # > diff --git a/tests/atomic-test/helpers.c b/tests/atomic-test/helpers.c > index 8ac8c2c..fd2d4cf 100644 > --- a/tests/atomic-test/helpers.c > +++ b/tests/atomic-test/helpers.c > @@ -82,3 +82,24 @@ void power_off(void) > /* Shut down system */ > psci_call(PSCI_SYSTEM_OFF, 0, 0, 0); > } > + > +void atomic_lock(int *lock_var) > +{ > + while (__sync_lock_test_and_set(lock_var, 1)); > +} > + > +void atomic_unlock(int *lock_var) > +{ > + __sync_lock_release(lock_var); > +} Do these builtins actually do anything without enabling the MMU first? > + > +void non_atomic_lock(int *lock_var) > +{ > + while (*lock_var != 0); > + *lock_var = 1; > +} > + > +void non_atomic_unlock(int *lock_var) > +{ > + *lock_var = 0; > +} > diff --git a/tests/atomic-test/helpers.h b/tests/atomic-test/helpers.h > index 66d440e..93036be 100644 > --- a/tests/atomic-test/helpers.h > +++ b/tests/atomic-test/helpers.h > @@ -29,6 +29,18 @@ > #define PSCI_CPU_OFF 0x84000002 > #define PSCI_SYSTEM_OFF 0x84000008 > > +#ifdef ATOMIC > +#define LOCK atomic_lock > +#define UNLOCK atomic_unlock > +#else > +#define LOCK non_atomic_lock > +#define UNLOCK non_atomic_unlock > +#endif > + > +int global_lock; > +int global_a; > +int global_b; > + > int get_cpuid(void); > void power_secondary(void); > void power_off(); > diff --git a/tests/atomic-test/main.c b/tests/atomic-test/main.c > index 72eaf59..3143f7c 100644 > --- a/tests/atomic-test/main.c > +++ b/tests/atomic-test/main.c > @@ -15,9 +15,42 @@ > * along with this program. If not, see . > */ > > +#include "helpers.h" > + > +#define LOOP_SIZE 1000000 > + > +void test_spinlock() > +{ > + int i, errors = 0; > + int cpu = get_cpuid(); > + > + for (i = 0; i < LOOP_SIZE; i++) { > + LOCK(&global_lock); > + > + if (global_a == (cpu + 1) % 2) { > + global_a = 1; > + global_b = 0; > + } else { > + global_a = 0; > + global_b = 1; > + } > + > + if (global_a == global_b) { > + errors++; > + } > + UNLOCK(&global_lock); > + } > + > + printf("CPU%d: Done - Errors: %d\n", cpu, errors); > +} > + > void main(void) > { > - printf("CPU %d on\n", get_cpuid()); > + if (!get_cpuid()) { > + printf("Starting test\n"); > + } > + > + test_spinlock(); > power_off(); > } You could have saved a ton of time by just putting these 50 lines into a new kvm-unit-tests test. If you need the mmu disabled for some reason, then we can add an mmu_disable() to the API. drew