netdev - Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Nov 2021 13:36:10 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Yishai Hadas <yishaih@...dia.com>, bhelgaas@...gle.com,
        saeedm@...dia.com, linux-pci@...r.kernel.org, kvm@...r.kernel.org,
        netdev@...r.kernel.org, kuba@...nel.org, leonro@...dia.com,
        kwankhede@...dia.com, mgurtovoy@...dia.com, maorg@...dia.com,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>
Subject: Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio_pci driver
 for mlx5 devices

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