[PATCH v2 00/19] Control VQ support in vDPA

[PATCH v2 00/19] Control VQ support in vDPA

[Gautam Dawar]

Hi All:

This series tries to add the support for control virtqueue in vDPA.

Control virtqueue is used by networking device for accepting various
commands from the driver. It's a must to support multiqueue and other
configurations.

When used by vhost-vDPA bus driver for VM, the control virtqueue
should be shadowed via userspace VMM (Qemu) instead of being assigned
directly to Guest. This is because Qemu needs to know the device state
in order to start and stop device correctly (e.g for Live Migration).

This requies to isolate the memory mapping for control virtqueue
presented by vhost-vDPA to prevent guest from accessing it directly.

To achieve this, vDPA introduce two new abstractions:

- address space: identified through address space id (ASID) and a set
                 of memory mapping in maintained
- virtqueue group: the minimal set of virtqueues that must share an
                 address space

Device needs to advertise the following attributes to vDPA:

- the number of address spaces supported in the device
- the number of virtqueue groups supported in the device
- the mappings from a specific virtqueue to its virtqueue groups

The mappings from virtqueue to virtqueue groups is fixed and defined
by vDPA device driver. E.g:

- For the device that has hardware ASID support, it can simply
  advertise a per virtqueue group.
- For the device that does not have hardware ASID support, it can
  simply advertise a single virtqueue group that contains all
  virtqueues. Or if it wants a software emulated control virtqueue, it
  can advertise two virtqueue groups, one is for cvq, another is for
  the rest virtqueues.

vDPA also allow to change the association between virtqueue group and
address space. So in the case of control virtqueue, userspace
VMM(Qemu) may use a dedicated address space for the control virtqueue
group to isolate the memory mapping.

The vhost/vhost-vDPA is also extend for the userspace to:

- query the number of virtqueue groups and address spaces supported by
  the device
- query the virtqueue group for a specific virtqueue
- assocaite a virtqueue group with an address space
- send ASID based IOTLB commands

This will help userspace VMM(Qemu) to detect whether the control vq
could be supported and isolate memory mappings of control virtqueue
from the others.

To demonstrate the usage, vDPA simulator is extended to support
setting MAC address via a emulated control virtqueue.

Please review.

Changes since RFC v2:

- Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
- Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
  changed its return type to void.
- Removed insignificant used_as member field from struct vhost_vdpa.
- Corrected the iommu parameter in call to vringh_set_iotlb() from
  vdpasim_set_group_asid()
- Fixed build errors with vdpa_sim_net
- Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
  call updated vDPA APIs and ensured successful build
- Tested control (MAC address configuration) and data-path using
  single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device 
  and vdpa_sim_net software device using QEMU release at [1]
- Removed two extra blank lines after set_group_asid() in
  include/linux/vdpa.h

Changes since v1:

- Rebased the v1 patch series on vhost branch of MST vhost git repo
  git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
- Updates to accommodate vdpa_sim changes from monolithic module in
  kernel used v1 patch series to current modularized class (net, block)
  based approach.
- Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
  propagated them from vdpa_sim_net to vdpa_sim
- Widened the data-type for "asid" member of vhost_msg_v2 to __u32
  to accommodate PASID
- Fixed the buildbot warnings
- Resolved all checkpatch.pl errors and warnings
- Tested both control and datapath with Xilinx Smartnic SN1000 series
  device using QEMU implementing the Shadow virtqueue and support for
  VQ groups and ASID available at [1]

Changes since RFC:

- tweak vhost uAPI documentation
- switch to use device specific IOTLB really in patch 4
- tweak the commit log
- fix that ASID in vhost is claimed to be 32 actually but 16bit
  actually
- fix use after free when using ASID with IOTLB batching requests
- switch to use Stefano's patch for having separated iov
- remove unused "used_as" variable
- fix the iotlb/asid checking in vhost_vdpa_unmap()

[1] Development QEMU release with support for SVQ, VQ groups and ASID:
  github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
  asid_groups-v1.d%2F00

Thanks

Gautam Dawar (19):
  vhost: move the backend feature bits to vhost_types.h
  virtio-vdpa: don't set callback if virtio doesn't need it
  vhost-vdpa: passing iotlb to IOMMU mapping helpers
  vhost-vdpa: switch to use vhost-vdpa specific IOTLB
  vdpa: introduce virtqueue groups
  vdpa: multiple address spaces support
  vdpa: introduce config operations for associating ASID to a virtqueue
    group
  vhost_iotlb: split out IOTLB initialization
  vhost: support ASID in IOTLB API
  vhost-vdpa: introduce asid based IOTLB
  vhost-vdpa: introduce uAPI to get the number of virtqueue groups
  vhost-vdpa: introduce uAPI to get the number of address spaces
  vhost-vdpa: uAPI to get virtqueue group id
  vhost-vdpa: introduce uAPI to set group ASID
  vhost-vdpa: support ASID based IOTLB API
  vdpa_sim: advertise VIRTIO_NET_F_MTU
  vdpa_sim: factor out buffer completion logic
  vdpa_sim: filter destination mac address
  vdpasim: control virtqueue support

 drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
 drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
 drivers/vdpa/vdpa.c                  |   5 +
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
 drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
 drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
 drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
 drivers/vhost/iotlb.c                |  23 ++-
 drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
 drivers/vhost/vhost.c                |  23 ++-
 drivers/vhost/vhost.h                |   4 +-
 drivers/virtio/virtio_vdpa.c         |   2 +-
 include/linux/vdpa.h                 |  44 ++++-
 include/linux/vhost_iotlb.h          |   2 +
 include/uapi/linux/vhost.h           |  26 ++-
 include/uapi/linux/vhost_types.h     |  11 +-
 18 files changed, 563 insertions(+), 142 deletions(-)

-- 
2.30.1

[PATCH v2 01/19] vhost: move the backend feature bits to vhost_types.h

[Gautam Dawar]

We should store feature bits in vhost_types.h as what has been done
for e.g VHOST_F_LOG_ALL.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 include/uapi/linux/vhost.h       | 5 -----
 include/uapi/linux/vhost_types.h | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 5d99e7c242a2..8f7b4a95d6f9 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -89,11 +89,6 @@
 
 /* Set or get vhost backend capability */
 
-/* Use message type V2 */
-#define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
-/* IOTLB can accept batching hints */
-#define VHOST_BACKEND_F_IOTLB_BATCH  0x2
-
 #define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
 #define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64)
 
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index f7f6a3a28977..76ee7016c501 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -153,4 +153,9 @@ struct vhost_vdpa_iova_range {
 /* vhost-net should add virtio_net_hdr for RX, and strip for TX packets. */
 #define VHOST_NET_F_VIRTIO_NET_HDR 27
 
+/* Use message type V2 */
+#define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
+/* IOTLB can accept batching hints */
+#define VHOST_BACKEND_F_IOTLB_BATCH  0x2
+
 #endif
-- 
2.30.1

[PATCH v2 02/19] virtio-vdpa: don't set callback if virtio doesn't need it

[Gautam Dawar]

There's no need for setting callbacks for the driver that doesn't care
about that.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/virtio/virtio_vdpa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 76504559bc25..46c71653f508 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -184,7 +184,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
 	}
 
 	/* Setup virtqueue callback */
-	cb.callback = virtio_vdpa_virtqueue_cb;
+	cb.callback = callback ? virtio_vdpa_virtqueue_cb : NULL;
 	cb.private = info;
 	ops->set_vq_cb(vdpa, index, &cb);
 	ops->set_vq_num(vdpa, index, virtqueue_get_vring_size(vq));
-- 
2.30.1

[PATCH v2 03/19] vhost-vdpa: passing iotlb to IOMMU mapping helpers

[Gautam Dawar]

To prepare for the ASID support for vhost-vdpa, try to pass IOTLB
object to dma helpers. No functional changes, it's just a preparation
for support multiple IOTLBs.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c | 67 ++++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 4c2f0bd06285..6d670e32e67b 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -537,10 +537,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 	return r;
 }
 
-static void vhost_vdpa_pa_unmap(struct vhost_vdpa *v, u64 start, u64 last)
+static void vhost_vdpa_pa_unmap(struct vhost_vdpa *v,
+				struct vhost_iotlb *iotlb,
+				u64 start, u64 last)
 {
 	struct vhost_dev *dev = &v->vdev;
-	struct vhost_iotlb *iotlb = dev->iotlb;
 	struct vhost_iotlb_map *map;
 	struct page *page;
 	unsigned long pfn, pinned;
@@ -559,10 +560,10 @@ static void vhost_vdpa_pa_unmap(struct vhost_vdpa *v, u64 start, u64 last)
 	}
 }
 
-static void vhost_vdpa_va_unmap(struct vhost_vdpa *v, u64 start, u64 last)
+static void vhost_vdpa_va_unmap(struct vhost_vdpa *v,
+				struct vhost_iotlb *iotlb,
+				u64 start, u64 last)
 {
-	struct vhost_dev *dev = &v->vdev;
-	struct vhost_iotlb *iotlb = dev->iotlb;
 	struct vhost_iotlb_map *map;
 	struct vdpa_map_file *map_file;
 
@@ -574,21 +575,24 @@ static void vhost_vdpa_va_unmap(struct vhost_vdpa *v, u64 start, u64 last)
 	}
 }
 
-static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v, u64 start, u64 last)
+static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v,
+				   struct vhost_iotlb *iotlb,
+				   u64 start, u64 last)
 {
 	struct vdpa_device *vdpa = v->vdpa;
 
 	if (vdpa->use_va)
-		return vhost_vdpa_va_unmap(v, start, last);
+		return vhost_vdpa_va_unmap(v, iotlb, start, last);
 
-	return vhost_vdpa_pa_unmap(v, start, last);
+	return vhost_vdpa_pa_unmap(v, iotlb, start, last);
 }
 
 static void vhost_vdpa_iotlb_free(struct vhost_vdpa *v)
 {
 	struct vhost_dev *dev = &v->vdev;
+	struct vhost_iotlb *iotlb = dev->iotlb;
 
-	vhost_vdpa_iotlb_unmap(v, 0ULL, 0ULL - 1);
+	vhost_vdpa_iotlb_unmap(v, iotlb, 0ULL, 0ULL - 1);
 	kfree(dev->iotlb);
 	dev->iotlb = NULL;
 }
@@ -615,15 +619,15 @@ static int perm_to_iommu_flags(u32 perm)
 	return flags | IOMMU_CACHE;
 }
 
-static int vhost_vdpa_map(struct vhost_vdpa *v, u64 iova,
-			  u64 size, u64 pa, u32 perm, void *opaque)
+static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
+			  u64 iova, u64 size, u64 pa, u32 perm, void *opaque)
 {
 	struct vhost_dev *dev = &v->vdev;
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
 	int r = 0;
 
-	r = vhost_iotlb_add_range_ctx(dev->iotlb, iova, iova + size - 1,
+	r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1,
 				      pa, perm, opaque);
 	if (r)
 		return r;
@@ -632,13 +636,13 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, u64 iova,
 		r = ops->dma_map(vdpa, iova, size, pa, perm, opaque);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			r = ops->set_map(vdpa, dev->iotlb);
+			r = ops->set_map(vdpa, iotlb);
 	} else {
 		r = iommu_map(v->domain, iova, pa, size,
 			      perm_to_iommu_flags(perm));
 	}
 	if (r) {
-		vhost_iotlb_del_range(dev->iotlb, iova, iova + size - 1);
+		vhost_iotlb_del_range(iotlb, iova, iova + size - 1);
 		return r;
 	}
 
@@ -648,25 +652,27 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, u64 iova,
 	return 0;
 }
 
-static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 iova, u64 size)
+static void vhost_vdpa_unmap(struct vhost_vdpa *v,
+			     struct vhost_iotlb *iotlb,
+			     u64 iova, u64 size)
 {
-	struct vhost_dev *dev = &v->vdev;
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
 
-	vhost_vdpa_iotlb_unmap(v, iova, iova + size - 1);
+	vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
 
 	if (ops->dma_map) {
 		ops->dma_unmap(vdpa, iova, size);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			ops->set_map(vdpa, dev->iotlb);
+			ops->set_map(vdpa, iotlb);
 	} else {
 		iommu_unmap(v->domain, iova, size);
 	}
 }
 
 static int vhost_vdpa_va_map(struct vhost_vdpa *v,
+			     struct vhost_iotlb *iotlb,
 			     u64 iova, u64 size, u64 uaddr, u32 perm)
 {
 	struct vhost_dev *dev = &v->vdev;
@@ -696,7 +702,7 @@ static int vhost_vdpa_va_map(struct vhost_vdpa *v,
 		offset = (vma->vm_pgoff << PAGE_SHIFT) + uaddr - vma->vm_start;
 		map_file->offset = offset;
 		map_file->file = get_file(vma->vm_file);
-		ret = vhost_vdpa_map(v, map_iova, map_size, uaddr,
+		ret = vhost_vdpa_map(v, iotlb, map_iova, map_size, uaddr,
 				     perm, map_file);
 		if (ret) {
 			fput(map_file->file);
@@ -709,7 +715,7 @@ static int vhost_vdpa_va_map(struct vhost_vdpa *v,
 		map_iova += map_size;
 	}
 	if (ret)
-		vhost_vdpa_unmap(v, iova, map_iova - iova);
+		vhost_vdpa_unmap(v, iotlb, iova, map_iova - iova);
 
 	mmap_read_unlock(dev->mm);
 
@@ -717,6 +723,7 @@ static int vhost_vdpa_va_map(struct vhost_vdpa *v,
 }
 
 static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
+			     struct vhost_iotlb *iotlb,
 			     u64 iova, u64 size, u64 uaddr, u32 perm)
 {
 	struct vhost_dev *dev = &v->vdev;
@@ -780,7 +787,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			if (last_pfn && (this_pfn != last_pfn + 1)) {
 				/* Pin a contiguous chunk of memory */
 				csize = PFN_PHYS(last_pfn - map_pfn + 1);
-				ret = vhost_vdpa_map(v, iova, csize,
+				ret = vhost_vdpa_map(v, iotlb, iova, csize,
 						     PFN_PHYS(map_pfn),
 						     perm, NULL);
 				if (ret) {
@@ -810,7 +817,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 	}
 
 	/* Pin the rest chunk */
-	ret = vhost_vdpa_map(v, iova, PFN_PHYS(last_pfn - map_pfn + 1),
+	ret = vhost_vdpa_map(v, iotlb, iova, PFN_PHYS(last_pfn - map_pfn + 1),
 			     PFN_PHYS(map_pfn), perm, NULL);
 out:
 	if (ret) {
@@ -830,7 +837,7 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 			for (pfn = map_pfn; pfn <= last_pfn; pfn++)
 				unpin_user_page(pfn_to_page(pfn));
 		}
-		vhost_vdpa_unmap(v, start, size);
+		vhost_vdpa_unmap(v, iotlb, start, size);
 	}
 unlock:
 	mmap_read_unlock(dev->mm);
@@ -841,11 +848,10 @@ static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
 }
 
 static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
+					   struct vhost_iotlb *iotlb,
 					   struct vhost_iotlb_msg *msg)
 {
-	struct vhost_dev *dev = &v->vdev;
 	struct vdpa_device *vdpa = v->vdpa;
-	struct vhost_iotlb *iotlb = dev->iotlb;
 
 	if (msg->iova < v->range.first || !msg->size ||
 	    msg->iova > U64_MAX - msg->size + 1 ||
@@ -857,10 +863,10 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
 		return -EEXIST;
 
 	if (vdpa->use_va)
-		return vhost_vdpa_va_map(v, msg->iova, msg->size,
+		return vhost_vdpa_va_map(v, iotlb, msg->iova, msg->size,
 					 msg->uaddr, msg->perm);
 
-	return vhost_vdpa_pa_map(v, msg->iova, msg->size, msg->uaddr,
+	return vhost_vdpa_pa_map(v, iotlb, msg->iova, msg->size, msg->uaddr,
 				 msg->perm);
 }
 
@@ -870,6 +876,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
 	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
+	struct vhost_iotlb *iotlb = dev->iotlb;
 	int r = 0;
 
 	mutex_lock(&dev->mutex);
@@ -880,17 +887,17 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
 
 	switch (msg->type) {
 	case VHOST_IOTLB_UPDATE:
-		r = vhost_vdpa_process_iotlb_update(v, msg);
+		r = vhost_vdpa_process_iotlb_update(v, iotlb, msg);
 		break;
 	case VHOST_IOTLB_INVALIDATE:
-		vhost_vdpa_unmap(v, msg->iova, msg->size);
+		vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
 		break;
 	case VHOST_IOTLB_BATCH_BEGIN:
 		v->in_batch = true;
 		break;
 	case VHOST_IOTLB_BATCH_END:
 		if (v->in_batch && ops->set_map)
-			ops->set_map(vdpa, dev->iotlb);
+			ops->set_map(vdpa, iotlb);
 		v->in_batch = false;
 		break;
 	default:
-- 
2.30.1

[PATCH v2 04/19] vhost-vdpa: switch to use vhost-vdpa specific IOTLB

[Gautam Dawar]

To ease the implementation of per group ASID support for vDPA
device. This patch switches to use a vhost-vdpa specific IOTLB to
avoid the unnecessary refactoring of the vhost core.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 6d670e32e67b..632c43eb5ecf 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -39,6 +39,7 @@ struct vhost_vdpa {
 	struct vhost_virtqueue *vqs;
 	struct completion completion;
 	struct vdpa_device *vdpa;
+	struct vhost_iotlb *iotlb;
 	struct device dev;
 	struct cdev cdev;
 	atomic_t opened;
@@ -589,12 +590,11 @@ static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v,
 
 static void vhost_vdpa_iotlb_free(struct vhost_vdpa *v)
 {
-	struct vhost_dev *dev = &v->vdev;
-	struct vhost_iotlb *iotlb = dev->iotlb;
+	struct vhost_iotlb *iotlb = v->iotlb;
 
 	vhost_vdpa_iotlb_unmap(v, iotlb, 0ULL, 0ULL - 1);
-	kfree(dev->iotlb);
-	dev->iotlb = NULL;
+	kfree(v->iotlb);
+	v->iotlb = NULL;
 }
 
 static int perm_to_iommu_flags(u32 perm)
@@ -876,7 +876,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
 	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
-	struct vhost_iotlb *iotlb = dev->iotlb;
+	struct vhost_iotlb *iotlb = v->iotlb;
 	int r = 0;
 
 	mutex_lock(&dev->mutex);
@@ -1017,15 +1017,15 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 	vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
 		       vhost_vdpa_process_iotlb_msg);
 
-	dev->iotlb = vhost_iotlb_alloc(0, 0);
-	if (!dev->iotlb) {
+	v->iotlb = vhost_iotlb_alloc(0, 0);
+	if (!v->iotlb) {
 		r = -ENOMEM;
 		goto err_init_iotlb;
 	}
 
 	r = vhost_vdpa_alloc_domain(v);
 	if (r)
-		goto err_init_iotlb;
+		goto err_alloc_domain;
 
 	vhost_vdpa_set_iova_range(v);
 
@@ -1033,6 +1033,8 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 
 	return 0;
 
+err_alloc_domain:
+	vhost_vdpa_iotlb_free(v);
 err_init_iotlb:
 	vhost_dev_cleanup(&v->vdev);
 	kfree(vqs);
-- 
2.30.1

[PATCH v2 05/19] vdpa: introduce virtqueue groups

[Gautam Dawar]

This patch introduces virtqueue groups to vDPA device. The virtqueue
group is the minimal set of virtqueues that must share an address
space. And the address space identifier could only be attached to
a specific virtqueue group.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c    |  8 +++++++-
 drivers/vdpa/mlx5/net/mlx5_vnet.c  |  8 +++++++-
 drivers/vdpa/vdpa.c                |  3 +++
 drivers/vdpa/vdpa_sim/vdpa_sim.c   |  9 ++++++++-
 drivers/vdpa/vdpa_sim/vdpa_sim.h   |  1 +
 drivers/vdpa/vdpa_user/vduse_dev.c |  2 +-
 drivers/vdpa/virtio_pci/vp_vdpa.c  |  2 +-
 include/linux/vdpa.h               | 16 ++++++++++++----
 9 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
index f480d54f308c..3e93c5eb0cf9 100644
--- a/drivers/vdpa/alibaba/eni_vdpa.c
+++ b/drivers/vdpa/alibaba/eni_vdpa.c
@@ -470,7 +470,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 
 	eni_vdpa = vdpa_alloc_device(struct eni_vdpa, vdpa,
-				     dev, &eni_vdpa_ops, NULL, false);
+				     dev, &eni_vdpa_ops, 1, NULL, false);
 	if (IS_ERR(eni_vdpa)) {
 		ENI_ERR(pdev, "failed to allocate vDPA structure\n");
 		return PTR_ERR(eni_vdpa);
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 4366320fb68d..fde33e143246 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -626,6 +626,11 @@ static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev)
 	return  vf->config_size;
 }
 
+static u32 ifcvf_vdpa_get_vq_group(struct vdpa_device *vdpa, u16 idx)
+{
+	return 0;
+}
+
 static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev,
 				  unsigned int offset,
 				  void *buf, unsigned int len)
@@ -704,6 +709,7 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
 	.get_device_id	= ifcvf_vdpa_get_device_id,
 	.get_vendor_id	= ifcvf_vdpa_get_vendor_id,
 	.get_vq_align	= ifcvf_vdpa_get_vq_align,
+	.get_vq_group	= ifcvf_vdpa_get_vq_group,
 	.get_config_size	= ifcvf_vdpa_get_config_size,
 	.get_config	= ifcvf_vdpa_get_config,
 	.set_config	= ifcvf_vdpa_set_config,
@@ -758,7 +764,7 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	pdev = ifcvf_mgmt_dev->pdev;
 	dev = &pdev->dev;
 	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
-				    dev, &ifc_vdpa_ops, name, false);
+				    dev, &ifc_vdpa_ops, 1, name, false);
 	if (IS_ERR(adapter)) {
 		IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
 		return PTR_ERR(adapter);
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 79001301b383..89874624eb2a 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1910,6 +1910,11 @@ static u32 mlx5_vdpa_get_vq_align(struct vdpa_device *vdev)
 	return PAGE_SIZE;
 }
 
+static u32 mlx5_vdpa_get_vq_group(struct vdpa_device *vdpa, u16 idx)
+{
+	return 0;
+}
+
 enum { MLX5_VIRTIO_NET_F_GUEST_CSUM = 1 << 9,
 	MLX5_VIRTIO_NET_F_CSUM = 1 << 10,
 	MLX5_VIRTIO_NET_F_HOST_TSO6 = 1 << 11,
@@ -2454,6 +2459,7 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
 	.get_vq_notification = mlx5_get_vq_notification,
 	.get_vq_irq = mlx5_get_vq_irq,
 	.get_vq_align = mlx5_vdpa_get_vq_align,
+	.get_vq_group = mlx5_vdpa_get_vq_group,
 	.get_device_features = mlx5_vdpa_get_device_features,
 	.set_driver_features = mlx5_vdpa_set_driver_features,
 	.get_driver_features = mlx5_vdpa_get_driver_features,
@@ -2688,7 +2694,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	}
 
 	ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops,
-				 name, false);
+				 1, name, false);
 	if (IS_ERR(ndev))
 		return PTR_ERR(ndev);
 
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 2b75c00b1005..946cc536078c 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -159,6 +159,7 @@ static void vdpa_release_dev(struct device *d)
  * initialized but before registered.
  * @parent: the parent device
  * @config: the bus operations that is supported by this device
+ * @ngroups: number of groups supported by this device
  * @size: size of the parent structure that contains private data
  * @name: name of the vdpa device; optional.
  * @use_va: indicate whether virtual address must be used by this device
@@ -171,6 +172,7 @@ static void vdpa_release_dev(struct device *d)
  */
 struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 					const struct vdpa_config_ops *config,
+					unsigned int ngroups,
 					size_t size, const char *name,
 					bool use_va)
 {
@@ -203,6 +205,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 	vdev->config = config;
 	vdev->features_valid = false;
 	vdev->use_va = use_va;
+	vdev->ngroups = ngroups;
 
 	if (name)
 		err = dev_set_name(&vdev->dev, "%s", name);
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index ddbe142af09a..c98cb1f869fa 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -250,7 +250,7 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	else
 		ops = &vdpasim_config_ops;
 
-	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
+	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
 				    dev_attr->name, false);
 	if (IS_ERR(vdpasim)) {
 		ret = PTR_ERR(vdpasim);
@@ -399,6 +399,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
 	return VDPASIM_QUEUE_ALIGN;
 }
 
+static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
+{
+	return 0;
+}
+
 static u64 vdpasim_get_device_features(struct vdpa_device *vdpa)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -620,6 +625,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
 	.set_vq_state           = vdpasim_set_vq_state,
 	.get_vq_state           = vdpasim_get_vq_state,
 	.get_vq_align           = vdpasim_get_vq_align,
+	.get_vq_group           = vdpasim_get_vq_group,
 	.get_device_features    = vdpasim_get_device_features,
 	.set_driver_features    = vdpasim_set_driver_features,
 	.get_driver_features    = vdpasim_get_driver_features,
@@ -650,6 +656,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.set_vq_state           = vdpasim_set_vq_state,
 	.get_vq_state           = vdpasim_get_vq_state,
 	.get_vq_align           = vdpasim_get_vq_align,
+	.get_vq_group           = vdpasim_get_vq_group,
 	.get_device_features    = vdpasim_get_device_features,
 	.set_driver_features    = vdpasim_set_driver_features,
 	.get_driver_features    = vdpasim_get_driver_features,
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
index cd58e888bcf3..0be7c1e7ef80 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -63,6 +63,7 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
+	u32 groups;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index f85d1a08ed87..4ee6850b9a68 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1495,7 +1495,7 @@ static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
 		return -EEXIST;
 
 	vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
-				 &vduse_vdpa_config_ops, name, true);
+				 &vduse_vdpa_config_ops, 1, name, true);
 	if (IS_ERR(vdev))
 		return PTR_ERR(vdev);
 
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
index cce101e6a940..e18dfe993901 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -466,7 +466,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 
 	vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
-				    dev, &vp_vdpa_ops, NULL, false);
+				    dev, &vp_vdpa_ops, 1, NULL, false);
 	if (IS_ERR(vp_vdpa)) {
 		dev_err(dev, "vp_vdpa: Failed to allocate vDPA structure\n");
 		return PTR_ERR(vp_vdpa);
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 8943a209202e..f7764e097bab 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -85,6 +85,7 @@ struct vdpa_device {
 	bool use_va;
 	u32 nvqs;
 	struct vdpa_mgmt_dev *mdev;
+	unsigned int ngroups;
 };
 
 /**
@@ -172,6 +173,10 @@ struct vdpa_map_file {
  *				for the device
  *				@vdev: vdpa device
  *				Returns virtqueue algin requirement
+ * @get_vq_group:		Get the group id for a specific virtqueue
+ *				@vdev: vdpa device
+ *				@idx: virtqueue index
+ *				Returns u32: group id for this virtqueue
  * @get_device_features:	Get virtio features supported by the device
  *				@vdev: vdpa device
  *				Returns the virtio features support by the
@@ -283,6 +288,7 @@ struct vdpa_config_ops {
 
 	/* Device ops */
 	u32 (*get_vq_align)(struct vdpa_device *vdev);
+	u32 (*get_vq_group)(struct vdpa_device *vdev, u16 idx);
 	u64 (*get_device_features)(struct vdpa_device *vdev);
 	int (*set_driver_features)(struct vdpa_device *vdev, u64 features);
 	u64 (*get_driver_features)(struct vdpa_device *vdev);
@@ -315,6 +321,7 @@ struct vdpa_config_ops {
 
 struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 					const struct vdpa_config_ops *config,
+					unsigned int ngroups,
 					size_t size, const char *name,
 					bool use_va);
 
@@ -325,17 +332,18 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
  * @member: the name of struct vdpa_device within the @dev_struct
  * @parent: the parent device
  * @config: the bus operations that is supported by this device
+ * @ngroups: the number of virtqueue groups supported by this device
  * @name: name of the vdpa device
  * @use_va: indicate whether virtual address must be used by this device
  *
  * Return allocated data structure or ERR_PTR upon error
  */
-#define vdpa_alloc_device(dev_struct, member, parent, config, name, use_va)   \
-			  container_of(__vdpa_alloc_device( \
-				       parent, config, \
+#define vdpa_alloc_device(dev_struct, member, parent, config, ngroups, name, use_va)   \
+			  container_of((__vdpa_alloc_device( \
+				       parent, config, ngroups, \
 				       sizeof(dev_struct) + \
 				       BUILD_BUG_ON_ZERO(offsetof( \
-				       dev_struct, member)), name, use_va), \
+				       dev_struct, member)), name, use_va)), \
 				       dev_struct, member)
 
 int vdpa_register_device(struct vdpa_device *vdev, u32 nvqs);
-- 
2.30.1

[PATCH v2 06/19] vdpa: multiple address spaces support

[Gautam Dawar]

This patches introduces the multiple address spaces support for vDPA
device. This idea is to identify a specific address space via an
dedicated identifier - ASID.

During vDPA device allocation, vDPA device driver needs to report the
number of address spaces supported by the device then the DMA mapping
ops of the vDPA device needs to be extended to support ASID.

This helps to isolate the environments for the virtqueue that will not
be assigned directly. E.g in the case of virtio-net, the control
virtqueue will not be assigned directly to guest.

As a start, simply claim 1 virtqueue groups and 1 address spaces for
all vDPA devices. And vhost-vDPA will simply reject the device with
more than 1 virtqueue groups or address spaces.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c    |  2 +-
 drivers/vdpa/mlx5/net/mlx5_vnet.c  |  5 +++--
 drivers/vdpa/vdpa.c                |  4 +++-
 drivers/vdpa/vdpa_sim/vdpa_sim.c   | 10 ++++++----
 drivers/vdpa/vdpa_user/vduse_dev.c |  3 ++-
 drivers/vdpa/virtio_pci/vp_vdpa.c  |  2 +-
 drivers/vhost/vdpa.c               | 14 +++++++++-----
 include/linux/vdpa.h               | 28 +++++++++++++++++++---------
 9 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
index 3e93c5eb0cf9..5a09a09cca70 100644
--- a/drivers/vdpa/alibaba/eni_vdpa.c
+++ b/drivers/vdpa/alibaba/eni_vdpa.c
@@ -470,7 +470,7 @@ static int eni_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 
 	eni_vdpa = vdpa_alloc_device(struct eni_vdpa, vdpa,
-				     dev, &eni_vdpa_ops, 1, NULL, false);
+				     dev, &eni_vdpa_ops, 1, 1, NULL, false);
 	if (IS_ERR(eni_vdpa)) {
 		ENI_ERR(pdev, "failed to allocate vDPA structure\n");
 		return PTR_ERR(eni_vdpa);
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index fde33e143246..c1767a0ce630 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -764,7 +764,7 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	pdev = ifcvf_mgmt_dev->pdev;
 	dev = &pdev->dev;
 	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
-				    dev, &ifc_vdpa_ops, 1, name, false);
+				    dev, &ifc_vdpa_ops, 1, 1, name, false);
 	if (IS_ERR(adapter)) {
 		IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
 		return PTR_ERR(adapter);
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 89874624eb2a..85dc3e93758e 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2368,7 +2368,8 @@ static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
 	return mvdev->generation;
 }
 
-static int mlx5_vdpa_set_map(struct vdpa_device *vdev, struct vhost_iotlb *iotlb)
+static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
+			     struct vhost_iotlb *iotlb)
 {
 	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
 	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
@@ -2694,7 +2695,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	}
 
 	ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops,
-				 1, name, false);
+				 1, 1, name, false);
 	if (IS_ERR(ndev))
 		return PTR_ERR(ndev);
 
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 946cc536078c..36be04621fb3 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -160,6 +160,7 @@ static void vdpa_release_dev(struct device *d)
  * @parent: the parent device
  * @config: the bus operations that is supported by this device
  * @ngroups: number of groups supported by this device
+ * @nas: number of address spaces supported by this device
  * @size: size of the parent structure that contains private data
  * @name: name of the vdpa device; optional.
  * @use_va: indicate whether virtual address must be used by this device
@@ -172,7 +173,7 @@ static void vdpa_release_dev(struct device *d)
  */
 struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 					const struct vdpa_config_ops *config,
-					unsigned int ngroups,
+					unsigned int ngroups, unsigned int nas,
 					size_t size, const char *name,
 					bool use_va)
 {
@@ -206,6 +207,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 	vdev->features_valid = false;
 	vdev->use_va = use_va;
 	vdev->ngroups = ngroups;
+	vdev->nas = nas;
 
 	if (name)
 		err = dev_set_name(&vdev->dev, "%s", name);
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c98cb1f869fa..659e2e2e4b0c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -251,7 +251,7 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		ops = &vdpasim_config_ops;
 
 	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
-				    dev_attr->name, false);
+				    1, dev_attr->name, false);
 	if (IS_ERR(vdpasim)) {
 		ret = PTR_ERR(vdpasim);
 		goto err_alloc;
@@ -539,7 +539,7 @@ static struct vdpa_iova_range vdpasim_get_iova_range(struct vdpa_device *vdpa)
 	return range;
 }
 
-static int vdpasim_set_map(struct vdpa_device *vdpa,
+static int vdpasim_set_map(struct vdpa_device *vdpa, unsigned int asid,
 			   struct vhost_iotlb *iotlb)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -566,7 +566,8 @@ static int vdpasim_set_map(struct vdpa_device *vdpa,
 	return ret;
 }
 
-static int vdpasim_dma_map(struct vdpa_device *vdpa, u64 iova, u64 size,
+static int vdpasim_dma_map(struct vdpa_device *vdpa, unsigned int asid,
+			   u64 iova, u64 size,
 			   u64 pa, u32 perm, void *opaque)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -580,7 +581,8 @@ static int vdpasim_dma_map(struct vdpa_device *vdpa, u64 iova, u64 size,
 	return ret;
 }
 
-static int vdpasim_dma_unmap(struct vdpa_device *vdpa, u64 iova, u64 size)
+static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid,
+			     u64 iova, u64 size)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 4ee6850b9a68..d503848b3b6e 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -693,6 +693,7 @@ static u32 vduse_vdpa_get_generation(struct vdpa_device *vdpa)
 }
 
 static int vduse_vdpa_set_map(struct vdpa_device *vdpa,
+				unsigned int asid,
 				struct vhost_iotlb *iotlb)
 {
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
@@ -1495,7 +1496,7 @@ static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
 		return -EEXIST;
 
 	vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
-				 &vduse_vdpa_config_ops, 1, name, true);
+				 &vduse_vdpa_config_ops, 1, 1, name, true);
 	if (IS_ERR(vdev))
 		return PTR_ERR(vdev);
 
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
index e18dfe993901..35acba0e8d6d 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -466,7 +466,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 
 	vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
-				    dev, &vp_vdpa_ops, 1, NULL, false);
+				    dev, &vp_vdpa_ops, 1, 1, NULL, false);
 	if (IS_ERR(vp_vdpa)) {
 		dev_err(dev, "vp_vdpa: Failed to allocate vDPA structure\n");
 		return PTR_ERR(vp_vdpa);
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 632c43eb5ecf..9202ff97ddb5 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -633,10 +633,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 		return r;
 
 	if (ops->dma_map) {
-		r = ops->dma_map(vdpa, iova, size, pa, perm, opaque);
+		r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			r = ops->set_map(vdpa, iotlb);
+			r = ops->set_map(vdpa, 0, iotlb);
 	} else {
 		r = iommu_map(v->domain, iova, pa, size,
 			      perm_to_iommu_flags(perm));
@@ -662,10 +662,10 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
 	vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
 
 	if (ops->dma_map) {
-		ops->dma_unmap(vdpa, iova, size);
+		ops->dma_unmap(vdpa, 0, iova, size);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			ops->set_map(vdpa, iotlb);
+			ops->set_map(vdpa, 0, iotlb);
 	} else {
 		iommu_unmap(v->domain, iova, size);
 	}
@@ -897,7 +897,7 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
 		break;
 	case VHOST_IOTLB_BATCH_END:
 		if (v->in_batch && ops->set_map)
-			ops->set_map(vdpa, iotlb);
+			ops->set_map(vdpa, 0, iotlb);
 		v->in_batch = false;
 		break;
 	default:
@@ -1163,6 +1163,10 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
 	int minor;
 	int r;
 
+	/* Only support 1 address space and 1 groups */
+	if (vdpa->ngroups != 1 || vdpa->nas != 1)
+		return -EOPNOTSUPP;
+
 	v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
 	if (!v)
 		return -ENOMEM;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index f7764e097bab..7ab0e29ae466 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -69,6 +69,8 @@ struct vdpa_mgmt_dev;
  * @cf_mutex: Protects get and set access to configuration layout.
  * @index: device index
  * @features_valid: were features initialized? for legacy guests
+ * @ngroups: the number of virtqueue groups
+ * @nas: the number of address spaces
  * @use_va: indicate whether virtual address must be used by this device
  * @nvqs: maximum number of supported virtqueues
  * @mdev: management device pointer; caller must setup when registering device as part
@@ -86,6 +88,7 @@ struct vdpa_device {
 	u32 nvqs;
 	struct vdpa_mgmt_dev *mdev;
 	unsigned int ngroups;
+	unsigned int nas;
 };
 
 /**
@@ -241,6 +244,7 @@ struct vdpa_map_file {
  *				Needed for device that using device
  *				specific DMA translation (on-chip IOMMU)
  *				@vdev: vdpa device
+ *				@asid: address space identifier
  *				@iotlb: vhost memory mapping to be
  *				used by the vDPA
  *				Returns integer: success (0) or error (< 0)
@@ -249,6 +253,7 @@ struct vdpa_map_file {
  *				specific DMA translation (on-chip IOMMU)
  *				and preferring incremental map.
  *				@vdev: vdpa device
+ *				@asid: address space identifier
  *				@iova: iova to be mapped
  *				@size: size of the area
  *				@pa: physical address for the map
@@ -260,6 +265,7 @@ struct vdpa_map_file {
  *				specific DMA translation (on-chip IOMMU)
  *				and preferring incremental unmap.
  *				@vdev: vdpa device
+ *				@asid: address space identifier
  *				@iova: iova to be unmapped
  *				@size: size of the area
  *				Returns integer: success (0) or error (< 0)
@@ -310,10 +316,12 @@ struct vdpa_config_ops {
 	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
 
 	/* DMA ops */
-	int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb);
-	int (*dma_map)(struct vdpa_device *vdev, u64 iova, u64 size,
-		       u64 pa, u32 perm, void *opaque);
-	int (*dma_unmap)(struct vdpa_device *vdev, u64 iova, u64 size);
+	int (*set_map)(struct vdpa_device *vdev, unsigned int asid,
+		       struct vhost_iotlb *iotlb);
+	int (*dma_map)(struct vdpa_device *vdev, unsigned int asid,
+		       u64 iova, u64 size, u64 pa, u32 perm, void *opaque);
+	int (*dma_unmap)(struct vdpa_device *vdev, unsigned int asid,
+			 u64 iova, u64 size);
 
 	/* Free device resources */
 	void (*free)(struct vdpa_device *vdev);
@@ -321,7 +329,7 @@ struct vdpa_config_ops {
 
 struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 					const struct vdpa_config_ops *config,
-					unsigned int ngroups,
+					unsigned int ngroups, unsigned int nas,
 					size_t size, const char *name,
 					bool use_va);
 
@@ -333,17 +341,19 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
  * @parent: the parent device
  * @config: the bus operations that is supported by this device
  * @ngroups: the number of virtqueue groups supported by this device
+ * @nas: the number of address spaces
  * @name: name of the vdpa device
  * @use_va: indicate whether virtual address must be used by this device
  *
  * Return allocated data structure or ERR_PTR upon error
  */
-#define vdpa_alloc_device(dev_struct, member, parent, config, ngroups, name, use_va)   \
+#define vdpa_alloc_device(dev_struct, member, parent, config, ngroups, nas, \
+			  name, use_va) \
 			  container_of((__vdpa_alloc_device( \
-				       parent, config, ngroups, \
-				       sizeof(dev_struct) + \
+				       parent, config, ngroups, nas, \
+				       (sizeof(dev_struct) + \
 				       BUILD_BUG_ON_ZERO(offsetof( \
-				       dev_struct, member)), name, use_va)), \
+				       dev_struct, member))), name, use_va)), \
 				       dev_struct, member)
 
 int vdpa_register_device(struct vdpa_device *vdev, u32 nvqs);
-- 
2.30.1

[PATCH v2 07/19] vdpa: introduce config operations for associating ASID to a virtqueue group

[Gautam Dawar]

This patch introduces a new bus operation to allow the vDPA bus driver
to associate an ASID to a virtqueue group.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 include/linux/vdpa.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 7ab0e29ae466..f4d8c916e0d8 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -240,6 +240,12 @@ struct vdpa_map_file {
  *				@vdev: vdpa device
  *				Returns the iova range supported by
  *				the device.
+ * @set_group_asid:		Set address space identifier for a
+ *				virtqueue group
+ *				@vdev: vdpa device
+ *				@group: virtqueue group
+ *				@asid: address space id for this group
+ *				Returns integer: success (0) or error (< 0)
  * @set_map:			Set device memory mapping (optional)
  *				Needed for device that using device
  *				specific DMA translation (on-chip IOMMU)
@@ -322,6 +328,8 @@ struct vdpa_config_ops {
 		       u64 iova, u64 size, u64 pa, u32 perm, void *opaque);
 	int (*dma_unmap)(struct vdpa_device *vdev, unsigned int asid,
 			 u64 iova, u64 size);
+	int (*set_group_asid)(struct vdpa_device *vdev, unsigned int group,
+			      unsigned int asid);
 
 	/* Free device resources */
 	void (*free)(struct vdpa_device *vdev);
-- 
2.30.1

[PATCH v2 08/19] vhost_iotlb: split out IOTLB initialization

[Gautam Dawar]

This patch splits out IOTLB initialization to make sure it could be
reused by external modules.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/iotlb.c       | 23 ++++++++++++++++++-----
 include/linux/vhost_iotlb.h |  2 ++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/iotlb.c b/drivers/vhost/iotlb.c
index 5829cf2d0552..ea61330a3431 100644
--- a/drivers/vhost/iotlb.c
+++ b/drivers/vhost/iotlb.c
@@ -125,6 +125,23 @@ void vhost_iotlb_del_range(struct vhost_iotlb *iotlb, u64 start, u64 last)
 }
 EXPORT_SYMBOL_GPL(vhost_iotlb_del_range);
 
+/**
+ * vhost_iotlb_init - initialize a vhost IOTLB
+ * @iotlb: the IOTLB that needs to be initialized
+ * @limit: maximum number of IOTLB entries
+ * @flags: VHOST_IOTLB_FLAG_XXX
+ */
+void vhost_iotlb_init(struct vhost_iotlb *iotlb, unsigned int limit,
+		      unsigned int flags)
+{
+	iotlb->root = RB_ROOT_CACHED;
+	iotlb->limit = limit;
+	iotlb->nmaps = 0;
+	iotlb->flags = flags;
+	INIT_LIST_HEAD(&iotlb->list);
+}
+EXPORT_SYMBOL_GPL(vhost_iotlb_init);
+
 /**
  * vhost_iotlb_alloc - add a new vhost IOTLB
  * @limit: maximum number of IOTLB entries
@@ -139,11 +156,7 @@ struct vhost_iotlb *vhost_iotlb_alloc(unsigned int limit, unsigned int flags)
 	if (!iotlb)
 		return NULL;
 
-	iotlb->root = RB_ROOT_CACHED;
-	iotlb->limit = limit;
-	iotlb->nmaps = 0;
-	iotlb->flags = flags;
-	INIT_LIST_HEAD(&iotlb->list);
+	vhost_iotlb_init(iotlb, limit, flags);
 
 	return iotlb;
 }
diff --git a/include/linux/vhost_iotlb.h b/include/linux/vhost_iotlb.h
index 2d0e2f52f938..e79a40838998 100644
--- a/include/linux/vhost_iotlb.h
+++ b/include/linux/vhost_iotlb.h
@@ -36,6 +36,8 @@ int vhost_iotlb_add_range(struct vhost_iotlb *iotlb, u64 start, u64 last,
 			  u64 addr, unsigned int perm);
 void vhost_iotlb_del_range(struct vhost_iotlb *iotlb, u64 start, u64 last);
 
+void vhost_iotlb_init(struct vhost_iotlb *iotlb, unsigned int limit,
+		      unsigned int flags);
 struct vhost_iotlb *vhost_iotlb_alloc(unsigned int limit, unsigned int flags);
 void vhost_iotlb_free(struct vhost_iotlb *iotlb);
 void vhost_iotlb_reset(struct vhost_iotlb *iotlb);
-- 
2.30.1

[PATCH v2 09/19] vhost: support ASID in IOTLB API

[Gautam Dawar]

This patches allows userspace to send ASID based IOTLB message to
vhost. This idea is to use the reserved u32 field in the existing V2
IOTLB message. Vhost device should advertise this capability via
VHOST_BACKEND_F_IOTLB_ASID backend feature.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c             |  5 ++++-
 drivers/vhost/vhost.c            | 23 ++++++++++++++++++-----
 drivers/vhost/vhost.h            |  4 ++--
 include/uapi/linux/vhost_types.h |  6 +++++-
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 9202ff97ddb5..174c9e81df4e 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -870,7 +870,7 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
 				 msg->perm);
 }
 
-static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
+static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
 					struct vhost_iotlb_msg *msg)
 {
 	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
@@ -879,6 +879,9 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
 	struct vhost_iotlb *iotlb = v->iotlb;
 	int r = 0;
 
+	if (asid != 0)
+		return -EINVAL;
+
 	mutex_lock(&dev->mutex);
 
 	r = vhost_dev_check_owner(dev);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d02173fb290c..d1e58f976f6e 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -468,7 +468,7 @@ void vhost_dev_init(struct vhost_dev *dev,
 		    struct vhost_virtqueue **vqs, int nvqs,
 		    int iov_limit, int weight, int byte_weight,
 		    bool use_worker,
-		    int (*msg_handler)(struct vhost_dev *dev,
+		    int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 				       struct vhost_iotlb_msg *msg))
 {
 	struct vhost_virtqueue *vq;
@@ -1090,11 +1090,14 @@ static bool umem_access_ok(u64 uaddr, u64 size, int access)
 	return true;
 }
 
-static int vhost_process_iotlb_msg(struct vhost_dev *dev,
+static int vhost_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
 				   struct vhost_iotlb_msg *msg)
 {
 	int ret = 0;
 
+	if (asid != 0)
+		return -EINVAL;
+
 	mutex_lock(&dev->mutex);
 	vhost_dev_lock_vqs(dev);
 	switch (msg->type) {
@@ -1141,6 +1144,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 	struct vhost_iotlb_msg msg;
 	size_t offset;
 	int type, ret;
+	u32 asid = 0;
 
 	ret = copy_from_iter(&type, sizeof(type), from);
 	if (ret != sizeof(type)) {
@@ -1156,7 +1160,16 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 		offset = offsetof(struct vhost_msg, iotlb) - sizeof(int);
 		break;
 	case VHOST_IOTLB_MSG_V2:
-		offset = sizeof(__u32);
+		if (vhost_backend_has_feature(dev->vqs[0],
+					      VHOST_BACKEND_F_IOTLB_ASID)) {
+			ret = copy_from_iter(&asid, sizeof(asid), from);
+			if (ret != sizeof(asid)) {
+				ret = -EINVAL;
+				goto done;
+			}
+			offset = sizeof(__u16);
+		} else
+			offset = sizeof(__u32);
 		break;
 	default:
 		ret = -EINVAL;
@@ -1178,9 +1191,9 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 	}
 
 	if (dev->msg_handler)
-		ret = dev->msg_handler(dev, &msg);
+		ret = dev->msg_handler(dev, asid, &msg);
 	else
-		ret = vhost_process_iotlb_msg(dev, &msg);
+		ret = vhost_process_iotlb_msg(dev, asid, &msg);
 	if (ret) {
 		ret = -EFAULT;
 		goto done;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 638bb640d6b4..9f238d6c7b58 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -161,7 +161,7 @@ struct vhost_dev {
 	int byte_weight;
 	u64 kcov_handle;
 	bool use_worker;
-	int (*msg_handler)(struct vhost_dev *dev,
+	int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 			   struct vhost_iotlb_msg *msg);
 };
 
@@ -169,7 +169,7 @@ bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
 void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
 		    int nvqs, int iov_limit, int weight, int byte_weight,
 		    bool use_worker,
-		    int (*msg_handler)(struct vhost_dev *dev,
+		    int (*msg_handler)(struct vhost_dev *dev, u32 asid,
 				       struct vhost_iotlb_msg *msg));
 long vhost_dev_set_owner(struct vhost_dev *dev);
 bool vhost_dev_has_owner(struct vhost_dev *dev);
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index 76ee7016c501..634cee485abb 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -87,7 +87,7 @@ struct vhost_msg {
 
 struct vhost_msg_v2 {
 	__u32 type;
-	__u32 reserved;
+	__u32 asid;
 	union {
 		struct vhost_iotlb_msg iotlb;
 		__u8 padding[64];
@@ -157,5 +157,9 @@ struct vhost_vdpa_iova_range {
 #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
 /* IOTLB can accept batching hints */
 #define VHOST_BACKEND_F_IOTLB_BATCH  0x2
