All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
@ 2018-02-08 19:08 Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 01/26] Revert "vhost: add traces for memory listeners" Michael S. Tsirkin
                   ` (26 more replies)
  0 siblings, 27 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:

  Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into staging (2018-02-08 14:31:51 +0000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:

  virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42 +0200)

----------------------------------------------------------------
virtio,vhost,pci,pc: features, fixes and cleanups

- a new vhost crypto device
- new stats in virtio balloon
- virtio eventfd rework for boot speedup
- vhost memory rework for boot speedup
- fixes and cleanups all over the place

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Changpeng Liu (1):
      virtio-blk: enable multiple vectors when using multiple I/O queues

Dr. David Alan Gilbert (7):
      vhost: Build temporary section list and deref after commit
      vhost: Simplify ring verification checks
      vhost: Merge sections added to temporary list
      vhost: Regenerate region list from changed sections list
      vhost: Clean out old vhost_set_memory and friends
      vhost: Merge and delete unused callbacks
      vhost: Move log_dirty check

Gal Hammer (2):
      virtio: remove event notifier cleanup call on de-assign
      virtio: improve virtio devices initialization time

Gonglei (4):
      cryptodev: add vhost-user as a new cryptodev backend
      cryptodev: add vhost support
      cryptodev-vhost-user: add crypto session handler
      cryptodev-vhost-user: set the key length

Igor Mammedov (1):
      tests: acpi: fix FADT not being compared to reference table

Laszlo Ersek (1):
      pci-bridge/i82801b11: clear bridge registers on platform reset

Marcel Apfelbaum (1):
      hw/pci-bridge: fix pcie root port's IO hints capability

Michael S. Tsirkin (3):
      Revert "vhost: add traces for memory listeners"
      lpc: drop pcie host dependency
      acpi-test: update FADT

Peter Xu (1):
      pci/bus: let it has higher migration priority

Tiwei Bie (1):
      virtio-balloon: unref the memory region before continuing

Tomáš Golembiovský (1):
      virtio-balloon: include statistics of disk/file caches

Yongji Xie (2):
      libvhost-user: Fix resource leak
      libvhost-user: Support across-memory-boundary access

Yoni Bettan (1):
      pci: removed the is_express field since a uniform interface was inserted

 docs/interop/vhost-user.txt                     |  26 ++
 docs/pcie_pci_bridge.txt                        |   2 +-
 configure                                       |  15 +
 contrib/libvhost-user/libvhost-user.h           |   3 +-
 include/hw/compat.h                             |   8 +
 include/hw/pci/pci.h                            |   3 -
 include/hw/pci/pci_bridge.h                     |   4 +-
 include/hw/virtio/vhost-backend.h               |   8 +
 include/hw/virtio/vhost.h                       |   5 +-
 include/hw/virtio/virtio-bus.h                  |   2 +
 include/hw/virtio/virtio-crypto.h               |   1 +
 include/migration/vmstate.h                     |   1 +
 include/standard-headers/linux/virtio_balloon.h |   3 +-
 include/sysemu/cryptodev-vhost-user.h           |  47 +++
 include/sysemu/cryptodev-vhost.h                | 154 ++++++++
 include/sysemu/cryptodev.h                      |   8 +
 backends/cryptodev-builtin.c                    |   1 +
 backends/cryptodev-vhost-user.c                 | 379 ++++++++++++++++++
 backends/cryptodev-vhost.c                      | 347 ++++++++++++++++
 contrib/libvhost-user/libvhost-user.c           | 147 ++++++-
 hw/block/dataplane/virtio-blk.c                 |   2 +
 hw/block/nvme.c                                 |   1 -
 hw/isa/lpc_ich9.c                               |   1 -
 hw/net/e1000e.c                                 |   1 -
 hw/pci-bridge/gen_pcie_root_port.c              |   1 +
 hw/pci-bridge/i82801b11.c                       |   2 +
 hw/pci-bridge/ioh3420.c                         |   1 +
 hw/pci-bridge/pci_bridge_dev.c                  |   1 +
 hw/pci-bridge/pcie_pci_bridge.c                 |   3 +-
 hw/pci-bridge/pcie_root_port.c                  |   1 -
 hw/pci-bridge/xio3130_downstream.c              |   2 +-
 hw/pci-bridge/xio3130_upstream.c                |   2 +-
 hw/pci-host/xilinx-pcie.c                       |   1 -
 hw/pci/pci.c                                    |   8 +-
 hw/pci/pci_bridge.c                             |  24 +-
 hw/scsi/megasas.c                               |   4 -
 hw/scsi/virtio-scsi-dataplane.c                 |   2 +
 hw/usb/hcd-xhci.c                               |   9 +-
 hw/vfio/pci.c                                   |   5 +-
 hw/virtio/vhost-user.c                          | 104 +++++
 hw/virtio/vhost.c                               | 504 ++++++++----------------
 hw/virtio/virtio-balloon.c                      |   2 +
 hw/virtio/virtio-bus.c                          |  14 +-
 hw/virtio/virtio-crypto.c                       |  70 ++++
 hw/virtio/virtio-pci.c                          |  14 +-
 hw/virtio/virtio.c                              |  22 +-
 hw/xen/xen_pt.c                                 |   9 +-
 tests/bios-tables-test.c                        |  35 +-
 vl.c                                            |   6 +
 backends/Makefile.objs                          |   6 +
 hw/virtio/Makefile.objs                         |   2 +-
 hw/virtio/trace-events                          |  12 +-
 qemu-options.hx                                 |  21 +
 tests/acpi-test-data/pc/FACP                    | Bin 116 -> 116 bytes
 tests/acpi-test-data/q35/FACP                   | Bin 116 -> 244 bytes
 55 files changed, 1662 insertions(+), 394 deletions(-)
 create mode 100644 include/sysemu/cryptodev-vhost-user.h
 create mode 100644 include/sysemu/cryptodev-vhost.h
 create mode 100644 backends/cryptodev-vhost-user.c
 create mode 100644 backends/cryptodev-vhost.c

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 01/26] Revert "vhost: add traces for memory listeners"
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
@ 2018-02-08 19:08 ` Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 02/26] virtio: remove event notifier cleanup call on de-assign Michael S. Tsirkin
                   ` (25 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

This reverts commit 0750b060216de69ed1f14bc08181bf4ad27fc622.

Follow up patches are reworking the memory listeners, the new mechanism
will add its own set of traces.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c      | 7 -------
 hw/virtio/trace-events | 6 ------
 2 files changed, 13 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 338e439..23b9e17 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -27,7 +27,6 @@
 #include "hw/virtio/virtio-access.h"
 #include "migration/blocker.h"
 #include "sysemu/dma.h"
-#include "trace.h"
 
 /* enabled until disconnected backend stabilizes */
 #define _VHOST_DEBUG 1
@@ -694,7 +693,6 @@ static void vhost_region_add(MemoryListener *listener,
         return;
     }
 
-    trace_vhost_region_add(dev, section->mr->name ?: NULL);
     ++dev->n_mem_sections;
     dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
                                 dev->n_mem_sections);
@@ -714,7 +712,6 @@ static void vhost_region_del(MemoryListener *listener,
         return;
     }
 
-    trace_vhost_region_del(dev, section->mr->name ?: NULL);
     vhost_set_memory(listener, section, false);
     memory_region_unref(section->mr);
     for (i = 0; i < dev->n_mem_sections; ++i) {
@@ -752,8 +749,6 @@ static void vhost_iommu_region_add(MemoryListener *listener,
         return;
     }
 
-    trace_vhost_iommu_region_add(dev, section->mr->name ?: NULL);
-
     iommu = g_malloc0(sizeof(*iommu));
     end = int128_add(int128_make64(section->offset_within_region),
                      section->size);
@@ -782,8 +777,6 @@ static void vhost_iommu_region_del(MemoryListener *listener,
         return;
     }
 
-    trace_vhost_iommu_region_del(dev, section->mr->name ?: NULL);
-
     QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) {
         if (iommu->mr == section->mr &&
             iommu->n.start == section->offset_within_region) {
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 2b8f81e..775461a 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -25,9 +25,3 @@ virtio_balloon_handle_output(const char *name, uint64_t gpa) "section name: %s g
 virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: %d actual: %d"
 virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d oldactual: %d"
 virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: 0x%"PRIx64" num_pages: %d"
-
-# hw/virtio/vhost.c
-vhost_region_add(void *p, const char *mr) "dev %p mr %s"
-vhost_region_del(void *p, const char *mr) "dev %p mr %s"
-vhost_iommu_region_add(void *p, const char *mr) "dev %p mr %s"
-vhost_iommu_region_del(void *p, const char *mr) "dev %p mr %s"
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 02/26] virtio: remove event notifier cleanup call on de-assign
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 01/26] Revert "vhost: add traces for memory listeners" Michael S. Tsirkin
@ 2018-02-08 19:08 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 04/26] vhost: Build temporary section list and deref after commit Michael S. Tsirkin
                   ` (24 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Gal Hammer, Greg Kurz, Stefan Hajnoczi,
	Kevin Wolf, Max Reitz, Paolo Bonzini, Fam Zheng, qemu-block

From: Gal Hammer <ghammer@redhat.com>

The virtio_bus_set_host_notifier function no longer calls
event_notifier_cleanup when a event notifier is removed.

The commit updates the code to match the new behavior and calls
virtio_bus_cleanup_host_notifier after the notifier was de-assign
and no longer in use.

This change is a preparation to allow executing the
virtio_bus_set_host_notifier function in a memory region
transaction.

Signed-off-by: Gal Hammer <ghammer@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-bus.h  |  2 ++
 hw/block/dataplane/virtio-blk.c |  2 ++
 hw/scsi/virtio-scsi-dataplane.c |  2 ++
 hw/virtio/vhost.c               |  2 ++
 hw/virtio/virtio-bus.c          | 14 ++++++++++----
 hw/virtio/virtio.c              |  2 ++
 6 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index a63c1d2..ced3d2d 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -148,5 +148,7 @@ int virtio_bus_grab_ioeventfd(VirtioBusState *bus);
 void virtio_bus_release_ioeventfd(VirtioBusState *bus);
 /* Switch from/to the generic ioeventfd handler */
 int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign);
+/* Tell the bus that the ioeventfd handler is no longer required. */
+void virtio_bus_cleanup_host_notifier(VirtioBusState *bus, int n);
 
 #endif /* VIRTIO_BUS_H */
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index f6fc639..2cb9909 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -192,6 +192,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
             fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
             while (i--) {
                 virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+                virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
             }
             goto fail_guest_notifiers;
         }
@@ -267,6 +268,7 @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
 
     for (i = 0; i < nvqs; i++) {
         virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
     }
 
     /* Clean up guest notifier (irq) */
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index add4b3f..1c33322 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -175,6 +175,7 @@ fail_vrings:
     aio_context_release(s->ctx);
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
         virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
     }
     k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
 fail_guest_notifiers:
@@ -213,6 +214,7 @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
 
     for (i = 0; i < vs->conf.num_queues + 2; i++) {
         virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
     }
 
     /* Clean up guest notifier (irq) */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 23b9e17..56885aa 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1418,6 +1418,7 @@ fail_vq:
             error_report("vhost VQ %d notifier cleanup error: %d", i, -r);
         }
         assert (e >= 0);
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i);
     }
     virtio_device_release_ioeventfd(vdev);
 fail:
@@ -1441,6 +1442,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
             error_report("vhost VQ %d notifier cleanup failed: %d", i, -r);
         }
         assert (r >= 0);
+        virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i);
     }
     virtio_device_release_ioeventfd(vdev);
 }
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 3042232..f9bc9ea 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -283,20 +283,26 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
         r = k->ioeventfd_assign(proxy, notifier, n, true);
         if (r < 0) {
             error_report("%s: unable to assign ioeventfd: %d", __func__, r);
-            goto cleanup_event_notifier;
+            virtio_bus_cleanup_host_notifier(bus, n);
         }
-        return 0;
     } else {
         k->ioeventfd_assign(proxy, notifier, n, false);
     }
 
-cleanup_event_notifier:
+    return r;
+}
+
+void virtio_bus_cleanup_host_notifier(VirtioBusState *bus, int n)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+
     /* Test and clear notifier after disabling event,
      * in case poll callback didn't have time to run.
      */
     virtio_queue_host_notifier_read(notifier);
     event_notifier_cleanup(notifier);
-    return r;
 }
 
 static char *virtio_bus_get_dev_path(DeviceState *dev)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d6002ee..3667cd6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2608,6 +2608,7 @@ assign_error:
         event_notifier_set_handler(&vq->host_notifier, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
+        virtio_bus_cleanup_host_notifier(qbus, n);
     }
     return err;
 }
@@ -2634,6 +2635,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
         event_notifier_set_handler(&vq->host_notifier, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
+        virtio_bus_cleanup_host_notifier(qbus, n);
     }
 }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 04/26] vhost: Build temporary section list and deref after commit
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 01/26] Revert "vhost: add traces for memory listeners" Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 02/26] virtio: remove event notifier cleanup call on de-assign Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 05/26] vhost: Simplify ring verification checks Michael S. Tsirkin
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert, Igor Mammedov

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Igor spotted that there's a race, where a region that's unref'd
in a _del callback might be free'd before the set_mem_table call in
the _commit callback, and thus the vhost might end up using free memory.

Fix this by building a complete temporary sections list, ref'ing every
section (during add and nop) and then unref'ing the whole list right
at the end of commit.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/vhost.h |  2 ++
 hw/virtio/vhost.c         | 73 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 1dc2d73..09854b6 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -60,6 +60,8 @@ struct vhost_dev {
     struct vhost_memory *mem;
     int n_mem_sections;
     MemoryRegionSection *mem_sections;
+    int n_tmp_sections;
+    MemoryRegionSection *tmp_sections;
     struct vhost_virtqueue *vqs;
     int nvqs;
     /* the first virtqueue which would be used by this vhost dev */
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 56885aa..92c9500 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -627,6 +627,8 @@ static void vhost_begin(MemoryListener *listener)
                                          memory_listener);
     dev->mem_changed_end_addr = 0;
     dev->mem_changed_start_addr = -1;
+    dev->tmp_sections = NULL;
+    dev->n_tmp_sections = 0;
 }
 
 static void vhost_commit(MemoryListener *listener)
@@ -635,17 +637,25 @@ static void vhost_commit(MemoryListener *listener)
                                          memory_listener);
     hwaddr start_addr = 0;
     ram_addr_t size = 0;
+    MemoryRegionSection *old_sections;
+    int n_old_sections;
+
     uint64_t log_size;
     int r;
 
+    old_sections = dev->mem_sections;
+    n_old_sections = dev->n_mem_sections;
+    dev->mem_sections = dev->tmp_sections;
+    dev->n_mem_sections = dev->n_tmp_sections;
+
     if (!dev->memory_changed) {
-        return;
+        goto out;
     }
     if (!dev->started) {
-        return;
+        goto out;
     }
     if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) {
-        return;
+        goto out;
     }
 
     if (dev->started) {
@@ -662,7 +672,7 @@ static void vhost_commit(MemoryListener *listener)
             VHOST_OPS_DEBUG("vhost_set_mem_table failed");
         }
         dev->memory_changed = false;
-        return;
+        goto out;
     }
     log_size = vhost_get_log_size(dev);
     /* We allocate an extra 4K bytes to log,
@@ -681,6 +691,27 @@ static void vhost_commit(MemoryListener *listener)
         vhost_dev_log_resize(dev, log_size);
     }
     dev->memory_changed = false;
+
+out:
+    /* Deref the old list of sections, this must happen _after_ the
+     * vhost_set_mem_table to ensure the client isn't still using the
+     * section we're about to unref.
+     */
+    while (n_old_sections--) {
+        memory_region_unref(old_sections[n_old_sections].mr);
+    }
+    g_free(old_sections);
+    return;
+}
+
+static void vhost_add_section(struct vhost_dev *dev,
+                              MemoryRegionSection *section)
+{
+    ++dev->n_tmp_sections;
+    dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
+                                dev->n_tmp_sections);
+    dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
+    memory_region_ref(section->mr);
 }
 
 static void vhost_region_add(MemoryListener *listener,
@@ -693,36 +724,31 @@ static void vhost_region_add(MemoryListener *listener,
         return;
     }
 
-    ++dev->n_mem_sections;
-    dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
-                                dev->n_mem_sections);
-    dev->mem_sections[dev->n_mem_sections - 1] = *section;
-    memory_region_ref(section->mr);
+    vhost_add_section(dev, section);
     vhost_set_memory(listener, section, true);
 }
 
-static void vhost_region_del(MemoryListener *listener,
+static void vhost_region_nop(MemoryListener *listener,
                              MemoryRegionSection *section)
 {
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
-    int i;
 
     if (!vhost_section(section)) {
         return;
     }
 
-    vhost_set_memory(listener, section, false);
-    memory_region_unref(section->mr);
-    for (i = 0; i < dev->n_mem_sections; ++i) {
-        if (dev->mem_sections[i].offset_within_address_space
-            == section->offset_within_address_space) {
-            --dev->n_mem_sections;
-            memmove(&dev->mem_sections[i], &dev->mem_sections[i+1],
-                    (dev->n_mem_sections - i) * sizeof(*dev->mem_sections));
-            break;
-        }
+    vhost_add_section(dev, section);
+}
+
+static void vhost_region_del(MemoryListener *listener,
+                             MemoryRegionSection *section)
+{
+    if (!vhost_section(section)) {
+        return;
     }
+
+    vhost_set_memory(listener, section, false);
 }
 
 static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
@@ -789,11 +815,6 @@ static void vhost_iommu_region_del(MemoryListener *listener,
     }
 }
 
-static void vhost_region_nop(MemoryListener *listener,
-                             MemoryRegionSection *section)
-{
-}
-
 static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
                                     struct vhost_virtqueue *vq,
                                     unsigned idx, bool enable_log)
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 05/26] vhost: Simplify ring verification checks
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 04/26] vhost: Build temporary section list and deref after commit Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 07/26] vhost: Regenerate region list from changed sections list Michael S. Tsirkin
                   ` (22 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert, Igor Mammedov

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

vhost_verify_ring_mappings() were used to verify that
rings are still accessible and related memory hasn't
been moved after flatview is updated.

It was doing checks by mapping ring's GPA+len and
checking that HVA hadn't changed with new memory map.
To avoid maybe expensive mapping call, we were
identifying address range that changed and were doing
mapping only if ring was in changed range.

However it's not neccessary to perform ring's GPA
mapping as we already have its current HVA and all
we need is to verify that ring's GPA translates to
the same HVA in updated flatview.

This will allow the following patches to simplify the range
comparison that was previously needed to avoid expensive
verify_ring_mapping calls.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
with modifications by:
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c | 79 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 37 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 92c9500..91cab51 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -455,35 +455,37 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
     }
 }
 
-static int vhost_verify_ring_part_mapping(struct vhost_dev *dev,
-                                          void *part,
-                                          uint64_t part_addr,
-                                          uint64_t part_size,
-                                          uint64_t start_addr,
-                                          uint64_t size)
-{
-    hwaddr l;
-    void *p;
-    int r = 0;
-
-    if (!ranges_overlap(start_addr, size, part_addr, part_size)) {
+static int vhost_verify_ring_part_mapping(void *ring_hva,
+                                          uint64_t ring_gpa,
+                                          uint64_t ring_size,
+                                          void *reg_hva,
+                                          uint64_t reg_gpa,
+                                          uint64_t reg_size)
+{
+    uint64_t hva_ring_offset;
+    uint64_t ring_last = range_get_last(ring_gpa, ring_size);
+    uint64_t reg_last = range_get_last(reg_gpa, reg_size);
+
+    if (ring_last < reg_gpa || ring_gpa > reg_last) {
         return 0;
     }
-    l = part_size;
-    p = vhost_memory_map(dev, part_addr, &l, 1);
-    if (!p || l != part_size) {
-        r = -ENOMEM;
+    /* check that whole ring's is mapped */
+    if (ring_last > reg_last) {
+        return -ENOMEM;
     }
-    if (p != part) {
-        r = -EBUSY;
+    /* check that ring's MemoryRegion wasn't replaced */
+    hva_ring_offset = ring_gpa - reg_gpa;
+    if (ring_hva != reg_hva + hva_ring_offset) {
+        return -EBUSY;
     }
-    vhost_memory_unmap(dev, p, l, 0, 0);
-    return r;
+
+    return 0;
 }
 
 static int vhost_verify_ring_mappings(struct vhost_dev *dev,
-                                      uint64_t start_addr,
-                                      uint64_t size)
+                                      void *reg_hva,
+                                      uint64_t reg_gpa,
+                                      uint64_t reg_size)
 {
     int i, j;
     int r = 0;
@@ -497,22 +499,25 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
         struct vhost_virtqueue *vq = dev->vqs + i;
 
         j = 0;
-        r = vhost_verify_ring_part_mapping(dev, vq->desc, vq->desc_phys,
-                                           vq->desc_size, start_addr, size);
+        r = vhost_verify_ring_part_mapping(
+                vq->desc, vq->desc_phys, vq->desc_size,
+                reg_hva, reg_gpa, reg_size);
         if (r) {
             break;
         }
 
         j++;
-        r = vhost_verify_ring_part_mapping(dev, vq->avail, vq->avail_phys,
-                                           vq->avail_size, start_addr, size);
+        r = vhost_verify_ring_part_mapping(
+                vq->desc, vq->desc_phys, vq->desc_size,
+                reg_hva, reg_gpa, reg_size);
         if (r) {
             break;
         }
 
         j++;
-        r = vhost_verify_ring_part_mapping(dev, vq->used, vq->used_phys,
-                                           vq->used_size, start_addr, size);
+        r = vhost_verify_ring_part_mapping(
+                vq->desc, vq->desc_phys, vq->desc_size,
+                reg_hva, reg_gpa, reg_size);
         if (r) {
             break;
         }
@@ -635,13 +640,11 @@ static void vhost_commit(MemoryListener *listener)
 {
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
-    hwaddr start_addr = 0;
-    ram_addr_t size = 0;
     MemoryRegionSection *old_sections;
     int n_old_sections;
-
     uint64_t log_size;
     int r;
+    int i;
 
     old_sections = dev->mem_sections;
     n_old_sections = dev->n_mem_sections;
@@ -658,12 +661,14 @@ static void vhost_commit(MemoryListener *listener)
         goto out;
     }
 
-    if (dev->started) {
-        start_addr = dev->mem_changed_start_addr;
-        size = dev->mem_changed_end_addr - dev->mem_changed_start_addr + 1;
-
-        r = vhost_verify_ring_mappings(dev, start_addr, size);
-        assert(r >= 0);
+    for (i = 0; i < dev->mem->nregions; i++) {
+        if (vhost_verify_ring_mappings(dev,
+                       (void *)(uintptr_t)dev->mem->regions[i].userspace_addr,
+                       dev->mem->regions[i].guest_phys_addr,
+                       dev->mem->regions[i].memory_size)) {
+            error_report("Verify ring failure on region %d", i);
+            abort();
+        }
     }
 
     if (!dev->log_enabled) {
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 06/26] vhost: Merge sections added to temporary list
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (4 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 07/26] vhost: Regenerate region list from changed sections list Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 09/26] vhost: Merge and delete unused callbacks Michael S. Tsirkin
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert, Igor Mammedov

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

As sections are reported by the listener to the _nop and _add
methods, add them to the temporary section list but now merge them
with the previous section if the new one abuts and the backend allows.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c      | 76 ++++++++++++++++++++++++++++++++++++++++++++------
 hw/virtio/trace-events |  4 +++
 2 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 91cab51..351c691 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -709,14 +709,71 @@ out:
     return;
 }
 
-static void vhost_add_section(struct vhost_dev *dev,
-                              MemoryRegionSection *section)
+/* Adds the section data to the tmp_section structure.
+ * It relies on the listener calling us in memory address order
+ * and for each region (via the _add and _nop methods) to
+ * join neighbours.
+ */
+static void vhost_region_add_section(struct vhost_dev *dev,
+                                     MemoryRegionSection *section)
 {
-    ++dev->n_tmp_sections;
-    dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
-                                dev->n_tmp_sections);
-    dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
-    memory_region_ref(section->mr);
+    bool need_add = true;
+    uint64_t mrs_size = int128_get64(section->size);
+    uint64_t mrs_gpa = section->offset_within_address_space;
+    uintptr_t mrs_host = (uintptr_t)memory_region_get_ram_ptr(section->mr) +
+                         section->offset_within_region;
+
+    trace_vhost_region_add_section(section->mr->name, mrs_gpa, mrs_size,
+                                   mrs_host);
+
+    bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
+                        ~(1 << DIRTY_MEMORY_MIGRATION);
+    if (log_dirty) {
+        return;
+    }
+
+    if (dev->n_tmp_sections) {
+        /* Since we already have at least one section, lets see if
+         * this extends it; since we're scanning in order, we only
+         * have to look at the last one, and the FlatView that calls
+         * us shouldn't have overlaps.
+         */
+        MemoryRegionSection *prev_sec = dev->tmp_sections +
+                                               (dev->n_tmp_sections - 1);
+        uint64_t prev_gpa_start = prev_sec->offset_within_address_space;
+        uint64_t prev_size = int128_get64(prev_sec->size);
+        uint64_t prev_gpa_end   = range_get_last(prev_gpa_start, prev_size);
+        uint64_t prev_host_start =
+                        (uintptr_t)memory_region_get_ram_ptr(prev_sec->mr) +
+                        prev_sec->offset_within_region;
+        uint64_t prev_host_end   = range_get_last(prev_host_start, prev_size);
+
+        if (prev_gpa_end + 1 == mrs_gpa &&
+            prev_host_end + 1 == mrs_host &&
+            section->mr == prev_sec->mr &&
+            (!dev->vhost_ops->vhost_backend_can_merge ||
+                dev->vhost_ops->vhost_backend_can_merge(dev,
+                    mrs_host, mrs_size,
+                    prev_host_start, prev_size))) {
+            /* The two sections abut */
+            need_add = false;
+            prev_sec->size = int128_add(prev_sec->size, section->size);
+            trace_vhost_region_add_section_abut(section->mr->name,
+                                                mrs_size + prev_size);
+        }
+    }
+
+    if (need_add) {
+        ++dev->n_tmp_sections;
+        dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
+                                    dev->n_tmp_sections);
+        dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
+        /* The flatview isn't stable and we don't use it, making it NULL
+         * means we can memcmp the list.
+         */
+        dev->tmp_sections[dev->n_tmp_sections - 1].fv = NULL;
+        memory_region_ref(section->mr);
+    }
 }
 
 static void vhost_region_add(MemoryListener *listener,
@@ -728,11 +785,12 @@ static void vhost_region_add(MemoryListener *listener,
     if (!vhost_section(section)) {
         return;
     }
+    vhost_region_add_section(dev, section);
 
-    vhost_add_section(dev, section);
     vhost_set_memory(listener, section, true);
 }
 
+/* Called on regions that have not changed */
 static void vhost_region_nop(MemoryListener *listener,
                              MemoryRegionSection *section)
 {
@@ -743,7 +801,7 @@ static void vhost_region_nop(MemoryListener *listener,
         return;
     }
 
-    vhost_add_section(dev, section);
+    vhost_region_add_section(dev, section);
 }
 
 static void vhost_region_del(MemoryListener *listener,
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 775461a..ab9da06 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -1,5 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# hw/virtio/vhost.c
+vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
+vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 0x%"PRIx64
+
 # hw/virtio/virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
 virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 07/26] vhost: Regenerate region list from changed sections list
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (3 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 05/26] vhost: Simplify ring verification checks Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 06/26] vhost: Merge sections added to temporary list Michael S. Tsirkin
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert, Igor Mammedov

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Compare the sections list that's just been generated, and if it's
different from the old one regenerate the region list.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/virtio/vhost.c      | 39 +++++++++++++++++++++++++++++++++++----
 hw/virtio/trace-events |  1 +
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 351c691..f3badc4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -643,21 +643,52 @@ static void vhost_commit(MemoryListener *listener)
     MemoryRegionSection *old_sections;
     int n_old_sections;
     uint64_t log_size;
+    size_t regions_size;
     int r;
     int i;
+    bool changed = false;
 
+    /* Note we can be called before the device is started, but then
+     * starting the device calls set_mem_table, so we need to have
+     * built the data structures.
+     */
     old_sections = dev->mem_sections;
     n_old_sections = dev->n_mem_sections;
     dev->mem_sections = dev->tmp_sections;
     dev->n_mem_sections = dev->n_tmp_sections;
 
-    if (!dev->memory_changed) {
-        goto out;
+    if (dev->n_mem_sections != n_old_sections) {
+        changed = true;
+    } else {
+        /* Same size, lets check the contents */
+        changed = n_old_sections && memcmp(dev->mem_sections, old_sections,
+                         n_old_sections * sizeof(old_sections[0])) != 0;
     }
-    if (!dev->started) {
+
+    trace_vhost_commit(dev->started, changed);
+    if (!changed) {
         goto out;
     }
-    if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) {
+
+    /* Rebuild the regions list from the new sections list */
+    regions_size = offsetof(struct vhost_memory, regions) +
+                       dev->n_mem_sections * sizeof dev->mem->regions[0];
+    dev->mem = g_realloc(dev->mem, regions_size);
+    dev->mem->nregions = dev->n_mem_sections;
+    used_memslots = dev->mem->nregions;
+    for (i = 0; i < dev->n_mem_sections; i++) {
+        struct vhost_memory_region *cur_vmr = dev->mem->regions + i;
+        struct MemoryRegionSection *mrs = dev->mem_sections + i;
+
+        cur_vmr->guest_phys_addr = mrs->offset_within_address_space;
+        cur_vmr->memory_size     = int128_get64(mrs->size);
+        cur_vmr->userspace_addr  =
+            (uintptr_t)memory_region_get_ram_ptr(mrs->mr) +
+            mrs->offset_within_region;
+        cur_vmr->flags_padding   = 0;
+    }
+
+    if (!dev->started) {
         goto out;
     }
 
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index ab9da06..606e4fc 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -1,6 +1,7 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # hw/virtio/vhost.c
+vhost_commit(bool started, bool changed) "Started: %d Changed: %d"
 vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
 vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 0x%"PRIx64
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 09/26] vhost: Merge and delete unused callbacks
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (5 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 06/26] vhost: Merge sections added to temporary list Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 08/26] vhost: Clean out old vhost_set_memory and friends Michael S. Tsirkin
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert, Igor Mammedov

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Now that the olf vhost_set_memory code is gone, the _nop and _add
callbacks are identical and can be merged.  The _del callback is
no longer needed.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c7814d1..326f168 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -560,8 +560,9 @@ static void vhost_region_add_section(struct vhost_dev *dev,
     }
 }
 
-static void vhost_region_add(MemoryListener *listener,
-                             MemoryRegionSection *section)
+/* Used for both add and nop callbacks */
+static void vhost_region_addnop(MemoryListener *listener,
+                                MemoryRegionSection *section)
 {
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
@@ -572,29 +573,6 @@ static void vhost_region_add(MemoryListener *listener,
     vhost_region_add_section(dev, section);
 }
 
-/* Called on regions that have not changed */
-static void vhost_region_nop(MemoryListener *listener,
-                             MemoryRegionSection *section)
-{
-    struct vhost_dev *dev = container_of(listener, struct vhost_dev,
-                                         memory_listener);
-
-    if (!vhost_section(section)) {
-        return;
-    }
-
-    vhost_region_add_section(dev, section);
-}
-
-static void vhost_region_del(MemoryListener *listener,
-                             MemoryRegionSection *section)
-{
-    if (!vhost_section(section)) {
-        return;
-    }
-
-}
-
 static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
 {
     struct vhost_iommu *iommu = container_of(n, struct vhost_iommu, n);
@@ -1163,9 +1141,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     hdev->memory_listener = (MemoryListener) {
         .begin = vhost_begin,
         .commit = vhost_commit,
-        .region_add = vhost_region_add,
-        .region_del = vhost_region_del,
-        .region_nop = vhost_region_nop,
+        .region_add = vhost_region_addnop,
+        .region_nop = vhost_region_addnop,
         .log_start = vhost_log_start,
         .log_stop = vhost_log_stop,
         .log_sync = vhost_log_sync,
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 08/26] vhost: Clean out old vhost_set_memory and friends
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (6 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 09/26] vhost: Merge and delete unused callbacks Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 10/26] vhost: Move log_dirty check Michael S. Tsirkin
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Remove the old update mechanism, vhost_set_memory, and the functions
and flags it used.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/vhost.h |   3 -
 hw/virtio/vhost.c         | 251 ----------------------------------------------
 2 files changed, 254 deletions(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 09854b6..a7f449f 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -75,9 +75,6 @@ struct vhost_dev {
     bool log_enabled;
     uint64_t log_size;
     Error *migration_blocker;
-    bool memory_changed;
-    hwaddr mem_changed_start_addr;
-    hwaddr mem_changed_end_addr;
     const VhostOps *vhost_ops;
     void *opaque;
     struct vhost_log *log;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f3badc4..c7814d1 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -155,160 +155,6 @@ static void vhost_log_sync_range(struct vhost_dev *dev,
     }
 }
 
-/* Assign/unassign. Keep an unsorted array of non-overlapping
- * memory regions in dev->mem. */
-static void vhost_dev_unassign_memory(struct vhost_dev *dev,
-                                      uint64_t start_addr,
-                                      uint64_t size)
-{
-    int from, to, n = dev->mem->nregions;
-    /* Track overlapping/split regions for sanity checking. */
-    int overlap_start = 0, overlap_end = 0, overlap_middle = 0, split = 0;
-
-    for (from = 0, to = 0; from < n; ++from, ++to) {
-        struct vhost_memory_region *reg = dev->mem->regions + to;
-        uint64_t reglast;
-        uint64_t memlast;
-        uint64_t change;
-
-        /* clone old region */
-        if (to != from) {
-            memcpy(reg, dev->mem->regions + from, sizeof *reg);
-        }
-
-        /* No overlap is simple */
-        if (!ranges_overlap(reg->guest_phys_addr, reg->memory_size,
-                            start_addr, size)) {
-            continue;
-        }
-
-        /* Split only happens if supplied region
-         * is in the middle of an existing one. Thus it can not
-         * overlap with any other existing region. */
-        assert(!split);
-
-        reglast = range_get_last(reg->guest_phys_addr, reg->memory_size);
-        memlast = range_get_last(start_addr, size);
-
-        /* Remove whole region */
-        if (start_addr <= reg->guest_phys_addr && memlast >= reglast) {
-            --dev->mem->nregions;
-            --to;
-            ++overlap_middle;
-            continue;
-        }
-
-        /* Shrink region */
-        if (memlast >= reglast) {
-            reg->memory_size = start_addr - reg->guest_phys_addr;
-            assert(reg->memory_size);
-            assert(!overlap_end);
-            ++overlap_end;
-            continue;
-        }
-
-        /* Shift region */
-        if (start_addr <= reg->guest_phys_addr) {
-            change = memlast + 1 - reg->guest_phys_addr;
-            reg->memory_size -= change;
-            reg->guest_phys_addr += change;
-            reg->userspace_addr += change;
-            assert(reg->memory_size);
-            assert(!overlap_start);
-            ++overlap_start;
-            continue;
-        }
-
-        /* This only happens if supplied region
-         * is in the middle of an existing one. Thus it can not
-         * overlap with any other existing region. */
-        assert(!overlap_start);
-        assert(!overlap_end);
-        assert(!overlap_middle);
-        /* Split region: shrink first part, shift second part. */
-        memcpy(dev->mem->regions + n, reg, sizeof *reg);
-        reg->memory_size = start_addr - reg->guest_phys_addr;
-        assert(reg->memory_size);
-        change = memlast + 1 - reg->guest_phys_addr;
-        reg = dev->mem->regions + n;
-        reg->memory_size -= change;
-        assert(reg->memory_size);
-        reg->guest_phys_addr += change;
-        reg->userspace_addr += change;
-        /* Never add more than 1 region */
-        assert(dev->mem->nregions == n);
-        ++dev->mem->nregions;
-        ++split;
-    }
-}
-
-/* Called after unassign, so no regions overlap the given range. */
-static void vhost_dev_assign_memory(struct vhost_dev *dev,
-                                    uint64_t start_addr,
-                                    uint64_t size,
-                                    uint64_t uaddr)
-{
-    int from, to;
-    struct vhost_memory_region *merged = NULL;
-    for (from = 0, to = 0; from < dev->mem->nregions; ++from, ++to) {
-        struct vhost_memory_region *reg = dev->mem->regions + to;
-        uint64_t prlast, urlast;
-        uint64_t pmlast, umlast;
-        uint64_t s, e, u;
-
-        /* clone old region */
-        if (to != from) {
-            memcpy(reg, dev->mem->regions + from, sizeof *reg);
-        }
-        prlast = range_get_last(reg->guest_phys_addr, reg->memory_size);
-        pmlast = range_get_last(start_addr, size);
-        urlast = range_get_last(reg->userspace_addr, reg->memory_size);
-        umlast = range_get_last(uaddr, size);
-
-        /* check for overlapping regions: should never happen. */
-        assert(prlast < start_addr || pmlast < reg->guest_phys_addr);
-        /* Not an adjacent or overlapping region - do not merge. */
-        if ((prlast + 1 != start_addr || urlast + 1 != uaddr) &&
-            (pmlast + 1 != reg->guest_phys_addr ||
-             umlast + 1 != reg->userspace_addr)) {
-            continue;
-        }
-
-        if (dev->vhost_ops->vhost_backend_can_merge &&
-            !dev->vhost_ops->vhost_backend_can_merge(dev, uaddr, size,
-                                                     reg->userspace_addr,
-                                                     reg->memory_size)) {
-            continue;
-        }
-
-        if (merged) {
-            --to;
-            assert(to >= 0);
-        } else {
-            merged = reg;
-        }
-        u = MIN(uaddr, reg->userspace_addr);
-        s = MIN(start_addr, reg->guest_phys_addr);
-        e = MAX(pmlast, prlast);
-        uaddr = merged->userspace_addr = u;
-        start_addr = merged->guest_phys_addr = s;
-        size = merged->memory_size = e - s + 1;
-        assert(merged->memory_size);
-    }
-
-    if (!merged) {
-        struct vhost_memory_region *reg = dev->mem->regions + to;
-        memset(reg, 0, sizeof *reg);
-        reg->memory_size = size;
-        assert(reg->memory_size);
-        reg->guest_phys_addr = start_addr;
-        reg->userspace_addr = uaddr;
-        ++to;
-    }
-    assert(to <= dev->mem->nregions + 1);
-    dev->mem->nregions = to;
-}
-
 static uint64_t vhost_get_log_size(struct vhost_dev *dev)
 {
     uint64_t log_size = 0;
@@ -531,95 +377,6 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
     return r;
 }
 
-static struct vhost_memory_region *vhost_dev_find_reg(struct vhost_dev *dev,
-						      uint64_t start_addr,
-						      uint64_t size)
-{
-    int i, n = dev->mem->nregions;
-    for (i = 0; i < n; ++i) {
-        struct vhost_memory_region *reg = dev->mem->regions + i;
-        if (ranges_overlap(reg->guest_phys_addr, reg->memory_size,
-                           start_addr, size)) {
-            return reg;
-        }
-    }
-    return NULL;
-}
-
-static bool vhost_dev_cmp_memory(struct vhost_dev *dev,
-                                 uint64_t start_addr,
-                                 uint64_t size,
-                                 uint64_t uaddr)
-{
-    struct vhost_memory_region *reg = vhost_dev_find_reg(dev, start_addr, size);
-    uint64_t reglast;
-    uint64_t memlast;
-
-    if (!reg) {
-        return true;
-    }
-
-    reglast = range_get_last(reg->guest_phys_addr, reg->memory_size);
-    memlast = range_get_last(start_addr, size);
-
-    /* Need to extend region? */
-    if (start_addr < reg->guest_phys_addr || memlast > reglast) {
-        return true;
-    }
-    /* userspace_addr changed? */
-    return uaddr != reg->userspace_addr + start_addr - reg->guest_phys_addr;
-}
-
-static void vhost_set_memory(MemoryListener *listener,
-                             MemoryRegionSection *section,
-                             bool add)
-{
-    struct vhost_dev *dev = container_of(listener, struct vhost_dev,
-                                         memory_listener);
-    hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
-    bool log_dirty =
-        memory_region_get_dirty_log_mask(section->mr) & ~(1 << DIRTY_MEMORY_MIGRATION);
-    int s = offsetof(struct vhost_memory, regions) +
-        (dev->mem->nregions + 1) * sizeof dev->mem->regions[0];
-    void *ram;
-
-    dev->mem = g_realloc(dev->mem, s);
-
-    if (log_dirty) {
-        add = false;
-    }
-
-    assert(size);
-
-    /* Optimize no-change case. At least cirrus_vga does this a lot at this time. */
-    ram = memory_region_get_ram_ptr(section->mr) + section->offset_within_region;
-    if (add) {
-        if (!vhost_dev_cmp_memory(dev, start_addr, size, (uintptr_t)ram)) {
-            /* Region exists with same address. Nothing to do. */
-            return;
-        }
-    } else {
-        if (!vhost_dev_find_reg(dev, start_addr, size)) {
-            /* Removing region that we don't access. Nothing to do. */
-            return;
-        }
-    }
-
-    vhost_dev_unassign_memory(dev, start_addr, size);
-    if (add) {
-        /* Add given mapping, merging adjacent regions if any */
-        vhost_dev_assign_memory(dev, start_addr, size, (uintptr_t)ram);
-    } else {
-        /* Remove old mapping for this memory, if any. */
-        vhost_dev_unassign_memory(dev, start_addr, size);
-    }
-    dev->mem_changed_start_addr = MIN(dev->mem_changed_start_addr, start_addr);
-    dev->mem_changed_end_addr = MAX(dev->mem_changed_end_addr, start_addr + size - 1);
-    dev->memory_changed = true;
-    used_memslots = dev->mem->nregions;
-}
-
 static bool vhost_section(MemoryRegionSection *section)
 {
     return memory_region_is_ram(section->mr) &&
@@ -630,8 +387,6 @@ static void vhost_begin(MemoryListener *listener)
 {
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
-    dev->mem_changed_end_addr = 0;
-    dev->mem_changed_start_addr = -1;
     dev->tmp_sections = NULL;
     dev->n_tmp_sections = 0;
 }
@@ -707,7 +462,6 @@ static void vhost_commit(MemoryListener *listener)
         if (r < 0) {
             VHOST_OPS_DEBUG("vhost_set_mem_table failed");
         }
-        dev->memory_changed = false;
         goto out;
     }
     log_size = vhost_get_log_size(dev);
@@ -726,7 +480,6 @@ static void vhost_commit(MemoryListener *listener)
     if (dev->log_size > log_size + VHOST_LOG_BUFFER) {
         vhost_dev_log_resize(dev, log_size);
     }
-    dev->memory_changed = false;
 
 out:
     /* Deref the old list of sections, this must happen _after_ the
@@ -817,8 +570,6 @@ static void vhost_region_add(MemoryListener *listener,
         return;
     }
     vhost_region_add_section(dev, section);
-
-    vhost_set_memory(listener, section, true);
 }
 
 /* Called on regions that have not changed */
@@ -842,7 +593,6 @@ static void vhost_region_del(MemoryListener *listener,
         return;
     }
 
-    vhost_set_memory(listener, section, false);
 }
 
 static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
@@ -1457,7 +1207,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     hdev->log_size = 0;
     hdev->log_enabled = false;
     hdev->started = false;
-    hdev->memory_changed = false;
     memory_listener_register(&hdev->memory_listener, &address_space_memory);
     QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
     return 0;
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 10/26] vhost: Move log_dirty check
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (7 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 08/26] vhost: Clean out old vhost_set_memory and friends Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset Michael S. Tsirkin
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Dr. David Alan Gilbert

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Move the log_dirty check into vhost_section.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c      | 20 +++++++++++++-------
 hw/virtio/trace-events |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 326f168..4a44e6e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -27,6 +27,7 @@
 #include "hw/virtio/virtio-access.h"
 #include "migration/blocker.h"
 #include "sysemu/dma.h"
+#include "trace.h"
 
 /* enabled until disconnected backend stabilizes */
 #define _VHOST_DEBUG 1
@@ -379,8 +380,19 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
 
 static bool vhost_section(MemoryRegionSection *section)
 {
-    return memory_region_is_ram(section->mr) &&
+    bool result;
+    bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
+                     ~(1 << DIRTY_MEMORY_MIGRATION);
+    result = memory_region_is_ram(section->mr) &&
         !memory_region_is_rom(section->mr);
+
+    /* Vhost doesn't handle any block which is doing dirty-tracking other
+     * than migration; this typically fires on VGA areas.
+     */
+    result &= !log_dirty;
+
+    trace_vhost_section(section->mr->name, result);
+    return result;
 }
 
 static void vhost_begin(MemoryListener *listener)
@@ -510,12 +522,6 @@ static void vhost_region_add_section(struct vhost_dev *dev,
     trace_vhost_region_add_section(section->mr->name, mrs_gpa, mrs_size,
                                    mrs_host);
 
-    bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
-                        ~(1 << DIRTY_MEMORY_MIGRATION);
-    if (log_dirty) {
-        return;
-    }
-
     if (dev->n_tmp_sections) {
         /* Since we already have at least one section, lets see if
          * this extends it; since we're scanning in order, we only
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 606e4fc..742ff0f 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -4,6 +4,7 @@
 vhost_commit(bool started, bool changed) "Started: %d Changed: %d"
 vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
 vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 0x%"PRIx64
+vhost_section(const char *name, int r) "%s:%d"
 
 # hw/virtio/virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (8 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 10/26] vhost: Move log_dirty check Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-03-23 18:42   ` Laszlo Ersek
  2018-02-08 19:09 ` [Qemu-devel] [PULL 12/26] pci/bus: let it has higher migration priority Michael S. Tsirkin
                   ` (16 subsequent siblings)
  26 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Laszlo Ersek, Marcel Apfelbaum, qemu-stable

From: Laszlo Ersek <lersek@redhat.com>

The "i82801b11-bridge" device model is a descendant of "base-pci-bridge"
(TYPE_PCI_BRIDGE). However, unlike other similar devices, such as

- pci-bridge,
- pcie-pci-bridge,
- PCIE Root Port,
- xio3130 switch upstream and downstream ports,
- dec-21154-p2p-bridge,
- pbm-bridge,
- xilinx-pcie-root,

"i82801b11-bridge" does not clear the bridge specific registers at
platform reset.

This is a problem because devices on "i82801b11-bridge" continue to
respond to config space cycles after platform reset, when addressed with
the bus number that was previously programmed into the secondary bus
number register of "i82801b11-bridge". This error breaks OVMF's search for
extra (PXB) root buses, for example.

The device class reset method for "i82801b11-bridge" is currently NULL;
set it directly to pci_bridge_reset(), like the last three bridge models
in the above listing do.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: qemu-stable@nongnu.org
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1541839
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-bridge/i82801b11.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index cb522bf..ebf7f5f 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -98,6 +98,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)
     k->realize = i82801b11_bridge_realize;
     k->config_write = pci_bridge_write_config;
     dc->vmsd = &i82801b11_bridge_dev_vmstate;
