All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
@ 2015-06-24 15:34 Alex Bennée
  2015-06-24 16:01 ` Paolo Bonzini
  2015-06-24 23:55 ` Alexander Spyridakis
  0 siblings, 2 replies; 12+ messages in thread
From: Alex Bennée @ 2015-06-24 15:34 UTC (permalink / raw)
  To: mttcg, mark.burton, fred.konrad
  Cc: peter.maydell, Alex Bennée, qemu-devel, Alexander Spyridakis

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);
+
         ret = 0;
         break;
     case QEMU_PSCI_0_1_FN_CPU_OFF:
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 15:34 [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG) Alex Bennée
@ 2015-06-24 16:01 ` Paolo Bonzini
  2015-06-24 17:18   ` Alex Bennée
  2015-06-24 23:55 ` Alexander Spyridakis
  1 sibling, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-06-24 16:01 UTC (permalink / raw)
  To: Alex Bennée, mttcg, mark.burton, fred.konrad
  Cc: peter.maydell, qemu-devel, Alexander Spyridakis



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.

Paolo

>          ret = 0;
>          break;
>      case QEMU_PSCI_0_1_FN_CPU_OFF:
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 16:01 ` Paolo Bonzini
@ 2015-06-24 17:18   ` Alex Bennée
  2015-06-24 17:23     ` Paolo Bonzini
  2015-06-24 19:12     ` Peter Maydell
  0 siblings, 2 replies; 12+ messages in thread
From: Alex Bennée @ 2015-06-24 17:18 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: mttcg, peter.maydell, Alexander Spyridakis, mark.burton,
	qemu-devel, fred.konrad


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?

The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?

>
> Paolo
>
>>          ret = 0;
>>          break;
>>      case QEMU_PSCI_0_1_FN_CPU_OFF:
>> 

-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 17:18   ` Alex Bennée
@ 2015-06-24 17:23     ` Paolo Bonzini
  2015-06-24 18:15       ` Alex Bennée
  2015-06-24 19:12     ` Peter Maydell
  1 sibling, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-06-24 17:23 UTC (permalink / raw)
  To: Alex Bennée
  Cc: mttcg, peter.maydell, Alexander Spyridakis, mark.burton,
	qemu-devel, fred.konrad



On 24/06/2015 19:18, Alex Bennée wrote:
>>> >> @@ -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?
> 
> The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
> a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?

On KVM (and I assume on MT-TCG), each CPU has a different halt_cond.

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 17:23     ` Paolo Bonzini
@ 2015-06-24 18:15       ` Alex Bennée
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2015-06-24 18:15 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: mttcg, peter.maydell, Alexander Spyridakis, mark.burton,
	qemu-devel, fred.konrad


Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/06/2015 19:18, Alex Bennée wrote:
>>>> >> @@ -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?
>> 
>> The qemu_cpu_kick does a qemu_cond_broadcast(cpu->halt_cond) which seems
>> a little excessive? Won't all sleeping CPUs wake up (and return to sleep)?
>
> On KVM (and I assume on MT-TCG), each CPU has a different halt_cond.

You are right of course, I got my sense the wrong way around. Given it
is per-cpu I wonder if you will ever have multiple threads waiting on
it?

Anyway I'll fix that up and re-submit after I've tested to see of these
test cases break current KVM.

>
> Paolo

-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 17:18   ` Alex Bennée
  2015-06-24 17:23     ` Paolo Bonzini
@ 2015-06-24 19:12     ` Peter Maydell
  2015-06-25 15:44       ` Andrew Jones
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2015-06-24 19:12 UTC (permalink / raw)
  To: Alex Bennée
  Cc: mttcg, Alexander Spyridakis, Mark Burton, QEMU Developers,
	Paolo Bonzini, KONRAD Frédéric

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.

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 15:34 [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG) Alex Bennée
  2015-06-24 16:01 ` Paolo Bonzini
@ 2015-06-24 23:55 ` Alexander Spyridakis
  2015-06-25  6:27   ` Alex Bennée
  2015-06-25 12:43   ` Frederic Konrad
  1 sibling, 2 replies; 12+ messages in thread
From: Alexander Spyridakis @ 2015-06-24 23:55 UTC (permalink / raw)
  To: Alex Bennée
  Cc: mttcg, Peter Maydell, Alexander Spyridakis, Mark Burton,
	QEMU Developers, KONRAD Frédéric

On 24 June 2015 at 17:34, Alex Bennée <alex.bennee@linaro.org> 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.

Thanks Alex. Works for me, also with qemu_cpu_kick(target_cpu_state)
as Paolo mentioned.

The test seems to stress the current multi-threaded implementation
quite a lot. With 8 CPUs running, the resulting errors are in the
range of 500 per vCPU (10 million iterations).
Performance is another issue as mentioned before, but even more
pronounced with 8 cores. Upstream QEMU needs around 10 seconds to
complete, with multi-threading around 100 seconds for the same test.

Best regards.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 23:55 ` Alexander Spyridakis
@ 2015-06-25  6:27   ` Alex Bennée
  2015-06-25 12:43   ` Frederic Konrad
  1 sibling, 0 replies; 12+ messages in thread
From: Alex Bennée @ 2015-06-25  6:27 UTC (permalink / raw)
  To: Alexander Spyridakis
  Cc: mttcg, Peter Maydell, Mark Burton, QEMU Developers,
	KONRAD Frédéric


