[PATCH 5/8] rust: drm: add DRM driver registration

[Maxime Ripard]

Hi Danilo,

On Fri, Mar 28, 2025 at 03:50:13PM +0100, Danilo Krummrich wrote:
> On Fri, Mar 28, 2025 at 03:28:04PM +0100, Maxime Ripard wrote:
> > On Wed, Mar 26, 2025 at 11:46:54AM +0100, Danilo Krummrich wrote:
> > > On Wed, Mar 26, 2025 at 10:24:43AM +0100, Maxime Ripard wrote:
> > > > On Wed, Mar 26, 2025 at 12:54:32AM +0100, Danilo Krummrich wrote:
> > 
> > > > drm_dev_unregister also have an hotplug-aware variant in
> > > > drm_dev_unplug(). However, most devices are hotpluggable, even if only
> > > > through sysfs. So drm_dev_unplug() is generally a better option. Should
> > > > we use it here, and / or should we provide multiple options still?
> > > > 
> > > > Another thing worth mentioning I think is that drm_dev_unplug() works by
> > > > setting a flag, and drivers are expected to check that their access to
> > > > device-bound resources (so registers mapping, clocks, regulators, etc.)
> > > > are still there through drm_dev_enter/drm_dev_exit. It's pretty fragile
> > > > overall, so I wonder if it's something we could abstract away in Rust.
> > > 
> > > DRM should not have to take care of the lifetime of device resources of the
> > > parent device. This is the responsibility of the driver core abstractions.
> > >
> > > At least for the device resources we directly give out to drivers, it has to be,
> > > since otherwise it would mean that the driver core abstraction is unsound.
> > > Drivers could otherwise extend the lifetime of device resources arbitrarily.
> > 
> > Sure.
> > 
> > > For instance, I/O memory is only ever given out by bus abstractions embedded in
> > > a Devres container (e.g. Devres<pci::Bar>). Once the parent device is unbound
> > > the pci::Bar within the Devres container won't be accessible any more and will
> > > be dropped internally. So, that's nothing DRM has to take care of.
> > > 
> > > However, there are cases where it's better to let subsystem abstractions manage
> > > the lifetime of device resources, (e.g. DMA mappings). The relevant thing is,
> > > that we never hand out device resources to a driver in a way that the driver can
> > > extend their lifetime arbitrarily.
> > 
> > I was talking about the opposite though: when the driver is still around
> > but the device (and its resources) aren't anymore.
> 
> Well, that's what I was talking about too. :)
> 
> > It makes total sense to tie the lifetime of the device resources to the
> > device. However, the DRM device and driver will far outlive the device
> > it was bound to so it needs to deal with that kind of degraded "the DRM
> > driver can still be called by userspace, but it doesn't have a device
> > anymore" scenario. That's what I was talking about.
> 
> This scenario should be covered by the things I mentioned above. Let's take the
> I/O memory example.
> 
> If you call into the DRM driver from userspace when the underlying bus device
> has already been unbound, the DRM driver may still hold a Devres<pci::Bar>
> instance.
> 
> If the DRM driver then calls bar.try_access() (in order to subsequently call
> writeX() or readX()) the try_access() call will fail, since the corresponding
> PCI device has been unbound already.
> 
> In other words the pci::Bar instance within the Devres container will be dropped
> (which includes unmapping the bar and releasing the resource region) when the
> PCI device is unbound internally and the Devres container will restrict
> subsequent accesses from drivers.
> 
> It pretty much does the same thing as drm_dev_enter() / drm_dev_exit(), but
> additionally prevents access to the (meanwhile) invalid pointer to the device
> resource and ensures that the driver can't make device resources out-live device
> unbind.
> 
> As mentioned above, the Devres container is just one example of how we prevent
> such things; it depends on the exact scenario. In either case, I don't want the
> driver itself to be responsible to setup the corresponding guards, that would
> make the corresponding abstractions unsound.

Thanks for the explanation :) It makes total sense now.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250410/d0bc2680/attachment-0001.sig>