+/* IOTLB can accept address space identifier through V2 type of IOTLB
+ * message
+ */
+#define VHOST_BACKEND_F_IOTLB_ASID  0x3
 
 #endif
-- 
2.30.1

[PATCH v2 10/19] vhost-vdpa: introduce asid based IOTLB

[Gautam Dawar]

This patch converts the vhost-vDPA device to support multiple IOTLBs
tagged via ASID via hlist. This will be used for supporting multiple
address spaces in the following patches.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c | 97 ++++++++++++++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 25 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 174c9e81df4e..cd1bee536c46 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -33,13 +33,21 @@ enum {
 
 #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
 
+#define VHOST_VDPA_IOTLB_BUCKETS 16
+
+struct vhost_vdpa_as {
+	struct hlist_node hash_link;
+	struct vhost_iotlb iotlb;
+	u32 id;
+};
+
 struct vhost_vdpa {
 	struct vhost_dev vdev;
 	struct iommu_domain *domain;
 	struct vhost_virtqueue *vqs;
 	struct completion completion;
 	struct vdpa_device *vdpa;
-	struct vhost_iotlb *iotlb;
+	struct hlist_head as[VHOST_VDPA_IOTLB_BUCKETS];
 	struct device dev;
 	struct cdev cdev;
 	atomic_t opened;
@@ -55,6 +63,51 @@ static DEFINE_IDA(vhost_vdpa_ida);
 
 static dev_t vhost_vdpa_major;
 
+static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
+{
+	struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
+	struct vhost_vdpa_as *as;
+
+	hlist_for_each_entry(as, head, hash_link)
+		if (as->id == asid)
+			return as;
+
+	return NULL;
+}
+
+static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
+{
+	struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
+	struct vhost_vdpa_as *as;
+
+	if (asid_to_as(v, asid))
+		return NULL;
+
+	as = kmalloc(sizeof(*as), GFP_KERNEL);
+	if (!as)
+		return NULL;
+
+	vhost_iotlb_init(&as->iotlb, 0, 0);
+	as->id = asid;
+	hlist_add_head(&as->hash_link, head);
+
+	return as;
+}
+
+static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
+{
+	struct vhost_vdpa_as *as = asid_to_as(v, asid);
+
+	if (!as)
+		return -EINVAL;
+
+	hlist_del(&as->hash_link);
+	vhost_iotlb_reset(&as->iotlb);
+	kfree(as);
+
+	return 0;
+}
+
 static void handle_vq_kick(struct vhost_work *work)
 {
 	struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue,
@@ -588,15 +641,6 @@ static void vhost_vdpa_iotlb_unmap(struct vhost_vdpa *v,
 	return vhost_vdpa_pa_unmap(v, iotlb, start, last);
 }
 
-static void vhost_vdpa_iotlb_free(struct vhost_vdpa *v)
-{
-	struct vhost_iotlb *iotlb = v->iotlb;
-
-	vhost_vdpa_iotlb_unmap(v, iotlb, 0ULL, 0ULL - 1);
-	kfree(v->iotlb);
-	v->iotlb = NULL;
-}
-
 static int perm_to_iommu_flags(u32 perm)
 {
 	int flags = 0;
@@ -876,7 +920,8 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
 	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
-	struct vhost_iotlb *iotlb = v->iotlb;
+	struct vhost_vdpa_as *as = asid_to_as(v, 0);
+	struct vhost_iotlb *iotlb = &as->iotlb;
 	int r = 0;
 
 	if (asid != 0)
@@ -987,6 +1032,13 @@ static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v)
 	}
 }
 
+static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
+{
+	vhost_dev_cleanup(&v->vdev);
+	kfree(v->vdev.vqs);
+	vhost_vdpa_remove_as(v, 0);
+}
+
 static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 {
 	struct vhost_vdpa *v;
@@ -1020,15 +1072,12 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 	vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
 		       vhost_vdpa_process_iotlb_msg);
 
-	v->iotlb = vhost_iotlb_alloc(0, 0);
-	if (!v->iotlb) {
-		r = -ENOMEM;
-		goto err_init_iotlb;
-	}
+	if (!vhost_vdpa_alloc_as(v, 0))
+		goto err_alloc_as;
 
 	r = vhost_vdpa_alloc_domain(v);
 	if (r)
-		goto err_alloc_domain;
+		goto err_alloc_as;
 
 	vhost_vdpa_set_iova_range(v);
 
@@ -1036,11 +1085,8 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 
 	return 0;
 
-err_alloc_domain:
-	vhost_vdpa_iotlb_free(v);
-err_init_iotlb:
-	vhost_dev_cleanup(&v->vdev);
-	kfree(vqs);
+err_alloc_as:
+	vhost_vdpa_cleanup(v);
 err:
 	atomic_dec(&v->opened);
 	return r;
@@ -1064,11 +1110,9 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep)
 	vhost_vdpa_clean_irq(v);
 	vhost_vdpa_reset(v);
 	vhost_dev_stop(&v->vdev);
-	vhost_vdpa_iotlb_free(v);
 	vhost_vdpa_free_domain(v);
 	vhost_vdpa_config_put(v);
 	vhost_dev_cleanup(&v->vdev);
-	kfree(v->vdev.vqs);
 	mutex_unlock(&d->mutex);
 
 	atomic_dec(&v->opened);
@@ -1164,7 +1208,7 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
 	const struct vdpa_config_ops *ops = vdpa->config;
 	struct vhost_vdpa *v;
 	int minor;
-	int r;
+	int i, r;
 
 	/* Only support 1 address space and 1 groups */
 	if (vdpa->ngroups != 1 || vdpa->nas != 1)
@@ -1212,6 +1256,9 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
 	init_completion(&v->completion);
 	vdpa_set_drvdata(vdpa, v);
 
+	for (i = 0; i < VHOST_VDPA_IOTLB_BUCKETS; i++)
+		INIT_HLIST_HEAD(&v->as[i]);
+
 	return 0;
 
 err:
-- 
2.30.1

[PATCH v2 11/19] vhost-vdpa: introduce uAPI to get the number of virtqueue groups

[Gautam Dawar]

Follows the vDPA support for multiple address spaces, this patch
introduce uAPI for the userspace to know the number of virtqueue
groups supported by the vDPA device.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c       | 4 ++++
 include/uapi/linux/vhost.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index cd1bee536c46..92f78df0f685 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -559,6 +559,10 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 	case VHOST_VDPA_GET_VRING_NUM:
 		r = vhost_vdpa_get_vring_num(v, argp);
 		break;
+	case VHOST_VDPA_GET_GROUP_NUM:
+		r = copy_to_user(argp, &v->vdpa->ngroups,
+				 sizeof(v->vdpa->ngroups));
+		break;
 	case VHOST_SET_LOG_BASE:
 	case VHOST_SET_LOG_FD:
 		r = -ENOIOCTLCMD;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 8f7b4a95d6f9..61317c61d768 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -145,11 +145,13 @@
 /* Get the valid iova range */
 #define VHOST_VDPA_GET_IOVA_RANGE	_IOR(VHOST_VIRTIO, 0x78, \
 					     struct vhost_vdpa_iova_range)
-
 /* Get the config size */
 #define VHOST_VDPA_GET_CONFIG_SIZE	_IOR(VHOST_VIRTIO, 0x79, __u32)
 
 /* Get the count of all virtqueues */
 #define VHOST_VDPA_GET_VQS_COUNT	_IOR(VHOST_VIRTIO, 0x80, __u32)
 
+/* Get the number of virtqueue groups. */
+#define VHOST_VDPA_GET_GROUP_NUM	_IOR(VHOST_VIRTIO, 0x81, __u32)
+
 #endif
-- 
2.30.1

[PATCH v2 12/19] vhost-vdpa: introduce uAPI to get the number of address spaces

[Gautam Dawar]

This patch introduces the uAPI for getting the number of address
spaces supported by this vDPA device.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c       | 3 +++
 include/uapi/linux/vhost.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 92f78df0f685..a017011ad1f5 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -563,6 +563,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 		r = copy_to_user(argp, &v->vdpa->ngroups,
 				 sizeof(v->vdpa->ngroups));
 		break;
+	case VHOST_VDPA_GET_AS_NUM:
+		r = copy_to_user(argp, &v->vdpa->nas, sizeof(v->vdpa->nas));
+		break;
 	case VHOST_SET_LOG_BASE:
 	case VHOST_SET_LOG_FD:
 		r = -ENOIOCTLCMD;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 61317c61d768..51322008901a 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -154,4 +154,6 @@
 /* Get the number of virtqueue groups. */
 #define VHOST_VDPA_GET_GROUP_NUM	_IOR(VHOST_VIRTIO, 0x81, __u32)
 
+/* Get the number of address spaces. */
+#define VHOST_VDPA_GET_AS_NUM		_IOR(VHOST_VIRTIO, 0x7A, unsigned int)
 #endif
-- 
2.30.1

[PATCH v2 13/19] vhost-vdpa: uAPI to get virtqueue group id

[Gautam Dawar]

Follows the support for virtqueue group in vDPA. This patches
introduces uAPI to get the virtqueue group ID for a specific virtqueue
in vhost-vdpa.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c       | 8 ++++++++
 include/uapi/linux/vhost.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a017011ad1f5..aa5cacdc5263 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -465,6 +465,14 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 			return -EFAULT;
 		ops->set_vq_ready(vdpa, idx, s.num);
 		return 0;
+	case VHOST_VDPA_GET_VRING_GROUP:
+		s.index = idx;
+		s.num = ops->get_vq_group(vdpa, idx);
+		if (s.num >= vdpa->ngroups)
+			return -EIO;
+		else if (copy_to_user(argp, &s, sizeof(s)))
+			return -EFAULT;
+		return 0;
 	case VHOST_GET_VRING_BASE:
 		r = ops->get_vq_state(v->vdpa, idx, &vq_state);
 		if (r)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 51322008901a..668914c87f74 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -156,4 +156,12 @@
 
 /* Get the number of address spaces. */
 #define VHOST_VDPA_GET_AS_NUM		_IOR(VHOST_VIRTIO, 0x7A, unsigned int)
+
+/* Get the group for a virtqueue: read index, write group in num,
+ * The virtqueue index is stored in the index field of
+ * vhost_vring_state. The group for this specific virtqueue is
+ * returned via num field of vhost_vring_state.
+ */
+#define VHOST_VDPA_GET_VRING_GROUP	_IOWR(VHOST_VIRTIO, 0x7B,	\
+					      struct vhost_vring_state)
 #endif
-- 
2.30.1

[PATCH v2 14/19] vhost-vdpa: introduce uAPI to set group ASID

[Gautam Dawar]

Follows the vDPA support for associating ASID to a specific virtqueue
group. This patch adds a uAPI to support setting them from userspace.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c       | 8 ++++++++
 include/uapi/linux/vhost.h | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index aa5cacdc5263..6c7ee0f18892 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -473,6 +473,14 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 		else if (copy_to_user(argp, &s, sizeof(s)))
 			return -EFAULT;
 		return 0;
+	case VHOST_VDPA_SET_GROUP_ASID:
+		if (copy_from_user(&s, argp, sizeof(s)))
+			return -EFAULT;
+		if (s.num >= vdpa->nas)
+			return -EINVAL;
+		if (!ops->set_group_asid)
+			return -EOPNOTSUPP;
+		return ops->set_group_asid(vdpa, idx, s.num);
 	case VHOST_GET_VRING_BASE:
 		r = ops->get_vq_state(v->vdpa, idx, &vq_state);
 		if (r)
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 668914c87f74..cab645d4a645 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -164,4 +164,11 @@
  */
 #define VHOST_VDPA_GET_VRING_GROUP	_IOWR(VHOST_VIRTIO, 0x7B,	\
 					      struct vhost_vring_state)
+/* Set the ASID for a virtqueue group. The group index is stored in
+ * the index field of vhost_vring_state, the ASID associated with this
+ * group is stored at num field of vhost_vring_state.
+ */
+#define VHOST_VDPA_SET_GROUP_ASID	_IOW(VHOST_VIRTIO, 0x7C, \
+					     struct vhost_vring_state)
+
 #endif
-- 
2.30.1

[PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Gautam Dawar]

This patch extends the vhost-vdpa to support ASID based IOTLB API. The
vhost-vdpa device will allocated multiple IOTLBs for vDPA device that
supports multiple address spaces. The IOTLBs and vDPA device memory
mappings is determined and maintained through ASID.

Note that we still don't support vDPA device with more than one
address spaces that depends on platform IOMMU. This work will be done
by moving the IOMMU logic from vhost-vDPA to vDPA device driver.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
 drivers/vhost/vhost.c |   2 +-
 2 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 6c7ee0f18892..1f1d1c425573 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -28,7 +28,8 @@
 enum {
 	VHOST_VDPA_BACKEND_FEATURES =
 	(1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
-	(1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
+	(1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
+	(1ULL << VHOST_BACKEND_F_IOTLB_ASID),
 };
 
 #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
@@ -57,12 +58,20 @@ struct vhost_vdpa {
 	struct eventfd_ctx *config_ctx;
 	int in_batch;
 	struct vdpa_iova_range range;
+	u32 batch_asid;
 };
 
 static DEFINE_IDA(vhost_vdpa_ida);
 
 static dev_t vhost_vdpa_major;
 
+static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb)
+{
+	struct vhost_vdpa_as *as = container_of(iotlb, struct
+						vhost_vdpa_as, iotlb);
+	return as->id;
+}
+
 static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
 {
 	struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
@@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
 	return NULL;
 }
 
+static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32 asid)
+{
+	struct vhost_vdpa_as *as = asid_to_as(v, asid);
+
+	if (!as)
+		return NULL;
+
+	return &as->iotlb;
+}
+
 static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
 {
 	struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
@@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
 	if (asid_to_as(v, asid))
 		return NULL;
 
+	if (asid >= v->vdpa->nas)
+		return NULL;
+
 	as = kmalloc(sizeof(*as), GFP_KERNEL);
 	if (!as)
 		return NULL;
@@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
 	return as;
 }
 
+static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
+						      u32 asid)
+{
+	struct vhost_vdpa_as *as = asid_to_as(v, asid);
+
+	if (as)
+		return as;
+
+	return vhost_vdpa_alloc_as(v, asid);
+}
+
 static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
 {
 	struct vhost_vdpa_as *as = asid_to_as(v, asid);
@@ -692,6 +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 	struct vhost_dev *dev = &v->vdev;
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
+	u32 asid = iotlb_to_asid(iotlb);
 	int r = 0;
 
 	r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1,
@@ -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
 		return r;
 
 	if (ops->dma_map) {
-		r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
+		r = ops->dma_map(vdpa, asid, iova, size, pa, perm, opaque);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			r = ops->set_map(vdpa, 0, iotlb);
+			r = ops->set_map(vdpa, asid, iotlb);
 	} else {
 		r = iommu_map(v->domain, iova, pa, size,
 			      perm_to_iommu_flags(perm));
@@ -725,17 +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
 {
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
+	u32 asid = iotlb_to_asid(iotlb);
 
 	vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
 
 	if (ops->dma_map) {
-		ops->dma_unmap(vdpa, 0, iova, size);
+		ops->dma_unmap(vdpa, asid, iova, size);
 	} else if (ops->set_map) {
 		if (!v->in_batch)
-			ops->set_map(vdpa, 0, iotlb);
+			ops->set_map(vdpa, asid, iotlb);
 	} else {
 		iommu_unmap(v->domain, iova, size);
 	}
+
+	/* If we are in the middle of batch processing, delay the free
+	 * of AS until BATCH_END.
+	 */
+	if (!v->in_batch && !iotlb->nmaps)
+		vhost_vdpa_remove_as(v, asid);
 }
 
 static int vhost_vdpa_va_map(struct vhost_vdpa *v,
@@ -943,19 +984,38 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
 	struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
 	struct vdpa_device *vdpa = v->vdpa;
 	const struct vdpa_config_ops *ops = vdpa->config;
-	struct vhost_vdpa_as *as = asid_to_as(v, 0);
-	struct vhost_iotlb *iotlb = &as->iotlb;
+	struct vhost_iotlb *iotlb = NULL;
+	struct vhost_vdpa_as *as = NULL;
 	int r = 0;
 
-	if (asid != 0)
-		return -EINVAL;
-
 	mutex_lock(&dev->mutex);
 
 	r = vhost_dev_check_owner(dev);
 	if (r)
 		goto unlock;
 
+	if (msg->type == VHOST_IOTLB_UPDATE ||
+	    msg->type == VHOST_IOTLB_BATCH_BEGIN) {
+		as = vhost_vdpa_find_alloc_as(v, asid);
+		if (!as) {
+			dev_err(&v->dev, "can't find and alloc asid %d\n",
+				asid);
+			return -EINVAL;
+		}
+		iotlb = &as->iotlb;
+	} else
+		iotlb = asid_to_iotlb(v, asid);
+
+	if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
+		if (v->in_batch && v->batch_asid != asid) {
+			dev_info(&v->dev, "batch id %d asid %d\n",
+				 v->batch_asid, asid);
+		}
+		if (!iotlb)
+			dev_err(&v->dev, "no iotlb for asid %d\n", asid);
+		return -EINVAL;
+	}
+
 	switch (msg->type) {
 	case VHOST_IOTLB_UPDATE:
 		r = vhost_vdpa_process_iotlb_update(v, iotlb, msg);
@@ -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
 		vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
 		break;
 	case VHOST_IOTLB_BATCH_BEGIN:
+		v->batch_asid = asid;
 		v->in_batch = true;
 		break;
 	case VHOST_IOTLB_BATCH_END:
 		if (v->in_batch && ops->set_map)
-			ops->set_map(vdpa, 0, iotlb);
+			ops->set_map(vdpa, asid, iotlb);
 		v->in_batch = false;
+		if (!iotlb->nmaps)
+			vhost_vdpa_remove_as(v, asid);
 		break;
 	default:
 		r = -EINVAL;
@@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v)
 
 static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
 {
+	struct vhost_vdpa_as *as;
+	u32 asid;
+
 	vhost_dev_cleanup(&v->vdev);
 	kfree(v->vdev.vqs);
-	vhost_vdpa_remove_as(v, 0);
+
+	for (asid = 0; asid < v->vdpa->nas; asid++) {
+		as = asid_to_as(v, asid);
+		if (as)
+			vhost_vdpa_remove_as(v, asid);
+	}
 }
 
 static int vhost_vdpa_open(struct inode *inode, struct file *filep)
@@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 	vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
 		       vhost_vdpa_process_iotlb_msg);
 
-	if (!vhost_vdpa_alloc_as(v, 0))
-		goto err_alloc_as;
-
 	r = vhost_vdpa_alloc_domain(v);
 	if (r)
-		goto err_alloc_as;
+		goto err_alloc_domain;
 
 	vhost_vdpa_set_iova_range(v);
 
@@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 
 	return 0;
 
-err_alloc_as:
+err_alloc_domain:
 	vhost_vdpa_cleanup(v);
 err:
 	atomic_dec(&v->opened);
@@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
 	int minor;
 	int i, r;
 
-	/* Only support 1 address space and 1 groups */
-	if (vdpa->ngroups != 1 || vdpa->nas != 1)
+	/* We can't support platform IOMMU device with more than 1
+	 * group or as
+	 */
+	if (!ops->set_map && !ops->dma_map &&
+	    (vdpa->ngroups > 1 || vdpa->nas > 1))
 		return -EOPNOTSUPP;
 
 	v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d1e58f976f6e..5022c648d9c0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 				ret = -EINVAL;
 				goto done;
 			}
-			offset = sizeof(__u16);
+			offset = 0;
 		} else
 			offset = sizeof(__u32);
 		break;
-- 
2.30.1

[PATCH v2 16/19] vdpa_sim: advertise VIRTIO_NET_F_MTU

[Gautam Dawar]

We've already reported maximum mtu via config space, so let's
advertise the feature.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index d5324f6fd8c7..2d1d8c59d0ea 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -26,7 +26,8 @@
 #define DRV_LICENSE  "GPL v2"
 
 #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
-				 (1ULL << VIRTIO_NET_F_MAC))
+				 (1ULL << VIRTIO_NET_F_MAC) | \
+				 (1ULL << VIRTIO_NET_F_MTU))
 
 #define VDPASIM_NET_VQ_NUM	2
 
-- 
2.30.1

[PATCH v2 17/19] vdpa_sim: factor out buffer completion logic

[Gautam Dawar]

Wrap up common buffer completion logic in to vdpasim_net_complete

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 33 +++++++++++++++-------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index 2d1d8c59d0ea..f4607172b0b8 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -31,6 +31,22 @@
 
 #define VDPASIM_NET_VQ_NUM	2
 
+static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
+{
+	/* Make sure data is wrote before advancing index */
+	smp_wmb();
+
+	vringh_complete_iotlb(&vq->vring, vq->head, len);
+
+	/* Make sure used is visible before rasing the interrupt. */
+	smp_wmb();
+
+	local_bh_disable();
+	if (vringh_need_notify_iotlb(&vq->vring) > 0)
+		vringh_notify(&vq->vring);
+	local_bh_enable();
+}
+
 static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
@@ -78,21 +94,8 @@ static void vdpasim_net_work(struct work_struct *work)
 			total_write += write;
 		}
 
-		/* Make sure data is wrote before advancing index */
-		smp_wmb();
-
-		vringh_complete_iotlb(&txq->vring, txq->head, 0);
-		vringh_complete_iotlb(&rxq->vring, rxq->head, total_write);
-
-		/* Make sure used is visible before rasing the interrupt. */
-		smp_wmb();
-
-		local_bh_disable();
-		if (vringh_need_notify_iotlb(&txq->vring) > 0)
-			vringh_notify(&txq->vring);
-		if (vringh_need_notify_iotlb(&rxq->vring) > 0)
-			vringh_notify(&rxq->vring);
-		local_bh_enable();
+		vdpasim_net_complete(txq, 0);
+		vdpasim_net_complete(rxq, total_write);
 
 		if (++pkts > 4) {
 			schedule_work(&vdpasim->work);
-- 
2.30.1

[PATCH v2 18/19] vdpa_sim: filter destination mac address

[Gautam Dawar]

This patch implements a simple unicast filter for vDPA simulator.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 49 ++++++++++++++++++----------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index f4607172b0b8..5fa59d4fddc8 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -47,13 +47,28 @@ static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
 	local_bh_enable();
 }
 
+static bool receive_filter(struct vdpasim *vdpasim, size_t len)
+{
+	bool modern = vdpasim->features & (1ULL << VIRTIO_F_VERSION_1);
+	size_t hdr_len = modern ? sizeof(struct virtio_net_hdr_v1) :
+				  sizeof(struct virtio_net_hdr);
+	struct virtio_net_config *vio_config = vdpasim->config;
+
+	if (len < ETH_ALEN + hdr_len)
+		return false;
+
+	if (!strncmp(vdpasim->buffer + hdr_len, vio_config->mac, ETH_ALEN))
+		return true;
+
+	return false;
+}
+
 static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
 	struct vdpasim_virtqueue *txq = &vdpasim->vqs[1];
 	struct vdpasim_virtqueue *rxq = &vdpasim->vqs[0];
 	ssize_t read, write;
-	size_t total_write;
 	int pkts = 0;
 	int err;
 
@@ -66,36 +81,34 @@ static void vdpasim_net_work(struct work_struct *work)
 		goto out;
 
 	while (true) {
-		total_write = 0;
 		err = vringh_getdesc_iotlb(&txq->vring, &txq->out_iov, NULL,
 					   &txq->head, GFP_ATOMIC);
 		if (err <= 0)
 			break;
 
+		read = vringh_iov_pull_iotlb(&txq->vring, &txq->out_iov,
+					     vdpasim->buffer,
+					     PAGE_SIZE);
+
+		if (!receive_filter(vdpasim, read)) {
+			vdpasim_net_complete(txq, 0);
+			continue;
+		}
+
 		err = vringh_getdesc_iotlb(&rxq->vring, NULL, &rxq->in_iov,
 					   &rxq->head, GFP_ATOMIC);
 		if (err <= 0) {
-			vringh_complete_iotlb(&txq->vring, txq->head, 0);
+			vdpasim_net_complete(txq, 0);
 			break;
 		}
 
-		while (true) {
-			read = vringh_iov_pull_iotlb(&txq->vring, &txq->out_iov,
-						     vdpasim->buffer,
-						     PAGE_SIZE);
-			if (read <= 0)
-				break;
-
-			write = vringh_iov_push_iotlb(&rxq->vring, &rxq->in_iov,
-						      vdpasim->buffer, read);
-			if (write <= 0)
-				break;
-
-			total_write += write;
-		}
+		write = vringh_iov_push_iotlb(&rxq->vring, &rxq->in_iov,
+					      vdpasim->buffer, read);
+		if (write <= 0)
+			break;
 
 		vdpasim_net_complete(txq, 0);
-		vdpasim_net_complete(rxq, total_write);
+		vdpasim_net_complete(rxq, write);
 
 		if (++pkts > 4) {
 			schedule_work(&vdpasim->work);
-- 
2.30.1

[PATCH v2 19/19] vdpasim: control virtqueue support

[Gautam Dawar]

This patch introduces the control virtqueue support for vDPA
simulator. This is a requirement for supporting advanced features like
multiqueue.

A requirement for control virtqueue is to isolate its memory access
from the rx/tx virtqueues. This is because when using vDPA device
for VM, the control virqueue is not directly assigned to VM. Userspace
(Qemu) will present a shadow control virtqueue to control for
recording the device states.

The isolation is done via the virtqueue groups and ASID support in
vDPA through vhost-vdpa. The simulator is extended to have:

1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
   contains CVQ
3) two address spaces and the simulator simply implements the address
   spaces by mapping it 1:1 to IOTLB.

For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
to group 1. So we have:

1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
   RX and TX can be assigned to guest directly.
2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
   is the buffers that allocated and managed by VMM only. So CVQ of
   vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
   of vhost-vdpa.

For the other use cases, since AS 0 is associated to all virtqueue
groups by default. All virtqueues share the same mapping by default.

To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
implemented in the simulator for the driver to set mac address.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
 drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
 3 files changed, 161 insertions(+), 20 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 659e2e2e4b0c..51bd0bafce06 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
 {
 	int i;
 
-	for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
+	spin_lock(&vdpasim->iommu_lock);
+
+	for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
 		vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
+		vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
+				 &vdpasim->iommu_lock);
+	}
+
+	for (i = 0; i < vdpasim->dev_attr.nas; i++)
+		vhost_iotlb_reset(&vdpasim->iommu[i]);
 
-	spin_lock(&vdpasim->iommu_lock);
-	vhost_iotlb_reset(vdpasim->iommu);
 	spin_unlock(&vdpasim->iommu_lock);
 
 	vdpasim->features = 0;
@@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
 	dma_addr = iova_dma_addr(&vdpasim->iova, iova);
 
 	spin_lock(&vdpasim->iommu_lock);
-	ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
+	ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
 				    (u64)dma_addr + size - 1, (u64)paddr, perm);
 	spin_unlock(&vdpasim->iommu_lock);
 
@@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
 				size_t size)
 {
 	spin_lock(&vdpasim->iommu_lock);
-	vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
+	vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
 			      (u64)dma_addr + size - 1);
 	spin_unlock(&vdpasim->iommu_lock);
 
@@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	else
 		ops = &vdpasim_config_ops;
 
-	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
-				    1, dev_attr->name, false);
+	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
+				    dev_attr->ngroups, dev_attr->nas,
+				    dev_attr->name, false);
 	if (IS_ERR(vdpasim)) {
 		ret = PTR_ERR(vdpasim);
 		goto err_alloc;
@@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	if (!vdpasim->vqs)
 		goto err_iommu;
 
-	vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
+	vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
+				       sizeof(*vdpasim->iommu), GFP_KERNEL);
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
+	for (i = 0; i < vdpasim->dev_attr.nas; i++)
+		vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
+
 	vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
 	if (!vdpasim->buffer)
 		goto err_iommu;
 
 	for (i = 0; i < dev_attr->nvqs; i++)
-		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
+		vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
 				 &vdpasim->iommu_lock);
 
 	ret = iova_cache_get();
@@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
 
 static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
 {
-	return 0;
+	/* RX and TX belongs to group 0, CVQ belongs to group 1 */
+	if (idx == 2)
+		return 1;
+	else
+		return 0;
 }
 
 static u64 vdpasim_get_device_features(struct vdpa_device *vdpa)
@@ -539,20 +554,53 @@ static struct vdpa_iova_range vdpasim_get_iova_range(struct vdpa_device *vdpa)
 	return range;
 }
 
+static int vdpasim_set_group_asid(struct vdpa_device *vdpa, unsigned int group,
+				  unsigned int asid)
+{
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+	struct vhost_iotlb *iommu;
+	int i;
+
+	if (group > vdpasim->dev_attr.ngroups)
+		return -EINVAL;
+
+	if (asid > vdpasim->dev_attr.nas)
+		return -EINVAL;
+
+	iommu = &vdpasim->iommu[asid];
+
+	spin_lock(&vdpasim->lock);
+
+	for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
+		if (vdpasim_get_vq_group(vdpa, i) == group)
+			vringh_set_iotlb(&vdpasim->vqs[i].vring, iommu,
+					 &vdpasim->iommu_lock);
+
+	spin_unlock(&vdpasim->lock);
+
+	return 0;
+}
+
 static int vdpasim_set_map(struct vdpa_device *vdpa, unsigned int asid,
 			   struct vhost_iotlb *iotlb)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 	struct vhost_iotlb_map *map;
+	struct vhost_iotlb *iommu;
 	u64 start = 0ULL, last = 0ULL - 1;
 	int ret;
 
+	if (asid >= vdpasim->dev_attr.nas)
+		return -EINVAL;
+
 	spin_lock(&vdpasim->iommu_lock);
-	vhost_iotlb_reset(vdpasim->iommu);
+
+	iommu = &vdpasim->iommu[asid];
+	vhost_iotlb_reset(iommu);
 
 	for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
 	     map = vhost_iotlb_itree_next(map, start, last)) {
-		ret = vhost_iotlb_add_range(vdpasim->iommu, map->start,
+		ret = vhost_iotlb_add_range(iommu, map->start,
 					    map->last, map->addr, map->perm);
 		if (ret)
 			goto err;
@@ -561,7 +609,7 @@ static int vdpasim_set_map(struct vdpa_device *vdpa, unsigned int asid,
 	return 0;
 
 err:
-	vhost_iotlb_reset(vdpasim->iommu);
+	vhost_iotlb_reset(iommu);
 	spin_unlock(&vdpasim->iommu_lock);
 	return ret;
 }
@@ -573,9 +621,12 @@ static int vdpasim_dma_map(struct vdpa_device *vdpa, unsigned int asid,
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 	int ret;
 
+	if (asid >= vdpasim->dev_attr.nas)
+		return -EINVAL;
+
 	spin_lock(&vdpasim->iommu_lock);
-	ret = vhost_iotlb_add_range_ctx(vdpasim->iommu, iova, iova + size - 1,
-					pa, perm, opaque);
+	ret = vhost_iotlb_add_range_ctx(&vdpasim->iommu[asid], iova,
+					iova + size - 1, pa, perm, opaque);
 	spin_unlock(&vdpasim->iommu_lock);
 
 	return ret;
@@ -586,8 +637,11 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, unsigned int asid,
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
+	if (asid >= vdpasim->dev_attr.nas)
+		return -EINVAL;
+
 	spin_lock(&vdpasim->iommu_lock);
-	vhost_iotlb_del_range(vdpasim->iommu, iova, iova + size - 1);
+	vhost_iotlb_del_range(&vdpasim->iommu[asid], iova, iova + size - 1);
 	spin_unlock(&vdpasim->iommu_lock);
 
 	return 0;
@@ -611,8 +665,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	}
 
 	kvfree(vdpasim->buffer);
-	if (vdpasim->iommu)
-		vhost_iotlb_free(vdpasim->iommu);
+	vhost_iotlb_free(vdpasim->iommu);
 	kfree(vdpasim->vqs);
 	kfree(vdpasim->config);
 }
@@ -643,6 +696,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
 	.set_config             = vdpasim_set_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
+	.set_group_asid         = vdpasim_set_group_asid,
 	.dma_map                = vdpasim_dma_map,
 	.dma_unmap              = vdpasim_dma_unmap,
 	.free                   = vdpasim_free,
@@ -674,6 +728,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.set_config             = vdpasim_set_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
+	.set_group_asid         = vdpasim_set_group_asid,
 	.set_map                = vdpasim_set_map,
 	.free                   = vdpasim_free,
 };
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
index 0be7c1e7ef80..622782e92239 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -41,6 +41,8 @@ struct vdpasim_dev_attr {
 	size_t buffer_size;
 	int nvqs;
 	u32 id;
+	u32 ngroups;
+	u32 nas;
 
 	work_func_t work_fn;
 	void (*get_config)(struct vdpasim *vdpasim, void *config);
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index 5fa59d4fddc8..5125976a4df8 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -27,9 +27,14 @@
 
 #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
 				 (1ULL << VIRTIO_NET_F_MAC) | \
-				 (1ULL << VIRTIO_NET_F_MTU))
+				 (1ULL << VIRTIO_NET_F_MTU) | \
+				 (1ULL << VIRTIO_NET_F_CTRL_VQ) | \
+				 (1ULL << VIRTIO_NET_F_CTRL_MAC_ADDR))
 
-#define VDPASIM_NET_VQ_NUM	2
+/* 3 virtqueues, 2 address spaces, 2 virtqueue groups */
+#define VDPASIM_NET_VQ_NUM	3
+#define VDPASIM_NET_AS_NUM	2
+#define VDPASIM_NET_GROUP_NUM	2
 
 static void vdpasim_net_complete(struct vdpasim_virtqueue *vq, size_t len)
 {
@@ -63,6 +68,81 @@ static bool receive_filter(struct vdpasim *vdpasim, size_t len)
 	return false;
 }
 
+static virtio_net_ctrl_ack vdpasim_handle_ctrl_mac(struct vdpasim *vdpasim,
+						   u8 cmd)
+{
+	struct virtio_net_config *vio_config = vdpasim->config;
+	struct vdpasim_virtqueue *cvq = &vdpasim->vqs[2];
+	virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
+	size_t read;
+
+	switch (cmd) {
+	case VIRTIO_NET_CTRL_MAC_ADDR_SET:
+		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov,
+					     vio_config->mac, ETH_ALEN);
+		if (read == ETH_ALEN)
+			status = VIRTIO_NET_OK;
+		break;
+	default:
+		break;
+	}
+
+	return status;
+}
+
+static void vdpasim_handle_cvq(struct vdpasim *vdpasim)
+{
+	struct vdpasim_virtqueue *cvq = &vdpasim->vqs[2];
+	virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
+	struct virtio_net_ctrl_hdr ctrl;
+	size_t read, write;
+	int err;
+
+	if (!(vdpasim->features & (1ULL << VIRTIO_NET_F_CTRL_VQ)))
+		return;
+
+	if (!cvq->ready)
+		return;
+
+	while (true) {
+		err = vringh_getdesc_iotlb(&cvq->vring, &cvq->in_iov,
+					   &cvq->out_iov,
+					   &cvq->head, GFP_ATOMIC);
+		if (err <= 0)
+			break;
+
+		read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl,
+					     sizeof(ctrl));
+		if (read != sizeof(ctrl))
+			break;
+
+		switch (ctrl.class) {
+		case VIRTIO_NET_CTRL_MAC:
+			status = vdpasim_handle_ctrl_mac(vdpasim, ctrl.cmd);
+			break;
+		default:
+			break;
+		}
+
+		/* Make sure data is wrote before advancing index */
+		smp_wmb();
+
+		write = vringh_iov_push_iotlb(&cvq->vring, &cvq->out_iov,
+					      &status, sizeof(status));
+		vringh_complete_iotlb(&cvq->vring, cvq->head, write);
+		vringh_kiov_cleanup(&cvq->in_iov);
+		vringh_kiov_cleanup(&cvq->out_iov);
+
+		/* Make sure used is visible before rasing the interrupt. */
+		smp_wmb();
+
+		local_bh_disable();
+		if (cvq->cb)
+			cvq->cb(cvq->private);
+		local_bh_enable();
+	}
+}
+
 static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
@@ -77,6 +157,8 @@ static void vdpasim_net_work(struct work_struct *work)
 	if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
 		goto out;
 
+	vdpasim_handle_cvq(vdpasim);
+
 	if (!txq->ready || !rxq->ready)
 		goto out;
 
@@ -162,6 +244,8 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	dev_attr.id = VIRTIO_ID_NET;
 	dev_attr.supported_features = VDPASIM_NET_FEATURES;
 	dev_attr.nvqs = VDPASIM_NET_VQ_NUM;
+	dev_attr.ngroups = VDPASIM_NET_GROUP_NUM;
+	dev_attr.nas = VDPASIM_NET_AS_NUM;
 	dev_attr.config_size = sizeof(struct virtio_net_config);
 	dev_attr.get_config = vdpasim_net_get_config;
 	dev_attr.work_fn = vdpasim_net_work;
-- 
2.30.1

Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Jason Wang]

On Thu, Mar 31, 2022 at 2:17 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> This patch extends the vhost-vdpa to support ASID based IOTLB API. The
> vhost-vdpa device will allocated multiple IOTLBs for vDPA device that
> supports multiple address spaces. The IOTLBs and vDPA device memory
> mappings is determined and maintained through ASID.
>
> Note that we still don't support vDPA device with more than one
> address spaces that depends on platform IOMMU. This work will be done
> by moving the IOMMU logic from vhost-vDPA to vDPA device driver.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>  drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
>  drivers/vhost/vhost.c |   2 +-
>  2 files changed, 91 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 6c7ee0f18892..1f1d1c425573 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -28,7 +28,8 @@
>  enum {
>         VHOST_VDPA_BACKEND_FEATURES =
>         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> -       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
> +       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
>  };
>
>  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
> @@ -57,12 +58,20 @@ struct vhost_vdpa {
>         struct eventfd_ctx *config_ctx;
>         int in_batch;
>         struct vdpa_iova_range range;
> +       u32 batch_asid;
>  };
>
>  static DEFINE_IDA(vhost_vdpa_ida);
>
>  static dev_t vhost_vdpa_major;
>
> +static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb)
> +{
> +       struct vhost_vdpa_as *as = container_of(iotlb, struct
> +                                               vhost_vdpa_as, iotlb);
> +       return as->id;
> +}
> +
>  static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
> @@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
>         return NULL;
>  }
>
> +static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32 asid)
> +{
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (!as)
> +               return NULL;
> +
> +       return &as->iotlb;
> +}
> +
>  static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
> @@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         if (asid_to_as(v, asid))
>                 return NULL;
>
> +       if (asid >= v->vdpa->nas)
> +               return NULL;
> +
>         as = kmalloc(sizeof(*as), GFP_KERNEL);
>         if (!as)
>                 return NULL;
> @@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         return as;
>  }
>
> +static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
> +                                                     u32 asid)
> +{
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (as)
> +               return as;
> +
> +       return vhost_vdpa_alloc_as(v, asid);
> +}
> +
>  static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct vhost_vdpa_as *as = asid_to_as(v, asid);
> @@ -692,6 +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>         struct vhost_dev *dev = &v->vdev;
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>         int r = 0;
>
>         r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1,
> @@ -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>                 return r;
>
>         if (ops->dma_map) {
> -               r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
> +               r = ops->dma_map(vdpa, asid, iova, size, pa, perm, opaque);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       r = ops->set_map(vdpa, 0, iotlb);
> +                       r = ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 r = iommu_map(v->domain, iova, pa, size,
>                               perm_to_iommu_flags(perm));
> @@ -725,17 +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
>  {
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>
>         vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
>
>         if (ops->dma_map) {
> -               ops->dma_unmap(vdpa, 0, iova, size);
> +               ops->dma_unmap(vdpa, asid, iova, size);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 iommu_unmap(v->domain, iova, size);
>         }
> +
> +       /* If we are in the middle of batch processing, delay the free
> +        * of AS until BATCH_END.
> +        */
> +       if (!v->in_batch && !iotlb->nmaps)
> +               vhost_vdpa_remove_as(v, asid);
>  }
>
>  static int vhost_vdpa_va_map(struct vhost_vdpa *v,
> @@ -943,19 +984,38 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>         struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> -       struct vhost_vdpa_as *as = asid_to_as(v, 0);
> -       struct vhost_iotlb *iotlb = &as->iotlb;
> +       struct vhost_iotlb *iotlb = NULL;
> +       struct vhost_vdpa_as *as = NULL;
>         int r = 0;
>
> -       if (asid != 0)
> -               return -EINVAL;
> -
>         mutex_lock(&dev->mutex);
>
>         r = vhost_dev_check_owner(dev);
>         if (r)
>                 goto unlock;
>
> +       if (msg->type == VHOST_IOTLB_UPDATE ||
> +           msg->type == VHOST_IOTLB_BATCH_BEGIN) {
> +               as = vhost_vdpa_find_alloc_as(v, asid);

I wonder if it's better to mandate the ASID to [0, dev->nas),
otherwise user space is free to use arbitrary IDs which may exceeds
the #address spaces that is supported by the device.

Thanks

> +               if (!as) {
> +                       dev_err(&v->dev, "can't find and alloc asid %d\n",
> +                               asid);
> +                       return -EINVAL;
> +               }
> +               iotlb = &as->iotlb;
> +       } else
> +               iotlb = asid_to_iotlb(v, asid);
> +
> +       if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
> +               if (v->in_batch && v->batch_asid != asid) {
> +                       dev_info(&v->dev, "batch id %d asid %d\n",
> +                                v->batch_asid, asid);
> +               }
> +               if (!iotlb)
> +                       dev_err(&v->dev, "no iotlb for asid %d\n", asid);
> +               return -EINVAL;
> +       }
> +
>         switch (msg->type) {
>         case VHOST_IOTLB_UPDATE:
>                 r = vhost_vdpa_process_iotlb_update(v, iotlb, msg);
> @@ -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>                 vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
>                 break;
>         case VHOST_IOTLB_BATCH_BEGIN:
> +               v->batch_asid = asid;
>                 v->in_batch = true;
>                 break;
>         case VHOST_IOTLB_BATCH_END:
>                 if (v->in_batch && ops->set_map)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>                 v->in_batch = false;
> +               if (!iotlb->nmaps)
> +                       vhost_vdpa_remove_as(v, asid);
>                 break;
>         default:
>                 r = -EINVAL;
> @@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v)
>
>  static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
>  {
> +       struct vhost_vdpa_as *as;
> +       u32 asid;
> +
>         vhost_dev_cleanup(&v->vdev);
>         kfree(v->vdev.vqs);
> -       vhost_vdpa_remove_as(v, 0);
> +
> +       for (asid = 0; asid < v->vdpa->nas; asid++) {
> +               as = asid_to_as(v, asid);
> +               if (as)
> +                       vhost_vdpa_remove_as(v, asid);
> +       }
>  }
>
>  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> @@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>         vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
>                        vhost_vdpa_process_iotlb_msg);
>
> -       if (!vhost_vdpa_alloc_as(v, 0))
> -               goto err_alloc_as;
> -
>         r = vhost_vdpa_alloc_domain(v);
>         if (r)
> -               goto err_alloc_as;
> +               goto err_alloc_domain;
>
>         vhost_vdpa_set_iova_range(v);
>
> @@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>
>         return 0;
>
> -err_alloc_as:
> +err_alloc_domain:
>         vhost_vdpa_cleanup(v);
>  err:
>         atomic_dec(&v->opened);
> @@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
>         int minor;
>         int i, r;
>
> -       /* Only support 1 address space and 1 groups */
> -       if (vdpa->ngroups != 1 || vdpa->nas != 1)
> +       /* We can't support platform IOMMU device with more than 1
> +        * group or as
> +        */
> +       if (!ops->set_map && !ops->dma_map &&
> +           (vdpa->ngroups > 1 || vdpa->nas > 1))
>                 return -EOPNOTSUPP;
>
>         v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d1e58f976f6e..5022c648d9c0 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>                                 ret = -EINVAL;
>                                 goto done;
>                         }
> -                       offset = sizeof(__u16);
> +                       offset = 0;
>                 } else
>                         offset = sizeof(__u32);
>                 break;
> --
> 2.30.1
>

Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Jason Wang]

On Thu, Mar 31, 2022 at 2:17 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> This patch extends the vhost-vdpa to support ASID based IOTLB API. The
> vhost-vdpa device will allocated multiple IOTLBs for vDPA device that
> supports multiple address spaces. The IOTLBs and vDPA device memory
> mappings is determined and maintained through ASID.
>
> Note that we still don't support vDPA device with more than one
> address spaces that depends on platform IOMMU. This work will be done
> by moving the IOMMU logic from vhost-vDPA to vDPA device driver.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>  drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
>  drivers/vhost/vhost.c |   2 +-
>  2 files changed, 91 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 6c7ee0f18892..1f1d1c425573 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -28,7 +28,8 @@
>  enum {
>         VHOST_VDPA_BACKEND_FEATURES =
>         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> -       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
> +       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
>  };
>
>  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS)
> @@ -57,12 +58,20 @@ struct vhost_vdpa {
>         struct eventfd_ctx *config_ctx;
>         int in_batch;
>         struct vdpa_iova_range range;
> +       u32 batch_asid;
>  };
>
>  static DEFINE_IDA(vhost_vdpa_ida);
>
>  static dev_t vhost_vdpa_major;
>
> +static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb)
> +{
> +       struct vhost_vdpa_as *as = container_of(iotlb, struct
> +                                               vhost_vdpa_as, iotlb);
> +       return as->id;
> +}
> +
>  static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
> @@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
>         return NULL;
>  }
>
> +static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32 asid)
> +{
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (!as)
> +               return NULL;
> +
> +       return &as->iotlb;
> +}
> +
>  static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct hlist_head *head = &v->as[asid % VHOST_VDPA_IOTLB_BUCKETS];
> @@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         if (asid_to_as(v, asid))
>                 return NULL;
>
> +       if (asid >= v->vdpa->nas)
> +               return NULL;
> +
>         as = kmalloc(sizeof(*as), GFP_KERNEL);
>         if (!as)
>                 return NULL;
> @@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         return as;
>  }
>
> +static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
> +                                                     u32 asid)
> +{
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (as)
> +               return as;
> +
> +       return vhost_vdpa_alloc_as(v, asid);
> +}
> +
>  static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)
>  {
>         struct vhost_vdpa_as *as = asid_to_as(v, asid);
> @@ -692,6 +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>         struct vhost_dev *dev = &v->vdev;
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>         int r = 0;
>
>         r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1,
> @@ -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>                 return r;
>
>         if (ops->dma_map) {
> -               r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
> +               r = ops->dma_map(vdpa, asid, iova, size, pa, perm, opaque);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       r = ops->set_map(vdpa, 0, iotlb);
> +                       r = ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 r = iommu_map(v->domain, iova, pa, size,
>                               perm_to_iommu_flags(perm));
> @@ -725,17 +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
>  {
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>
>         vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
>
>         if (ops->dma_map) {
> -               ops->dma_unmap(vdpa, 0, iova, size);
> +               ops->dma_unmap(vdpa, asid, iova, size);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 iommu_unmap(v->domain, iova, size);
>         }
> +
> +       /* If we are in the middle of batch processing, delay the free
> +        * of AS until BATCH_END.
> +        */
> +       if (!v->in_batch && !iotlb->nmaps)
> +               vhost_vdpa_remove_as(v, asid);
>  }
>
>  static int vhost_vdpa_va_map(struct vhost_vdpa *v,
> @@ -943,19 +984,38 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>         struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> -       struct vhost_vdpa_as *as = asid_to_as(v, 0);
> -       struct vhost_iotlb *iotlb = &as->iotlb;
> +       struct vhost_iotlb *iotlb = NULL;
> +       struct vhost_vdpa_as *as = NULL;
>         int r = 0;
>
> -       if (asid != 0)
> -               return -EINVAL;
> -
>         mutex_lock(&dev->mutex);
>
>         r = vhost_dev_check_owner(dev);
>         if (r)
>                 goto unlock;
>
> +       if (msg->type == VHOST_IOTLB_UPDATE ||
> +           msg->type == VHOST_IOTLB_BATCH_BEGIN) {
> +               as = vhost_vdpa_find_alloc_as(v, asid);

I wonder if it's better to mandate the ASID to [0, dev->nas),
otherwise user space is free to use arbitrary IDs which may exceeds
the #address spaces that is supported by the device.

Thanks

> +               if (!as) {
> +                       dev_err(&v->dev, "can't find and alloc asid %d\n",
> +                               asid);
> +                       return -EINVAL;
> +               }
> +               iotlb = &as->iotlb;
> +       } else
> +               iotlb = asid_to_iotlb(v, asid);
> +
> +       if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
> +               if (v->in_batch && v->batch_asid != asid) {
> +                       dev_info(&v->dev, "batch id %d asid %d\n",
> +                                v->batch_asid, asid);
> +               }
> +               if (!iotlb)
> +                       dev_err(&v->dev, "no iotlb for asid %d\n", asid);
> +               return -EINVAL;
> +       }
> +
>         switch (msg->type) {
>         case VHOST_IOTLB_UPDATE:
>                 r = vhost_vdpa_process_iotlb_update(v, iotlb, msg);
> @@ -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>                 vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
>                 break;
>         case VHOST_IOTLB_BATCH_BEGIN:
> +               v->batch_asid = asid;
>                 v->in_batch = true;
>                 break;
>         case VHOST_IOTLB_BATCH_END:
>                 if (v->in_batch && ops->set_map)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>                 v->in_batch = false;
> +               if (!iotlb->nmaps)
> +                       vhost_vdpa_remove_as(v, asid);
>                 break;
>         default:
>                 r = -EINVAL;
> @@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v)
>
>  static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
>  {
> +       struct vhost_vdpa_as *as;
> +       u32 asid;
> +
>         vhost_dev_cleanup(&v->vdev);
>         kfree(v->vdev.vqs);
> -       vhost_vdpa_remove_as(v, 0);
> +
> +       for (asid = 0; asid < v->vdpa->nas; asid++) {
> +               as = asid_to_as(v, asid);
> +               if (as)
> +                       vhost_vdpa_remove_as(v, asid);
> +       }
>  }
>
>  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> @@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>         vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
>                        vhost_vdpa_process_iotlb_msg);
>
> -       if (!vhost_vdpa_alloc_as(v, 0))
> -               goto err_alloc_as;
> -
>         r = vhost_vdpa_alloc_domain(v);
>         if (r)
> -               goto err_alloc_as;
> +               goto err_alloc_domain;
>
>         vhost_vdpa_set_iova_range(v);
>
> @@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>
>         return 0;
>
> -err_alloc_as:
> +err_alloc_domain:
>         vhost_vdpa_cleanup(v);
>  err:
>         atomic_dec(&v->opened);
> @@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
>         int minor;
>         int i, r;
>
> -       /* Only support 1 address space and 1 groups */
> -       if (vdpa->ngroups != 1 || vdpa->nas != 1)
> +       /* We can't support platform IOMMU device with more than 1
> +        * group or as
> +        */
> +       if (!ops->set_map && !ops->dma_map &&
> +           (vdpa->ngroups > 1 || vdpa->nas > 1))
>                 return -EOPNOTSUPP;
>
>         v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d1e58f976f6e..5022c648d9c0 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>                                 ret = -EINVAL;
>                                 goto done;
>                         }
> -                       offset = sizeof(__u16);
> +                       offset = 0;
>                 } else
>                         offset = sizeof(__u32);
>                 break;
> --
> 2.30.1
>

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

