RE: [virtio-dev] Re: [PATCH 0/5] virtio: introduce SUSPEND bit and vq state

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, September 21, 2023 3:32 PM
> 
> On Thu, Sep 21, 2023 at 09:17:53AM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, September 21, 2023 1:41 PM
> >
> > > > I mean to say,
> > > > Virtio spec has not achieved mediation less, device migration.
> > > > Virtio spec has not achieved device migration using mediation.
> > >
> > > But yes it has - it was implemented with shadow vq.
> > >
> > Shadow vq + several other trap points on config space, cvq and more.
> 
> exactly.
> 
Ok.

> > we cannot suspend the whole device and resume from where it was left off.
> > Those extensions are happening now.
> >
> > > > And two proposals are trying to do it.
> > > >
> > > > >
> > > > > > >
> > > > > > > > > 1. For mediation something that works within existing
> > > > > > > > > mediation framework - e.g. reusing as he does feature
> > > > > > > > > bits - will require less support than a completely separate facility.
> > > > > > > > > I think Zhu Lingshan also believes that since there will
> > > > > > > > > be less code -> less security issues.
> > > > > > > > >
> > > > > > > > With approach of [1], there is less code in the core
> > > > > > > > device migration flow
> > > > > > > because none of those fields etc are parsed/read/written by
> > > > > > > the driver software.
> > > > > > >
> > > > > > > What is or is not executed in a specific flow is a separate question.
> > > > > > > But the point is vdpa and any mediation have to talk virtio
> > > > > > > things such as feature bits. So reusing e.g. feature bits
> > > > > > > needs less code than operating the admin command machinery
> > > > > > > to check what is supported. Yes, you can operate this
> > > > > > > machinery during setup and not during migration itself. It's still less
> code to maintain.
> > > > > > >
> > > > > > I wouldn't go down the path of code comparison.
> > > > > > But if you want to: we can take a concrete example of what is
> > > > > > done by similar
> > > > > device who uses admin command approach.
> > > > > > The admin command-based approach migration driver is likely
> > > > > > 10x smaller
> > > > > than the actual driver driving the feature bits and rest of the config.
> > > > >
> > > > > yes but mediation driver already has to do feature bits. so if
> > > > > doing mediation then the cost of adding this specific extension is low.
> > > > >
> > > > I thought first you were counting the cost of the code and not the
> > > > spec in your
> > > point "feature bits needs less code than operating".
> > >
> > > yes - with vdpa it's mostly just
> > > 	vdev->status |= SUSPEND
> > > 	vdev->status &= ~SUSPEND
> > > all over the place.
> > >
> > + inflight descriptors.
> 
> for sure, this is just stopping it.
> 
> > > > > > If one needs more precise numbers of number of lines of code,
> > > > > > I can
> > > derive it.
> > > > > > As features and functionality grows, every line of code gets
> > > > > > added there in
> > > > > mediation too.
> > > > > > I agree such mediation has value and use case, as we know it
> > > > > > is not the only
> > > > > approach fitting all use cases.
> > > > >
> > > > > Do you see how this extension is easier for mediation than
> > > > > driving admin queue though?
> > > > >
> > > > If we count the total cost of code than building the mediation
> > > > framework +
> > > extensions, than it is not.
> > > > But as I said, I wouldn't compare two solutions as they are
> > > > addressing a slightly
> > > different requirement.
> > >
> > > yes they are. the point of comparison was explaining why people who
> > > use mediation anyway might not want to also use aq. can i assume that's
> clear?
> > >
> > I am not fully sure.
> > I frankly don't find it right for member virtio device itself to be mediated.
> > Vdpa stack make total sense when the underlying device is not virtio and
> hence emulation.
> > But when there is native virtio member device, further mediation is overkill
> for certain scenarios.
> > But I understand that it helps to utilize a vdpa stack and thereby overcome
> some limitations, while it introduces other limitations...
> 
> yes. whether it makes sense depends on the use-case.
> 
Both are valid use cases so vdpa over virtio can find the uses to overcome some of the limitations.

When one wants to use virtio member device as passthrough, when needed attributes match, one can use it too.

> > This just fine, because FLR is not resetting what PF has done.
> > PF is not touching FLR side of things either.
> >
> > > will get you a VF that is not running and can not be given to
> > > another guest. You have to
> > > - queue stop command on PF
> > > - stop command is seen by PF
> > > - flr on VF
> > > in this order.
> > >
> > Since you are discussing the admin patches of [1], better to discuss there.
> > But even if we follow the sequence you described, it is also fine.
> 
> well then flr itself is now insufficient to reset the VF completely to its original
> state.  
That is how it is in the PCI spec and other consortium based spec and many vendor implementations too.

> stopped before assigning it to guest or not.
> a stopped device can't be used by guest the way it expects.
> And to make sure, one has to wait for some admin command to complete, one
> way or another.

Yes, sw stack which dynamically keep mapping VFs to different VMs already do it for virtio and non virtio devices.
The migration driver will make sure whatever it has setup is cleared by it before handing over the VF.