Received-SPF: pass (google.com: domain of linux-crypto+bounces-2324-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 26 Feb 2024 11:55:56 -0700
From: Alex Williamson <alex.williamson@redhat.com>
To: Xin Zeng <xin.zeng@intel.com>, jgg@nvidia.com
Cc: herbert@gondor.apana.org.au, yishaih@nvidia.com,
 shameerali.kolothum.thodi@huawei.com, kevin.tian@intel.com,
 linux-crypto@vger.kernel.org, kvm@vger.kernel.org, qat-linux@intel.com,
 Yahui Cao <yahui.cao@intel.com>
Subject: Re: [PATCH v3 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF
 devices
Message-ID: <20240226115556.3f494157.alex.williamson@redhat.com>
In-Reply-To: <20240221155008.960369-11-xin.zeng@intel.com>
References: <20240221155008.960369-1-xin.zeng@intel.com>
 <20240221155008.960369-11-xin.zeng@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 21 Feb 2024 23:50:08 +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  |  12 +
>  drivers/vfio/pci/intel/qat/Makefile |   3 +
>  drivers/vfio/pci/intel/qat/main.c   | 663 ++++++++++++++++++++++++++++
>  6 files changed, 690 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 5a4051996f1e..8961c7033b31 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23099,6 +23099,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/
> +

Alphabetical please.

>  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"

This will be the first intel vfio-pci variant driver, I don't think we
need an intel sub-directory just yet.

Tangentially, I think an issue we're running into with
PCI_DRIVER_OVERRIDE_DEVICE_VFIO is that we require driver_override to
bind the device and therefore the id_table becomes little more than a
suggestion.  Our QE is already asking, for example, if they should use
mlx5-vfio-pci for all mlx5 compatible devices.

I wonder if all vfio-pci variant drivers that specify an id_table
shouldn't include in their probe function:

	if (!pci_match_id(pdev, id)) {
		pci_info(pdev, "Incompatible device, disallowing driver_override\n");
		return -ENODEV;
	}

(And yes, I see the irony that vfio introduced driver_override and
we've created variant drivers that require driver_override and now we
want to prevent driver_overrides)

Jason, are you seeing any of this as well and do you have a better
suggestion how we might address the issue?  Thanks,

Alex