Richard Henderson <r...@twiddle.net> 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: <1473678761-8885-1-git-send-email-pbonz...@redhat.com> > >>> +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