RE: [PATCH] Revert "vhost-blk: set features before setting inflight feature"

2020-11-02 Thread Yu, Jin
Hi  Stefan,
I have sent a version 2 and it should fix this issue.
I also test it several times and I did not meet this issue again.

Thanks for your report.

Best Regards
Jin

> -Original Message-
> From: Stefan Hajnoczi 
> Sent: Tuesday, November 3, 2020 12:57 AM
> To: qemu-de...@nongnu.org
> Cc: qemu-block@nongnu.org; Raphael Norwitz
> ; Max Reitz ; Kevin
> Wolf ; Michael S. Tsirkin ; Stefan
> Hajnoczi ; Yu, Jin 
> Subject: [PATCH] Revert "vhost-blk: set features before setting inflight
> feature"
> 
> This reverts commit adb29c027341ba095a3ef4beef6aaef86d3a520e.
> 
> The commit broke -device vhost-user-blk-pci because the
> vhost_dev_prepare_inflight() function it introduced segfaults in
> vhost_dev_set_features() when attempting to access struct vhost_dev's vdev
> pointer before it has been assigned.
> 
> To reproduce the segfault simply launch a vhost-user-blk device with the
> contrib vhost-user-blk device backend:
> 
>   $ build/contrib/vhost-user-blk/vhost-user-blk -s /tmp/vhost-user-blk.sock -r
> -b /var/tmp/foo.img
>   $ build/qemu-system-x86_64 \
> -device vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 \
> -object memory-backend-memfd,id=mem,size=1G,share=on \
> -M memory-backend=mem,accel=kvm \
> -chardev socket,id=char1,path=/tmp/vhost-user-blk.sock
>   Segmentation fault (core dumped)
> 
> Cc: Jin Yu 
> Cc: Raphael Norwitz 
> Cc: Michael S. Tsirkin 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  include/hw/virtio/vhost.h |  1 -
>  hw/block/vhost-user-blk.c |  6 --
>  hw/virtio/vhost.c | 18 --
>  3 files changed, 25 deletions(-)
> 
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index
> 839bfb153c..94585067f7 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -141,7 +141,6 @@ void vhost_dev_reset_inflight(struct vhost_inflight
> *inflight);  void vhost_dev_free_inflight(struct vhost_inflight *inflight);  
> void
> vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);  int
> vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f); -int
> vhost_dev_prepare_inflight(struct vhost_dev *hdev);  int
> vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight);  int
> vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size, diff --git
> a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index
> f67b29bbf3..a076b1e54d 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -131,12 +131,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
> 
>  s->dev.acked_features = vdev->guest_features;
> 
> -ret = vhost_dev_prepare_inflight(&s->dev);
> -if (ret < 0) {
> -error_report("Error set inflight format: %d", -ret);
> -goto err_guest_notifiers;
> -}
> -
>  if (!s->inflight->addr) {
>  ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight);
>  if (ret < 0) {
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
> f2482378c6..79b2be20df 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1645,24 +1645,6 @@ int vhost_dev_load_inflight(struct vhost_inflight
> *inflight, QEMUFile *f)
>  return 0;
>  }
> 
> -int vhost_dev_prepare_inflight(struct vhost_dev *hdev) -{
> -int r;
> -
> -if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
> -hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
> -return 0;
> -}
> -
> -r = vhost_dev_set_features(hdev, hdev->log_enabled);
> -if (r < 0) {
> -VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
> -return r;
> -}
> -
> -return 0;
> -}
> -
>  int vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight)  {
> --
> 2.28.0




[PATCH v2] vhost-blk: set features before setting inflight feature

2020-11-02 Thread Jin Yu
Virtqueue has split and packed, so before setting inflight,
you need to inform the back-end virtqueue format.

Signed-off-by: Jin Yu 
Acked-by: Raphael Norwitz 
---
v2:
* Fixed the segfault.
---
 hw/block/vhost-user-blk.c |  6 ++
 hw/virtio/vhost.c | 20 
 include/hw/virtio/vhost.h |  1 +
 3 files changed, 27 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 39aec42dae..db079a89c0 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -131,6 +131,12 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
 
 s->dev.acked_features = vdev->guest_features;
 
+ret = vhost_dev_prepare_inflight(&s->dev, vdev);
+if (ret < 0) {
+error_report("Error set inflight format: %d", -ret);
+goto err_guest_notifiers;
+}
+
 if (!s->inflight->addr) {
 ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight);
 if (ret < 0) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1a1384e7a6..6ffbfbfb9e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1616,6 +1616,26 @@ int vhost_dev_load_inflight(struct vhost_inflight 
*inflight, QEMUFile *f)
 return 0;
 }
 
+int vhost_dev_prepare_inflight(struct vhost_dev *hdev, VirtIODevice *vdev)
+{
+int r;
+
+if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
+hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
+return 0;
+}
+
+hdev->vdev = vdev;
+
+r = vhost_dev_set_features(hdev, hdev->log_enabled);
+if (r < 0) {
+VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
+return r;
+}
+
+return 0;
+}
+
 int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight)
 {
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 767a95ec0b..d25f0947f7 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -140,6 +140,7 @@ void vhost_dev_reset_inflight(struct vhost_inflight 
*inflight);
 void vhost_dev_free_inflight(struct vhost_inflight *inflight);
 void vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);
 int vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f);
+int vhost_dev_prepare_inflight(struct vhost_dev *hdev, VirtIODevice *vdev);
 int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight);
 int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
-- 
2.17.2




