[PATCH 0/4] Implement TryFrom<&Device> for bus specific devices

[PATCH 0/4] Implement TryFrom<&Device> for bus specific devices

[Danilo Krummrich]

This series provides a mechanism to safely convert a struct device into its
corresponding bus specific device instance, if any.

In C a generic struct device is typically converted to a specific bus device
with container_of(). This requires the caller to know whether the generic struct
device is indeed embedded within the expected bus specific device type.

In Rust we can do the same thing by implementing the TryFrom trait, e.g.

        impl TryFrom<&Device> for pci::Device

This is a safe operation, since we can check whether dev->bus equals the the
expected struct bus_type.

Additionally, provide an accessor for a device' parent.

A branch containing the patches can be found in [1].

This is needed for the auxiliary bus abstractions and connecting nova-core with
nova-drm. [2]

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/device
[2] https://gitlab.freedesktop.org/drm/nova/-/tree/staging/nova-drm

Danilo Krummrich (4):
  rust: device: implement Device::parent()
  rust: device: implement bus_type_raw()
  rust: pci: impl TryFrom<&Device> for &pci::Device
  rust: platform: impl TryFrom<&Device> for &platform::Device

 rust/kernel/device.rs   | 19 +++++++++++++++++++
 rust/kernel/pci.rs      | 21 +++++++++++++++++++--
 rust/kernel/platform.rs | 22 ++++++++++++++++++++--
 3 files changed, 58 insertions(+), 4 deletions(-)

-- 
2.48.1

[PATCH 1/4] rust: device: implement Device::parent()

[Danilo Krummrich]

Device::parent() returns a reference to the device' parent device, if
any.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 21b343a1dc4d..76b341441f3f 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
         self.0.get()
     }
 
+    /// Returns a reference to the parent device, if any.
+    pub fn parent<'a>(&self) -> Option<&'a Self> {
+        // SAFETY: By the type invariant `self.as_raw()` is always valid.
+        let parent = unsafe { *self.as_raw() }.parent;
+
+        if parent.is_null() {
+            None
+        } else {
+            // SAFETY: Since `parent` is not NULL, it must be a valid pointer to a `struct device`.
+            Some(unsafe { Self::as_ref(parent) })
+        }
+    }
+
     /// Convert a raw C `struct device` pointer to a `&'a Device`.
     ///
     /// # Safety
-- 
2.48.1

Re: [PATCH 1/4] rust: device: implement Device::parent()

[Alice Ryhl]

On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote:
> Device::parent() returns a reference to the device' parent device, if
> any.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 21b343a1dc4d..76b341441f3f 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
>          self.0.get()
>      }
>  
> +    /// Returns a reference to the parent device, if any.
> +    pub fn parent<'a>(&self) -> Option<&'a Self> {
> +        // SAFETY: By the type invariant `self.as_raw()` is always valid.
> +        let parent = unsafe { *self.as_raw() }.parent;

This means:
1. Copy the entire `struct device` onto the stack.
2. Read the `parent` field of the copy.

Please write this instead to only read the `parent` field:
let parent = unsafe { *self.as_raw().parent };

> +        if parent.is_null() {
> +            None
> +        } else {
> +            // SAFETY: Since `parent` is not NULL, it must be a valid pointer to a `struct device`.
> +            Some(unsafe { Self::as_ref(parent) })
> +        }
> +    }
> +
>      /// Convert a raw C `struct device` pointer to a `&'a Device`.
>      ///
>      /// # Safety
> -- 
> 2.48.1
> 

Re: [PATCH 1/4] rust: device: implement Device::parent()

[Alice Ryhl]

