Received-SPF: pass (google.com: domain of linux-crypto+bounces-2481-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: "Zeng, Xin" <xin.zeng@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
CC: "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
	"jgg@nvidia.com" <jgg@nvidia.com>, "yishaih@nvidia.com" <yishaih@nvidia.com>,
	"shameerali.kolothum.thodi@huawei.com"
	<shameerali.kolothum.thodi@huawei.com>, "Tian, Kevin" <kevin.tian@intel.com>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, qat-linux <qat-linux@intel.com>,
	"Cao, Yahui" <yahui.cao@intel.com>
Subject: RE: [PATCH v4 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF
 devices
Thread-Topic: [PATCH v4 10/10] vfio/qat: Add vfio_pci driver for Intel QAT VF
 devices
Thread-Index: AQHaalR9StzovMarUEC10fvX40lRFLEjlCQAgAOiZ0A=
Date: Mon, 4 Mar 2024 11:10:46 +0000
Message-ID: <BL1PR11MB549484AC60C56912F260933488232@BL1PR11MB5494.namprd11.prod.outlook.com>
References: <20240228143402.89219-1-xin.zeng@intel.com>
	<20240228143402.89219-11-xin.zeng@intel.com>
 <20240301165848.37cfffaf.alex.williamson@redhat.com>
In-Reply-To: <20240301165848.37cfffaf.alex.williamson@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?WYNAo6xVUUn6ayhqOKcsEtyLaYOB4p24rUuxSiReq+fb1v7rzzCuSr+L8yss?=
 =?us-ascii?Q?GuHJE8E6WQZ2TMc9rpjr1ZR+5YTBGybJ4mNPVIfwIN1BUfe3Vl0AQdg7Tizg?=
 =?us-ascii?Q?iFfMPv5bnrNgVYdj8J8Yb8u8tgB3iYzcaAURHRYUg2020XbRdJE3XwmxsAD4?=
 =?us-ascii?Q?1yzgqNuZJl4d6KkaFKHphd2Mf8Zfd97PCOFsLSb03PepF4hakTpQpZkgC1j5?=
 =?us-ascii?Q?R1zwnvPLhYfZ9GjOihodDzSigqX1WMTzghV11B+InKuH5p9cX9i6mmnCKMa6?=
 =?us-ascii?Q?S05HdXBFF1XMVoNaRtpQqvGDP9czJ7U5GbRx9nPiHOSAA5SpJEFAxqAC8Ty0?=
 =?us-ascii?Q?2hCbWQWx+EdVMzUSb8d0uUtZEFoQoE51dTG6Bnv3cPF8ZmXe9D50rnsn7ZZx?=
 =?us-ascii?Q?+EAD6780j/n6DGGuuigo9iAGpJ/TnRB93Pp7kznL8Br1so+T5TbnFerIZIRO?=
 =?us-ascii?Q?ARoQYU+ozxJA766jR8B/jGhiElHPvniOJViLuvM9UUTpNpx6h2vF3VdvTvdS?=
 =?us-ascii?Q?IdvTkurjCsUv6cV6d9GaJuVbN1j9o6MD303dX+fJgADX91FI+RJ593zw3Ev0?=
 =?us-ascii?Q?rsXUTTTkIWzmMbfkeZnmFFP/VJ+lfT9yNUtmIHfdr3ono2jAB+9GfR0wsdyD?=
 =?us-ascii?Q?ZeAuv9HdJEYGvy/l5p6U5Mqaa5owSH3zGw0sSK963kOZdw9MrFaDdXVYNmrF?=
 =?us-ascii?Q?CdmuKiQwX1krlgnSwiQcl6tS6LdbuAgD56CyXGgd6tyYZVBK9Inr4Js25P0X?=
 =?us-ascii?Q?rqxl4/uNSNKmroxB4vvJBoCM1je70vPbgnhW6ZPvLHkrCaVEhxYlMPN0trRA?=
 =?us-ascii?Q?oBeqKOxu8pHLoG56x3+8XDsovJmDcowmkjwxuLJQAY0VeRjhrUxXXDzs7eqS?=
 =?us-ascii?Q?Pxdc4ST4NQnYL+TdaTevyznta1+jPBkrR56gBqztnbNFxMDO6zRcgzwNGxBr?=
 =?us-ascii?Q?gGYU+MD465wRtke/e5WkvfDAp9z2pq8nNnDsWoH/xSbf0xQHjBPB7O10RZBs?=
 =?us-ascii?Q?gldSMtfZqk8zM4D+vbuHwm5drszGmwXMFjn/jzdFKtwlWs+v0xtcgzrRO+P1?=
 =?us-ascii?Q?lP5C95P31/4QwT+DBAVTFedqSgmws0pu5NCh3U7cJtQNt7ltjns3d/s2+txk?=
 =?us-ascii?Q?1VgI1DKMVlGz73x2N3829uo8zx7R75NRXFOGFq+dPsvhI/1PZH9KpZv0nTcg?=
 =?us-ascii?Q?EgIFLGP7eZ56/WZhEssW399WF5WZLNfp2tTMBdNLBxk5sOZQpXw29UN+78O8?=
 =?us-ascii?Q?haTsHSGvL6SXjjo2mkl0qKX2koSe7dxQC8cD6BWnrkCR9CgkPxGtCeqgYJ9M?=
 =?us-ascii?Q?uhmM5qZyhghVDhh6SA90Cm6iDHaGsaAwnmEtl7h95+3+xq+jAc/dU68KG4s3?=
 =?us-ascii?Q?jmluRWhUl6yQa+tjREaCAcYdiDkjU2d8/YfbOh7vnQ3k4ffEw52aZZDXRxkO?=
 =?us-ascii?Q?SDQxmNr2CU+Ib2MgrVXtrkkHijcaRxXPD68mLZetNf2NIy0Hxhn+1qZ890gk?=
 =?us-ascii?Q?dgse2ud3acRkNWFCZawVWmF6jRl+6Vo5SLBw+SOwFz37R/4POO/1xcyuw6Pg?=
 =?us-ascii?Q?uccp6b/1f+FudGlxATk=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-crypto@vger.kernel.org
List-Id: <linux-crypto.vger.kernel.org>
List-Subscribe: <mailto:linux-crypto+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-crypto+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5494.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4fae25e0-6129-40af-a2df-08dc3c3bbdbf
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Mar 2024 11:10:46.3777
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8YyewXF4NrmkEcBSf0kt80sxp47tR9hc8V3fshyR/izIu4v6bNySZmeoH9omZObHUNF+6binTq2mZM5QmOSypA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR11MB8698
X-OriginatorOrg: intel.com

On Saturday, March 2, 2024 7:59 AM, Alex Williamson <alex.williamson@redhat=
.com> wrote:
> On Wed, 28 Feb 2024 22:34:02 +0800
> Xin Zeng <xin.zeng@intel.com> wrote:
> > +static int
> > +qat_vf_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> *id)
> > +{
> > +	struct device *dev =3D &pdev->dev;
> > +	struct qat_vf_core_device *qat_vdev;
> > +	int ret;
> > +
> > +	if (!pci_match_id(id, pdev)) {
> > +		pci_err(pdev, "Incompatible device, disallowing
> driver_override\n");
> > +		return -ENODEV;
> > +	}
>=20
> I think the question of whether this is the right thing to do is still
> up for debate, but as noted in the thread where I raised the question,
> this mechanism doesn't actually work.
>=20
> The probe callback is passed the matching ID from the set of dynamic
> IDs added via new_id, the driver id_table, or pci_device_id_any for a
> strictly driver_override match.  Any of those would satisfy
> pci_match_id().
>=20
> If we wanted the probe function to exclusively match devices in the
> id_table, we should call this as:
>=20
> 	if (!pci_match_id(qat_vf_vfio_pci_table, pdev))...
>=20
> If we wanted to still allow dynamic IDs, the test might be more like:
>=20
> 	if (id =3D=3D &pci_device_id_any)...
>=20
> Allowing dynamic IDs but failing driver_override requires a slightly
> more sophisticated user, but is inconsistent.  Do we have any
> consensus on this?  Thanks,
>=20

As migration support is required to be reported at probe time in the
variant driver, and migration support is device specific, the variant
driver should be considered as device specific driver, not a general
purpose vfio-pci stand-in.
From this point of view, maybe it's not necessary to allow dynamic id
arbitrarily, matching devices in the id_table provided by the variant
driver exclusively would be the preference. If this sound good to you,
I'll update the implementation (to use the correct id table)  in next
version.
Thanks,
Xin