[PATCH v2 1/9] rust: device: implement impl_device_context_deref!

Danilo Krummrich posted 9 patches 8 months, 1 week ago
[PATCH v2 1/9] rust: device: implement impl_device_context_deref!
Posted by Danilo Krummrich 8 months, 1 week ago
The Deref hierarchy for device context generics is the same for every
(bus specific) device.

Implement those with a generic macro to avoid duplicated boiler plate
code and ensure the correct Deref hierarchy for every device
implementation.

Co-developed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs   | 44 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/pci.rs      | 16 +++------------
 rust/kernel/platform.rs | 17 +++-------------
 3 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 21b343a1dc4d..7cb6f0fc005d 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -235,6 +235,50 @@ impl Sealed for super::Normal {}
 impl DeviceContext for Core {}
 impl DeviceContext for Normal {}
 
+/// # Safety
+///
+/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the
+/// generic argument of `$device`.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __impl_device_context_deref {
+    (unsafe { $device:ident, $src:ty => $dst:ty }) => {
+        impl ::core::ops::Deref for $device<$src> {
+            type Target = $device<$dst>;
+
+            fn deref(&self) -> &Self::Target {
+                let ptr: *const Self = self;
+
+                // CAST: `$device<$src>` and `$device<$dst>` transparently wrap the same type by the
+                // safety requirement of the macro.
+                let ptr = ptr.cast::<Self::Target>();
+
+                // SAFETY: `ptr` was derived from `&self`.
+                unsafe { &*ptr }
+            }
+        }
+    };
+}
+
+/// Implement [`core::ops::Deref`] traits for allowed [`DeviceContext`] conversions of a (bus
+/// specific) device.
+///
+/// # Safety
+///
+/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the
+/// generic argument of `$device`.
+#[macro_export]
+macro_rules! impl_device_context_deref {
+    (unsafe { $device:ident }) => {
+        // SAFETY: This macro has the exact same safety requirement as
+        // `__impl_device_context_deref!`.
+        kernel::__impl_device_context_deref!(unsafe {
+            $device,
+            $crate::device::Core => $crate::device::Normal
+        });
+    };
+}
+
 #[doc(hidden)]
 #[macro_export]
 macro_rules! dev_printk {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c97d6d470b28..8474608e7a90 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -422,19 +422,9 @@ pub fn set_master(&self) {
     }
 }
 
-impl Deref for Device<device::Core> {
-    type Target = Device;
-
-    fn deref(&self) -> &Self::Target {
-        let ptr: *const Self = self;
-
-        // CAST: `Device<Ctx>` is a transparent wrapper of `Opaque<bindings::pci_dev>`.
-        let ptr = ptr.cast::<Device>();
-
-        // SAFETY: `ptr` was derived from `&self`.
-        unsafe { &*ptr }
-    }
-}
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
 
 impl From<&Device<device::Core>> for ARef<Device> {
     fn from(dev: &Device<device::Core>) -> Self {
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 4917cb34e2fe..22590bdff7bb 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -16,7 +16,6 @@
 
 use core::{
     marker::PhantomData,
-    ops::Deref,
     ptr::{addr_of_mut, NonNull},
 };
 
@@ -190,19 +189,9 @@ fn as_raw(&self) -> *mut bindings::platform_device {
     }
 }
 
-impl Deref for Device<device::Core> {
-    type Target = Device;
-
-    fn deref(&self) -> &Self::Target {
-        let ptr: *const Self = self;
-
-        // CAST: `Device<Ctx>` is a transparent wrapper of `Opaque<bindings::platform_device>`.
-        let ptr = ptr.cast::<Device>();
-
-        // SAFETY: `ptr` was derived from `&self`.
-        unsafe { &*ptr }
-    }
-}
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
 
 impl From<&Device<device::Core>> for ARef<Device> {
     fn from(dev: &Device<device::Core>) -> Self {
-- 
2.49.0
Re: [PATCH v2 1/9] rust: device: implement impl_device_context_deref!
Posted by Benno Lossin 8 months, 1 week ago
On Sun Apr 13, 2025 at 7:36 PM CEST, Danilo Krummrich wrote:
> +/// Implement [`core::ops::Deref`] traits for allowed [`DeviceContext`] conversions of a (bus
> +/// specific) device.
> +///
> +/// # Safety
> +///
> +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the
> +/// generic argument of `$device`.
> +#[macro_export]
> +macro_rules! impl_device_context_deref {
> +    (unsafe { $device:ident }) => {
> +        // SAFETY: This macro has the exact same safety requirement as
> +        // `__impl_device_context_deref!`.
> +        kernel::__impl_device_context_deref!(unsafe {

Missing `::` in front of `kernel`.

---
Cheers,
Benno

> +            $device,
> +            $crate::device::Core => $crate::device::Normal
> +        });
> +    };
> +}
> +
>  #[doc(hidden)]
>  #[macro_export]
>  macro_rules! dev_printk {
Re: [PATCH v2 1/9] rust: device: implement impl_device_context_deref!
Posted by Christian Schrefl 8 months, 1 week ago
On 13.04.25 7:36 PM, Danilo Krummrich wrote:
> The Deref hierarchy for device context generics is the same for every
> (bus specific) device.
> 
> Implement those with a generic macro to avoid duplicated boiler plate
> code and ensure the correct Deref hierarchy for every device
> implementation.
> 
> Co-developed-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs   | 44 +++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/pci.rs      | 16 +++------------
>  rust/kernel/platform.rs | 17 +++-------------
>  3 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 21b343a1dc4d..7cb6f0fc005d 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -235,6 +235,50 @@ impl Sealed for super::Normal {}
>  impl DeviceContext for Core {}
>  impl DeviceContext for Normal {}
>  
> +/// # Safety
> +///
> +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the
> +/// generic argument of `$device`.
Maybe explicitly mention that the memory layout/representation 
therefore doesn't depend on the generic arguments.

Either way:

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Cheers
Christian