Thanks for your comments, Alex.
On Wednesday, February 7, 2024 5:15 AM, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:
> > diff --git a/drivers/vfio/pci/intel/qat/Kconfig
> b/drivers/vfio/pci/intel/qat/Kconfig
> > new file mode 100644
> > index 000000000000..71b28ac0bf6a
> > --- /dev/null
> > +++ b/drivers/vfio/pci/intel/qat/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +config QAT_VFIO_PCI
> > + tristate "VFIO support for QAT VF PCI devices"
> > + select VFIO_PCI_CORE
> > + depends on CRYPTO_DEV_QAT
> > + depends on CRYPTO_DEV_QAT_4XXX
>
> CRYPTO_DEV_QAT_4XXX selects CRYPTO_DEV_QAT, is it necessary to depend
> on CRYPTO_DEV_QAT here?
You are right, we don't need it, will update it in next version.
>
> > + help
> > + This provides migration support for Intel(R) QAT Virtual Function
> > + using the VFIO framework.
> > +
> > + To compile this as a module, choose M here: the module
> > + will be called qat_vfio_pci. If you don't know what to do here,
> > + say N.
> > diff --git a/drivers/vfio/pci/intel/qat/Makefile
> b/drivers/vfio/pci/intel/qat/Makefile
> > new file mode 100644
> > index 000000000000..9289ae4c51bf
> > --- /dev/null
> > +++ b/drivers/vfio/pci/intel/qat/Makefile
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +obj-$(CONFIG_QAT_VFIO_PCI) += qat_vfio_pci.o
> > +qat_vfio_pci-y := main.o
> > +
>
> Blank line at end of file.
Good catch, will update it in next version.
>
> > diff --git a/drivers/vfio/pci/intel/qat/main.c
> b/drivers/vfio/pci/intel/qat/main.c
> > new file mode 100644
> > index 000000000000..85d0ed701397
> > --- /dev/null
> > +++ b/drivers/vfio/pci/intel/qat/main.c
> > @@ -0,0 +1,572 @@
> ...
> > +static int qat_vf_pci_init_dev(struct vfio_device *core_vdev)
> > +{
> > + struct qat_vf_core_device *qat_vdev = container_of(core_vdev,
> > + struct qat_vf_core_device, core_device.vdev);
> > + struct qat_migdev_ops *ops;
> > + struct qat_mig_dev *mdev;
> > + struct pci_dev *parent;
> > + int ret, vf_id;
> > +
> > + core_vdev->migration_flags = VFIO_MIGRATION_STOP_COPY |
> VFIO_MIGRATION_P2P;
>
> AFAICT it's always good practice to support VFIO_MIGRATION_PRE_COPY,
> even if only to leave yourself an option to mark an incompatible ABI
> change in the data stream that can be checked before the stop-copy
> phase of migration. See for example the mtty sample driver. Thanks,
> Alex
If the good practice is to check migration stream compatibility, then
we do incorporate additional information as part of device state for ABI
compatibility testing such as magic, version and device capability in stop
copy phase. Can we ignore the optional VFIO_MIGRATION_PRE_COPY support?
Thanks,
Xin