+    dc->reset = pci_bridge_reset;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 12/26] pci/bus: let it has higher migration priority
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (9 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 13/26] virtio-blk: enable multiple vectors when using multiple I/O queues Michael S. Tsirkin
                   ` (15 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Peter Xu, Alex Williamson, Marcel Apfelbaum,
	Dr . David Alan Gilbert, Juan Quintela, Laurent Vivier,
	Maxime Coquelin

From: Peter Xu <peterx@redhat.com>

In the past, we prioritized IOMMU migration so that we have such a
priority order:

    IOMMU > PCI Devices

When migrating a guest with both vIOMMU and a pcie-root-port, we'll
always migrate vIOMMU first, since pci buses will be seen to have the
same priority of general PCI devices.

That's problematic.

The thing is that PCI bus number information is stored in the root port,
and that is needed by vIOMMU during post_load(), e.g., to figure out
context entry for a device.  If we don't have correct bus numbers for
devices, we won't be able to recover device state of the DMAR memory
regions, and things will be messed up.

So let's boost the PCIe root ports to be even with higher priority:

   PCIe Root Port > IOMMU > PCI Devices

A smoke test shows that this patch fixes bug 1538953.

Also, apply this rule to all the PCI bus/bridge devices: ioh3420,
xio3130_downstream, xio3130_upstream, pcie_pci_bridge, pci-pci bridge,
i82801b11.

I noted that we set pcie_pci_bridge_dev_vmstate twice.  Clean that up
together.

CC: Alex Williamson <alex.williamson@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: Laurent Vivier <lvivier@redhat.com>
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1538953
Reported-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/migration/vmstate.h        | 1 +
 hw/pci-bridge/gen_pcie_root_port.c | 1 +
 hw/pci-bridge/i82801b11.c          | 1 +
 hw/pci-bridge/ioh3420.c            | 1 +
 hw/pci-bridge/pci_bridge_dev.c     | 1 +
 hw/pci-bridge/pcie_pci_bridge.c    | 2 +-
 hw/pci-bridge/xio3130_downstream.c | 1 +
 hw/pci-bridge/xio3130_upstream.c   | 1 +
 8 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 8c38894..df463fd 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -148,6 +148,7 @@ enum VMStateFlags {
 typedef enum {
     MIG_PRI_DEFAULT = 0,
     MIG_PRI_IOMMU,              /* Must happen before PCI devices */
+    MIG_PRI_PCI_BUS,            /* Must happen before IOMMU */
     MIG_PRI_GICV3_ITS,          /* Must happen before PCI devices */
     MIG_PRI_GICV3,              /* Must happen before the ITS */
     MIG_PRI_MAX,
diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index 3dbacc6..d117e20 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -101,6 +101,7 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
 
 static const VMStateDescription vmstate_rp_dev = {
     .name = "pcie-root-port",
+    .priority = MIG_PRI_PCI_BUS,
     .version_id = 1,
     .minimum_version_id = 1,
     .post_load = pcie_cap_slot_post_load,
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index ebf7f5f..620b435 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -80,6 +80,7 @@ err_bridge:
 
 static const VMStateDescription i82801b11_bridge_dev_vmstate = {
     .name = "i82801b11_bridge",
+    .priority = MIG_PRI_PCI_BUS,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCIBridge),
         VMSTATE_END_OF_LIST()
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 5f56a2f..a7bfbdd 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -83,6 +83,7 @@ static void ioh3420_interrupts_uninit(PCIDevice *d)
 
 static const VMStateDescription vmstate_ioh3420 = {
     .name = "ioh-3240-express-root-port",
+    .priority = MIG_PRI_PCI_BUS,
     .version_id = 1,
     .minimum_version_id = 1,
     .post_load = pcie_cap_slot_post_load,
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index d56f663..b2d861d 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -174,6 +174,7 @@ static bool pci_device_shpc_present(void *opaque, int version_id)
 
 static const VMStateDescription pci_bridge_dev_vmstate = {
     .name = "pci_bridge",
+    .priority = MIG_PRI_PCI_BUS,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCIBridge),
         SHPC_VMSTATE(shpc, PCIDevice, pci_device_shpc_present),
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index a4d827c..e5ac797 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -129,6 +129,7 @@ static Property pcie_pci_bridge_dev_properties[] = {
 
 static const VMStateDescription pcie_pci_bridge_dev_vmstate = {
         .name = TYPE_PCIE_PCI_BRIDGE_DEV,
+        .priority = MIG_PRI_PCI_BUS,
         .fields = (VMStateField[]) {
             VMSTATE_PCI_DEVICE(parent_obj, PCIBridge),
             SHPC_VMSTATE(shpc, PCIDevice, NULL),
@@ -178,7 +179,6 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
     k->config_write = pcie_pci_bridge_write_config;
     dc->vmsd = &pcie_pci_bridge_dev_vmstate;
     dc->props = pcie_pci_bridge_dev_properties;
-    dc->vmsd = &pcie_pci_bridge_dev_vmstate;
     dc->reset = &pcie_pci_bridge_reset;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     hc->plug = pcie_pci_bridge_hotplug_cb;
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 1e09d2a..4dd2e65 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -161,6 +161,7 @@ static Property xio3130_downstream_props[] = {
 
 static const VMStateDescription vmstate_xio3130_downstream = {
     .name = "xio3130-express-downstream-port",
+    .priority = MIG_PRI_PCI_BUS,
     .version_id = 1,
     .minimum_version_id = 1,
     .post_load = pcie_cap_slot_post_load,
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 227997c..c5f02a6 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -133,6 +133,7 @@ PCIEPort *xio3130_upstream_init(PCIBus *bus, int devfn, bool multifunction,
 
 static const VMStateDescription vmstate_xio3130_upstream = {
     .name = "xio3130-express-upstream-port",
+    .priority = MIG_PRI_PCI_BUS,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 13/26] virtio-blk: enable multiple vectors when using multiple I/O queues
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (10 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 12/26] pci/bus: let it has higher migration priority Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09   ` Michael S. Tsirkin
                   ` (14 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Changpeng Liu, Paolo Bonzini

From: Changpeng Liu <changpeng.liu@intel.com>

Currently virtio-pci driver hardcoded 2 vectors for virtio-blk device,
for multiple I/O queues scenario, all the I/O queues will share one
interrupt vector, while here, enable multiple vectors according to
the number of I/O queues.

Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/compat.h    |  8 ++++++++
 hw/virtio/virtio-pci.c | 14 ++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 7f31850..bc9e3a6 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -6,6 +6,14 @@
         .driver   = "hpet",\
         .property = "hpet-offset-saved",\
         .value    = "false",\
+    },{\
+        .driver   = "virtio-blk-pci",\
+        .property = "vectors",\
+        .value    = "2",\
+    },{\
+        .driver   = "vhost-user-blk-pci",\
+        .property = "vectors",\
+        .value    = "2",\
     },
 
 #define HW_COMPAT_2_10 \
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c20537f..b55dfcf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1932,7 +1932,8 @@ static Property virtio_blk_pci_properties[] = {
     DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1941,6 +1942,10 @@ static void virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = dev->vdev.conf.num_queues + 1;
+    }
+
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
     object_property_set_bool(OBJECT(vdev), true, "realized", errp);
 }
@@ -1983,7 +1988,8 @@ static const TypeInfo virtio_blk_pci_info = {
 
 static Property vhost_user_blk_pci_properties[] = {
     DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1992,6 +1998,10 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = dev->vdev.num_queues + 1;
+    }
+
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
     object_property_set_bool(OBJECT(vdev), true, "realized", errp);
 }
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 14/26] pci: removed the is_express field since a uniform interface was inserted
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
@ 2018-02-08 19:09   ` Michael S. Tsirkin
  2018-02-08 19:08 ` [Qemu-devel] [PULL 02/26] virtio: remove event notifier cleanup call on de-assign Michael S. Tsirkin
                     ` (25 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Yoni Bettan, Marcel Apfelbaum, Eduardo Habkost,
	Keith Busch, Kevin Wolf, Max Reitz, Dmitry Fleytman, Jason Wang,
	Paul Burton, Paolo Bonzini, Fam Zheng, Hannes Reinecke,
	Gerd Hoffmann, Alex Williamson, Stefano Stabellini,
	Anthony Perard, qemu-block, xen-devel

From: Yoni Bettan <ybettan@redhat.com>

according to Eduardo Habkost's commit fd3b02c889 all PCIEs now implement
INTERFACE_PCIE_DEVICE so we don't need is_express field anymore.

Devices that implements only INTERFACE_PCIE_DEVICE (is_express == 1)
or
devices that implements only INTERFACE_CONVENTIONAL_PCI_DEVICE (is_express == 0)
where not affected by the change.

The only devices that were affected are those that are hybrid and also
had (is_express == 1) - therefor only:
  - hw/vfio/pci.c
  - hw/usb/hcd-xhci.c
  - hw/xen/xen_pt.c

For those 3 I made sure that QEMU_PCI_CAP_EXPRESS is on in instance_init()

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Yoni Bettan <ybettan@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/pcie_pci_bridge.txt           | 2 +-
 include/hw/pci/pci.h               | 3 ---
 hw/block/nvme.c                    | 1 -
 hw/net/e1000e.c                    | 1 -
 hw/pci-bridge/pcie_pci_bridge.c    | 1 -
 hw/pci-bridge/pcie_root_port.c     | 1 -
 hw/pci-bridge/xio3130_downstream.c | 1 -
 hw/pci-bridge/xio3130_upstream.c   | 1 -
 hw/pci-host/xilinx-pcie.c          | 1 -
 hw/pci/pci.c                       | 8 ++++++--
 hw/scsi/megasas.c                  | 4 ----
 hw/usb/hcd-xhci.c                  | 9 ++++++++-
 hw/vfio/pci.c                      | 5 ++++-
 hw/xen/xen_pt.c                    | 9 ++++++++-
 14 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/docs/pcie_pci_bridge.txt b/docs/pcie_pci_bridge.txt
index 5a4203f..ab35ebf 100644
--- a/docs/pcie_pci_bridge.txt
+++ b/docs/pcie_pci_bridge.txt
@@ -110,5 +110,5 @@ To enable device hot-plug into the bridge on Linux there're 3 ways:
 Implementation
 ==============
 The PCIE-PCI bridge is based on PCI-PCI bridge, but also accumulates PCI Express
-features as a PCI Express device (is_express=1).
+features as a PCI Express device.
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 15ced96..d8c18c7 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -236,9 +236,6 @@ typedef struct PCIDeviceClass {
      */
     int is_bridge;
 
-    /* pcie stuff */
-    int is_express;   /* is this device pci express? */
-
     /* rom bar */
     const char *romfile;
 } PCIDeviceClass;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 51a58fe..85d2406 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1360,7 +1360,6 @@ static void nvme_class_init(ObjectClass *oc, void *data)
     pc->vendor_id = PCI_VENDOR_ID_INTEL;
     pc->device_id = 0x5845;
     pc->revision = 2;
-    pc->is_express = 1;
 
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->desc = "Non-Volatile Memory Express";
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 191398a..16a9417 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -675,7 +675,6 @@ static void e1000e_class_init(ObjectClass *class, void *data)
     c->revision = 0;
     c->romfile = "efi-e1000e.rom";
     c->class_id = PCI_CLASS_NETWORK_ETHERNET;
-    c->is_express = 1;
 
     dc->desc = "Intel 82574L GbE Controller";
     dc->reset = e1000e_qdev_reset;
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index e5ac797..04cf5a6 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -170,7 +170,6 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->vendor_id = PCI_VENDOR_ID_REDHAT;
     k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE;
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 9b6e4ce..45f9e8c 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -145,7 +145,6 @@ static void rp_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = rp_write_config;
     k->realize = rp_realize;
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 4dd2e65..b202657 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -178,7 +178,6 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = xio3130_downstream_write_config;
     k->realize = xio3130_downstream_realize;
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index c5f02a6..556f471 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -149,7 +149,6 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = xio3130_upstream_write_config;
     k->realize = xio3130_upstream_realize;
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 53b561f..044e312 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -297,7 +297,6 @@ static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
     k->device_id = 0x7021;
     k->revision = 0;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
-    k->is_express = true;
     k->is_bridge = true;
     k->realize = xilinx_pcie_root_realize;
     k->exit = pci_bridge_exitfn;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fc25cde..ef43422 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2005,11 +2005,15 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
+    ObjectClass *klass = OBJECT_CLASS(pc);
     Error *local_err = NULL;
     bool is_default_rom;
 
-    /* initialize cap_present for pci_is_express() and pci_config_size() */
-    if (pc->is_express) {
+    /* initialize cap_present for pci_is_express() and pci_config_size(),
+     * Note that hybrid PCIs are not set automatically and need to manage
+     * QEMU_PCI_CAP_EXPRESS manually */
+    if (object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE) &&
+       !object_class_dynamic_cast(klass, INTERFACE_CONVENTIONAL_PCI_DEVICE)) {
         pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
     }
 
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 3e38e9e..ba1afa3 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2447,7 +2447,6 @@ typedef struct MegasasInfo {
     uint16_t subsystem_id;
     int ioport_bar;
     int mmio_bar;
-    bool is_express;
     int osts;
     const VMStateDescription *vmsd;
     Property *props;
@@ -2465,7 +2464,6 @@ static struct MegasasInfo megasas_devices[] = {
         .ioport_bar = 2,
         .mmio_bar = 0,
         .osts = MFI_1078_RM | 1,
-        .is_express = false,
         .vmsd = &vmstate_megasas_gen1,
         .props = megasas_properties_gen1,
         .interfaces = (InterfaceInfo[]) {
@@ -2482,7 +2480,6 @@ static struct MegasasInfo megasas_devices[] = {
         .ioport_bar = 0,
         .mmio_bar = 1,
         .osts = MFI_GEN2_RM,
-        .is_express = true,
         .vmsd = &vmstate_megasas_gen2,
         .props = megasas_properties_gen2,
         .interfaces = (InterfaceInfo[]) {
@@ -2506,7 +2503,6 @@ static void megasas_class_init(ObjectClass *oc, void *data)
     pc->subsystem_vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     pc->subsystem_id = info->subsystem_id;
     pc->class_id = PCI_CLASS_STORAGE_RAID;
-    pc->is_express = info->is_express;
     e->mmio_bar = info->mmio_bar;
     e->ioport_bar = info->ioport_bar;
     e->osts = info->osts;
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 228e82b..721beb5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3649,6 +3649,13 @@ static Property xhci_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xhci_instance_init(Object *obj)
+{
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
+}
+
 static void xhci_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -3661,7 +3668,6 @@ static void xhci_class_init(ObjectClass *klass, void *data)
     k->realize      = usb_xhci_realize;
     k->exit         = usb_xhci_exit;
     k->class_id     = PCI_CLASS_SERIAL_USB;
-    k->is_express   = 1;
 }
 
 static const TypeInfo xhci_info = {
@@ -3669,6 +3675,7 @@ static const TypeInfo xhci_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(XHCIState),
     .class_init    = xhci_class_init,
+    .instance_init = xhci_instance_init,
     .abstract      = true,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_PCIE_DEVICE },
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 879510c..b33c5e8 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3113,6 +3113,10 @@ static void vfio_instance_init(Object *obj)
     vdev->host.function = ~0U;
 
     vdev->nv_gpudirect_clique = 0xFF;
+
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
 }
 
 static Property vfio_pci_dev_properties[] = {
@@ -3171,7 +3175,6 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
     pdc->exit = vfio_exitfn;
     pdc->config_read = vfio_pci_read_config;
     pdc->config_write = vfio_pci_write_config;
-    pdc->is_express = 1; /* We might be */
 }
 
 static const TypeInfo vfio_pci_dev_info = {
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index f662f30..9b7a960 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -937,6 +937,13 @@ static Property xen_pci_passthrough_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xen_pci_passthrough_instance_init(Object *obj)
+{
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
+}
+
 static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -946,7 +953,6 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
     k->exit = xen_pt_unregister_device;
     k->config_read = xen_pt_pci_read_config;
     k->config_write = xen_pt_pci_write_config;
-    k->is_express = 1; /* We might be */
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->desc = "Assign an host PCI device with Xen";
     dc->props = xen_pci_passthrough_properties;
@@ -965,6 +971,7 @@ static const TypeInfo xen_pci_passthrough_info = {
     .instance_size = sizeof(XenPCIPassthroughState),
     .instance_finalize = xen_pci_passthrough_finalize,
     .class_init = xen_pci_passthrough_class_init,
+    .instance_init = xen_pci_passthrough_instance_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { INTERFACE_PCIE_DEVICE },
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PULL 14/26] pci: removed the is_express field since a uniform interface was inserted
@ 2018-02-08 19:09   ` Michael S. Tsirkin
  0 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Hannes Reinecke, Dmitry Fleytman,
	Fam Zheng, Eduardo Habkost, qemu-block, Yoni Bettan,
	Alex Williamson, Max Reitz, Keith Busch, Anthony Perard,
	Paul Burton, Stefano Stabellini, Gerd Hoffmann, xen-devel,
	Marcel Apfelbaum, Paolo Bonzini, Jason Wang

From: Yoni Bettan <ybettan@redhat.com>

according to Eduardo Habkost's commit fd3b02c889 all PCIEs now implement
INTERFACE_PCIE_DEVICE so we don't need is_express field anymore.

Devices that implements only INTERFACE_PCIE_DEVICE (is_express == 1)
or
devices that implements only INTERFACE_CONVENTIONAL_PCI_DEVICE (is_express == 0)
where not affected by the change.

The only devices that were affected are those that are hybrid and also
had (is_express == 1) - therefor only:
  - hw/vfio/pci.c
  - hw/usb/hcd-xhci.c
  - hw/xen/xen_pt.c

For those 3 I made sure that QEMU_PCI_CAP_EXPRESS is on in instance_init()

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Yoni Bettan <ybettan@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/pcie_pci_bridge.txt           | 2 +-
 include/hw/pci/pci.h               | 3 ---
 hw/block/nvme.c                    | 1 -
 hw/net/e1000e.c                    | 1 -
 hw/pci-bridge/pcie_pci_bridge.c    | 1 -
 hw/pci-bridge/pcie_root_port.c     | 1 -
 hw/pci-bridge/xio3130_downstream.c | 1 -
 hw/pci-bridge/xio3130_upstream.c   | 1 -
 hw/pci-host/xilinx-pcie.c          | 1 -
 hw/pci/pci.c                       | 8 ++++++--
 hw/scsi/megasas.c                  | 4 ----
 hw/usb/hcd-xhci.c                  | 9 ++++++++-
 hw/vfio/pci.c                      | 5 ++++-
 hw/xen/xen_pt.c                    | 9 ++++++++-
 14 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/docs/pcie_pci_bridge.txt b/docs/pcie_pci_bridge.txt
index 5a4203f..ab35ebf 100644
--- a/docs/pcie_pci_bridge.txt
+++ b/docs/pcie_pci_bridge.txt
@@ -110,5 +110,5 @@ To enable device hot-plug into the bridge on Linux there're 3 ways:
 Implementation
 ==============
 The PCIE-PCI bridge is based on PCI-PCI bridge, but also accumulates PCI Express
-features as a PCI Express device (is_express=1).
+features as a PCI Express device.
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 15ced96..d8c18c7 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -236,9 +236,6 @@ typedef struct PCIDeviceClass {
      */
     int is_bridge;
 
-    /* pcie stuff */
-    int is_express;   /* is this device pci express? */
-
     /* rom bar */
     const char *romfile;
 } PCIDeviceClass;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 51a58fe..85d2406 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1360,7 +1360,6 @@ static void nvme_class_init(ObjectClass *oc, void *data)
     pc->vendor_id = PCI_VENDOR_ID_INTEL;
     pc->device_id = 0x5845;
     pc->revision = 2;
-    pc->is_express = 1;
 
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     dc->desc = "Non-Volatile Memory Express";
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 191398a..16a9417 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -675,7 +675,6 @@ static void e1000e_class_init(ObjectClass *class, void *data)
     c->revision = 0;
     c->romfile = "efi-e1000e.rom";
     c->class_id = PCI_CLASS_NETWORK_ETHERNET;
-    c->is_express = 1;
 
     dc->desc = "Intel 82574L GbE Controller";
     dc->reset = e1000e_qdev_reset;
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index e5ac797..04cf5a6 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -170,7 +170,6 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->vendor_id = PCI_VENDOR_ID_REDHAT;
     k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE;
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 9b6e4ce..45f9e8c 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -145,7 +145,6 @@ static void rp_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = rp_write_config;
     k->realize = rp_realize;
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 4dd2e65..b202657 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -178,7 +178,6 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = xio3130_downstream_write_config;
     k->realize = xio3130_downstream_realize;
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index c5f02a6..556f471 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -149,7 +149,6 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->is_express = 1;
     k->is_bridge = 1;
     k->config_write = xio3130_upstream_write_config;
     k->realize = xio3130_upstream_realize;
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 53b561f..044e312 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -297,7 +297,6 @@ static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
     k->device_id = 0x7021;
     k->revision = 0;
     k->class_id = PCI_CLASS_BRIDGE_HOST;
-    k->is_express = true;
     k->is_bridge = true;
     k->realize = xilinx_pcie_root_realize;
     k->exit = pci_bridge_exitfn;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fc25cde..ef43422 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2005,11 +2005,15 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
+    ObjectClass *klass = OBJECT_CLASS(pc);
     Error *local_err = NULL;
     bool is_default_rom;
 
-    /* initialize cap_present for pci_is_express() and pci_config_size() */
-    if (pc->is_express) {
+    /* initialize cap_present for pci_is_express() and pci_config_size(),
+     * Note that hybrid PCIs are not set automatically and need to manage
+     * QEMU_PCI_CAP_EXPRESS manually */
+    if (object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE) &&
+       !object_class_dynamic_cast(klass, INTERFACE_CONVENTIONAL_PCI_DEVICE)) {
         pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
     }
 
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 3e38e9e..ba1afa3 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2447,7 +2447,6 @@ typedef struct MegasasInfo {
     uint16_t subsystem_id;
     int ioport_bar;
     int mmio_bar;
-    bool is_express;
     int osts;
     const VMStateDescription *vmsd;
     Property *props;
@@ -2465,7 +2464,6 @@ static struct MegasasInfo megasas_devices[] = {
         .ioport_bar = 2,
         .mmio_bar = 0,
         .osts = MFI_1078_RM | 1,
-        .is_express = false,
         .vmsd = &vmstate_megasas_gen1,
         .props = megasas_properties_gen1,
         .interfaces = (InterfaceInfo[]) {
@@ -2482,7 +2480,6 @@ static struct MegasasInfo megasas_devices[] = {
         .ioport_bar = 0,
         .mmio_bar = 1,
         .osts = MFI_GEN2_RM,
-        .is_express = true,
         .vmsd = &vmstate_megasas_gen2,
         .props = megasas_properties_gen2,
         .interfaces = (InterfaceInfo[]) {
@@ -2506,7 +2503,6 @@ static void megasas_class_init(ObjectClass *oc, void *data)
     pc->subsystem_vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     pc->subsystem_id = info->subsystem_id;
     pc->class_id = PCI_CLASS_STORAGE_RAID;
-    pc->is_express = info->is_express;
     e->mmio_bar = info->mmio_bar;
     e->ioport_bar = info->ioport_bar;
     e->osts = info->osts;
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 228e82b..721beb5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3649,6 +3649,13 @@ static Property xhci_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xhci_instance_init(Object *obj)
+{
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
+}
+
 static void xhci_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -3661,7 +3668,6 @@ static void xhci_class_init(ObjectClass *klass, void *data)
     k->realize      = usb_xhci_realize;
     k->exit         = usb_xhci_exit;
     k->class_id     = PCI_CLASS_SERIAL_USB;
-    k->is_express   = 1;
 }
 
 static const TypeInfo xhci_info = {
@@ -3669,6 +3675,7 @@ static const TypeInfo xhci_info = {
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(XHCIState),
     .class_init    = xhci_class_init,
+    .instance_init = xhci_instance_init,
     .abstract      = true,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_PCIE_DEVICE },
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 879510c..b33c5e8 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3113,6 +3113,10 @@ static void vfio_instance_init(Object *obj)
     vdev->host.function = ~0U;
 
     vdev->nv_gpudirect_clique = 0xFF;
+
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
 }
 
 static Property vfio_pci_dev_properties[] = {
@@ -3171,7 +3175,6 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
     pdc->exit = vfio_exitfn;
     pdc->config_read = vfio_pci_read_config;
     pdc->config_write = vfio_pci_write_config;
-    pdc->is_express = 1; /* We might be */
 }
 
 static const TypeInfo vfio_pci_dev_info = {
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index f662f30..9b7a960 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -937,6 +937,13 @@ static Property xen_pci_passthrough_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xen_pci_passthrough_instance_init(Object *obj)
+{
+    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices */
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
+}
+
 static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -946,7 +953,6 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
     k->exit = xen_pt_unregister_device;
     k->config_read = xen_pt_pci_read_config;
     k->config_write = xen_pt_pci_write_config;
-    k->is_express = 1; /* We might be */
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     dc->desc = "Assign an host PCI device with Xen";
     dc->props = xen_pci_passthrough_properties;
@@ -965,6 +971,7 @@ static const TypeInfo xen_pci_passthrough_info = {
     .instance_size = sizeof(XenPCIPassthroughState),
     .instance_finalize = xen_pci_passthrough_finalize,
     .class_init = xen_pci_passthrough_class_init,
+    .instance_init = xen_pci_passthrough_instance_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { INTERFACE_PCIE_DEVICE },
-- 
MST


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 15/26] cryptodev: add vhost-user as a new cryptodev backend
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (12 preceding siblings ...)
  2018-02-08 19:09   ` Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-13 16:54   ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 17/26] cryptodev-vhost-user: add crypto session handler Michael S. Tsirkin
                   ` (12 subsequent siblings)
  26 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gonglei, Longpeng, Jay Zhou, Paolo Bonzini

From: Gonglei <arei.gonglei@huawei.com>

Usage:
 -chardev socket,id=charcrypto0,path=/path/to/your/socket
 -object cryptodev-vhost-user,id=cryptodev0,chardev=charcrypto0
 -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 configure                        |  15 ++
 include/sysemu/cryptodev-vhost.h | 154 ++++++++++++++++++
 backends/cryptodev-vhost-user.c  | 333 +++++++++++++++++++++++++++++++++++++++
 backends/cryptodev-vhost.c       |  89 +++++++++++
 vl.c                             |   6 +
 backends/Makefile.objs           |   6 +
 qemu-options.hx                  |  21 +++
 7 files changed, 624 insertions(+)
 create mode 100644 include/sysemu/cryptodev-vhost.h
 create mode 100644 backends/cryptodev-vhost-user.c
 create mode 100644 backends/cryptodev-vhost.c

diff --git a/configure b/configure
index 831ebf2..296f980 100755
--- a/configure
+++ b/configure
@@ -344,6 +344,7 @@ xfs=""
 tcg="yes"
 
 vhost_net="no"
+vhost_crypto="no"
 vhost_scsi="no"
 vhost_vsock="no"
 vhost_user=""
@@ -814,6 +815,7 @@ Linux)
   linux_user="yes"
   kvm="yes"
   vhost_net="yes"
+  vhost_crypto="yes"
   vhost_scsi="yes"
   vhost_vsock="yes"
   QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
@@ -1184,6 +1186,14 @@ for opt do
   ;;
   --enable-vhost-net) vhost_net="yes"
   ;;
+  --disable-vhost-crypto) vhost_crypto="no"
+  ;;
+  --enable-vhost-crypto)
+      vhost_crypto="yes"
+      if test "$mingw32" = "yes"; then
+          error_exit "vhost-crypto isn't available on win32"
+      fi
+  ;;
   --disable-vhost-scsi) vhost_scsi="no"
   ;;
   --enable-vhost-scsi) vhost_scsi="yes"
@@ -1582,6 +1592,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   cap-ng          libcap-ng support
   attr            attr and xattr support
   vhost-net       vhost-net acceleration support
+  vhost-crypto    vhost-crypto acceleration support
   spice           spice
   rbd             rados block device (rbd)
   libiscsi        iscsi support
@@ -5704,6 +5715,7 @@ echo "madvise           $madvise"
 echo "posix_madvise     $posix_madvise"
 echo "libcap-ng support $cap_ng"
 echo "vhost-net support $vhost_net"
+echo "vhost-crypto support $vhost_crypto"
 echo "vhost-scsi support $vhost_scsi"
 echo "vhost-vsock support $vhost_vsock"
 echo "vhost-user support $vhost_user"
@@ -6783,6 +6795,9 @@ if supported_kvm_target $target; then
             echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> $config_host_mak
         fi
     fi
+    if test "$vhost_crypto" = "yes" ; then
+        echo "CONFIG_VHOST_CRYPTO=y" >> $config_target_mak
+    fi
 fi
 if supported_hax_target $target; then
     echo "CONFIG_HAX=y" >> $config_target_mak
diff --git a/include/sysemu/cryptodev-vhost.h b/include/sysemu/cryptodev-vhost.h
new file mode 100644
index 0000000..fb26b86
--- /dev/null
+++ b/include/sysemu/cryptodev-vhost.h
@@ -0,0 +1,154 @@
+/*
+ * QEMU Crypto Device Common Vhost Implement
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *    Jay Zhou <jianjay.zhou@huawei.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#ifndef CRYPTODEV_VHOST_H
+#define CRYPTODEV_VHOST_H
+
+#include "qemu-common.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-backend.h"
+#include "chardev/char.h"
+
+#include "sysemu/cryptodev.h"
+
+
+typedef struct CryptoDevBackendVhostOptions {
+    VhostBackendType backend_type;
+    void *opaque;
+    int total_queues;
+    CryptoDevBackendClient *cc;
+} CryptoDevBackendVhostOptions;
+
+typedef struct CryptoDevBackendVhost {
+    struct vhost_dev dev;
+    struct vhost_virtqueue vqs[1];
+    int backend;
+    CryptoDevBackendClient *cc;
+} CryptoDevBackendVhost;
+
+/**
+ * cryptodev_vhost_get_max_queues:
+ * @crypto: the cryptodev backend common vhost object
+ *
+ * Get the maximum queue number of @crypto.
+ *
+ *
+ * Returns: the maximum queue number
+ */
+uint64_t
+cryptodev_vhost_get_max_queues(
+                        CryptoDevBackendVhost *crypto);
+
+
+/**
+ * cryptodev_vhost_init:
+ * @options: the common vhost object's option
+ *
+ * Creates a new cryptodev backend common vhost object
+ *
+ ** The returned object must be released with
+ * cryptodev_vhost_cleanup() when no
+ * longer required
+ *
+ * Returns: the cryptodev backend common vhost object
+ */
+struct CryptoDevBackendVhost *
+cryptodev_vhost_init(
+             CryptoDevBackendVhostOptions *options);
+
+/**
+ * cryptodev_vhost_cleanup:
+ * @crypto: the cryptodev backend common vhost object
+ *
+ * Clean the resouce associated with @crypto that realizaed
+ * by cryptodev_vhost_init()
+ *
+ */
+void cryptodev_vhost_cleanup(
+                        CryptoDevBackendVhost *crypto);
+
+/**
+ * cryptodev_get_vhost:
+ * @cc: the client object for each queue
+ * @b: the cryptodev backend common vhost object
+ * @queue: the cryptodev backend queue index
+ *
+ * Gets a new cryptodev backend common vhost object based on
+ * @b and @queue
+ *
+ * Returns: the cryptodev backend common vhost object
+ */
+CryptoDevBackendVhost *
+cryptodev_get_vhost(CryptoDevBackendClient *cc,
+                            CryptoDevBackend *b,
+                            uint16_t queue);
+/**
+ * cryptodev_vhost_start:
+ * @dev: the virtio crypto object
+ * @total_queues: the total count of queue
+ *
+ * Starts the vhost crypto logic
+ *
+ * Returns: 0 for success, negative for errors
+ */
+int cryptodev_vhost_start(VirtIODevice *dev, int total_queues);
+
+/**
+ * cryptodev_vhost_stop:
+ * @dev: the virtio crypto object
+ * @total_queues: the total count of queue
+ *
+ * Stops the vhost crypto logic
+ *
+ */
+void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues);
+
+/**
+ * cryptodev_vhost_virtqueue_mask:
+ * @dev: the virtio crypto object
+ * @queue: the cryptodev backend queue index
+ * @idx: the virtqueue index
+ * @mask: mask or not (true or false)
+ *
+ * Mask/unmask events for @idx virtqueue on @dev device
+ *
+ */
+void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
+                                           int queue,
+                                           int idx, bool mask);
+
+/**
+ * cryptodev_vhost_virtqueue_pending:
+ * @dev: the virtio crypto object
+ * @queue: the cryptodev backend queue index
+ * @idx: the virtqueue index
+ *
+ * Test and clear event pending status for @idx virtqueue on @dev device.
+ * Should be called after unmask to avoid losing events.
+ *
+ * Returns: true for success, false for errors
+ */
+bool cryptodev_vhost_virtqueue_pending(VirtIODevice *dev,
+                                              int queue, int idx);
+
+#endif /* CRYPTODEV_VHOST_H */
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
new file mode 100644
index 0000000..4e63ece
--- /dev/null
+++ b/backends/cryptodev-vhost-user.c
@@ -0,0 +1,333 @@
+/*
+ * QEMU Cryptodev backend for QEMU cipher APIs
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "standard-headers/linux/virtio_crypto.h"
+#include "sysemu/cryptodev-vhost.h"
+#include "chardev/char-fe.h"
+
+
+/**
+ * @TYPE_CRYPTODEV_BACKEND_VHOST_USER:
+ * name of backend that uses vhost user server
+ */
+#define TYPE_CRYPTODEV_BACKEND_VHOST_USER "cryptodev-vhost-user"
+
+#define CRYPTODEV_BACKEND_VHOST_USER(obj) \
+    OBJECT_CHECK(CryptoDevBackendVhostUser, \
+                 (obj), TYPE_CRYPTODEV_BACKEND_VHOST_USER)
+
+
+typedef struct CryptoDevBackendVhostUser {
+    CryptoDevBackend parent_obj;
+
+    CharBackend chr;
+    char *chr_name;
+    bool opened;
+    CryptoDevBackendVhost *vhost_crypto[MAX_CRYPTO_QUEUE_NUM];
+} CryptoDevBackendVhostUser;
+
+static int
+cryptodev_vhost_user_running(
+             CryptoDevBackendVhost *crypto)
+{
+    return crypto ? 1 : 0;
+}
+
+static void cryptodev_vhost_user_stop(int queues,
+                          CryptoDevBackendVhostUser *s)
+{
+    size_t i;
+
+    for (i = 0; i < queues; i++) {
+        if (!cryptodev_vhost_user_running(s->vhost_crypto[i])) {
+            continue;
+        }
+
+        if (s->vhost_crypto) {
+            cryptodev_vhost_cleanup(s->vhost_crypto[i]);
+            s->vhost_crypto[i] = NULL;
+        }
+    }
+}
+
+static int
+cryptodev_vhost_user_start(int queues,
+                         CryptoDevBackendVhostUser *s)
+{
+    CryptoDevBackendVhostOptions options;
+    CryptoDevBackend *b = CRYPTODEV_BACKEND(s);
+    int max_queues;
+    size_t i;
+
+    for (i = 0; i < queues; i++) {
+        if (cryptodev_vhost_user_running(s->vhost_crypto[i])) {
+            continue;
+        }
+
+        options.opaque = &s->chr;
+        options.backend_type = VHOST_BACKEND_TYPE_USER;
+        options.cc = b->conf.peers.ccs[i];
+        s->vhost_crypto[i] = cryptodev_vhost_init(&options);
+        if (!s->vhost_crypto[i]) {
+            error_report("failed to init vhost_crypto for queue %lu", i);
+            goto err;
+        }
+
+        if (i == 0) {
+            max_queues =
+              cryptodev_vhost_get_max_queues(s->vhost_crypto[i]);
+            if (queues > max_queues) {
+                error_report("you are asking more queues than supported: %d",
+                             max_queues);
+                goto err;
+            }
+        }
+    }
+
+    return 0;
+
+err:
+    cryptodev_vhost_user_stop(i + 1, s);
+    return -1;
+}
+
+static Chardev *
+cryptodev_vhost_claim_chardev(CryptoDevBackendVhostUser *s,
+                                    Error **errp)
+{
+    Chardev *chr;
+
+    if (s->chr_name == NULL) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "chardev", "a valid character device");
+        return NULL;
+    }
+
+    chr = qemu_chr_find(s->chr_name);
+    if (chr == NULL) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", s->chr_name);
+        return NULL;
+    }
+
+    return chr;
+}
+
+static void cryptodev_vhost_user_event(void *opaque, int event)
+{
+    CryptoDevBackendVhostUser *s = opaque;
+    CryptoDevBackend *b = CRYPTODEV_BACKEND(s);
+    Error *err = NULL;
+    int queues = b->conf.peers.queues;
+
+    assert(queues < MAX_CRYPTO_QUEUE_NUM);
+
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        if (cryptodev_vhost_user_start(queues, s) < 0) {
+            exit(1);
+        }
+        b->ready = true;
+        break;
+    case CHR_EVENT_CLOSED:
+        b->ready = false;
+        cryptodev_vhost_user_stop(queues, s);
+        break;
+    }
+
+    if (err) {
+        error_report_err(err);
+    }
+}
+
+static void cryptodev_vhost_user_init(
+             CryptoDevBackend *backend, Error **errp)
+{
+    int queues = backend->conf.peers.queues;
+    size_t i;
+    Error *local_err = NULL;
+    Chardev *chr;
+    CryptoDevBackendClient *cc;
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(backend);
+
+    chr = cryptodev_vhost_claim_chardev(s, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    s->opened = true;
+
+    for (i = 0; i < queues; i++) {
+        cc = cryptodev_backend_new_client(
+                  "cryptodev-vhost-user", NULL);
+        cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
+                                       i, chr->label);
+        cc->queue_index = i;
+
+        backend->conf.peers.ccs[i] = cc;
+
+        if (i == 0) {
+            if (!qemu_chr_fe_init(&s->chr, chr, &local_err)) {
+                error_propagate(errp, local_err);
+                return;
+            }
+        }
+    }
+
+    qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
+                     cryptodev_vhost_user_event, NULL, s, NULL, true);
+
+    backend->conf.crypto_services =
+                         1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
+                         1u << VIRTIO_CRYPTO_SERVICE_HASH |
+                         1u << VIRTIO_CRYPTO_SERVICE_MAC;
+    backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
+    backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
+}
+
+static int64_t cryptodev_vhost_user_sym_create_session(
+           CryptoDevBackend *backend,
+           CryptoDevBackendSymSessionInfo *sess_info,
+           uint32_t queue_index, Error **errp)
+{
+    return 0;
+}
+
+static int cryptodev_vhost_user_sym_close_session(
+           CryptoDevBackend *backend,
+           uint64_t session_id,
+           uint32_t queue_index, Error **errp)
+{
+    return 0;
+}
+
+static int cryptodev_vhost_user_sym_operation(
+                 CryptoDevBackend *backend,
+                 CryptoDevBackendSymOpInfo *op_info,
+                 uint32_t queue_index, Error **errp)
+{
+    return VIRTIO_CRYPTO_OK;
+}
+
+static void cryptodev_vhost_user_cleanup(
+             CryptoDevBackend *backend,
+             Error **errp)
+{
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(backend);
+    size_t i;
+    int queues = backend->conf.peers.queues;
+    CryptoDevBackendClient *cc;
+
+    cryptodev_vhost_user_stop(queues, s);
+
+    for (i = 0; i < queues; i++) {
+        cc = backend->conf.peers.ccs[i];
+        if (cc) {
+            cryptodev_backend_free_client(cc);
+            backend->conf.peers.ccs[i] = NULL;
+        }
+    }
+}
+
+static void cryptodev_vhost_user_set_chardev(Object *obj,
+                                    const char *value, Error **errp)
+{
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(obj);
+
+    if (s->opened) {
+        error_setg(errp, QERR_PERMISSION_DENIED);
+    } else {
+        g_free(s->chr_name);
+        s->chr_name = g_strdup(value);
+    }
+}
+
+static char *
+cryptodev_vhost_user_get_chardev(Object *obj, Error **errp)
+{
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(obj);
+    Chardev *chr = qemu_chr_fe_get_driver(&s->chr);
+
+    if (chr && chr->label) {
+        return g_strdup(chr->label);
+    }
+
+    return NULL;
+}
+
+static void cryptodev_vhost_user_instance_int(Object *obj)
+{
+    object_property_add_str(obj, "chardev",
+                            cryptodev_vhost_user_get_chardev,
+                            cryptodev_vhost_user_set_chardev,
+                            NULL);
+}
+
+static void cryptodev_vhost_user_finalize(Object *obj)
+{
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(obj);
+
+    qemu_chr_fe_deinit(&s->chr, false);
+
+    g_free(s->chr_name);
+}
+
+static void
+cryptodev_vhost_user_class_init(ObjectClass *oc, void *data)
+{
+    CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_CLASS(oc);
+
+    bc->init = cryptodev_vhost_user_init;
+    bc->cleanup = cryptodev_vhost_user_cleanup;
+    bc->create_session = cryptodev_vhost_user_sym_create_session;
+    bc->close_session = cryptodev_vhost_user_sym_close_session;
+    bc->do_sym_op = cryptodev_vhost_user_sym_operation;
+}
+
+static const TypeInfo cryptodev_vhost_user_info = {
+    .name = TYPE_CRYPTODEV_BACKEND_VHOST_USER,
+    .parent = TYPE_CRYPTODEV_BACKEND,
+    .class_init = cryptodev_vhost_user_class_init,
+    .instance_init = cryptodev_vhost_user_instance_int,
+    .instance_finalize = cryptodev_vhost_user_finalize,
+    .instance_size = sizeof(CryptoDevBackendVhostUser),
+};
+
+static void
+cryptodev_vhost_user_register_types(void)
+{
+    type_register_static(&cryptodev_vhost_user_info);
+}
+
+type_init(cryptodev_vhost_user_register_types);
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
new file mode 100644
index 0000000..27e1c4a
--- /dev/null
+++ b/backends/cryptodev-vhost.c
@@ -0,0 +1,89 @@
+/*
+ * QEMU Cryptodev backend for QEMU cipher APIs
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *    Jay Zhou <jianjay.zhou@huawei.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/cryptodev-vhost.h"
+
+#ifdef CONFIG_VHOST_CRYPTO
+uint64_t
+cryptodev_vhost_get_max_queues(
+                        CryptoDevBackendVhost *crypto)
+{
+    return crypto->dev.max_queues;
+}
+
+void cryptodev_vhost_cleanup(CryptoDevBackendVhost *crypto)
+{
+    vhost_dev_cleanup(&crypto->dev);
+    g_free(crypto);
+}
+
+struct CryptoDevBackendVhost *
+cryptodev_vhost_init(
+             CryptoDevBackendVhostOptions *options)
+{
+    int r;
+    CryptoDevBackendVhost *crypto;
+
+    crypto = g_new(CryptoDevBackendVhost, 1);
+    crypto->dev.max_queues = 1;
+    crypto->dev.nvqs = 1;
+    crypto->dev.vqs = crypto->vqs;
+
+    crypto->cc = options->cc;
+
+    crypto->dev.protocol_features = 0;
+    crypto->backend = -1;
+
+    /* vhost-user needs vq_index to initiate a specific queue pair */
+    crypto->dev.vq_index = crypto->cc->queue_index * crypto->dev.nvqs;
+
+    r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type, 0);
+    if (r < 0) {
+        goto fail;
+    }
+
+    return crypto;
+fail:
+    g_free(crypto);
+    return NULL;
+}
+
+#else
+uint64_t
+cryptodev_vhost_get_max_queues(CryptoDevBackendVhost *crypto)
+{
+    return 0;
+}
+
+void cryptodev_vhost_cleanup(CryptoDevBackendVhost *crypto)
+{
+}
+
+struct CryptoDevBackendVhost *
+cryptodev_vhost_init(CryptoDevBackendVhostOptions *options)
+{
+    return NULL;
+}
+#endif
diff --git a/vl.c b/vl.c
index 32db91d..4ef4e67 100644
--- a/vl.c
+++ b/vl.c
@@ -2852,6 +2852,12 @@ static bool object_create_initial(const char *type)
         return false;
     }
 
+#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
+    if (g_str_equal(type, "cryptodev-vhost-user")) {
+        return false;
+    }
+#endif
+
     /*
      * return false for concrete netfilters since
      * they depend on netdevs already existing
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 67eeeba..efdcc5a 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -10,3 +10,9 @@ common-obj-y += cryptodev.o
 common-obj-y += cryptodev-builtin.o
 
 common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
+
+ifeq ($(CONFIG_VIRTIO),y)
+common-obj-$(CONFIG_LINUX) += cryptodev-vhost.o
+common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_LINUX)) += \
+    cryptodev-vhost-user.o
+endif
diff --git a/qemu-options.hx b/qemu-options.hx
index d15c171..cdc464c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4225,6 +4225,27 @@ which specify the queue number of cryptodev backend, the default of
    [...]
 @end example
 
+@item -object cryptodev-vhost-user,id=@var{id},chardev=@var{chardevid}[,queues=@var{queues}]
+
+Creates a vhost-user cryptodev backend, backed by a chardev @var{chardevid}.
+The @var{id} parameter is a unique ID that will be used to reference this
+cryptodev backend from the @option{virtio-crypto} device.
+The chardev should be a unix domain socket backed one. The vhost-user uses
+a specifically defined protocol to pass vhost ioctl replacement messages
+to an application on the other end of the socket.
+The @var{queues} parameter is optional, which specify the queue number
+of cryptodev backend for multiqueue vhost-user, the default of @var{queues} is 1.
+
+@example
+
+ # qemu-system-x86_64 \
+   [...] \
+       -chardev socket,id=chardev0,path=/path/to/socket \
+       -object cryptodev-vhost-user,id=cryptodev0,chardev=chardev0 \
+       -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
+   [...]
+@end example
+
 @item -object secret,id=@var{id},data=@var{string},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
 @item -object secret,id=@var{id},file=@var{filename},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 17/26] cryptodev-vhost-user: add crypto session handler
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (13 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 15/26] cryptodev: add vhost-user as a new cryptodev backend Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 16/26] cryptodev: add vhost support Michael S. Tsirkin
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gonglei, Longpeng, Jay Zhou

From: Gonglei <arei.gonglei@huawei.com>

Introduce two vhost-user meassges: VHOST_USER_CREATE_CRYPTO_SESSION
and VHOST_USER_CLOSE_CRYPTO_SESSION. At this point, the QEMU side
support crypto operation in cryptodev host-user backend.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/interop/vhost-user.txt       |  26 ++++++++++
 include/hw/virtio/vhost-backend.h |   8 +++
 backends/cryptodev-vhost-user.c   |  48 ++++++++++++++----
 hw/virtio/vhost-user.c            | 104 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 11 deletions(-)

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index 9fcf48d..cb3a759 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -368,6 +368,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_MTU            4
 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
+#define VHOST_USER_PROTOCOL_F_CRYPTO_SESSION 7
 
 Master message types
 --------------------
@@ -663,6 +664,31 @@ Master message types
       field, and slaves MUST NOT accept SET_CONFIG for read-only
       configuration space fields unless the live migration bit is set.
 
+* VHOST_USER_CREATE_CRYPTO_SESSION
+
+     Id: 26
+     Equivalent ioctl: N/A
+     Master payload: crypto session description
+     Slave payload: crypto session description
+
+     Create a session for crypto operation. The server side must return the
+     session id, 0 or positive for success, negative for failure.
+     This request should be sent only when VHOST_USER_PROTOCOL_F_CRYPTO_SESSION
+     feature has been successfully negotiated.
+     It's a required feature for crypto devices.
+
+* VHOST_USER_CLOSE_CRYPTO_SESSION
+
+     Id: 27
+     Equivalent ioctl: N/A
+     Master payload: u64
+
+     Close a session for crypto operation which was previously
+     created by VHOST_USER_CREATE_CRYPTO_SESSION.
+     This request should be sent only when VHOST_USER_PROTOCOL_F_CRYPTO_SESSION
+     feature has been successfully negotiated.
+     It's a required feature for crypto devices.
+
 Slave message types
 -------------------
 
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 592254f..5dac61f 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -95,6 +95,12 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *data,
 typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config,
                                    uint32_t config_len);
 
+typedef int (*vhost_crypto_create_session_op)(struct vhost_dev *dev,
+                                              void *session_info,
+                                              uint64_t *session_id);
+typedef int (*vhost_crypto_close_session_op)(struct vhost_dev *dev,
+                                             uint64_t session_id);
+
 typedef struct VhostOps {
     VhostBackendType backend_type;
     vhost_backend_init vhost_backend_init;
@@ -130,6 +136,8 @@ typedef struct VhostOps {
     vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg;
     vhost_get_config_op vhost_get_config;
     vhost_set_config_op vhost_set_config;
+    vhost_crypto_create_session_op vhost_crypto_create_session;
+    vhost_crypto_close_session_op vhost_crypto_close_session;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index 0b1f049..7bd0929 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -233,7 +233,25 @@ static int64_t cryptodev_vhost_user_sym_create_session(
            CryptoDevBackendSymSessionInfo *sess_info,
            uint32_t queue_index, Error **errp)
 {
-    return 0;
+    CryptoDevBackendClient *cc =
+                   backend->conf.peers.ccs[queue_index];
+    CryptoDevBackendVhost *vhost_crypto;
+    uint64_t session_id = 0;
+    int ret;
+
+    vhost_crypto = cryptodev_vhost_user_get_vhost(cc, backend, queue_index);
+    if (vhost_crypto) {
+        struct vhost_dev *dev = &(vhost_crypto->dev);
+        ret = dev->vhost_ops->vhost_crypto_create_session(dev,
+                                                          sess_info,
+                                                          &session_id);
+        if (ret < 0) {
+            return -1;
+        } else {
+            return session_id;
+        }
+    }
+    return -1;
 }
 
 static int cryptodev_vhost_user_sym_close_session(
@@ -241,15 +259,23 @@ static int cryptodev_vhost_user_sym_close_session(
            uint64_t session_id,
            uint32_t queue_index, Error **errp)
 {
-    return 0;
-}
-
-static int cryptodev_vhost_user_sym_operation(
-                 CryptoDevBackend *backend,
-                 CryptoDevBackendSymOpInfo *op_info,
-                 uint32_t queue_index, Error **errp)
-{
-    return VIRTIO_CRYPTO_OK;
+    CryptoDevBackendClient *cc =
+                  backend->conf.peers.ccs[queue_index];
+    CryptoDevBackendVhost *vhost_crypto;
+    int ret;
+
+    vhost_crypto = cryptodev_vhost_user_get_vhost(cc, backend, queue_index);
+    if (vhost_crypto) {
+        struct vhost_dev *dev = &(vhost_crypto->dev);
+        ret = dev->vhost_ops->vhost_crypto_close_session(dev,
+                                                         session_id);
+        if (ret < 0) {
+            return -1;
+        } else {
+            return 0;
+        }
+    }
+    return -1;
 }
 
 static void cryptodev_vhost_user_cleanup(
@@ -328,7 +354,7 @@ cryptodev_vhost_user_class_init(ObjectClass *oc, void *data)
     bc->cleanup = cryptodev_vhost_user_cleanup;
     bc->create_session = cryptodev_vhost_user_sym_create_session;
     bc->close_session = cryptodev_vhost_user_sym_close_session;
-    bc->do_sym_op = cryptodev_vhost_user_sym_operation;
+    bc->do_sym_op = NULL;
 }
 
 static const TypeInfo cryptodev_vhost_user_info = {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 6eb9798..41ff5cf 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -17,6 +17,7 @@
 #include "sysemu/kvm.h"
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
+#include "sysemu/cryptodev.h"
 
 #include <sys/ioctl.h>
 #include <sys/socket.h>
@@ -39,6 +40,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_NET_MTU = 4,
     VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
     VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
+    VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
 
     VHOST_USER_PROTOCOL_F_MAX
 };
@@ -72,6 +74,8 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ENDIAN = 23,
     VHOST_USER_GET_CONFIG = 24,
     VHOST_USER_SET_CONFIG = 25,
+    VHOST_USER_CREATE_CRYPTO_SESSION = 26,
+    VHOST_USER_CLOSE_CRYPTO_SESSION = 27,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -107,6 +111,17 @@ typedef struct VhostUserConfig {
     uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
 } VhostUserConfig;
 
+#define VHOST_CRYPTO_SYM_HMAC_MAX_KEY_LEN    512
+#define VHOST_CRYPTO_SYM_CIPHER_MAX_KEY_LEN  64
+
+typedef struct VhostUserCryptoSession {
+    /* session id for success, -1 on errors */
+    int64_t session_id;
+    CryptoDevBackendSymSessionInfo session_setup_data;
+    uint8_t key[VHOST_CRYPTO_SYM_CIPHER_MAX_KEY_LEN];
+    uint8_t auth_key[VHOST_CRYPTO_SYM_HMAC_MAX_KEY_LEN];
+} VhostUserCryptoSession;
+
 static VhostUserConfig c __attribute__ ((unused));
 #define VHOST_USER_CONFIG_HDR_SIZE (sizeof(c.offset) \
                                    + sizeof(c.size) \
@@ -132,6 +147,7 @@ typedef union {
         VhostUserLog log;
         struct vhost_iotlb_msg iotlb;
         VhostUserConfig config;
+        VhostUserCryptoSession session;
 } VhostUserPayload;
 
 typedef struct VhostUserMsg {
@@ -1054,6 +1070,92 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
     return 0;
 }
 
+static int vhost_user_crypto_create_session(struct vhost_dev *dev,
+                                            void *session_info,
+                                            uint64_t *session_id)
+{
+    bool crypto_session = virtio_has_feature(dev->protocol_features,
+                                       VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
+    CryptoDevBackendSymSessionInfo *sess_info = session_info;
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_CREATE_CRYPTO_SESSION,
+        .hdr.flags = VHOST_USER_VERSION,
+        .hdr.size = sizeof(msg.payload.session),
+    };
+
+    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+    if (!crypto_session) {
+        error_report("vhost-user trying to send unhandled ioctl");
+        return -1;
+    }
+
+    memcpy(&msg.payload.session.session_setup_data, sess_info,
+              sizeof(CryptoDevBackendSymSessionInfo));
+    if (sess_info->key_len) {
+        memcpy(&msg.payload.session.key, sess_info->cipher_key,
+               sess_info->key_len);
+    }
+    if (sess_info->auth_key_len > 0) {
+        memcpy(&msg.payload.session.auth_key, sess_info->auth_key,
+               sess_info->auth_key_len);
+    }
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        error_report("vhost_user_write() return -1, create session failed");
+        return -1;
+    }
+
+    if (vhost_user_read(dev, &msg) < 0) {
+        error_report("vhost_user_read() return -1, create session failed");
+        return -1;
+    }
+
+    if (msg.hdr.request != VHOST_USER_CREATE_CRYPTO_SESSION) {
+        error_report("Received unexpected msg type. Expected %d received %d",
+                     VHOST_USER_CREATE_CRYPTO_SESSION, msg.hdr.request);
+        return -1;
+    }
+
+    if (msg.hdr.size != sizeof(msg.payload.session)) {
+        error_report("Received bad msg size.");
+        return -1;
+    }
+
+    if (msg.payload.session.session_id < 0) {
+        error_report("Bad session id: %" PRId64 "",
+                              msg.payload.session.session_id);
+        return -1;
+    }
+    *session_id = msg.payload.session.session_id;
+
+    return 0;
+}
+
+static int
+vhost_user_crypto_close_session(struct vhost_dev *dev, uint64_t session_id)
+{
+    bool crypto_session = virtio_has_feature(dev->protocol_features,
+                                       VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_CLOSE_CRYPTO_SESSION,
+        .hdr.flags = VHOST_USER_VERSION,
+        .hdr.size = sizeof(msg.payload.u64),
+    };
+    msg.payload.u64 = session_id;
+
+    if (!crypto_session) {
+        error_report("vhost-user trying to send unhandled ioctl");
+        return -1;
+    }
+
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        error_report("vhost_user_write() return -1, close session failed");
+        return -1;
+    }
+
+    return 0;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -1082,4 +1184,6 @@ const VhostOps user_ops = {
         .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
         .vhost_get_config = vhost_user_get_config,
         .vhost_set_config = vhost_user_set_config,
+        .vhost_crypto_create_session = vhost_user_crypto_create_session,
+        .vhost_crypto_close_session = vhost_user_crypto_close_session,
 };
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 16/26] cryptodev: add vhost support
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (14 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 17/26] cryptodev-vhost-user: add crypto session handler Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 20/26] libvhost-user: Fix resource leak Michael S. Tsirkin
                   ` (10 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gonglei, Longpeng, Jay Zhou

From: Gonglei <arei.gonglei@huawei.com>

Impliment the vhost-crypto's funtions, such as startup,
stop and notification etc. Introduce an enum
QCryptoCryptoDevBackendOptionsType in order to
identify the cryptodev vhost backend is vhost-user
or vhost-kernel-module (If exist).

At this point, the cryptdoev-vhost-user works.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-crypto.h     |   1 +
 include/sysemu/cryptodev-vhost-user.h |  44 ++++++
 include/sysemu/cryptodev.h            |   8 ++
 backends/cryptodev-builtin.c          |   1 +
 backends/cryptodev-vhost-user.c       |  16 +++
 backends/cryptodev-vhost.c            | 258 ++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-crypto.c             |  70 +++++++++
 hw/virtio/Makefile.objs               |   2 +-
 8 files changed, 399 insertions(+), 1 deletion(-)
 create mode 100644 include/sysemu/cryptodev-vhost-user.h

diff --git a/include/hw/virtio/virtio-crypto.h b/include/hw/virtio/virtio-crypto.h
index a00a0bf..ca3a049 100644
--- a/include/hw/virtio/virtio-crypto.h
+++ b/include/hw/virtio/virtio-crypto.h
@@ -96,6 +96,7 @@ typedef struct VirtIOCrypto {
     int multiqueue;
     uint32_t curr_queues;
     size_t config_size;
+    uint8_t vhost_started;
 } VirtIOCrypto;
 
 #endif /* _QEMU_VIRTIO_CRYPTO_H */
diff --git a/include/sysemu/cryptodev-vhost-user.h b/include/sysemu/cryptodev-vhost-user.h
new file mode 100644
index 0000000..937217b
--- /dev/null
+++ b/include/sysemu/cryptodev-vhost-user.h
@@ -0,0 +1,44 @@
+/*
+ * QEMU Crypto Device Common Vhost User Implement
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#ifndef CRYPTODEV_VHOST_USER_H
+#define CRYPTODEV_VHOST_USER_H
+
+
+/**
+ * cryptodev_vhost_user_get_vhost:
+ * @cc: the client object for each queue
+ * @b: the cryptodev backend common vhost object
+ * @queue: the queue index
+ *
+ * Gets a new cryptodev backend common vhost object based on
+ * @b and @queue
+ *
+ * Returns: the cryptodev backend common vhost object
+ */
+CryptoDevBackendVhost *
+cryptodev_vhost_user_get_vhost(
+                         CryptoDevBackendClient *cc,
+                         CryptoDevBackend *b,
+                         uint16_t queue);
+
+#endif /* CRYPTODEV_VHOST_USER_H */
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index a9d0d1e..faeb6f8 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -163,12 +163,20 @@ typedef struct CryptoDevBackendClass {
                      uint32_t queue_index, Error **errp);
 } CryptoDevBackendClass;
 
+typedef enum CryptoDevBackendOptionsType {
+    CRYPTODEV_BACKEND_TYPE_NONE = 0,
+    CRYPTODEV_BACKEND_TYPE_BUILTIN = 1,
+    CRYPTODEV_BACKEND_TYPE_VHOST_USER = 2,
+    CRYPTODEV_BACKEND_TYPE__MAX,
+} CryptoDevBackendOptionsType;
 
 struct CryptoDevBackendClient {
+    CryptoDevBackendOptionsType type;
     char *model;
     char *name;
     char *info_str;
     unsigned int queue_index;
+    int vring_enable;
     QTAILQ_ENTRY(CryptoDevBackendClient) next;
 };
 
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 657c0ba..9fb0bd5 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -78,6 +78,7 @@ static void cryptodev_builtin_init(
               "cryptodev-builtin", NULL);
     cc->info_str = g_strdup_printf("cryptodev-builtin0");
     cc->queue_index = 0;
+    cc->type = CRYPTODEV_BACKEND_TYPE_BUILTIN;
     backend->conf.peers.ccs[0] = cc;
 
     backend->conf.crypto_services =
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index 4e63ece..0b1f049 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -29,6 +29,7 @@
 #include "standard-headers/linux/virtio_crypto.h"
 #include "sysemu/cryptodev-vhost.h"
 #include "chardev/char-fe.h"
+#include "sysemu/cryptodev-vhost-user.h"
 
 
 /**
@@ -58,6 +59,20 @@ cryptodev_vhost_user_running(
     return crypto ? 1 : 0;
 }
 
+CryptoDevBackendVhost *
+cryptodev_vhost_user_get_vhost(
+                         CryptoDevBackendClient *cc,
+                         CryptoDevBackend *b,
+                         uint16_t queue)
+{
+    CryptoDevBackendVhostUser *s =
+                      CRYPTODEV_BACKEND_VHOST_USER(b);
+    assert(cc->type == CRYPTODEV_BACKEND_TYPE_VHOST_USER);
+    assert(queue < MAX_CRYPTO_QUEUE_NUM);
+
+    return s->vhost_crypto[queue];
+}
+
 static void cryptodev_vhost_user_stop(int queues,
                           CryptoDevBackendVhostUser *s)
 {
@@ -190,6 +205,7 @@ static void cryptodev_vhost_user_init(
         cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
                                        i, chr->label);
         cc->queue_index = i;
+        cc->type = CRYPTODEV_BACKEND_TYPE_VHOST_USER;
 
         backend->conf.peers.ccs[i] = cc;
 
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index 27e1c4a..8337c9a 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -23,9 +23,16 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/virtio/virtio-bus.h"
 #include "sysemu/cryptodev-vhost.h"
 
 #ifdef CONFIG_VHOST_CRYPTO
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
+#include "hw/virtio/virtio-crypto.h"
+#include "sysemu/cryptodev-vhost-user.h"
+
 uint64_t
 cryptodev_vhost_get_max_queues(
                         CryptoDevBackendVhost *crypto)
@@ -70,6 +77,228 @@ fail:
     return NULL;
 }
 
+static int
+cryptodev_vhost_start_one(CryptoDevBackendVhost *crypto,
+                                  VirtIODevice *dev)
+{
+    int r;
+
+    crypto->dev.nvqs = 1;
+    crypto->dev.vqs = crypto->vqs;
+
+    r = vhost_dev_enable_notifiers(&crypto->dev, dev);
+    if (r < 0) {
+        goto fail_notifiers;
+    }
+
+    r = vhost_dev_start(&crypto->dev, dev);
+    if (r < 0) {
+        goto fail_start;
+    }
+
+    return 0;
+
+fail_start:
+    vhost_dev_disable_notifiers(&crypto->dev, dev);
+fail_notifiers:
+    return r;
+}
+
+static void
+cryptodev_vhost_stop_one(CryptoDevBackendVhost *crypto,
+                                 VirtIODevice *dev)
+{
+    vhost_dev_stop(&crypto->dev, dev);
+    vhost_dev_disable_notifiers(&crypto->dev, dev);
+}
+
+CryptoDevBackendVhost *
+cryptodev_get_vhost(CryptoDevBackendClient *cc,
+                            CryptoDevBackend *b,
+                            uint16_t queue)
+{
+    CryptoDevBackendVhost *vhost_crypto = NULL;
+
+    if (!cc) {
+        return NULL;
+    }
+
+    switch (cc->type) {
+#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
+    case CRYPTODEV_BACKEND_TYPE_VHOST_USER:
+        vhost_crypto = cryptodev_vhost_user_get_vhost(cc, b, queue);
+        break;
+#endif
+    default:
+        break;
+    }
+
+    return vhost_crypto;
+}
+
+static void
+cryptodev_vhost_set_vq_index(CryptoDevBackendVhost *crypto,
+                                     int vq_index)
+{
+    crypto->dev.vq_index = vq_index;
+}
+
+static int
+vhost_set_vring_enable(CryptoDevBackendClient *cc,
+                            CryptoDevBackend *b,
+                            uint16_t queue, int enable)
+{
+    CryptoDevBackendVhost *crypto =
+                       cryptodev_get_vhost(cc, b, queue);
+    const VhostOps *vhost_ops;
+
+    cc->vring_enable = enable;
+
+    if (!crypto) {
+        return 0;
+    }
+
+    vhost_ops = crypto->dev.vhost_ops;
+    if (vhost_ops->vhost_set_vring_enable) {
+        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
+    }
+
+    return 0;
+}
+
+int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+    int r, e;
+    int i;
+    CryptoDevBackend *b = vcrypto->cryptodev;
+    CryptoDevBackendVhost *vhost_crypto;
+    CryptoDevBackendClient *cc;
+
+    if (!k->set_guest_notifiers) {
+        error_report("binding does not support guest notifiers");
+        return -ENOSYS;
+    }
+
+    for (i = 0; i < total_queues; i++) {
+        cc = b->conf.peers.ccs[i];
+
+        vhost_crypto = cryptodev_get_vhost(cc, b, i);
+        cryptodev_vhost_set_vq_index(vhost_crypto, i);
+
+        /* Suppress the masking guest notifiers on vhost user
+         * because vhost user doesn't interrupt masking/unmasking
+         * properly.
+         */
+        if (cc->type == CRYPTODEV_BACKEND_TYPE_VHOST_USER) {
+            dev->use_guest_notifier_mask = false;
+        }
+     }
+
+    r = k->set_guest_notifiers(qbus->parent, total_queues, true);
+    if (r < 0) {
+        error_report("error binding guest notifier: %d", -r);
+        goto err;
+    }
+
+    for (i = 0; i < total_queues; i++) {
+        cc = b->conf.peers.ccs[i];
+
+        vhost_crypto = cryptodev_get_vhost(cc, b, i);
+        r = cryptodev_vhost_start_one(vhost_crypto, dev);
+
+        if (r < 0) {
+            goto err_start;
+        }
+
+        if (cc->vring_enable) {
+            /* restore vring enable state */
+            r = vhost_set_vring_enable(cc, b, i, cc->vring_enable);
+
+            if (r < 0) {
+                goto err_start;
+            }
+        }
+    }
+
+    return 0;
+
+err_start:
+    while (--i >= 0) {
+        cc = b->conf.peers.ccs[i];
+        vhost_crypto = cryptodev_get_vhost(cc, b, i);
+        cryptodev_vhost_stop_one(vhost_crypto, dev);
+    }
+    e = k->set_guest_notifiers(qbus->parent, total_queues, false);
+    if (e < 0) {
+        error_report("vhost guest notifier cleanup failed: %d", e);
+    }
+err:
+    return r;
+}
+
+void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
+    VirtioBusState *vbus = VIRTIO_BUS(qbus);
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    CryptoDevBackend *b = vcrypto->cryptodev;
+    CryptoDevBackendVhost *vhost_crypto;
+    CryptoDevBackendClient *cc;
+    size_t i;
+    int r;
+
+    for (i = 0; i < total_queues; i++) {
+        cc = b->conf.peers.ccs[i];
+
+        vhost_crypto = cryptodev_get_vhost(cc, b, i);
+        cryptodev_vhost_stop_one(vhost_crypto, dev);
+    }
+
+    r = k->set_guest_notifiers(qbus->parent, total_queues, false);
+    if (r < 0) {
+        error_report("vhost guest notifier cleanup failed: %d", r);
+    }
+    assert(r >= 0);
+}
+
+void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
+                                           int queue,
+                                           int idx, bool mask)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    CryptoDevBackend *b = vcrypto->cryptodev;
+    CryptoDevBackendVhost *vhost_crypto;
+    CryptoDevBackendClient *cc;
+
+    assert(queue < MAX_CRYPTO_QUEUE_NUM);
+
+    cc = b->conf.peers.ccs[queue];
+    vhost_crypto = cryptodev_get_vhost(cc, b, queue);
+
+    vhost_virtqueue_mask(&vhost_crypto->dev, dev, idx, mask);
+}
+
+bool cryptodev_vhost_virtqueue_pending(VirtIODevice *dev,
+                                              int queue, int idx)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    CryptoDevBackend *b = vcrypto->cryptodev;
+    CryptoDevBackendVhost *vhost_crypto;
+    CryptoDevBackendClient *cc;
+
+    assert(queue < MAX_CRYPTO_QUEUE_NUM);
+
+    cc = b->conf.peers.ccs[queue];
+    vhost_crypto = cryptodev_get_vhost(cc, b, queue);
+
+    return vhost_virtqueue_pending(&vhost_crypto->dev, idx);
+}
+
 #else
 uint64_t
 cryptodev_vhost_get_max_queues(CryptoDevBackendVhost *crypto)
