[v5,7/8] vhost: vDPA blk device gets ready when the first queue is ready
Checks
Commit Message
When boot from virtio blk device, seabios in QEMU only enables one queue.
To work in this scenario, vDPA BLK device back-end configure device
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 | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
Comments
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Monday, October 17, 2022 3:14 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
> Subject: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the first
> queue is ready
>
> When boot from virtio blk device, seabios in QEMU only enables one queue.
> To work in this scenario, vDPA BLK device back-end configure device
> 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 | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index cd65257..f5206dd 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -1441,11 +1441,15 @@
> }
>
> #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
> +#define VIRTIO_BLK_NUM_VQS_TO_BE_READY 1u
>
> static int
> virtio_is_ready(struct virtio_net *dev)
> {
> + struct rte_vdpa_device *vdpa_dev;
> struct vhost_virtqueue *vq;
> + uint32_t vdpa_type;
> + int ret = 0;
> uint32_t i, nr_vring = dev->nr_vring;
>
> if (dev->flags & VIRTIO_DEV_READY)
> @@ -1454,13 +1458,32 @@
> if (!dev->nr_vring)
> return 0;
>
> - if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) {
> - nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
> + 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");
> + return -1;
> + }
> + } else {
> + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> + }
> + } else {
> + vdpa_type = -1;
> + }
Looking at it again, instead of getting the device type again and again,
Should we just get and set the type into vdpa_dev when registering a vdpa device?
What do you think?
Thanks,
Chenbo
>
> - if (dev->nr_vring < nr_vring)
> - return 0;
> + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> + nr_vring = VIRTIO_BLK_NUM_VQS_TO_BE_READY;
> + } else {
> + if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET)
> + nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
> }
>
> + if (dev->nr_vring < nr_vring)
> + return 0;
> +
> for (i = 0; i < nr_vring; i++) {
> vq = dev->virtqueue[i];
>
> --
> 1.8.3.1
On 10/17/22 09:54, Xia, Chenbo wrote:
>> -----Original Message-----
>> From: Pei, Andy <andy.pei@intel.com>
>> Sent: Monday, October 17, 2022 3:14 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
>> Subject: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the first
>> queue is ready
>>
>> When boot from virtio blk device, seabios in QEMU only enables one queue.
>> To work in this scenario, vDPA BLK device back-end configure device
>> 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 | 31 +++++++++++++++++++++++++++----
>> 1 file changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>> index cd65257..f5206dd 100644
>> --- a/lib/vhost/vhost_user.c
>> +++ b/lib/vhost/vhost_user.c
>> @@ -1441,11 +1441,15 @@
>> }
>>
>> #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
>> +#define VIRTIO_BLK_NUM_VQS_TO_BE_READY 1u
>>
>> static int
>> virtio_is_ready(struct virtio_net *dev)
>> {
>> + struct rte_vdpa_device *vdpa_dev;
>> struct vhost_virtqueue *vq;
>> + uint32_t vdpa_type;
>> + int ret = 0;
>> uint32_t i, nr_vring = dev->nr_vring;
>>
>> if (dev->flags & VIRTIO_DEV_READY)
>> @@ -1454,13 +1458,32 @@
>> if (!dev->nr_vring)
>> return 0;
>>
>> - if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) {
>> - nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
>> + 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");
>> + return -1;
>> + }
>> + } else {
>> + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
>> + }
>> + } else {
>> + vdpa_type = -1;
>> + }
>
> Looking at it again, instead of getting the device type again and again,
> Should we just get and set the type into vdpa_dev when registering a vdpa device?
>
> What do you think?
I think this would be preferable.
If it is to be in struct virtio_net, please take care to place it
properly not to create new holes. Also, it does not have to be placed in
hot cachelines.
Thanks,
Maxime
> Thanks,
> Chenbo
>
>>
>> - if (dev->nr_vring < nr_vring)
>> - return 0;
>> + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
>> + nr_vring = VIRTIO_BLK_NUM_VQS_TO_BE_READY;
>> + } else {
>> + if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET)
>> + nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
>> }
>>
>> + if (dev->nr_vring < nr_vring)
>> + return 0;
>> +
>> for (i = 0; i < nr_vring; i++) {
>> vq = dev->virtqueue[i];
>>
>> --
>> 1.8.3.1
>
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, October 17, 2022 4:37 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; 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>
> Subject: Re: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the
> first queue is ready
>
>
>
> On 10/17/22 09:54, Xia, Chenbo wrote:
> >> -----Original Message-----
> >> From: Pei, Andy <andy.pei@intel.com>
> >> Sent: Monday, October 17, 2022 3:14 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
> >> Subject: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the
> first
> >> queue is ready
> >>
> >> When boot from virtio blk device, seabios in QEMU only enables one
> queue.
> >> To work in this scenario, vDPA BLK device back-end configure device
> >> 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 | 31 +++++++++++++++++++++++++++----
> >> 1 file changed, 27 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> >> index cd65257..f5206dd 100644
> >> --- a/lib/vhost/vhost_user.c
> >> +++ b/lib/vhost/vhost_user.c
> >> @@ -1441,11 +1441,15 @@
> >> }
> >>
> >> #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
> >> +#define VIRTIO_BLK_NUM_VQS_TO_BE_READY 1u
> >>
> >> static int
> >> virtio_is_ready(struct virtio_net *dev)
> >> {
> >> + struct rte_vdpa_device *vdpa_dev;
> >> struct vhost_virtqueue *vq;
> >> + uint32_t vdpa_type;
> >> + int ret = 0;
> >> uint32_t i, nr_vring = dev->nr_vring;
> >>
> >> if (dev->flags & VIRTIO_DEV_READY)
> >> @@ -1454,13 +1458,32 @@
> >> if (!dev->nr_vring)
> >> return 0;
> >>
> >> - if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) {
> >> - nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
> >> + 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");
> >> + return -1;
> >> + }
> >> + } else {
> >> + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
> >> + }
> >> + } else {
> >> + vdpa_type = -1;
> >> + }
> >
> > Looking at it again, instead of getting the device type again and again,
> > Should we just get and set the type into vdpa_dev when registering a
> vdpa device?
> >
> > What do you think?
>
> I think this would be preferable.
>
> If it is to be in struct virtio_net, please take care to place it
> properly not to create new holes. Also, it does not have to be placed in
> hot cachelines.
I am thinking struct rte_vdpa_device. Because if no vdpa device, there will
be no vdpa device type. What do you think?
Thanks,
Chenbo
>
> Thanks,
> Maxime
> > Thanks,
> > Chenbo
> >
> >>
> >> - if (dev->nr_vring < nr_vring)
> >> - return 0;
> >> + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
> >> + nr_vring = VIRTIO_BLK_NUM_VQS_TO_BE_READY;
> >> + } else {
> >> + if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET)
> >> + nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
> >> }
> >>
> >> + if (dev->nr_vring < nr_vring)
> >> + return 0;
> >> +
> >> for (i = 0; i < nr_vring; i++) {
> >> vq = dev->virtqueue[i];
> >>
> >> --
> >> 1.8.3.1
> >
On 10/17/22 10:42, Xia, Chenbo wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Monday, October 17, 2022 4:37 PM
>> To: Xia, Chenbo <chenbo.xia@intel.com>; 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>
>> Subject: Re: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the
>> first queue is ready
>>
>>
>>
>> On 10/17/22 09:54, Xia, Chenbo wrote:
>>>> -----Original Message-----
>>>> From: Pei, Andy <andy.pei@intel.com>
>>>> Sent: Monday, October 17, 2022 3:14 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
>>>> Subject: [PATCH v5 7/8] vhost: vDPA blk device gets ready when the
>> first
>>>> queue is ready
>>>>
>>>> When boot from virtio blk device, seabios in QEMU only enables one
>> queue.
>>>> To work in this scenario, vDPA BLK device back-end configure device
>>>> 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 | 31 +++++++++++++++++++++++++++----
>>>> 1 file changed, 27 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>>>> index cd65257..f5206dd 100644
>>>> --- a/lib/vhost/vhost_user.c
>>>> +++ b/lib/vhost/vhost_user.c
>>>> @@ -1441,11 +1441,15 @@
>>>> }
>>>>
>>>> #define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
>>>> +#define VIRTIO_BLK_NUM_VQS_TO_BE_READY 1u
>>>>
>>>> static int
>>>> virtio_is_ready(struct virtio_net *dev)
>>>> {
>>>> + struct rte_vdpa_device *vdpa_dev;
>>>> struct vhost_virtqueue *vq;
>>>> + uint32_t vdpa_type;
>>>> + int ret = 0;
>>>> uint32_t i, nr_vring = dev->nr_vring;
>>>>
>>>> if (dev->flags & VIRTIO_DEV_READY)
>>>> @@ -1454,13 +1458,32 @@
>>>> if (!dev->nr_vring)
>>>> return 0;
>>>>
>>>> - if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) {
>>>> - nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
>>>> + 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");
>>>> + return -1;
>>>> + }
>>>> + } else {
>>>> + vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
>>>> + }
>>>> + } else {
>>>> + vdpa_type = -1;
>>>> + }
>>>
>>> Looking at it again, instead of getting the device type again and again,
>>> Should we just get and set the type into vdpa_dev when registering a
>> vdpa device?
>>>
>>> What do you think?
>>
>> I think this would be preferable.
>>
>> If it is to be in struct virtio_net, please take care to place it
>> properly not to create new holes. Also, it does not have to be placed in
>> hot cachelines.
>
> I am thinking struct rte_vdpa_device. Because if no vdpa device, there will
> be no vdpa device type. What do you think?
Just looked at the code and indeed, struct rte_vdpa_device is the right
place.
Thanks,
Maxime
> Thanks,
> Chenbo
>
>>
>> Thanks,
>> Maxime
>>> Thanks,
>>> Chenbo
>>>
>>>>
>>>> - if (dev->nr_vring < nr_vring)
>>>> - return 0;
>>>> + if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
>>>> + nr_vring = VIRTIO_BLK_NUM_VQS_TO_BE_READY;
>>>> + } else {
>>>> + if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET)
>>>> + nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
>>>> }
>>>>
>>>> + if (dev->nr_vring < nr_vring)
>>>> + return 0;
>>>> +
>>>> for (i = 0; i < nr_vring; i++) {
>>>> vq = dev->virtqueue[i];
>>>>
>>>> --
>>>> 1.8.3.1
>>>
>
@@ -1441,11 +1441,15 @@
}
#define VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY 2u
+#define VIRTIO_BLK_NUM_VQS_TO_BE_READY 1u
static int
virtio_is_ready(struct virtio_net *dev)
{
+ struct rte_vdpa_device *vdpa_dev;
struct vhost_virtqueue *vq;
+ uint32_t vdpa_type;
+ int ret = 0;
uint32_t i, nr_vring = dev->nr_vring;
if (dev->flags & VIRTIO_DEV_READY)
@@ -1454,13 +1458,32 @@
if (!dev->nr_vring)
return 0;
- if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET) {
- nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
+ 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");
+ return -1;
+ }
+ } else {
+ vdpa_type = RTE_VHOST_VDPA_DEVICE_TYPE_NET;
+ }
+ } else {
+ vdpa_type = -1;
+ }
- if (dev->nr_vring < nr_vring)
- return 0;
+ if (vdpa_type == RTE_VHOST_VDPA_DEVICE_TYPE_BLK) {
+ nr_vring = VIRTIO_BLK_NUM_VQS_TO_BE_READY;
+ } else {
+ if (dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET)
+ nr_vring = VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY;
}
+ if (dev->nr_vring < nr_vring)
+ return 0;
+
for (i = 0; i < nr_vring; i++) {
vq = dev->virtqueue[i];