On 03/06/16 23:40, Alex Bennée wrote:
> There are a number of changes that occur at the same time here:
>
> - tb_lock is no longer a NOP for SoftMMU
>
> The tb_lock protects both translation and memory map structures. The
> debug assert is updated to reflect this.
This could be a separate patch.
If we use tb_lock in system-mode to protect the structures protected by
mmap_lock in user-mode then maybe we can merge those two locks because,
as I remember, tb_lock in user-mode emulation is only held outside of
mmap_lock for patching TB for direct jumps.
>
> - introduce a single vCPU qemu_tcg_cpu_thread_fn
>
> One of these is spawned per vCPU with its own Thread and Condition
> variables. qemu_tcg_single_cpu_thread_fn is the new name for the old
> single threaded function.
So we have 'tcg_current_rr_cpu' and 'qemu_cpu_kick_rr_cpu() at this
moment, maybe name this function like qemu_tcg_rr_cpu_thread_fn()? ;)
>
> - the TLS current_cpu variable is now live for the lifetime of MTTCG
> vCPU threads. This is for future work where async jobs need to know
> the vCPU context they are operating in.
This is important change because we set 'current_cpu' to NULL outside of
cpu_exec() before, I wonder why.
>
> The user to switch on multi-thread behaviour and spawn a thread
> per-vCPU. For a simple test like:
>
> ./arm/run ./arm/locking-test.flat -smp 4 -accel tcg,thread=multi
It would be nice to mention that the simple test is from kvm_unit_tests.
>
> Will now use 4 vCPU threads and have an expected FAIL (instead of the
> unexpected PASS) as the default mode of the test has no protection when
> incrementing a shared variable.
>
> However we still default to a single thread for all vCPUs as individual
> front-end and back-ends need additional fixes to safely support:
> - atomic behaviour
> - tb invalidation
> - memory ordering
>
> The function default_mttcg_enabled can be tweaked as support is added.
>
> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> [AJB: Some fixes, conditionally, commit rewording]
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>
(snip)
> diff --git a/cpus.c b/cpus.c
> index 35374fd..419caa2 100644
> --- a/cpus.c
> +++ b/cpus.c
(snip)
> @@ -1042,9 +1039,7 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
> qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> }
>
> - CPU_FOREACH(cpu) {
> - qemu_wait_io_event_common(cpu);
> - }
> + qemu_wait_io_event_common(cpu);
Is it okay for single-threaded CPU loop?
> }
>
> static void qemu_kvm_wait_io_event(CPUState *cpu)
(snip)
> @@ -1331,6 +1324,69 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> return NULL;
> }
>
> +/* Multi-threaded TCG
> + *
> + * In the multi-threaded case each vCPU has its own thread. The TLS
> + * variable current_cpu can be used deep in the code to find the
> + * current CPUState for a given thread.
> + */
> +
> +static void *qemu_tcg_cpu_thread_fn(void *arg)
> +{
> + CPUState *cpu = arg;
> +
> + rcu_register_thread();
> +
> + qemu_mutex_lock_iothread();
> + qemu_thread_get_self(cpu->thread);
> +
> + cpu->thread_id = qemu_get_thread_id();
> + cpu->created = true;
> + cpu->can_do_io = 1;
> + current_cpu = cpu;
> + qemu_cond_signal(&qemu_cpu_cond);
> +
> + /* process any pending work */
> + atomic_mb_set(&cpu->exit_request, 1);
> +
> + while (1) {
> + bool sleep = false;
> +
> + if (cpu_can_run(cpu)) {
> + int r = tcg_cpu_exec(cpu);
> + switch (r) {
> + case EXCP_DEBUG:
> + cpu_handle_guest_debug(cpu);
> + break;
> + case EXCP_HALTED:
> + /* during start-up the vCPU is reset and the thread is
> + * kicked several times. If we don't ensure we go back
> + * to sleep in the halted state we won't cleanly
> + * start-up when the vCPU is enabled.
> + */
> + sleep = true;
> + break;
> + default:
> + /* Ignore everything else? */
> + break;
> + }
> + } else {
> + sleep = true;
> + }
> +
> + handle_icount_deadline();
> +
> + if (sleep) {
> + qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> + }
> +
> + atomic_mb_set(&cpu->exit_request, 0);
> + qemu_tcg_wait_io_event(cpu);
Do we really want to wait in qemu_tcg_wait_io_event() while
"all_cpu_threads_idle()"?
> + }
> +
> + return NULL;
> +}
> +
> static void qemu_cpu_kick_thread(CPUState *cpu)
> {
> #ifndef _WIN32
> @@ -1355,7 +1411,7 @@ void qemu_cpu_kick(CPUState *cpu)
> qemu_cond_broadcast(cpu->halt_cond);
> if (tcg_enabled()) {
> cpu_exit(cpu);
> - /* Also ensure current RR cpu is kicked */
> + /* NOP unless doing single-thread RR */
> qemu_cpu_kick_rr_cpu();
> } else {
> qemu_cpu_kick_thread(cpu);
> @@ -1422,13 +1478,6 @@ void pause_all_vcpus(void)
>
> if (qemu_in_vcpu_thread()) {
> cpu_stop_current();
> - if (!kvm_enabled()) {
> - CPU_FOREACH(cpu) {
> - cpu->stop = false;
> - cpu->stopped = true;
> - }
> - return;
> - }
I think this change is incompatible with single-threaded CPU loop as well.
> }
>
> while (!all_vcpus_paused()) {
>
(snip)
Kind regards,
Sergey