@@ -86,4 +315,33 @@ cryptodev_vhost_init(CryptoDevBackendVhostOptions *options)
 {
     return NULL;
 }
+
+CryptoDevBackendVhost *
+cryptodev_get_vhost(CryptoDevBackendClient *cc,
+                    CryptoDevBackend *b,
+                    uint16_t queue)
+{
+    return NULL;
+}
+
+int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
+{
+    return -1;
+}
+
+void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues)
+{
+}
+
+void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
+                                    int queue,
+                                    int idx, bool mask)
+{
+}
+
+bool cryptodev_vhost_virtqueue_pending(VirtIODevice *dev,
+                                       int queue, int idx)
+{
+    return false;
+}
 #endif
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 19c82e0..9a9fa49 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -20,6 +20,7 @@
 #include "hw/virtio/virtio-crypto.h"
 #include "hw/virtio/virtio-access.h"
 #include "standard-headers/linux/virtio_ids.h"
+#include "sysemu/cryptodev-vhost.h"
 
 #define VIRTIO_CRYPTO_VM_VERSION 1
 
@@ -880,6 +881,72 @@ static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(config, &crypto_cfg, c->config_size);
 }
 
+static bool virtio_crypto_started(VirtIOCrypto *c, uint8_t status)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(c);
+    return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+        (c->status & VIRTIO_CRYPTO_S_HW_READY) && vdev->vm_running;
+}
+
+static void virtio_crypto_vhost_status(VirtIOCrypto *c, uint8_t status)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(c);
+    int queues = c->multiqueue ? c->max_queues : 1;
+    CryptoDevBackend *b = c->cryptodev;
+    CryptoDevBackendClient *cc = b->conf.peers.ccs[0];
+
+    if (!cryptodev_get_vhost(cc, b, 0)) {
+        return;
+    }
+
+    if ((virtio_crypto_started(c, status)) == !!c->vhost_started) {
+        return;
+    }
+
+    if (!c->vhost_started) {
+        int r;
+
+        c->vhost_started = 1;
+        r = cryptodev_vhost_start(vdev, queues);
+        if (r < 0) {
+            error_report("unable to start vhost crypto: %d: "
+                         "falling back on userspace virtio", -r);
+            c->vhost_started = 0;
+        }
+    } else {
+        cryptodev_vhost_stop(vdev, queues);
+        c->vhost_started = 0;
+    }
+}
+
+static void virtio_crypto_set_status(VirtIODevice *vdev, uint8_t status)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+
+    virtio_crypto_vhost_status(vcrypto, status);
+}
+
+static void virtio_crypto_guest_notifier_mask(VirtIODevice *vdev, int idx,
+                                           bool mask)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+    int queue = virtio_crypto_vq2q(idx);
+
+    assert(vcrypto->vhost_started);
+
+    cryptodev_vhost_virtqueue_mask(vdev, queue, idx, mask);
+}
+
+static bool virtio_crypto_guest_notifier_pending(VirtIODevice *vdev, int idx)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+    int queue = virtio_crypto_vq2q(idx);
+
+    assert(vcrypto->vhost_started);
+
+    return cryptodev_vhost_virtqueue_pending(vdev, queue, idx);
+}
+
 static void virtio_crypto_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -893,6 +960,9 @@ static void virtio_crypto_class_init(ObjectClass *klass, void *data)
     vdc->get_config = virtio_crypto_get_config;
     vdc->get_features = virtio_crypto_get_features;
     vdc->reset = virtio_crypto_reset;
