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

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Sep 21, 2023 at 06:31:01AM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, September 21, 2023 11:37 AM
> 
> > > We should be enhancing the device context so that more and more items can
> > be annotated.
> > > I started with small to get the design and idea through and will expand the
> > device context so that cross vendors can migrate.
> > 
> > As I said, doable without mediation but already done with.
> >
> It is not done in the virtio spec that can work without mediation right?

have trouble parsing this sentence

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

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

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

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

> This aspect is also covered in the proposal [2] in the DISCARD command and stop dirty page tracking (aka stop write reporting).
> 
> [1] https://lore.kernel.org/virtio-comment/20230909142911.524407-7-parav@nvidia.com/T/#mf15b68617f772770c6bf79f70e8ddc6fea834cfa
> 
> To me, frankly, both methods are addressing two slightly different requirement due to which it may not be worth the comparison.
> It is not either or.
> And so far, technically what I understand is, both methods has its pros, cons and limitations due to which both requirements (passthrough and nest) cannot be addressed uniformly.
> Both has its space for the solution it offers.
> At best, two methods can find some common ground of commands or plumbing to reuse, it will be great.
> If I am understanding is wrong, I would like to learn and discuss.

okay now that's progress. what it would be great next if there was a
review of this proposal that does not say "just use my patch instead it
does the same" but instead says "here is how you can reuse plumbing from
my patch and it will address most of your use-case".

-- 
MST