RE: [virtio-comment] Re: [PATCH v1 3/8] device-context: Define the device context fields for device migration

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, November 1, 2023 12:07 PM
> 
> On Wed, Nov 01, 2023 at 05:42:56AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Wednesday, November 1, 2023 11:01 AM
> > >
> > > On Wed, Nov 01, 2023 at 02:54:47AM +0000, Parav Pandit wrote:
> > > >
> > > >
> > > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > > Sent: Tuesday, October 31, 2023 3:44 PM
> > > > >
> > > > > On Tue, Oct 31, 2023 at 05:42:29PM +0800, Zhu, Lingshan wrote:
> > > > > > > Your answer is not relevant to this discussion at all.
> > > > > > > Why?
> > > > > > > Because we were discussing the schemes where registers are not
> used.
> > > > > > > One example of that was IMS. It does not matter MSI or MSIX.
> > > > > > > As explained in Intel's commit message, the key to focus for
> > > > > > > IMS is "queue
> > > > > memory" not some hw register like MSI or MSI-X.
> > > > > > you know the device always need to know a address and the data
> > > > > > to send a MSI, right?
> > > > >
> > > > > So if virtio is to use IMS then we'll need to add interfaces to
> > > > > program IMS, I think. As part of that patch - it's reasonable to
> > > > > assume - we will also need to add a way to retrieve IMS so it
> > > > > can be
> > > migrated.
> > > > >
> > > > > However, what this example demonstrates is that the approach
> > > > > taken by this proposal to migrate control path structures -
> > > > > namely, by defining a structure used just for migration - means
> > > > > that we will need to come up with a migration interface each time.
> > > > > And that is unfortunate.
> > > > >
> > > > When the device supports a new feature it has supported new
> functionality.
> > > > Hence the live migration side also got updated.
> > > > However, the live migration driver does not have to understand
> > > > what is inside
> > > the control path structures.
> > > > It is just byte stream.
> > > > Only if the hypervisor live migration drive involved in emulating,
> > > > it will parse
> > > and that is fine as like other control structures.
> > >
> > > The point is that any new field needs to be added in two places now
> > > and that is not great at all.
> > >
> > Most control structs are well defined. So only its type field is added to
> migrating driver side.
> > This is very low overhead field and handled in generic way for all device types
> and for all common types.
> 
> Weird, not what I see.  E.g. you seem to have a structure duplicating queue
> fields. Each new field will have to be added there in addition to the transport.
> 
Didn't follow.
Which structure is duplicated?
PCI does not even have a structure for q configuration.

> > > We need a stronger compatiblity story here I think.
> > >
> > > One way to show how it's designed to work would be to split the
> > > patches. For example, add queue notify data and queue reset separately.
> > I didn't follow the suggestion. Can you explain splitting patches and its relation
> to the structure?
Did I miss your response? Or it is in below msix?

> >
> > >
> > > Another is to add MSIX table migration option for when MSIX table is
> > > passed through to guest.
> > Yes, this will be added in future when there is actual hypervisor for it.
> 
> You are tying the architecture to an extremely implementation specific detail.
>
Which part?
 
Not really. can you please which software will use MSI-X table migration?

> Hypervisors *already* have migrate the MSIX table. Just in a hypervisor specific
> way. 
MSI-X table is in the PCI BAR memory.

> queue vector is an index into this table. So the index is migrated through
> the device but the table itself has to be trapped and emulated by hypervisor?
Do you have a hypervisor and a platform that has not done MSI-X table emulation for which you are asking to add?
I don't know any.
I am asking to add it when there is a _real_ user of it. Why it cannot be added when the _real_ user arrive?