+    vdc->set_status = virtio_crypto_set_status;
+    vdc->guest_notifier_mask = virtio_crypto_guest_notifier_mask;
+    vdc->guest_notifier_pending = virtio_crypto_guest_notifier_pending;
 }
 
 static void virtio_crypto_instance_init(Object *obj)
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 765d363..c65dca2 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -7,7 +7,7 @@ common-obj-y += virtio-mmio.o
 obj-y += virtio.o virtio-balloon.o 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
-obj-y += virtio-crypto.o
+obj-$(CONFIG_LINUX) += virtio-crypto.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
 endif
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 20/26] libvhost-user: Fix resource leak
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (15 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 16/26] cryptodev: add vhost support Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 19/26] virtio-balloon: unref the memory region before continuing Michael S. Tsirkin
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Yongji Xie, Yongji Xie, Marc-André Lureau,
	Maxime Coquelin, Dr. David Alan Gilbert,
	Philippe Mathieu-Daudé

From: Yongji Xie <elohimes@gmail.com>

Free the mmaped memory when we need to mmap new memory
space on vu_set_mem_table_exec() and vu_set_log_base_exec() to
avoid memory leak.

Also close the corresponding fd after mmap() on
vu_set_log_base_exec() to avoid fd leak.

Signed-off-by: Yongji Xie <xieyongji@baidu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 contrib/libvhost-user/libvhost-user.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 27cc597..54dbc93 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -407,6 +407,15 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
     int i;
     VhostUserMemory *memory = &vmsg->payload.memory;
