diff mbox series

[RFC] virtio-user: ctrl vq support for packed

Message ID 20181221152948.20714-1-jfreimann@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers show
Series [RFC] virtio-user: ctrl vq support for packed | expand

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Jens Freimann Dec. 21, 2018, 3:29 p.m. UTC
This adds support to virtio-user for control virtqueues
and reverts commit "5e4e7a752 net/virtio-user: fail if
cq used with packed vq".

I add a struct virtio_user_queue to have a place for wrap
counters and avail/used index (which is not needed for split
ring because it has those in shared memory).

This is a RFC because it only supports in-order use of descriptors
in the ring. I'm looking for ideas how to change this without the
.next field in the descriptor as we have it with split virtqueues.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c            |  11 +-
 .../net/virtio/virtio_user/virtio_user_dev.c  | 101 ++++++++++++++++--
 .../net/virtio/virtio_user/virtio_user_dev.h  |  14 ++-
 drivers/net/virtio/virtio_user_ethdev.c       |  25 ++++-
 4 files changed, 135 insertions(+), 16 deletions(-)

Comments

Maxime Coquelin Jan. 7, 2019, 10:30 a.m. UTC | #1
Hi Jens,

On 12/21/18 4:29 PM, Jens Freimann wrote:
> This adds support to virtio-user for control virtqueues
> and reverts commit "5e4e7a752 net/virtio-user: fail if
> cq used with packed vq".
> 
> I add a struct virtio_user_queue to have a place for wrap
> counters and avail/used index (which is not needed for split
> ring because it has those in shared memory).
> 
> This is a RFC because it only supports in-order use of descriptors
> in the ring. I'm looking for ideas how to change this without the
> .next field in the descriptor as we have it with split virtqueues.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
>   drivers/net/virtio/virtio_ethdev.c            |  11 +-
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 101 ++++++++++++++++--
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  14 ++-
>   drivers/net/virtio/virtio_user_ethdev.c       |  25 ++++-
>   4 files changed, 135 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 446c338fc..010ab6489 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -153,6 +153,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   	uint16_t flags;
>   	int sum = 0;
>   	int k;
> +	int ndescs = 0;
>   
>   	/*
>   	 * Format is enforced in qemu code:
> @@ -162,7 +163,6 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   	 */
>   	head = vq->vq_avail_idx;
>   	wrap_counter = vq->avail_wrap_counter;
> -	desc[head].flags = VRING_DESC_F_NEXT;
>   	desc[head].addr = cvq->virtio_net_hdr_mem;
>   	desc[head].len = sizeof(struct virtio_net_ctrl_hdr);
>   	vq->vq_free_cnt--;
> @@ -170,6 +170,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   		vq->vq_avail_idx -= vq->vq_nentries;
>   		vq->avail_wrap_counter ^= 1;
>   	}
> +	ndescs++;
>   
>   	for (k = 0; k < pkt_num; k++) {
>   		desc[vq->vq_avail_idx].addr = cvq->virtio_net_hdr_mem
> @@ -188,6 +189,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   			vq->vq_avail_idx -= vq->vq_nentries;
>   			vq->avail_wrap_counter ^= 1;
>   		}
> +		ndescs++;
>   	}
>   
>   
> @@ -198,6 +200,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   	flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
>   		 VRING_DESC_F_USED(!vq->avail_wrap_counter);
>   	desc[vq->vq_avail_idx].flags = flags;
> +
>   	flags = VRING_DESC_F_NEXT;
>   	flags |= VRING_DESC_F_AVAIL(wrap_counter) |
>   		 VRING_DESC_F_USED(!wrap_counter);
> @@ -209,6 +212,7 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   		vq->vq_avail_idx -= vq->vq_nentries;
>   		vq->avail_wrap_counter ^= 1;
>   	}
> +	ndescs++;
>   
>   	virtqueue_notify(vq);
>   
> @@ -220,8 +224,9 @@ virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
>   
>   	/* now get used descriptors */
>   	while (desc_is_used(&desc[vq->vq_used_cons_idx], vq)) {
> -		vq->vq_free_cnt++;
> -		if (++vq->vq_used_cons_idx >= vq->vq_nentries) {
> +		vq->vq_free_cnt += ndescs;
> +		vq->vq_used_cons_idx += ndescs;
> +		if (vq->vq_used_cons_idx >= vq->vq_nentries) {
>   			vq->vq_used_cons_idx -= vq->vq_nentries;
>   			vq->used_wrap_counter ^= 1;
>   		}

I'm not sure why this change is needed.
Why not getting used descriptor one by one as it was before?
If it does not work, doesn't that mean that the desc chain is not
marked as used properly by the device?

Thanks,
Maxime
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 446c338fc..010ab6489 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -153,6 +153,7 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 	uint16_t flags;
 	int sum = 0;
 	int k;
+	int ndescs = 0;
 
 	/*
 	 * Format is enforced in qemu code:
@@ -162,7 +163,6 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 	 */
 	head = vq->vq_avail_idx;
 	wrap_counter = vq->avail_wrap_counter;
-	desc[head].flags = VRING_DESC_F_NEXT;
 	desc[head].addr = cvq->virtio_net_hdr_mem;
 	desc[head].len = sizeof(struct virtio_net_ctrl_hdr);
 	vq->vq_free_cnt--;
@@ -170,6 +170,7 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 		vq->vq_avail_idx -= vq->vq_nentries;
 		vq->avail_wrap_counter ^= 1;
 	}
+	ndescs++;
 
 	for (k = 0; k < pkt_num; k++) {
 		desc[vq->vq_avail_idx].addr = cvq->virtio_net_hdr_mem
@@ -188,6 +189,7 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 			vq->vq_avail_idx -= vq->vq_nentries;
 			vq->avail_wrap_counter ^= 1;
 		}
+		ndescs++;
 	}
 
 
