The following changes since commit 171199f56f5f9bdf1e5d670d09ef1351d8f01bae:

Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200619-3' into staging (2020-06-22 14:45:25 +0100)

https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 7838c67f22a81fcf669785cd6c0876438422071a:

block/nvme: support nested aio_poll() (2020-06-23 15:46:08 +0100)

Daniele Buono (4):

coroutine: support SafeStack in ucontext backend

coroutine: add check for SafeStack in sigaltstack

configure: add flags to support SafeStack

check-block: enable iotests with SafeStack

Stefan Hajnoczi (8):

minikconf: explicitly set encoding to UTF-8

block/nvme: poll queues without q->lock

block/nvme: drop tautologous assertion

block/nvme: don't access CQE after moving cq.head

block/nvme: switch to a NVMeRequest freelist

block/nvme: clarify that free_req_queue is protected by q->lock

block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair

block/nvme: support nested aio_poll()

configure | 73 ++++++++++++

include/qemu/coroutine_int.h | 5 +

block/nvme.c | 220 +++++++++++++++++++++++++----------

util/coroutine-sigaltstack.c | 4 +

util/coroutine-ucontext.c | 28 +++++

block/trace-events | 2 +-

scripts/minikconf.py | 6 +-

tests/check-block.sh | 12 +-

8 files changed, 284 insertions(+), 66 deletions(-)

2.26.2

From: Daniele Buono <dbuono@linux.vnet.ibm.com>

LLVM's SafeStack instrumentation does not yet support programs that make

use of the APIs in ucontext.h

With the current implementation of coroutine-ucontext, the resulting

binary is incorrect, with different coroutines sharing the same unsafe

stack and producing undefined behavior at runtime.

This fix allocates an additional unsafe stack area for each coroutine,

and sets the new unsafe stack pointer before calling swapcontext() in

qemu_coroutine_new.

This is the only place where the pointer needs to be manually updated,

since sigsetjmp/siglongjmp are already instrumented by LLVM to properly

support SafeStack.

The additional stack is then freed in qemu_coroutine_delete.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>

Message-id: 20200529205122.714-2-dbuono@linux.vnet.ibm.com

include/qemu/coroutine_int.h | 5 +++++

util/coroutine-ucontext.c | 28 ++++++++++++++++++++++++++++

2 files changed, 33 insertions(+)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h

--- a/include/qemu/coroutine_int.h

+++ b/include/qemu/coroutine_int.h

@@ -XXX,XX +XXX,XX @@

#include "qemu/queue.h"

#include "qemu/coroutine.h"

+#ifdef CONFIG_SAFESTACK

+/* Pointer to the unsafe stack, defined by the compiler */

+extern __thread void *__safestack_unsafe_stack_ptr;

+

#define COROUTINE_STACK_SIZE (1 << 20)

typedef enum {

diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c

--- a/util/coroutine-ucontext.c

+++ b/util/coroutine-ucontext.c

@@ -XXX,XX +XXX,XX @@ typedef struct {

Coroutine base;

void *stack;

size_t stack_size;

+#ifdef CONFIG_SAFESTACK

+ /* Need an unsafe stack for each coroutine */

+ void *unsafe_stack;

+ size_t unsafe_stack_size;

+#endif

sigjmp_buf env;

void *tsan_co_fiber;

@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)

co = g_malloc0(sizeof(*co));

co->stack_size = COROUTINE_STACK_SIZE;

co->stack = qemu_alloc_stack(&co->stack_size);

+#ifdef CONFIG_SAFESTACK

+ co->unsafe_stack_size = COROUTINE_STACK_SIZE;

+ co->unsafe_stack = qemu_alloc_stack(&co->unsafe_stack_size);

+#endif

co->base.entry_arg = &old_env; /* stash away our jmp_buf */

uc.uc_link = &old_uc;

@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)

COROUTINE_YIELD,

&fake_stack_save,

co->stack, co->stack_size, co->tsan_co_fiber);

+

+#ifdef CONFIG_SAFESTACK

