From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1486B81F12; Fri, 11 Jan 2019 09:13:08 +0000 (UTC) Received: from [10.36.112.52] (ovpn-112-52.ams2.redhat.com [10.36.112.52]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5F0CB17D0D; Fri, 11 Jan 2019 09:13:01 +0000 (UTC) To: Jens Freimann , dev@dpdk.org Cc: tiwei.bie@intel.com References: <20190110214727.1142-1-jfreimann@redhat.com> <20190110214727.1142-3-jfreimann@redhat.com> From: Maxime Coquelin Message-ID: <8455a67c-2bdb-7458-f247-3f4946cbe9e8@redhat.com> Date: Fri, 11 Jan 2019 10:12:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20190110214727.1142-3-jfreimann@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 11 Jan 2019 09:13:08 +0000 (UTC) Subject: Re: [dpdk-dev] [Patch v3 2/2] net/virtio-user: ctrl vq support for packed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jan 2019 09:13:09 -0000 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