Subject: Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio_pci driver for mlx5 devices

From: Jason Gunthorpe <[email-masked]>
Date: Tue, 2 Nov 2021 13:36:10 -0300

On Tue, Nov 02, 2021 at 10:22:36AM -0600, Alex Williamson wrote:

> > > There's no point at which we can do SET_IRQS other than in the
> > > _RESUMING state.  Generally SET_IRQS ioctls are coordinated with the
> > > guest driver based on actions to the device, we can't be mucking
> > > with IRQs while the device is presumed running and already
> > > generating interrupt conditions.  
> > 
> > We need to do it in state 000
> > 
> > ie resume should go 
> > 
> >   000 -> 100 -> 000 -> 001
> > 
> > With SET_IRQS and any other fixing done during the 2nd 000, after the
> > migration data has been loaded into the device.
> 
> Again, this is not how QEMU works today.

I know, I think it is a poor choice to carve out certain changes to
the device that must be preserved across loading the migration state.

> > The uAPI comment does not define when to do the SET_IRQS, it seems
> > this has been missed.
> > 
> > We really should fix it, unless you feel strongly that the
> > experimental API in qemu shouldn't be changed.
> 
> I think the QEMU implementation fills in some details of how the uAPI
> is expected to work.

Well, we already know QEMU has problems, like the P2P thing. Is this a
bug, or a preferred limitation as designed?

> MSI/X is expected to be restored while _RESUMING based on the
> config space of the device, there is no intermediate step between
> _RESUMING and _RUNNING.  Introducing such a requirement precludes
> the option of a post-copy implementation of (_RESUMING | _RUNNING).

Not precluded, a new state bit would be required to implement some
future post-copy.

0000 -> 1100 -> 1000 -> 1001 -> 0001

Instead of overloading the meaning of RUNNING.

I think this is cleaner anyhow.

(though I don't know how we'd structure the save side to get two
bitstreams)

Thanks,
Jason