Alexander Spyridakis <a.spyridakis@virtualopensystems.com> writes:

> On 24 June 2015 at 17:34, Alex Bennée <alex.bennee@linaro.org> 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.
>
> Thanks Alex. Works for me, also with qemu_cpu_kick(target_cpu_state)
> as Paolo mentioned.
>
> The test seems to stress the current multi-threaded implementation
> quite a lot. With 8 CPUs running, the resulting errors are in the
> range of 500 per vCPU (10 million iterations).

We need to get to the bottom of this one first as obviously the
implementation needs to bullet proof for all the various synchronisation
patterns the CPU can use.

> Performance is another issue as mentioned before, but even more
> pronounced with 8 cores. Upstream QEMU needs around 10 seconds to
> complete, with multi-threading around 100 seconds for the same test.

I'm not overly surprised as this is a high-contention test and the
additional locking implies a lot of extra overhead. It's certainly a
useful test to compare the comparative performance of the various
approaches to atomics/exclusives but I hope in real world tasks we gain
a bunch of performance for normal unlocked code running across multiple
cores.

I wonder if the perf tools can give us some insight to where the extra
latency is coming from?

>
> Best regards.

-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 23:55 ` Alexander Spyridakis
  2015-06-25  6:27   ` Alex Bennée
@ 2015-06-25 12:43   ` Frederic Konrad
  1 sibling, 0 replies; 12+ messages in thread
From: Frederic Konrad @ 2015-06-25 12:43 UTC (permalink / raw)
  To: Alexander Spyridakis, Alex Bennée
  Cc: mttcg, Peter Maydell, Mark Burton, QEMU Developers

On 25/06/2015 01:55, Alexander Spyridakis wrote:
> On 24 June 2015 at 17:34, Alex Bennée <alex.bennee@linaro.org> 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.
> Thanks Alex. Works for me, also with qemu_cpu_kick(target_cpu_state)
> as Paolo mentioned.
>
> The test seems to stress the current multi-threaded implementation
> quite a lot. With 8 CPUs running, the resulting errors are in the
> range of 500 per vCPU (10 million iterations).
> Performance is another issue as mentioned before, but even more
> pronounced with 8 cores. Upstream QEMU needs around 10 seconds to
> complete, with multi-threading around 100 seconds for the same test.
>
> Best regards.
Hi,

I can reproduce the atomic errors with the test something is definitely 
wrong.
I know about the performance issue I fixed it in the patch-set I'll send.

Thanks,
Fred

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-24 19:12     ` Peter Maydell
@ 2015-06-25 15:44       ` Andrew Jones
  2015-06-26  7:06         ` Alex Bennée
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Jones @ 2015-06-25 15:44 UTC (permalink / raw)
  To: Peter Maydell
  Cc: mttcg, Alexander Spyridakis, Mark Burton, QEMU Developers,
	Paolo Bonzini, Alex Bennée, KONRAD Frédéric

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

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,
drew

[1] git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git

[-- Attachment #2: vos-spinlock-test.patch --]
[-- Type: text/plain, Size: 3054 bytes --]

>From dea26966a92cc65f23c0ac7bf2620982e0a9c57d Mon Sep 17 00:00:00 2001
From: Andrew Jones <drjones@redhat.com>
Date: Thu, 25 Jun 2015 17:28:53 +0200
Subject: [PATCH] arm/arm64: add spinlock torture test

This is port of the test in virtualopensystems tcg_baremetal_tests.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/vos-spinlock-test.c      | 92 ++++++++++++++++++++++++++++++++++++++++++++
 config/config-arm-common.mak |  4 +-
 2 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 arm/vos-spinlock-test.c

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
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-25 15:44       ` Andrew Jones
@ 2015-06-26  7:06         ` Alex Bennée
  2015-06-26  8:05           ` Andrew Jones
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Bennée @ 2015-06-26  7:06 UTC (permalink / raw)
  To: Andrew Jones
  Cc: mttcg, Peter Maydell, Alexander Spyridakis, Mark Burton,
	QEMU Developers, Paolo Bonzini, KONRAD Frédéric


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

-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG)
  2015-06-26  7:06         ` Alex Bennée
@ 2015-06-26  8:05           ` Andrew Jones
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jones @ 2015-06-26  8:05 UTC (permalink / raw)
  To: Alex Bennée
  Cc: mttcg, Peter Maydell, Alexander Spyridakis, Mark Burton,
	QEMU Developers, Paolo Bonzini, KONRAD Frédéric

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
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-06-26  8:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-24 15:34 [Qemu-devel] [RFC PATCH] target-arm/psci.c: wake up sleeping CPUs (MTTCG) Alex Bennée
2015-06-24 16:01 ` Paolo Bonzini
2015-06-24 17:18   ` Alex Bennée
2015-06-24 17:23     ` Paolo Bonzini
2015-06-24 18:15       ` Alex Bennée
2015-06-24 19:12     ` Peter Maydell
2015-06-25 15:44       ` Andrew Jones
2015-06-26  7:06         ` Alex Bennée
2015-06-26  8:05           ` Andrew Jones
2015-06-24 23:55 ` Alexander Spyridakis
2015-06-25  6:27   ` Alex Bennée
2015-06-25 12:43   ` Frederic Konrad

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.