From: Danilo Krummrich <dakr@kernel.org>

Some device methods require mutable references, since they change the
underlying struct device without lock protection.

Hence, make it possible to retrieve a mutable reference to a Device from
a mutable pci::Device.

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

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4c98b5b9aa1e..141430dac2d5 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
         &self.0
     }
 }
+
+impl AsMut<device::Device> for Device {
+    fn as_mut(&mut self) -> &mut device::Device {
+        // SAFETY:
+        // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
+        // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
+        //    mutable reference for `device::Device` if we have a mutable reference to the
+        //    corresponding `pci::Device`.
+        unsafe { &mut *self.0.as_raw().cast() }
+    }
+}
-- 
2.43.0

On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> From: Danilo Krummrich <dakr@kernel.org>
> 
> Some device methods require mutable references, since they change the
> underlying struct device without lock protection.
> 
> Hence, make it possible to retrieve a mutable reference to a Device from
> a mutable pci::Device.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Note, you can't forward on a patch from someone else without also
signing off on it.  Please read the DCO for what this means and why.

thanks,

greg k-h

On 07/03/2025 16:18, Greg KH wrote:
> On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
>> From: Danilo Krummrich <dakr@kernel.org>
>>
>> Some device methods require mutable references, since they change the
>> underlying struct device without lock protection.
>>
>> Hence, make it possible to retrieve a mutable reference to a Device from
>> a mutable pci::Device.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
> 
> Note, you can't forward on a patch from someone else without also
> signing off on it.  Please read the DCO for what this means and why.
> 
Thanks for the reminder! Will do so in next revision.

Regards,
Abdiel

On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> From: Danilo Krummrich <dakr@kernel.org>
> 
> Some device methods require mutable references, since they change the
> underlying struct device without lock protection.
> 
> Hence, make it possible to retrieve a mutable reference to a Device from
> a mutable pci::Device.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/pci.rs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 4c98b5b9aa1e..141430dac2d5 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
>          &self.0
>      }
>  }
> +
> +impl AsMut<device::Device> for Device {
> +    fn as_mut(&mut self) -> &mut device::Device {
> +        // SAFETY:
> +        // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
> +        // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
> +        //    mutable reference for `device::Device` if we have a mutable reference to the
> +        //    corresponding `pci::Device`.
> +        unsafe { &mut *self.0.as_raw().cast() }
> +    }
> +}

This makes it possible to call `mem::swap` on two devices, which I doubt
is allowed.

Alice

On Fri, Mar 07, 2025 at 11:18:03AM +0000, Alice Ryhl wrote:
> On Fri, Mar 07, 2025 at 01:06:20PM +0200, Abdiel Janulgue wrote:
> > From: Danilo Krummrich <dakr@kernel.org>
> > 
> > Some device methods require mutable references, since they change the
> > underlying struct device without lock protection.
> > 
> > Hence, make it possible to retrieve a mutable reference to a Device from
> > a mutable pci::Device.
> > 
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  rust/kernel/pci.rs | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 4c98b5b9aa1e..141430dac2d5 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -432,3 +432,14 @@ fn as_ref(&self) -> &device::Device {
> >          &self.0
> >      }
> >  }
> > +
> > +impl AsMut<device::Device> for Device {
> > +    fn as_mut(&mut self) -> &mut device::Device {
> > +        // SAFETY:
> > +        // - `self.0.as_raw()` is valid by the type invariant of `device::Device`,
> > +        // - `struct device` is embedded in `struct pci_dev`, hence it is safe to give out a
> > +        //    mutable reference for `device::Device` if we have a mutable reference to the
> > +        //    corresponding `pci::Device`.
> > +        unsafe { &mut *self.0.as_raw().cast() }
> > +    }
> > +}
> 
> This makes it possible to call `mem::swap` on two devices, which I doubt
> is allowed.

Good catch, that's of course not allowed.

Obviously, this is for Device::dma_set_mask(), introduced in a subsequent patch.

Thinking about it, maybe it should be implemented by bus devices instead (e.g.
pci::Device) -- dma_set_mask() doesn't make sense for every device.