| Message ID | 20150625154432.GA6535@hawk.localdomain |
|---|---|
| State | New |
| Headers | show |
Andrew Jones <drjones@redhat.com> writes: > On Wed, Jun 24, 2015 at 08:12:52PM +0100, Peter Maydell wrote: >> On 24 June 2015 at 18:18, Alex Bennée <alex.bennee@linaro.org> wrote: >> > >> > Paolo Bonzini <pbonzini@redhat.com> writes: >> > >> >> On 24/06/2015 17:34, Alex Bennée wrote: >> >>> Testing with Alexander's bare metal syncronisation tests fails in MTTCG >> >>> leaving one CPU spinning forever waiting for the second CPU to wake up. >> >>> We simply need to poke the halt_cond once we have processed the PSCI >> >>> power on call. >> >>> >> >>> Tested-by: Alex Bennée <alex.bennee@linaro.org> >> >>> CC: Alexander Spyridakis <a.spyridakis@virtualopensystems.com> >> >>> >> >>> --- >> >>> TODO >> >>> - exactly how does the vexpress wake up it's sleeping CPUs? >> >>> --- >> >>> target-arm/psci.c | 2 ++ >> >>> 1 file changed, 2 insertions(+) >> >>> >> >>> diff --git a/target-arm/psci.c b/target-arm/psci.c >> >>> index d8fafab..661ff28 100644 >> >>> --- a/target-arm/psci.c >> >>> +++ b/target-arm/psci.c >> >>> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu) >> >>> } >> >>> target_cpu_class->set_pc(target_cpu_state, entry); >> >>> >> >>> + qemu_cond_signal(target_cpu_state->halt_cond); >> >> >> >> That's called qemu_cpu_kick(target_cpu_state). :) The patch should be >> >> acceptable now upstream, I think. >> > >> > Oh so this might well fail in KVM too? >> >> In KVM we won't use target-arm/psci.c because PSCI calls >> are handled in the kernel. >> > > It's also not valid to use Alexander's test on KVM, as the test > framework doesn't enable the mmu, and thus {ldr,str}ex won't work > as expected. > > I guess I need to do a better job at advertising/documenting > kvm-unit-tests/arm, as that framework would suit this test just > fine. I've attached a patch porting the test over to k-u-t[1]. > After applying the patch, do > > ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu- > OR > ./configure --arch=arm --cross-prefix=arm-linux-gnu- > > make > > arm/run arm/vos-spinlock-test.flat -smp 4 # non-atomic locks > OR > arm/run arm/vos-spinlock-test.flat -smp 4 -append atomic # atomic > locks Thanks for that. I shall have a play with it today. > > Thanks, > drew > > [1] git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
On Fri, Jun 26, 2015 at 08:06:55AM +0100, Alex Bennée wrote: > > Andrew Jones <drjones@redhat.com> writes: > > > On Wed, Jun 24, 2015 at 08:12:52PM +0100, Peter Maydell wrote: > >> On 24 June 2015 at 18:18, Alex Bennée <alex.bennee@linaro.org> wrote: > >> > > >> > Paolo Bonzini <pbonzini@redhat.com> writes: > >> > > >> >> On 24/06/2015 17:34, Alex Bennée wrote: > >> >>> Testing with Alexander's bare metal syncronisation tests fails in MTTCG > >> >>> leaving one CPU spinning forever waiting for the second CPU to wake up. > >> >>> We simply need to poke the halt_cond once we have processed the PSCI > >> >>> power on call. > >> >>> > >> >>> Tested-by: Alex Bennée <alex.bennee@linaro.org> > >> >>> CC: Alexander Spyridakis <a.spyridakis@virtualopensystems.com> > >> >>> > >> >>> --- > >> >>> TODO > >> >>> - exactly how does the vexpress wake up it's sleeping CPUs? > >> >>> --- > >> >>> target-arm/psci.c | 2 ++ > >> >>> 1 file changed, 2 insertions(+) > >> >>> > >> >>> diff --git a/target-arm/psci.c b/target-arm/psci.c > >> >>> index d8fafab..661ff28 100644 > >> >>> --- a/target-arm/psci.c > >> >>> +++ b/target-arm/psci.c > >> >>> @@ -196,6 +196,8 @@ void arm_handle_psci_call(ARMCPU *cpu) > >> >>> } > >> >>> target_cpu_class->set_pc(target_cpu_state, entry); > >> >>> > >> >>> + qemu_cond_signal(target_cpu_state->halt_cond); > >> >> > >> >> That's called qemu_cpu_kick(target_cpu_state). :) The patch should be > >> >> acceptable now upstream, I think. > >> > > >> > Oh so this might well fail in KVM too? > >> > >> In KVM we won't use target-arm/psci.c because PSCI calls > >> are handled in the kernel. > >> > > > > It's also not valid to use Alexander's test on KVM, as the test > > framework doesn't enable the mmu, and thus {ldr,str}ex won't work > > as expected. > > > > I guess I need to do a better job at advertising/documenting > > kvm-unit-tests/arm, as that framework would suit this test just > > fine. I've attached a patch porting the test over to k-u-t[1]. > > After applying the patch, do > > > > ./configure --arch=arm64 --cross-prefix=aarch64-linux-gnu- > > OR > > ./configure --arch=arm --cross-prefix=arm-linux-gnu- > > > > make One more step here, that you probably already figured out export QEMU=<path-to-qemu-you-want-to-test> > > > > arm/run arm/vos-spinlock-test.flat -smp 4 # non-atomic locks > > OR > > arm/run arm/vos-spinlock-test.flat -smp 4 -append atomic # atomic > > locks > > Thanks for that. I shall have a play with it today. Feel free to make suggestions/patches for the framework, and, of course, to write unit tests :-) drew > > > > > Thanks, > > drew > > > > [1] git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git > > -- > Alex Bennée > >
diff --git a/arm/vos-spinlock-test.c b/arm/vos-spinlock-test.c new file mode 100644 index 0000000000000..6cc62c6f49dfb --- /dev/null +++ b/arm/vos-spinlock-test.c @@ -0,0 +1,92 @@ +#include <libcflat.h> +#include <asm/smp.h> +#include <asm/cpumask.h> +#include <asm/barrier.h> + +#define LOOP_SIZE 10000000 + +struct lock_ops { + void (*lock)(int *v); + void (*unlock)(int *v); +}; +static struct lock_ops lock_ops; + +static void gcc_builtin_lock(int *lock_var) +{ + while (__sync_lock_test_and_set(lock_var, 1)); +} +static void gcc_builtin_unlock(int *lock_var) +{ + __sync_lock_release(lock_var); +} +static void none_lock(int *lock_var) +{ + while (*lock_var != 0); + *lock_var = 1; +} +static void none_unlock(int *lock_var) +{ + *lock_var = 0; +} + +static int global_a, global_b; +static int global_lock; + +static cpumask_t smp_test_complete; + +static void test_spinlock(void) +{ + int i, errors = 0; + int cpu = smp_processor_id(); + + printf("CPU%d online\n", cpu); + + for (i = 0; i < LOOP_SIZE; i++) { + + lock_ops.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++; + + lock_ops.unlock(&global_lock); + } + report("CPU%d: Done - Errors: %d\n", errors == 0, cpu, errors); + + cpumask_set_cpu(cpu, &smp_test_complete); + if (cpu != 0) + halt(); +} + +int main(int argc, char **argv) +{ + int cpu; + + if (argc && strcmp(argv[0], "atomic") == 0) { + lock_ops.lock = gcc_builtin_lock; + lock_ops.unlock = gcc_builtin_unlock; + } else { + lock_ops.lock = none_lock; + lock_ops.unlock = none_unlock; + } + + for_each_present_cpu(cpu) { + if (cpu == 0) + continue; + smp_boot_secondary(cpu, test_spinlock); + } + + test_spinlock(); + + while (!cpumask_full(&smp_test_complete)) + cpu_relax(); + + return report_summary(); +} diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak index 314261ef60cf7..6f4c8ee163c3d 100644 --- a/config/config-arm-common.mak +++ b/config/config-arm-common.mak @@ -10,7 +10,8 @@ ifeq ($(LOADADDR),) endif tests-common = \ - $(TEST_DIR)/selftest.flat + $(TEST_DIR)/selftest.flat \ + $(TEST_DIR)/vos-spinlock-test.flat all: test_cases @@ -70,3 +71,4 @@ generated_files = $(asm-offsets) test_cases: $(generated_files) $(tests-common) $(tests) $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o +$(TEST_DIR)/vos-spinlock-test.elf: $(cstart.o) $(TEST_DIR)/vos-spinlock-test.o