Re: [PATCH RFC v1 0/3] Introduce vfio-pci-core subsystem

From: Cornelia Huck <cohuck@redhat.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>, <alex.williamson@redhat.com>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<liranl@nvidia.com>, <oren@nvidia.com>, <tzahio@nvidia.com>,
	<leonro@nvidia.com>, <yarong@nvidia.com>, <aviadye@nvidia.com>,
	<shahafs@nvidia.com>, <artemp@nvidia.com>, <kwankhede@nvidia.com>,
	<ACurrid@nvidia.com>, <gmataev@nvidia.com>, <cjia@nvidia.com>
Subject: Re: [PATCH RFC v1 0/3] Introduce vfio-pci-core subsystem
Date: Mon, 18 Jan 2021 17:00:09 +0100	[thread overview]
Message-ID: <20210118170009.058c8c52.cohuck@redhat.com> (raw)
In-Reply-To: <20210118151020.GJ4147@nvidia.com>

On Mon, 18 Jan 2021 11:10:20 -0400
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, Jan 18, 2021 at 02:38:06PM +0100, Cornelia Huck wrote:
> 
> > > These devices will be seen on the Auxiliary bus as:
> > > mlx5_core.vfio_pci.2048 -> ../../../devices/pci0000:00/0000:00:02.0/0000:05:00.0/0000:06:00.0/0000:07:00.0/mlx5_core.vfio_pci.2048
> > > mlx5_core.vfio_pci.2304 -> ../../../devices/pci0000:00/0000:00:02.0/0000:05:00.0/0000:06:00.0/0000:07:00.1/mlx5_core.vfio_pci.2304
> > > 
> > > 2048 represents BDF 08:00.0 and 2304 represents BDF 09:00.0 in decimal
> > > view. In this manner, the administrator will be able to locate the
> > > correct vfio-pci module it should bind the desired BDF to (by finding
> > > the pointer to the module according to the Auxiliary driver of that
> > > BDF).  
> > 
> > I'm not familiar with that auxiliary framework (it seems to be fairly
> > new?);   
> 
> Auxillary bus is for connecting two parts of the same driver that
> reside in different modules/subystems. Here it is connecting the vfio
> part to the core part of mlx5 (running on the PF).

Yes, that's also what I understood so far; still need to do more reading up.

> 
> > but can you maybe create an auxiliary device unconditionally and
> > contain all hardware-specific things inside a driver for it? Or is
> > that not flexible enough?  
> 
> The goal is to make a vfio device, auxiliary bus is only in the
> picture because a VF device under vfio needs to interact with the PF
> mlx5_core driver, auxiliary bus provides that connection.

Nod.

> 
> You can say that all the HW specific things are in the mlx5_vfio_pci
> driver. It is an unusual driver because it must bind to both the PCI
> VF with a pci_driver and to the mlx5_core PF using an
> auxiliary_driver. This is needed for the object lifetimes to be
> correct.

Hm... I might be confused about the usage of the term 'driver' here.
IIUC, there are two drivers, one on the pci bus and one on the
auxiliary bus. Is the 'driver' you're talking about here more the
module you load (and not a driver in the driver core sense?)

> 
> The PF is providing services to control a full VF which mlx5_vfio_pci
> converts into VFIO API.
> 
> > >  drivers/vfio/pci/Kconfig            |   22 +-
> > >  drivers/vfio/pci/Makefile           |   16 +-
> > >  drivers/vfio/pci/mlx5_vfio_pci.c    |  253 +++
> > >  drivers/vfio/pci/vfio_pci.c         | 2386 +--------------------------
> > >  drivers/vfio/pci/vfio_pci_core.c    | 2311 ++++++++++++++++++++++++++  
> > 
> > Especially regarding this diffstat...   
> 
> It is a bit misleading because it doesn't show the rename

Yes, sure. But it also shows that mlx5_vfio_pci aka the device-specific
code is rather small in comparison to the common vfio-pci code.
Therefore my question whether it will gain more specific changes (that
cannot be covered via the auxiliary driver.)