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 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.

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.

> > > > 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...

> > What to compare is what can be reused between two solutions.
> >
> > > > >
> > > > > > > 2. With or without mediation, the mapping of commands to VFs
> > > > > > > is simpler, allowing more control - for example, let's say
> > > > > > > you want to reset a VF - you do not need to flush the queue
> > > > > > > of existing commands, which might potentially take a long
> > > > > > > time because some other VFs are very busy - you just reset
> > > > > > > the VF which any unmap flow will
> > > > > already do.
> > > > > > >
> > > > > > If I understand you right, to reset a VF, no need to flush the
> > > > > > queues without
> > > > > mediation too.
> > > > > > Just do VF FLR or do device reset, both will be fine.
> > > > >
> > > > > Not to reset the VF - that's a narrow definition. To put it back
> > > > > in its original state unrelated to any VMs.  Nope, FLR is not
> > > > > enough - there could be commands in queue addressing the VF. If
> > > > > they take effect after FLR VF state changed and it can't be
> > > > > cleanly assigned to a new
> > > VM.
> > > > >
> > > > If you mean the SR-PCIM command, than it is virtio spec to define them.
> > > > We ratified this recently in the PCI-SIG of what gets cleared on
> > > > VF FLR and
> > > what stays that SR-PCIM interface to do.
> > >
> > > Interesting. Which ECN exactly do you refer to?
> > >
> > B405.
> >
> > > > So if other commands are in pipe, only after they are done, such
> > > > VF will be
> > > assigned to new VM.
> > >
> > > Exactly. this is exactly what I meant when I said "flush the queue"
> > > - you have to wait until these commands are done, then do reset.
> > >
> > Not exactly. VF reset is fully controlled by the guest.
> > Hence, it does not collide with admin side of commands,
> 
> No, host does VF reset in a number of situations, including guest restart, guest
> shutdown, etc.
> 
That is fine, it can do.

> > Same admin command for dirty page tracking and device context do not mess
> with FLR.
> > This is the critical because VF FLR cannot clear the page addresses already
> reported and not yet read by the driver.
> > It is covered in admin proposal.
> 
> They might not mess with it in that you can still do FLR but they will mess with
> what hosts commonly try to achieve with FLR which is getting a clean VF that
> has nothing to do with a given guest state.
> For example
> - queue stop command on PF
> - flr on VF
> - stop command is seen by PF
>
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.