Merge ~rafaeldtinoco/ubuntu/+source/qemu:lp1805256-eoan into ubuntu/+source/qemu:ubuntu/eoan-devel

[Rafael David Tinoco]

BUG: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1805256
PPA: https://launchpad.net/~rafaeldtinoco/+archive/ubuntu/lp1805256

[Rafael David Tinoco (rafaeldtinoco)]

Same as:

https://code.launchpad.net/~rafaeldtinoco/ubuntu/+source/qemu/+git/qemu/+merge/383530/comments/1006812

[dann frazier (dannf)]

Verified by building and testing the source package in your PPA (faster than waiting for LP builder), and verifying that it passed my test case for > 100 loops. Thanks @rafaeldtinoco!

review: Approve

[Ike Panhc (ikepanhc)]

Test on d06 for 1000 run and all passed

review: Approve

[Christian Ehrhardt  (paelzer)]

Also needs a rebase due to CVE-2020-11869 fix in 1:4.0+dfsg-0ubuntu9.6
It otherwise looks ok to me

[Christian Ehrhardt  (paelzer)]

Can you update the PPA (https://launchpad.net/~rafaeldtinoco/+archive/ubuntu/lp1805256/) after the rebase. I'll then run a regression test set and if ok sponsor it into the -unapproved queue after the groovy upload completed.

[Rafael David Tinoco (rafaeldtinoco)]

Done!

review: Approve

[Christian Ehrhardt  (paelzer)]

Thanks for the rebase, non case specific regression testing is a few thousand tests in and still working.
Approving the MP now.
If no issue shows up late I'll sponsor this to -unapproved later on.

review: Approve

[Christian Ehrhardt  (paelzer)]

Regression tests completed - sponsoring once groovy is complete

[Christian Ehrhardt  (paelzer)]

Groovy is completed, SRU sponsored into -unapproved

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/qemu
 * [new tag] upload/1%4.0+dfsg-0ubuntu9.7 -> upload/1%4.0+dfsg-0ubuntu9.7

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading qemu_4.0+dfsg-0ubuntu9.7.dsc: done.
  Uploading qemu_4.0+dfsg-0ubuntu9.7.debian.tar.xz: done.
  Uploading qemu_4.0+dfsg-0ubuntu9.7_source.buildinfo: done.
  Uploading qemu_4.0+dfsg-0ubuntu9.7_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 5cf0eef..4450378 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+qemu (1:4.0+dfsg-0ubuntu9.7) eoan; urgency=medium
+
+  * d/p/ubuntu/lp-1805256*: Fixes for QEMU on aarch64 ARM hosts
+    - async: use explicit memory barriers (LP: #1805256)
+    - aio-wait: delegate polling of main AioContext if BQL not held
+
+ -- Rafael David Tinoco <[e-mail]>  Wed, 27 May 2020 20:07:57 +0000
+
 qemu (1:4.0+dfsg-0ubuntu9.6) eoan-security; urgency=medium
 
   * SECURITY UPDATE: overflow via PCIe extended config space
diff --git a/debian/patches/series b/debian/patches/series
index 8edcf9d..43a5b90 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -52,3 +52,5 @@ ubuntu/CVE-2019-15034.patch
 ubuntu/CVE-2019-20382.patch
 ubuntu/CVE-2020-10702.patch
 ubuntu/CVE-2020-1983.patch
+ubuntu/lp-1805256-async_use_explicit_mem_barriers.patch
+ubuntu/lp-1805256-aio-wait_delegate_poll_aioctx_bql.patch
diff --git a/debian/patches/ubuntu/lp-1805256-aio-wait_delegate_poll_aioctx_bql.patch b/debian/patches/ubuntu/lp-1805256-aio-wait_delegate_poll_aioctx_bql.patch
new file mode 100644
index 0000000..c304a0b
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1805256-aio-wait_delegate_poll_aioctx_bql.patch
@@ -0,0 +1,110 @@
+Description: aio-wait: delegate polling of main AioContext if BQL not held
+
+Any thread that is not a iothread returns NULL for qemu_get_current_aio_context().
+As a result, it would also return true for
+in_aio_context_home_thread(qemu_get_aio_context()), causing
+AIO_WAIT_WHILE to invoke aio_poll() directly.  This is incorrect
+if the BQL is not held, because aio_poll() does not expect to
+run concurrently from multiple threads, and it can actually
+happen when savevm writes to the vmstate file from the
+migration thread.
+
+Therefore, restrict in_aio_context_home_thread to return true
+for the main AioContext only if the BQL is held.
+
+The function is moved to aio-wait.h because it is mostly used
+there and to avoid a circular reference between main-loop.h
+and block/aio.h.
+
+Signed-off-by: Paolo Bonzini <[e-mail]>
+Message-Id: <[e-mail]>
+Signed-off-by: Stefan Hajnoczi <[e-mail]>
+
+Author: Paolo Bonzini <[e-mail]>
+Origin: upstream, https://github.com/qemu/qemu/commit/3c18a92dc
+Bug: https://launchpad.net/bugs/1805256
+Bug-Ubuntu: https://launchpad.net/bugs/1805256
+Reviewed-By: Rafael David Tinoco <[e-mail]>
+Last-Update: 2020-05-06
+
+--- qemu-4.0+dfsg.orig/include/block/aio-wait.h
++++ qemu-4.0+dfsg/include/block/aio-wait.h
+@@ -26,6 +26,7 @@
+ #define QEMU_AIO_WAIT_H
+ 
+ #include "block/aio.h"
++#include "qemu/main-loop.h"
+ 
+ /**
+  * AioWait:
+@@ -124,4 +125,25 @@ void aio_wait_kick(void);
+  */
+ void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+ 
++/**
++ * in_aio_context_home_thread:
++ * @ctx: the aio context
++ *
++ * Return whether we are running in the thread that normally runs @ctx.  Note
++ * that acquiring/releasing ctx does not affect the outcome, each AioContext
++ * still only has one home thread that is responsible for running it.
++ */
++static inline bool in_aio_context_home_thread(AioContext *ctx)
++{
++    if (ctx == qemu_get_current_aio_context()) {
++        return true;
++    }
++
++    if (ctx == qemu_get_aio_context()) {
++        return qemu_mutex_iothread_locked();
++    } else {
++        return false;
++    }
++}
++
+ #endif /* QEMU_AIO_WAIT */
+--- qemu-4.0+dfsg.orig/include/block/aio.h
++++ qemu-4.0+dfsg/include/block/aio.h
+@@ -61,12 +61,16 @@ struct AioContext {
+     QLIST_HEAD(, AioHandler) aio_handlers;
+ 
+     /* Used to avoid unnecessary event_notifier_set calls in aio_notify;
+-     * accessed with atomic primitives.  If this field is 0, everything
+-     * (file descriptors, bottom halves, timers) will be re-evaluated
+-     * before the next blocking poll(), thus the event_notifier_set call
+-     * can be skipped.  If it is non-zero, you may need to wake up a
+-     * concurrent aio_poll or the glib main event loop, making
+-     * event_notifier_set necessary.
++     * only written from the AioContext home thread, or under the BQL in
++     * the case of the main AioContext.  However, it is read from any
++     * thread so it is still accessed with atomic primitives.
++     *
++     * If this field is 0, everything (file descriptors, bottom halves,
++     * timers) will be re-evaluated before the next blocking poll() or
++     * io_uring wait; therefore, the event_notifier_set call can be
++     * skipped.  If it is non-zero, you may need to wake up a concurrent
++     * aio_poll or the glib main event loop, making event_notifier_set
++     * necessary.
+      *
+      * Bit 0 is reserved for GSource usage of the AioContext, and is 1
+      * between a call to aio_ctx_prepare and the next call to aio_ctx_check.
+@@ -582,19 +586,6 @@ void aio_co_enter(AioContext *ctx, struc
+ AioContext *qemu_get_current_aio_context(void);
+ 
+ /**
+- * in_aio_context_home_thread:
+- * @ctx: the aio context
+- *
+- * Return whether we are running in the thread that normally runs @ctx.  Note
+- * that acquiring/releasing ctx does not affect the outcome, each AioContext
+- * still only has one home thread that is responsible for running it.
+- */
+-static inline bool in_aio_context_home_thread(AioContext *ctx)
+-{
+-    return ctx == qemu_get_current_aio_context();
+-}
+-
+-/**
+  * aio_context_setup:
+  * @ctx: the aio context
+  *
diff --git a/debian/patches/ubuntu/lp-1805256-async_use_explicit_mem_barriers.patch b/debian/patches/ubuntu/lp-1805256-async_use_explicit_mem_barriers.patch
new file mode 100644
index 0000000..3ed09e1
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1805256-async_use_explicit_mem_barriers.patch
@@ -0,0 +1,159 @@
+Description: async: use explicit memory barriers
+
+When using C11 atomics, non-seqcst reads and writes do not participate
+in the total order of seqcst operations.  In util/async.c and util/aio-posix.c,
+in particular, the pattern that we use
+
+          write ctx->notify_me                 write bh->scheduled
+          read bh->scheduled                   read ctx->notify_me
+          if !bh->scheduled, sleep             if ctx->notify_me, notify
+
+needs to use seqcst operations for both the write and the read.  In
+general this is something that we do not want, because there can be
+many sources that are polled in addition to bottom halves.  The
+alternative is to place a seqcst memory barrier between the write
+and the read.  This also comes with a disadvantage, in that the
+memory barrier is implicit on strongly-ordered architectures and
+it wastes a few dozen clock cycles.
+
+Fortunately, ctx->notify_me is never written concurrently by two
+threads, so we can assert that and relax the writes to ctx->notify_me.
+The resulting solution works and performs well on both aarch64 and x86.
+
+Note that the atomic_set/atomic_read combination is not an atomic
+read-modify-write, and therefore it is even weaker than C11 ATOMIC_RELAXED;
+on x86, ATOMIC_RELAXED compiles to a locked operation.
+
+Analyzed-by: Ying Fang <[e-mail]>
+Signed-off-by: Paolo Bonzini <[e-mail]>
+Tested-by: Ying Fang <[e-mail]>
+Message-Id: <[e-mail]>
+Signed-off-by: Stefan Hajnoczi <[e-mail]>
+
+Author: Paolo Bonzini <[e-mail]>
+Origin: upstream, https://github.com/qemu/qemu/commit/5710a3e09f
+Bug: https://bugs.launchpad.net/bugs/1805256
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1805256
+Reviewed-By: Rafael David Tinoco <[e-mail]>
+Last-Update: 2020-05-06
+
+--- qemu-4.2.orig/util/aio-posix.c
++++ qemu-4.2/util/aio-posix.c
+@@ -616,6 +616,11 @@ bool aio_poll(AioContext *ctx, bool bloc
+     int64_t timeout;
+     int64_t start = 0;
+ 
++    /*
++     * There cannot be two concurrent aio_poll calls for the same AioContext (or
++     * an aio_poll concurrent with a GSource prepare/check/dispatch callback).
++     * We rely on this below to avoid slow locked accesses to ctx->notify_me.
++     */
+     assert(in_aio_context_home_thread(ctx));
+ 
+     /* aio_notify can avoid the expensive event_notifier_set if
+@@ -626,7 +631,13 @@ bool aio_poll(AioContext *ctx, bool bloc
+      * so disable the optimization now.
+      */
+     if (blocking) {
+-        atomic_add(&ctx->notify_me, 2);
++        atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
++        /*
++         * Write ctx->notify_me before computing the timeout
++         * (reading bottom half flags, etc.).  Pairs with
++         * smp_mb in aio_notify().
++         */
++        smp_mb();
+     }
+ 
+     qemu_lockcnt_inc(&ctx->list_lock);
+@@ -671,7 +682,8 @@ bool aio_poll(AioContext *ctx, bool bloc
+     }
+ 
+     if (blocking) {
+-        atomic_sub(&ctx->notify_me, 2);
++        /* Finish the poll before clearing the flag.  */
++        atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) - 2);
+         aio_notify_accept(ctx);
+     }
+ 
+--- qemu-4.2.orig/util/aio-win32.c
++++ qemu-4.2/util/aio-win32.c
+@@ -321,6 +321,12 @@ bool aio_poll(AioContext *ctx, bool bloc
+     int count;
+     int timeout;
+ 
++    /*
++     * There cannot be two concurrent aio_poll calls for the same AioContext (or
++     * an aio_poll concurrent with a GSource prepare/check/dispatch callback).
++     * We rely on this below to avoid slow locked accesses to ctx->notify_me.
++     */
++    assert(in_aio_context_home_thread(ctx));
+     progress = false;
+ 
+     /* aio_notify can avoid the expensive event_notifier_set if
+@@ -331,7 +337,13 @@ bool aio_poll(AioContext *ctx, bool bloc
+      * so disable the optimization now.
+      */
+     if (blocking) {
+-        atomic_add(&ctx->notify_me, 2);
++        atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
++        /*
++         * Write ctx->notify_me before computing the timeout
++         * (reading bottom half flags, etc.).  Pairs with
++         * smp_mb in aio_notify().
++         */
++        smp_mb();
+     }
+ 
+     qemu_lockcnt_inc(&ctx->list_lock);
+@@ -364,8 +376,7 @@ bool aio_poll(AioContext *ctx, bool bloc
+         ret = WaitForMultipleObjects(count, events, FALSE, timeout);
+         if (blocking) {
+             assert(first);
+-            assert(in_aio_context_home_thread(ctx));
+-            atomic_sub(&ctx->notify_me, 2);
++            atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) - 2);
+             aio_notify_accept(ctx);
+         }
+ 
+--- qemu-4.2.orig/util/async.c
++++ qemu-4.2/util/async.c
+@@ -220,7 +220,14 @@ aio_ctx_prepare(GSource *source, gint
+ {
+     AioContext *ctx = (AioContext *) source;
+ 
+-    atomic_or(&ctx->notify_me, 1);
++    atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) | 1);
++
++    /*
++     * Write ctx->notify_me before computing the timeout
++     * (reading bottom half flags, etc.).  Pairs with
++     * smp_mb in aio_notify().
++     */
++    smp_mb();
+ 
+     /* We assume there is no timeout already supplied */
+     *timeout = qemu_timeout_ns_to_ms(aio_compute_timeout(ctx));
+@@ -238,7 +245,8 @@ aio_ctx_check(GSource *source)
+     AioContext *ctx = (AioContext *) source;
+     QEMUBH *bh;
+ 
+-    atomic_and(&ctx->notify_me, ~1);
++    /* Finish computing the timeout before clearing the flag.  */
++    atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) & ~1);
+     aio_notify_accept(ctx);
+ 
+     for (bh = ctx->first_bh; bh; bh = bh->next) {
+@@ -343,10 +351,10 @@ LinuxAioState *aio_get_linux_aio(AioCont
+ void aio_notify(AioContext *ctx)
+ {
+     /* Write e.g. bh->scheduled before reading ctx->notify_me.  Pairs
+-     * with atomic_or in aio_ctx_prepare or atomic_add in aio_poll.
++     * with smp_mb in aio_ctx_prepare or aio_poll.
+      */
+     smp_mb();
+-    if (ctx->notify_me) {
++    if (atomic_read(&ctx->notify_me)) {
+         event_notifier_set(&ctx->notifier);
+         atomic_mb_set(&ctx->notified, true);
+     }