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

[Simona Vetter]

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@xxxxxxxxxx>
> > > > > 
> > > > > ---
> > > > > 
> > > > > 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.
-Sima
-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch