[Qemu-devel] Rationalising exit_request, cpu->exit_request and tcg_exit_req?

[Qemu-devel] Rationalising exit_request, cpu->exit_request and tcg_exit_req?

[Alex Bennée]

Hi,

While looking at Fred's current MTTCG WIP branch I ran into a problem
where:

  - async_safe_work_pending() was true
  - this triggered setting cpu->exit_request
  - however we never left tcg_exec_all()
  - because the global exit_request wasn't set
  - hence qemu_tcg_wait_io_event() never drained the async work queue

While trying to understand why we have both a cpu and a global
exit_request I then discovered there is also cpu->tcg_exit_req which is
the actual variable the TCG examines. This leads to sequences like:

void cpu_exit(CPUState *cpu)
{
    cpu->exit_request = 1;
    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
    smp_wmb();
    cpu->tcg_exit_req = 1;
}

which itself is amusingly called from:

static void qemu_cpu_kick_no_halt(void)
{
    CPUState *cpu;
    /* Ensure whatever caused the exit has reached the CPU threads before
     * writing exit_request.
     */
    atomic_mb_set(&exit_request, 1);
    cpu = atomic_mb_read(&tcg_current_cpu);
    if (cpu) {
        cpu_exit(cpu);
    }
}

This seems to me to be slightly insane as we now have 3 variables that
struggle to be kept in sync. Could all this not be rationalised into a
single variable or am I missing a subtly in their different semantics?

One problem I can think of when we get to the MTTCG world is a race when
signalling other CPUs to exit and making sure that request is not
dropped as we clear an old exit_request.

The other complication is the main cpu_exec loop which works hard to
avoid leaving the main loop when processing interrupts (which require
an exit_request to trigger). This means there a potentially multiple
places where exit_requests are drained.

I don't know if there is clean-up that can happen in master or if this
all needs to be done in the mttcg work but would it make sense just to
keep cpu->exit_request, make it visible to the TCG code and make all
exits fall out to qemu_tcg_cpu_thread_fn which would be the only place
to clear the flag?

I did have a brief look at the KVM side of the code and it only seems to
reference cpu->exit_request so I think the rest of this is a TCG
problem.

Thoughts?

--
Alex Bennée

Re: [Qemu-devel] Rationalising exit_request, cpu->exit_request and tcg_exit_req?

[Paolo Bonzini]

On 16/12/2015 18:14, Alex Bennée wrote:
> 
> While looking at Fred's current MTTCG WIP branch I ran into a problem
> where:
> 
>   - async_safe_work_pending() was true
>   - this triggered setting cpu->exit_request
>   - however we never left tcg_exec_all()
>   - because the global exit_request wasn't set
>   - hence qemu_tcg_wait_io_event() never drained the async work queue

exit_request should disappear with MTTCG. It should only have
cpu->tcg_exit_req and cpu->exit_request.

> While trying to understand why we have both a cpu and a global
> exit_request I then discovered there is also cpu->tcg_exit_req which is
> the actual variable the TCG examines. This leads to sequences like:

> This seems to me to be slightly insane as we now have 3 variables that
> struggle to be kept in sync. Could all this not be rationalised into a
> single variable or am I missing a subtly in their different semantics?

They do.

cpu->tcg_exit_req is the one that is set from generated code.  It is set
if you do not have to exit cpu_exec.

cpu->exit_request and exit_request are both necessary, in order to
synchronize exits with the setting of tcg_current_cpu.  Again, this is
only needed in single-threaded TCG, because MTTCG gets rid of
tcg_current_cpu.  It's documented here:
http://article.gmane.org/gmane.comp.emulators.qemu/357210.

> I don't know if there is clean-up that can happen in master or if this
> all needs to be done in the mttcg work but would it make sense just to
> keep cpu->exit_request,

Yes, and it's actually necessary. :)

> I did have a brief look at the KVM side of the code and it only seems to
> reference cpu->exit_request so I think the rest of this is a TCG
> problem.

Yes.  With MTTCG you still need cpu->tcg_exit_req because you still have
something like KVM's kernel- and userspace-vmexits.  In TCG the
lightweight exits are those that do not leave cpu_exec.  Those only set
cpu->tcg_exit_req.

Paolo