From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Coquelin Subject: Re: [Patch v3 2/2] net/virtio-user: ctrl vq support for packed Date: Fri, 11 Jan 2019 10:12:59 +0100 Message-ID: <8455a67c-2bdb-7458-f247-3f4946cbe9e8@redhat.com> References: <20190110214727.1142-1-jfreimann@redhat.com> <20190110214727.1142-3-jfreimann@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: tiwei.bie@intel.com To: Jens Freimann , dev@dpdk.org Return-path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id BA34D1B938 for ; Fri, 11 Jan 2019 10:13:08 +0100 (CET) In-Reply-To: <20190110214727.1142-3-jfreimann@redhat.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 1/10/19 10:47 PM, Jens Freimann wrote: > Add support to virtio-user for control virtqueues. > > Signed-off-by: Jens Freimann > --- > .../net/virtio/virtio_user/virtio_user_dev.c | 104 ++++++++++++++++-- > .../net/virtio/virtio_user/virtio_user_dev.h | 15 ++- > drivers/net/virtio/virtio_user_ethdev.c | 56 +++++++++- > 3 files changed, 159 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c > index b9044faff..e7d1cf225 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); > @@ -467,15 +478,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; > - } > - } else { > + if (packed_vq) > + dev->device_features |= (1ull << VIRTIO_F_RING_PACKED); > + else > dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED); > - } I think you missed Tiwei comment on v3, i.e. this should be enough: if (!packed_vq) dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED); Other than that, the patch looks good to me. With above fixed, feel free to add my: Reviewed-by: Maxime Coquelin Thanks! Maxime