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(-)

Do not access a CQE after incrementing q->cq.head and releasing q->lock.

It is unlikely that this causes problems in practice but it's a latent

bug.

The reason why it should be safe at the moment is that completion

processing is not re-entrant and the CQ doorbell isn't written until the

end of nvme_process_completion().

Make this change now because QEMU expects completion processing to be

re-entrant and later patches will do that.

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-4-stefanha@redhat.com

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

1 file changed, 4 insertions(+), 1 deletion(-)

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

q->busy = true;

assert(q->inflight >= 0);

while (q->inflight) {

+ int ret;

int16_t cid;

c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];

if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {

+ ret = nvme_translate_error(c);

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

preq->busy = false;

preq->cb = preq->opaque = NULL;

qemu_mutex_unlock(&q->lock);

- req.cb(req.opaque, nvme_translate_error(c));

+ req.cb(req.opaque, ret);

qemu_mutex_lock(&q->lock);

q->inflight--;

progress = true;

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

--- a/block/nvme.c

+++ b/block/nvme.c

#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) {

@@ -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);

data.co = qemu_coroutine_self();

while (data.ret == -EINPROGRESS) {

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

assert(s->nr_queues > 1);

req = nvme_get_free_req(ioq);

assert(req);

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

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

data.co = qemu_coroutine_self();

if (data.ret == -EINPROGRESS) {

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

req = nvme_get_free_req(ioq);

assert(req);

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

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

data.co = qemu_coroutine_self();

while (data.ret == -EINPROGRESS) {

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

qemu_co_mutex_unlock(&s->dma_map_lock);

if (ret) {

- nvme_put_free_req_and_wake(s, ioq, req);

+ nvme_put_free_req_and_wake(ioq, req);

goto out;

}

trace_nvme_dsm(s, offset, bytes);

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

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

data.co = qemu_coroutine_self();

while (data.ret == -EINPROGRESS) {

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

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

NVMeQueuePair *q = s->queues[i];

qemu_mutex_lock(&q->lock);

- nvme_kick(s, q);

- nvme_process_completion(s, q);

+ nvme_kick(q);

+ nvme_process_completion(q);

qemu_mutex_unlock(&q->lock);

}