The following changes since commit 8507c9d5c9a62de2a0e281b640f995e26eac46af:

Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2020-11-03 15:59:44 +0000)

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

for you to fetch changes up to fc107d86840b3364e922c26cf7631b7fd38ce523:

util/vfio-helpers: Assert offset is aligned to page size (2020-11-03 19:06:23 +0000)

Pull request for 5.2

NVMe fixes to solve IOMMU issues on non-x86 and error message/tracing

improvements. Elena Afanasova's ioeventfd fixes are also included.

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

Elena Afanasova (2):

accel/kvm: add PIO ioeventfds only in case kvm_eventfds_allowed is

true

softmmu/memory: fix memory_region_ioeventfd_equal()

Eric Auger (4):

block/nvme: Change size and alignment of IDENTIFY response buffer

block/nvme: Change size and alignment of queue

block/nvme: Change size and alignment of prp_list_pages

block/nvme: Align iov's va and size on host page size

Philippe Mathieu-Daudé (27):

MAINTAINERS: Cover "block/nvme.h" file

block/nvme: Use hex format to display offset in trace events

block/nvme: Report warning with warn_report()

block/nvme: Trace controller capabilities

block/nvme: Trace nvme_poll_queue() per queue

block/nvme: Improve nvme_free_req_queue_wait() trace information

block/nvme: Trace queue pair creation/deletion

block/nvme: Move definitions before structure declarations

block/nvme: Use unsigned integer for queue counter/size

block/nvme: Make nvme_identify() return boolean indicating error

block/nvme: Make nvme_init_queue() return boolean indicating error

block/nvme: Introduce Completion Queue definitions

block/nvme: Use definitions instead of magic values in add_io_queue()

block/nvme: Correctly initialize Admin Queue Attributes

block/nvme: Simplify ADMIN queue access

block/nvme: Simplify nvme_cmd_sync()

block/nvme: Set request_alignment at initialization

block/nvme: Correct minimum device page size

block/nvme: Fix use of write-only doorbells page on Aarch64 arch

block/nvme: Fix nvme_submit_command() on big-endian host

util/vfio-helpers: Improve reporting unsupported IOMMU type

util/vfio-helpers: Trace PCI I/O config accesses

util/vfio-helpers: Trace PCI BAR region info

util/vfio-helpers: Trace where BARs are mapped

util/vfio-helpers: Improve DMA trace events

util/vfio-helpers: Convert vfio_dump_mapping to trace events

util/vfio-helpers: Assert offset is aligned to page size

MAINTAINERS | 2 +

include/block/nvme.h | 18 ++--

accel/kvm/kvm-all.c | 6 +-

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

softmmu/memory.c | 11 ++-

util/vfio-helpers.c | 43 +++++----

block/trace-events | 30 ++++---

util/trace-events | 10 ++-

8 files changed, 195 insertions(+), 134 deletions(-)

2.28.0

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

The QEMU_VFIO_DEBUG definition is only modifiable at build-time.

Trace events can be enabled at run-time. As we prefer the latter,

convert qemu_vfio_dump_mappings() to use trace events instead

of fprintf().

Reviewed-by: Fam Zheng <fam@euphon.net>

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

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

Message-id: 20201103020733.2303148-7-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

util/vfio-helpers.c | 19 ++++---------------

util/trace-events | 1 +

2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ QEMUVFIOState *qemu_vfio_open_pci(const char *device, Error **errp)

return s;

}

-static void qemu_vfio_dump_mapping(IOVAMapping *m)

-{

- if (QEMU_VFIO_DEBUG) {

- printf(" vfio mapping %p %" PRIx64 " to %" PRIx64 "\n", m->host,

- (uint64_t)m->size, (uint64_t)m->iova);

- }

-}

-

static void qemu_vfio_dump_mappings(QEMUVFIOState *s)

{

- int i;

-

- if (QEMU_VFIO_DEBUG) {

- printf("vfio mappings\n");

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

- qemu_vfio_dump_mapping(&s->mappings[i]);

- }

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

+ trace_qemu_vfio_dump_mapping(s->mappings[i].host,

+ s->mappings[i].iova,

+ s->mappings[i].size);

}

}

