Re: [PATCH 23/31] vdpa: Add custom IOTLB translations to SVQ

[Jason Wang]

在 2022/2/1 上午3:11, Eugenio Perez Martin 写道:

+return false;
+}
+
+/*
+ * Map->iova chunk size is ignored. What to do if descriptor
+ * (addr, size) does not fit is delegated to the device.
+ */

I think we need at least check the size and fail if the size doesn't
match here. Or is it possible that we have a buffer that may cross two
memory regions?


It should be impossible, since both iova_tree and VirtQueue should be
in sync regarding the memory regions updates. If a VirtQueue buffer
crosses many memory regions, iovec has more entries.

I can add a return false, but I'm not able to trigger that situation
even with a malformed driver.



Ok, but it won't harm to add a warn here.

Thanks

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 23/31] vdpa: Add custom IOTLB translations to SVQ

[Jason Wang]

在 2022/1/22 上午4:27, Eugenio Pérez 写道:

Use translations added in VhostIOVATree in SVQ.

Only introduce usage here, not allocation and deallocation. As with
previous patches, we use the dead code paths of shadow_vqs_enabled to
avoid commiting too many changes at once. These are impossible to take
at the moment.

Signed-off-by: Eugenio Pérez 
---
  hw/virtio/vhost-shadow-virtqueue.h |   3 +-
  include/hw/virtio/vhost-vdpa.h |   3 +
  hw/virtio/vhost-shadow-virtqueue.c | 111 
  hw/virtio/vhost-vdpa.c | 161 +
  4 files changed, 238 insertions(+), 40 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
b/hw/virtio/vhost-shadow-virtqueue.h
index 19c934af49..c6f67d6f76 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -12,6 +12,7 @@
  
  #include "hw/virtio/vhost.h"

  #include "qemu/event_notifier.h"
+#include "hw/virtio/vhost-iova-tree.h"
  
  typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
  
@@ -37,7 +38,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,

   VirtQueue *vq);
  void vhost_svq_stop(VhostShadowVirtqueue *svq);
  
-VhostShadowVirtqueue *vhost_svq_new(uint16_t qsize);

+VhostShadowVirtqueue *vhost_svq_new(uint16_t qsize, VhostIOVATree *iova_map);
  
  void vhost_svq_free(VhostShadowVirtqueue *vq);
  
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h

index 009a9f3b6b..cd2388b3be 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -14,6 +14,7 @@
  
  #include 
  
+#include "hw/virtio/vhost-iova-tree.h"

  #include "hw/virtio/virtio.h"
  #include "standard-headers/linux/vhost_types.h"
  
@@ -30,6 +31,8 @@ typedef struct vhost_vdpa {

  MemoryListener listener;
  struct vhost_vdpa_iova_range iova_range;
  bool shadow_vqs_enabled;
+/* IOVA mapping used by Shadow Virtqueue */
+VhostIOVATree *iova_tree;
  GPtrArray *shadow_vqs;
  struct vhost_dev *dev;
  VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
b/hw/virtio/vhost-shadow-virtqueue.c
index a1a404f68f..c7888eb8cf 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -11,6 +11,7 @@
  #include "hw/virtio/vhost-shadow-virtqueue.h"
  #include "hw/virtio/vhost.h"
  #include "hw/virtio/virtio-access.h"
+#include "hw/virtio/vhost-iova-tree.h"
  #include "standard-headers/linux/vhost_types.h"
  
  #include "qemu/error-report.h"

@@ -45,6 +46,9 @@ typedef struct VhostShadowVirtqueue {
  /* Virtio device */
  VirtIODevice *vdev;
  
+/* IOVA mapping */

+VhostIOVATree *iova_tree;
+
  /* Map for returning guest's descriptors */
  VirtQueueElement **ring_id_maps;
  
@@ -97,13 +101,7 @@ bool vhost_svq_valid_device_features(uint64_t *dev_features)

  continue;
  
  case VIRTIO_F_ACCESS_PLATFORM:

-/* SVQ does not know how to translate addresses */
-if (*dev_features & BIT_ULL(b)) {
-clear_bit(b, dev_features);
-r = false;
-}
-break;
-
+/* SVQ trust in host's IOMMU to translate addresses */
  case VIRTIO_F_VERSION_1:
  /* SVQ trust that guest vring is little endian */
  if (!(*dev_features & BIT_ULL(b))) {
@@ -205,7 +203,55 @@ static void 
vhost_svq_set_notification(VhostShadowVirtqueue *svq, bool enable)
  }
  }
  
+/**

+ * Translate addresses between qemu's virtual address and SVQ IOVA
+ *
+ * @svqShadow VirtQueue
+ * @vaddr  Translated IOVA addresses
+ * @iovec  Source qemu's VA addresses
+ * @numLength of iovec and minimum length of vaddr
+ */
+static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
+ void **addrs, const struct iovec *iovec,
+ size_t num)
+{
+size_t i;
+
+if (num == 0) {
+return true;
+}
+
+for (i = 0; i < num; ++i) {
+DMAMap needle = {
+.translated_addr = (hwaddr)iovec[i].iov_base,
+.size = iovec[i].iov_len,
+};
+size_t off;
+
+const DMAMap *map = vhost_iova_tree_find_iova(svq->iova_tree, &needle);
+/*
+ * Map cannot be NULL since iova map contains all guest space and
+ * qemu already has a physical address mapped
+ */
+if (unlikely(!map)) {
+error_report("Invalid address 0x%"HWADDR_PRIx" given by guest",
+ needle.translated_addr);



This can be triggered by guest, we need use once or log_guest_error() etc.



+return false;
+}
+
+/*
+ * Map->iova chunk size is ignored. What to do if descriptor
+ * (addr, size) does not fit is delegated to the device.
+ */



I think we need at least check the size and fail if the size doesn't 
match h