Re: [PATCH for 8.1 v2 5/6] vdpa: move CVQ isolation check to net_init_vhost_vdpa

[Jason Wang <jasowang@redhat.com>]

在 2023/3/30 18:42, Eugenio Perez Martin 写道:
> On Thu, Mar 30, 2023 at 8:23 AM Jason Wang <jasowang@redhat.com> wrote:
>> On Thu, Mar 30, 2023 at 2:20 PM Jason Wang <jasowang@redhat.com> wrote:
>>> On Fri, Mar 24, 2023 at 3:54 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>>>> Evaluating it at start time instead of initialization time may make the
>>>> guest capable of dynamically adding or removing migration blockers.
>>>>
>>>> Also, moving to initialization reduces the number of ioctls in the
>>>> migration, reducing failure possibilities.
>>>>
>>>> As a drawback we need to check for CVQ isolation twice: one time with no
>>>> MQ negotiated and another one acking it, as long as the device supports
>>>> it.  This is because Vring ASID / group management is based on vq
>>>> indexes, but we don't know the index of CVQ before negotiating MQ.
>>> We need to fail if we see a device that can isolate cvq without MQ but
>>> not with MQ.
>>>
>>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>>>> ---
>>>> v2: Take out the reset of the device from vhost_vdpa_cvq_is_isolated
>>>> ---
>>>>   net/vhost-vdpa.c | 194 ++++++++++++++++++++++++++++++++++++-----------
>>>>   1 file changed, 151 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>>>> index 4397c0d4b3..db2c9afcb3 100644
>>>> --- a/net/vhost-vdpa.c
>>>> +++ b/net/vhost-vdpa.c
>>>> @@ -43,6 +43,13 @@ typedef struct VhostVDPAState {
>>>>
>>>>       /* The device always have SVQ enabled */
>>>>       bool always_svq;
>>>> +
>>>> +    /* The device can isolate CVQ in its own ASID if MQ is negotiated */
>>>> +    bool cvq_isolated_mq;
>>>> +
>>>> +    /* The device can isolate CVQ in its own ASID if MQ is not negotiated */
>>>> +    bool cvq_isolated;
>>> As stated above, if we need a device that cvq_isolated_mq^cvq_isolated
>>> == true, we need to fail. This may reduce the complexity of the code?
>>>
>>> Thanks
>> Since we are the mediation layer, Qemu can alway choose to negotiate
>> MQ regardless whether or not it is supported by the guest. In this
>> way, we can have a stable virtqueue index for cvq.
>>
> I think it is a great idea and it simplifies this patch somehow.
> However, we need something like the queue mapping [1] to do so :).
>
> To double confirm:
> * If the device supports MQ, only probe MQ. If not, only probe !MQ.
> * Only store cvq_isolated in VhostVDPAState.
>
> Now, if the device does not negotiate MQ but the device supports MQ:


I'm not sure I understand here, if device supports MQ it should accepts 
MQ or we can fail the initialization here.


> * All the requests to queue 3 must be redirected to the last queue in
> the device. That includes set_vq_address, notifiers regions, etc.


This also means we will only mediate the case:

1) Qemu emulated virtio-net has 1 queue but device support multiple queue

but not

2) Qemu emulated virtio-net has M queue but device support N queue (N>M)


>
> I'm totally ok to go this route but it's not immediate.


Yes but I mean, we can start from failing the device if 
cvq_isolated_mq^cvq_isolated == true (or I wonder if we can meet this 
condition for any existing parents).

Thanks


>
> Thanks!
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg07157.html
>