Re: [dpdk-dev] [PATCH 3/9] vdpa/ifc: add support to vDPA queue enable

[Jason Wang <jasowang@redhat.com>]

On 2020/5/18 上午11:09, Wang, Xiao W wrote:
> Hi,
>
> Comments inline.
>
> Best Regards,
> Xiao
>
>> -----Original Message-----
>> From: Jason Wang<jasowang@redhat.com>
>> Sent: Friday, May 15, 2020 6:09 PM
>> To: Wang, Xiao W<xiao.w.wang@intel.com>; Maxime Coquelin
>> <maxime.coquelin@redhat.com>; Ye, Xiaolong<xiaolong.ye@intel.com>;
>> shahafs@mellanox.com;matan@mellanox.com;amorenoz@redhat.com;
>> viacheslavo@mellanox.com;dev@dpdk.org
>> Cc:lulu@redhat.com; Xu, Rosen<rosen.xu@intel.com>
>> Subject: Re: [PATCH 3/9] vdpa/ifc: add support to vDPA queue enable
>>
>>
>> On 2020/5/15 下午5:42, Wang, Xiao W wrote:
>>> Hi,
>>>
>>> Best Regards,
>>>
>>> Xiao
>>>
>>>> -----Original Message-----
>>>> From: Jason Wang<jasowang@redhat.com>
>>>> Sent: Friday, May 15, 2020 5:09 PM
>>>> To: Maxime Coquelin<maxime.coquelin@redhat.com>; Ye, Xiaolong
>>>> <xiaolong.ye@intel.com>;shahafs@mellanox.com;matan@mellanox.com;
>>>> amorenoz@redhat.com; Wang, Xiao W<xiao.w.wang@intel.com>;
>>>> viacheslavo@mellanox.com;dev@dpdk.org
>>>> Cc:lulu@redhat.com
>>>> Subject: Re: [PATCH 3/9] vdpa/ifc: add support to vDPA queue enable
>>>> On 2020/5/14 下午4:02, Maxime Coquelin wrote:
>>>>> This patch adds support to enabling and disabling
>>>>> vrings on a per-vring granularity.
>>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com
>>> <mailto:maxime.coquelin@redhat.com>>
>>>
>>>> A question here, I see in qemu peer_attach() may try to generate
>>>> VHOST_USER_SET_VRING_ENABLE, but just from the name I think it should
>>>> behave as queue_enable defined in virtio specification which is
>>>> explicitly under the control of guest?
>>>> (Note, in Cindy's vDPA series, we must invent new vhost_ops to differ
>>>> from this one).
>>>  From my view, common_cfg.enable reg is used for registering a queue to
>>> hypervisor&vhost, but not ENABLE.
>>>
>> Well, what's your definition of "enable" in this context?
> "Enable a queue" means traffic can pass through this queue.
>
>> Spec said:
>>
>> queue_enable
>>      The driver uses this to selectively prevent the device from
>>      executing requests from this virtqueue. 1 - enabled; 0 - disabled.
>>
>> This means, if queue_enable is not set to 1, device can not execute
>> request for this specific virtqueue.
>>
> For queue enabling in virtio MQ case, there're 2 steps needed:
> 1. select a queue and write 1 to common_cfg.enable reg


Note that:

1) queue_enable doesn't mean you can disable a queue by writing zero to 
that (which is not allowed by the spec)
2) queue_enable is not specific to MQ, you need write 1 to all the 
queues that will be used by this driver
3) it's not allowed to write 1 to queue_enable after DRIVER_OK


> 2. send control vq message VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> If no step2, by default there's only 1 queue pair enabled.


Yes, and if you read the git history. This command is invented by me :)

Thanks


>