Re: [PATCH] crypto: Add spaces around operator

2020-11-02 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com
Subject: [PATCH] crypto: Add spaces around operator

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com -> 
patchew/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com
Switched to a new branch 'test'
53a011e crypto: Add spaces around operator

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#24: FILE: crypto/aes.c:1083:
+if (bits == 128)
[...]
-else if (bits==192)
[...]
 key->rounds = 12;
[...]

ERROR: braces {} are necessary for all arms of this statement
#27: FILE: crypto/aes.c:1085:
+else if (bits == 192)
[...]
 else
[...]

ERROR: space prohibited after that open parenthesis '('
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];

ERROR: space prohibited before that close parenthesis ')'
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];

ERROR: space required before the open parenthesis '('
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];

ERROR: trailing statements should be on next line
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];

ERROR: braces {} are necessary for all arms of this statement
#49: FILE: crypto/desrfb.c:96:
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
[...]

total: 7 errors, 0 warnings, 35 lines checked

Commit 53a011edf848 (crypto: Add spaces around operator) has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/bb45d748-01e4-2fde-9fe3-32a7db4b6...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH] crypto: Add spaces around operator

2020-11-02 Thread shiliyang
I am reading crypto related code, find some code style problems while
using checkpatch.pl to check crypto folder. Fix the error style
problems.

Signed-off-by: Liyang Shi 

---
 crypto/aes.c  | 6 +++---
 crypto/desrfb.c   | 2 +-
 crypto/tlscredsx509.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/crypto/aes.c b/crypto/aes.c
