On 01/06/16 13:30, Alex Bennée wrote:
> Sergey Fedorov <serge.f...@gmail.com> writes:
>
>> On 05/04/16 18:32, Alex Bennée wrote:
>> (snip)
>>> diff --git a/exec.c b/exec.c
>>> index 17f390e..c46c123 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -2111,6 +2111,9 @@ static void check_watchpoint(int offset, int len,
>>> MemTxAttrs attrs, int flags)
>>> continue;
>>> }
>>> cpu->watchpoint_hit = wp;
>>> +
>>> + /* Unlocked by cpu_loop_exit or cpu_resume_from_signal. */
>> In fact, neither cpu_resume_from_signal() nor cpu_loop_exit() unlocks
>> the lock by itself, it gets unlocked after sigsetjmp() returns via
>> siglongjmp() back to cpu_exec(). So maybe it would be more clear to say
>> something like "'tb_lock' gets unlocked after siglongjmp()"?
>
> "Locks are reset when we longjmp back to the main cpu_exec loop"?
Yes, it this looks fine.
> Looking at where the patch is though I think I need to bring that bit
> forward from the main series.
>
>>> + tb_lock();
>>> tb_check_watchpoint(cpu);
>>> if (wp->flags & BP_STOP_BEFORE_ACCESS) {
>>> cpu->exception_index = EXCP_DEBUG;
>> (snip)
>>> diff --git a/translate-all.c b/translate-all.c
>>> index a7ff5e7..935d24c 100644
>>> --- a/translate-all.c
>>> +++ b/translate-all.c
>>> @@ -834,7 +834,9 @@ static void page_flush_tb(void)
>>> }
>>>
>>> /* flush all the translation blocks */
>>> -/* XXX: tb_flush is currently not thread safe */
>>> +/* XXX: tb_flush is currently not thread safe. System emulation calls it
>>> only
>>> + * with tb_lock taken or from safe_work, so no need to take tb_lock here.
>>> + */
>> "System emulation"? What about user-mode emulation?
> It's still not thread safe ;-)
>
> It's a harder problem to solve because we can't just suspend all
> threads to reset the translation buffer. I'm not sure we want to try and
> fix it in this series.
I think it could be possible to do something like start_exclusive() to
achieve this in user-only emulation.
>>> void tb_flush(CPUState *cpu)
>>> {
>>> #if defined(DEBUG_FLUSH)
Kind regards,
Sergey
