? 2021/8/12 ??3:01, Eli Cohen ??:> On Thu, Aug 12, 2021 at 02:47:06PM +0800, Jason Wang wrote:
>> On Thu, Aug 12, 2021 at 12:55 PM Eli Cohen <elic at nvidia.com>
wrote:
>>> On Thu, Aug 12, 2021 at 11:19:19AM +0800, Jason Wang wrote:
>>>> ? 2021/8/11 ??7:04, Eli Cohen ??:
>>>>> On Wed, Aug 11, 2021 at 04:37:44PM +0800, Jason Wang wrote:
>>>>>> ? 2021/8/11 ??3:53, Eli Cohen ??:
>>>>>>>> One thing need to solve for mq is that the:
>>>>>>>>
>>>>>>>>
>>>>>>>>> +static u16 ctrl_vq_idx(struct
mlx5_vdpa_dev *mvdev)
>>>>>>>>> +{
>>>>>>>>> + return 2 *
mlx5_vdpa_max_qps(mvdev->max_vqs);
>>>>>>>>> +}
>>>>>>>> We should handle the case when MQ is supported
by the device but not the
>>>>>>>> driver.
>>>>>>>>
>>>>>>>> E.g in the case when we have 2 queue pairs:
>>>>>>>>
>>>>>>>> When MQ is enabled, cvq is queue 4
>>>>>>>>
>>>>>>>> When MQ is not enabled, cvq is queue 2
>>>>>>>>
>>>>>>> There's some issue with this. I get callbacks
targeting specific
>>>>>>> virtqueues before features negotiation has been
completed.
>>>>>>>
>>>>>>> Specifically, I get set_vq_cb() calls. At this
point I must know the
>>>>>>> control vq index.
>>>>>> So I think we need do both:
>>>>>>
>>>>>> 1) At one hand, it's a bug for the userspace to use
vq_index before feature
>>>>>> is negotiated
>>>>>>
>>>>>> (looks like a bug in my cvq series that will call
SET_VRING_CALL before
>>>>>> feature is negotiate, which I will look).
>>>>>>
>>>>>> 2) At the other hand, the driver should be able to deal
with that
>>>>>>
>>>>> All I can do is drop callbacks for VQs before features
negotation has
>>>>> been completed.
>>>>
>>>> Or just leave queue index 0, 1 work.
>>>>
>>>> Since it is not expected to be changed.
>>>>
>>> Right, will do.
>>>
>>>>>>> I think the CVQ index must not depend on the
negotiated features but
>>>>>>> rather depend of the value the device driver
provides in the call to
>>>>>>> _vdpa_register_device().
>>>>>> At the virtio level, it's too late to change that
and it breaks the backward
>>>>>> compatibility.
>>>>>>
>>>>>> But at the vDPA level, the under layer device can map
virtio cvq to any of
>>>>>> it's virtqueue.
>>>>>>
>>>>>> E.g map cvq (index 2) to mlx5 cvq (the last).
>>>>>>
>>>>> I am not following you here. I still don't know what
index is cvq.
>>>>
>>>> Right, we still need to wait for the feature being negotiated
in order to
>>>> proceed.
>>>>
>>> So to summarise, before feature negotiation complete, I accept
calls
>>> referring to VQs only for indices 0 and 1.
>>> After feature negotiation complete I know CVQ index and will accept
>>> indices 0 to cvq index.
>> I don't get this "accept indices 0 to cvq index".
> What I meant to say is that there are several callbacks that refer to
> specific virtqueues, e.g. set_vq_address(), set_vq_num() etc. They all
> accept virtqueue index as an argument.
>
> What we want to do is verify wheather the index provided is valid or
> not. If it is not valid, either return error (if the callback can return
> a value) or just avoid processing it. If the index is valid then we
> process it normally.
>
> Now we need to decide which index is valid or not. We need something
> like this to identifiy valid indexes range:
>
> CVQ clear: 0 and 1
> CVQ set, MQ clear: 0, 1 and 2 (for CVQ).
> CVQ set, MQ set: 0..nvq where nvq is whatever provided to
> _vdpa_register_device()
Yes.
>
> And while writing this, I think this logic does not belog in mlx5_vdpa
> but probably in vdpa.c
The problem is that vdpa should be unaware of a specific device type.
E.g the above indices may work only for virtio-net but not other.
Thanks
>
>> Thanks
>>
>>>> Thanks
>>>>
>>>>
>>>>>> Thanks
>>>>>>
>>>>>>