Re: [PATCH v2 0/4] Improve soundness of bus device abstractions

[Greg KH]

On Mon, Mar 17, 2025 at 12:46:01PM +0100, Danilo Krummrich wrote:
> On Sat, Mar 15, 2025 at 09:34:17AM +0100, Greg KH wrote:
> > On Fri, Mar 14, 2025 at 05:09:03PM +0100, Danilo Krummrich wrote:
> > > Currently, when sharing references of bus devices (e.g. ARef<pci::Device>), we
> > > do not have a way to restrict which functions of a bus device can be called.
> > > 
> > > Consequently, it is possible to call all bus device functions concurrently from
> > > any context. This includes functions, which access fields of the (bus) device,
> > > which are not protected against concurrent access.
> > > 
> > > This is improved by applying an execution context to the bus device in form of a
> > > generic type.
> > > 
> > > For instance, the PCI device reference that is passed to probe() has the type
> > > pci::Device<Core>, which implements all functions that are only allowed to be
> > > called from bus callbacks.
> > > 
> > > The implementation for the default context (pci::Device) contains all functions
> > > that are safe to call from any context concurrently.
> > > 
> > > The context types can be extended as required, e.g. to limit availability  of
> > > certain (bus) device functions to probe().
> > > 
> > > 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
> > > 
> > > Changes in v2:
> > >   - make `DeviceContext` trait sealed
> > >   - impl From<&pci::Device<device::Core>> for ARef<pci::Device>
> > >   - impl From<&platform::Device<device::Core>> for ARef<platform::Device>
> > >   - rebase onto v6.14-rc6
> > >   - apply RBs
> > > 
> > > Danilo Krummrich (4):
> > >   rust: pci: use to_result() in enable_device_mem()
> > >   rust: device: implement device context marker
> > >   rust: pci: fix unrestricted &mut pci::Device
> > >   rust: platform: fix unrestricted &mut platform::Device
> > > 
> > >  rust/kernel/device.rs                |  26 +++++
> > >  rust/kernel/pci.rs                   | 137 +++++++++++++++++----------
> > >  rust/kernel/platform.rs              |  95 +++++++++++++------
> > >  samples/rust/rust_driver_pci.rs      |   8 +-
> > >  samples/rust/rust_driver_platform.rs |  11 ++-
> > >  5 files changed, 187 insertions(+), 90 deletions(-)
> > 
> > Thanks for doing this work, looks good to me.  Mind if I suck it into
> > the driver-core tree now?  Or do you want it to go through a different
> > tree?
> 
> This series has a conflict with nova-core, it will require the following fixup
> in -next and Linus' tree when he pulls things.
> 
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 63c19f140fbd..a08fb6599267 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> -use kernel::{bindings, c_str, pci, prelude::*};
> +use kernel::{bindings, c_str, device::Core, pci, prelude::*};
>  
>  use crate::gpu::Gpu;
>  
> @@ -27,7 +27,7 @@ impl pci::Driver for NovaCore {
>      type IdInfo = ();
>      const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
>  
> -    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> +    fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
>          dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
>  
>          pdev.enable_device_mem()?;
> 

Ok, shouldn't be that hard of a merge issue, thanks for the diff as
linux-next should soon hit this as well.

greg k-h