+
+    for (i = 0; i < dev->nregions; i++) {
+        VuDevRegion *r = &dev->regions[i];
+        void *m = (void *) (uintptr_t) r->mmap_addr;
+
+        if (m) {
+            munmap(m, r->size + r->mmap_offset);
+        }
+    }
     dev->nregions = memory->nregions;
 
     DPRINT("Nregions: %d\n", memory->nregions);
@@ -472,9 +481,14 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
 
     rc = mmap(0, log_mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
               log_mmap_offset);
+    close(fd);
     if (rc == MAP_FAILED) {
         perror("log mmap error");
     }
+
+    if (dev->log_table) {
+        munmap(dev->log_table, dev->log_size);
+    }
     dev->log_table = rc;
     dev->log_size = log_mmap_size;
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 19/26] virtio-balloon: unref the memory region before continuing
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (16 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 20/26] libvhost-user: Fix resource leak Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 18/26] cryptodev-vhost-user: set the key length Michael S. Tsirkin
                   ` (8 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Tiwei Bie, qemu-stable

From: Tiwei Bie <tiwei.bie@intel.com>

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 14e08d2..f2104bf 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -234,6 +234,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
                 memory_region_is_rom(section.mr) ||
                 memory_region_is_romd(section.mr)) {
                 trace_virtio_balloon_bad_addr(pa);
+                memory_region_unref(section.mr);
                 continue;
             }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 18/26] cryptodev-vhost-user: set the key length
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (17 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 19/26] virtio-balloon: unref the memory region before continuing Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 21/26] libvhost-user: Support across-memory-boundary access Michael S. Tsirkin
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gonglei

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/sysemu/cryptodev-vhost-user.h | 3 +++
 backends/cryptodev-vhost-user.c       | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/include/sysemu/cryptodev-vhost-user.h b/include/sysemu/cryptodev-vhost-user.h
index 937217b..6debf53 100644
--- a/include/sysemu/cryptodev-vhost-user.h
+++ b/include/sysemu/cryptodev-vhost-user.h
@@ -23,6 +23,9 @@
 #ifndef CRYPTODEV_VHOST_USER_H
 #define CRYPTODEV_VHOST_USER_H
 
+#define VHOST_USER_MAX_AUTH_KEY_LEN    512
+#define VHOST_USER_MAX_CIPHER_KEY_LEN  64
+
 
 /**
  * cryptodev_vhost_user_get_vhost:
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index 7bd0929..11b834b 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -226,6 +226,10 @@ static void cryptodev_vhost_user_init(
                          1u << VIRTIO_CRYPTO_SERVICE_MAC;
     backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
     backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
+
+    backend->conf.max_size = UINT64_MAX;
+    backend->conf.max_cipher_key_len = VHOST_USER_MAX_AUTH_KEY_LEN;
+    backend->conf.max_auth_key_len = VHOST_USER_MAX_AUTH_KEY_LEN;
 }
 
 static int64_t cryptodev_vhost_user_sym_create_session(
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 21/26] libvhost-user: Support across-memory-boundary access
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (18 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 18/26] cryptodev-vhost-user: set the key length Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 22/26] hw/pci-bridge: fix pcie root port's IO hints capability Michael S. Tsirkin
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Yongji Xie, Yongji Xie, Maxime Coquelin,
	Marc-André Lureau, Dr. David Alan Gilbert,
	Philippe Mathieu-Daudé,
	Paolo Bonzini

From: Yongji Xie <elohimes@gmail.com>

The sg list/indirect descriptor table may be contigious
in GPA but not in HVA address space. But libvhost-user
wasn't aware of that. This would cause out-of-bounds
access. Even a malicious guest could use it to get
information from the vhost-user backend.

Introduce a plen parameter in vu_gpa_to_va() so we can
handle this case, returning the actual mapped length.

Signed-off-by: Yongji Xie <xieyongji@baidu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 contrib/libvhost-user/libvhost-user.h |   3 +-
 contrib/libvhost-user/libvhost-user.c | 133 ++++++++++++++++++++++++++++++----
 2 files changed, 122 insertions(+), 14 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h
index f8a730b..18f95f6 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -327,11 +327,12 @@ bool vu_dispatch(VuDev *dev);
 /**
  * vu_gpa_to_va:
  * @dev: a VuDev context
+ * @plen: guest memory size
  * @guest_addr: guest address
  *
  * Translate a guest address to a pointer. Returns NULL on failure.
  */
-void *vu_gpa_to_va(VuDev *dev, uint64_t guest_addr);
+void *vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr);
 
 /**
  * vu_get_queue:
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 54dbc93..2e358b5 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -118,15 +118,22 @@ vu_panic(VuDev *dev, const char *msg, ...)
 
 /* Translate guest physical address to our virtual address.  */
 void *
-vu_gpa_to_va(VuDev *dev, uint64_t guest_addr)
+vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr)
 {
     int i;
 
+    if (*plen == 0) {
+        return NULL;
+    }
+
     /* Find matching memory region.  */
     for (i = 0; i < dev->nregions; i++) {
         VuDevRegion *r = &dev->regions[i];
 
         if ((guest_addr >= r->gpa) && (guest_addr < (r->gpa + r->size))) {
+            if ((guest_addr + *plen) > (r->gpa + r->size)) {
+                *plen = r->gpa + r->size - guest_addr;
+            }
             return (void *)(uintptr_t)
                 guest_addr - r->gpa + r->mmap_addr + r->mmap_offset;
         }
@@ -1116,6 +1123,37 @@ virtqueue_get_head(VuDev *dev, VuVirtq *vq,
     return true;
 }
 
+static int
+virtqueue_read_indirect_desc(VuDev *dev, struct vring_desc *desc,
+                             uint64_t addr, size_t len)
+{
+    struct vring_desc *ori_desc;
+    uint64_t read_len;
+
+    if (len > (VIRTQUEUE_MAX_SIZE * sizeof(struct vring_desc))) {
+        return -1;
+    }
+
+    if (len == 0) {
+        return -1;
+    }
+
+    while (len) {
+        read_len = len;
+        ori_desc = vu_gpa_to_va(dev, &read_len, addr);
+        if (!ori_desc) {
+            return -1;
+        }
+
+        memcpy(desc, ori_desc, read_len);
+        len -= read_len;
+        addr += read_len;
+        desc += read_len;
+    }
+
+    return 0;
+}
+
 enum {
     VIRTQUEUE_READ_DESC_ERROR = -1,
     VIRTQUEUE_READ_DESC_DONE = 0,   /* end of chain */
@@ -1162,8 +1200,10 @@ vu_queue_get_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int *in_bytes,
     }
 
     while ((rc = virtqueue_num_heads(dev, vq, idx)) > 0) {
-        unsigned int max, num_bufs, indirect = 0;
+        unsigned int max, desc_len, num_bufs, indirect = 0;
+        uint64_t desc_addr, read_len;
         struct vring_desc *desc;
+        struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE];
         unsigned int i;
 
         max = vq->vring.num;
@@ -1187,8 +1227,24 @@ vu_queue_get_avail_bytes(VuDev *dev, VuVirtq *vq, unsigned int *in_bytes,
 
             /* loop over the indirect descriptor table */
             indirect = 1;
-            max = desc[i].len / sizeof(struct vring_desc);
-            desc = vu_gpa_to_va(dev, desc[i].addr);
+            desc_addr = desc[i].addr;
+            desc_len = desc[i].len;
+            max = desc_len / sizeof(struct vring_desc);
+            read_len = desc_len;
+            desc = vu_gpa_to_va(dev, &read_len, desc_addr);
+            if (unlikely(desc && read_len != desc_len)) {
+                /* Failed to use zero copy */
+                desc = NULL;
+                if (!virtqueue_read_indirect_desc(dev, desc_buf,
+                                                  desc_addr,
+                                                  desc_len)) {
+                    desc = desc_buf;
+                }
+            }
+            if (!desc) {
+                vu_panic(dev, "Invalid indirect buffer table");
+                goto err;
+            }
             num_bufs = i = 0;
         }
 
@@ -1386,9 +1442,24 @@ virtqueue_map_desc(VuDev *dev,
         return;
     }
 
-    iov[num_sg].iov_base = vu_gpa_to_va(dev, pa);
-    iov[num_sg].iov_len = sz;
-    num_sg++;
+    while (sz) {
+        uint64_t len = sz;
+
+        if (num_sg == max_num_sg) {
+            vu_panic(dev, "virtio: too many descriptors in indirect table");
+            return;
+        }
+
+        iov[num_sg].iov_base = vu_gpa_to_va(dev, &len, pa);
+        if (iov[num_sg].iov_base == NULL) {
+            vu_panic(dev, "virtio: invalid address for buffers");
+            return;
+        }
+        iov[num_sg].iov_len = len;
+        num_sg++;
+        sz -= len;
+        pa += len;
+    }
 
     *p_num_sg = num_sg;
 }
@@ -1420,10 +1491,12 @@ virtqueue_alloc_element(size_t sz,
 void *
 vu_queue_pop(VuDev *dev, VuVirtq *vq, size_t sz)
 {
-    unsigned int i, head, max;
+    unsigned int i, head, max, desc_len;
+    uint64_t desc_addr, read_len;
     VuVirtqElement *elem;
     unsigned out_num, in_num;
     struct iovec iov[VIRTQUEUE_MAX_SIZE];
+    struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE];
     struct vring_desc *desc;
     int rc;
 
@@ -1464,8 +1537,24 @@ vu_queue_pop(VuDev *dev, VuVirtq *vq, size_t sz)
         }
 
         /* loop over the indirect descriptor table */
-        max = desc[i].len / sizeof(struct vring_desc);
-        desc = vu_gpa_to_va(dev, desc[i].addr);
+        desc_addr = desc[i].addr;
+        desc_len = desc[i].len;
+        max = desc_len / sizeof(struct vring_desc);
+        read_len = desc_len;
+        desc = vu_gpa_to_va(dev, &read_len, desc_addr);
+        if (unlikely(desc && read_len != desc_len)) {
+            /* Failed to use zero copy */
+            desc = NULL;
+            if (!virtqueue_read_indirect_desc(dev, desc_buf,
+                                              desc_addr,
+                                              desc_len)) {
+                desc = desc_buf;
+            }
+        }
+        if (!desc) {
+            vu_panic(dev, "Invalid indirect buffer table");
+            return NULL;
+        }
         i = 0;
     }
 
@@ -1541,7 +1630,9 @@ vu_log_queue_fill(VuDev *dev, VuVirtq *vq,
                   unsigned int len)
 {
     struct vring_desc *desc = vq->vring.desc;
-    unsigned int i, max, min;
+    unsigned int i, max, min, desc_len;
+    uint64_t desc_addr, read_len;
+    struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE];
     unsigned num_bufs = 0;
 
     max = vq->vring.num;
@@ -1553,8 +1644,24 @@ vu_log_queue_fill(VuDev *dev, VuVirtq *vq,
         }
 
         /* loop over the indirect descriptor table */
-        max = desc[i].len / sizeof(struct vring_desc);
-        desc = vu_gpa_to_va(dev, desc[i].addr);
+        desc_addr = desc[i].addr;
+        desc_len = desc[i].len;
+        max = desc_len / sizeof(struct vring_desc);
+        read_len = desc_len;
+        desc = vu_gpa_to_va(dev, &read_len, desc_addr);
+        if (unlikely(desc && read_len != desc_len)) {
+            /* Failed to use zero copy */
+            desc = NULL;
+            if (!virtqueue_read_indirect_desc(dev, desc_buf,
+                                              desc_addr,
+                                              desc_len)) {
+                desc = desc_buf;
+            }
+        }
+        if (!desc) {
+            vu_panic(dev, "Invalid indirect buffer table");
+            return;
+        }
         i = 0;
     }
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 22/26] hw/pci-bridge: fix pcie root port's IO hints capability
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (19 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 21/26] libvhost-user: Support across-memory-boundary access Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 23/26] tests: acpi: fix FADT not being compared to reference table Michael S. Tsirkin
                   ` (5 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Marcel Apfelbaum

From: Marcel Apfelbaum <marcel@redhat.com>

The gen_pcie_root_port mem-reserve and pref32-reserve properties are
defined as size (so uint64_t), but passed as uint32_t when building
the 'IO hints' vendor specific capability.
Passing 4G (or more) gets truncated and passed as a zero reservation.
Is not a huge issue since the guest firmware will always compare the
hints with the default value and take the maximum.

Fix it by passing the values as uint64_t and failing to init the
gen_pcie_root_port id invalid values are used.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci_bridge.h |  4 ++--
 hw/pci/pci_bridge.c         | 24 +++++++++++++++++++-----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 9b44ffd..0347da5 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -135,8 +135,8 @@ typedef struct PCIBridgeQemuCap {
 
 int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
                               uint32_t bus_reserve, uint64_t io_reserve,
-                              uint32_t mem_non_pref_reserve,
-                              uint32_t mem_pref_32_reserve,
+                              uint64_t mem_non_pref_reserve,
+                              uint64_t mem_pref_32_reserve,
                               uint64_t mem_pref_64_reserve,
                               Error **errp);
 
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index b2e50c3..40a39f5 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -412,22 +412,36 @@ void pci_bridge_map_irq(PCIBridge *br, const char* bus_name,
 
 int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
                                      uint32_t bus_reserve, uint64_t io_reserve,
-                                     uint32_t mem_non_pref_reserve,
-                                     uint32_t mem_pref_32_reserve,
+                                     uint64_t mem_non_pref_reserve,
+                                     uint64_t mem_pref_32_reserve,
                                      uint64_t mem_pref_64_reserve,
                                      Error **errp)
 {
-    if (mem_pref_32_reserve != (uint32_t)-1 &&
+    if (mem_pref_32_reserve != (uint64_t)-1 &&
         mem_pref_64_reserve != (uint64_t)-1) {
         error_setg(errp,
                    "PCI resource reserve cap: PREF32 and PREF64 conflict");
         return -EINVAL;
     }
 
+    if (mem_non_pref_reserve != (uint64_t)-1 &&
+        mem_non_pref_reserve >= (1ULL << 32)) {
+        error_setg(errp,
+                   "PCI resource reserve cap: mem-reserve must be less than 4G");
+        return -EINVAL;
+    }
+
+    if (mem_pref_32_reserve != (uint64_t)-1 &&
+        mem_pref_32_reserve >= (1ULL << 32)) {
+        error_setg(errp,
+                   "PCI resource reserve cap: pref32-reserve  must be less than 4G");
+        return -EINVAL;
+    }
+
     if (bus_reserve == (uint32_t)-1 &&
         io_reserve == (uint64_t)-1 &&
-        mem_non_pref_reserve == (uint32_t)-1 &&
-        mem_pref_32_reserve == (uint32_t)-1 &&
+        mem_non_pref_reserve == (uint64_t)-1 &&
+        mem_pref_32_reserve == (uint64_t)-1 &&
         mem_pref_64_reserve == (uint64_t)-1) {
         return 0;
     }
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 23/26] tests: acpi: fix FADT not being compared to reference table
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (20 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 22/26] hw/pci-bridge: fix pcie root port's IO hints capability Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 25/26] acpi-test: update FADT Michael S. Tsirkin
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Igor Mammedov

From: Igor Mammedov <imammedo@redhat.com>

It turns out that FADT isn't actually tested for changes
against reference table, since it happens to be the 1st
table in RSDT which is currently ignored.
Fix it by making sure that all tables from RSDT are added
to test list.

NOTE: FADT contains guest allocated pointers to FACS/DSDT,
zero them out so that possible FACS/DSDT address change
won't affect test results.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index b354aaa..2b332ed 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -194,6 +194,35 @@ static void test_acpi_fadt_table(test_data *data)
                                  le32_to_cpu(fadt_table->length)));
 }
 
