Re: [Qemu-devel] [Qemu-ppc] qemu-system-ppc video artifacts since "tcg: drop global lock during TCG code execution"

[Alex Bennée]

Mark Cave-Ayland <address@hidden> writes:

> On 15/03/17 11:14, Alex Bennée wrote:
>
>> BALATON Zoltan <address@hidden> writes:
>>
>>> On Tue, 14 Mar 2017, Alex Bennée wrote:
>>>> So from a single-threaded -smp guest case there should be no difference
>>>> in behaviour.
>> <snip>
>>>> However this shouldn't affect
>>>> anything in the single-threaded world.
>>>
>>> I think we have a single CPU and thread for these ppc machines here so
>>> I'm not sure how this could be relevant.
>>>
>>>> However delaying tlb_flushes() could certainly expose/hide stuff that is
>>>> accessing the dirty mechanism. tlb_flush itself now takes the tb_lock() to
>>>> avoid racing with the TB invalidation logic. The act of the flush will
>>>> certainly wipe all existing SoftMMU entries and force a re-load on each
>>>> memory access.
>>>>
>>>> So is the dirty status of memory being read from outside a vCPU
>>>> execution context?
>>>
>>> Like from the display controller models that use
>>> memory_region_get_dirty() to check if the frambuffer needs to be
>>> updated? But all display adaptors seem to do this and the problem was
>>> only seem on ppc so it may be related to something ppc specific.
>>
>> So this accesses the memory_region API which is under RCU control.
>> AFAIUI this should mean the dirty status may be read late (e.g. next
>> update) but should never be incorrect (e.g. miss a dirtying operation).
>
> AFAICT check_tlb_flush() gets passed a global parameter which if set
> true invalidates the TLB across all CPU TLBs rather than just the local
> CPU TLB - but then in this case we're only running with a single CPU so
> I can't see how this is relevant.

Not quite. tlb_flush used to take a global flag but it ignored it. It
has been removed in recent updates to the cputlb API ;-)

> Have you been able to reproduce the artifacts locally at all? I'm
> wondering if once the icount fixup patches are in, it might be easier to
> debug if enabling icount causes the artifacts to appear in a
> deterministic manner.

I have, and I can make them go away as well by forcing a full update.
See my other longer email for a description of what I think is
happening. Now I just need a neat and upstreamable fix ;-)

--
Alex Bennée