Re: [Qemu-devel] [RFC PATCH] ui/console: ensure graphic updates don't race with TCG vCPUs

[Laurent Desnogues]

Hi Alex,

this patch breaks:

  http://wiki.qemu.org/download/arm-test-0.2.tar.gz

qemu-system-arm -kernel zImage.integrator -initrd arm_root.img
-append "console=ttyAMA0" -machine integratorcp -serial stdio -icount
0
Uncompressing 
Linux..........................................................................
done, booting the kernel.
qemu-system-arm: /work/qemu/qemu/memory.c:920:
memory_region_transaction_commit: Assertion
`qemu_mutex_iothread_locked()' failed.

The backtrace looks like this:

#4  0x00005555557ced26 in memory_region_transaction_commit ()
    at /work/qemu/qemu/memory.c:920
#5  0x0000555555959b82 in pl110_update_display (opaque=0x555556a839e0)
    at hw/display/pl110.c:245
#6  0x0000555555a60931 in sdl_refresh (dcl=0x555556b90d40) at ui/sdl.c:821
#7  0x00005555558f9f91 in process_queued_cpu_work (
    address@hidden) at cpus-common.c:338
#8  0x00005555557ba5a8 in qemu_wait_io_event_common (
    address@hidden) at /work/qemu/qemu/cpus.c:1027
#9  0x00005555557ba9bb in qemu_tcg_wait_io_event (address@hidden)
    at /work/qemu/qemu/cpus.c:1048
#10 0x00005555557bc2bd in qemu_tcg_cpu_thread_fn (arg=0x5555566c6810)
    at /work/qemu/qemu/cpus.c:1440
#11 0x00007ffff5f4a6ba in start_thread (arg=0x7fffee650700)
    at pthread_create.c:333
#12 0x00007ffff5c8082d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

BTW is it easier if I enter such bug reports in QEMU launchpad?

Thanks,

Laurent

On Wed, Mar 15, 2017 at 3:48 PM, Alex Bennée <address@hidden> wrote:
> Commit 8d04fb55..
>
>   tcg: drop global lock during TCG code execution
>
> ..broke the assumption that updates to the GUI couldn't happen at the
> same time as TCG vCPUs where running. As a result the TCG vCPU could
> still be updating a directly mapped frame-buffer while the display
> side was updating. This would cause artefacts to appear when the
> update code assumed that memory block hadn't changed.
>
> The simplest solution is to ensure the two things can't happen at the
> same time like the old BQL locking scheme. Here we use the solution
> introduced for MTTCG and schedule the update as async_safe_work when
> we know no vCPUs can be running.
>
> Reported-by: Mark Cave-Ayland <address@hidden>
> Cc: BALATON Zoltan <address@hidden>
> Cc: Gerd Hoffmann <address@hidden>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  ui/console.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/ui/console.c b/ui/console.c
> index d1ff7504ec..37a69e27de 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1575,13 +1575,29 @@ bool dpy_gfx_check_format(QemuConsole *con,
>      return true;
>  }
>
> +/*
> + * Safe DPY refresh for TCG guests. This runs when the TCG vCPUs are
> + * quiescent so we can avoid races between dirty page tracking for
> + * direct frame-buffer access by the guest.
> + */
> +static void do_safe_dpy_refresh(CPUState *cpu, run_on_cpu_data opaque)
> +{
> +    DisplayChangeListener *dcl = opaque.host_ptr;
> +    dcl->ops->dpy_refresh(dcl);
> +}
> +
>  static void dpy_refresh(DisplayState *s)
>  {
>      DisplayChangeListener *dcl;
>
>      QLIST_FOREACH(dcl, &s->listeners, next) {
>          if (dcl->ops->dpy_refresh) {
> -            dcl->ops->dpy_refresh(dcl);
> +            if (tcg_enabled()) {
> +                async_safe_run_on_cpu(first_cpu, do_safe_dpy_refresh,
> +                                      RUN_ON_CPU_HOST_PTR(dcl));
> +            } else {
> +                dcl->ops->dpy_refresh(dcl);
> +            }
>          }
>      }
>  }
> --
> 2.11.0
>
>