+static void sanitize_fadt_ptrs(test_data *data)
+{
+    /* fixup pointers in FADT */
+    int i;
+
+    for (i = 0; i < data->tables->len; i++) {
+        AcpiSdtTable *sdt = &g_array_index(data->tables, AcpiSdtTable, i);
+
+        if (memcmp(&sdt->header.signature, "FACP", 4)) {
+            continue;
+        }
+
+        /* sdt->aml field offset := spec offset - header size */
+        memset(sdt->aml + 0, 0, 4); /* sanitize FIRMWARE_CTRL(36) ptr */
+        memset(sdt->aml + 4, 0, 4); /* sanitize DSDT(40) ptr */
+        if (sdt->header.revision >= 3) {
+            memset(sdt->aml + 96, 0, 8); /* sanitize X_FIRMWARE_CTRL(132) ptr */
+            memset(sdt->aml + 104, 0, 8); /* sanitize X_DSDT(140) ptr */
+        }
+
+        /* update checksum */
+        sdt->header.checksum = 0;
+        sdt->header.checksum -=
+            acpi_calc_checksum((uint8_t *)sdt, sizeof(AcpiTableHeader)) +
+            acpi_calc_checksum((uint8_t *)sdt->aml, sdt->aml_len);
+        break;
+    }
+}
+
 static void test_acpi_facs_table(test_data *data)
 {
     AcpiFacsDescriptorRev1 *facs_table = &data->facs_table;
@@ -248,14 +277,14 @@ static void test_acpi_dsdt_table(test_data *data)
 /* Load all tables and add to test list directly RSDT referenced tables */
 static void fetch_rsdt_referenced_tables(test_data *data)
 {
-    int tables_nr = data->rsdt_tables_nr - 1; /* fadt is first */
+    int tables_nr = data->rsdt_tables_nr;
     int i;
 
     for (i = 0; i < tables_nr; i++) {
         AcpiSdtTable ssdt_table;
         uint32_t addr;
 
-        addr = le32_to_cpu(data->rsdt_tables_addr[i + 1]); /* fadt is first */
+        addr = le32_to_cpu(data->rsdt_tables_addr[i]);
         fetch_table(&ssdt_table, addr);
 
         /* Add table to ASL test tables list */
@@ -650,6 +679,8 @@ static void test_acpi_one(const char *params, test_data *data)
     test_acpi_dsdt_table(data);
     fetch_rsdt_referenced_tables(data);
 
+    sanitize_fadt_ptrs(data);
+
     if (iasl) {
         if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
             dump_aml_files(data, true);
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 25/26] acpi-test: update FADT
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (21 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 23/26] tests: acpi: fix FADT not being compared to reference table Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 24/26] lpc: drop pcie host dependency Michael S. Tsirkin
                   ` (3 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

Previous commit ("tests: acpi: fix FADT not being compared to reference table")
started tracking changes to the FADT. Generate the expected FACP files -
apparently these weren't updated since 2013.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/FACP  | Bin 116 -> 116 bytes
 tests/acpi-test-data/q35/FACP | Bin 116 -> 244 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/acpi-test-data/pc/FACP b/tests/acpi-test-data/pc/FACP
index 0639999ed1f748d44977f88e63d6a0ab49add040..261ebdc5d1c3bdf18fb7935314a04fd7f6f92a7a 100644
GIT binary patch
delta 70
zcmXRZ;c|0y4k%$@U|?K0kxN<=$N&LG22O@eK>FhcAi)L_VPIf^(jYbm+eAM(4kI9I
IX#)cT0Ciyr82|tP

delta 70
zcmXRZ;c|0y4k%$@U|_sCkxN?h!T)*(AZBFXWY`20{P+P#Yye^)V1d#gHphmEesUZj
J*3t$B1^}tB5E%df

diff --git a/tests/acpi-test-data/q35/FACP b/tests/acpi-test-data/q35/FACP
index 19f3ac3ce6ab732caa750d835ce1261bc7343cf2..72c9d97902a4bbf14896023d9ba78e0899d6517b 100644
GIT binary patch
literal 244
zcmZ>BbPo8!z`(#P@8s|75v<@85#a0w6k`O6f!H7#1{fJQ88!hqOw2%n4I;_{r9nIn
zAX@<@&cwhX02KSr|DPYCl7Ybp$XMFKz`)4C!0?j?A_|v;DFV`r3P1wMTp$k&7=Z>N
X+XoXzrWq9=?f{7~HXz&s;==#{<X{c~

literal 116
zcmZ>BbPgzCU|?YEaPoKd2v%^42yk`-iZKGkKx`1raN&Qw0}wMZa58KHa+#Qc#0HQA
i0|N_`2C+GSYz3G&69bC?Q0zbde}0f03@mM6U;qFojS~R?

-- 
MST

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 24/26] lpc: drop pcie host dependency
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (22 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 25/26] acpi-test: update FADT Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:09 ` [Qemu-devel] [PULL 26/26] virtio-balloon: include statistics of disk/file caches Michael S. Tsirkin
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Marcel Apfelbaum

Doesn't look like that header is used.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/isa/lpc_ich9.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index adcf077..e692b9f 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -39,7 +39,6 @@
 #include "hw/isa/apm.h"
 #include "hw/i386/ioapic.h"
 #include "hw/pci/pci.h"
-#include "hw/pci/pcie_host.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/i386/ich9.h"
 #include "hw/acpi/acpi.h"
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 26/26] virtio-balloon: include statistics of disk/file caches
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (23 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 24/26] lpc: drop pcie host dependency Michael S. Tsirkin
@ 2018-02-08 19:09 ` Michael S. Tsirkin
  2018-02-08 19:11 ` [Qemu-devel] [PULL 03/26] virtio: improve virtio devices initialization time Michael S. Tsirkin
  2018-02-09 10:06 ` [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Peter Maydell
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Tomáš Golembiovský

From: Tomáš Golembiovský <tgolembi@redhat.com>

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 hw/virtio/virtio-balloon.c                      | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 9d06ccd..7b0a41b 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -52,7 +52,8 @@ struct virtio_balloon_config {
 #define VIRTIO_BALLOON_S_MEMFREE  4   /* Total amount of free memory */
 #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
 #define VIRTIO_BALLOON_S_AVAIL    6   /* Available memory as in /proc */
-#define VIRTIO_BALLOON_S_NR       7
+#define VIRTIO_BALLOON_S_CACHES   7   /* Disk caches */
+#define VIRTIO_BALLOON_S_NR       8
 
 /*
  * Memory statistics structure.
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index f2104bf..179c8f5 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
    [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
    [VIRTIO_BALLOON_S_NR] = NULL
 };
 
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PULL 03/26] virtio: improve virtio devices initialization time
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (24 preceding siblings ...)
  2018-02-08 19:09 ` [Qemu-devel] [PULL 26/26] virtio-balloon: include statistics of disk/file caches Michael S. Tsirkin
@ 2018-02-08 19:11 ` Michael S. Tsirkin
  2018-02-09 10:06 ` [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Peter Maydell
  26 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-08 19:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gal Hammer, Greg Kurz

From: Gal Hammer <ghammer@redhat.com>

The loading time of a VM is quite significant when its virtio
devices use a large amount of virt-queues (e.g. a virtio-serial
device with max_ports=511). Most of the time is spend in the
creation of all the required event notifiers (ioeventfd and memory
regions).

This patch pack all the changes to the memory regions in a
single memory transaction.

Reported-by: Sitong Liu
Reported-by: Xiaoling Gao
Signed-off-by: Gal Hammer <ghammer@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Greg Kurz <groug@kaod.org>
---
 hw/virtio/virtio.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3667cd6..006d3d1 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2572,8 +2572,9 @@ static Property virtio_properties[] = {
 static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
 {
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    int n, r, err;
+    int i, n, r, err;
 
+    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
@@ -2596,9 +2597,11 @@ static int virtio_device_start_ioeventfd_impl(VirtIODevice *vdev)
         }
         event_notifier_set(&vq->host_notifier);
     }
+    memory_region_transaction_commit();
     return 0;
 
 assign_error:
+    i = n; /* save n for a second iteration after transaction is committed. */
     while (--n >= 0) {
         VirtQueue *vq = &vdev->vq[n];
         if (!virtio_queue_get_num(vdev, n)) {
@@ -2608,7 +2611,14 @@ assign_error:
         event_notifier_set_handler(&vq->host_notifier, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
-        virtio_bus_cleanup_host_notifier(qbus, n);
+    }
+    memory_region_transaction_commit();
+
+    while (--i >= 0) {
+        if (!virtio_queue_get_num(vdev, i)) {
+            continue;
+        }
+        virtio_bus_cleanup_host_notifier(qbus, i);
     }
     return err;
 }
@@ -2626,6 +2636,7 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
     VirtioBusState *qbus = VIRTIO_BUS(qdev_get_parent_bus(DEVICE(vdev)));
     int n, r;
 
+    memory_region_transaction_begin();
     for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
         VirtQueue *vq = &vdev->vq[n];
 
@@ -2635,6 +2646,13 @@ static void virtio_device_stop_ioeventfd_impl(VirtIODevice *vdev)
         event_notifier_set_handler(&vq->host_notifier, NULL);
         r = virtio_bus_set_host_notifier(qbus, n, false);
         assert(r >= 0);
+    }
+    memory_region_transaction_commit();
+
+    for (n = 0; n < VIRTIO_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
         virtio_bus_cleanup_host_notifier(qbus, n);
     }
 }
-- 
MST

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
                   ` (25 preceding siblings ...)
  2018-02-08 19:11 ` [Qemu-devel] [PULL 03/26] virtio: improve virtio devices initialization time Michael S. Tsirkin
@ 2018-02-09 10:06 ` Peter Maydell
  2018-02-09 17:07   ` Michael S. Tsirkin
  2018-02-10 11:26   ` Gonglei (Arei)
  26 siblings, 2 replies; 39+ messages in thread
From: Peter Maydell @ 2018-02-09 10:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers

On 8 February 2018 at 19:08, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
>
>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into staging (2018-02-08 14:31:51 +0000)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
>
>   virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42 +0200)
>
> ----------------------------------------------------------------
> virtio,vhost,pci,pc: features, fixes and cleanups
>
> - a new vhost crypto device
> - new stats in virtio balloon
> - virtio eventfd rework for boot speedup
> - vhost memory rework for boot speedup
> - fixes and cleanups all over the place
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>

Hi. This has some format-string issues:

/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
'cryptodev_vhost_user_start':
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
error: format '%lu' expects argument of type 'long unsigned int', but
argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
             error_report("failed to init vhost_crypto for queue %lu", i);
                          ^
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
'cryptodev_vhost_user_init':
/home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
error: format '%lu' expects argument of type 'long unsigned int', but
argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
         cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
                                        ^

thanks
-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-09 10:06 ` [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Peter Maydell
@ 2018-02-09 17:07   ` Michael S. Tsirkin
  2018-02-12  9:35     ` Peter Maydell
  2018-02-10 11:26   ` Gonglei (Arei)
  1 sibling, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-09 17:07 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On Fri, Feb 09, 2018 at 10:06:42AM +0000, Peter Maydell wrote:
> On 8 February 2018 at 19:08, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into staging (2018-02-08 14:31:51 +0000)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
> >
> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42 +0200)
> >
> > ----------------------------------------------------------------
> > virtio,vhost,pci,pc: features, fixes and cleanups
> >
> > - a new vhost crypto device
> > - new stats in virtio balloon
> > - virtio eventfd rework for boot speedup
> > - vhost memory rework for boot speedup
> > - fixes and cleanups all over the place
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> 
> Hi. This has some format-string issues:
> 
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_start':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>              error_report("failed to init vhost_crypto for queue %lu", i);
>                           ^
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_init':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>          cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
>                                         ^
> 
> thanks
> -- PMM

Fixed up now, new hash d94c5ae38f25a6e77e07007dcd510b7203a687e8

-- 
MST

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-09 10:06 ` [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Peter Maydell
  2018-02-09 17:07   ` Michael S. Tsirkin
@ 2018-02-10 11:26   ` Gonglei (Arei)
  1 sibling, 0 replies; 39+ messages in thread
From: Gonglei (Arei) @ 2018-02-10 11:26 UTC (permalink / raw)
  To: Peter Maydell, Michael S. Tsirkin; +Cc: QEMU Developers

> -----Original Message-----
> From: Qemu-devel
> [mailto:qemu-devel-bounces+arei.gonglei=huawei.com@nongnu.org] On
> Behalf Of Peter Maydell
> Sent: Friday, February 09, 2018 6:07 PM
> To: Michael S. Tsirkin
> Cc: QEMU Developers
> Subject: Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and
> cleanups
> 
> On 8 February 2018 at 19:08, Michael S. Tsirkin <mst@redhat.com> wrote:
> > The following changes since commit
> 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
> >
> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request'
> into staging (2018-02-08 14:31:51 +0000)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to
> f4ac9b2e04e8d98854a97bc473353207765aa9e7:
> >
> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42
> +0200)
> >
> > ----------------------------------------------------------------
> > virtio,vhost,pci,pc: features, fixes and cleanups
> >
> > - a new vhost crypto device
> > - new stats in virtio balloon
> > - virtio eventfd rework for boot speedup
> > - vhost memory rework for boot speedup
> > - fixes and cleanups all over the place
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> 
> Hi. This has some format-string issues:
> 
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_start':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:112:26:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>              error_report("failed to init vhost_crypto for queue %lu", i);
>                           ^
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c: In function
> 'cryptodev_vhost_user_init':
> /home/peter.maydell/qemu/backends/cryptodev-vhost-user.c:205:40:
> error: format '%lu' expects argument of type 'long unsigned int', but
> argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
>          cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
>                                         ^
> 
Using %zu instead of %lu will be correct. Michael, could you pls fix it directly?

Very sorry for the inconvenience. :(

Thanks,
-Gonglei


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-09 17:07   ` Michael S. Tsirkin
@ 2018-02-12  9:35     ` Peter Maydell
  2018-02-13 16:33       ` Peter Maydell
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2018-02-12  9:35 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers

On 9 February 2018 at 17:07, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Feb 09, 2018 at 10:06:42AM +0000, Peter Maydell wrote:
>> On 8 February 2018 at 19:08, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > The following changes since commit 008a51bbb343972dd8cf09126da8c3b87f4e1c96:
>> >
>> >   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into staging (2018-02-08 14:31:51 +0000)
>> >
>> > are available in the git repository at:
>> >
>> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>> >
>> > for you to fetch changes up to f4ac9b2e04e8d98854a97bc473353207765aa9e7:
>> >
>> >   virtio-balloon: include statistics of disk/file caches (2018-02-08 21:06:42 +0200)
>> >
>> > ----------------------------------------------------------------
>> > virtio,vhost,pci,pc: features, fixes and cleanups
>> >
>> > - a new vhost crypto device
>> > - new stats in virtio balloon
>> > - virtio eventfd rework for boot speedup
>> > - vhost memory rework for boot speedup
>> > - fixes and cleanups all over the place
>> >
>> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> >
>>
>> Hi. This has some format-string issues:

> Fixed up now, new hash d94c5ae38f25a6e77e07007dcd510b7203a687e8

This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:

TEST: tests/device-introspect-test... (pid=19530)
  /aarch64/device/introspect/list:                                     OK
  /aarch64/device/introspect/list-fields:                              OK
  /aarch64/device/introspect/none:                                     OK
  /aarch64/device/introspect/abstract:                                 OK
  /aarch64/device/introspect/concrete:                                 **
ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
assertion failed: (type != NULL)
Broken pipe
FAIL
GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
(pid=12686)
  /aarch64/device/introspect/abstract-interfaces:                      OK
FAIL: tests/device-introspect-test

(other archs fail too, aarch64 is just the first one we hit). This
error is often "device A instantiates device B, but device B is
conditionally compiled in and A is always compiled; so test fails
trying to create device A on hosts where device B isn't built" I think.

clang build on x86 fails to compile:
/home/petmay01/linaro/qemu-for-merges/backends/cryptodev-vhost-user.c:86:16:
error: address of array 's->vhost_crypto' will always evaluate to
'true' [-Werror,-Wpointer-bool-conversion]
        if (s->vhost_crypto) {
        ~~  ~~~^~~~~~~~~~~~

thanks
-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-12  9:35     ` Peter Maydell
@ 2018-02-13 16:33       ` Peter Maydell
  2018-02-13 16:52         ` Michael S. Tsirkin
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2018-02-13 16:33 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers

On 12 February 2018 at 09:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
>
> TEST: tests/device-introspect-test... (pid=19530)
>   /aarch64/device/introspect/list:                                     OK
>   /aarch64/device/introspect/list-fields:                              OK
>   /aarch64/device/introspect/none:                                     OK
>   /aarch64/device/introspect/abstract:                                 OK
>   /aarch64/device/introspect/concrete:                                 **
> ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> assertion failed: (type != NULL)
> Broken pipe
> FAIL
> GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> (pid=12686)
>   /aarch64/device/introspect/abstract-interfaces:                      OK
> FAIL: tests/device-introspect-test
>
> (other archs fail too, aarch64 is just the first one we hit). This
> error is often "device A instantiates device B, but device B is
> conditionally compiled in and A is always compiled; so test fails
> trying to create device A on hosts where device B isn't built" I think.

This part is indeed that problem -- the 'virtio-crypto-device' object
is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
former you get a virtio-crypto-pci device that crashes when instantiated.

The fix should be
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
 obj-$(CONFIG_LINUX) += virtio-crypto.o
-obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
+obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) += virtio-crypto-pci.o
 endif

 common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o


thanks
-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-13 16:33       ` Peter Maydell
@ 2018-02-13 16:52         ` Michael S. Tsirkin
  2018-02-13 18:23           ` Peter Maydell
  2018-02-14  2:21           ` Zhoujian (jay)
  0 siblings, 2 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-13 16:52 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On Tue, Feb 13, 2018 at 04:33:14PM +0000, Peter Maydell wrote:
> On 12 February 2018 at 09:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> > This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
> >
> > TEST: tests/device-introspect-test... (pid=19530)
> >   /aarch64/device/introspect/list:                                     OK
> >   /aarch64/device/introspect/list-fields:                              OK
> >   /aarch64/device/introspect/none:                                     OK
> >   /aarch64/device/introspect/abstract:                                 OK
> >   /aarch64/device/introspect/concrete:                                 **
> > ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> > assertion failed: (type != NULL)
> > Broken pipe
> > FAIL
> > GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> > (pid=12686)
> >   /aarch64/device/introspect/abstract-interfaces:                      OK
> > FAIL: tests/device-introspect-test
> >
> > (other archs fail too, aarch64 is just the first one we hit). This
> > error is often "device A instantiates device B, but device B is
> > conditionally compiled in and A is always compiled; so test fails
> > trying to create device A on hosts where device B isn't built" I think.
> 
> This part is indeed that problem -- the 'virtio-crypto-device' object
> is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
> CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
> former you get a virtio-crypto-pci device that crashes when instantiated.
> 
> The fix should be
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
>  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
>  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
>  obj-$(CONFIG_LINUX) += virtio-crypto.o
> -obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> +obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) += virtio-crypto-pci.o
>  endif
> 
>  common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o
> 
> 
> thanks
> -- PMM

Thanks for your help with this!

I think the root cause is that one of the patches disables
virtio-crypto build on non linux which used to be enabled.

This has been silently done by a patch which was supposed to
merely add a vhost crypto device, I'm sorry I missed that
in the review.

I've dropped the crypto vhost patches from the pull request for now.

Pushed with the same name - should be fine now.

-- 
MST

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 15/26] cryptodev: add vhost-user as a new cryptodev backend
  2018-02-08 19:09 ` [Qemu-devel] [PULL 15/26] cryptodev: add vhost-user as a new cryptodev backend Michael S. Tsirkin
@ 2018-02-13 16:54   ` Michael S. Tsirkin
  0 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2018-02-13 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gonglei, Longpeng, Jay Zhou, Paolo Bonzini

On Thu, Feb 08, 2018 at 09:09:13PM +0200, Michael S. Tsirkin wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Usage:
>  -chardev socket,id=charcrypto0,path=/path/to/your/socket
>  -object cryptodev-vhost-user,id=cryptodev0,chardev=charcrypto0
>  -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Dropped this and follow-up patches due to failures
on non-linux hosts and stop corrections.

