Re: [PATCH] rust/kernel: Add faux device bindings

[Danilo Krummrich]

On Thu, Feb 06, 2025 at 06:04:03PM -0500, Lyude Paul wrote:
> On Thu, 2025-02-06 at 23:30 +0100, Danilo Krummrich wrote:
> > > +//! Abstractions for the faux bus.
> > > +//!
> > > +//! This crate provides bindings for working with faux devices in kernel modules. It should be
> > > +//! preferred for creating virtual devices over the platform API.
> > 
> > "preferred" implies a bit that platform devices are still an option for that
> > (even if not preferred). Maybe just not mention it at all. But if you want to,
> > maybe something along the lines of "faux devices are the solution for the
> > historical abuse of platform devices as virtual devices"?
> > 
> > > +//!
> > > +//! C header: [`include/linux/device/faux.h`]
> > > +use crate::{bindings, device, error::from_err_ptr, prelude::*};
> > > +use core::ptr::{addr_of_mut, null, NonNull};
> > > +
> > > +/// The faux device representation.
> > > +///
> > > +/// This type represents the registration of a [`struct faux_device`]. When an instance of this type
> > > +/// is dropped, its respective faux device will be unregistered from the system.
> > 
> > Ultimately, this will be used to be passed to C APIs, such as drm_dev_alloc(),
> > which increment the reference count of the underlying struct device.
> > 
> > Should we consider that in Rust we may have a need to take additional references
> > in the future too?
> > 
> > Maybe it would be more future proof to call this structure `Registration` and
> > leave us the option to define faux::Device for reference counting later on.
> 
> Yeah I was considering calling this Registration rather than Device, but
> mainly for the reason that a device registration (at least to me) is a unique
> resource.

What about the fact that your comment says "This type represents the
registration [...]"? :-)

> I think actually taking references to the Device should be the job
> of the kernel device core though

Everyone who stores a pointer to a reference counted thing has to take a
reference.

drm_dev_init() for instance, takes a refernece because a drm_device can outlive
the parent device' (in this case the faux device') registration.

Once we get to native Rust APIs of this kind in the future, we'd need to take
our own reference of this device.

The `Registration` structure's lifetime should represent the time in which a
device is registered in the system.

Whereas the `Device` structure's lifetime should represent the lifetime of a
single reference to the device. This is exactly what pci::Device,
platform::Device, and the base device::Device do.

For the faux device it's that faux_device_create() allocates, initializes and
registers the device at once and faux_device_destroy() unregisters the device
and drops the initial reference from faux_device_create() at once. After that,
the device is unregistered, but depending on whether there are still references
held to the device, it can still be alive.

I also suggest to have a look at `MiscDeviceRegistration` registration, which is
similar from the registration side of things.