RE: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Gautam Dawar]

-----Original Message-----
From: Jason Wang <jasowang@redhat.com> 
Sent: Friday, April 1, 2022 9:55 AM
To: Gautam Dawar <gdawar@xilinx.com>
Cc: Michael S. Tsirkin <mst@redhat.com>; kvm <kvm@vger.kernel.org>; virtualization <virtualization@lists.linux-foundation.org>; netdev <netdev@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>; Martin Petrus Hubertus Habets <martinh@xilinx.com>; Harpreet Singh Anand <hanand@xilinx.com>; Martin Porter <martinpo@xilinx.com>; Pablo Cascon <pabloc@xilinx.com>; Dinan Gunawardena <dinang@xilinx.com>; tanuj.kamde@amd.com; habetsm.xilinx@gmail.com; ecree.xilinx@gmail.com; eperezma <eperezma@redhat.com>; Gautam Dawar <gdawar@xilinx.com>; Wu Zongyong <wuzongyong@linux.alibaba.com>; Christophe JAILLET <christophe.jaillet@wanadoo.fr>; Eli Cohen <elic@nvidia.com>; Zhu Lingshan <lingshan.zhu@intel.com>; Stefano Garzarella <sgarzare@redhat.com>; Xie Yongji <xieyongji@bytedance.com>; Si-Wei Liu <si-wei.liu@oracle.com>; Parav Pandit <parav@nvidia.com>; Longpeng <longpeng2@huawei.com>; Dan Carpenter <dan.carpenter@oracle.com>; Zhang Min <zhang.min9@zte.com.cn>
Subject: Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

On Thu, Mar 31, 2022 at 2:17 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> This patch extends the vhost-vdpa to support ASID based IOTLB API. The 
> vhost-vdpa device will allocated multiple IOTLBs for vDPA device that 
> supports multiple address spaces. The IOTLBs and vDPA device memory 
> mappings is determined and maintained through ASID.
>
> Note that we still don't support vDPA device with more than one 
> address spaces that depends on platform IOMMU. This work will be done 
> by moving the IOMMU logic from vhost-vDPA to vDPA device driver.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>  drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
>  drivers/vhost/vhost.c |   2 +-
>  2 files changed, 91 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 
> 6c7ee0f18892..1f1d1c425573 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -28,7 +28,8 @@
>  enum {
>         VHOST_VDPA_BACKEND_FEATURES =
>         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> -       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
> +       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
>  };
>
>  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS) @@ -57,12 +58,20 @@ 
> struct vhost_vdpa {
>         struct eventfd_ctx *config_ctx;
>         int in_batch;
>         struct vdpa_iova_range range;
> +       u32 batch_asid;
>  };
>
>  static DEFINE_IDA(vhost_vdpa_ida);
>
>  static dev_t vhost_vdpa_major;
>
> +static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb) {
> +       struct vhost_vdpa_as *as = container_of(iotlb, struct
> +                                               vhost_vdpa_as, iotlb);
> +       return as->id;
> +}
> +
>  static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 
> asid)  {
>         struct hlist_head *head = &v->as[asid % 
> VHOST_VDPA_IOTLB_BUCKETS]; @@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
>         return NULL;
>  }
>
> +static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32 
> +asid) {
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (!as)
> +               return NULL;
> +
> +       return &as->iotlb;
> +}
> +
>  static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa 
> *v, u32 asid)  {
>         struct hlist_head *head = &v->as[asid % 
> VHOST_VDPA_IOTLB_BUCKETS]; @@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         if (asid_to_as(v, asid))
>                 return NULL;
>
> +       if (asid >= v->vdpa->nas)
> +               return NULL;
> +
>         as = kmalloc(sizeof(*as), GFP_KERNEL);
>         if (!as)
>                 return NULL;
> @@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
>         return as;
>  }
>
> +static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
> +                                                     u32 asid) {
> +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> +
> +       if (as)
> +               return as;
> +
> +       return vhost_vdpa_alloc_as(v, asid); }
> +
>  static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)  {
>         struct vhost_vdpa_as *as = asid_to_as(v, asid); @@ -692,6 
> +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>         struct vhost_dev *dev = &v->vdev;
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>         int r = 0;
>
>         r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1, @@ 
> -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
>                 return r;
>
>         if (ops->dma_map) {
> -               r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
> +               r = ops->dma_map(vdpa, asid, iova, size, pa, perm, 
> + opaque);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       r = ops->set_map(vdpa, 0, iotlb);
> +                       r = ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 r = iommu_map(v->domain, iova, pa, size,
>                               perm_to_iommu_flags(perm)); @@ -725,17 
> +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,  {
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       u32 asid = iotlb_to_asid(iotlb);
>
>         vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
>
>         if (ops->dma_map) {
> -               ops->dma_unmap(vdpa, 0, iova, size);
> +               ops->dma_unmap(vdpa, asid, iova, size);
>         } else if (ops->set_map) {
>                 if (!v->in_batch)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>         } else {
>                 iommu_unmap(v->domain, iova, size);
>         }
> +
> +       /* If we are in the middle of batch processing, delay the free
> +        * of AS until BATCH_END.
> +        */
> +       if (!v->in_batch && !iotlb->nmaps)
> +               vhost_vdpa_remove_as(v, asid);
>  }
>
>  static int vhost_vdpa_va_map(struct vhost_vdpa *v, @@ -943,19 +984,38 
> @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>         struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
>         struct vdpa_device *vdpa = v->vdpa;
>         const struct vdpa_config_ops *ops = vdpa->config;
> -       struct vhost_vdpa_as *as = asid_to_as(v, 0);
> -       struct vhost_iotlb *iotlb = &as->iotlb;
> +       struct vhost_iotlb *iotlb = NULL;
> +       struct vhost_vdpa_as *as = NULL;
>         int r = 0;
>
> -       if (asid != 0)
> -               return -EINVAL;
> -
>         mutex_lock(&dev->mutex);
>
>         r = vhost_dev_check_owner(dev);
>         if (r)
>                 goto unlock;
>
> +       if (msg->type == VHOST_IOTLB_UPDATE ||
> +           msg->type == VHOST_IOTLB_BATCH_BEGIN) {
> +               as = vhost_vdpa_find_alloc_as(v, asid);

I wonder if it's better to mandate the ASID to [0, dev->nas), otherwise user space is free to use arbitrary IDs which may exceeds the #address spaces that is supported by the device.
[GD>>] Isn’t the following check in vhost_vdpa_alloc_as () sufficient to ensure ASID's value in the range [0, dev->nas):
        if (asid >= v->vdpa->nas)
                return NULL;

Thanks

> +               if (!as) {
> +                       dev_err(&v->dev, "can't find and alloc asid %d\n",
> +                               asid);
> +                       return -EINVAL;
> +               }
> +               iotlb = &as->iotlb;
> +       } else
> +               iotlb = asid_to_iotlb(v, asid);
> +
> +       if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
> +               if (v->in_batch && v->batch_asid != asid) {
> +                       dev_info(&v->dev, "batch id %d asid %d\n",
> +                                v->batch_asid, asid);
> +               }
> +               if (!iotlb)
> +                       dev_err(&v->dev, "no iotlb for asid %d\n", asid);
> +               return -EINVAL;
> +       }
> +
>         switch (msg->type) {
>         case VHOST_IOTLB_UPDATE:
>                 r = vhost_vdpa_process_iotlb_update(v, iotlb, msg); @@ 
> -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
>                 vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
>                 break;
>         case VHOST_IOTLB_BATCH_BEGIN:
> +               v->batch_asid = asid;
>                 v->in_batch = true;
>                 break;
>         case VHOST_IOTLB_BATCH_END:
>                 if (v->in_batch && ops->set_map)
> -                       ops->set_map(vdpa, 0, iotlb);
> +                       ops->set_map(vdpa, asid, iotlb);
>                 v->in_batch = false;
> +               if (!iotlb->nmaps)
> +                       vhost_vdpa_remove_as(v, asid);
>                 break;
>         default:
>                 r = -EINVAL;
> @@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct 
> vhost_vdpa *v)
>
>  static void vhost_vdpa_cleanup(struct vhost_vdpa *v)  {
> +       struct vhost_vdpa_as *as;
> +       u32 asid;
> +
>         vhost_dev_cleanup(&v->vdev);
>         kfree(v->vdev.vqs);
> -       vhost_vdpa_remove_as(v, 0);
> +
> +       for (asid = 0; asid < v->vdpa->nas; asid++) {
> +               as = asid_to_as(v, asid);
> +               if (as)
> +                       vhost_vdpa_remove_as(v, asid);
> +       }
>  }
>
>  static int vhost_vdpa_open(struct inode *inode, struct file *filep) 
> @@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>         vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
>                        vhost_vdpa_process_iotlb_msg);
>
> -       if (!vhost_vdpa_alloc_as(v, 0))
> -               goto err_alloc_as;
> -
>         r = vhost_vdpa_alloc_domain(v);
>         if (r)
> -               goto err_alloc_as;
> +               goto err_alloc_domain;
>
>         vhost_vdpa_set_iova_range(v);
>
> @@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode, 
> struct file *filep)
>
>         return 0;
>
> -err_alloc_as:
> +err_alloc_domain:
>         vhost_vdpa_cleanup(v);
>  err:
>         atomic_dec(&v->opened);
> @@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
>         int minor;
>         int i, r;
>
> -       /* Only support 1 address space and 1 groups */
> -       if (vdpa->ngroups != 1 || vdpa->nas != 1)
> +       /* We can't support platform IOMMU device with more than 1
> +        * group or as
> +        */
> +       if (!ops->set_map && !ops->dma_map &&
> +           (vdpa->ngroups > 1 || vdpa->nas > 1))
>                 return -EOPNOTSUPP;
>
>         v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL); 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 
> d1e58f976f6e..5022c648d9c0 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>                                 ret = -EINVAL;
>                                 goto done;
>                         }
> -                       offset = sizeof(__u16);
> +                       offset = 0;
>                 } else
>                         offset = sizeof(__u32);
>                 break;
> --
> 2.30.1
>

Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Jason Wang]

On Thu, Apr 28, 2022 at 2:28 PM Gautam Dawar <gdawar@xilinx.com> wrote:
>
> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, April 1, 2022 9:55 AM
> To: Gautam Dawar <gdawar@xilinx.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>; kvm <kvm@vger.kernel.org>; virtualization <virtualization@lists.linux-foundation.org>; netdev <netdev@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>; Martin Petrus Hubertus Habets <martinh@xilinx.com>; Harpreet Singh Anand <hanand@xilinx.com>; Martin Porter <martinpo@xilinx.com>; Pablo Cascon <pabloc@xilinx.com>; Dinan Gunawardena <dinang@xilinx.com>; tanuj.kamde@amd.com; habetsm.xilinx@gmail.com; ecree.xilinx@gmail.com; eperezma <eperezma@redhat.com>; Gautam Dawar <gdawar@xilinx.com>; Wu Zongyong <wuzongyong@linux.alibaba.com>; Christophe JAILLET <christophe.jaillet@wanadoo.fr>; Eli Cohen <elic@nvidia.com>; Zhu Lingshan <lingshan.zhu@intel.com>; Stefano Garzarella <sgarzare@redhat.com>; Xie Yongji <xieyongji@bytedance.com>; Si-Wei Liu <si-wei.liu@oracle.com>; Parav Pandit <parav@nvidia.com>; Longpeng <longpeng2@huawei.com>; Dan Carpenter <dan.carpenter@oracle.com>; Zhang Min <zhang.min9@zte.com.cn>
> Subject: Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API
>
> On Thu, Mar 31, 2022 at 2:17 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> >
> > This patch extends the vhost-vdpa to support ASID based IOTLB API. The
> > vhost-vdpa device will allocated multiple IOTLBs for vDPA device that
> > supports multiple address spaces. The IOTLBs and vDPA device memory
> > mappings is determined and maintained through ASID.
> >
> > Note that we still don't support vDPA device with more than one
> > address spaces that depends on platform IOMMU. This work will be done
> > by moving the IOMMU logic from vhost-vDPA to vDPA device driver.
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> > ---
> >  drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
> >  drivers/vhost/vhost.c |   2 +-
> >  2 files changed, 91 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index
> > 6c7ee0f18892..1f1d1c425573 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -28,7 +28,8 @@
> >  enum {
> >         VHOST_VDPA_BACKEND_FEATURES =
> >         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> > -       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
> > +       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> > +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
> >  };
> >
> >  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS) @@ -57,12 +58,20 @@
> > struct vhost_vdpa {
> >         struct eventfd_ctx *config_ctx;
> >         int in_batch;
> >         struct vdpa_iova_range range;
> > +       u32 batch_asid;
> >  };
> >
> >  static DEFINE_IDA(vhost_vdpa_ida);
> >
> >  static dev_t vhost_vdpa_major;
> >
> > +static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb) {
> > +       struct vhost_vdpa_as *as = container_of(iotlb, struct
> > +                                               vhost_vdpa_as, iotlb);
> > +       return as->id;
> > +}
> > +
> >  static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32
> > asid)  {
> >         struct hlist_head *head = &v->as[asid %
> > VHOST_VDPA_IOTLB_BUCKETS]; @@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
> >         return NULL;
> >  }
> >
> > +static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32
> > +asid) {
> > +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> > +
> > +       if (!as)
> > +               return NULL;
> > +
> > +       return &as->iotlb;
> > +}
> > +
> >  static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa
> > *v, u32 asid)  {
> >         struct hlist_head *head = &v->as[asid %
> > VHOST_VDPA_IOTLB_BUCKETS]; @@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
> >         if (asid_to_as(v, asid))
> >                 return NULL;
> >
> > +       if (asid >= v->vdpa->nas)
> > +               return NULL;
> > +
> >         as = kmalloc(sizeof(*as), GFP_KERNEL);
> >         if (!as)
> >                 return NULL;
> > @@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
> >         return as;
> >  }
> >
> > +static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
> > +                                                     u32 asid) {
> > +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> > +
> > +       if (as)
> > +               return as;
> > +
> > +       return vhost_vdpa_alloc_as(v, asid); }
> > +
> >  static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)  {
> >         struct vhost_vdpa_as *as = asid_to_as(v, asid); @@ -692,6
> > +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
> >         struct vhost_dev *dev = &v->vdev;
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > +       u32 asid = iotlb_to_asid(iotlb);
> >         int r = 0;
> >
> >         r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1, @@
> > -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
> >                 return r;
> >
> >         if (ops->dma_map) {
> > -               r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
> > +               r = ops->dma_map(vdpa, asid, iova, size, pa, perm,
> > + opaque);
> >         } else if (ops->set_map) {
> >                 if (!v->in_batch)
> > -                       r = ops->set_map(vdpa, 0, iotlb);
> > +                       r = ops->set_map(vdpa, asid, iotlb);
> >         } else {
> >                 r = iommu_map(v->domain, iova, pa, size,
> >                               perm_to_iommu_flags(perm)); @@ -725,17
> > +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,  {
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > +       u32 asid = iotlb_to_asid(iotlb);
> >
> >         vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
> >
> >         if (ops->dma_map) {
> > -               ops->dma_unmap(vdpa, 0, iova, size);
> > +               ops->dma_unmap(vdpa, asid, iova, size);
> >         } else if (ops->set_map) {
> >                 if (!v->in_batch)
> > -                       ops->set_map(vdpa, 0, iotlb);
> > +                       ops->set_map(vdpa, asid, iotlb);
> >         } else {
> >                 iommu_unmap(v->domain, iova, size);
> >         }
> > +
> > +       /* If we are in the middle of batch processing, delay the free
> > +        * of AS until BATCH_END.
> > +        */
> > +       if (!v->in_batch && !iotlb->nmaps)
> > +               vhost_vdpa_remove_as(v, asid);
> >  }
> >
> >  static int vhost_vdpa_va_map(struct vhost_vdpa *v, @@ -943,19 +984,38
> > @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> >         struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > -       struct vhost_vdpa_as *as = asid_to_as(v, 0);
> > -       struct vhost_iotlb *iotlb = &as->iotlb;
> > +       struct vhost_iotlb *iotlb = NULL;
> > +       struct vhost_vdpa_as *as = NULL;
> >         int r = 0;
> >
> > -       if (asid != 0)
> > -               return -EINVAL;
> > -
> >         mutex_lock(&dev->mutex);
> >
> >         r = vhost_dev_check_owner(dev);
> >         if (r)
> >                 goto unlock;
> >
> > +       if (msg->type == VHOST_IOTLB_UPDATE ||
> > +           msg->type == VHOST_IOTLB_BATCH_BEGIN) {
> > +               as = vhost_vdpa_find_alloc_as(v, asid);
>
> I wonder if it's better to mandate the ASID to [0, dev->nas), otherwise user space is free to use arbitrary IDs which may exceeds the #address spaces that is supported by the device.
> [GD>>] Isn’t the following check in vhost_vdpa_alloc_as () sufficient to ensure ASID's value in the range [0, dev->nas):
>         if (asid >= v->vdpa->nas)
>                 return NULL;

I think you're right.

So we are fine.

Thanks

>
> Thanks
>
> > +               if (!as) {
> > +                       dev_err(&v->dev, "can't find and alloc asid %d\n",
> > +                               asid);
> > +                       return -EINVAL;
> > +               }
> > +               iotlb = &as->iotlb;
> > +       } else
> > +               iotlb = asid_to_iotlb(v, asid);
> > +
> > +       if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
> > +               if (v->in_batch && v->batch_asid != asid) {
> > +                       dev_info(&v->dev, "batch id %d asid %d\n",
> > +                                v->batch_asid, asid);
> > +               }
> > +               if (!iotlb)
> > +                       dev_err(&v->dev, "no iotlb for asid %d\n", asid);
> > +               return -EINVAL;
> > +       }
> > +
> >         switch (msg->type) {
> >         case VHOST_IOTLB_UPDATE:
> >                 r = vhost_vdpa_process_iotlb_update(v, iotlb, msg); @@
> > -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> >                 vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
> >                 break;
> >         case VHOST_IOTLB_BATCH_BEGIN:
> > +               v->batch_asid = asid;
> >                 v->in_batch = true;
> >                 break;
> >         case VHOST_IOTLB_BATCH_END:
> >                 if (v->in_batch && ops->set_map)
> > -                       ops->set_map(vdpa, 0, iotlb);
> > +                       ops->set_map(vdpa, asid, iotlb);
> >                 v->in_batch = false;
> > +               if (!iotlb->nmaps)
> > +                       vhost_vdpa_remove_as(v, asid);
> >                 break;
> >         default:
> >                 r = -EINVAL;
> > @@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct
> > vhost_vdpa *v)
> >
> >  static void vhost_vdpa_cleanup(struct vhost_vdpa *v)  {
> > +       struct vhost_vdpa_as *as;
> > +       u32 asid;
> > +
> >         vhost_dev_cleanup(&v->vdev);
> >         kfree(v->vdev.vqs);
> > -       vhost_vdpa_remove_as(v, 0);
> > +
> > +       for (asid = 0; asid < v->vdpa->nas; asid++) {
> > +               as = asid_to_as(v, asid);
> > +               if (as)
> > +                       vhost_vdpa_remove_as(v, asid);
> > +       }
> >  }
> >
> >  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> > @@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> >         vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
> >                        vhost_vdpa_process_iotlb_msg);
> >
> > -       if (!vhost_vdpa_alloc_as(v, 0))
> > -               goto err_alloc_as;
> > -
> >         r = vhost_vdpa_alloc_domain(v);
> >         if (r)
> > -               goto err_alloc_as;
> > +               goto err_alloc_domain;
> >
> >         vhost_vdpa_set_iova_range(v);
> >
> > @@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode,
> > struct file *filep)
> >
> >         return 0;
> >
> > -err_alloc_as:
> > +err_alloc_domain:
> >         vhost_vdpa_cleanup(v);
> >  err:
> >         atomic_dec(&v->opened);
> > @@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
> >         int minor;
> >         int i, r;
> >
> > -       /* Only support 1 address space and 1 groups */
> > -       if (vdpa->ngroups != 1 || vdpa->nas != 1)
> > +       /* We can't support platform IOMMU device with more than 1
> > +        * group or as
> > +        */
> > +       if (!ops->set_map && !ops->dma_map &&
> > +           (vdpa->ngroups > 1 || vdpa->nas > 1))
> >                 return -EOPNOTSUPP;
> >
> >         v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index
> > d1e58f976f6e..5022c648d9c0 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
> >                                 ret = -EINVAL;
> >                                 goto done;
> >                         }
> > -                       offset = sizeof(__u16);
> > +                       offset = 0;
> >                 } else
> >                         offset = sizeof(__u32);
> >                 break;
> > --
> > 2.30.1
> >
>

Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API

[Jason Wang]

