Received-SPF: pass (google.com: domain of linux-crypto+bounces-2328-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 26 Feb 2024 15:49:52 -0400
From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Xin Zeng <xin.zeng@intel.com>, 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: <20240226194952.GO13330@nvidia.com>
References: <20240221155008.960369-1-xin.zeng@intel.com>
 <20240221155008.960369-11-xin.zeng@intel.com>
 <20240226115556.3f494157.alex.williamson@redhat.com>
 <20240226191220.GM13330@nvidia.com>
 <20240226124107.4317b3c3.alex.williamson@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240226124107.4317b3c3.alex.williamson@redhat.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?6LuH/ufHdjffC+o7A8jgAsT+pFNAg5+6zUs35XtSr2CPp2c3pngdCpXb79U7?=
 =?us-ascii?Q?JMTU9TOlMhl9+pSzhQTWz0N0zwdQAkQchgCf3DzqxLloOktKZgGpC5U2wNk1?=
 =?us-ascii?Q?nW36OziHXh50pn+QXjh0gDyQKl2sW9RwGZezmoIWIq5mIDIebzDRDRpFi+Fo?=
 =?us-ascii?Q?BsuR1Hl76Sjq5F7XfgC0uV/938ENG7K57A7TKGbuzYK816jGJJy6Xcu37MiY?=
 =?us-ascii?Q?HJNlnBO0kCIMAf+RQDc2D0djxo4FPd3Xh4MZ+qh+r7fErtD5BIRCORl+wG/U?=
 =?us-ascii?Q?7msineP+tiEojjChbSGrcQvRldxlKz9uUeflW5VtWRyQfJRnhFwAQ59dx14c?=
 =?us-ascii?Q?QSCJ0hNSzrCZLRGXJ7wqa1hsy+STy7SVAC9se7WIVqebHKnCtZTmIPMZXq7p?=
 =?us-ascii?Q?LatHyxQxZrc5pFTqDgXSnjAoj9VzETqtPWY1PCCZt23/vanpaW50mVhhWS66?=
 =?us-ascii?Q?uNpmMlC4tLsFxnIDcFE8tNUMUR+1hIjzyHZVfcPKycipRk8ALiM6V3cvBXh2?=
 =?us-ascii?Q?pDSkp50fQwuoNgMkTHW2htPSpLDcfC5fwSGc6Nzn/3NULwA/DpKaCPQY4Pp2?=
 =?us-ascii?Q?2q4Dkrb+IYeXwVbzq4YHDknab8EV5jUTm1yqO6/3VCGqfIhHblbD6lH+TGmL?=
 =?us-ascii?Q?o1HSu/ZoAeqZLDtIqOYBnhwvUVI+9W0l+VlrQzHueg5Tyvq8JWqyFRKW9dm1?=
 =?us-ascii?Q?KkTWCrI5BJDyVbejOpEx7f4II2TINPd1ls5lIgdSKSlsKXO5FcHAT02isoQq?=
 =?us-ascii?Q?HYH5fg7TI3scwGOSeZEFq+R3JeQ8ky3ECf0J3Mfk9PGMIE70mWoXJwnpdYS2?=
 =?us-ascii?Q?MibHD2Z/WPtLPCRlRb/SfJHsJHRme9Ap3gOD21DNLfegvJFwxy/jpsYqItHJ?=
 =?us-ascii?Q?zgtivdx3OMgS+HmSkaFs4/Ezh5o1VNIexCldlcUI3GJk7DKFnaSzMnfdZfuS?=
 =?us-ascii?Q?z+WsS5FEfjmBpXpUocCL7b52owrx3Nw8gu8RcUxzWXbNTOe9TlLWgAWxalYK?=
 =?us-ascii?Q?CAkn7uiXLJzc/Kvpq0XCzfEyQw9Vxn8sVXRmzT2iISUz7YfF3MhqUPe8iI+C?=
 =?us-ascii?Q?ZjrO4/lMcZiNsvT2VEDj76utBn8BFjTbQrj+coLt+zns1oBgSZwOL25wq6Xw?=
 =?us-ascii?Q?H5im1CJk6n2NzsAvIXd/a2tSTazTXASUf4kPGRYHR2Xr9m/3qfx8EftgO96X?=
 =?us-ascii?Q?EM6T4EVd/5ETyid6H67ZUFAmoSo8SbTtL6pyNJIyM/jmj+FqlZLCob2+ndHj?=
 =?us-ascii?Q?EIJjyljCg8qF+7dleUTqD+8/u/dKb0F9GKBkQT8noiMt/H02yr+IOqIhCT7I?=
 =?us-ascii?Q?ghYmRGfLhOni6303qKZzF9gdTtxaMIkBhPe9HqXrj8O9okPJ0IIm/0q6rCrT?=
 =?us-ascii?Q?OJamWNT6LN1RHHfqMImTd8xdSAKhFvab0djDASNrYHdxDuCwZBNOyJpQW+BK?=
 =?us-ascii?Q?0FRMghF8u0Qs2qGBDQy9yYi5blTEtzTpx6TPDdBtCuZJA0TW+aKrbAwBPDeH?=
 =?us-ascii?Q?SfyCfLf6E9Xe7EOZHbBNE4vbmnlMaLTUqrZLZnr/h8yxzpN9gdAu352+H2kC?=
 =?us-ascii?Q?3I5/nK8kEqyROE6X+xD0MuEWPqu2hHywk6HLFTsk?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ec1ab55-0ed5-4c40-a55e-08dc37041a61
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3849.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Feb 2024 19:49:54.2887
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dzKFo+2VEt4I5K5rPp7vMZUyrEe3nrbcro6f071AKC2N2cDfuMu9dTLL1Hr2KQIF
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7526

On Mon, Feb 26, 2024 at 12:41:07PM -0700, Alex Williamson wrote:
> On Mon, 26 Feb 2024 15:12:20 -0400
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Mon, Feb 26, 2024 at 11:55:56AM -0700, Alex Williamson wrote:
> > > 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.  
> > 
> > They don't have to, but it works fine, there is no reason not to.
> 
> But there's also no reason to.  None of the metadata exposed by the
> driver suggests it should be a general purpose vfio-pci stand-in.

I think the intent was to bind it to all the devices in its ID table
automatically for VFIO use and it should always be OK to do that. Not
doing so is a micro optimization.

Userspace binding it to other random things is a Bad Thing.

> > You are worried about someone wrongly loading a mlx5 driver on, say,
> > an Intel device?
> 
> That's sort of where we're headed if we consider it valid to bind a CX5
> to mlx5-vfio-pci just because they have a host driver with a similar
> name in common. 

I hope nobody is doing that, everyone should be using a tool that
checks that ID table.. If we lack a usable tool for that then it that
is the problemm.

> It's essentially a free for all.  I worry about test matrices, user
> confusion, and being on guard for arbitrary devices at every turn in
> variant drivers if our policy is that they should all work
> equivalent to a basic vfio-pci-core implementation for anything.

Today most of the drivers will just NOP themeslves if they can't find
a compatible PF driver, the most likely bug in this path would be a
NULL ptr deref or something in an untested path, or just total failure
to bind.

We could insist that VF drivers are always able to find their PF or
binding fails, that would narrow things considerably.

> libvirt recently implemented support for managed="yes" with variant
> drivers where it will find the best "vfio_pci" driver for a device
> using an algorithm like Max suggested, but in practice it's not clear
> how useful that will be considering devices likes CX7 require
> configuration before binding to the variant driver.  libvirt has no
> hooks to specify or perform configuration at that point.

I don't think this is fully accurate (or at least not what was
intended), the VFIO device can be configured any time up until the VM
mlx5 driver reaches the device startup.

Is something preventing this? Did we accidentally cache the migratable
flag in vfio or something??

> The driverctl script also exists and could maybe consider the
> "vfio-pci" driver name to be a fungible alias for the best matching
> vfio_pci class driver, but I'm not sure if driverctl has a sufficient
> following to make a difference.

I was also thinking about this tool as an alternative instruction to
using libvirt.

Maybe this would ecourage more people to use it if it implemented it?

Jason