Richard Henderson <[email protected]> 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: <[email protected]>
>
>>> +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