> ---
>  configure                        |  15 ++
>  include/sysemu/cryptodev-vhost.h | 154 ++++++++++++++++++
>  backends/cryptodev-vhost-user.c  | 333 +++++++++++++++++++++++++++++++++++++++
>  backends/cryptodev-vhost.c       |  89 +++++++++++
>  vl.c                             |   6 +
>  backends/Makefile.objs           |   6 +
>  qemu-options.hx                  |  21 +++
>  7 files changed, 624 insertions(+)
>  create mode 100644 include/sysemu/cryptodev-vhost.h
>  create mode 100644 backends/cryptodev-vhost-user.c
>  create mode 100644 backends/cryptodev-vhost.c
> 
> diff --git a/configure b/configure
> index 831ebf2..296f980 100755
> --- a/configure
> +++ b/configure
> @@ -344,6 +344,7 @@ xfs=""
>  tcg="yes"
>  
>  vhost_net="no"
> +vhost_crypto="no"
>  vhost_scsi="no"
>  vhost_vsock="no"
>  vhost_user=""
> @@ -814,6 +815,7 @@ Linux)
>    linux_user="yes"
>    kvm="yes"
>    vhost_net="yes"
> +  vhost_crypto="yes"
>    vhost_scsi="yes"
>    vhost_vsock="yes"
>    QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers $QEMU_INCLUDES"
> @@ -1184,6 +1186,14 @@ for opt do
>    ;;
>    --enable-vhost-net) vhost_net="yes"
>    ;;
> +  --disable-vhost-crypto) vhost_crypto="no"
> +  ;;
> +  --enable-vhost-crypto)
> +      vhost_crypto="yes"
> +      if test "$mingw32" = "yes"; then
> +          error_exit "vhost-crypto isn't available on win32"
> +      fi
> +  ;;
>    --disable-vhost-scsi) vhost_scsi="no"
>    ;;
>    --enable-vhost-scsi) vhost_scsi="yes"
> @@ -1582,6 +1592,7 @@ disabled with --disable-FEATURE, default is enabled if available:
>    cap-ng          libcap-ng support
>    attr            attr and xattr support
>    vhost-net       vhost-net acceleration support
> +  vhost-crypto    vhost-crypto acceleration support
>    spice           spice
>    rbd             rados block device (rbd)
>    libiscsi        iscsi support
> @@ -5704,6 +5715,7 @@ echo "madvise           $madvise"
>  echo "posix_madvise     $posix_madvise"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
> +echo "vhost-crypto support $vhost_crypto"
>  echo "vhost-scsi support $vhost_scsi"
>  echo "vhost-vsock support $vhost_vsock"
>  echo "vhost-user support $vhost_user"
> @@ -6783,6 +6795,9 @@ if supported_kvm_target $target; then
>              echo "CONFIG_VHOST_USER_NET_TEST_$target_name=y" >> $config_host_mak
>          fi
>      fi
> +    if test "$vhost_crypto" = "yes" ; then
> +        echo "CONFIG_VHOST_CRYPTO=y" >> $config_target_mak
> +    fi
>  fi
>  if supported_hax_target $target; then
>      echo "CONFIG_HAX=y" >> $config_target_mak
> diff --git a/include/sysemu/cryptodev-vhost.h b/include/sysemu/cryptodev-vhost.h
> new file mode 100644
> index 0000000..fb26b86
> --- /dev/null
> +++ b/include/sysemu/cryptodev-vhost.h
> @@ -0,0 +1,154 @@
> +/*
> + * QEMU Crypto Device Common Vhost Implement
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <arei.gonglei@huawei.com>
> + *    Jay Zhou <jianjay.zhou@huawei.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +#ifndef CRYPTODEV_VHOST_H
> +#define CRYPTODEV_VHOST_H
> +
> +#include "qemu-common.h"
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-backend.h"
> +#include "chardev/char.h"
> +
> +#include "sysemu/cryptodev.h"
> +
> +
> +typedef struct CryptoDevBackendVhostOptions {
> +    VhostBackendType backend_type;
> +    void *opaque;
> +    int total_queues;
> +    CryptoDevBackendClient *cc;
> +} CryptoDevBackendVhostOptions;
> +
> +typedef struct CryptoDevBackendVhost {
> +    struct vhost_dev dev;
> +    struct vhost_virtqueue vqs[1];
> +    int backend;
> +    CryptoDevBackendClient *cc;
> +} CryptoDevBackendVhost;
> +
> +/**
> + * cryptodev_vhost_get_max_queues:
> + * @crypto: the cryptodev backend common vhost object
> + *
> + * Get the maximum queue number of @crypto.
> + *
> + *
> + * Returns: the maximum queue number
> + */
> +uint64_t
> +cryptodev_vhost_get_max_queues(
> +                        CryptoDevBackendVhost *crypto);
> +
> +
> +/**
> + * cryptodev_vhost_init:
> + * @options: the common vhost object's option
> + *
> + * Creates a new cryptodev backend common vhost object
> + *
> + ** The returned object must be released with
> + * cryptodev_vhost_cleanup() when no
> + * longer required
> + *
> + * Returns: the cryptodev backend common vhost object
> + */
> +struct CryptoDevBackendVhost *
> +cryptodev_vhost_init(
> +             CryptoDevBackendVhostOptions *options);
> +
> +/**
> + * cryptodev_vhost_cleanup:
> + * @crypto: the cryptodev backend common vhost object
> + *
> + * Clean the resouce associated with @crypto that realizaed
> + * by cryptodev_vhost_init()
> + *
> + */
> +void cryptodev_vhost_cleanup(
> +                        CryptoDevBackendVhost *crypto);
> +
> +/**
> + * cryptodev_get_vhost:
> + * @cc: the client object for each queue
> + * @b: the cryptodev backend common vhost object
> + * @queue: the cryptodev backend queue index
> + *
> + * Gets a new cryptodev backend common vhost object based on
> + * @b and @queue
> + *
> + * Returns: the cryptodev backend common vhost object
> + */
> +CryptoDevBackendVhost *
> +cryptodev_get_vhost(CryptoDevBackendClient *cc,
> +                            CryptoDevBackend *b,
> +                            uint16_t queue);
> +/**
> + * cryptodev_vhost_start:
> + * @dev: the virtio crypto object
> + * @total_queues: the total count of queue
> + *
> + * Starts the vhost crypto logic
> + *
> + * Returns: 0 for success, negative for errors
> + */
> +int cryptodev_vhost_start(VirtIODevice *dev, int total_queues);
> +
> +/**
> + * cryptodev_vhost_stop:
> + * @dev: the virtio crypto object
> + * @total_queues: the total count of queue
> + *
> + * Stops the vhost crypto logic
> + *
> + */
> +void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues);
> +
> +/**
> + * cryptodev_vhost_virtqueue_mask:
> + * @dev: the virtio crypto object
> + * @queue: the cryptodev backend queue index
> + * @idx: the virtqueue index
> + * @mask: mask or not (true or false)
> + *
> + * Mask/unmask events for @idx virtqueue on @dev device
> + *
> + */
> +void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
> +                                           int queue,
> +                                           int idx, bool mask);
> +
> +/**
> + * cryptodev_vhost_virtqueue_pending:
> + * @dev: the virtio crypto object
> + * @queue: the cryptodev backend queue index
> + * @idx: the virtqueue index
> + *
> + * Test and clear event pending status for @idx virtqueue on @dev device.
> + * Should be called after unmask to avoid losing events.
> + *
> + * Returns: true for success, false for errors
> + */
> +bool cryptodev_vhost_virtqueue_pending(VirtIODevice *dev,
> +                                              int queue, int idx);
> +
> +#endif /* CRYPTODEV_VHOST_H */
> diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
> new file mode 100644
> index 0000000..4e63ece
> --- /dev/null
> +++ b/backends/cryptodev-vhost-user.c
> @@ -0,0 +1,333 @@
> +/*
> + * QEMU Cryptodev backend for QEMU cipher APIs
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <arei.gonglei@huawei.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/boards.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/error-report.h"
> +#include "standard-headers/linux/virtio_crypto.h"
> +#include "sysemu/cryptodev-vhost.h"
> +#include "chardev/char-fe.h"
> +
> +
> +/**
> + * @TYPE_CRYPTODEV_BACKEND_VHOST_USER:
> + * name of backend that uses vhost user server
> + */
> +#define TYPE_CRYPTODEV_BACKEND_VHOST_USER "cryptodev-vhost-user"
> +
> +#define CRYPTODEV_BACKEND_VHOST_USER(obj) \
> +    OBJECT_CHECK(CryptoDevBackendVhostUser, \
> +                 (obj), TYPE_CRYPTODEV_BACKEND_VHOST_USER)
> +
> +
> +typedef struct CryptoDevBackendVhostUser {
> +    CryptoDevBackend parent_obj;
> +
> +    CharBackend chr;
> +    char *chr_name;
> +    bool opened;
> +    CryptoDevBackendVhost *vhost_crypto[MAX_CRYPTO_QUEUE_NUM];
> +} CryptoDevBackendVhostUser;
> +
> +static int
> +cryptodev_vhost_user_running(
> +             CryptoDevBackendVhost *crypto)
> +{
> +    return crypto ? 1 : 0;
> +}
> +
> +static void cryptodev_vhost_user_stop(int queues,
> +                          CryptoDevBackendVhostUser *s)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < queues; i++) {
> +        if (!cryptodev_vhost_user_running(s->vhost_crypto[i])) {
> +            continue;
> +        }
> +
> +        if (s->vhost_crypto) {
> +            cryptodev_vhost_cleanup(s->vhost_crypto[i]);
> +            s->vhost_crypto[i] = NULL;
> +        }
> +    }
> +}
> +
> +static int
> +cryptodev_vhost_user_start(int queues,
> +                         CryptoDevBackendVhostUser *s)
> +{
> +    CryptoDevBackendVhostOptions options;
> +    CryptoDevBackend *b = CRYPTODEV_BACKEND(s);
> +    int max_queues;
> +    size_t i;
> +
> +    for (i = 0; i < queues; i++) {
> +        if (cryptodev_vhost_user_running(s->vhost_crypto[i])) {
> +            continue;
> +        }
> +
> +        options.opaque = &s->chr;
> +        options.backend_type = VHOST_BACKEND_TYPE_USER;
> +        options.cc = b->conf.peers.ccs[i];
> +        s->vhost_crypto[i] = cryptodev_vhost_init(&options);
> +        if (!s->vhost_crypto[i]) {
> +            error_report("failed to init vhost_crypto for queue %lu", i);
> +            goto err;
> +        }
> +
> +        if (i == 0) {
> +            max_queues =
> +              cryptodev_vhost_get_max_queues(s->vhost_crypto[i]);
> +            if (queues > max_queues) {
> +                error_report("you are asking more queues than supported: %d",
> +                             max_queues);
> +                goto err;
> +            }
> +        }
> +    }
> +
> +    return 0;
> +
> +err:
> +    cryptodev_vhost_user_stop(i + 1, s);
> +    return -1;
> +}
> +
> +static Chardev *
> +cryptodev_vhost_claim_chardev(CryptoDevBackendVhostUser *s,
> +                                    Error **errp)
> +{
> +    Chardev *chr;
> +
> +    if (s->chr_name == NULL) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> +                   "chardev", "a valid character device");
> +        return NULL;
> +    }
> +
> +    chr = qemu_chr_find(s->chr_name);
> +    if (chr == NULL) {
> +        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> +                  "Device '%s' not found", s->chr_name);
> +        return NULL;
> +    }
> +
> +    return chr;
> +}
> +
> +static void cryptodev_vhost_user_event(void *opaque, int event)
> +{
> +    CryptoDevBackendVhostUser *s = opaque;
> +    CryptoDevBackend *b = CRYPTODEV_BACKEND(s);
> +    Error *err = NULL;
> +    int queues = b->conf.peers.queues;
> +
> +    assert(queues < MAX_CRYPTO_QUEUE_NUM);
> +
> +    switch (event) {
> +    case CHR_EVENT_OPENED:
> +        if (cryptodev_vhost_user_start(queues, s) < 0) {
> +            exit(1);
> +        }
> +        b->ready = true;
> +        break;
> +    case CHR_EVENT_CLOSED:
> +        b->ready = false;
> +        cryptodev_vhost_user_stop(queues, s);
> +        break;
> +    }
> +
> +    if (err) {
> +        error_report_err(err);
> +    }
> +}
> +
> +static void cryptodev_vhost_user_init(
> +             CryptoDevBackend *backend, Error **errp)
> +{
> +    int queues = backend->conf.peers.queues;
> +    size_t i;
> +    Error *local_err = NULL;
> +    Chardev *chr;
> +    CryptoDevBackendClient *cc;
> +    CryptoDevBackendVhostUser *s =
> +                      CRYPTODEV_BACKEND_VHOST_USER(backend);
> +
> +    chr = cryptodev_vhost_claim_chardev(s, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    s->opened = true;
> +
> +    for (i = 0; i < queues; i++) {
> +        cc = cryptodev_backend_new_client(
> +                  "cryptodev-vhost-user", NULL);
> +        cc->info_str = g_strdup_printf("cryptodev-vhost-user%lu to %s ",
> +                                       i, chr->label);
> +        cc->queue_index = i;
> +
> +        backend->conf.peers.ccs[i] = cc;
> +
> +        if (i == 0) {
> +            if (!qemu_chr_fe_init(&s->chr, chr, &local_err)) {
> +                error_propagate(errp, local_err);
> +                return;
> +            }
> +        }
> +    }
> +
> +    qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
> +                     cryptodev_vhost_user_event, NULL, s, NULL, true);
> +
> +    backend->conf.crypto_services =
> +                         1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
> +                         1u << VIRTIO_CRYPTO_SERVICE_HASH |
> +                         1u << VIRTIO_CRYPTO_SERVICE_MAC;
> +    backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
> +    backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
> +}
> +
> +static int64_t cryptodev_vhost_user_sym_create_session(
> +           CryptoDevBackend *backend,
> +           CryptoDevBackendSymSessionInfo *sess_info,
> +           uint32_t queue_index, Error **errp)
> +{
> +    return 0;
> +}
> +
> +static int cryptodev_vhost_user_sym_close_session(
> +           CryptoDevBackend *backend,
> +           uint64_t session_id,
> +           uint32_t queue_index, Error **errp)
> +{
> +    return 0;
> +}
> +
> +static int cryptodev_vhost_user_sym_operation(
> +                 CryptoDevBackend *backend,
> +                 CryptoDevBackendSymOpInfo *op_info,
> +                 uint32_t queue_index, Error **errp)
> +{
> +    return VIRTIO_CRYPTO_OK;
> +}
> +
> +static void cryptodev_vhost_user_cleanup(
> +             CryptoDevBackend *backend,
> +             Error **errp)
> +{
> +    CryptoDevBackendVhostUser *s =
> +                      CRYPTODEV_BACKEND_VHOST_USER(backend);
> +    size_t i;
> +    int queues = backend->conf.peers.queues;
> +    CryptoDevBackendClient *cc;
> +
> +    cryptodev_vhost_user_stop(queues, s);
> +
> +    for (i = 0; i < queues; i++) {
> +        cc = backend->conf.peers.ccs[i];
> +        if (cc) {
> +            cryptodev_backend_free_client(cc);
> +            backend->conf.peers.ccs[i] = NULL;
> +        }
> +    }
> +}
> +
> +static void cryptodev_vhost_user_set_chardev(Object *obj,
> +                                    const char *value, Error **errp)
> +{
> +    CryptoDevBackendVhostUser *s =
> +                      CRYPTODEV_BACKEND_VHOST_USER(obj);
> +
> +    if (s->opened) {
> +        error_setg(errp, QERR_PERMISSION_DENIED);
> +    } else {
> +        g_free(s->chr_name);
> +        s->chr_name = g_strdup(value);
> +    }
> +}
> +
> +static char *
> +cryptodev_vhost_user_get_chardev(Object *obj, Error **errp)
> +{
> +    CryptoDevBackendVhostUser *s =
> +                      CRYPTODEV_BACKEND_VHOST_USER(obj);
> +    Chardev *chr = qemu_chr_fe_get_driver(&s->chr);
> +
> +    if (chr && chr->label) {
> +        return g_strdup(chr->label);
> +    }
> +
> +    return NULL;
> +}
> +
> +static void cryptodev_vhost_user_instance_int(Object *obj)
> +{
> +    object_property_add_str(obj, "chardev",
> +                            cryptodev_vhost_user_get_chardev,
> +                            cryptodev_vhost_user_set_chardev,
> +                            NULL);
> +}
> +
> +static void cryptodev_vhost_user_finalize(Object *obj)
> +{
> +    CryptoDevBackendVhostUser *s =
> +                      CRYPTODEV_BACKEND_VHOST_USER(obj);
> +
> +    qemu_chr_fe_deinit(&s->chr, false);
> +
> +    g_free(s->chr_name);
> +}
> +
> +static void
> +cryptodev_vhost_user_class_init(ObjectClass *oc, void *data)
> +{
> +    CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_CLASS(oc);
> +
> +    bc->init = cryptodev_vhost_user_init;
> +    bc->cleanup = cryptodev_vhost_user_cleanup;
> +    bc->create_session = cryptodev_vhost_user_sym_create_session;
> +    bc->close_session = cryptodev_vhost_user_sym_close_session;
> +    bc->do_sym_op = cryptodev_vhost_user_sym_operation;
> +}
> +
> +static const TypeInfo cryptodev_vhost_user_info = {
> +    .name = TYPE_CRYPTODEV_BACKEND_VHOST_USER,
> +    .parent = TYPE_CRYPTODEV_BACKEND,
> +    .class_init = cryptodev_vhost_user_class_init,
> +    .instance_init = cryptodev_vhost_user_instance_int,
> +    .instance_finalize = cryptodev_vhost_user_finalize,
> +    .instance_size = sizeof(CryptoDevBackendVhostUser),
> +};
> +
> +static void
> +cryptodev_vhost_user_register_types(void)
> +{
> +    type_register_static(&cryptodev_vhost_user_info);
> +}
> +
> +type_init(cryptodev_vhost_user_register_types);
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> new file mode 100644
> index 0000000..27e1c4a
> --- /dev/null
> +++ b/backends/cryptodev-vhost.c
> @@ -0,0 +1,89 @@
> +/*
> + * QEMU Cryptodev backend for QEMU cipher APIs
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <arei.gonglei@huawei.com>
> + *    Jay Zhou <jianjay.zhou@huawei.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/cryptodev-vhost.h"
> +
> +#ifdef CONFIG_VHOST_CRYPTO
> +uint64_t
> +cryptodev_vhost_get_max_queues(
> +                        CryptoDevBackendVhost *crypto)
> +{
> +    return crypto->dev.max_queues;
> +}
> +
> +void cryptodev_vhost_cleanup(CryptoDevBackendVhost *crypto)
> +{
> +    vhost_dev_cleanup(&crypto->dev);
> +    g_free(crypto);
> +}
> +
> +struct CryptoDevBackendVhost *
> +cryptodev_vhost_init(
> +             CryptoDevBackendVhostOptions *options)
> +{
> +    int r;
> +    CryptoDevBackendVhost *crypto;
> +
> +    crypto = g_new(CryptoDevBackendVhost, 1);
> +    crypto->dev.max_queues = 1;
> +    crypto->dev.nvqs = 1;
> +    crypto->dev.vqs = crypto->vqs;
> +
> +    crypto->cc = options->cc;
> +
> +    crypto->dev.protocol_features = 0;
> +    crypto->backend = -1;
> +
> +    /* vhost-user needs vq_index to initiate a specific queue pair */
> +    crypto->dev.vq_index = crypto->cc->queue_index * crypto->dev.nvqs;
> +
> +    r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type, 0);
> +    if (r < 0) {
> +        goto fail;
> +    }
> +
> +    return crypto;
> +fail:
> +    g_free(crypto);
> +    return NULL;
> +}
> +
> +#else
> +uint64_t
> +cryptodev_vhost_get_max_queues(CryptoDevBackendVhost *crypto)
> +{
> +    return 0;
> +}
> +
> +void cryptodev_vhost_cleanup(CryptoDevBackendVhost *crypto)
> +{
> +}
> +
> +struct CryptoDevBackendVhost *
> +cryptodev_vhost_init(CryptoDevBackendVhostOptions *options)
> +{
> +    return NULL;
> +}
> +#endif
> diff --git a/vl.c b/vl.c
> index 32db91d..4ef4e67 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2852,6 +2852,12 @@ static bool object_create_initial(const char *type)
>          return false;
>      }
>  
> +#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
> +    if (g_str_equal(type, "cryptodev-vhost-user")) {
> +        return false;
> +    }
> +#endif
> +
>      /*
>       * return false for concrete netfilters since
>       * they depend on netdevs already existing
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 67eeeba..efdcc5a 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -10,3 +10,9 @@ common-obj-y += cryptodev.o
>  common-obj-y += cryptodev-builtin.o
>  
>  common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
> +
> +ifeq ($(CONFIG_VIRTIO),y)
> +common-obj-$(CONFIG_LINUX) += cryptodev-vhost.o
> +common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_LINUX)) += \
> +    cryptodev-vhost-user.o
> +endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d15c171..cdc464c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4225,6 +4225,27 @@ which specify the queue number of cryptodev backend, the default of
>     [...]
>  @end example
>  
> +@item -object cryptodev-vhost-user,id=@var{id},chardev=@var{chardevid}[,queues=@var{queues}]
> +
> +Creates a vhost-user cryptodev backend, backed by a chardev @var{chardevid}.
> +The @var{id} parameter is a unique ID that will be used to reference this
> +cryptodev backend from the @option{virtio-crypto} device.
> +The chardev should be a unix domain socket backed one. The vhost-user uses
> +a specifically defined protocol to pass vhost ioctl replacement messages
> +to an application on the other end of the socket.
> +The @var{queues} parameter is optional, which specify the queue number
> +of cryptodev backend for multiqueue vhost-user, the default of @var{queues} is 1.
> +
> +@example
> +
> + # qemu-system-x86_64 \
> +   [...] \
> +       -chardev socket,id=chardev0,path=/path/to/socket \
> +       -object cryptodev-vhost-user,id=cryptodev0,chardev=chardev0 \
> +       -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
> +   [...]
> +@end example
> +
>  @item -object secret,id=@var{id},data=@var{string},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
>  @item -object secret,id=@var{id},file=@var{filename},format=@var{raw|base64}[,keyid=@var{secretid},iv=@var{string}]
>  
> -- 
> MST
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-13 16:52         ` Michael S. Tsirkin
@ 2018-02-13 18:23           ` Peter Maydell
  2018-02-14  2:21           ` Zhoujian (jay)
  1 sibling, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2018-02-13 18:23 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers

On 13 February 2018 at 16:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> I've dropped the crypto vhost patches from the pull request for now.
>
> Pushed with the same name - should be fine now.

Applied the fixed version, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups
  2018-02-13 16:52         ` Michael S. Tsirkin
  2018-02-13 18:23           ` Peter Maydell
@ 2018-02-14  2:21           ` Zhoujian (jay)
  1 sibling, 0 replies; 39+ messages in thread
From: Zhoujian (jay) @ 2018-02-14  2:21 UTC (permalink / raw)
  To: Michael S. Tsirkin, Peter Maydell; +Cc: QEMU Developers

> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> bounces+jianjay.zhou=huawei.com@nongnu.org] On Behalf Of Michael S. Tsirkin
> Sent: Wednesday, February 14, 2018 12:52 AM
> To: Peter Maydell <peter.maydell@linaro.org>
> Cc: QEMU Developers <qemu-devel@nongnu.org>
> Subject: Re: [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features,
> fixes and cleanups
> 
> On Tue, Feb 13, 2018 at 04:33:14PM +0000, Peter Maydell wrote:
> > On 12 February 2018 at 09:35, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> > > This asserts in 'make check' for NetBSD, FreeBSD, OpenBSD, OSX:
> > >
> > > TEST: tests/device-introspect-test... (pid=19530)
> > >   /aarch64/device/introspect/list:                                     OK
> > >   /aarch64/device/introspect/list-fields:                              OK
> > >   /aarch64/device/introspect/none:                                     OK
> > >   /aarch64/device/introspect/abstract:                                 OK
> > >   /aarch64/device/introspect/concrete:                                 **
> > > ERROR:/root/qemu/qom/object.c:372:object_initialize_with_type:
> > > assertion failed: (type != NULL)
> > > Broken pipe
> > > FAIL
> > > GTester: last random seed: R02Sd4e2c04f6ac00d843a31ccac4de0d914
> > > (pid=12686)
> > >   /aarch64/device/introspect/abstract-interfaces:                      OK
> > > FAIL: tests/device-introspect-test
> > >
> > > (other archs fail too, aarch64 is just the first one we hit). This
> > > error is often "device A instantiates device B, but device B is
> > > conditionally compiled in and A is always compiled; so test fails
> > > trying to create device A on hosts where device B isn't built" I think.
> >
> > This part is indeed that problem -- the 'virtio-crypto-device' object
> > is built only if CONFIG_LINUX, but 'virtio-crypto-pci' is built if
> > CONFIG_VIRTIO_PCI, so on systems where the latter is true but not the
> > former you get a virtio-crypto-pci device that crashes when instantiated.
> >
> > The fix should be
> > --- a/hw/virtio/Makefile.objs
> > +++ b/hw/virtio/Makefile.objs
> > @@ -8,7 +8,7 @@ obj-y += virtio.o virtio-balloon.o
> >  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
> >  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
> >  obj-$(CONFIG_LINUX) += virtio-crypto.o
> > -obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> > +obj-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_PCI)) +=
> > +virtio-crypto-pci.o
> >  endif
> >
> >  common-obj-$(call lnot,$(CONFIG_LINUX)) += vhost-stub.o
> >
> >
> > thanks
> > -- PMM
> 
> Thanks for your help with this!
> 
> I think the root cause is that one of the patches disables virtio-crypto
> build on non linux which used to be enabled.
> 
> This has been silently done by a patch which was supposed to merely add a
> vhost crypto device, I'm sorry I missed that in the review.

Hi Michael, Peter

Sorry for the late response and the trouble I have made, I just returned
from a vacation.
Thanks for your help and pointing out the root cause, I should explicitly
put the config change of virtio-crypto into a separate patch to make
review much easier.

> 
> I've dropped the crypto vhost patches from the pull request for now.

Will fix the issue in the next version, sorry again for the inconvenience.

Regards,
Jay

> 
> Pushed with the same name - should be fine now.
> 
> --
> MST

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset
  2018-02-08 19:09 ` [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset Michael S. Tsirkin
@ 2018-03-23 18:42   ` Laszlo Ersek
  2018-04-05 21:32     ` Michael Roth
  0 siblings, 1 reply; 39+ messages in thread
From: Laszlo Ersek @ 2018-03-23 18:42 UTC (permalink / raw)
  To: Peter Maydell, Michael Roth
  Cc: Michael S. Tsirkin, qemu-devel, Marcel Apfelbaum, qemu-stable

Michael, Peter,

On 02/08/18 20:09, Michael S. Tsirkin wrote:
> From: Laszlo Ersek <lersek@redhat.com>
> 
> The "i82801b11-bridge" device model is a descendant of "base-pci-bridge"
> (TYPE_PCI_BRIDGE). However, unlike other similar devices, such as
> 
> - pci-bridge,
> - pcie-pci-bridge,
> - PCIE Root Port,
> - xio3130 switch upstream and downstream ports,
> - dec-21154-p2p-bridge,
> - pbm-bridge,
> - xilinx-pcie-root,
> 
> "i82801b11-bridge" does not clear the bridge specific registers at
> platform reset.
> 
> This is a problem because devices on "i82801b11-bridge" continue to
> respond to config space cycles after platform reset, when addressed with
> the bus number that was previously programmed into the secondary bus
> number register of "i82801b11-bridge". This error breaks OVMF's search for
> extra (PXB) root buses, for example.
> 
> The device class reset method for "i82801b11-bridge" is currently NULL;
> set it directly to pci_bridge_reset(), like the last three bridge models
> in the above listing do.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: qemu-stable@nongnu.org
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1541839
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/pci-bridge/i82801b11.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
> index cb522bf..ebf7f5f 100644
> --- a/hw/pci-bridge/i82801b11.c
> +++ b/hw/pci-bridge/i82801b11.c
> @@ -98,6 +98,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)
>      k->realize = i82801b11_bridge_realize;
>      k->config_write = pci_bridge_write_config;
>      dc->vmsd = &i82801b11_bridge_dev_vmstate;
> +    dc->reset = pci_bridge_reset;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  }
>  
> 

this patch didn't get included in the 2.11.1 round-up:

http://mid.mail-archive.com/20180206191515.25830-1-mdroth@linux.vnet.ibm.com

(The patch was posted, CC stable, between Mike's round-up set and the
actual 2.11.1 release.)

Can we please make sure that the commit doesn't miss the 2.11.2 bus?
2.12 is still a month out, and without this patch, rebooting an UEFI
guest OS (with OVMF) hangs, on certain VM configs (see the commit message).

(Apologies if there is a "queue" or "next" branch for stable releases --
in that case I should have probably checked that branch before sending
this email.)

The commit hash on the master branch is
ed247f40db84c8bd4bb7d10948702cd47cc4d5ae; it's part of v2.12.0-rc0.

Thanks!
Laszlo

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset
  2018-03-23 18:42   ` Laszlo Ersek
@ 2018-04-05 21:32     ` Michael Roth
  0 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2018-04-05 21:32 UTC (permalink / raw)
  To: Laszlo Ersek, Peter Maydell
  Cc: Michael S. Tsirkin, qemu-devel, Marcel Apfelbaum, qemu-stable

Quoting Laszlo Ersek (2018-03-23 13:42:07)
> Michael, Peter,
> 
> On 02/08/18 20:09, Michael S. Tsirkin wrote:
> > From: Laszlo Ersek <lersek@redhat.com>
> > 
> > The "i82801b11-bridge" device model is a descendant of "base-pci-bridge"
> > (TYPE_PCI_BRIDGE). However, unlike other similar devices, such as
> > 
> > - pci-bridge,
> > - pcie-pci-bridge,
> > - PCIE Root Port,
> > - xio3130 switch upstream and downstream ports,
> > - dec-21154-p2p-bridge,
> > - pbm-bridge,
> > - xilinx-pcie-root,
> > 
> > "i82801b11-bridge" does not clear the bridge specific registers at
> > platform reset.
> > 
> > This is a problem because devices on "i82801b11-bridge" continue to
> > respond to config space cycles after platform reset, when addressed with
> > the bus number that was previously programmed into the secondary bus
> > number register of "i82801b11-bridge". This error breaks OVMF's search for
> > extra (PXB) root buses, for example.
> > 
> > The device class reset method for "i82801b11-bridge" is currently NULL;
> > set it directly to pci_bridge_reset(), like the last three bridge models
> > in the above listing do.
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel@redhat.com>
> > Cc: qemu-stable@nongnu.org
> > Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1541839
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/pci-bridge/i82801b11.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
> > index cb522bf..ebf7f5f 100644
> > --- a/hw/pci-bridge/i82801b11.c
> > +++ b/hw/pci-bridge/i82801b11.c
> > @@ -98,6 +98,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)
> >      k->realize = i82801b11_bridge_realize;
> >      k->config_write = pci_bridge_write_config;
> >      dc->vmsd = &i82801b11_bridge_dev_vmstate;
> > +    dc->reset = pci_bridge_reset;
> >      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >  }
> >  
> > 
> 
> this patch didn't get included in the 2.11.1 round-up:
> 
> http://mid.mail-archive.com/20180206191515.25830-1-mdroth@linux.vnet.ibm.com
> 
> (The patch was posted, CC stable, between Mike's round-up set and the
> actual 2.11.1 release.)

Looks like I had it tagged as pending but the PULL got held up for a while
and it didn't hit master till the day before release. I'm okay with
cherry-picking from a maintainer tree but generally only do it if I get
poked about it or it's on my radar for other reasons.

> 
> Can we please make sure that the commit doesn't miss the 2.11.2 bus?
> 2.12 is still a month out, and without this patch, rebooting an UEFI
> guest OS (with OVMF) hangs, on certain VM configs (see the commit message).
> 
> (Apologies if there is a "queue" or "next" branch for stable releases --
> in that case I should have probably checked that branch before sending
> this email.)

There's https://github.com/mdroth/qemu/commits/stable-2.11-staging but
as you can see it generally doesn't see much action in-between releases.

Will start queueing patches for 2.11.2 soon and make sure it gets in,
but probably looking at a similar release timeframe as 2.12.

Thanks for the heads up!

> 
> The commit hash on the master branch is
> ed247f40db84c8bd4bb7d10948702cd47cc4d5ae; it's part of v2.12.0-rc0.
> 
> Thanks!
> Laszlo
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2018-04-05 21:32 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-08 19:08 [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Michael S. Tsirkin
2018-02-08 19:08 ` [Qemu-devel] [PULL 01/26] Revert "vhost: add traces for memory listeners" Michael S. Tsirkin
2018-02-08 19:08 ` [Qemu-devel] [PULL 02/26] virtio: remove event notifier cleanup call on de-assign Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 04/26] vhost: Build temporary section list and deref after commit Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 05/26] vhost: Simplify ring verification checks Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 07/26] vhost: Regenerate region list from changed sections list Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 06/26] vhost: Merge sections added to temporary list Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 09/26] vhost: Merge and delete unused callbacks Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 08/26] vhost: Clean out old vhost_set_memory and friends Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 10/26] vhost: Move log_dirty check Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 11/26] pci-bridge/i82801b11: clear bridge registers on platform reset Michael S. Tsirkin
2018-03-23 18:42   ` Laszlo Ersek
2018-04-05 21:32     ` Michael Roth
2018-02-08 19:09 ` [Qemu-devel] [PULL 12/26] pci/bus: let it has higher migration priority Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 13/26] virtio-blk: enable multiple vectors when using multiple I/O queues Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 14/26] pci: removed the is_express field since a uniform interface was inserted Michael S. Tsirkin
2018-02-08 19:09   ` Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 15/26] cryptodev: add vhost-user as a new cryptodev backend Michael S. Tsirkin
2018-02-13 16:54   ` Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 17/26] cryptodev-vhost-user: add crypto session handler Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 16/26] cryptodev: add vhost support Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 20/26] libvhost-user: Fix resource leak Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 19/26] virtio-balloon: unref the memory region before continuing Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 18/26] cryptodev-vhost-user: set the key length Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 21/26] libvhost-user: Support across-memory-boundary access Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 22/26] hw/pci-bridge: fix pcie root port's IO hints capability Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 23/26] tests: acpi: fix FADT not being compared to reference table Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 25/26] acpi-test: update FADT Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 24/26] lpc: drop pcie host dependency Michael S. Tsirkin
2018-02-08 19:09 ` [Qemu-devel] [PULL 26/26] virtio-balloon: include statistics of disk/file caches Michael S. Tsirkin
2018-02-08 19:11 ` [Qemu-devel] [PULL 03/26] virtio: improve virtio devices initialization time Michael S. Tsirkin
2018-02-09 10:06 ` [Qemu-devel] [PULL 00/26] virtio, vhost, pci, pc: features, fixes and cleanups Peter Maydell
2018-02-09 17:07   ` Michael S. Tsirkin
2018-02-12  9:35     ` Peter Maydell
2018-02-13 16:33       ` Peter Maydell
2018-02-13 16:52         ` Michael S. Tsirkin
2018-02-13 18:23           ` Peter Maydell
2018-02-14  2:21           ` Zhoujian (jay)
2018-02-10 11:26   ` Gonglei (Arei)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.