diff --git a/util/trace-events b/util/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/util/trace-events

+++ b/util/trace-events

@@ -XXX,XX +XXX,XX @@ qemu_mutex_unlock(void *mutex, const char *file, const int line) "released mutex

qemu_vfio_dma_reset_temporary(void *s) "s %p"

qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s %p host %p size 0x%zx"

qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p host %p size 0x%zx"

+qemu_vfio_dump_mapping(void *host, uint64_t iova, size_t size) "vfio mapping %p to iova 0x%08" PRIx64 " size 0x%zx"

qemu_vfio_find_mapping(void *s, void *p) "s %p host %p"

qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova) "s %p host %p size 0x%zx index %d iova 0x%"PRIx64

qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%"PRIx64 " size 0x%zx"

2.28.0

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

Change the confuse "VFIO IOMMU check failed" error message by

the explicit "VFIO IOMMU Type1 is not supported" once.

Example on POWER:

$ qemu-system-ppc64 -drive if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw

qemu-system-ppc64: -drive if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw: VFIO IOMMU Type1 is not supported

Suggested-by: Alex Williamson <alex.williamson@redhat.com>

Reviewed-by: Fam Zheng <fam@euphon.net>

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

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

Message-id: 20201103020733.2303148-2-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

util/vfio-helpers.c | 2 +-

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char *device,

}

if (!ioctl(s->container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {

- error_setg_errno(errp, errno, "VFIO IOMMU check failed");

+ error_setg_errno(errp, errno, "VFIO IOMMU Type1 is not supported");

ret = -EINVAL;

goto fail_container;

2.28.0

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

For debugging purpose, trace where DMA regions are mapped.

Reviewed-by: Fam Zheng <fam@euphon.net>

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

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

Message-id: 20201103020733.2303148-6-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

util/vfio-helpers.c | 3 ++-

util/trace-events | 5 +++--

2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c

--- a/util/vfio-helpers.c

+++ b/util/vfio-helpers.c

@@ -XXX,XX +XXX,XX @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,

.vaddr = (uintptr_t)host,

.size = size,

};

- trace_qemu_vfio_do_mapping(s, host, size, iova);

+ trace_qemu_vfio_do_mapping(s, host, iova, size);

if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, &dma_map)) {

error_report("VFIO_MAP_DMA failed: %s", strerror(errno));

@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,

}

}

}

+ trace_qemu_vfio_dma_mapped(s, host, iova0, size);

if (iova) {

*iova = iova0;

}

diff --git a/util/trace-events b/util/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/util/trace-events

+++ b/util/trace-events

@@ -XXX,XX +XXX,XX @@ qemu_vfio_ram_block_added(void *s, void *p, size_t size) "s %p host %p size 0x%z

qemu_vfio_ram_block_removed(void *s, void *p, size_t size) "s %p host %p size 0x%zx"

qemu_vfio_find_mapping(void *s, void *p) "s %p host %p"

qemu_vfio_new_mapping(void *s, void *host, size_t size, int index, uint64_t iova) "s %p host %p size 0x%zx index %d iova 0x%"PRIx64

-qemu_vfio_do_mapping(void *s, void *host, size_t size, uint64_t iova) "s %p host %p size 0x%zx iova 0x%"PRIx64

-qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size 0x%zx temporary %d iova %p"

+qemu_vfio_do_mapping(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%"PRIx64 " size 0x%zx"

+qemu_vfio_dma_map(void *s, void *host, size_t size, bool temporary, uint64_t *iova) "s %p host %p size 0x%zx temporary %d &iova %p"

+qemu_vfio_dma_mapped(void *s, void *host, uint64_t iova, size_t size) "s %p host %p <-> iova 0x%"PRIx64" size 0x%zx"

qemu_vfio_dma_unmap(void *s, void *host) "s %p host %p"

qemu_vfio_pci_read_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "read cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%"PRIx64" size 0x%"PRIx64")"

qemu_vfio_pci_write_config(void *buf, int ofs, int size, uint64_t region_ofs, uint64_t region_size) "write cfg ptr %p ofs 0x%x size 0x%x (region addr 0x%"PRIx64" size 0x%"PRIx64")"

2.28.0

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

As all commands use the ADMIN queue, it is pointless to pass

it as argument each time. Remove the argument, and rename the

function as nvme_admin_cmd_sync() to make this new behavior

clearer.

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Tested-by: Eric Auger <eric.auger@redhat.com>

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

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

Message-id: 20201029093306.1063879-17-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

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

1 file changed, 10 insertions(+), 9 deletions(-)

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

--- a/block/nvme.c

+++ b/block/nvme.c

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

qemu_mutex_unlock(&q->lock);

}

