[PATCH v2 0/9] Implement "Bound" device context

[PATCH v2 0/9] Implement "Bound" device context

[Danilo Krummrich]

Currently, we do not ensure that APIs that require a bound device instance can
only be called with a bound device.

Examples of such APIs are Devres, dma::CoherentAllocation and
pci::Device::iomap_region().

This patch series introduces the "Bound" device context such that we can ensure
to only ever pass a bound device to APIs that require this precondition.

In order to get there, we need some prerequisites:

(1) Implement macros to consistently derive Deref implementations for the
    different device contexts. For instance, Device<Core> can be dereferenced to
    Device<Bound>, since all device references we get from "core" bus callbacks
    are guaranteed to be from a bound device. Device<Bound> can always be
    dereferenced to Device (i.e. Device<Normal>), since the "Normal" device
    context has no specific requirements.

(2) Implement device context support for the generic Device type. Some APIs such
    as Devres and dma::CoherentAllocation work with generic devices.

(3) Preserve device context generics in bus specific device' AsRef
    implementation, such that we can derive the device context when converting
    from a bus specific device reference to a generic device reference.

With this, Devres::new(), for instance, can take a &Device<Bound> argument and
hence ensure that it can't be called with a Device reference that is not
guaranteed to be bound to a driver.

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

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/device-bound

Changes in v2:
  - add a safety requirement for impl_device_context_deref! (thanks to Benno for
    working this out)

Danilo Krummrich (9):
  rust: device: implement impl_device_context_deref!
  rust: device: implement impl_device_context_into_aref!
  rust: device: implement device context for Device
  rust: platform: preserve device context in AsRef
  rust: pci: preserve device context in AsRef
  rust: device: implement Bound device context
  rust: pci: move iomap_region() to impl Device<Bound>
  rust: devres: require a bound device
  rust: dma: require a bound device

 rust/kernel/device.rs   | 90 ++++++++++++++++++++++++++++++++++++++++-
 rust/kernel/devres.rs   | 17 ++++----
 rust/kernel/dma.rs      | 14 +++----
 rust/kernel/pci.rs      | 33 +++++----------
 rust/kernel/platform.rs | 32 ++++-----------
 5 files changed, 121 insertions(+), 65 deletions(-)


base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
-- 
2.49.0

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

[Danilo Krummrich]

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

[PATCH v2 2/9] rust: device: implement impl_device_context_into_aref!

[Danilo Krummrich]

Implement a macro to implement all From conversions of a certain device
to ARef<Device>.

This avoids unnecessary boiler plate code for every device
implementation.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/device.rs   | 21 +++++++++++++++++++++
 rust/kernel/pci.rs      |  7 +------
 rust/kernel/platform.rs |  9 ++-------
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 7cb6f0fc005d..26e71224460b 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -279,6 +279,27 @@ macro_rules! impl_device_context_deref {
     };
 }
 
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __impl_device_context_into_aref {
+    ($src:ty, $device:tt) => {
+        impl core::convert::From<&$device<$src>> for $crate::types::ARef<$device> {
+            fn from(dev: &$device<$src>) -> Self {
+                (&**dev).into()
+            }
+        }
+    };
+}
+
+/// Implement [`core::convert::From`], such that all `&Device<Ctx>` can be converted to an
+/// `ARef<Device>`.
+#[macro_export]
+macro_rules! impl_device_context_into_aref {
+    ($device:tt) => {
+        kernel::__impl_device_context_into_aref!($crate::device::Core, $device);
+    };
+}
+
 #[doc(hidden)]
 #[macro_export]
 macro_rules! dev_printk {
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 8474608e7a90..7c6ec05383e0 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -425,12 +425,7 @@ pub fn set_master(&self) {
 // 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 {
-        (&**dev).into()
-    }
-}
+kernel::impl_device_context_into_aref!(Device);
 
 // SAFETY: Instances of `Device` are always reference-counted.
 unsafe impl crate::types::AlwaysRefCounted for Device {
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 22590bdff7bb..9476b717c425 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -10,7 +10,7 @@
     of,
     prelude::*,
     str::CStr,
-    types::{ARef, ForeignOwnable, Opaque},
+    types::{ForeignOwnable, Opaque},
     ThisModule,
 };
 
