[PATCH v5 7/8] vhost: vDPA blk device gets ready when the first queue is ready

[Maxime Coquelin]

On 10/17/22 10:42, Xia, Chenbo wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>> Sent: Monday, October 17, 2022 4:37 PM
>> To: Xia, Chenbo <chenbo.xia at intel.com>; Pei, Andy <andy.pei at intel.com>;
>> dev at dpdk.org
>> Cc: Xu, Rosen <rosen.xu at intel.com>; Huang, Wei <wei.huang at intel.com>; Cao,
>> Gang <gang.cao at 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 at intel.com>
>>>> Sent: Monday, October 17, 2022 3:14 PM
>>>> To: dev at dpdk.org
>>>> Cc: Xia, Chenbo <chenbo.xia at intel.com>; Xu, Rosen <rosen.xu at intel.com>;
>>>> Huang, Wei <wei.huang at intel.com>; Cao, Gang <gang.cao at intel.com>;
>>>> maxime.coquelin at 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 at intel.com>
>>>> Signed-off-by: Huang Wei <wei.huang at 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
>>>
>