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

[Jason Wang <jasowang@redhat.com>]

On Wed, Nov 22, 2023 at 12:25âAM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Tuesday, November 21, 2023 9:53 AM
> > On Thu, Nov 16, 2023 at 2:34âPM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > >
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Thursday, November 16, 2023 11:53 AM
> > > >
> > > > On Thu, Nov 16, 2023 at 05:28:19AM +0000, Parav Pandit wrote:
> > > > > You continue to want to overload admin commands for dual purpose,
> > > > > does
> > > > not make sense to me.
> > > >
> > > > dual -> as a transport and for migration? why can't they be used for
> > > > this? I was really hoping to cover these two cases when I proposed them.
> > > For following reasons.
> > >
> > > 1. migration needs incremental reads of only changed context between
> > > two reads
> >
> > This is wrong. We need to invent general facilities. I've pointed out sufficient
> > issues, and what's more delta doesn't work for the following cases:
> >
> I disagree to your above comments. Please read below.
> It works.
>
> > 1) migration but fail the another try for migration
>
> When hypervisor wants to retry the migration, it invokes the DISCARD command present in v4.

Well, I've explained in your first version, your design introduce
unnecessary complexities:

1) there's no requirement for live migrate the device state, that is,
one or two rounds at most are sufficient
2) there's no much data that needs to be migrated

So it doesn't behave like RAM, having things like DISCARD might
complicate the implementation of both software and hardware.

> And starts reading the device context again.
>
> > 2) save vm state twice
> >
> This can be saved twice when needed.
>
> > It request a lot of tricks in hypervisor to do that (e.g cache the last state?).
> Not really.
> Hypervisor can always discard what it read and re-read it again as fresh device context.

Why not leave the policy in the software?

>
> >
> > >
> > > 2. migration writes covers large part of the configurations not just virtio
> > common config and device config.
> >
> > You invent a duplication of common_cfg structure no?
> >
> Nop.
> Common configuration is written using a MMIO, byte/word etc boundary by the guest directly in guest owned area.

We are discussing the function no?

>
> > What's wrong if we just allow them to be R/W over adminq/cmmands?
> >
> As explained before,
> Each guest has its own dedicated non mediated interface as defined in virtio spec to not involve hypervisor.

So what's wrong with inventing per VF queue to do that? For example
transport virtqueue.

> This is uniform for PF and SR-IOV VFs.
> And it will be uniform for backward compatible SIOV tomorrow, one the performance numbers for SIOV are available.
> For non-backward compatible SIOV, there is better way to not have such a large config space anyway.
>
> > > Such as configuration occurred through the CVQ. All of these is not needed
> > when done from guest directly via member's own CVQ.
> >
> > That's the device type specific state which requires new commands forsure. I
> > don't see any connection. The SIOV device needs to be migrated as well.
> >
> And they will use all majority of the device context as presented here.
>
> > >
> > > For backward compatible SIOV transport, one may need them to transport
> > without above two properties.
> >
> > Why, just mediate between virtual PCI and adminq.
> >
> I donât understand this.

I mean, in order to run "legacy" guest that can only recognize
virtio-pci, a hypervisor can mediate between it and transport.

>
> > >
> > > 3. None of this transport is needed for PFs, VFs and non-backward
> > compatible SIOVs.
> > > Each device to have its own transport that is not intercepted by the
> > hypervisor and follow the equivalency principle uniformly for all 3 device types.
> >
> > You can have per VF transport q, what's wrong with that?
> As explained in the doc and in multiple emails, it is inefficient. CVQ is just enough.

Let me repeat again, transport q is not intended to replace CVQ. Where
did you see or get such a wrong conclusion in the series of transport
q?

One of its goals is to have a transport where the register doesn't scale.

Thanks