On Thu, Apr 28, 2022 at 2:28 PM Gautam Dawar <gdawar@xilinx.com> wrote:
>
> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, April 1, 2022 9:55 AM
> To: Gautam Dawar <gdawar@xilinx.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>; kvm <kvm@vger.kernel.org>; virtualization <virtualization@lists.linux-foundation.org>; netdev <netdev@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>; Martin Petrus Hubertus Habets <martinh@xilinx.com>; Harpreet Singh Anand <hanand@xilinx.com>; Martin Porter <martinpo@xilinx.com>; Pablo Cascon <pabloc@xilinx.com>; Dinan Gunawardena <dinang@xilinx.com>; tanuj.kamde@amd.com; habetsm.xilinx@gmail.com; ecree.xilinx@gmail.com; eperezma <eperezma@redhat.com>; Gautam Dawar <gdawar@xilinx.com>; Wu Zongyong <wuzongyong@linux.alibaba.com>; Christophe JAILLET <christophe.jaillet@wanadoo.fr>; Eli Cohen <elic@nvidia.com>; Zhu Lingshan <lingshan.zhu@intel.com>; Stefano Garzarella <sgarzare@redhat.com>; Xie Yongji <xieyongji@bytedance.com>; Si-Wei Liu <si-wei.liu@oracle.com>; Parav Pandit <parav@nvidia.com>; Longpeng <longpeng2@huawei.com>; Dan Carpenter <dan.carpenter@oracle.com>; Zhang Min <zhang.min9@zte.com.cn>
> Subject: Re: [PATCH v2 15/19] vhost-vdpa: support ASID based IOTLB API
>
> On Thu, Mar 31, 2022 at 2:17 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> >
> > This patch extends the vhost-vdpa to support ASID based IOTLB API. The
> > vhost-vdpa device will allocated multiple IOTLBs for vDPA device that
> > supports multiple address spaces. The IOTLBs and vDPA device memory
> > mappings is determined and maintained through ASID.
> >
> > Note that we still don't support vDPA device with more than one
> > address spaces that depends on platform IOMMU. This work will be done
> > by moving the IOMMU logic from vhost-vDPA to vDPA device driver.
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> > ---
> >  drivers/vhost/vdpa.c  | 109 ++++++++++++++++++++++++++++++++++--------
> >  drivers/vhost/vhost.c |   2 +-
> >  2 files changed, 91 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index
> > 6c7ee0f18892..1f1d1c425573 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -28,7 +28,8 @@
> >  enum {
> >         VHOST_VDPA_BACKEND_FEATURES =
> >         (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> > -       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
> > +       (1ULL << VHOST_BACKEND_F_IOTLB_BATCH) |
> > +       (1ULL << VHOST_BACKEND_F_IOTLB_ASID),
> >  };
> >
> >  #define VHOST_VDPA_DEV_MAX (1U << MINORBITS) @@ -57,12 +58,20 @@
> > struct vhost_vdpa {
> >         struct eventfd_ctx *config_ctx;
> >         int in_batch;
> >         struct vdpa_iova_range range;
> > +       u32 batch_asid;
> >  };
> >
> >  static DEFINE_IDA(vhost_vdpa_ida);
> >
> >  static dev_t vhost_vdpa_major;
> >
> > +static inline u32 iotlb_to_asid(struct vhost_iotlb *iotlb) {
> > +       struct vhost_vdpa_as *as = container_of(iotlb, struct
> > +                                               vhost_vdpa_as, iotlb);
> > +       return as->id;
> > +}
> > +
> >  static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32
> > asid)  {
> >         struct hlist_head *head = &v->as[asid %
> > VHOST_VDPA_IOTLB_BUCKETS]; @@ -75,6 +84,16 @@ static struct vhost_vdpa_as *asid_to_as(struct vhost_vdpa *v, u32 asid)
> >         return NULL;
> >  }
> >
> > +static struct vhost_iotlb *asid_to_iotlb(struct vhost_vdpa *v, u32
> > +asid) {
> > +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> > +
> > +       if (!as)
> > +               return NULL;
> > +
> > +       return &as->iotlb;
> > +}
> > +
> >  static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa
> > *v, u32 asid)  {
> >         struct hlist_head *head = &v->as[asid %
> > VHOST_VDPA_IOTLB_BUCKETS]; @@ -83,6 +102,9 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
> >         if (asid_to_as(v, asid))
> >                 return NULL;
> >
> > +       if (asid >= v->vdpa->nas)
> > +               return NULL;
> > +
> >         as = kmalloc(sizeof(*as), GFP_KERNEL);
> >         if (!as)
> >                 return NULL;
> > @@ -94,6 +116,17 @@ static struct vhost_vdpa_as *vhost_vdpa_alloc_as(struct vhost_vdpa *v, u32 asid)
> >         return as;
> >  }
> >
> > +static struct vhost_vdpa_as *vhost_vdpa_find_alloc_as(struct vhost_vdpa *v,
> > +                                                     u32 asid) {
> > +       struct vhost_vdpa_as *as = asid_to_as(v, asid);
> > +
> > +       if (as)
> > +               return as;
> > +
> > +       return vhost_vdpa_alloc_as(v, asid); }
> > +
> >  static int vhost_vdpa_remove_as(struct vhost_vdpa *v, u32 asid)  {
> >         struct vhost_vdpa_as *as = asid_to_as(v, asid); @@ -692,6
> > +725,7 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
> >         struct vhost_dev *dev = &v->vdev;
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > +       u32 asid = iotlb_to_asid(iotlb);
> >         int r = 0;
> >
> >         r = vhost_iotlb_add_range_ctx(iotlb, iova, iova + size - 1, @@
> > -700,10 +734,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v, struct vhost_iotlb *iotlb,
> >                 return r;
> >
> >         if (ops->dma_map) {
> > -               r = ops->dma_map(vdpa, 0, iova, size, pa, perm, opaque);
> > +               r = ops->dma_map(vdpa, asid, iova, size, pa, perm,
> > + opaque);
> >         } else if (ops->set_map) {
> >                 if (!v->in_batch)
> > -                       r = ops->set_map(vdpa, 0, iotlb);
> > +                       r = ops->set_map(vdpa, asid, iotlb);
> >         } else {
> >                 r = iommu_map(v->domain, iova, pa, size,
> >                               perm_to_iommu_flags(perm)); @@ -725,17
> > +759,24 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,  {
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > +       u32 asid = iotlb_to_asid(iotlb);
> >
> >         vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1);
> >
> >         if (ops->dma_map) {
> > -               ops->dma_unmap(vdpa, 0, iova, size);
> > +               ops->dma_unmap(vdpa, asid, iova, size);
> >         } else if (ops->set_map) {
> >                 if (!v->in_batch)
> > -                       ops->set_map(vdpa, 0, iotlb);
> > +                       ops->set_map(vdpa, asid, iotlb);
> >         } else {
> >                 iommu_unmap(v->domain, iova, size);
> >         }
> > +
> > +       /* If we are in the middle of batch processing, delay the free
> > +        * of AS until BATCH_END.
> > +        */
> > +       if (!v->in_batch && !iotlb->nmaps)
> > +               vhost_vdpa_remove_as(v, asid);
> >  }
> >
> >  static int vhost_vdpa_va_map(struct vhost_vdpa *v, @@ -943,19 +984,38
> > @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> >         struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
> >         struct vdpa_device *vdpa = v->vdpa;
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > -       struct vhost_vdpa_as *as = asid_to_as(v, 0);
> > -       struct vhost_iotlb *iotlb = &as->iotlb;
> > +       struct vhost_iotlb *iotlb = NULL;
> > +       struct vhost_vdpa_as *as = NULL;
> >         int r = 0;
> >
> > -       if (asid != 0)
> > -               return -EINVAL;
> > -
> >         mutex_lock(&dev->mutex);
> >
> >         r = vhost_dev_check_owner(dev);
> >         if (r)
> >                 goto unlock;
> >
> > +       if (msg->type == VHOST_IOTLB_UPDATE ||
> > +           msg->type == VHOST_IOTLB_BATCH_BEGIN) {
> > +               as = vhost_vdpa_find_alloc_as(v, asid);
>
> I wonder if it's better to mandate the ASID to [0, dev->nas), otherwise user space is free to use arbitrary IDs which may exceeds the #address spaces that is supported by the device.
> [GD>>] Isn’t the following check in vhost_vdpa_alloc_as () sufficient to ensure ASID's value in the range [0, dev->nas):
>         if (asid >= v->vdpa->nas)
>                 return NULL;

I think you're right.

So we are fine.

Thanks

>
> Thanks
>
> > +               if (!as) {
> > +                       dev_err(&v->dev, "can't find and alloc asid %d\n",
> > +                               asid);
> > +                       return -EINVAL;
> > +               }
> > +               iotlb = &as->iotlb;
> > +       } else
> > +               iotlb = asid_to_iotlb(v, asid);
> > +
> > +       if ((v->in_batch && v->batch_asid != asid) || !iotlb) {
> > +               if (v->in_batch && v->batch_asid != asid) {
> > +                       dev_info(&v->dev, "batch id %d asid %d\n",
> > +                                v->batch_asid, asid);
> > +               }
> > +               if (!iotlb)
> > +                       dev_err(&v->dev, "no iotlb for asid %d\n", asid);
> > +               return -EINVAL;
> > +       }
> > +
> >         switch (msg->type) {
> >         case VHOST_IOTLB_UPDATE:
> >                 r = vhost_vdpa_process_iotlb_update(v, iotlb, msg); @@
> > -964,12 +1024,15 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> >                 vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
> >                 break;
> >         case VHOST_IOTLB_BATCH_BEGIN:
> > +               v->batch_asid = asid;
> >                 v->in_batch = true;
> >                 break;
> >         case VHOST_IOTLB_BATCH_END:
> >                 if (v->in_batch && ops->set_map)
> > -                       ops->set_map(vdpa, 0, iotlb);
> > +                       ops->set_map(vdpa, asid, iotlb);
> >                 v->in_batch = false;
> > +               if (!iotlb->nmaps)
> > +                       vhost_vdpa_remove_as(v, asid);
> >                 break;
> >         default:
> >                 r = -EINVAL;
> > @@ -1057,9 +1120,17 @@ static void vhost_vdpa_set_iova_range(struct
> > vhost_vdpa *v)
> >
> >  static void vhost_vdpa_cleanup(struct vhost_vdpa *v)  {
> > +       struct vhost_vdpa_as *as;
> > +       u32 asid;
> > +
> >         vhost_dev_cleanup(&v->vdev);
> >         kfree(v->vdev.vqs);
> > -       vhost_vdpa_remove_as(v, 0);
> > +
> > +       for (asid = 0; asid < v->vdpa->nas; asid++) {
> > +               as = asid_to_as(v, asid);
> > +               if (as)
> > +                       vhost_vdpa_remove_as(v, asid);
> > +       }
> >  }
> >
> >  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> > @@ -1095,12 +1166,9 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep)
> >         vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
> >                        vhost_vdpa_process_iotlb_msg);
> >
> > -       if (!vhost_vdpa_alloc_as(v, 0))
> > -               goto err_alloc_as;
> > -
> >         r = vhost_vdpa_alloc_domain(v);
> >         if (r)
> > -               goto err_alloc_as;
> > +               goto err_alloc_domain;
> >
> >         vhost_vdpa_set_iova_range(v);
> >
> > @@ -1108,7 +1176,7 @@ static int vhost_vdpa_open(struct inode *inode,
> > struct file *filep)
> >
> >         return 0;
> >
> > -err_alloc_as:
> > +err_alloc_domain:
> >         vhost_vdpa_cleanup(v);
> >  err:
> >         atomic_dec(&v->opened);
> > @@ -1233,8 +1301,11 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
> >         int minor;
> >         int i, r;
> >
> > -       /* Only support 1 address space and 1 groups */
> > -       if (vdpa->ngroups != 1 || vdpa->nas != 1)
> > +       /* We can't support platform IOMMU device with more than 1
> > +        * group or as
> > +        */
> > +       if (!ops->set_map && !ops->dma_map &&
> > +           (vdpa->ngroups > 1 || vdpa->nas > 1))
> >                 return -EOPNOTSUPP;
> >
> >         v = kzalloc(sizeof(*v), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index
> > d1e58f976f6e..5022c648d9c0 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1167,7 +1167,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
> >                                 ret = -EINVAL;
> >                                 goto done;
> >                         }
> > -                       offset = sizeof(__u16);
> > +                       offset = 0;
> >                 } else
> >                         offset = sizeof(__u32);
> >                 break;
> > --
> > 2.30.1
> >
>

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

Re: [PATCH v2 00/19] Control VQ support in vDPA

[Jason Wang]

On Thu, Mar 31, 2022 at 2:05 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> Hi All:
>
> This series tries to add the support for control virtqueue in vDPA.
>
> Control virtqueue is used by networking device for accepting various
> commands from the driver. It's a must to support multiqueue and other
> configurations.
>
> When used by vhost-vDPA bus driver for VM, the control virtqueue
> should be shadowed via userspace VMM (Qemu) instead of being assigned
> directly to Guest. This is because Qemu needs to know the device state
> in order to start and stop device correctly (e.g for Live Migration).
>
> This requies to isolate the memory mapping for control virtqueue
> presented by vhost-vDPA to prevent guest from accessing it directly.
>
> To achieve this, vDPA introduce two new abstractions:
>
> - address space: identified through address space id (ASID) and a set
>                  of memory mapping in maintained
> - virtqueue group: the minimal set of virtqueues that must share an
>                  address space
>
> Device needs to advertise the following attributes to vDPA:
>
> - the number of address spaces supported in the device
> - the number of virtqueue groups supported in the device
> - the mappings from a specific virtqueue to its virtqueue groups
>
> The mappings from virtqueue to virtqueue groups is fixed and defined
> by vDPA device driver. E.g:
>
> - For the device that has hardware ASID support, it can simply
>   advertise a per virtqueue group.
> - For the device that does not have hardware ASID support, it can
>   simply advertise a single virtqueue group that contains all
>   virtqueues. Or if it wants a software emulated control virtqueue, it
>   can advertise two virtqueue groups, one is for cvq, another is for
>   the rest virtqueues.
>
> vDPA also allow to change the association between virtqueue group and
> address space. So in the case of control virtqueue, userspace
> VMM(Qemu) may use a dedicated address space for the control virtqueue
> group to isolate the memory mapping.
>
> The vhost/vhost-vDPA is also extend for the userspace to:
>
> - query the number of virtqueue groups and address spaces supported by
>   the device
> - query the virtqueue group for a specific virtqueue
> - assocaite a virtqueue group with an address space
> - send ASID based IOTLB commands
>
> This will help userspace VMM(Qemu) to detect whether the control vq
> could be supported and isolate memory mappings of control virtqueue
> from the others.
>
> To demonstrate the usage, vDPA simulator is extended to support
> setting MAC address via a emulated control virtqueue.
>
> Please review.

Michael, this looks good to me, do you have comments on this?

Thanks

>
> Changes since RFC v2:
>
> - Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
> - Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
>   changed its return type to void.
> - Removed insignificant used_as member field from struct vhost_vdpa.
> - Corrected the iommu parameter in call to vringh_set_iotlb() from
>   vdpasim_set_group_asid()
> - Fixed build errors with vdpa_sim_net
> - Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
>   call updated vDPA APIs and ensured successful build
> - Tested control (MAC address configuration) and data-path using
>   single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device
>   and vdpa_sim_net software device using QEMU release at [1]
> - Removed two extra blank lines after set_group_asid() in
>   include/linux/vdpa.h
>
> Changes since v1:
>
> - Rebased the v1 patch series on vhost branch of MST vhost git repo
>   git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> - Updates to accommodate vdpa_sim changes from monolithic module in
>   kernel used v1 patch series to current modularized class (net, block)
>   based approach.
> - Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
>   propagated them from vdpa_sim_net to vdpa_sim
> - Widened the data-type for "asid" member of vhost_msg_v2 to __u32
>   to accommodate PASID
> - Fixed the buildbot warnings
> - Resolved all checkpatch.pl errors and warnings
> - Tested both control and datapath with Xilinx Smartnic SN1000 series
>   device using QEMU implementing the Shadow virtqueue and support for
>   VQ groups and ASID available at [1]
>
> Changes since RFC:
>
> - tweak vhost uAPI documentation
> - switch to use device specific IOTLB really in patch 4
> - tweak the commit log
> - fix that ASID in vhost is claimed to be 32 actually but 16bit
>   actually
> - fix use after free when using ASID with IOTLB batching requests
> - switch to use Stefano's patch for having separated iov
> - remove unused "used_as" variable
> - fix the iotlb/asid checking in vhost_vdpa_unmap()
>
> [1] Development QEMU release with support for SVQ, VQ groups and ASID:
>   github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
>   asid_groups-v1.d%2F00
>
> Thanks
>
> Gautam Dawar (19):
>   vhost: move the backend feature bits to vhost_types.h
>   virtio-vdpa: don't set callback if virtio doesn't need it
>   vhost-vdpa: passing iotlb to IOMMU mapping helpers
>   vhost-vdpa: switch to use vhost-vdpa specific IOTLB
>   vdpa: introduce virtqueue groups
>   vdpa: multiple address spaces support
>   vdpa: introduce config operations for associating ASID to a virtqueue
>     group
>   vhost_iotlb: split out IOTLB initialization
>   vhost: support ASID in IOTLB API
>   vhost-vdpa: introduce asid based IOTLB
>   vhost-vdpa: introduce uAPI to get the number of virtqueue groups
>   vhost-vdpa: introduce uAPI to get the number of address spaces
>   vhost-vdpa: uAPI to get virtqueue group id
>   vhost-vdpa: introduce uAPI to set group ASID
>   vhost-vdpa: support ASID based IOTLB API
>   vdpa_sim: advertise VIRTIO_NET_F_MTU
>   vdpa_sim: factor out buffer completion logic
>   vdpa_sim: filter destination mac address
>   vdpasim: control virtqueue support
>
>  drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
>  drivers/vdpa/vdpa.c                  |   5 +
>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
>  drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
>  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
>  drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
>  drivers/vhost/iotlb.c                |  23 ++-
>  drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
>  drivers/vhost/vhost.c                |  23 ++-
>  drivers/vhost/vhost.h                |   4 +-
>  drivers/virtio/virtio_vdpa.c         |   2 +-
>  include/linux/vdpa.h                 |  44 ++++-
>  include/linux/vhost_iotlb.h          |   2 +
>  include/uapi/linux/vhost.h           |  26 ++-
>  include/uapi/linux/vhost_types.h     |  11 +-
>  18 files changed, 563 insertions(+), 142 deletions(-)
>
> --
> 2.30.1
>

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

Re: [PATCH v2 00/19] Control VQ support in vDPA

[Jason Wang]

On Thu, Mar 31, 2022 at 2:05 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> Hi All:
>
> This series tries to add the support for control virtqueue in vDPA.
>
> Control virtqueue is used by networking device for accepting various
> commands from the driver. It's a must to support multiqueue and other
> configurations.
>
> When used by vhost-vDPA bus driver for VM, the control virtqueue
> should be shadowed via userspace VMM (Qemu) instead of being assigned
> directly to Guest. This is because Qemu needs to know the device state
> in order to start and stop device correctly (e.g for Live Migration).
>
> This requies to isolate the memory mapping for control virtqueue
> presented by vhost-vDPA to prevent guest from accessing it directly.
>
> To achieve this, vDPA introduce two new abstractions:
>
> - address space: identified through address space id (ASID) and a set
>                  of memory mapping in maintained
> - virtqueue group: the minimal set of virtqueues that must share an
>                  address space
>
> Device needs to advertise the following attributes to vDPA:
>
> - the number of address spaces supported in the device
> - the number of virtqueue groups supported in the device
> - the mappings from a specific virtqueue to its virtqueue groups
>
> The mappings from virtqueue to virtqueue groups is fixed and defined
> by vDPA device driver. E.g:
>
> - For the device that has hardware ASID support, it can simply
>   advertise a per virtqueue group.
> - For the device that does not have hardware ASID support, it can
>   simply advertise a single virtqueue group that contains all
>   virtqueues. Or if it wants a software emulated control virtqueue, it
>   can advertise two virtqueue groups, one is for cvq, another is for
>   the rest virtqueues.
>
> vDPA also allow to change the association between virtqueue group and
> address space. So in the case of control virtqueue, userspace
> VMM(Qemu) may use a dedicated address space for the control virtqueue
> group to isolate the memory mapping.
>
> The vhost/vhost-vDPA is also extend for the userspace to:
>
> - query the number of virtqueue groups and address spaces supported by
>   the device
> - query the virtqueue group for a specific virtqueue
> - assocaite a virtqueue group with an address space
> - send ASID based IOTLB commands
>
> This will help userspace VMM(Qemu) to detect whether the control vq
> could be supported and isolate memory mappings of control virtqueue
> from the others.
>
> To demonstrate the usage, vDPA simulator is extended to support
> setting MAC address via a emulated control virtqueue.
>
> Please review.

Michael, this looks good to me, do you have comments on this?

Thanks

>
> Changes since RFC v2:
>
> - Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
> - Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
>   changed its return type to void.
> - Removed insignificant used_as member field from struct vhost_vdpa.
> - Corrected the iommu parameter in call to vringh_set_iotlb() from
>   vdpasim_set_group_asid()
> - Fixed build errors with vdpa_sim_net
> - Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
>   call updated vDPA APIs and ensured successful build
> - Tested control (MAC address configuration) and data-path using
>   single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device
>   and vdpa_sim_net software device using QEMU release at [1]
> - Removed two extra blank lines after set_group_asid() in
>   include/linux/vdpa.h
>
> Changes since v1:
>
> - Rebased the v1 patch series on vhost branch of MST vhost git repo
>   git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> - Updates to accommodate vdpa_sim changes from monolithic module in
>   kernel used v1 patch series to current modularized class (net, block)
>   based approach.
> - Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
>   propagated them from vdpa_sim_net to vdpa_sim
> - Widened the data-type for "asid" member of vhost_msg_v2 to __u32
>   to accommodate PASID
> - Fixed the buildbot warnings
> - Resolved all checkpatch.pl errors and warnings
> - Tested both control and datapath with Xilinx Smartnic SN1000 series
>   device using QEMU implementing the Shadow virtqueue and support for
>   VQ groups and ASID available at [1]
>
> Changes since RFC:
>
> - tweak vhost uAPI documentation
> - switch to use device specific IOTLB really in patch 4
> - tweak the commit log
> - fix that ASID in vhost is claimed to be 32 actually but 16bit
>   actually
> - fix use after free when using ASID with IOTLB batching requests
> - switch to use Stefano's patch for having separated iov
> - remove unused "used_as" variable
> - fix the iotlb/asid checking in vhost_vdpa_unmap()
>
> [1] Development QEMU release with support for SVQ, VQ groups and ASID:
>   github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
>   asid_groups-v1.d%2F00
>
> Thanks
>
> Gautam Dawar (19):
>   vhost: move the backend feature bits to vhost_types.h
>   virtio-vdpa: don't set callback if virtio doesn't need it
>   vhost-vdpa: passing iotlb to IOMMU mapping helpers
>   vhost-vdpa: switch to use vhost-vdpa specific IOTLB
>   vdpa: introduce virtqueue groups
>   vdpa: multiple address spaces support
>   vdpa: introduce config operations for associating ASID to a virtqueue
>     group
>   vhost_iotlb: split out IOTLB initialization
>   vhost: support ASID in IOTLB API
>   vhost-vdpa: introduce asid based IOTLB
>   vhost-vdpa: introduce uAPI to get the number of virtqueue groups
>   vhost-vdpa: introduce uAPI to get the number of address spaces
>   vhost-vdpa: uAPI to get virtqueue group id
>   vhost-vdpa: introduce uAPI to set group ASID
>   vhost-vdpa: support ASID based IOTLB API
>   vdpa_sim: advertise VIRTIO_NET_F_MTU
>   vdpa_sim: factor out buffer completion logic
>   vdpa_sim: filter destination mac address
>   vdpasim: control virtqueue support
>
>  drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
>  drivers/vdpa/vdpa.c                  |   5 +
>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
>  drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
>  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
>  drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
>  drivers/vhost/iotlb.c                |  23 ++-
>  drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
>  drivers/vhost/vhost.c                |  23 ++-
>  drivers/vhost/vhost.h                |   4 +-
>  drivers/virtio/virtio_vdpa.c         |   2 +-
>  include/linux/vdpa.h                 |  44 ++++-
>  include/linux/vhost_iotlb.h          |   2 +
>  include/uapi/linux/vhost.h           |  26 ++-
>  include/uapi/linux/vhost_types.h     |  11 +-
>  18 files changed, 563 insertions(+), 142 deletions(-)
>
> --
> 2.30.1
>

Re: [PATCH v2 00/19] Control VQ support in vDPA

[Michael S. Tsirkin]

On Mon, May 09, 2022 at 11:42:10AM +0800, Jason Wang wrote:
> On Thu, Mar 31, 2022 at 2:05 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> >
> > Hi All:
> >
> > This series tries to add the support for control virtqueue in vDPA.
> >
> > Control virtqueue is used by networking device for accepting various
> > commands from the driver. It's a must to support multiqueue and other
> > configurations.
> >
> > When used by vhost-vDPA bus driver for VM, the control virtqueue
> > should be shadowed via userspace VMM (Qemu) instead of being assigned
> > directly to Guest. This is because Qemu needs to know the device state
> > in order to start and stop device correctly (e.g for Live Migration).
> >
> > This requies to isolate the memory mapping for control virtqueue
> > presented by vhost-vDPA to prevent guest from accessing it directly.
> >
> > To achieve this, vDPA introduce two new abstractions:
> >
> > - address space: identified through address space id (ASID) and a set
> >                  of memory mapping in maintained
> > - virtqueue group: the minimal set of virtqueues that must share an
> >                  address space
> >
> > Device needs to advertise the following attributes to vDPA:
> >
> > - the number of address spaces supported in the device
> > - the number of virtqueue groups supported in the device
> > - the mappings from a specific virtqueue to its virtqueue groups
> >
> > The mappings from virtqueue to virtqueue groups is fixed and defined
> > by vDPA device driver. E.g:
> >
> > - For the device that has hardware ASID support, it can simply
> >   advertise a per virtqueue group.
> > - For the device that does not have hardware ASID support, it can
> >   simply advertise a single virtqueue group that contains all
> >   virtqueues. Or if it wants a software emulated control virtqueue, it
> >   can advertise two virtqueue groups, one is for cvq, another is for
> >   the rest virtqueues.
> >
> > vDPA also allow to change the association between virtqueue group and
> > address space. So in the case of control virtqueue, userspace
> > VMM(Qemu) may use a dedicated address space for the control virtqueue
> > group to isolate the memory mapping.
> >
> > The vhost/vhost-vDPA is also extend for the userspace to:
> >
> > - query the number of virtqueue groups and address spaces supported by
> >   the device
> > - query the virtqueue group for a specific virtqueue
> > - assocaite a virtqueue group with an address space
> > - send ASID based IOTLB commands
> >
> > This will help userspace VMM(Qemu) to detect whether the control vq
> > could be supported and isolate memory mappings of control virtqueue
> > from the others.
> >
> > To demonstrate the usage, vDPA simulator is extended to support
> > setting MAC address via a emulated control virtqueue.
> >
> > Please review.
> 
> Michael, this looks good to me, do you have comments on this?
> 
> Thanks


I'll merge this for next.

> >
> > Changes since RFC v2:
> >
> > - Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
> > - Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
> >   changed its return type to void.
> > - Removed insignificant used_as member field from struct vhost_vdpa.
> > - Corrected the iommu parameter in call to vringh_set_iotlb() from
> >   vdpasim_set_group_asid()
> > - Fixed build errors with vdpa_sim_net
> > - Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
> >   call updated vDPA APIs and ensured successful build
> > - Tested control (MAC address configuration) and data-path using
> >   single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device
> >   and vdpa_sim_net software device using QEMU release at [1]
> > - Removed two extra blank lines after set_group_asid() in
> >   include/linux/vdpa.h
> >
> > Changes since v1:
> >
> > - Rebased the v1 patch series on vhost branch of MST vhost git repo
> >   git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> > - Updates to accommodate vdpa_sim changes from monolithic module in
> >   kernel used v1 patch series to current modularized class (net, block)
> >   based approach.
> > - Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
> >   propagated them from vdpa_sim_net to vdpa_sim
> > - Widened the data-type for "asid" member of vhost_msg_v2 to __u32
> >   to accommodate PASID
> > - Fixed the buildbot warnings
> > - Resolved all checkpatch.pl errors and warnings
> > - Tested both control and datapath with Xilinx Smartnic SN1000 series
> >   device using QEMU implementing the Shadow virtqueue and support for
> >   VQ groups and ASID available at [1]
> >
> > Changes since RFC:
> >
> > - tweak vhost uAPI documentation
> > - switch to use device specific IOTLB really in patch 4
> > - tweak the commit log
> > - fix that ASID in vhost is claimed to be 32 actually but 16bit
> >   actually
> > - fix use after free when using ASID with IOTLB batching requests
> > - switch to use Stefano's patch for having separated iov
> > - remove unused "used_as" variable
> > - fix the iotlb/asid checking in vhost_vdpa_unmap()
> >
> > [1] Development QEMU release with support for SVQ, VQ groups and ASID:
> >   github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
> >   asid_groups-v1.d%2F00
> >
> > Thanks
> >
> > Gautam Dawar (19):
> >   vhost: move the backend feature bits to vhost_types.h
> >   virtio-vdpa: don't set callback if virtio doesn't need it
> >   vhost-vdpa: passing iotlb to IOMMU mapping helpers
> >   vhost-vdpa: switch to use vhost-vdpa specific IOTLB
> >   vdpa: introduce virtqueue groups
> >   vdpa: multiple address spaces support
> >   vdpa: introduce config operations for associating ASID to a virtqueue
> >     group
> >   vhost_iotlb: split out IOTLB initialization
> >   vhost: support ASID in IOTLB API
> >   vhost-vdpa: introduce asid based IOTLB
> >   vhost-vdpa: introduce uAPI to get the number of virtqueue groups
> >   vhost-vdpa: introduce uAPI to get the number of address spaces
> >   vhost-vdpa: uAPI to get virtqueue group id
> >   vhost-vdpa: introduce uAPI to set group ASID
> >   vhost-vdpa: support ASID based IOTLB API
> >   vdpa_sim: advertise VIRTIO_NET_F_MTU
> >   vdpa_sim: factor out buffer completion logic
> >   vdpa_sim: filter destination mac address
> >   vdpasim: control virtqueue support
> >
> >  drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
> >  drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
> >  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
> >  drivers/vdpa/vdpa.c                  |   5 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
> >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
> >  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
> >  drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
> >  drivers/vhost/iotlb.c                |  23 ++-
> >  drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
> >  drivers/vhost/vhost.c                |  23 ++-
> >  drivers/vhost/vhost.h                |   4 +-
> >  drivers/virtio/virtio_vdpa.c         |   2 +-
> >  include/linux/vdpa.h                 |  44 ++++-
> >  include/linux/vhost_iotlb.h          |   2 +
> >  include/uapi/linux/vhost.h           |  26 ++-
> >  include/uapi/linux/vhost_types.h     |  11 +-
> >  18 files changed, 563 insertions(+), 142 deletions(-)
> >
> > --
> > 2.30.1
> >

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

Re: [PATCH v2 00/19] Control VQ support in vDPA

[Michael S. Tsirkin]

On Mon, May 09, 2022 at 11:42:10AM +0800, Jason Wang wrote:
> On Thu, Mar 31, 2022 at 2:05 AM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> >
> > Hi All:
> >
> > This series tries to add the support for control virtqueue in vDPA.
> >
> > Control virtqueue is used by networking device for accepting various
> > commands from the driver. It's a must to support multiqueue and other
> > configurations.
> >
> > When used by vhost-vDPA bus driver for VM, the control virtqueue
> > should be shadowed via userspace VMM (Qemu) instead of being assigned
> > directly to Guest. This is because Qemu needs to know the device state
> > in order to start and stop device correctly (e.g for Live Migration).
> >
> > This requies to isolate the memory mapping for control virtqueue
> > presented by vhost-vDPA to prevent guest from accessing it directly.
> >
> > To achieve this, vDPA introduce two new abstractions:
> >
> > - address space: identified through address space id (ASID) and a set
> >                  of memory mapping in maintained
> > - virtqueue group: the minimal set of virtqueues that must share an
> >                  address space
> >
> > Device needs to advertise the following attributes to vDPA:
> >
> > - the number of address spaces supported in the device
> > - the number of virtqueue groups supported in the device
> > - the mappings from a specific virtqueue to its virtqueue groups
> >
> > The mappings from virtqueue to virtqueue groups is fixed and defined
> > by vDPA device driver. E.g:
> >
> > - For the device that has hardware ASID support, it can simply
> >   advertise a per virtqueue group.
> > - For the device that does not have hardware ASID support, it can
> >   simply advertise a single virtqueue group that contains all
> >   virtqueues. Or if it wants a software emulated control virtqueue, it
> >   can advertise two virtqueue groups, one is for cvq, another is for
> >   the rest virtqueues.
> >
> > vDPA also allow to change the association between virtqueue group and
> > address space. So in the case of control virtqueue, userspace
> > VMM(Qemu) may use a dedicated address space for the control virtqueue
> > group to isolate the memory mapping.
> >
> > The vhost/vhost-vDPA is also extend for the userspace to:
> >
> > - query the number of virtqueue groups and address spaces supported by
> >   the device
> > - query the virtqueue group for a specific virtqueue
> > - assocaite a virtqueue group with an address space
> > - send ASID based IOTLB commands
> >
> > This will help userspace VMM(Qemu) to detect whether the control vq
> > could be supported and isolate memory mappings of control virtqueue
> > from the others.
> >
> > To demonstrate the usage, vDPA simulator is extended to support
> > setting MAC address via a emulated control virtqueue.
> >
> > Please review.
> 
> Michael, this looks good to me, do you have comments on this?
> 
> Thanks


I'll merge this for next.

> >
> > Changes since RFC v2:
> >
> > - Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
> > - Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
> >   changed its return type to void.
> > - Removed insignificant used_as member field from struct vhost_vdpa.
> > - Corrected the iommu parameter in call to vringh_set_iotlb() from
> >   vdpasim_set_group_asid()
> > - Fixed build errors with vdpa_sim_net
> > - Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
> >   call updated vDPA APIs and ensured successful build
> > - Tested control (MAC address configuration) and data-path using
> >   single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device
> >   and vdpa_sim_net software device using QEMU release at [1]
> > - Removed two extra blank lines after set_group_asid() in
> >   include/linux/vdpa.h
> >
> > Changes since v1:
> >
> > - Rebased the v1 patch series on vhost branch of MST vhost git repo
> >   git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> > - Updates to accommodate vdpa_sim changes from monolithic module in
> >   kernel used v1 patch series to current modularized class (net, block)
> >   based approach.
> > - Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
> >   propagated them from vdpa_sim_net to vdpa_sim
> > - Widened the data-type for "asid" member of vhost_msg_v2 to __u32
> >   to accommodate PASID
> > - Fixed the buildbot warnings
> > - Resolved all checkpatch.pl errors and warnings
> > - Tested both control and datapath with Xilinx Smartnic SN1000 series
> >   device using QEMU implementing the Shadow virtqueue and support for
> >   VQ groups and ASID available at [1]
> >
> > Changes since RFC:
> >
> > - tweak vhost uAPI documentation
> > - switch to use device specific IOTLB really in patch 4
> > - tweak the commit log
> > - fix that ASID in vhost is claimed to be 32 actually but 16bit
> >   actually
> > - fix use after free when using ASID with IOTLB batching requests
> > - switch to use Stefano's patch for having separated iov
> > - remove unused "used_as" variable
> > - fix the iotlb/asid checking in vhost_vdpa_unmap()
> >
> > [1] Development QEMU release with support for SVQ, VQ groups and ASID:
> >   github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
> >   asid_groups-v1.d%2F00
> >
> > Thanks
> >
> > Gautam Dawar (19):
> >   vhost: move the backend feature bits to vhost_types.h
> >   virtio-vdpa: don't set callback if virtio doesn't need it
> >   vhost-vdpa: passing iotlb to IOMMU mapping helpers
> >   vhost-vdpa: switch to use vhost-vdpa specific IOTLB
> >   vdpa: introduce virtqueue groups
> >   vdpa: multiple address spaces support
> >   vdpa: introduce config operations for associating ASID to a virtqueue
> >     group
> >   vhost_iotlb: split out IOTLB initialization
> >   vhost: support ASID in IOTLB API
> >   vhost-vdpa: introduce asid based IOTLB
> >   vhost-vdpa: introduce uAPI to get the number of virtqueue groups
> >   vhost-vdpa: introduce uAPI to get the number of address spaces
> >   vhost-vdpa: uAPI to get virtqueue group id
> >   vhost-vdpa: introduce uAPI to set group ASID
> >   vhost-vdpa: support ASID based IOTLB API
> >   vdpa_sim: advertise VIRTIO_NET_F_MTU
> >   vdpa_sim: factor out buffer completion logic
> >   vdpa_sim: filter destination mac address
> >   vdpasim: control virtqueue support
> >
> >  drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
> >  drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
> >  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
> >  drivers/vdpa/vdpa.c                  |   5 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
> >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
> >  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
> >  drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
> >  drivers/vhost/iotlb.c                |  23 ++-
> >  drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
> >  drivers/vhost/vhost.c                |  23 ++-
> >  drivers/vhost/vhost.h                |   4 +-
> >  drivers/virtio/virtio_vdpa.c         |   2 +-
> >  include/linux/vdpa.h                 |  44 ++++-
> >  include/linux/vhost_iotlb.h          |   2 +
> >  include/uapi/linux/vhost.h           |  26 ++-
> >  include/uapi/linux/vhost_types.h     |  11 +-
> >  18 files changed, 563 insertions(+), 142 deletions(-)
> >
> > --
> > 2.30.1
> >

Re: [PATCH v2 00/19] Control VQ support in vDPA

[Eugenio Perez Martin]

On Wed, Mar 30, 2022 at 8:05 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> Hi All:
>
> This series tries to add the support for control virtqueue in vDPA.
>
> Control virtqueue is used by networking device for accepting various
> commands from the driver. It's a must to support multiqueue and other
> configurations.
>
> When used by vhost-vDPA bus driver for VM, the control virtqueue
> should be shadowed via userspace VMM (Qemu) instead of being assigned
> directly to Guest. This is because Qemu needs to know the device state
> in order to start and stop device correctly (e.g for Live Migration).
>
> This requies to isolate the memory mapping for control virtqueue
> presented by vhost-vDPA to prevent guest from accessing it directly.
>
> To achieve this, vDPA introduce two new abstractions:
>
> - address space: identified through address space id (ASID) and a set
>                  of memory mapping in maintained
> - virtqueue group: the minimal set of virtqueues that must share an
>                  address space
>
> Device needs to advertise the following attributes to vDPA:
>
> - the number of address spaces supported in the device
> - the number of virtqueue groups supported in the device
> - the mappings from a specific virtqueue to its virtqueue groups
>
> The mappings from virtqueue to virtqueue groups is fixed and defined
> by vDPA device driver. E.g:
>
> - For the device that has hardware ASID support, it can simply
>   advertise a per virtqueue group.
> - For the device that does not have hardware ASID support, it can
>   simply advertise a single virtqueue group that contains all
>   virtqueues. Or if it wants a software emulated control virtqueue, it
>   can advertise two virtqueue groups, one is for cvq, another is for
>   the rest virtqueues.
>
> vDPA also allow to change the association between virtqueue group and
> address space. So in the case of control virtqueue, userspace
> VMM(Qemu) may use a dedicated address space for the control virtqueue
> group to isolate the memory mapping.
>
> The vhost/vhost-vDPA is also extend for the userspace to:
>
> - query the number of virtqueue groups and address spaces supported by
>   the device
> - query the virtqueue group for a specific virtqueue
> - assocaite a virtqueue group with an address space
> - send ASID based IOTLB commands
>
> This will help userspace VMM(Qemu) to detect whether the control vq
> could be supported and isolate memory mappings of control virtqueue
> from the others.
>
> To demonstrate the usage, vDPA simulator is extended to support
> setting MAC address via a emulated control virtqueue.
>
> Please review.
>
> Changes since RFC v2:
>
> - Fixed memory leak for asid 0 in vhost_vdpa_remove_as()
> - Removed unnecessary NULL check for iotlb in vhost_vdpa_unmap() and
>   changed its return type to void.
> - Removed insignificant used_as member field from struct vhost_vdpa.
> - Corrected the iommu parameter in call to vringh_set_iotlb() from
>   vdpasim_set_group_asid()
> - Fixed build errors with vdpa_sim_net
> - Updated alibaba, vdpa_user and virtio_pci vdpa parent drivers to
>   call updated vDPA APIs and ensured successful build
> - Tested control (MAC address configuration) and data-path using
>   single virtqueue pair on Xilinx (now AMD) SN1022 SmartNIC device
>   and vdpa_sim_net software device using QEMU release at [1]
> - Removed two extra blank lines after set_group_asid() in
>   include/linux/vdpa.h
>
> Changes since v1:
>
> - Rebased the v1 patch series on vhost branch of MST vhost git repo
>   git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> - Updates to accommodate vdpa_sim changes from monolithic module in
>   kernel used v1 patch series to current modularized class (net, block)
>   based approach.
> - Added new attributes (ngroups and nas) to "vdpasim_dev_attr" and
>   propagated them from vdpa_sim_net to vdpa_sim
> - Widened the data-type for "asid" member of vhost_msg_v2 to __u32
>   to accommodate PASID
> - Fixed the buildbot warnings
> - Resolved all checkpatch.pl errors and warnings
> - Tested both control and datapath with Xilinx Smartnic SN1000 series
>   device using QEMU implementing the Shadow virtqueue and support for
>   VQ groups and ASID available at [1]
>
> Changes since RFC:
>
> - tweak vhost uAPI documentation
> - switch to use device specific IOTLB really in patch 4
> - tweak the commit log
> - fix that ASID in vhost is claimed to be 32 actually but 16bit
>   actually
> - fix use after free when using ASID with IOTLB batching requests
> - switch to use Stefano's patch for having separated iov
> - remove unused "used_as" variable
> - fix the iotlb/asid checking in vhost_vdpa_unmap()
>
> [1] Development QEMU release with support for SVQ, VQ groups and ASID:
>   github.com/eugpermar/qemu/releases/tag/vdpa_sw_live_migration.d%2F
>   asid_groups-v1.d%2F00
>
> Thanks
>
> Gautam Dawar (19):
>   vhost: move the backend feature bits to vhost_types.h
>   virtio-vdpa: don't set callback if virtio doesn't need it
>   vhost-vdpa: passing iotlb to IOMMU mapping helpers
>   vhost-vdpa: switch to use vhost-vdpa specific IOTLB
>   vdpa: introduce virtqueue groups
>   vdpa: multiple address spaces support
>   vdpa: introduce config operations for associating ASID to a virtqueue
>     group
>   vhost_iotlb: split out IOTLB initialization
>   vhost: support ASID in IOTLB API
>   vhost-vdpa: introduce asid based IOTLB
>   vhost-vdpa: introduce uAPI to get the number of virtqueue groups
>   vhost-vdpa: introduce uAPI to get the number of address spaces
>   vhost-vdpa: uAPI to get virtqueue group id
>   vhost-vdpa: introduce uAPI to set group ASID
>   vhost-vdpa: support ASID based IOTLB API
>   vdpa_sim: advertise VIRTIO_NET_F_MTU
>   vdpa_sim: factor out buffer completion logic
>   vdpa_sim: filter destination mac address
>   vdpasim: control virtqueue support
>
>  drivers/vdpa/alibaba/eni_vdpa.c      |   2 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c      |   8 +-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  11 +-
>  drivers/vdpa/vdpa.c                  |   5 +
>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 100 ++++++++--
>  drivers/vdpa/vdpa_sim/vdpa_sim.h     |   3 +
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 169 +++++++++++++----
>  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
>  drivers/vdpa/virtio_pci/vp_vdpa.c    |   2 +-
>  drivers/vhost/iotlb.c                |  23 ++-
>  drivers/vhost/vdpa.c                 | 267 +++++++++++++++++++++------
>  drivers/vhost/vhost.c                |  23 ++-
>  drivers/vhost/vhost.h                |   4 +-
>  drivers/virtio/virtio_vdpa.c         |   2 +-
>  include/linux/vdpa.h                 |  44 ++++-
>  include/linux/vhost_iotlb.h          |   2 +
>  include/uapi/linux/vhost.h           |  26 ++-
>  include/uapi/linux/vhost_types.h     |  11 +-
>  18 files changed, 563 insertions(+), 142 deletions(-)
>

Hoping it's not too late for this, but I tested this series with my
latest qemu CVQ SVQ RFC (v8). So, in that regard,

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Stefano Garzarella]