@@ -192,12 +192,7 @@ fn as_raw(&self) -> *mut bindings::platform_device {
 // 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 {
-        (&**dev).into()
-    }
-}
+kernel::impl_device_context_into_aref!(Device);
 
 // SAFETY: Instances of `Device` are always reference-counted.
 unsafe impl crate::types::AlwaysRefCounted for Device {
-- 
2.49.0

[PATCH v2 3/9] rust: device: implement device context for Device

[Danilo Krummrich]

Analogous to bus specific device, implement the DeviceContext generic
for generic devices.

This is used for APIs that work with generic devices (such as Devres) to
evaluate the device' context.

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

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 26e71224460b..487211842f77 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -9,7 +9,7 @@
     str::CStr,
     types::{ARef, Opaque},
 };
-use core::{fmt, ptr};
+use core::{fmt, marker::PhantomData, ptr};
 
 #[cfg(CONFIG_PRINTK)]
 use crate::c_str;
@@ -42,7 +42,7 @@
 /// `bindings::device::release` is valid to be called from any thread, hence `ARef<Device>` can be
 /// dropped from any thread.
 #[repr(transparent)]
-pub struct Device(Opaque<bindings::device>);
+pub struct Device<Ctx: DeviceContext = Normal>(Opaque<bindings::device>, PhantomData<Ctx>);
 
 impl Device {
     /// Creates a new reference-counted abstraction instance of an existing `struct device` pointer.
@@ -59,7 +59,9 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef<Self> {
         // SAFETY: By the safety requirements ptr is valid
         unsafe { Self::as_ref(ptr) }.into()
     }
+}
 