On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote:
> > Device::parent() returns a reference to the device' parent device, if
> > any.
> >
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  rust/kernel/device.rs | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 21b343a1dc4d..76b341441f3f 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
> >          self.0.get()
> >      }
> >
> > +    /// Returns a reference to the parent device, if any.
> > +    pub fn parent<'a>(&self) -> Option<&'a Self> {
> > +        // SAFETY: By the type invariant `self.as_raw()` is always valid.
> > +        let parent = unsafe { *self.as_raw() }.parent;
>
> This means:
> 1. Copy the entire `struct device` onto the stack.
> 2. Read the `parent` field of the copy.
>
> Please write this instead to only read the `parent` field:
> let parent = unsafe { *self.as_raw().parent };

Sorry I meant (*self.as_raw()).parent

Re: [PATCH 1/4] rust: device: implement Device::parent()

[Danilo Krummrich]

On Thu, Mar 20, 2025 at 09:40:45AM +0100, Alice Ryhl wrote:
> On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote:
> > > Device::parent() returns a reference to the device' parent device, if
> > > any.
> > >
> > > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > > ---
> > >  rust/kernel/device.rs | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > > index 21b343a1dc4d..76b341441f3f 100644
> > > --- a/rust/kernel/device.rs
> > > +++ b/rust/kernel/device.rs
> > > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
> > >          self.0.get()
> > >      }
> > >
> > > +    /// Returns a reference to the parent device, if any.
> > > +    pub fn parent<'a>(&self) -> Option<&'a Self> {
> > > +        // SAFETY: By the type invariant `self.as_raw()` is always valid.
> > > +        let parent = unsafe { *self.as_raw() }.parent;
> >
> > This means:
> > 1. Copy the entire `struct device` onto the stack.
> > 2. Read the `parent` field of the copy.
> >
> > Please write this instead to only read the `parent` field:
> > let parent = unsafe { *self.as_raw().parent };
> 
> Sorry I meant (*self.as_raw()).parent

Good catch, thanks! 

Re: [PATCH 1/4] rust: device: implement Device::parent()

[Alice Ryhl]

On Thu, Mar 20, 2025 at 01:19:11PM +0100, Danilo Krummrich wrote:
> On Thu, Mar 20, 2025 at 09:40:45AM +0100, Alice Ryhl wrote:
> > On Thu, Mar 20, 2025 at 9:34 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > >
> > > On Wed, Mar 19, 2025 at 09:30:25PM +0100, Danilo Krummrich wrote:
> > > > Device::parent() returns a reference to the device' parent device, if
> > > > any.
> > > >
> > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > > > ---
> > > >  rust/kernel/device.rs | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > > > index 21b343a1dc4d..76b341441f3f 100644
> > > > --- a/rust/kernel/device.rs
> > > > +++ b/rust/kernel/device.rs
> > > > @@ -65,6 +65,19 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
> > > >          self.0.get()
> > > >      }
> > > >
> > > > +    /// Returns a reference to the parent device, if any.
> > > > +    pub fn parent<'a>(&self) -> Option<&'a Self> {
> > > > +        // SAFETY: By the type invariant `self.as_raw()` is always valid.
> > > > +        let parent = unsafe { *self.as_raw() }.parent;
> > >
> > > This means:
> > > 1. Copy the entire `struct device` onto the stack.
> > > 2. Read the `parent` field of the copy.
> > >
> > > Please write this instead to only read the `parent` field:
> > > let parent = unsafe { *self.as_raw().parent };
> > 
> > Sorry I meant (*self.as_raw()).parent
> 
> Good catch, thanks! 

With that fixed you may add
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH 2/4] rust: device: implement bus_type_raw()

[Danilo Krummrich]

Implement bus_type_raw(), which returns a raw pointer to the device'
struct bus_type.

This is useful for bus devices, to implement the following trait.

	impl TryFrom<&Device> for &pci::Device

With this a caller can try to get the bus specific device from a generic
device in a safe way. try_from() will only succeed if the generic
device' bus type pointer matches the pointer of the bus' type.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 76b341441f3f..e2de0efd4a27 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -78,6 +78,13 @@ pub fn parent<'a>(&self) -> Option<&'a Self> {
         }
     }
 
+    /// Returns a raw pointer to the device' bus type.
+    #[expect(unused)]
+    pub(crate) fn bus_type_raw(&self) -> *const bindings::bus_type {
+        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+        unsafe { (*self.as_raw()).bus }
+    }
+
     /// Convert a raw C `struct device` pointer to a `&'a Device`.
     ///
     /// # Safety
-- 
2.48.1

Re: [PATCH 2/4] rust: device: implement bus_type_raw()

[Alice Ryhl]