Hi Gautam,

On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> This patch introduces the control virtqueue support for vDPA
> simulator. This is a requirement for supporting advanced features like
> multiqueue.
>
> A requirement for control virtqueue is to isolate its memory access
> from the rx/tx virtqueues. This is because when using vDPA device
> for VM, the control virqueue is not directly assigned to VM. Userspace
> (Qemu) will present a shadow control virtqueue to control for
> recording the device states.
>
> The isolation is done via the virtqueue groups and ASID support in
> vDPA through vhost-vdpa. The simulator is extended to have:
>
> 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
> 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
>    contains CVQ
> 3) two address spaces and the simulator simply implements the address
>    spaces by mapping it 1:1 to IOTLB.
>
> For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
> to group 1. So we have:
>
> 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
>    RX and TX can be assigned to guest directly.
> 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
>    is the buffers that allocated and managed by VMM only. So CVQ of
>    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
>    of vhost-vdpa.
>
> For the other use cases, since AS 0 is associated to all virtqueue
> groups by default. All virtqueues share the same mapping by default.
>
> To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
> implemented in the simulator for the driver to set mac address.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
>  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
>  3 files changed, 161 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 659e2e2e4b0c..51bd0bafce06 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
>  {
>         int i;
>
> -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
> +       spin_lock(&vdpasim->iommu_lock);
> +
> +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
>                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
> +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> +                                &vdpasim->iommu_lock);
> +       }
> +
> +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> +               vhost_iotlb_reset(&vdpasim->iommu[i]);
>
> -       spin_lock(&vdpasim->iommu_lock);
> -       vhost_iotlb_reset(vdpasim->iommu);
>         spin_unlock(&vdpasim->iommu_lock);
>
>         vdpasim->features = 0;
> @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
>         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
>
>         spin_lock(&vdpasim->iommu_lock);
> -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
> +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
>                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
>         spin_unlock(&vdpasim->iommu_lock);
>
> @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
>                                 size_t size)
>  {
>         spin_lock(&vdpasim->iommu_lock);
> -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
> +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
>                               (u64)dma_addr + size - 1);
>         spin_unlock(&vdpasim->iommu_lock);
>
> @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>         else
>                 ops = &vdpasim_config_ops;
>
> -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
> -                                   1, dev_attr->name, false);
> +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
> +                                   dev_attr->ngroups, dev_attr->nas,
> +                                   dev_attr->name, false);
>         if (IS_ERR(vdpasim)) {
>                 ret = PTR_ERR(vdpasim);
>                 goto err_alloc;
> @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>         if (!vdpasim->vqs)
>                 goto err_iommu;
>
> -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
> +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
> +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
>         if (!vdpasim->iommu)
>                 goto err_iommu;
>
> +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
> +
>         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
>         if (!vdpasim->buffer)
>                 goto err_iommu;
>
>         for (i = 0; i < dev_attr->nvqs; i++)
> -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
> +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>                                  &vdpasim->iommu_lock);
>
>         ret = iova_cache_get();
> @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
>
>  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
>  {
> -       return 0;
> +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
> +       if (idx == 2)
> +               return 1;
> +       else
> +               return 0;

This code only works for the vDPA-net simulator, since 
vdpasim_get_vq_group() is also shared with other simulators (e.g.  
vdpa_sim_blk), should we move this net-specific code into 
vdpa_sim_net.c, maybe adding a callback implemented by the different 
simulators?

Thanks,
Stefano

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Stefano Garzarella]

Hi Gautam,