+impl<Ctx: DeviceContext> Device<Ctx> {
     /// Obtain the raw `struct device *`.
     pub(crate) fn as_raw(&self) -> *mut bindings::device {
         self.0.get()
@@ -189,6 +191,11 @@ pub fn property_present(&self, name: &CStr) -> bool {
     }
 }
 
+// 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 });
+kernel::impl_device_context_into_aref!(Device);
+
 // SAFETY: Instances of `Device` are always reference-counted.
 unsafe impl crate::types::AlwaysRefCounted for Device {
     fn inc_ref(&self) {
-- 
2.49.0

[PATCH v2 4/9] rust: platform: preserve device context in AsRef

[Danilo Krummrich]

Since device::Device has a generic over its context, preserve this
device context in AsRef.

For instance, when calling platform::Device<Core> the new AsRef
implementation returns device::Device<Core>.

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

diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 9476b717c425..b1c48cd95cd6 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -183,7 +183,7 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
     PhantomData<Ctx>,
 );
 
-impl Device {
+impl<Ctx: device::DeviceContext> Device<Ctx> {
     fn as_raw(&self) -> *mut bindings::platform_device {
         self.0.get()
     }
@@ -207,8 +207,8 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl AsRef<device::Device> for Device {
-    fn as_ref(&self) -> &device::Device {
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
         // `struct platform_device`.
         let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
-- 
2.49.0

[PATCH v2 5/9] rust: pci: preserve device context in AsRef

[Danilo Krummrich]

Since device::Device has a generic over its context, preserve this
device context in AsRef.

For instance, when calling pci::Device<Core> the new AsRef implementation
returns device::Device<Core>.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci.rs | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 7c6ec05383e0..1234b0c4a403 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -360,11 +360,13 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl Device {
+impl<Ctx: device::DeviceContext> Device<Ctx> {
     fn as_raw(&self) -> *mut bindings::pci_dev {
         self.0.get()
     }
+}
 
+impl Device {
     /// Returns the PCI vendor ID.
     pub fn vendor_id(&self) -> u16 {
         // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
@@ -440,8 +442,8 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl AsRef<device::Device> for Device {
-    fn as_ref(&self) -> &device::Device {
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
         // `struct pci_dev`.
         let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
-- 
2.49.0

[PATCH v2 6/9] rust: device: implement Bound device context

[Danilo Krummrich]

The Bound device context indicates that a device is bound to a driver.
It must be used for APIs that require the device to be bound, such as
Devres or dma::CoherentAllocation.

Implement Bound and add the corresponding Deref hierarchy, as well as the
corresponding ARef conversion for this device context.

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

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 487211842f77..585a3fcfeea3 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {}
 /// any of the bus callbacks, such as `probe()`.
 pub struct Core;
 
+/// The [`Bound`] context is the context of a bus specific device reference when it is guranteed to
+/// be bound for the duration of its lifetime.
+pub struct Bound;
+
 mod private {
     pub trait Sealed {}
 
+    impl Sealed for super::Bound {}
     impl Sealed for super::Core {}
     impl Sealed for super::Normal {}
 }
 
+impl DeviceContext for Bound {}
 impl DeviceContext for Core {}
 impl DeviceContext for Normal {}
 
@@ -281,7 +287,14 @@ macro_rules! impl_device_context_deref {
         // `__impl_device_context_deref!`.
         kernel::__impl_device_context_deref!(unsafe {
             $device,
-            $crate::device::Core => $crate::device::Normal
+            $crate::device::Core => $crate::device::Bound
+        });
+
+        // SAFETY: This macro has the exact same safety requirement as
+        // `__impl_device_context_deref!`.
+        kernel::__impl_device_context_deref!(unsafe {
+            $device,
+            $crate::device::Bound => $crate::device::Normal
         });
     };
 }
@@ -304,6 +317,7 @@ fn from(dev: &$device<$src>) -> Self {
 macro_rules! impl_device_context_into_aref {
     ($device:tt) => {
         kernel::__impl_device_context_into_aref!($crate::device::Core, $device);
+        kernel::__impl_device_context_into_aref!($crate::device::Bound, $device);
     };
 }
 
-- 
2.49.0

[PATCH v2 7/9] rust: pci: move iomap_region() to impl Device<Bound>

[Danilo Krummrich]

Require the Bound device context to be able to call iomap_region() and
iomap_region_sized(). Creating I/O mapping requires the device to be
bound.

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

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 1234b0c4a403..3664d35b8e79 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -390,7 +390,9 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
         // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`.
         Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
     }
+}
 
+impl Device<device::Bound> {
     /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks
     /// can be performed on compile time for offsets (plus the requested type size) < SIZE.
     pub fn iomap_region_sized<const SIZE: usize>(
-- 
2.49.0

[PATCH v2 8/9] rust: devres: require a bound device

[Danilo Krummrich]

Require the Bound device context to be able to a new Devres container.
This ensures that we can't register devres callbacks for unbound
devices.

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

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index ddb1ce4a78d9..1e58f5d22044 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -8,7 +8,7 @@
 use crate::{
     alloc::Flags,
     bindings,
-    device::Device,
+    device::{Bound, Device},
     error::{Error, Result},
     ffi::c_void,
     prelude::*,
@@ -45,7 +45,7 @@ struct DevresInner<T> {
 /// # Example
 ///
 /// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, c_str, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}};
 /// # use core::ops::Deref;
 ///
 /// // See also [`pci::Bar`] for a real example.
@@ -83,13 +83,10 @@ struct DevresInner<T> {
 ///         unsafe { Io::from_raw(&self.0) }
 ///    }
 /// }
-/// # fn no_run() -> Result<(), Error> {
-/// # // SAFETY: Invalid usage; just for the example to get an `ARef<Device>` instance.
-/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) };
-///
+/// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
 /// // SAFETY: Invalid usage for example purposes.
 /// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
-/// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?;
+/// let devres = Devres::new(dev, iomem, GFP_KERNEL)?;
 ///
 /// let res = devres.try_access().ok_or(ENXIO)?;
 /// res.write8(0x42, 0x0);
@@ -99,7 +96,7 @@ struct DevresInner<T> {
 pub struct Devres<T>(Arc<DevresInner<T>>);
 
 impl<T> DevresInner<T> {
-    fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
+    fn new(dev: &Device<Bound>, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
         let inner = Arc::pin_init(
             pin_init!( DevresInner {
                 dev: dev.into(),
@@ -171,7 +168,7 @@ fn remove_action(this: &Arc<Self>) {
 impl<T> Devres<T> {
     /// Creates a new [`Devres`] instance of the given `data`. The `data` encapsulated within the
     /// returned `Devres` instance' `data` will be revoked once the device is detached.
-    pub fn new(dev: &Device, data: T, flags: Flags) -> Result<Self> {
+    pub fn new(dev: &Device<Bound>, data: T, flags: Flags) -> Result<Self> {
         let inner = DevresInner::new(dev, data, flags)?;
 
         Ok(Devres(inner))
@@ -179,7 +176,7 @@ pub fn new(dev: &Device, data: T, flags: Flags) -> Result<Self> {
 
     /// Same as [`Devres::new`], but does not return a `Devres` instance. Instead the given `data`
     /// is owned by devres and will be revoked / dropped, once the device is detached.
-    pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result {
+    pub fn new_foreign_owned(dev: &Device<Bound>, data: T, flags: Flags) -> Result {
         let _ = DevresInner::new(dev, data, flags)?;
 
         Ok(())
-- 
2.49.0

[PATCH v2 9/9] rust: dma: require a bound device

[Danilo Krummrich]

Require the Bound device context to be able to create new
dma::CoherentAllocation instances.

DMA memory allocations are only valid to be created for bound devices.

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

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 8cdc76043ee7..605e01e35715 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -6,7 +6,7 @@
 
 use crate::{
     bindings, build_assert,
-    device::Device,
+    device::{Bound, Device},
     error::code::*,
     error::Result,
     transmute::{AsBytes, FromBytes},
@@ -22,10 +22,10 @@
 /// # Examples
 ///
 /// ```
-/// use kernel::device::Device;
+/// # use kernel::device::{Bound, Device};
 /// use kernel::dma::{attrs::*, CoherentAllocation};
 ///
-/// # fn test(dev: &Device) -> Result {
+/// # fn test(dev: &Device<Bound>) -> Result {
 /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN;
 /// let c: CoherentAllocation<u64> =
 ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?;
@@ -143,16 +143,16 @@ impl<T: AsBytes + FromBytes> CoherentAllocation<T> {
     /// # Examples
     ///
     /// ```
-    /// use kernel::device::Device;
+    /// # use kernel::device::{Bound, Device};
     /// use kernel::dma::{attrs::*, CoherentAllocation};
     ///
-    /// # fn test(dev: &Device) -> Result {
+    /// # fn test(dev: &Device<Bound>) -> Result {
     /// let c: CoherentAllocation<u64> =
     ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
     /// # Ok::<(), Error>(()) }
     /// ```
     pub fn alloc_attrs(
-        dev: &Device,
+        dev: &Device<Bound>,
         count: usize,
         gfp_flags: kernel::alloc::Flags,
         dma_attrs: Attrs,
@@ -194,7 +194,7 @@ pub fn alloc_attrs(
     /// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the
     /// `dma_attrs` is 0 by default.
     pub fn alloc_coherent(
-        dev: &Device,
+        dev: &Device<Bound>,
         count: usize,
         gfp_flags: kernel::alloc::Flags,
     ) -> Result<CoherentAllocation<T>> {
-- 
2.49.0

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

[Christian Schrefl]

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

Re: [PATCH v2 2/9] rust: device: implement impl_device_context_into_aref!

[Benno Lossin]

On Sun Apr 13, 2025 at 7:36 PM CEST, Danilo Krummrich wrote:
> Implement a macro to implement all From conversions of a certain device
> to ARef<Device>.
>
> This avoids unnecessary boiler plate code for every device
> implementation.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

One nit below, with that fixed:

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
>  rust/kernel/device.rs   | 21 +++++++++++++++++++++
>  rust/kernel/pci.rs      |  7 +------
>  rust/kernel/platform.rs |  9 ++-------
>  3 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 7cb6f0fc005d..26e71224460b 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -279,6 +279,27 @@ macro_rules! impl_device_context_deref {
>      };
>  }
>  
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __impl_device_context_into_aref {
> +    ($src:ty, $device:tt) => {
> +        impl core::convert::From<&$device<$src>> for $crate::types::ARef<$device> {

Missing `::` in front of `core`.

> +            fn from(dev: &$device<$src>) -> Self {
> +                (&**dev).into()
> +            }
> +        }
> +    };
> +}
> +
> +/// Implement [`core::convert::From`], such that all `&Device<Ctx>` can be converted to an
> +/// `ARef<Device>`.
> +#[macro_export]
> +macro_rules! impl_device_context_into_aref {
> +    ($device:tt) => {
> +        kernel::__impl_device_context_into_aref!($crate::device::Core, $device);
> +    };
> +}
> +
>  #[doc(hidden)]
>  #[macro_export]
>  macro_rules! dev_printk {

Re: [PATCH v2 3/9] rust: device: implement device context for Device

[Benno Lossin]

On Sun Apr 13, 2025 at 7:36 PM CEST, Danilo Krummrich wrote:
> Analogous to bus specific device, implement the DeviceContext generic
> for generic devices.
>
> This is used for APIs that work with generic devices (such as Devres) to
> evaluate the device' context.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> ---
>  rust/kernel/device.rs | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Re: [PATCH v2 4/9] rust: platform: preserve device context in AsRef

[Benno Lossin]

On Sun Apr 13, 2025 at 7:36 PM CEST, Danilo Krummrich wrote:
> Since device::Device has a generic over its context, preserve this
> device context in AsRef.
>
> For instance, when calling platform::Device<Core> the new AsRef
> implementation returns device::Device<Core>.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> ---
>  rust/kernel/platform.rs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Re: [PATCH v2 5/9] rust: pci: preserve device context in AsRef

[Benno Lossin]

On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> Since device::Device has a generic over its context, preserve this
> device context in AsRef.
>
> For instance, when calling pci::Device<Core> the new AsRef implementation
> returns device::Device<Core>.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> ---
>  rust/kernel/pci.rs | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

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

[Benno Lossin]

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 6/9] rust: device: implement Bound device context

[Benno Lossin]

On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> The Bound device context indicates that a device is bound to a driver.
> It must be used for APIs that require the device to be bound, such as
> Devres or dma::CoherentAllocation.
>
> Implement Bound and add the corresponding Deref hierarchy, as well as the
> corresponding ARef conversion for this device context.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

One suggestion below, feel free to make it its own patch or fold it into
the correct ones. Also two `::` nits below, with those fixed:

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
>  rust/kernel/device.rs | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 487211842f77..585a3fcfeea3 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs

> @@ -281,7 +287,14 @@ macro_rules! impl_device_context_deref {
>          // `__impl_device_context_deref!`.
>          kernel::__impl_device_context_deref!(unsafe {
>              $device,
> -            $crate::device::Core => $crate::device::Normal
> +            $crate::device::Core => $crate::device::Bound
> +        });
> +
> +        // SAFETY: This macro has the exact same safety requirement as
> +        // `__impl_device_context_deref!`.
> +        kernel::__impl_device_context_deref!(unsafe {

Missing `::`.

> +            $device,
> +            $crate::device::Bound => $crate::device::Normal

IIUC, all "devices" (so eg `pci::Device`) will use this macro, right? In
that case, I think we can document this behavior a bit better, possibly
on the `DeviceContext` context trait and/or on the different type
states. So on `Core` we could say "The `Core` context is a supercontext
of the [`Bound`] context and devices also expose operations available in
that context while in `Core`." and similarly on `Bound` with `Normal`.

>          });
>      };
>  }
> @@ -304,6 +317,7 @@ fn from(dev: &$device<$src>) -> Self {
>  macro_rules! impl_device_context_into_aref {
>      ($device:tt) => {
>          kernel::__impl_device_context_into_aref!($crate::device::Core, $device);
> +        kernel::__impl_device_context_into_aref!($crate::device::Bound, $device);

Missing `::`.

---
Cheers,
Benno

>      };
>  }
>  

Re: [PATCH v2 7/9] rust: pci: move iomap_region() to impl Device<Bound>

[Benno Lossin]

On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> Require the Bound device context to be able to call iomap_region() and
> iomap_region_sized(). Creating I/O mapping requires the device to be
> bound.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> ---
>  rust/kernel/pci.rs | 2 ++
>  1 file changed, 2 insertions(+)

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Benno Lossin]

On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> The Bound device context indicates that a device is bound to a driver.
> It must be used for APIs that require the device to be bound, such as
> Devres or dma::CoherentAllocation.
>
> Implement Bound and add the corresponding Deref hierarchy, as well as the
> corresponding ARef conversion for this device context.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 487211842f77..585a3fcfeea3 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {}
>  /// any of the bus callbacks, such as `probe()`.
>  pub struct Core;
>  
> +/// The [`Bound`] context is the context of a bus specific device reference when it is guranteed to
> +/// be bound for the duration of its lifetime.
> +pub struct Bound;

One question about this: is it possible for me to
1. have access to a `ARef<Device<Bound>>` (or `Core`) via some callback,
2. store a clone of the `ARef` in some datastructure,
3. wait for the device to become unbound,
4. use a `Bound`-only context function and blow something up?

Depending on the severity of the "blow something up" we probably need to
change the design. If it's "only a bug" (and not a memory
vulnerability), then this is fine, since people should then "just not do
that" (and I think this design makes that painfully obvious when someone
tries to do something funny with a `Device<Bound>`).

---
Cheers,
Benno

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Danilo Krummrich]

On Mon, Apr 14, 2025 at 10:49:49AM +0000, Benno Lossin wrote:
> On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> > The Bound device context indicates that a device is bound to a driver.
> > It must be used for APIs that require the device to be bound, such as
> > Devres or dma::CoherentAllocation.
> >
> > Implement Bound and add the corresponding Deref hierarchy, as well as the
> > corresponding ARef conversion for this device context.
> >
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  rust/kernel/device.rs | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 487211842f77..585a3fcfeea3 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {}
> >  /// any of the bus callbacks, such as `probe()`.
> >  pub struct Core;
> >  
> > +/// The [`Bound`] context is the context of a bus specific device reference when it is guranteed to
> > +/// be bound for the duration of its lifetime.
> > +pub struct Bound;
> 
> One question about this: is it possible for me to
> 1. have access to a `ARef<Device<Bound>>` (or `Core`) via some callback,
> 2. store a clone of the `ARef` in some datastructure,
> 3. wait for the device to become unbound,
> 4. use a `Bound`-only context function and blow something up?

You can never get an ARef<Device> that has a different device context than
Normal.

A device must only ever implement AlwaysRefCounted for Device (i.e.
Device<Normal>).

This is why patch 2 ("rust: device: implement impl_device_context_into_aref!")
implements conversions from Device<Ctx> to ARef<Device>.

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Benno Lossin]

On Mon Apr 14, 2025 at 12:56 PM CEST, Danilo Krummrich wrote:
> On Mon, Apr 14, 2025 at 10:49:49AM +0000, Benno Lossin wrote:
>> On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
>> > The Bound device context indicates that a device is bound to a driver.
>> > It must be used for APIs that require the device to be bound, such as
>> > Devres or dma::CoherentAllocation.
>> >
>> > Implement Bound and add the corresponding Deref hierarchy, as well as the
>> > corresponding ARef conversion for this device context.
>> >
>> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> > ---
>> >  rust/kernel/device.rs | 16 +++++++++++++++-
>> >  1 file changed, 15 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
>> > index 487211842f77..585a3fcfeea3 100644
>> > --- a/rust/kernel/device.rs
>> > +++ b/rust/kernel/device.rs
>> > @@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {}
>> >  /// any of the bus callbacks, such as `probe()`.
>> >  pub struct Core;
>> >  
>> > +/// The [`Bound`] context is the context of a bus specific device reference when it is guranteed to
>> > +/// be bound for the duration of its lifetime.
>> > +pub struct Bound;
>> 
>> One question about this: is it possible for me to
>> 1. have access to a `ARef<Device<Bound>>` (or `Core`) via some callback,
>> 2. store a clone of the `ARef` in some datastructure,
>> 3. wait for the device to become unbound,
>> 4. use a `Bound`-only context function and blow something up?
>
> You can never get an ARef<Device> that has a different device context than
> Normal.
>
> A device must only ever implement AlwaysRefCounted for Device (i.e.
> Device<Normal>).

Ah yes, I thought there was something there, that's perfect. Might also
be a good idea to write this down before it becomes tribal knowledge :)

---
Cheers,
Benno

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Danilo Krummrich]

On Mon, Apr 14, 2025 at 10:44:35AM +0000, Benno Lossin wrote:
> On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
> 
> > +            $device,
> > +            $crate::device::Bound => $crate::device::Normal
> 
> IIUC, all "devices" (so eg `pci::Device`) will use this macro, right? In
> that case, I think we can document this behavior a bit better, possibly
> on the `DeviceContext` context trait and/or on the different type
> states. So on `Core` we could say "The `Core` context is a supercontext
> of the [`Bound`] context and devices also expose operations available in
> that context while in `Core`." and similarly on `Bound` with `Normal`.

Fully agree, I absolutely want to have this documented. I wasn't yet sure where
I want to document this though. device::DeviceContext seems to be a reasonable
place.

Besides that, I think I also want to rename it to device::Context, not sure if
it's worth though.

I will send a subsequent patch for the documentation on DeviceContext.

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Benno Lossin]

On Mon Apr 14, 2025 at 1:13 PM CEST, Danilo Krummrich wrote:
> On Mon, Apr 14, 2025 at 10:44:35AM +0000, Benno Lossin wrote:
>> On Sun Apr 13, 2025 at 7:37 PM CEST, Danilo Krummrich wrote:
>> 
>> > +            $device,
>> > +            $crate::device::Bound => $crate::device::Normal
>> 
>> IIUC, all "devices" (so eg `pci::Device`) will use this macro, right? In
>> that case, I think we can document this behavior a bit better, possibly
>> on the `DeviceContext` context trait and/or on the different type
>> states. So on `Core` we could say "The `Core` context is a supercontext
>> of the [`Bound`] context and devices also expose operations available in
>> that context while in `Core`." and similarly on `Bound` with `Normal`.
>
> Fully agree, I absolutely want to have this documented. I wasn't yet sure where
> I want to document this though. device::DeviceContext seems to be a reasonable
> place.
>
> Besides that, I think I also want to rename it to device::Context, not sure if
> it's worth though.

Sounds reasonable to me.

---
Cheers,
Benno

Re: [PATCH v2 7/9] rust: pci: move iomap_region() to impl Device<Bound>

[Bjorn Helgaas]

On Sun, Apr 13, 2025 at 07:37:02PM +0200, Danilo Krummrich wrote:
> Require the Bound device context to be able to call iomap_region() and
> iomap_region_sized(). Creating I/O mapping requires the device to be
> bound.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/pci.rs | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 1234b0c4a403..3664d35b8e79 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -390,7 +390,9 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
>          // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`.
>          Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
>      }
> +}
>  
> +impl Device<device::Bound> {
>      /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks
>      /// can be performed on compile time for offsets (plus the requested type size) < SIZE.

You didn't add these typos here, but

s/Mapps/Maps/
s/on compile time/at compile time/

>      pub fn iomap_region_sized<const SIZE: usize>(
> -- 
> 2.49.0
> 

Re: [PATCH v2 6/9] rust: device: implement Bound device context

[Bjorn Helgaas]

On Sun, Apr 13, 2025 at 07:37:01PM +0200, Danilo Krummrich wrote:
> The Bound device context indicates that a device is bound to a driver.
> It must be used for APIs that require the device to be bound, such as
> Devres or dma::CoherentAllocation.
> 
> Implement Bound and add the corresponding Deref hierarchy, as well as the
> corresponding ARef conversion for this device context.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/device.rs | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 487211842f77..585a3fcfeea3 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {}
>  /// any of the bus callbacks, such as `probe()`.
>  pub struct Core;
>  
> +/// The [`Bound`] context is the context of a bus specific device reference when it is guranteed to
> +/// be bound for the duration of its lifetime.

s/guranteed/guaranteed/

Re: [PATCH v2 3/9] rust: device: implement device context for Device

[Bjorn Helgaas]

On Sun, Apr 13, 2025 at 07:36:58PM +0200, Danilo Krummrich wrote:
> Analogous to bus specific device, implement the DeviceContext generic
> for generic devices.
> 
> This is used for APIs that work with generic devices (such as Devres) to
> evaluate the device' context.

Looks like a stray ', but maybe it's some rust thing I don't know
about.

Re: [PATCH v2 0/9] Implement "Bound" device context

[Danilo Krummrich]

On Sun, Apr 13, 2025 at 07:36:55PM +0200, Danilo Krummrich wrote:
> Currently, we do not ensure that APIs that require a bound device instance can
> only be called with a bound device.
> 
> Examples of such APIs are Devres, dma::CoherentAllocation and
> pci::Device::iomap_region().
> 
> This patch series introduces the "Bound" device context such that we can ensure
> to only ever pass a bound device to APIs that require this precondition.
> 
> In order to get there, we need some prerequisites:
> 
> (1) Implement macros to consistently derive Deref implementations for the
>     different device contexts. For instance, Device<Core> can be dereferenced to
>     Device<Bound>, since all device references we get from "core" bus callbacks
>     are guaranteed to be from a bound device. Device<Bound> can always be
>     dereferenced to Device (i.e. Device<Normal>), since the "Normal" device
>     context has no specific requirements.
> 
> (2) Implement device context support for the generic Device type. Some APIs such
>     as Devres and dma::CoherentAllocation work with generic devices.
> 
> (3) Preserve device context generics in bus specific device' AsRef
>     implementation, such that we can derive the device context when converting
>     from a bus specific device reference to a generic device reference.
> 
> With this, Devres::new(), for instance, can take a &Device<Bound> argument and
> hence ensure that it can't be called with a Device reference that is not
> guaranteed to be bound to a driver.
> 
> A branch containing the patches can be found in [1].
> 
> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/device-bound

With the following changes, applied to driver-core/topic/device-context, thanks!

  *  Add missing `::` prefix in macros.
  *  Fix typos pointed out by Bjorn.

- Danilo