[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 06/34] tcg: Add EXCP_ATOMIC
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v3 06/34] tcg: Add EXCP_ATOMIC |
Date: |
Tue, 13 Sep 2016 07:42:54 +0100 |
User-agent: |
mu4e 0.9.17; emacs 25.1.12 |
Richard Henderson <address@hidden> writes:
> On 09/12/2016 07:16 AM, Alex Bennée wrote:
>>> +void cpu_exec_step(CPUState *cpu)
>>> +{
>>> + CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>> + TranslationBlock *tb;
>>> + target_ulong cs_base, pc;
>>> + uint32_t flags;
>>> + bool old_tb_flushed;
>>> +
>>> + old_tb_flushed = cpu->tb_flushed;
>>> + cpu->tb_flushed = false;
>>
>> Advanced warning, these disappear soon when the async safe work (plus
>> safe tb flush) patches get merged.
>
> Fair enough.
>
> Having thought about this more, I think it may be better to handle this
> without
> flushing the tb. To have parallel_cpus included in the TB flags or somesuch
> and keep that TB around.
>
> My thinking is that there are certain things, like alignment, that could
> result
> in repeated single-stepping. So better to keep the TB around than keep having
> to regenerate it.
>
>>> + /* ??? When we begin running cpus in parallel, we should
>>> + stop all cpus, clear parallel_cpus, and interpret a
>>> + single insn with cpu_exec_step. In the meantime,
>>> + we should never get here. */
>>> + abort();
>>
>> Possibly more correct would be:
>>
>> g_assert(parallel_cpus == false);
>> abort();
>
> No, since it is here that we would *set* parallel_cpus to false. Or did you
> mean assert parallel_cpus true? Not that that helps for the moment...
For SoftMMU this particular loop should never hit because paralell_cpus
should be false, hence we never generate any code that might
EXCP_ATOMIC. It only fails at the moment because of the "hack" for
testing which makes parallel_cpus true.
The MTTCG adds a new thread function for MTTCG mode which
will have to handle EXCP_ATOMIC as discussed.
>
>>> +static void step_atomic(CPUState *cpu)
>>> +{
>>> + start_exclusive();
>>> +
>>> + /* Since we got here, we know that parallel_cpus must be true. */
>>> + parallel_cpus = false;
>>> + cpu_exec_step(cpu);
>>> + parallel_cpus = true;
>>> +
>>> + end_exclusive();
>>> +}
>>> +
>>
>> Paolo's safe work patches bring the start/end_exclusive functions into
>> cpu-exec-common so I think after that has been merged this function
>> can also be moved and called directly from the MTTCG loop on an
>> EXCP_ATOMIC exit.
>
> Excellent. Perhaps I should rebase this upon that right away. Have you got a
> pointer to a tree handy?
Paolo posted a new version recently but I haven't built a tree with it
yet. I was hoping some of the hot-path and maybe barrier stuff would get
merged while I finish reviewing this ;-)
See:
Subject: [PATCH v7 00/16] cpu-exec: Safe work in quiescent state
Date: Mon, 12 Sep 2016 13:12:25 +0200
Message-Id: <address@hidden>
>
>>> +bool parallel_cpus;
>>
>> So lets add some words to this to distinguish between this and the mttcg
>> enabled flags and its relation to -smp. Something like:
>>
>> parallel_cpus indicates to the front-ends if code needs to be
>> generated taking into account multiple threads of execution. It will
>> be true for linux-user after the first thread clone and if system mode
>> if MTTCG is enabled. On the transition from false->true any code
>> generated while false needs to be invalidated. It may be temporally
>> set to false when generating non-cached code in an exclusive context.
>
> Sure.
>
>
> r~
--
Alex Bennée
- [Qemu-devel] [PATCH v3 00/34] cmpxchg-based emulation of atomics, Richard Henderson, 2016/09/03
- [Qemu-devel] [PATCH v3 07/34] HACK: Always enable parallel_cpus, Richard Henderson, 2016/09/03
- [Qemu-devel] [PATCH v3 11/34] cputlb: Move most of iotlb code out of line, Richard Henderson, 2016/09/03
- [Qemu-devel] [PATCH v3 04/34] int128: Use __int128 if available, Richard Henderson, 2016/09/03
- [Qemu-devel] [PATCH v3 09/34] cputlb: Move probe_write out of softmmu_template.h, Richard Henderson, 2016/09/03
- [Qemu-devel] [PATCH v3 12/34] cputlb: Tidy some macros, Richard Henderson, 2016/09/03