On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>
> This patch introduces the control virtqueue support for vDPA
> simulator. This is a requirement for supporting advanced features like
> multiqueue.
>
> A requirement for control virtqueue is to isolate its memory access
> from the rx/tx virtqueues. This is because when using vDPA device
> for VM, the control virqueue is not directly assigned to VM. Userspace
> (Qemu) will present a shadow control virtqueue to control for
> recording the device states.
>
> The isolation is done via the virtqueue groups and ASID support in
> vDPA through vhost-vdpa. The simulator is extended to have:
>
> 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
> 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
>    contains CVQ
> 3) two address spaces and the simulator simply implements the address
>    spaces by mapping it 1:1 to IOTLB.
>
> For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
> to group 1. So we have:
>
> 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
>    RX and TX can be assigned to guest directly.
> 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
>    is the buffers that allocated and managed by VMM only. So CVQ of
>    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
>    of vhost-vdpa.
>
> For the other use cases, since AS 0 is associated to all virtqueue
> groups by default. All virtqueues share the same mapping by default.
>
> To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
> implemented in the simulator for the driver to set mac address.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
>  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
>  3 files changed, 161 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 659e2e2e4b0c..51bd0bafce06 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
>  {
>         int i;
>
> -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
> +       spin_lock(&vdpasim->iommu_lock);
> +
> +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
>                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
> +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> +                                &vdpasim->iommu_lock);
> +       }
> +
> +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> +               vhost_iotlb_reset(&vdpasim->iommu[i]);
>
> -       spin_lock(&vdpasim->iommu_lock);
> -       vhost_iotlb_reset(vdpasim->iommu);
>         spin_unlock(&vdpasim->iommu_lock);
>
>         vdpasim->features = 0;
> @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
>         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
>
>         spin_lock(&vdpasim->iommu_lock);
> -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
> +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
>                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
>         spin_unlock(&vdpasim->iommu_lock);
>
> @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
>                                 size_t size)
>  {
>         spin_lock(&vdpasim->iommu_lock);
> -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
> +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
>                               (u64)dma_addr + size - 1);
>         spin_unlock(&vdpasim->iommu_lock);
>
> @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>         else
>                 ops = &vdpasim_config_ops;
>
> -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
> -                                   1, dev_attr->name, false);
> +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
> +                                   dev_attr->ngroups, dev_attr->nas,
> +                                   dev_attr->name, false);
>         if (IS_ERR(vdpasim)) {
>                 ret = PTR_ERR(vdpasim);
>                 goto err_alloc;
> @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>         if (!vdpasim->vqs)
>                 goto err_iommu;
>
> -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
> +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
> +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
>         if (!vdpasim->iommu)
>                 goto err_iommu;
>
> +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
> +
>         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
>         if (!vdpasim->buffer)
>                 goto err_iommu;
>
>         for (i = 0; i < dev_attr->nvqs; i++)
> -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
> +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>                                  &vdpasim->iommu_lock);
>
>         ret = iova_cache_get();
> @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
>
>  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
>  {
> -       return 0;
> +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
> +       if (idx == 2)
> +               return 1;
> +       else
> +               return 0;

This code only works for the vDPA-net simulator, since 
vdpasim_get_vq_group() is also shared with other simulators (e.g.  
vdpa_sim_blk), should we move this net-specific code into 
vdpa_sim_net.c, maybe adding a callback implemented by the different 
simulators?

Thanks,
Stefano

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

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Eugenio Perez Martin]

On Tue, Jun 21, 2022 at 5:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> Hi Gautam,
>
> On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> >
> > This patch introduces the control virtqueue support for vDPA
> > simulator. This is a requirement for supporting advanced features like
> > multiqueue.
> >
> > A requirement for control virtqueue is to isolate its memory access
> > from the rx/tx virtqueues. This is because when using vDPA device
> > for VM, the control virqueue is not directly assigned to VM. Userspace
> > (Qemu) will present a shadow control virtqueue to control for
> > recording the device states.
> >
> > The isolation is done via the virtqueue groups and ASID support in
> > vDPA through vhost-vdpa. The simulator is extended to have:
> >
> > 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
> > 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
> >    contains CVQ
> > 3) two address spaces and the simulator simply implements the address
> >    spaces by mapping it 1:1 to IOTLB.
> >
> > For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
> > to group 1. So we have:
> >
> > 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
> >    RX and TX can be assigned to guest directly.
> > 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
> >    is the buffers that allocated and managed by VMM only. So CVQ of
> >    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
> >    of vhost-vdpa.
> >
> > For the other use cases, since AS 0 is associated to all virtqueue
> > groups by default. All virtqueues share the same mapping by default.
> >
> > To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
> > implemented in the simulator for the driver to set mac address.
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> > ---
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
> >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
> >  3 files changed, 161 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > index 659e2e2e4b0c..51bd0bafce06 100644
> > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
> >  {
> >         int i;
> >
> > -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
> > +       spin_lock(&vdpasim->iommu_lock);
> > +
> > +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
> >                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
> > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> > +                                &vdpasim->iommu_lock);
> > +       }
> > +
> > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> > +               vhost_iotlb_reset(&vdpasim->iommu[i]);
> >
> > -       spin_lock(&vdpasim->iommu_lock);
> > -       vhost_iotlb_reset(vdpasim->iommu);
> >         spin_unlock(&vdpasim->iommu_lock);
> >
> >         vdpasim->features = 0;
> > @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
> >         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
> >
> >         spin_lock(&vdpasim->iommu_lock);
> > -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
> > +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
> >                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
> >         spin_unlock(&vdpasim->iommu_lock);
> >
> > @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
> >                                 size_t size)
> >  {
> >         spin_lock(&vdpasim->iommu_lock);
> > -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
> > +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
> >                               (u64)dma_addr + size - 1);
> >         spin_unlock(&vdpasim->iommu_lock);
> >
> > @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
> >         else
> >                 ops = &vdpasim_config_ops;
> >
> > -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
> > -                                   1, dev_attr->name, false);
> > +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
> > +                                   dev_attr->ngroups, dev_attr->nas,
> > +                                   dev_attr->name, false);
> >         if (IS_ERR(vdpasim)) {
> >                 ret = PTR_ERR(vdpasim);
> >                 goto err_alloc;
> > @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
> >         if (!vdpasim->vqs)
> >                 goto err_iommu;
> >
> > -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
> > +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
> > +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
> >         if (!vdpasim->iommu)
> >                 goto err_iommu;
> >
> > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> > +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
> > +
> >         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
> >         if (!vdpasim->buffer)
> >                 goto err_iommu;
> >
> >         for (i = 0; i < dev_attr->nvqs; i++)
> > -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
> > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> >                                  &vdpasim->iommu_lock);
> >
> >         ret = iova_cache_get();
> > @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
> >
> >  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
> >  {
> > -       return 0;
> > +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
> > +       if (idx == 2)
> > +               return 1;
> > +       else
> > +               return 0;
>
> This code only works for the vDPA-net simulator, since
> vdpasim_get_vq_group() is also shared with other simulators (e.g.
> vdpa_sim_blk),

That's totally right.

> should we move this net-specific code into
> vdpa_sim_net.c, maybe adding a callback implemented by the different
> simulators?
>

At this moment, VDPASIM_BLK_VQ_NUM is fixed to 1, so maybe the right
thing to do for the -rc phase is to check if idx > vdpasim.attr.nvqs?
It's a more general fix.

For the general case, yes, a callback should be issued to the actual
simulator so it's not a surprise when VDPASIM_BLK_VQ_NUM increases,
either dynamically or by anyone testing it.

Thoughts?

Thanks!

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Eugenio Perez Martin]

On Wed, Jun 22, 2022 at 12:21 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Tue, Jun 21, 2022 at 5:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > Hi Gautam,
> >
> > On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
> > >
> > > This patch introduces the control virtqueue support for vDPA
> > > simulator. This is a requirement for supporting advanced features like
> > > multiqueue.
> > >
> > > A requirement for control virtqueue is to isolate its memory access
> > > from the rx/tx virtqueues. This is because when using vDPA device
> > > for VM, the control virqueue is not directly assigned to VM. Userspace
> > > (Qemu) will present a shadow control virtqueue to control for
> > > recording the device states.
> > >
> > > The isolation is done via the virtqueue groups and ASID support in
> > > vDPA through vhost-vdpa. The simulator is extended to have:
> > >
> > > 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
> > > 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
> > >    contains CVQ
> > > 3) two address spaces and the simulator simply implements the address
> > >    spaces by mapping it 1:1 to IOTLB.
> > >
> > > For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
> > > to group 1. So we have:
> > >
> > > 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
> > >    RX and TX can be assigned to guest directly.
> > > 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
> > >    is the buffers that allocated and managed by VMM only. So CVQ of
> > >    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
> > >    of vhost-vdpa.
> > >
> > > For the other use cases, since AS 0 is associated to all virtqueue
> > > groups by default. All virtqueues share the same mapping by default.
> > >
> > > To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
> > > implemented in the simulator for the driver to set mac address.
> > >
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
> > > ---
> > >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
> > >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
> > >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
> > >  3 files changed, 161 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > index 659e2e2e4b0c..51bd0bafce06 100644
> > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
> > >  {
> > >         int i;
> > >
> > > -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
> > > +       spin_lock(&vdpasim->iommu_lock);
> > > +
> > > +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
> > >                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> > > +                                &vdpasim->iommu_lock);
> > > +       }
> > > +
> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> > > +               vhost_iotlb_reset(&vdpasim->iommu[i]);
> > >
> > > -       spin_lock(&vdpasim->iommu_lock);
> > > -       vhost_iotlb_reset(vdpasim->iommu);
> > >         spin_unlock(&vdpasim->iommu_lock);
> > >
> > >         vdpasim->features = 0;
> > > @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
> > >         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
> > >
> > >         spin_lock(&vdpasim->iommu_lock);
> > > -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
> > > +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
> > >                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
> > >         spin_unlock(&vdpasim->iommu_lock);
> > >
> > > @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
> > >                                 size_t size)
> > >  {
> > >         spin_lock(&vdpasim->iommu_lock);
> > > -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
> > > +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
> > >                               (u64)dma_addr + size - 1);
> > >         spin_unlock(&vdpasim->iommu_lock);
> > >
> > > @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
> > >         else
> > >                 ops = &vdpasim_config_ops;
> > >
> > > -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
> > > -                                   1, dev_attr->name, false);
> > > +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
> > > +                                   dev_attr->ngroups, dev_attr->nas,
> > > +                                   dev_attr->name, false);
> > >         if (IS_ERR(vdpasim)) {
> > >                 ret = PTR_ERR(vdpasim);
> > >                 goto err_alloc;
> > > @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
> > >         if (!vdpasim->vqs)
> > >                 goto err_iommu;
> > >
> > > -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
> > > +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
> > > +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
> > >         if (!vdpasim->iommu)
> > >                 goto err_iommu;
> > >
> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
> > > +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
> > > +
> > >         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
> > >         if (!vdpasim->buffer)
> > >                 goto err_iommu;
> > >
> > >         for (i = 0; i < dev_attr->nvqs; i++)
> > > -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
> > >                                  &vdpasim->iommu_lock);
> > >
> > >         ret = iova_cache_get();
> > > @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
> > >
> > >  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
> > >  {
> > > -       return 0;
> > > +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
> > > +       if (idx == 2)
> > > +               return 1;
> > > +       else
> > > +               return 0;
> >
> > This code only works for the vDPA-net simulator, since
> > vdpasim_get_vq_group() is also shared with other simulators (e.g.
> > vdpa_sim_blk),
>
> That's totally right.
>
> > should we move this net-specific code into
> > vdpa_sim_net.c, maybe adding a callback implemented by the different
> > simulators?
> >
>
> At this moment, VDPASIM_BLK_VQ_NUM is fixed to 1, so maybe the right
> thing to do for the -rc phase is to check if idx > vdpasim.attr.nvqs?
> It's a more general fix.
>

Actually, that is already checked by vhost/vdpa.c.

Taking that into account, is it worth introducing the change for 5.19?
I'm totally ok with the change for 5.20.

Thanks!

> For the general case, yes, a callback should be issued to the actual
> simulator so it's not a surprise when VDPASIM_BLK_VQ_NUM increases,
> either dynamically or by anyone testing it.
>
> Thoughts?
>
> Thanks!

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Stefano Garzarella]

On Wed, Jun 22, 2022 at 05:04:44PM +0200, Eugenio Perez Martin wrote:
>On Wed, Jun 22, 2022 at 12:21 PM Eugenio Perez Martin
><eperezma@redhat.com> wrote:
>>
>> On Tue, Jun 21, 2022 at 5:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> >
>> > Hi Gautam,
>> >
>> > On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>> > >
>> > > This patch introduces the control virtqueue support for vDPA
>> > > simulator. This is a requirement for supporting advanced features like
>> > > multiqueue.
>> > >
>> > > A requirement for control virtqueue is to isolate its memory access
>> > > from the rx/tx virtqueues. This is because when using vDPA device
>> > > for VM, the control virqueue is not directly assigned to VM. Userspace
>> > > (Qemu) will present a shadow control virtqueue to control for
>> > > recording the device states.
>> > >
>> > > The isolation is done via the virtqueue groups and ASID support in
>> > > vDPA through vhost-vdpa. The simulator is extended to have:
>> > >
>> > > 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
>> > > 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
>> > >    contains CVQ
>> > > 3) two address spaces and the simulator simply implements the address
>> > >    spaces by mapping it 1:1 to IOTLB.
>> > >
>> > > For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
>> > > to group 1. So we have:
>> > >
>> > > 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
>> > >    RX and TX can be assigned to guest directly.
>> > > 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
>> > >    is the buffers that allocated and managed by VMM only. So CVQ of
>> > >    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
>> > >    of vhost-vdpa.
>> > >
>> > > For the other use cases, since AS 0 is associated to all virtqueue
>> > > groups by default. All virtqueues share the same mapping by default.
>> > >
>> > > To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
>> > > implemented in the simulator for the driver to set mac address.
>> > >
>> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
>> > > ---
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
>> > >  3 files changed, 161 insertions(+), 20 deletions(-)
>> > >
>> > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > index 659e2e2e4b0c..51bd0bafce06 100644
>> > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
>> > >  {
>> > >         int i;
>> > >
>> > > -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
>> > > +       spin_lock(&vdpasim->iommu_lock);
>> > > +
>> > > +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
>> > >                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
>> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>> > > +                                &vdpasim->iommu_lock);
>> > > +       }
>> > > +
>> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
>> > > +               vhost_iotlb_reset(&vdpasim->iommu[i]);
>> > >
>> > > -       spin_lock(&vdpasim->iommu_lock);
>> > > -       vhost_iotlb_reset(vdpasim->iommu);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > >         vdpasim->features = 0;
>> > > @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
>> > >         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
>> > >
>> > >         spin_lock(&vdpasim->iommu_lock);
>> > > -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
>> > > +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
>> > >                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > > @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
>> > >                                 size_t size)
>> > >  {
>> > >         spin_lock(&vdpasim->iommu_lock);
>> > > -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
>> > > +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
>> > >                               (u64)dma_addr + size - 1);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > > @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>> > >         else
>> > >                 ops = &vdpasim_config_ops;
>> > >
>> > > -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
>> > > -                                   1, dev_attr->name, false);
>> > > +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
>> > > +                                   dev_attr->ngroups, dev_attr->nas,
>> > > +                                   dev_attr->name, false);
>> > >         if (IS_ERR(vdpasim)) {
>> > >                 ret = PTR_ERR(vdpasim);
>> > >                 goto err_alloc;
>> > > @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>> > >         if (!vdpasim->vqs)
>> > >                 goto err_iommu;
>> > >
>> > > -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
>> > > +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
>> > > +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
>> > >         if (!vdpasim->iommu)
>> > >                 goto err_iommu;
>> > >
>> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
>> > > +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
>> > > +
>> > >         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
>> > >         if (!vdpasim->buffer)
>> > >                 goto err_iommu;
>> > >
>> > >         for (i = 0; i < dev_attr->nvqs; i++)
>> > > -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
>> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>> > >                                  &vdpasim->iommu_lock);
>> > >
>> > >         ret = iova_cache_get();
>> > > @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
>> > >
>> > >  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
>> > >  {
>> > > -       return 0;
>> > > +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
>> > > +       if (idx == 2)
>> > > +               return 1;
>> > > +       else
>> > > +               return 0;
>> >
>> > This code only works for the vDPA-net simulator, since
>> > vdpasim_get_vq_group() is also shared with other simulators (e.g.
>> > vdpa_sim_blk),
>>
>> That's totally right.
>>
>> > should we move this net-specific code into
>> > vdpa_sim_net.c, maybe adding a callback implemented by the different
>> > simulators?
>> >
>>
>> At this moment, VDPASIM_BLK_VQ_NUM is fixed to 1, so maybe the right
>> thing to do for the -rc phase is to check if idx > vdpasim.attr.nvqs?
>> It's a more general fix.
>>
>
>Actually, that is already checked by vhost/vdpa.c.
>
>Taking that into account, is it worth introducing the change for 5.19?
>I'm totally ok with the change for 5.20.
>
>Thanks!
>
>> For the general case, yes, a callback should be issued to the actual
>> simulator so it's not a surprise when VDPASIM_BLK_VQ_NUM increases,
>> either dynamically or by anyone testing it.

Exactly, since those parameters are not yet configurable at runtime 
(someday I hope they will be), I often recompile the module by changing 
them, so for me we should fix them in 5.19.

Obviously it's an advanced case, and I expect that if someone recompiles 
the module changing some hardwired thing, they can expect to have to 
change something else as well.

So, I'm also fine with leaving it that way for 5.19, but if you want I 
can fix it earlier.

Thanks,
Stefano

Re: [PATCH v2 19/19] vdpasim: control virtqueue support

[Stefano Garzarella]

On Wed, Jun 22, 2022 at 05:04:44PM +0200, Eugenio Perez Martin wrote:
>On Wed, Jun 22, 2022 at 12:21 PM Eugenio Perez Martin
><eperezma@redhat.com> wrote:
>>
>> On Tue, Jun 21, 2022 at 5:20 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> >
>> > Hi Gautam,
>> >
>> > On Wed, Mar 30, 2022 at 8:21 PM Gautam Dawar <gautam.dawar@xilinx.com> wrote:
>> > >
>> > > This patch introduces the control virtqueue support for vDPA
>> > > simulator. This is a requirement for supporting advanced features like
>> > > multiqueue.
>> > >
>> > > A requirement for control virtqueue is to isolate its memory access
>> > > from the rx/tx virtqueues. This is because when using vDPA device
>> > > for VM, the control virqueue is not directly assigned to VM. Userspace
>> > > (Qemu) will present a shadow control virtqueue to control for
>> > > recording the device states.
>> > >
>> > > The isolation is done via the virtqueue groups and ASID support in
>> > > vDPA through vhost-vdpa. The simulator is extended to have:
>> > >
>> > > 1) three virtqueues: RXVQ, TXVQ and CVQ (control virtqueue)
>> > > 2) two virtqueue groups: group 0 contains RXVQ and TXVQ; group 1
>> > >    contains CVQ
>> > > 3) two address spaces and the simulator simply implements the address
>> > >    spaces by mapping it 1:1 to IOTLB.
>> > >
>> > > For the VM use cases, userspace(Qemu) may set AS 0 to group 0 and AS 1
>> > > to group 1. So we have:
>> > >
>> > > 1) The IOTLB for virtqueue group 0 contains the mappings of guest, so
>> > >    RX and TX can be assigned to guest directly.
>> > > 2) The IOTLB for virtqueue group 1 contains the mappings of CVQ which
>> > >    is the buffers that allocated and managed by VMM only. So CVQ of
>> > >    vhost-vdpa is visible to VMM only. And Guest can not access the CVQ
>> > >    of vhost-vdpa.
>> > >
>> > > For the other use cases, since AS 0 is associated to all virtqueue
>> > > groups by default. All virtqueues share the same mapping by default.
>> > >
>> > > To demonstrate the function, VIRITO_NET_F_CTRL_MACADDR is
>> > > implemented in the simulator for the driver to set mac address.
>> > >
>> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > > Signed-off-by: Gautam Dawar <gdawar@xilinx.com>
>> > > ---
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 91 ++++++++++++++++++++++------
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim.h     |  2 +
>> > >  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 88 ++++++++++++++++++++++++++-
>> > >  3 files changed, 161 insertions(+), 20 deletions(-)
>> > >
>> > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > index 659e2e2e4b0c..51bd0bafce06 100644
>> > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> > > @@ -96,11 +96,17 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim)
>> > >  {
>> > >         int i;
>> > >
>> > > -       for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
>> > > +       spin_lock(&vdpasim->iommu_lock);
>> > > +
>> > > +       for (i = 0; i < vdpasim->dev_attr.nvqs; i++) {
>> > >                 vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
>> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>> > > +                                &vdpasim->iommu_lock);
>> > > +       }
>> > > +
>> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
>> > > +               vhost_iotlb_reset(&vdpasim->iommu[i]);
>> > >
>> > > -       spin_lock(&vdpasim->iommu_lock);
>> > > -       vhost_iotlb_reset(vdpasim->iommu);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > >         vdpasim->features = 0;
>> > > @@ -145,7 +151,7 @@ static dma_addr_t vdpasim_map_range(struct vdpasim *vdpasim, phys_addr_t paddr,
>> > >         dma_addr = iova_dma_addr(&vdpasim->iova, iova);
>> > >
>> > >         spin_lock(&vdpasim->iommu_lock);
>> > > -       ret = vhost_iotlb_add_range(vdpasim->iommu, (u64)dma_addr,
>> > > +       ret = vhost_iotlb_add_range(&vdpasim->iommu[0], (u64)dma_addr,
>> > >                                     (u64)dma_addr + size - 1, (u64)paddr, perm);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > > @@ -161,7 +167,7 @@ static void vdpasim_unmap_range(struct vdpasim *vdpasim, dma_addr_t dma_addr,
>> > >                                 size_t size)
>> > >  {
>> > >         spin_lock(&vdpasim->iommu_lock);
>> > > -       vhost_iotlb_del_range(vdpasim->iommu, (u64)dma_addr,
>> > > +       vhost_iotlb_del_range(&vdpasim->iommu[0], (u64)dma_addr,
>> > >                               (u64)dma_addr + size - 1);
>> > >         spin_unlock(&vdpasim->iommu_lock);
>> > >
>> > > @@ -250,8 +256,9 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>> > >         else
>> > >                 ops = &vdpasim_config_ops;
>> > >
>> > > -       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, 1,
>> > > -                                   1, dev_attr->name, false);
>> > > +       vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
>> > > +                                   dev_attr->ngroups, dev_attr->nas,
>> > > +                                   dev_attr->name, false);
>> > >         if (IS_ERR(vdpasim)) {
>> > >                 ret = PTR_ERR(vdpasim);
>> > >                 goto err_alloc;
>> > > @@ -278,16 +285,20 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>> > >         if (!vdpasim->vqs)
>> > >                 goto err_iommu;
>> > >
>> > > -       vdpasim->iommu = vhost_iotlb_alloc(max_iotlb_entries, 0);
>> > > +       vdpasim->iommu = kmalloc_array(vdpasim->dev_attr.nas,
>> > > +                                      sizeof(*vdpasim->iommu), GFP_KERNEL);
>> > >         if (!vdpasim->iommu)
>> > >                 goto err_iommu;
>> > >
>> > > +       for (i = 0; i < vdpasim->dev_attr.nas; i++)
>> > > +               vhost_iotlb_init(&vdpasim->iommu[i], 0, 0);
>> > > +
>> > >         vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
>> > >         if (!vdpasim->buffer)
>> > >                 goto err_iommu;
>> > >
>> > >         for (i = 0; i < dev_attr->nvqs; i++)
>> > > -               vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu,
>> > > +               vringh_set_iotlb(&vdpasim->vqs[i].vring, &vdpasim->iommu[0],
>> > >                                  &vdpasim->iommu_lock);
>> > >
>> > >         ret = iova_cache_get();
>> > > @@ -401,7 +412,11 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
>> > >
>> > >  static u32 vdpasim_get_vq_group(struct vdpa_device *vdpa, u16 idx)
>> > >  {
>> > > -       return 0;
>> > > +       /* RX and TX belongs to group 0, CVQ belongs to group 1 */
>> > > +       if (idx == 2)
>> > > +               return 1;
>> > > +       else
>> > > +               return 0;
>> >
>> > This code only works for the vDPA-net simulator, since
>> > vdpasim_get_vq_group() is also shared with other simulators (e.g.
>> > vdpa_sim_blk),
>>
>> That's totally right.
>>
>> > should we move this net-specific code into
>> > vdpa_sim_net.c, maybe adding a callback implemented by the different
>> > simulators?
>> >
>>
>> At this moment, VDPASIM_BLK_VQ_NUM is fixed to 1, so maybe the right
>> thing to do for the -rc phase is to check if idx > vdpasim.attr.nvqs?
>> It's a more general fix.
>>
>
>Actually, that is already checked by vhost/vdpa.c.
>
>Taking that into account, is it worth introducing the change for 5.19?
>I'm totally ok with the change for 5.20.
>
>Thanks!
>
>> For the general case, yes, a callback should be issued to the actual
>> simulator so it's not a surprise when VDPASIM_BLK_VQ_NUM increases,
>> either dynamically or by anyone testing it.

Exactly, since those parameters are not yet configurable at runtime 
(someday I hope they will be), I often recompile the module by changing 
them, so for me we should fix them in 5.19.

Obviously it's an advanced case, and I expect that if someone recompiles 
the module changing some hardwired thing, they can expect to have to 
change something else as well.

So, I'm also fine with leaving it that way for 5.19, but if you want I 
can fix it earlier.

Thanks,
Stefano

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