index 159800df65..af72ff7779 100644
--- a/crypto/aes.c
+++ b/crypto/aes.c
@@ -1080,9 +1080,9 @@ int AES_set_encrypt_key(const unsigned char *userKey, 
const int bits,

 rk = key->rd_key;

-if (bits==128)
+if (bits == 128)
 key->rounds = 10;
-else if (bits==192)
+else if (bits == 192)
 key->rounds = 12;
 else
 key->rounds = 14;
@@ -1182,7 +1182,7 @@ int AES_set_decrypt_key(const unsigned char *userKey, 
const int bits,
 rk = key->rd_key;

 /* invert the order of the round keys: */
-for (i = 0, j = 4*(key->rounds); i < j; i += 4, j -= 4) {
+for (i = 0, j = 4 * (key->rounds); i < j; i += 4, j -= 4) {
 temp = rk[i]; rk[i] = rk[j]; rk[j] = temp;
 temp = rk[i + 1]; rk[i + 1] = rk[j + 1]; rk[j + 1] = temp;
 temp = rk[i + 2]; rk[i + 2] = rk[j + 2]; rk[j + 2] = temp;
diff --git a/crypto/desrfb.c b/crypto/desrfb.c
index 3274c36510..14288fcf96 100644
--- a/crypto/desrfb.c
+++ b/crypto/desrfb.c
@@ -93,7 +93,7 @@ void deskey(unsigned char *key, int edf)
 }
 for( j = 0; j < 24; j++ ) {
 if( pcr[pc2[j]] ) kn[m] |= bigbyte[j];
-if( pcr[pc2[j+24]] ) kn[n] |= bigbyte[j];
+if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
 }
 }
 cookey(kn);
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dd7267ccdb..c89dd62435 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -143,7 +143,7 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 
*creds,
 if (status < 0) {
 if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
 usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
-GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_KEY_ENCIPHERMENT;
+GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT;
 } else {
 error_setg(errp,
"Unable to query certificate %s key usage: %s",
-- 
2.17.1.windows.2




[PATCH-for-5.2 v3 5/7] util/vfio-helpers: Improve DMA trace events

2020-11-02 Thread Philippe Mathieu-Daudé
For debugging purpose, trace where DMA regions are mapped.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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
index 278c54902e7..c24a510df82 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -627,7 +627,7 @@ 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));
@@ -783,6 +783,7 @@ 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 52d43cda3f3..08239941cb2 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -82,8 +82,9 @@ 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.26.2




[PATCH-for-5.2 v3 6/7] util/vfio-helpers: Convert vfio_dump_mapping to trace events

2020-11-02 Thread Philippe Mathieu-Daudé
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 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 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
index c24a510df82..73f7bfa7540 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -521,23 +521,12 @@ 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 08239941cb2..61e0d4bcdfe 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -80,6 +80,7 @@ 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.26.2




[PATCH-for-5.2 v3 7/7] util/vfio-helpers: Assert offset is aligned to page size

2020-11-02 Thread Philippe Mathieu-Daudé
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é 
---
 util/vfio-helpers.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 73f7bfa7540..804768d5c66 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -162,6 +162,7 @@ void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
 Error **errp)
 {
 void *p;
+assert(QEMU_IS_ALIGNED(offset, qemu_real_host_page_size));
 assert_bar_index_valid(s, index);
 p = mmap(NULL, MIN(size, s->bar_region_info[index].size - offset),
  prot, MAP_SHARED,
-- 
2.26.2




[PATCH-for-5.2 v3 4/7] util/vfio-helpers: Trace where BARs are mapped

2020-11-02 Thread Philippe Mathieu-Daudé
For debugging purpose, trace where a BAR is mapped.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 2 ++
 util/trace-events   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index cd6287c3a98..278c54902e7 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -166,6 +166,8 @@ void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
 p = mmap(NULL, MIN(size, s->bar_region_info[index].size - offset),
  prot, MAP_SHARED,
  s->device, s->bar_region_info[index].offset + offset);
+trace_qemu_vfio_pci_map_bar(index, s->bar_region_info[index].offset ,
+size, offset, p);
 if (p == MAP_FAILED) {
 error_setg_errno(errp, errno, "Failed to map BAR region");
 p = NULL;
diff --git a/util/trace-events b/util/trace-events
index 0753e4a0ed1..52d43cda3f3 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -88,3 +88,4 @@ 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")"
 qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t 
region_size, uint32_t cap_offset) "region '%s' addr 0x%"PRIx64" size 
0x%"PRIx64" cap_ofs 0x%"PRIx32
+qemu_vfio_pci_map_bar(int index, uint64_t region_ofs, uint64_t region_size, 
int ofs, void *host) "map region bar#%d addr 0x%"PRIx64" size 0x%"PRIx64" ofs 
0x%x host %p"
-- 
2.26.2




[PATCH-for-5.2 v3 2/7] util/vfio-helpers: Trace PCI I/O config accesses

2020-11-02 Thread Philippe Mathieu-Daudé
We sometime get kernel panic with some devices on Aarch64
hosts. Alex Williamson suggests it might be broken PCIe
root complex. Add trace event to record the latest I/O
access before crashing. In case, assert our accesses are
aligned.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 8 
 util/trace-events   | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 14a549510fe..1d4efafcaa4 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -227,6 +227,10 @@ static int qemu_vfio_pci_read_config(QEMUVFIOState *s, 
void *buf,
 {
 int ret;
 
+trace_qemu_vfio_pci_read_config(buf, ofs, size,
+s->config_region_info.offset,
+s->config_region_info.size);
+assert(QEMU_IS_ALIGNED(s->config_region_info.offset + ofs, size));
 do {
 ret = pread(s->device, buf, size, s->config_region_info.offset + ofs);
 } while (ret == -1 && errno == EINTR);
@@ -237,6 +241,10 @@ static int qemu_vfio_pci_write_config(QEMUVFIOState *s, 
void *buf, int size, int
 {
 int ret;
 
+trace_qemu_vfio_pci_write_config(buf, ofs, size,
+ s->config_region_info.offset,
+ s->config_region_info.size);
+assert(QEMU_IS_ALIGNED(s->config_region_info.offset + ofs, size));
 do {
 ret = pwrite(s->device, buf, size, s->config_region_info.offset + ofs);
 } while (ret == -1 && errno == EINTR);
diff --git a/util/trace-events b/util/trace-events
index 24c31803b01..8d3615e717b 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -85,3 +85,5 @@ qemu_vfio_new_mapping(void *s, void *host, size_t size, int 
index, uint64_t iova
 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_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.26.2




[PATCH-for-5.2 v3 3/7] util/vfio-helpers: Trace PCI BAR region info

2020-11-02 Thread Philippe Mathieu-Daudé
For debug purpose, trace BAR regions info.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 8 
 util/trace-events   | 1 +
 2 files changed, 9 insertions(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 1d4efafcaa4..cd6287c3a98 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -136,6 +136,7 @@ static inline void assert_bar_index_valid(QEMUVFIOState *s, 
int index)
 
 static int qemu_vfio_pci_init_bar(QEMUVFIOState *s, int index, Error **errp)
 {
+g_autofree char *barname = NULL;
 assert_bar_index_valid(s, index);
 s->bar_region_info[index] = (struct vfio_region_info) {
 .index = VFIO_PCI_BAR0_REGION_INDEX + index,
@@ -145,6 +146,10 @@ static int qemu_vfio_pci_init_bar(QEMUVFIOState *s, int 
index, Error **errp)
 error_setg_errno(errp, errno, "Failed to get BAR region info");
 return -errno;
 }
+barname = g_strdup_printf("bar[%d]", index);
+trace_qemu_vfio_region_info(barname, s->bar_region_info[index].offset,
+s->bar_region_info[index].size,
+s->bar_region_info[index].cap_offset);
 
 return 0;
 }
@@ -416,6 +421,9 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char 
*device,
 ret = -errno;
 goto fail;
 }
+trace_qemu_vfio_region_info("config", s->config_region_info.offset,
+s->config_region_info.size,
+s->config_region_info.cap_offset);
 
 for (i = 0; i < ARRAY_SIZE(s->bar_region_info); i++) {
 ret = qemu_vfio_pci_init_bar(s, i, errp);
diff --git a/util/trace-events b/util/trace-events
index 8d3615e717b..0753e4a0ed1 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -87,3 +87,4 @@ qemu_vfio_dma_map(void *s, void *host, size_t size, bool 
temporary, uint64_t *io
 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")"
+qemu_vfio_region_info(const char *desc, uint64_t region_ofs, uint64_t 
region_size, uint32_t cap_offset) "region '%s' addr 0x%"PRIx64" size 
0x%"PRIx64" cap_ofs 0x%"PRIx32
-- 
2.26.2




[PATCH-for-5.2 v3 0/7] util/vfio-helpers: Generic code strengthening

2020-11-02 Thread Philippe Mathieu-Daudé
v3:
- Extract reviewed patches from
  "util/vfio-helpers: Allow using multiple MSIX IRQs"
- Added "Assert offset is aligned to page size"
  which would have helped debugging:
  "block/nvme: Fix use of write-only doorbells page on Aarch64 arch"

Missing review: 7

Based-on: <20201029093306.1063879-1-phi...@redhat.com>

Philippe Mathieu-Daudé (7):
  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

 util/vfio-helpers.c | 43 ++-
 util/trace-events   | 10 --
 2 files changed, 34 insertions(+), 19 deletions(-)

-- 
2.26.2





[PATCH-for-5.2 v3 1/7] util/vfio-helpers: Improve reporting unsupported IOMMU type

2020-11-02 Thread Philippe Mathieu-Daudé
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 
Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index c469beb0616..14a549510fe 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -300,7 +300,7 @@ 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.26.2




Re: [PULL] nvme emulation patches for 5.2

2020-11-02 Thread Peter Maydell
On Mon, 2 Nov 2020 at 15:27, Keith Busch  wrote:
>
> Hi Peter,
>
> We are sorting out Klaus' signature authentication this week, but in the
> interest of timing, I've signed our pull request for this round.
>
> The following changes since commit 1dc887329a10903940501b43e8c0cc67af7c06d5:
>
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20201026' 
> into staging (2020-10-26 17:19:26 +)
>
> are available in the Git repository at:
>
>   git://git.infradead.org/qemu-nvme.git tags/pull-nvme-20201102
>
> for you to fetch changes up to 843c8f91a7ad63f8f3e4e564d3f41f3d030ab8a9:
>
>   hw/block/nvme: fix queue identifer validation (2020-10-27 11:29:25 +0100)
>
> nvme emulation patches for 5.2
>
>   - lots of cleanups
>   - add support for scatter/gather lists
>   - add support for multiple namespaces (adds new nvme-ns device)
>   - change default pci vendor/device id
>   - add support for per-namespace smart log
>
> 
> nvme pull 2 Nov 2020
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM



Re: [PULL v3 06/28] block/export: vhost-user block device backend server

2020-11-02 Thread Peter Maydell
On Fri, 23 Oct 2020 at 16:22, Stefan Hajnoczi  wrote:
>
> From: Coiby Xu 
>
> By making use of libvhost-user, block device drive can be shared to
> the connected vhost-user client. Only one client can connect to the
> server one time.
>
> Since vhost-user-server needs a block drive to be created first, delay
> the creation of this object.

Hi; Coverity points out a possible bug in this function
(CID 1435956):

> +static int coroutine_fn
> +vu_block_discard_write_zeroes(VuBlockReq *req, struct iovec *iov,
> +  uint32_t iovcnt, uint32_t type)
> +{
> +struct virtio_blk_discard_write_zeroes desc;
> +ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
> +if (unlikely(size != sizeof(desc))) {
> +error_report("Invalid size %zd, expect %zu", size, sizeof(desc));
> +return -EINVAL;
> +}
> +
> +VuBlockDev *vdev_blk = get_vu_block_device_by_server(req->server);
> +uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,
> +  le32_to_cpu(desc.num_sectors) << 9 };

Here we get a 32-bit integer from desc.num_sectors, and then the
shift left might make it overflow the 32-bit value before it gets
put into a uint64_t in the range array. Should this be

   (uint64_t)le32_to_cpu(desc.num_sectors) << 9

?

thanks
-- PMM



Re: [PATCH 00/12] block/export: vhost-user-blk server cleanups and tests

2020-11-02 Thread Stefan Hajnoczi
On Mon, Nov 02, 2020 at 05:43:19AM -0500, Michael S. Tsirkin wrote:
> On Fri, Oct 30, 2020 at 08:42:22AM -0400, Michael S. Tsirkin wrote:
> > On Tue, Oct 27, 2020 at 05:35:16PM +, Stefan Hajnoczi wrote:
> > > This patch series solves some issues with the new vhost-user-blk-server 
> > > and
> > > adds the qtest test case. The test case was not included in the pull 
> > > request
> > > that introduced the vhost-user-blk server because of reliability issues 
> > > that
> > > are fixed in this patch series.
> > 
> > 
> > Fails make check for me:
> > 
> > Running test qtest-i386/qos-test
> > Broken pipe
> > ../qemu/tests/qtest/libqtest.c:161: kill_qemu() detected QEMU death from 
> > signal 11 (Segmentation fault) (core dumped)
> > ERROR qtest-i386/qos-test - too few tests run (expected 92, got 65)
> > make: *** [Makefile.mtest:1857: run-test-230] Error 1
> 
> And here's the coredump:

Thanks! qemu.git/master is broken. The segfault was introduced in
adb29c027341ba095a3ef4beef6aaef86d3a520e ("vhost-blk: set features
before setting inflight feature"). The code in question has no test
coverage so we didn't know that vhost-user-blk is broken in QEMU.

I have sent a patch to revert the commit. Let's do that for QEMU 5.2
unless a straightforward fix can be provided in place of the revert.

Stefan


signature.asc
Description: PGP signature


[PATCH] Revert "vhost-blk: set features before setting inflight feature"

2020-11-02 Thread Stefan Hajnoczi
This reverts commit adb29c027341ba095a3ef4beef6aaef86d3a520e.

The commit broke -device vhost-user-blk-pci because the
vhost_dev_prepare_inflight() function it introduced segfaults in
vhost_dev_set_features() when attempting to access struct vhost_dev's
vdev pointer before it has been assigned.

To reproduce the segfault simply launch a vhost-user-blk device with the
contrib vhost-user-blk device backend:

  $ build/contrib/vhost-user-blk/vhost-user-blk -s /tmp/vhost-user-blk.sock -r 
-b /var/tmp/foo.img
  $ build/qemu-system-x86_64 \
-device vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 \
-object memory-backend-memfd,id=mem,size=1G,share=on \
-M memory-backend=mem,accel=kvm \
-chardev socket,id=char1,path=/tmp/vhost-user-blk.sock
  Segmentation fault (core dumped)

Cc: Jin Yu 
Cc: Raphael Norwitz 
Cc: Michael S. Tsirkin 
Signed-off-by: Stefan Hajnoczi 
---
 include/hw/virtio/vhost.h |  1 -
 hw/block/vhost-user-blk.c |  6 --
 hw/virtio/vhost.c | 18 --
 3 files changed, 25 deletions(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 839bfb153c..94585067f7 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -141,7 +141,6 @@ void vhost_dev_reset_inflight(struct vhost_inflight 
*inflight);
 void vhost_dev_free_inflight(struct vhost_inflight *inflight);
 void vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);
 int vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f);
-int vhost_dev_prepare_inflight(struct vhost_dev *hdev);
 int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight);
 int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f67b29bbf3..a076b1e54d 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -131,12 +131,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
 
 s->dev.acked_features = vdev->guest_features;
 
-ret = vhost_dev_prepare_inflight(&s->dev);
-if (ret < 0) {
-error_report("Error set inflight format: %d", -ret);
-goto err_guest_notifiers;
-}
-
 if (!s->inflight->addr) {
 ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight);
 if (ret < 0) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f2482378c6..79b2be20df 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1645,24 +1645,6 @@ int vhost_dev_load_inflight(struct vhost_inflight 
*inflight, QEMUFile *f)
 return 0;
 }
 
-int vhost_dev_prepare_inflight(struct vhost_dev *hdev)
-{
-int r;
- 
-if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
-hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
-return 0;
-}
- 
-r = vhost_dev_set_features(hdev, hdev->log_enabled);
-if (r < 0) {
-VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
-return r;
-}
-
-return 0;
-}
-
 int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight)
 {
-- 
2.28.0



Re: [PULL] nvme emulation patches for 5.2

2020-11-02 Thread Philippe Mathieu-Daudé
On 11/2/20 4:27 PM, Keith Busch wrote:
> Hi Peter,
> 
> We are sorting out Klaus' signature authentication this week, but in the
> interest of timing, I've signed our pull request for this round.
> 
> The following changes since commit 1dc887329a10903940501b43e8c0cc67af7c06d5:
> 
>   Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20201026' 
> into staging (2020-10-26 17:19:26 +)
> 
> are available in the Git repository at:
> 
>   git://git.infradead.org/qemu-nvme.git tags/pull-nvme-20201102
> 
> for you to fetch changes up to 843c8f91a7ad63f8f3e4e564d3f41f3d030ab8a9:
> 
>   hw/block/nvme: fix queue identifer validation (2020-10-27 11:29:25 +0100)
> 

FYI this part ...

> nvme emulation patches for 5.2
> 
>   - lots of cleanups
>   - add support for scatter/gather lists
>   - add support for multiple namespaces (adds new nvme-ns device)
>   - change default pci vendor/device id
>   - add support for per-namespace smart log
> 
> 
> nvme pull 2 Nov 2020

... goes here, this is the tag part committed (the part before
the --- mark is removed).

> 
> Dmitry Fomichev (1):
>   hw/block/nvme: report actual LBA data shift in LBAF
> 
> Gollu Appalanaidu (4):
>   hw/block/nvme: add support for sgl bit bucket descriptor
>   hw/block/nvme: fix prp mapping status codes
>   hw/block/nvme: fix create IO SQ/CQ status codes
>   hw/block/nvme: fix queue identifer validation
> 
> Keith Busch (5):
>   hw/block/nvme: remove pointless rw indirection
>   hw/block/nvme: fix log page offset check
>   hw/block/nvme: support per-namespace smart log
>   hw/block/nvme: validate command set selected
>   hw/block/nvme: support for admin-only command set
> 
> Klaus Jensen (20):
>   hw/block/nvme: fix typo in trace event
>   pci: pass along the return value of dma_memory_rw
>   hw/block/nvme: handle dma errors
>   hw/block/nvme: commonize nvme_rw error handling
>   hw/block/nvme: alignment style fixes
>   hw/block/nvme: add a lba to bytes helper
>   hw/block/nvme: fix endian conversion
>   hw/block/nvme: add symbolic command name to trace events
>   hw/block/nvme: refactor aio submission
>   hw/block/nvme: default request status to success
>   hw/block/nvme: harden cmb access
>   hw/block/nvme: add support for scatter gather lists
>   hw/block/nvme: refactor identify active namespace id list
>   hw/block/nvme: support multiple namespaces
>   pci: allocate pci id for nvme
>   hw/block/nvme: change controller pci id
>   hw/block/nvme: update nsid when registered
>   hw/block/nvme: reject io commands if only admin command set selected
>   hw/block/nvme: add nsid to get/setfeat trace events
>   hw/block/nvme: add trace event for requests with non-zero status code
> 
>  MAINTAINERS|   1 +
>  docs/specs/nvme.txt|  23 ++
>  docs/specs/pci-ids.txt |   1 +
>  hw/block/meson.build   |   2 +-
>  hw/block/nvme-ns.c | 168 +
>  hw/block/nvme-ns.h |  74 
>  hw/block/nvme.c| 915 
> +++--
>  hw/block/nvme.h|  83 -
>  hw/block/trace-events  |  32 +-
>  hw/core/machine.c  |   1 +
>  include/block/nvme.h   |  18 +-
>  include/hw/pci/pci.h   |   4 +-
>  12 files changed, 1022 insertions(+), 300 deletions(-)
>  create mode 100644 docs/specs/nvme.txt
>  create mode 100644 hw/block/nvme-ns.c
>  create mode 100644 hw/block/nvme-ns.h
> 
> 




[PULL] nvme emulation patches for 5.2

2020-11-02 Thread Keith Busch
Hi Peter,

We are sorting out Klaus' signature authentication this week, but in the
interest of timing, I've signed our pull request for this round.

The following changes since commit 1dc887329a10903940501b43e8c0cc67af7c06d5:

  Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sd-next-20201026' 
into staging (2020-10-26 17:19:26 +)

are available in the Git repository at:

  git://git.infradead.org/qemu-nvme.git tags/pull-nvme-20201102

for you to fetch changes up to 843c8f91a7ad63f8f3e4e564d3f41f3d030ab8a9:

  hw/block/nvme: fix queue identifer validation (2020-10-27 11:29:25 +0100)

nvme emulation patches for 5.2

  - lots of cleanups
  - add support for scatter/gather lists
  - add support for multiple namespaces (adds new nvme-ns device)
  - change default pci vendor/device id
  - add support for per-namespace smart log


nvme pull 2 Nov 2020


Dmitry Fomichev (1):
  hw/block/nvme: report actual LBA data shift in LBAF

Gollu Appalanaidu (4):
  hw/block/nvme: add support for sgl bit bucket descriptor
  hw/block/nvme: fix prp mapping status codes
  hw/block/nvme: fix create IO SQ/CQ status codes
  hw/block/nvme: fix queue identifer validation

Keith Busch (5):
  hw/block/nvme: remove pointless rw indirection
  hw/block/nvme: fix log page offset check
  hw/block/nvme: support per-namespace smart log
  hw/block/nvme: validate command set selected
  hw/block/nvme: support for admin-only command set

Klaus Jensen (20):
  hw/block/nvme: fix typo in trace event
  pci: pass along the return value of dma_memory_rw
  hw/block/nvme: handle dma errors
  hw/block/nvme: commonize nvme_rw error handling
  hw/block/nvme: alignment style fixes
  hw/block/nvme: add a lba to bytes helper
  hw/block/nvme: fix endian conversion
  hw/block/nvme: add symbolic command name to trace events
  hw/block/nvme: refactor aio submission
  hw/block/nvme: default request status to success
  hw/block/nvme: harden cmb access
  hw/block/nvme: add support for scatter gather lists
  hw/block/nvme: refactor identify active namespace id list
  hw/block/nvme: support multiple namespaces
  pci: allocate pci id for nvme
  hw/block/nvme: change controller pci id
  hw/block/nvme: update nsid when registered
  hw/block/nvme: reject io commands if only admin command set selected
  hw/block/nvme: add nsid to get/setfeat trace events
  hw/block/nvme: add trace event for requests with non-zero status code

 MAINTAINERS|   1 +
 docs/specs/nvme.txt|  23 ++
 docs/specs/pci-ids.txt |   1 +
 hw/block/meson.build   |   2 +-
 hw/block/nvme-ns.c | 168 +
 hw/block/nvme-ns.h |  74 
 hw/block/nvme.c| 915 +++--
 hw/block/nvme.h|  83 -
 hw/block/trace-events  |  32 +-
 hw/core/machine.c  |   1 +
 include/block/nvme.h   |  18 +-
 include/hw/pci/pci.h   |   4 +-
 12 files changed, 1022 insertions(+), 300 deletions(-)
 create mode 100644 docs/specs/nvme.txt
 create mode 100644 hw/block/nvme-ns.c
 create mode 100644 hw/block/nvme-ns.h




Re: [PATCH] qmp: fix aio_poll() assertion failure on Windows

2020-11-02 Thread Mark Cave-Ayland

On 21/10/2020 07:40, Volker Rümelin wrote:


Commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" modified
aio_poll() in util/aio-posix.c to avoid an assertion failure. This
change is missing in util/aio-win32.c.

Apply the changes to util/aio-posix.c to util/aio-win32.c too.
This fixes an assertion failure on Windows whenever QEMU exits.

$ ./qemu-system-x86_64.exe -machine pc,accel=tcg -display gtk
**
ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed:
(in_aio_context_home_thread(ctx))
Bail out! ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion
failed: (in_aio_context_home_thread(ctx))

Fixes: 9ce44e2ce2 ("qmp: Move dispatcher to a coroutine")
Signed-off-by: Volker Rümelin 
---
  util/aio-win32.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/util/aio-win32.c b/util/aio-win32.c
index e7b1d649e9..168717b51b 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -18,6 +18,7 @@
  #include "qemu/osdep.h"
  #include "qemu-common.h"
  #include "block/block.h"
+#include "qemu/main-loop.h"
  #include "qemu/queue.h"
  #include "qemu/sockets.h"
  #include "qapi/error.h"
@@ -333,8 +334,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
   * 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.
+ *
+ * aio_poll() may only be called in the AioContext's thread. iohandler_ctx
+ * is special in that it runs in the main thread, but that thread's context
+ * is qemu_aio_context.
   */
-assert(in_aio_context_home_thread(ctx));
+assert(in_aio_context_home_thread(ctx == iohandler_get_aio_context() ?
+  qemu_get_aio_context() : ctx));
  progress = false;
  
  /* aio_notify can avoid the expensive event_notifier_set if


Sorry about the delay, however I've just tried this on top of git master and it fixes 
the problem with the assert() when exiting QEMU on Win32 so:


Tested-by: Mark Cave-Ayland 

Is it possible to get this merged for 5.2?


ATB,

Mark.



[PATCH] block/vvfat: Fix bad printf format specifiers

2020-11-02 Thread AlexChen
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
In addition, fix two error format problems found by checkpatch.pl:
ERROR: space required after that ',' (ctx:VxV)
+fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n",
   ^
ERROR: line over 90 characters
+fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path : 
"(null)", commit->param.rename.cluster, commit->action);

Reported-by: Euler Robot 
Signed-off-by: Alex Chen 
---
 block/vvfat.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 5abb90e7c7..cc2ec9af21 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1437,7 +1437,7 @@ static void print_direntry(const direntry_t* direntry)
 for(i=0;i<11;i++)
 ADD_CHAR(direntry->name[i]);
 buffer[j] = 0;
-fprintf(stderr,"%s attributes=0x%02x begin=%d size=%d\n",
+fprintf(stderr, "%s attributes=0x%02x begin=%u size=%d\n",
 buffer,
 direntry->attributes,
 begin_of_direntry(direntry),le32_to_cpu(direntry->size));
@@ -1446,7 +1446,7 @@ static void print_direntry(const direntry_t* direntry)

 static void print_mapping(const mapping_t* mapping)
 {
-fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, "
+fprintf(stderr, "mapping (%p): begin, end = %u, %u, dir_index = %u, "
 "first_mapping_index = %d, name = %s, mode = 0x%x, " ,
 mapping, mapping->begin, mapping->end, mapping->dir_index,
 mapping->first_mapping_index, mapping->path, mapping->mode);
@@ -1454,7 +1454,7 @@ static void print_mapping(const mapping_t* mapping)
 if (mapping->mode & MODE_DIRECTORY)
 fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n", 
mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index);
 else
-fprintf(stderr, "offset = %d\n", mapping->info.file.offset);
+fprintf(stderr, "offset = %u\n", mapping->info.file.offset);
 }
 #endif

@@ -1588,7 +1588,7 @@ typedef struct commit_t {
 static void clear_commits(BDRVVVFATState* s)
 {
 int i;
-DLOG(fprintf(stderr, "clear_commits (%d commits)\n", s->commits.next));
+DLOG(fprintf(stderr, "clear_commits (%u commits)\n", s->commits.next));
 for (i = 0; i < s->commits.next; i++) {
 commit_t* commit = array_get(&(s->commits), i);
 assert(commit->path || commit->action == ACTION_WRITEOUT);
@@ -2648,7 +2648,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
 fprintf(stderr, "handle_renames\n");
 for (i = 0; i < s->commits.next; i++) {
 commit_t* commit = array_get(&(s->commits), i);
-fprintf(stderr, "%d, %s (%d, %d)\n", i, commit->path ? commit->path : 
"(null)", commit->param.rename.cluster, commit->action);
+fprintf(stderr, "%d, %s (%u, %d)\n", i,
+commit->path ? commit->path : "(null)",
+commit->param.rename.cluster, commit->action);
 }
 #endif

-- 
2.19.1



Re: [PATCH 00/12] block/export: vhost-user-blk server cleanups and tests

2020-11-02 Thread Michael S. Tsirkin
On Fri, Oct 30, 2020 at 08:42:22AM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 27, 2020 at 05:35:16PM +, Stefan Hajnoczi wrote:
> > This patch series solves some issues with the new vhost-user-blk-server and
> > adds the qtest test case. The test case was not included in the pull request
> > that introduced the vhost-user-blk server because of reliability issues that
> > are fixed in this patch series.
> 
> 
> Fails make check for me:
> 
> Running test qtest-i386/qos-test
> Broken pipe
> ../qemu/tests/qtest/libqtest.c:161: kill_qemu() detected QEMU death from 
> signal 11 (Segmentation fault) (core dumped)
> ERROR qtest-i386/qos-test - too few tests run (expected 92, got 65)
> make: *** [Makefile.mtest:1857: run-test-230] Error 1

And here's the coredump:



[mst@tuck qemu-oot]$ coredumpctl  debug 853792
   PID: 853792 (qemu-system-i38)
   UID: 1000 (mst)
   GID: 1000 (mst)
Signal: 11 (SEGV)
 Timestamp: Fri 2020-10-30 08:41:31 EDT (2 days ago)
  Command Line: ./qemu-system-i386 -qtest unix:/tmp/qtest-853536.sock 
-qtest-log /dev/null -chardev socket,path=/tmp/qtest-853536.qmp,id=char0 -mon 
chardev=char0,mode=control -display none -M pc -device 
vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 -object 
memory-backend-memfd,id=mem,size=256M,share=on -M memory-backend=mem -chardev 
socket,id=char1,path=/tmp/qtest-853536-sock.krlJyA -accel qtest
Executable: /scm/qemu-oot/qemu-system-i386
 Control Group: /user.slice/user-1000.slice/session-4.scope
  Unit: session-4.scope
 Slice: user-1000.slice
   Session: 4
 Owner UID: 1000 (mst)
   Boot ID: 978b4cacb2df46319a9c6310b653f95d
Machine ID: 6234c9d2c9c34980ad6b1b2de307f043
  Hostname: tuck.redhat.com
   Storage: 
/var/lib/systemd/coredump/core.qemu-system-i38.1000.978b4cacb2df46319a9c6310b653f95d.853792.160406169100.lz4
   Message: Process 853792 (qemu-system-i38) of user 1000 dumped core.

Stack trace of thread 853792:
#0  0x55b2ace9ca0b vhost_dev_has_iommu (qemu-system-i386 + 
0x657a0b)
#1  0x55b2ace9fbbf vhost_dev_prepare_inflight 
(qemu-system-i386 + 0x65abbf)
#2  0x55b2ace51d01 vhost_user_blk_start (qemu-system-i386 + 
0x60cd01)
#3  0x55b2ace51f1a vhost_user_blk_set_status 
(qemu-system-i386 + 0x60cf1a)
#4  0x55b2acde489b virtio_set_status (qemu-system-i386 + 
0x59f89b)
#5  0x55b2acb38638 virtio_pci_common_write 
(qemu-system-i386 + 0x2f3638)
#6  0x55b2ace4818c memory_region_write_accessor 
(qemu-system-i386 + 0x60318c)
#7  0x55b2ace46cae access_with_adjusted_size 
(qemu-system-i386 + 0x601cae)
#8  0x55b2ace4a4c3 memory_region_dispatch_write 
(qemu-system-i386 + 0x6054c3)
#9  0x55b2ace72010 flatview_write_continue 
(qemu-system-i386 + 0x62d010)
#10 0x55b2ace74fc5 flatview_write (qemu-system-i386 + 
0x62ffc5)
#11 0x55b2acdc0981 qtest_process_command (qemu-system-i386 
+ 0x57b981)
#12 0x55b2acdc111d qtest_process_inbuf (qemu-system-i386 + 
0x57c11d)
#13 0x55b2acf7092e tcp_chr_read (qemu-system-i386 + 
0x72b92e)
#14 0x7f0554a5f78f g_main_context_dispatch 
(libglib-2.0.so.0 + 0x5278f)
#15 0x55b2acfbbed8 glib_pollfds_poll (qemu-system-i386 + 
0x776ed8)
#16 0x55b2acdccbf2 qemu_main_loop (qemu-system-i386 + 
0x587bf2)
#17 0x55b2acb0a5be main (qemu-system-i386 + 0x2c55be)
#18 0x7f055342 __libc_start_main (libc.so.6 + 0x27042)
#19 0x55b2acb0e97e _start (qemu-system-i386 + 0x2c997e)

Stack trace of thread 853794:
#0  0x7f05530a1801 clock_nanosleep@@GLIBC_2.17 (libc.so.6 + 
0xc8801)
#1  0x7f05530a7157 __nanosleep (libc.so.6 + 0xce157)
#2  0x7f0554a8b2d7 g_usleep (libglib-2.0.so.0 + 0x7e2d7)
#3  0x55b2acfbfa9a call_rcu_thread (qemu-system-i386 + 
0x77aa9a)
#4  0x55b2acfd1cb9 qemu_thread_start (qemu-system-i386 + 
0x78ccb9)
#5  0x7f05531ac432 start_thread (libpthread.so.0 + 0x9432)
#6  0x7f05530da913 __clone (libc.so.6 + 0x101913)

Stack trace of thread 853796:
#0  0x7f0553016962 __sigtimedwait (libc.so.6 + 0x3d962)
#1  0x7f05531b75bc sigwait (libpthread.so.0 + 0x145bc)
#2  0x55b2ace8c223 dummy_cpu_thread_fn (qemu-system-i386 + 
0x647223)
#3  0x55b2acfd1cb9 qemu_thread_start (qemu-system-i386 + 
0x78ccb9)
#4  0x7f05531ac432 start_thread (libpthread.so.0 + 0x9432)
#5  0x7f05530da913 __clone (libc.so.6 + 0x101913)

   

[PATCH] qemu-img: Make sure @sn_opts can be deleted in all error cases

2020-11-02 Thread Tuguoyi
@sn_opts is initialized at the beginning, so it should be deleted
after jumping to the lable 'fail_getopt'

Signed-off-by: Guoyi Tu 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 2103507..229cdf9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2719,7 +2719,6 @@ out:
 qemu_progress_end();
 qemu_opts_del(opts);
 qemu_opts_free(create_opts);
-qemu_opts_del(sn_opts);
 qobject_unref(open_opts);
 blk_unref(s.target);
 if (s.src) {
@@ -2731,6 +2730,7 @@ out:
 g_free(s.src_sectors);
 g_free(s.src_alignment);
 fail_getopt:
+qemu_opts_del(sn_opts);
 g_free(options);
 
 return !!ret;
-- 
2.7.4


--
Best regards,
Guoyi



Re: [PATCH v2 0/2] Assorted fixes to tests that were broken by recent scsi changes

2020-11-02 Thread Paolo Bonzini
On 01/11/20 17:15, Maxim Levitsky wrote:
> While most of the patches in V1 of this series are already merged upstream,
> the patch that fixes iotest 240 was broken on s390 and was not accepted.
> 
> This is   an updated version of this patch, based on Paulo's suggestion,
> that hopefully makes this iotest work on both x86 and s390.
> 
> Best regards,
>   Maxim Levitsky
> 
> Maxim Levitsky (2):
>   iotests: add filter_qmp_virtio_scsi function
>   iotests: rewrite iotest 240 in python
> 
>  tests/qemu-iotests/240| 228 +++---
>  tests/qemu-iotests/240.out|  76 +++-
>  tests/qemu-iotests/iotests.py |  10 ++
>  3 files changed, 153 insertions(+), 161 deletions(-)
> 

Reviewed-by: Paolo Bonzini