+ /*

+ * Before we swap the context, set the new unsafe stack

+ * The unsafe stack grows just like the normal stack, so start from

+ * the last usable location of the memory area.

+ * NOTE: we don't have to re-set the usp afterwards because we are

+ * coming back to this context through a siglongjmp.

+ * The compiler already wrapped the corresponding sigsetjmp call with

+ * code that saves the usp on the (safe) stack before the call, and

+ * restores it right after (which is where we return with siglongjmp).

+ */

+ void *usp = co->unsafe_stack + co->unsafe_stack_size;

+ __safestack_unsafe_stack_ptr = usp;

+#endif

+

swapcontext(&old_uc, &uc);

@@ -XXX,XX +XXX,XX @@ void qemu_coroutine_delete(Coroutine *co_)

#endif

qemu_free_stack(co->stack, co->stack_size);

+#ifdef CONFIG_SAFESTACK

+ qemu_free_stack(co->unsafe_stack, co->unsafe_stack_size);

+#endif

g_free(co);

2.26.2

Passing around both BDRVNVMeState and NVMeQueuePair is unwieldy. Reduce

the number of function arguments by keeping the BDRVNVMeState pointer in

NVMeQueuePair. This will come in handly when a BH is introduced in a

later patch and only one argument can be passed to it.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Sergio Lopez <slp@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20200617132201.1832152-7-stefanha@redhat.com

block/nvme.c | 70 ++++++++++++++++++++++++++++------------------------

1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/block/nvme.c

+++ b/block/nvme.c

@@ -XXX,XX +XXX,XX @@

*/

#define NVME_NUM_REQS (NVME_QUEUE_SIZE - 1)

+typedef struct BDRVNVMeState BDRVNVMeState;

+

typedef struct {

int32_t head, tail;

uint8_t *queue;

@@ -XXX,XX +XXX,XX @@ typedef struct {

typedef struct {

QemuMutex lock;

+ /* Read from I/O code path, initialized under BQL */

+ BDRVNVMeState *s;

+ int index;

+

/* Fields protected by BQL */

- int index;

uint8_t *prp_list_pages;

/* Fields protected by @lock */

@@ -XXX,XX +XXX,XX @@ typedef volatile struct {

QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000);

-typedef struct {

+struct BDRVNVMeState {

AioContext *aio_context;

QEMUVFIOState *vfio;

NVMeRegs *regs;

@@ -XXX,XX +XXX,XX @@ typedef struct {

/* PCI address (required for nvme_refresh_filename()) */

char *device;

-} BDRVNVMeState;

#define NVME_BLOCK_OPT_DEVICE "device"

#define NVME_BLOCK_OPT_NAMESPACE "namespace"

@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,

-static void nvme_free_queue_pair(BlockDriverState *bs, NVMeQueuePair *q)

+static void nvme_free_queue_pair(NVMeQueuePair *q)

qemu_vfree(q->prp_list_pages);

qemu_vfree(q->sq.queue);

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,

uint64_t prp_list_iova;

qemu_mutex_init(&q->lock);

+ q->s = s;

q->index = idx;

qemu_co_queue_init(&q->free_req_queue);

q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);

@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,

return q;

fail:

- nvme_free_queue_pair(bs, q);

+ nvme_free_queue_pair(q);

return NULL;

/* With q->lock */

-static void nvme_kick(BDRVNVMeState *s, NVMeQueuePair *q)

+static void nvme_kick(NVMeQueuePair *q)

{

+ BDRVNVMeState *s = q->s;

+

if (s->plugged || !q->need_kick) {

return;

}

@@ -XXX,XX +XXX,XX @@ static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)

}

/* With q->lock */

-static void nvme_wake_free_req_locked(BDRVNVMeState *s, NVMeQueuePair *q)

+static void nvme_wake_free_req_locked(NVMeQueuePair *q)

{

if (!qemu_co_queue_empty(&q->free_req_queue)) {

- replay_bh_schedule_oneshot_event(s->aio_context,

+ replay_bh_schedule_oneshot_event(q->s->aio_context,

nvme_free_req_queue_cb, q);

}

}

/* Insert a request in the freelist and wake waiters */

-static void nvme_put_free_req_and_wake(BDRVNVMeState *s, NVMeQueuePair *q,

- NVMeRequest *req)

+static void nvme_put_free_req_and_wake(NVMeQueuePair *q, NVMeRequest *req)

{

qemu_mutex_lock(&q->lock);

nvme_put_free_req_locked(q, req);

- nvme_wake_free_req_locked(s, q);

+ nvme_wake_free_req_locked(q);

qemu_mutex_unlock(&q->lock);

}

@@ -XXX,XX +XXX,XX @@ static inline int nvme_translate_error(const NvmeCqe *c)

}

/* With q->lock */

-static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)

+static bool nvme_process_completion(NVMeQueuePair *q)

{

+ BDRVNVMeState *s = q->s;

bool progress = false;

NVMeRequest *preq;

NVMeRequest req;

@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)

/* Notify the device so it can post more completions. */

smp_mb_release();

*q->cq.doorbell = cpu_to_le32(q->cq.head);

- nvme_wake_free_req_locked(s, q);

+ nvme_wake_free_req_locked(q);

}

q->busy = false;

return progress;

@@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd)

}

}

