Re: [virtio-comment] [PATCH v1 1/8] admin: Add theory of operation for device migration

[Jason Wang <jasowang@redhat.com>]

On Fri, Oct 13, 2023 at 2:36âPM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Friday, October 13, 2023 6:47 AM
> >
> > On Thu, Oct 12, 2023 at 6:58âPM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > >
> > > > From: Zhu, Lingshan <lingshan.zhu@intel.com>
> > > > Sent: Thursday, October 12, 2023 3:51 PM
> > > >
> > > > On 10/11/2023 7:43 PM, Parav Pandit wrote:
> > > > >> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> > > > >> Sent: Wednesday, October 11, 2023 3:55 PM
> > > > >>>>>>> I donât have any strong opinion to keep it or remove it as
> > > > >>>>>>> most stakeholders
> > > > >>>>>> has the clear view of requirements now.
> > > > >>>>>>> Let me know.
> > > > >>>>>> So some people use VFs with VFIO. Hence the module name.
> > > > >>>>>> This sentence by itself seems to have zero value for the spec. Just
> > drop it.
> > > > >>>>> Ok. Will drop.
> > > > >>>> So why not build your admin vq live migration on our config
> > > > >>>> space solution, get out of the troubles, to make your life easier?
> > > > >>>>
> > > > >>> Your this question is completely unrelated to this reply or you
> > > > >>> misunderstood
> > > > >> what dropping commit log means.
> > > > >> if you can rebase admin vq LM on our basic facilities, I think
> > > > >> you dont need to talk about vfio in the first place, so I ask you
> > > > >> to re-consider
> > > > Jason's proposal.
> > > > > I donât really know why you are upset with the vfio term.
> > > > > It is the use case of the cloud operator and it is listed to
> > > > > indicate how proposal
> > > > fits in a such use case.
> > > > > If for some reason, you donât like vfio, fine. Ignore it and move on.
> > > > >
> > > > > I already answered that I will remove from the commit log, because
> > > > > the
> > > > requirements are well understood now by the committee.
> > > > >
> > > > > Your comment is again unrelated (repeated) to your past two questions.
> > > > >
> > > > > I explained you the technical problem that admin command (not
> > > > > admin VQ)
> > > > of basic facilities cannot be done using config registers without
> > > > any mediation layer.
> > > > OK, I pop-ed Jason's proposal to make everything easier, and I see it is
> > refused.
> > > Because it does not work for passthrough mode.
> >
> > How and why? What's wrong with just passing through the newly introduced 2
> > or 3 registers to guests?
> >
> If passed to the guest who is not involved in the live migration flow, cannot operate the device.
> VF = member device = controlled function
> PF = owner device = controlling function
> Device migration commands from the hypervisor are not forwarded inside the guest.

I don't see why.

For example, can you explain how a virtio reset in guests can survive
with your proposal but not a virtio suspend?

>
> > This is the question you never answer even if I keep asking.
> >
>
> > > Sure, as I explained the config register method do not work for passthrough
> > mode, and does not scale.
> >
> > We need to make sure your migration proposal can work for 1 VF which is still
> > questionable then we can talk about others like scaling. No?
> Sure but in making sure that, the interface is built so that it can work for N VFs too.
>
> >
> > And most of your concern regarding scalability seems more like a limitation of a
> > transport. Let's not mix the scalability for a specific transport with the one for
> > core virtio devices.
> Virtio device is for the defined transport.
> So it needs to work for the defined transport.
> Therefore, scalability cannot be ignored.

I'm not convinced that the scalability is broken by just having 2 or 3
more registers. We all know MSIX requires much more than this.

You can't solve all the issues in one series. As stated many times, if
you really care about the scalability, the correct way is to behave
like a real transport through virtqueue instead of trying to duplicate
the functionality of transport virtqueue slowly.

>
> It is not a question anymore as for any bulk transfer virtqueue is the specification choice for obvious technical gains.

See above.

Thanks

>
> >
> > >
> > > > >
> > > > >> inflight descriptor tracking will be implemented by Eugenio in V2.
> > > > > When we have near complete proposal from two device vendors, you
> > > > > want to push something to unknown future without reviewing the
> > > > > work; does not
> > > > make sense.
> > > > Didn't I ever provide feedback to you? Really?
> > > No. I didnât see why you need to post a new patch for dirty page tracking,
> > when it is already present in this series.
> >
> > You know there are various ways to do dirty paging? For example, the well
> > known bitmap and its variants. I think we've discussed several times in many
> > places in the past. I don't see where you explain why you choose one of them
> > but not the others but you want to forbid other types of dirty page logging?
> > Why?
> The well known bitmap simply do not work for the pci transport in atomic way, effectively.
> You tend to derive many conclusions to oppose the work frankly. :)
> Other types of dirty page logging is not forbidden.
>
> I donât see the need to explain every single word why a given scheme is chosen in the spec language.
> There is line drawn to avoid writing a book and a spec.
>
> > > Secondly I donât see how one can read 1M flows using config registers.
> >
> > Why can't we trap them? vIOMMU even migrates internal translation tables.
>
> Because they are added and removed over the virtqueues almost as data path operations.
> Virtio queues are not mediated/trapped when the native device is virtio member device itself.