[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DDMDBPDZHN6G.KI90E7ZWWX39@kernel.org>
Date: Sun, 19 Oct 2025 16:28:40 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Markus Probst" <markus.probst@...teo.de>
Cc: "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>, "Miguel Ojeda"
<ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Rafael J.
Wysocki" <rafael@...nel.org>, "Igor Korotin"
<igor.korotin.linux@...il.com>, "Lee Jones" <lee@...nel.org>, "Pavel
Machek" <pavel@...nel.org>, "Dave Ertman" <david.m.ertman@...el.com>, "Ira
Weiny" <ira.weiny@...el.com>, "Leon Romanovsky" <leon@...nel.org>, "Boqun
Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
<bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "Daniel Almeida"
<daniel.almeida@...labora.com>, "Bjorn Helgaas" <bhelgaas@...gle.com>,
<kwilczynski@...nel.org>, <linux-kernel@...r.kernel.org>,
<rust-for-linux@...r.kernel.org>, <linux-pci@...r.kernel.org>,
<linux-leds@...r.kernel.org>
Subject: Re: [PATCH v5 1/2] rust: Add trait to convert a device reference to
a bus device reference
On Sat Oct 18, 2025 at 10:59 PM CEST, Markus Probst wrote:
> Implement the `IntoBusDevice` trait for converting a `Device` reference to a
> bus device reference for all bus devices. `Device` implements this trait as a
> fallback.
>
> The `IntoBusDevice` trait allows abstractions to provide the bus device in
> class device callbacks.
>
> Signed-off-by: Markus Probst <markus.probst@...teo.de>
> ---
> rust/kernel/auxiliary.rs | 7 +++++++
> rust/kernel/device.rs | 41 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/i2c.rs | 7 +++++++
i2c is not upstream yet, hence it should not be part of this patch. Instead you
should include the platform bus though.
> rust/kernel/pci.rs | 7 +++++++
> rust/kernel/usb.rs | 6 ++++++
> 5 files changed, 68 insertions(+)
>
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index e11848bbf206..dea24265f549 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -15,6 +15,7 @@
> };
> use core::{
> marker::PhantomData,
> + mem::offset_of,
> ptr::{addr_of_mut, NonNull},
> };
>
> @@ -239,6 +240,12 @@ extern "C" fn release(dev: *mut bindings::device) {
> }
> }
>
> +// SAFETY: `auxilary::Device` is a transparent wrapper of `struct auxiliary_device`.
> +// The offset is guaranteed to point to a valid device field inside `auxilary::Device`.
> +unsafe impl<Ctx: device::DeviceContext> device::IntoBusDevice<Ctx> for Device<Ctx> {
> + const OFFSET: usize = offset_of!(bindings::auxiliary_device, dev);
> +}
> +
> // 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 });
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 1321e6f0b53c..5527854a195f 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -511,6 +511,47 @@ impl DeviceContext for Core {}
> impl DeviceContext for CoreInternal {}
> impl DeviceContext for Normal {}
>
> +/// Bus devices can implement this trait to allow abstractions to provide the bus device in
> +/// class device callbacks.
> +///
> +/// # Safety
> +///
> +/// `IntoBusDevice::OFFSET` must be a offset to a device field in the implemented struct.
I think we should also require that this must only be implemented by bus device
types.
> +pub(crate) unsafe trait IntoBusDevice<Ctx: DeviceContext>:
> + AsRef<Device<Ctx>>
We should probably name this AsBusDevice.
> +{
> + /// The relative offset to the device field.
> + ///
> + /// Use `offset_of!(bindings, field)` macro to avoid breakage.
> + const OFFSET: usize;
> +
> + /// Convert a reference to [`Device`] into `Self`.
> + ///
> + /// # Safety
> + ///
> + /// `dev` must be contained in `Self`.
> + unsafe fn from_device(dev: &Device<Ctx>) -> &Self
As mentioned in the other thread, my concern remains that this could be abused
by drivers.
For now the trait is pub(crate), but with the new build system coming soon,
we're able to split things out of the kernel crate, and hence bus abstractions
and driver-core code may end up in different crates requiring this to become
public.
We should at least document that this must not be used by drivers and is
intended for bus and class device abstractions only.
> + where
> + Self: Sized,
> + {
> + let raw = dev.as_raw();
> + // SAFETY: `raw - Self::OFFSET` is guaranteed by the safety requirements
> + // to be a valid pointer to `Self`.
> + unsafe { &*raw.byte_sub(Self::OFFSET).cast::<Self>() }
> + }
> +}
> +
> +// SAFETY: `Device` is a transparent wrapper of `device`.
> +unsafe impl<Ctx: DeviceContext> IntoBusDevice<Ctx> for Device<Ctx> {
> + const OFFSET: usize = 0;
> +}
A generic device is not guaranteed to be a bus device. Also, I don't see a
reason why class device abstractions would want to work with a generic device
rather than the actual bus device parent. Hence, let's drop this impl.
Powered by blists - more mailing lists