-static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,

- NVMeRequest *req,

+static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,

NvmeCmd *cmd, BlockCompletionFunc cb,

void *opaque)

{

@@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,

req->opaque = opaque;

cmd->cid = cpu_to_le32(req->cid);

- trace_nvme_submit_command(s, q->index, req->cid);

+ trace_nvme_submit_command(q->s, q->index, req->cid);

nvme_trace_command(cmd);

qemu_mutex_lock(&q->lock);

memcpy((uint8_t *)q->sq.queue +

q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));

q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;

q->need_kick++;

- nvme_kick(s, q);

- nvme_process_completion(s, q);

+ nvme_kick(q);

+ nvme_process_completion(q);

qemu_mutex_unlock(&q->lock);

}

@@ -XXX,XX +XXX,XX @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,

NvmeCmd *cmd)

{

NVMeRequest *req;

- BDRVNVMeState *s = bs->opaque;

int ret = -EINPROGRESS;

req = nvme_get_free_req(q);

if (!req) {

return -EBUSY;

}

- nvme_submit_command(s, q, req, cmd, nvme_cmd_sync_cb, &ret);

+ nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, &ret);

BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);

return ret;

@@ -XXX,XX +XXX,XX @@ static bool nvme_poll_queues(BDRVNVMeState *s)

}

qemu_mutex_lock(&q->lock);

- while (nvme_process_completion(s, q)) {

+ while (nvme_process_completion(q)) {

/* Keep polling */

progress = true;

}

@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)

};

if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {

error_setg(errp, "Failed to create io queue [%d]", n);

- nvme_free_queue_pair(bs, q);

+ nvme_free_queue_pair(q);

return false;

}

cmd = (NvmeCmd) {

@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)

};

if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {

error_setg(errp, "Failed to create io queue [%d]", n);

- nvme_free_queue_pair(bs, q);

+ nvme_free_queue_pair(q);

return false;

}

s->queues = g_renew(NVMeQueuePair *, s->queues, n + 1);

@@ -XXX,XX +XXX,XX @@ static void nvme_close(BlockDriverState *bs)

BDRVNVMeState *s = bs->opaque;

for (i = 0; i < s->nr_queues; ++i) {

- nvme_free_queue_pair(bs, s->queues[i]);

+ nvme_free_queue_pair(s->queues[i]);

}

g_free(s->queues);

aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,

r = nvme_cmd_map_qiov(bs, &cmd, req, qiov);

qemu_co_mutex_unlock(&s->dma_map_lock);

if (r) {

- nvme_put_free_req_and_wake(s, ioq, req);

+ nvme_put_free_req_and_wake(ioq, req);

return r;

}

- nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);

+ nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);