@@ -198,6 +200,7 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 	flags |= VRING_DESC_F_AVAIL(vq->avail_wrap_counter) |
 		 VRING_DESC_F_USED(!vq->avail_wrap_counter);
 	desc[vq->vq_avail_idx].flags = flags;
+
 	flags = VRING_DESC_F_NEXT;
 	flags |= VRING_DESC_F_AVAIL(wrap_counter) |
 		 VRING_DESC_F_USED(!wrap_counter);
@@ -209,6 +212,7 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 		vq->vq_avail_idx -= vq->vq_nentries;
 		vq->avail_wrap_counter ^= 1;
 	}
+	ndescs++;
 
 	virtqueue_notify(vq);
 
@@ -220,8 +224,9 @@  virtio_pq_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl,
 
 	/* now get used descriptors */
 	while (desc_is_used(&desc[vq->vq_used_cons_idx], vq)) {
-		vq->vq_free_cnt++;
-		if (++vq->vq_used_cons_idx >= vq->vq_nentries) {
+		vq->vq_free_cnt += ndescs;
+		vq->vq_used_cons_idx += ndescs;
+		if (vq->vq_used_cons_idx >= vq->vq_nentries) {
 			vq->vq_used_cons_idx -= vq->vq_nentries;
 			vq->used_wrap_counter ^= 1;
 		}
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 5560bd9a3..b324d4391 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -43,15 +43,26 @@  virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 	struct vhost_vring_file file;
 	struct vhost_vring_state state;
 	struct vring *vring = &dev->vrings[queue_sel];
+	struct vring_packed *pq_vring = &dev->packed_vrings[queue_sel];
 	struct vhost_vring_addr addr = {
 		.index = queue_sel,
-		.desc_user_addr = (uint64_t)(uintptr_t)vring->desc,
-		.avail_user_addr = (uint64_t)(uintptr_t)vring->avail,
-		.used_user_addr = (uint64_t)(uintptr_t)vring->used,
 		.log_guest_addr = 0,
 		.flags = 0, /* disable log */
 	};
 
+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
+		addr.desc_user_addr =
+			(uint64_t)(uintptr_t)pq_vring->desc_packed;
+		addr.avail_user_addr =
+			(uint64_t)(uintptr_t)pq_vring->driver_event;
+		addr.used_user_addr =
+			(uint64_t)(uintptr_t)pq_vring->device_event;
+	} else {
+		addr.desc_user_addr = (uint64_t)(uintptr_t)vring->desc;
+		addr.avail_user_addr = (uint64_t)(uintptr_t)vring->avail;
+		addr.used_user_addr = (uint64_t)(uintptr_t)vring->used;
+	}
+
 	state.index = queue_sel;
 	state.num = vring->num;
 	dev->ops->send_request(dev, VHOST_USER_SET_VRING_NUM, &state);
@@ -468,16 +479,10 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	if (!in_order)
 		dev->unsupported_features |= (1ull << VIRTIO_F_IN_ORDER);
 
-	if (packed_vq) {
-		if (cq) {
-			PMD_INIT_LOG(ERR, "control vq not supported yet with "
-					  "packed virtqueues\n");
-			return -1;
-		}
+	if (packed_vq)
 		dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
-	} else {
+	else
 		dev->device_features &= ~(1ull << VIRTIO_F_RING_PACKED);
-	}
 
 	if (dev->mac_specified) {
 		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
@@ -624,6 +629,80 @@  virtio_user_handle_ctrl_msg(struct virtio_user_dev *dev, struct vring *vring,
 	return n_descs;
 }
 
+static inline int
+desc_is_avail(struct vring_packed_desc *desc, bool wrap_counter)
+{
+	return wrap_counter == !!(desc->flags & VRING_DESC_F_AVAIL(1)) &&
+		wrap_counter != !!(desc->flags & VRING_DESC_F_USED(1));
+}
+
+static uint32_t
+virtio_user_handle_ctrl_msg_pq(struct virtio_user_dev *dev,
+			    struct vring_packed *vring,
+			    uint16_t idx_hdr)
+{
+	struct virtio_net_ctrl_hdr *hdr;
+	virtio_net_ctrl_ack status = ~0;
+	uint16_t i, idx_data, idx_status;
+	uint32_t n_descs = 0;
+
+	/* locate desc for header, data, and status */
+	idx_data = idx_hdr + 1;
+	n_descs++;
+
+	i = idx_data;
+	while (vring->desc_packed[i].flags & VRING_DESC_F_NEXT) {
+		i++;
+		n_descs++;
+	}
+
+	/* locate desc for status */
+	idx_status = i;
+	n_descs++;
+
+	hdr = (void *)(uintptr_t)vring->desc_packed[idx_hdr].addr;
+	if (hdr->class == VIRTIO_NET_CTRL_MQ &&
+	    hdr->cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+		uint16_t queues;
+
+		queues = *(uint16_t *)(uintptr_t)vring->desc_packed[idx_data].addr;
+		status = virtio_user_handle_mq(dev, queues);
+	}
+
+	/* Update status */
+	*(virtio_net_ctrl_ack *)(uintptr_t)vring->desc_packed[idx_status].addr = status;
+
+	return n_descs;
+}
+
+void
+virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx)
+{
+	struct virtio_user_queue *vq = &dev->queues[queue_idx];
+	struct vring_packed *vring = &dev->packed_vrings[queue_idx];
+	uint16_t id, n_descs;
+	bool wrap_counter = vq->used_wrap_counter;
+
+	while (desc_is_avail(&vring->desc_packed[vq->used_idx],
+			     vq->avail_wrap_counter)) {
+		id = vring->desc_packed[vq->used_idx].id;
+
+		n_descs = virtio_user_handle_ctrl_msg_pq(dev, vring, id);
+
+		vring->desc_packed[vq->used_idx].len = n_descs;
+		vring->desc_packed[vq->used_idx].id = vq->used_idx;
+		vq->used_idx += n_descs;
+		if (vq->used_idx >= dev->queue_size) {
+			vq->used_idx -= dev->queue_size;
+			vq->used_wrap_counter ^= 1;
+		}
+	}
+	desc->flags |= VRING_DESC_F_AVAIL(wrap_counter) |
+		       VRING_DESC_F_USED(wrap_counter);
+
+	vq->avail_idx = vq->used_idx;
+}
+
 void
 virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx)
 {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 672a8161a..26a3c6148 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -11,6 +11,12 @@ 
 #include "../virtio_ring.h"
 #include "vhost.h"
 
+struct virtio_user_queue {
+	uint16_t avail_idx, used_idx;
+	uint16_t used_wrap_counter;
+	uint16_t avail_wrap_counter;
+};
+
 struct virtio_user_dev {
 	/* for vhost_user backend */
 	int		vhostfd;
@@ -39,7 +45,12 @@  struct virtio_user_dev {
 	uint16_t	port_id;
 	uint8_t		mac_addr[ETHER_ADDR_LEN];
 	char		path[PATH_MAX];
-	struct vring	vrings[VIRTIO_MAX_VIRTQUEUES];
+	union {
+		struct vring		vrings[VIRTIO_MAX_VIRTQUEUES];
+		struct vring_packed	packed_vrings[VIRTIO_MAX_VIRTQUEUES];
+	};
+	struct virtio_user_queue queues[VIRTIO_MAX_VIRTQUEUES];
+
 	struct virtio_user_backend_ops *ops;
 	pthread_mutex_t	mutex;
 	bool		started;
@@ -53,5 +64,6 @@  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int mrg_rxbuf, int in_order, int packed_vq);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
+void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx);
 uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
 #endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index af2800605..854a01d98 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -277,6 +277,26 @@  virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 	uint16_t queue_idx = vq->vq_queue_index;
 	uint64_t desc_addr, avail_addr, used_addr;
+	struct virtio_user_queue *vq_user = &dev->queues[queue_idx];
+
+	if (vtpci_packed_queue(hw)) {
+		desc_addr = (uintptr_t)vq->vq_ring_virt_mem;
+		avail_addr = desc_addr + vq->vq_nentries *
+			sizeof(struct vring_packed_desc);
+		used_addr = RTE_ALIGN_CEIL(avail_addr +
+				   sizeof(struct vring_packed_desc_event),
+				   VIRTIO_PCI_VRING_ALIGN);
+		dev->packed_vrings[queue_idx].num = vq->vq_nentries;
+		dev->packed_vrings[queue_idx].desc_packed =
+			(void *)(uintptr_t)desc_addr;
+		dev->packed_vrings[queue_idx].driver_event =
+			(void *)(uintptr_t)avail_addr;
+		dev->packed_vrings[queue_idx].device_event =
+			(void*)(uintptr_t)used_addr;
+		vq_user->avail_wrap_counter = true;
+		vq_user->used_wrap_counter = true;
+		return 0;
+	}
 
 	desc_addr = (uintptr_t)vq->vq_ring_virt_mem;
 	avail_addr = desc_addr + vq->vq_nentries * sizeof(struct vring_desc);
@@ -317,7 +337,10 @@  virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	if (hw->cvq && (hw->cvq->vq == vq)) {
-		virtio_user_handle_cq(dev, vq->vq_queue_index);
+		if (vtpci_packed_queue(vq->hw))
+			virtio_user_handle_cq_packed(dev, vq->vq_queue_index);
+		else
+			virtio_user_handle_cq(dev, vq->vq_queue_index);
 		return;
 	}