On Wed, Mar 19, 2025 at 09:30:26PM +0100, Danilo Krummrich wrote:
> Implement bus_type_raw(), which returns a raw pointer to the device'
> struct bus_type.
> 
> This is useful for bus devices, to implement the following trait.
> 
> 	impl TryFrom<&Device> for &pci::Device
> 
> With this a caller can try to get the bus specific device from a generic
> device in a safe way. try_from() will only succeed if the generic
> device' bus type pointer matches the pointer of the bus' type.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 76b341441f3f..e2de0efd4a27 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -78,6 +78,13 @@ pub fn parent<'a>(&self) -> Option<&'a Self> {
>          }
>      }
>  
> +    /// Returns a raw pointer to the device' bus type.
> +    #[expect(unused)]
> +    pub(crate) fn bus_type_raw(&self) -> *const bindings::bus_type {
> +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.

Is this field immutable?

> +        unsafe { (*self.as_raw()).bus }
> +    }
> +
>      /// Convert a raw C `struct device` pointer to a `&'a Device`.
>      ///
>      /// # Safety
> -- 
> 2.48.1
> 

Re: [PATCH 2/4] rust: device: implement bus_type_raw()

[Danilo Krummrich]

On Thu, Mar 20, 2025 at 08:36:02AM +0000, Alice Ryhl wrote:
> On Wed, Mar 19, 2025 at 09:30:26PM +0100, Danilo Krummrich wrote:
> > Implement bus_type_raw(), which returns a raw pointer to the device'
> > struct bus_type.
> > 
> > This is useful for bus devices, to implement the following trait.
> > 
> > 	impl TryFrom<&Device> for &pci::Device
> > 
> > With this a caller can try to get the bus specific device from a generic
> > device in a safe way. try_from() will only succeed if the generic
> > device' bus type pointer matches the pointer of the bus' type.
> > 
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  rust/kernel/device.rs | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 76b341441f3f..e2de0efd4a27 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -78,6 +78,13 @@ pub fn parent<'a>(&self) -> Option<&'a Self> {
> >          }
> >      }
> >  
> > +    /// Returns a raw pointer to the device' bus type.
> > +    #[expect(unused)]
> > +    pub(crate) fn bus_type_raw(&self) -> *const bindings::bus_type {
> > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> 
> Is this field immutable?

dev->bus is a pointer to a const struct bus_type, yes.

> 
> > +        unsafe { (*self.as_raw()).bus }
> > +    }
> > +
> >      /// Convert a raw C `struct device` pointer to a `&'a Device`.
> >      ///
> >      /// # Safety
> > -- 
> > 2.48.1
> > 

Re: [PATCH 2/4] rust: device: implement bus_type_raw()

[Alice Ryhl]

On Thu, Mar 20, 2025 at 01:00:23PM +0100, Danilo Krummrich wrote:
> On Thu, Mar 20, 2025 at 08:36:02AM +0000, Alice Ryhl wrote:
> > On Wed, Mar 19, 2025 at 09:30:26PM +0100, Danilo Krummrich wrote:
> > > Implement bus_type_raw(), which returns a raw pointer to the device'
> > > struct bus_type.
> > > 
> > > This is useful for bus devices, to implement the following trait.
> > > 
> > > 	impl TryFrom<&Device> for &pci::Device
> > > 
> > > With this a caller can try to get the bus specific device from a generic
> > > device in a safe way. try_from() will only succeed if the generic
> > > device' bus type pointer matches the pointer of the bus' type.
> > > 
> > > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > > ---
> > >  rust/kernel/device.rs | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > > index 76b341441f3f..e2de0efd4a27 100644
> > > --- a/rust/kernel/device.rs
> > > +++ b/rust/kernel/device.rs
> > > @@ -78,6 +78,13 @@ pub fn parent<'a>(&self) -> Option<&'a Self> {
> > >          }
> > >      }
> > >  
> > > +    /// Returns a raw pointer to the device' bus type.
> > > +    #[expect(unused)]
> > > +    pub(crate) fn bus_type_raw(&self) -> *const bindings::bus_type {
> > > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> > 
> > Is this field immutable?
> 
> dev->bus is a pointer to a const struct bus_type, yes.

With that added to the SAFETY comment to justify reading the field is
data-race free, you may add:

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH 3/4] rust: pci: impl TryFrom<&Device> for &pci::Device

[Danilo Krummrich]

Implement TryFrom<&device::Device> for &Device.

This allows us to get a &pci::Device from a generic &Device in a safe
way; the conversion fails if the device' bus type does not match with
the PCI bus type.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs |  1 -
 rust/kernel/pci.rs    | 21 +++++++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index e2de0efd4a27..34244c139afc 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -79,7 +79,6 @@ pub fn parent<'a>(&self) -> Option<&'a Self> {
     }
 
     /// Returns a raw pointer to the device' bus type.
