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]

Message-ID: <20211019192328.GZ2744544@nvidia.com>
Date:   Tue, 19 Oct 2021 16:23:28 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     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
Subject: Re: [PATCH V2 mlx5-next 12/14] vfio/mlx5: Implement vfio_pci driver
 for mlx5 devices

On Tue, Oct 19, 2021 at 12:43:52PM -0600, Alex Williamson wrote:
> > +	/* Running switches on */
> > +	if (((old_state ^ state) & VFIO_DEVICE_STATE_RUNNING) &&
> > +	    (state & VFIO_DEVICE_STATE_RUNNING)) {
> > +		ret = mlx5vf_pci_unfreeze_device(mvdev);
> > +		if (ret)
> > +			return ret;
> > +		ret = mlx5vf_pci_unquiesce_device(mvdev);
> > +		if (ret) {
> > +			vmig->vfio_dev_state = VFIO_DEVICE_STATE_ERROR;
> > +			return ret;
> > +		}
> > +	}
> 
> Per previous discussion, I understand that freeze and quiesce are
> loosely stop-responding-to-dma and stop-sending-dma, respectively.
> Once we're quiesced and frozen, device state doesn't change.  What are
> the implications to userspace that we don't expose a quiesce state
> (yet)?  I'm wondering if this needs to be resolved before we introduce
> our first in-tree user of the uAPI (and before QEMU support becomes
> non-experimental).  Thanks,

The prototype patch I saw added a 4th bit to the state which was
   1 == 'not dma initiating'
As you suggested I think a cap bit someplace should be defined if the
driver supports the 4th bit.

Otherwise, I think it is backwards compatible, the new logic would be
two ifs

 if ((flipped & STATE_NDMA) &&
      (flipped & (STATE_NDMA | STATE_RUNNING)) == STATE_NDMA | STATE_RUNNING)
      mlx5vf_pci _quiesce_device()

 [..]

 if ((flipped == (STATE_NDMA)) &&
      (flipped & (STATE_NDMA | STATE_RUNNING)) == STATE_RUNNING)
      mlx5vf_pci_unquiesce_device()

Sequenced before/after the other calls to quiesce_device

So if userspace doesn't use it then the same driver behavior is kept,
as it never sees STATE_NDMA flip

Asking for STATE_NDMA !STATE_RUNNING is just ignored because !RUNNING
already implies NDMA

.. and some optimization of the logic to avoid duplicated work

Jason