On Wed, Mar 22, 2017 at 5:28 PM, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Laurent Desnogues <laurent.desnog...@gmail.com> writes:
>
>> 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.
>
> Doh - too many levels of BQL. So wi->exclusive tasks drop the iothread
> so they don't deadlock as any other threads try and wake up and get to
> cpu_exec_end. Of course the update was originally under BQL so the
> deferred work should be as well. We have to make a minor tweak to avoid
> triggering RCU clean-up outside the execution context though. Try this:
>
> ui/console: ensure do_safe_dpy_refresh holds BQL
>
> I missed the fact that when an exclusive work item runs it drops the
> BQL to ensure all no vCPUs are stuck waiting for it, hence causing a
> deadlock. However the actual helper needs to take the BQL especially
> as we'll be messing with device emulation bits during the update which
> all assume BQL is held.
>
> We make a minor cpu_reloading_memory_map which must try and unlock the
> RCU if we are actually outside the running context.
>
> Reported-by: Laurent Desnogues <laurent.desnog...@gmail.com>
> CC: Gerd Hoffmann <kra...@redhat.com>
> CC: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
It works fine on the image I reported and another AArch64 one.
Tested-by: Laurent Desnogues <laurent.desnog...@gmail.com>
Thanks for the quick fix,
Laurent
> 2 files changed, 3 insertions(+), 1 deletion(-)
> cpu-exec-common.c | 2 +-
> ui/console.c | 2 ++
>
> modified cpu-exec-common.c
> @@ -35,7 +35,7 @@ void cpu_loop_exit_noexc(CPUState *cpu)
> #if defined(CONFIG_SOFTMMU)
> void cpu_reloading_memory_map(void)
> {
> - if (qemu_in_vcpu_thread()) {
> + if (qemu_in_vcpu_thread() && current_cpu->running) {
> /* The guest can in theory prolong the RCU critical section as long
> * as it feels like. The major problem with this is that because it
> * can do multiple reconfigurations of the memory map within the
> modified ui/console.c
> @@ -1586,7 +1586,9 @@ bool dpy_gfx_check_format(QemuConsole *con,
> static void do_safe_dpy_refresh(CPUState *cpu, run_on_cpu_data opaque)
> {
> DisplayChangeListener *dcl = opaque.host_ptr;
> + qemu_mutex_lock_iothread();
> dcl->ops->dpy_refresh(dcl);
> + qemu_mutex_unlock_iothread();
> }
>
> static void dpy_refresh(DisplayState *s)
