Re: [PATCH v2 02/13] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind

From: Dan Williams <dan.j.williams@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	kvm@vger.kernel.org, Kirti Wankhede <kwankhede@nvidia.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	Leon Romanovsky <leonro@nvidia.com>,
	Max Gurtovoy <mgurtovoy@nvidia.com>,
	Tarun Gupta <targupta@nvidia.com>,
	dave.jiang@intel.com
Subject: Re: [PATCH v2 02/13] vfio/mdev: Allow the mdev_parent_ops to specify the device driver to bind
Date: Wed, 28 Apr 2021 12:58:29 -0700	[thread overview]
Message-ID: <CAPcyv4hnjX-HtoG08dPbPxJPeJyvnO-WaJosoY1aSRqm5oo14Q@mail.gmail.com> (raw)
In-Reply-To: <20210428140005.GS1370958@nvidia.com>

On Wed, Apr 28, 2021 at 7:00 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Apr 28, 2021 at 02:41:53PM +0200, Christoph Hellwig wrote:
> > On Wed, Apr 28, 2021 at 12:56:21AM -0700, Dan Williams wrote:
> > > > I still think this going the wrong way.  Why can't we enhance the core
> > > > driver code with a version of device_bind_driver() that does call into
> > > > ->probe?  That probably seems like a better model for those existing
> > > > direct users of device_bind_driver or device_attach with a pre-set
> > > > ->drv anyway.
> > >
> > > Wouldn't that just be "export device_driver_attach()" so that drivers
> > > can implement their own custom bind implementation?
> >
> > That looks like it might be all that is needed.
>
> I thought about doing it like that, it is generally a good idea,
> however, if I add new API surface to the driver core I really want to
> get rid of device_bind_driver(), or at least most of its users.

I might be missing where you are going with this comment, but
device_driver_attach() isn't a drop-in replacement for
device_bind_driver(). So while I agree with you that it's a
significant escalation of the driver core API surface, I don't see why
it would be necessarily predicated on removing device_bind_driver()?

If this export prevented a new device_bind_driver() user, I think
that's a net positive, because device_bind_driver() seems an odd way
to implement bus code to me.

> I'm pretty sure Greg will ask for it too.

I think it's worth asking.

I have an ulterior motive / additional use case in mind here which is
the work-in-progress cleanup of the DSA driver. It uses the driver
model to assign an engine to different use cases via driver binding.
However, it currently has a custom bind implementation that does not
operate like a typical /sys/bus/$bus/drivers interface. If
device_driver_attach() was exported then some DSA compat code could
model the current way while also allowing a transition path to the
right way. As is I was telling Dave that the compat code would need to
be built-in because I don't think fixing a DSA device-model problem is
enough justification on its own to ask for a device_driver_attach()
export.

> So, I need a way to sequence that which doesn't mean I have to shelf
> the mdev stuff for ages while I try to get acks from lots of places.

Lets see if it can stand alone.