-static void nvme_cmd_sync_cb(void *opaque, int ret)

+static void nvme_admin_cmd_sync_cb(void *opaque, int ret)

{

int *pret = opaque;

*pret = ret;

aio_wait_kick();

}

-static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,

- NvmeCmd *cmd)

+static int nvme_admin_cmd_sync(BlockDriverState *bs, NvmeCmd *cmd)

{

+ BDRVNVMeState *s = bs->opaque;

+ NVMeQueuePair *q = s->queues[INDEX_ADMIN];

AioContext *aio_context = bdrv_get_aio_context(bs);

NVMeRequest *req;

int ret = -EINPROGRESS;

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

if (!req) {

return -EBUSY;

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

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

AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS);

return ret;

@@ -XXX,XX +XXX,XX @@ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)

memset(id, 0, sizeof(*id));

cmd.dptr.prp1 = cpu_to_le64(iova);

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

+ if (nvme_admin_cmd_sync(bs, &cmd)) {

error_setg(errp, "Failed to identify controller");

goto out;

}

@@ -XXX,XX +XXX,XX @@ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)

memset(id, 0, sizeof(*id));

cmd.cdw10 = 0;

cmd.nsid = cpu_to_le32(namespace);

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

+ if (nvme_admin_cmd_sync(bs, &cmd)) {

error_setg(errp, "Failed to identify namespace");

goto out;

}

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

.cdw10 = cpu_to_le32(((queue_size - 1) << 16) | n),

.cdw11 = cpu_to_le32(NVME_CQ_IEN | NVME_CQ_PC),

};

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

+ if (nvme_admin_cmd_sync(bs, &cmd)) {

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

goto out_error;

}

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

.cdw10 = cpu_to_le32(((queue_size - 1) << 16) | n),

.cdw11 = cpu_to_le32(NVME_SQ_PC | (n << 16)),

};

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

+ if (nvme_admin_cmd_sync(bs, &cmd)) {

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

goto out_error;

}

@@ -XXX,XX +XXX,XX @@ static int nvme_enable_disable_write_cache(BlockDriverState *bs, bool enable,

.cdw11 = cpu_to_le32(enable ? 0x01 : 0x00),

};

- ret = nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd);

+ ret = nvme_admin_cmd_sync(bs, &cmd);

if (ret) {

error_setg(errp, "Failed to configure NVMe write cache");

2.28.0

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

mmap(2) states:

'offset' must be a multiple of the page size as returned

by sysconf(_SC_PAGE_SIZE).

Add an assertion to be sure we don't break this contract.

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

Message-id: 20201103020733.2303148-8-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

---

util/vfio-helpers.c | 1 +

1 file changed, 1 insertion(+)

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

Just for consistency, following the example documented since

commit e3fe3988d7 ("error: Document Error API usage rules"),

return a boolean value indicating an error is set or not.

Directly pass errp as the local_err is not requested in our

case.

Tested-by: Eric Auger <eric.auger@redhat.com>

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

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

Message-id: 20201029093306.1063879-11-philmd@redhat.com

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

Tested-by: Eric Auger <eric.auger@redhat.com>

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

1 file changed, 7 insertions(+), 5 deletions(-)

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

--- a/block/nvme.c

+++ b/block/nvme.c

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

-static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)

+/* Returns true on success, false on failure. */

+static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)

BDRVNVMeState *s = bs->opaque;

+ bool ret = false;

union {

NvmeIdCtrl ctrl;

NvmeIdNs ns;