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.0On 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/
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
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>.
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
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
> };
> }
>
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.
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
© 2016 - 2025 Red Hat, Inc.