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

Danilo Krummrich posted 4 patches 9 months, 1 week ago
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(-)
[PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Danilo Krummrich 9 months, 1 week ago
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(-)


base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
-- 
2.48.1
Re: [PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Greg KH 9 months, 1 week ago
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?

thanks,

greg k-h
Re: [PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Danilo Krummrich 9 months ago
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()?;
Re: [PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Greg KH 9 months ago
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
Re: [PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Boqun Feng 9 months, 1 week ago
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
> 

Again,

Acked-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> 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(-)
> 
> 
> base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
> -- 
> 2.48.1
>
Re: [PATCH v2 0/4] Improve soundness of bus device abstractions
Posted by Danilo Krummrich 9 months, 1 week ago
On Fri, Mar 14, 2025 at 10:28:09AM -0700, Boqun Feng 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
> > 
> 
> Again,
> 
> Acked-by: Boqun Feng <boqun.feng@gmail.com>

Sorry, I forgot to add your ACKs. Thanks for providing it again!