Re: [PATCH 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF devices

Alex Williamson
2024-02-06T21:14:52Z

On Thu, 1 Feb 2024 23:33:37 +0800
Xin Zeng <xin.zeng@intel.com> wrote:

> Add vfio pci driver for Intel QAT VF devices.
>
> This driver uses vfio_pci_core to register to the VFIO subsystem. It
> acts as a vfio agent and interacts with the QAT PF driver to implement
> VF live migration.
>
> Co-developed-by: Yahui Cao <yahui.cao@intel.com>
> Signed-off-by: Yahui Cao <yahui.cao@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
> MAINTAINERS | 8 +
> drivers/vfio/pci/Kconfig | 2 +
> drivers/vfio/pci/Makefile | 2 +
> drivers/vfio/pci/intel/qat/Kconfig | 13 +
> drivers/vfio/pci/intel/qat/Makefile | 4 +
> drivers/vfio/pci/intel/qat/main.c | 572 ++++++++++++++++++++++++++++
> 6 files changed, 601 insertions(+)
> create mode 100644 drivers/vfio/pci/intel/qat/Kconfig
> create mode 100644 drivers/vfio/pci/intel/qat/Makefile
> create mode 100644 drivers/vfio/pci/intel/qat/main.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d1052fa6a69..c1d3e4cb3892 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23095,6 +23095,14 @@ S: Maintained
> F: Documentation/networking/device_drivers/ethernet/amd/pds_vfio_pci.rst
> F: drivers/vfio/pci/pds/
>
> +VFIO QAT PCI DRIVER
> +M: Xin Zeng <xin.zeng@intel.com>
> +M: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> +L: kvm@vger.kernel.org
> +L: qat-linux@intel.com
> +S: Supported
> +F: drivers/vfio/pci/intel/qat/
> +
> VFIO PLATFORM DRIVER
> M: Eric Auger <eric.auger@redhat.com>
> L: kvm@vger.kernel.org
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index 18c397df566d..329d25c53274 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig"
>
> source "drivers/vfio/pci/virtio/Kconfig"
>
> +source "drivers/vfio/pci/intel/qat/Kconfig"
> +
> endmenu
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 046139a4eca5..a87b6b43ce1c 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/
> obj-$(CONFIG_PDS_VFIO_PCI) += pds/
>
> obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/
> +
> +obj-$(CONFIG_QAT_VFIO_PCI) += intel/qat/
> 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?

> + 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.

> 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

> + core_vdev->mig_ops = &qat_vf_pci_mig_ops;
> +
> + ret = vfio_pci_core_init_dev(core_vdev);
> + if (ret)
> + return ret;
> +
> + mutex_init(&qat_vdev->state_mutex);
> + spin_lock_init(&qat_vdev->reset_lock);
> +
> + parent = qat_vdev->core_device.pdev->physfn;
> + vf_id = pci_iov_vf_id(qat_vdev->core_device.pdev);
> + if (!parent || vf_id < 0) {
> + ret = -ENODEV;
> + goto err_rel;
> + }
> +
> + mdev = qat_vfmig_create(parent, vf_id);
> + if (IS_ERR(mdev)) {
> + ret = PTR_ERR(mdev);
> + goto err_rel;
> + }
> +
> + ops = mdev->ops;
> + if (!ops || !ops->init || !ops->cleanup ||
> + !ops->open || !ops->close ||
> + !ops->save_state || !ops->load_state ||
> + !ops->suspend || !ops->resume) {
> + ret = -EIO;
> + dev_err(&parent->dev, "Incomplete device migration ops structure!");
> + goto err_destroy;
> + }
> + ret = ops->init(mdev);
> + if (ret)
> + goto err_destroy;
> +
> + qat_vdev->mdev = mdev;
> +
> + return 0;
> +
> +err_destroy:
> + qat_vfmig_destroy(mdev);
> +err_rel:
> + vfio_pci_core_release_dev(core_vdev);
> + return ret;
> +}