Re: [PATCH] WIP: drivers/base: Add virtual_device_create()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Feb 03, 2025 at 10:45:37AM +0100, Simona Vetter wrote:
> On Sat, Feb 01, 2025 at 09:00:00AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Jan 31, 2025 at 07:43:07PM +0100, Danilo Krummrich wrote:
> > > On Fri, Jan 31, 2025 at 05:40:01PM +0100, Greg Kroah-Hartman wrote:
> > > > On Fri, Jan 31, 2025 at 09:00:32AM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Jan 30, 2025 at 04:28:26PM -0500, Lyude Paul wrote:
> > > > > > As Greg KH pointed out, we have a nice /sys/devices/virtual directory free
> > > > > > for the taking - but the vast majority of device drivers concerned with
> > > > > > virtual devices do not use this and instead misuse the platform device API.
> > > > > > 
> > > > > > To fix this, let's start by adding a simple function that can be used for
> > > > > > creating virtual devices - virtual_device_create().
> > > > > > 
> > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > > > > 
> > > > > > ---
> > > > > > 
> > > > > > So, WIP obviously because I wrote this up in a few minutes - but this goes
> > > > > > off the idea that Danilo suggested to me off-list of coming up with a
> > > > > > simple API for handling virtual devices that's a little more obvious to
> > > > > > use. I wanted to get people's feedback and if we're happy with this idea,
> > > > > > I'm willing to go through and add some pointers to this function in various
> > > > > > platform API docs - along with porting over the C version of VKMS over to
> > > > > > this API.
> > > > > 
> > > > > This is a big better, but not quite.  Let me carve out some time today
> > > > > to knock something a bit nicer together...
> > > > 
> > > > Ok, here's a rough first-cut.  It builds, and boots, and I've converted
> > > > a driver to use the api to prove it works here.  I'll add a bunch more
> > > > documentation before turning it into a "real" patch, but this should
> > > > give you something to work off of.
> > > > 
> > > > I've run out of time for tonight (dinner is calling), but I think you
> > > > get the idea, right?  If you want to knock up a rust binding for this
> > > > api, it should almost be identical to the platform api you were trying
> > > > to use before, right?
> > > 
> > > Yes, additionally, since this can't use the existing platform abstractions any
> > > more, we need the bus abstraction for the virtual bus, i.e. the corresponding
> > > driver::RegistrationOps implementation, module_virtual_driver macro, etc. Should
> > > be a little less than 200 lines of code.
> > 
> > I hope so as the original C code for this is less than 200 lines of code :)
> > 
> > I wonder what it would look like to do a "real" bus in rust, maybe I'll
> > try that someday, but for now, I want this to be used by C code...
> > 
> > > Other than in C, in Rust we don't need the "artificial" match between a virtual
> > > device and a virtual driver to have automatic cleanup through things like
> > > devm_kzalloc().
> > 
> > What artificial match?  Ah, you mean they would both be in the same
> > "object"?
> > 
> > > But I guess we want it for consistency and to have the corresponding sysfs
> > > entries and uevents. OOC, are there any other reasons?
> > 
> > I don't really understand the objection here.  Oooh, you want the C code
> > to both create/manage the driver AND the device at the same time?  Hey I
> > like that, it would make the interface to it even simpler!  Let me go
> > try that, and see if it is what you are thinking of here...
> 
> So at least in vkms we plan to allow instantiating more than one device,
> with the same driver, so not sure we really want this. The idea is that
> you can also validate the multi-gpu (or at least multi-display) code of
> compositors with entirely fake hw in CI. It is a pretty common pattern
> though for these virtual devices/drivers, but not universal I think.

See the patch I just posted, it creates internal drivers for every
device you create, so in reality, you don't need to worry about the
driver portion at all, just provide a probe/release callback if you want
to and you are good to go!

thanks,

greg "drivers for everyone!" k-h