-    #[expect(unused)]
     pub(crate) fn bus_type_raw(&self) -> *const bindings::bus_type {
         // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
         unsafe { (*self.as_raw()).bus }
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 22a32172b108..b717bcdb9abf 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -6,7 +6,7 @@
 
 use crate::{
     alloc::flags::*,
-    bindings, device,
+    bindings, container_of, device,
     device_id::RawDeviceId,
     devres::Devres,
     driver,
@@ -20,7 +20,7 @@
 use core::{
     marker::PhantomData,
     ops::Deref,
-    ptr::{addr_of_mut, NonNull},
+    ptr::{addr_of, addr_of_mut, NonNull},
 };
 use kernel::prelude::*;
 
@@ -466,6 +466,23 @@ fn as_ref(&self) -> &device::Device {
     }
 }
 
+impl TryFrom<&device::Device> for &Device {
+    type Error = kernel::error::Error;
+
+    fn try_from(dev: &device::Device) -> Result<Self, Self::Error> {
+        if dev.bus_type_raw() != addr_of!(bindings::pci_bus_type) {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We've just verified that the bus type of `dev` equals `bindings::pci_bus_type`,
+        // hence `dev` must be embedded in a valid `struct pci_dev`.
+        let pdev = unsafe { container_of!(dev.as_raw(), bindings::pci_dev, dev) };
+
+        // SAFETY: `pdev` is a valid pointer to a `struct pci_dev`.
+        Ok(unsafe { &*pdev.cast() })
+    }
+}
+
 // SAFETY: A `Device` is always reference-counted and can be released from any thread.
 unsafe impl Send for Device {}
 
-- 
2.48.1

Re: [PATCH 3/4] rust: pci: impl TryFrom<&Device> for &pci::Device

[Alice Ryhl]

On Wed, Mar 19, 2025 at 09:30:27PM +0100, Danilo Krummrich wrote:
> Implement TryFrom<&device::Device> for &Device.
> 
> This allows us to get a &pci::Device from a generic &Device in a safe
> way; the conversion fails if the device' bus type does not match with
> the PCI bus type.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH 4/4] rust: platform: impl TryFrom<&Device> for &platform::Device

[Danilo Krummrich]

Implement TryFrom<&device::Device> for &Device.

This allows us to get a &platform::Device from a generic &Device in a safe
way; the conversion fails if the device' bus type does not match with
the platform bus type.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/platform.rs | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index e37531bae8e9..c17fc6e7c596 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,7 +5,7 @@
 //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
 
 use crate::{
-    bindings, device, driver,
+    bindings, container_of, device, driver,
     error::{to_result, Result},
     of,
     prelude::*,
@@ -17,7 +17,7 @@
 use core::{
     marker::PhantomData,
     ops::Deref,
-    ptr::{addr_of_mut, NonNull},
+    ptr::{addr_of, addr_of_mut, NonNull},
 };
 
 /// An adapter for the registration of platform drivers.
@@ -234,6 +234,24 @@ fn as_ref(&self) -> &device::Device {
     }
 }
 
+impl TryFrom<&device::Device> for &Device {
+    type Error = kernel::error::Error;
+
+    fn try_from(dev: &device::Device) -> Result<Self, Self::Error> {
+        if dev.bus_type_raw() != addr_of!(bindings::platform_bus_type) {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We've just verified that the bus type of `dev` equals
+        // `bindings::platform_bus_type`, hence `dev` must be embedded in a valid
+        // `struct platform_device`.
+        let pdev = unsafe { container_of!(dev.as_raw(), bindings::platform_device, dev) };
+
+        // SAFETY: `pdev` is a valid pointer to a `struct platform_device`.
+        Ok(unsafe { &*pdev.cast() })
+    }
+}
+
 // SAFETY: A `Device` is always reference-counted and can be released from any thread.
 unsafe impl Send for Device {}
 
-- 
2.48.1

Re: [PATCH 4/4] rust: platform: impl TryFrom<&Device> for &platform::Device

[Alice Ryhl]

On Wed, Mar 19, 2025 at 09:30:28PM +0100, Danilo Krummrich wrote:
> Implement TryFrom<&device::Device> for &Device.
> 
> This allows us to get a &platform::Device from a generic &Device in a safe
> way; the conversion fails if the device' bus type does not match with
> the platform bus type.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>