[v2,7/8] vhost: configure device when any queue is ready for BLK device
Checks
Commit Message
When boot from virtio blk device, seabois in QEMU only enables one queue.
To work in this scenario, vDPA BLK device back-end conf_dev when any
queue is ready.
Signed-off-by: Andy Pei <andy.pei@intel.com>
Signed-off-by: Huang Wei <wei_huang@intel.com>
---
lib/vhost/vhost_user.c | 56 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 44 insertions(+), 12 deletions(-)
Comments
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Thursday, September 8, 2022 1:54 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Huang, Wei <wei.huang@intel.com>; Cao, Gang <gang.cao@intel.com>;
> maxime.coquelin@redhat.com; Huang Wei <wei_huang@intel.com>
> Subject: [PATCH v2 7/8] vhost: configure device when any queue is ready
> for BLK device
>
> When boot from virtio blk device, seabois in QEMU only enables one queue.
> To work in this scenario, vDPA BLK device back-end conf_dev when any
> queue is ready.
So this is the case only for vdpa blk, for SW vhost-blk, all queues need to
be ready?
>
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> Signed-off-by: Huang Wei <wei_huang@intel.com>
> ---
> lib/vhost/vhost_user.c | 56 +++++++++++++++++++++++++++++++++++++++------
> -----
> 1 file changed, 44 insertions(+), 12 deletions(-)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 4ad28ba..b65fba3 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -1451,6 +1451,25 @@
> #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
>
> static int
> +virtio_has_queue_ready(struct virtio_net *dev)
> +{
> + struct vhost_virtqueue *vq;
> + uint32_t i, nr_vring = dev->nr_vring;
> +
> + if (!dev->nr_vring)
> + return 0;
> +
> + for (i = 0; i < nr_vring; i++) {
> + vq = dev->virtqueue[i];
> +
> + if (vq_is_ready(dev, vq))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> virtio_is_ready(struct virtio_net *dev)
> {
> struct vhost_virtqueue *vq;
> @@ -3167,9 +3186,33 @@ static int is_vring_iotlb(struct virtio_net *dev,
> if (unlock_required)
> vhost_user_unlock_all_queue_pairs(dev);
>
> - if (ret != 0 || !virtio_is_ready(dev))
> + if (ret != 0)
> goto out;
>
> + vdpa_dev = dev->vdpa_dev;
> + if (vdpa_dev) {
> + if (vdpa_dev->ops->get_dev_type) {
> + ret = vdpa_dev->ops->get_dev_type(vdpa_dev, &vdpa_type);
> + if (ret) {
> + VHOST_LOG_CONFIG(dev->ifname, ERR,
> + "failed to get vdpa dev type.\n");
> + ret = -1;
> + goto out;
> + }
> + } else {
> + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> + }
> + }
> +
> + if (!virtio_is_ready(dev)) {
> + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> + if (!virtio_has_queue_ready(dev))
> + goto out;
> + } else {
> + goto out;
> + }
> + }
> +
I feel like if possible, above logic should all be in virtio_is_ready.
Thanks,
Chenbo
> /*
> * Virtio is now ready. If not done already, it is time
> * to notify the application it can process the rings and
> @@ -3181,20 +3224,9 @@ static int is_vring_iotlb(struct virtio_net *dev,
> dev->flags |= VIRTIO_DEV_RUNNING;
> }
>
> - vdpa_dev = dev->vdpa_dev;
> if (!vdpa_dev)
> goto out;
>
> - if (vdpa_dev->ops->get_dev_type) {
> - ret = vdpa_dev->ops->get_dev_type(vdpa_dev, &vdpa_type);
> - if (ret) {
> - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to get vdpa
> dev type.\n");
> - ret = -1;
> - goto out;
> - }
> - } else {
> - vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> - }
> if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
> && request != VHOST_USER_SET_VRING_CALL)
> goto out;
> --
> 1.8.3.1
HI Chenbo,
Thanks for your reply, my reply is inline.
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, September 14, 2022 10:51 AM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao,
> Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com
> Subject: RE: [PATCH v2 7/8] vhost: configure device when any queue is ready for
> BLK device
>
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Thursday, September 8, 2022 1:54 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao, Gang
> > <gang.cao@intel.com>; maxime.coquelin@redhat.com; Huang Wei
> > <wei_huang@intel.com>
> > Subject: [PATCH v2 7/8] vhost: configure device when any queue is
> > ready for BLK device
> >
> > When boot from virtio blk device, seabois in QEMU only enables one queue.
> > To work in this scenario, vDPA BLK device back-end conf_dev when any
> > queue is ready.
>
> So this is the case only for vdpa blk, for SW vhost-blk, all queues need to be
> ready?
>
The case I mention is that the OS image is in the vdpa device.
In this case, QEMU bios (seabios) will take charge. There is a virtio driver in seabios.
This driver only use one queue.
So in this case vdpa driver need to conf_dev when one queue is ready.
for SW vhost-blk, I am not sure you mean OS image is in your back-end vhost-blk.
I have not test that case.
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > Signed-off-by: Huang Wei <wei_huang@intel.com>
> > ---
> > lib/vhost/vhost_user.c | 56
> > +++++++++++++++++++++++++++++++++++++++------
> > -----
> > 1 file changed, 44 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > 4ad28ba..b65fba3 100644
> > --- a/lib/vhost/vhost_user.c
> > +++ b/lib/vhost/vhost_user.c
> > @@ -1451,6 +1451,25 @@
> > #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
> >
> > static int
> > +virtio_has_queue_ready(struct virtio_net *dev) {
> > + struct vhost_virtqueue *vq;
> > + uint32_t i, nr_vring = dev->nr_vring;
> > +
> > + if (!dev->nr_vring)
> > + return 0;
> > +
> > + for (i = 0; i < nr_vring; i++) {
> > + vq = dev->virtqueue[i];
> > +
> > + if (vq_is_ready(dev, vq))
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > virtio_is_ready(struct virtio_net *dev) {
> > struct vhost_virtqueue *vq;
> > @@ -3167,9 +3186,33 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > if (unlock_required)
> > vhost_user_unlock_all_queue_pairs(dev);
> >
> > - if (ret != 0 || !virtio_is_ready(dev))
> > + if (ret != 0)
> > goto out;
> >
> > + vdpa_dev = dev->vdpa_dev;
> > + if (vdpa_dev) {
> > + if (vdpa_dev->ops->get_dev_type) {
> > + ret = vdpa_dev->ops->get_dev_type(vdpa_dev,
> &vdpa_type);
> > + if (ret) {
> > + VHOST_LOG_CONFIG(dev->ifname, ERR,
> > + "failed to get vdpa dev type.\n");
> > + ret = -1;
> > + goto out;
> > + }
> > + } else {
> > + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> > + }
> > + }
> > +
> > + if (!virtio_is_ready(dev)) {
> > + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> > + if (!virtio_has_queue_ready(dev))
> > + goto out;
> > + } else {
> > + goto out;
> > + }
> > + }
> > +
>
> I feel like if possible, above logic should all be in virtio_is_ready.
>
Maybe I can introduce something like VIRTIO_DEV_BUILTIN_VIRTIO_NET,
What do you think?
> Thanks,
> Chenbo
>
> > /*
> > * Virtio is now ready. If not done already, it is time
> > * to notify the application it can process the rings and @@
> > -3181,20 +3224,9 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > dev->flags |= VIRTIO_DEV_RUNNING;
> > }
> >
> > - vdpa_dev = dev->vdpa_dev;
> > if (!vdpa_dev)
> > goto out;
> >
> > - if (vdpa_dev->ops->get_dev_type) {
> > - ret = vdpa_dev->ops->get_dev_type(vdpa_dev, &vdpa_type);
> > - if (ret) {
> > - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to get
> vdpa
> > dev type.\n");
> > - ret = -1;
> > - goto out;
> > - }
> > - } else {
> > - vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> > - }
> > if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
> > && request != VHOST_USER_SET_VRING_CALL)
> > goto out;
> > --
> > 1.8.3.1
@@ -1451,6 +1451,25 @@
#define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
static int
+virtio_has_queue_ready(struct virtio_net *dev)
+{
+ struct vhost_virtqueue *vq;
+ uint32_t i, nr_vring = dev->nr_vring;
+
+ if (!dev->nr_vring)
+ return 0;
+
+ for (i = 0; i < nr_vring; i++) {
+ vq = dev->virtqueue[i];
+
+ if (vq_is_ready(dev, vq))
+ return 1;
+ }
+
+ return 0;
+}
+
+static int
virtio_is_ready(struct virtio_net *dev)
{
struct vhost_virtqueue *vq;
@@ -3167,9 +3186,33 @@ static int is_vring_iotlb(struct virtio_net *dev,
if (unlock_required)
vhost_user_unlock_all_queue_pairs(dev);
- if (ret != 0 || !virtio_is_ready(dev))
+ if (ret != 0)
goto out;
+ vdpa_dev = dev->vdpa_dev;
+ if (vdpa_dev) {
+ if (vdpa_dev->ops->get_dev_type) {
+ ret = vdpa_dev->ops->get_dev_type(vdpa_dev, &vdpa_type);
+ if (ret) {
+ VHOST_LOG_CONFIG(dev->ifname, ERR,
+ "failed to get vdpa dev type.\n");
+ ret = -1;
+ goto out;
+ }
+ } else {
+ vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
+ }
+ }
+
+ if (!virtio_is_ready(dev)) {
+ if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
+ if (!virtio_has_queue_ready(dev))
+ goto out;
+ } else {
+ goto out;
+ }
+ }
+
/*
* Virtio is now ready. If not done already, it is time
* to notify the application it can process the rings and
@@ -3181,20 +3224,9 @@ static int is_vring_iotlb(struct virtio_net *dev,
dev->flags |= VIRTIO_DEV_RUNNING;
}
- vdpa_dev = dev->vdpa_dev;
if (!vdpa_dev)
goto out;
- if (vdpa_dev->ops->get_dev_type) {
- ret = vdpa_dev->ops->get_dev_type(vdpa_dev, &vdpa_type);
- if (ret) {
- VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to get vdpa dev type.\n");
- ret = -1;
- goto out;
- }
- } else {
- vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
- }
if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK
&& request != VHOST_USER_SET_VRING_CALL)
goto out;