Re: About restoring the state in vhost-vdpa device

[Eugenio Perez Martin]

On Fri, May 13, 2022 at 8:25 PM Parav Pandit <parav@nvidia.com> wrote:
>
> Hi Gautam,
>
> Please fix your email client to have right response format.
> Otherwise, it will be confusing for the rest and us to follow the 
> conversation.
>
> More below.
>
> > From: Gautam Dawar <gdawar@xilinx.com>
> > Sent: Friday, May 13, 2022 1:48 PM
>
> > > Our proposal diverge in step 7: Instead of enabling *all* the
> > > virtqueues, only enable the CVQ.
> > Just to double check, VQ 0 and 1 of the net are also not enabled, correct?
> > [GD>>] Yes, that's my understanding as well.
> >

That's correct. We can say that for a queue to be enabled three things
must happen:
* DRIVER_OK (Still not send)
* VHOST_VDPA_SET_VRING_ENABLE sent for that queue (Only sent for CVQ)
* If queue is not in first data queue pair and not cvq: send
VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET with a queue pair that include it.

> > > After that, send the DRIVER_OK and queue all the control commands to
> > > restore the device status (MQ, RSS, ...). Once all of them have been
> > > acknowledged ("device", or emulated cvq in host vdpa backend driver,
> > > has used all cvq buffers, enable (SET_VRING_ENABLE, set_vq_ready) all
> > > other queues.
> > >
> > What is special about doing DRIVER_OK and enqueuing the control
> > commands?
> > Why other configuration cannot be applied before DRIVER_OK?

There is nothing special beyond "they have a method to be set that
way, so reusing it avoids having to maintain many ways to set the same
things, simplifying implementations".

I'm not saying "it has been like that forever so we cannot change it":
I'm very open to the change but priority-wise we should first achieve
a working LM with packed, in_order, or even indirect.

> > [GD>>] For the device to be live (and any queue to be able to pass traffic),
> > DRIVER_OK is a must.
> This applies only to the vdpa device implemented over virtio device.
> For such use case/implementation anyway a proper virtio spec extension is 
> needed for it be efficient.
> And what that happens this scheme will still work.
>

Although it's a longer route, I'd very much prefer an in-band virtio
way to perform it rather than a linux/vdpa specific. It's one of the
reasons I prefer the CVQ behavior over a vdpa specific ioctl.

> Other vdpa devices doesn’t have to live with this limitation at this moment.
>
> > So, any configuration can be passed over the CVQ only
> > after it is started (vring configuration + DRIVER_OK). For an emulated 
> > queue,
> > if the order is reversed, I think the enqueued commands will remain
> > buffered and device should be able to service them when it goes live.
> I likely didn’t understand what you describe above.
>
> Vq avail index etc is programmed before doing DRIVER_OK anyway.
>
> Sequence is very straight forward at destination from user to kernel.
> 1. Set config space fields (such as virtio_net_config/virtio_blk_config).
> 2. Set other device attributes (max vqs, current num vqs)
> 3. Set net specific config (RSS, vlan, mac and more filters)
> 4. Set VQ fields (enable, msix, addr, avail indx)
> 5. Set DRIVER_OK, device resumes from where it left off
>
> Steps #1 to #4 can be done multiple times in pre-warm device up case in 
> future.

That requires creating a way to set all the parameters enumerated
there to vdpa devices. Each time a new feature is added to virtio-net
that modifies the guest-visible fronted device we would need to update
it. And all the layers of the stack need to maintain more state.

>From the guest point of view, to enable all the queues with
VHOST_VDPA_SET_VRING_ENABLE and don't send DRIVER_OK is the same as
send DRIVER_OK and not to enable any data queue with
VHOST_VDPA_SET_VRING_ENABLE. We can do all the pre-warming phase that
way too, avoiding adding more maintenance burden to vdpa.

> For now, they can be done once to get things started.