dma::Device trait and DMA mask
Subsystems (5)
Danilo Krummrich
maintainerv2 cover dma::Device trait and DMA mask
This patch series adds the dma::Device trait to be implemented by bus devices on DMA capable busses.
The dma::Device trait implements methods to set the DMA mask for for such devices.
The first two bus devices implementing the trait are PCI and platform.
Unfortunately, the DMA mask setters have to be unsafe for now, since, with reasonable effort, we can't prevent drivers from data races writing and reading the DMA mask fields concurrently (see also [1]).
Changes in v2:
- replace dma_bit_mask() with a new type DmaMask
- mention that DmaMask is the Rust equivalent of the C macro DMA_BIT_MASK()
- make DmaMask::new() fallible
- inline DmaMask methods
Danilo Krummrich (5):
rust: dma: implement dma::Device trait
rust: dma: add DMA addressing capabilities
rust: pci: implement the dma::Device trait
rust: platform: implement the dma::Device trait
rust: samples: dma: set DMA mask
rust/helpers/dma.c | 5 ++ rust/kernel/dma.rs | 125 ++++++++++++++++++++++++++++++++++++--- rust/kernel/pci.rs | 2 + rust/kernel/platform.rs | 2 + samples/rust/rust_dma.rs | 14 ++++- 5 files changed, 140 insertions(+), 8 deletions(-)
base-commit: d49ac7744f578bcc8708a845cce24d3b91f86260
Signature · 1 line
2.50.0
Danilo Krummrich
maintainerPATCH v2 1/5 rust: dma: implement `dma::Device` trait
Add a trait that defines the DMA specific methods of devices.
The dma::Device trait is to be implemented by bus device representations, where the underlying bus is capable of DMA, such as pci::Device or platform::Device.
--- rust/kernel/dma.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
rust/kernel/dma.rs +12 -5
@@ -5,14 +5,21 @@//! C header: [`include/linux/dma-mapping.h`](srctree/include/linux/dma-mapping.h) use crate ::{ - bindings, build_assert, - device::{Bound ,Device }, + bindings, build_assert, device, + device::Bound , error::code::*, error::Result , transmute::{AsBytes ,FromBytes }, types::ARef , }; +/// Trait to be implemented by DMA capable bus devices. +/// +/// The [`dma::Device`](Device) trait should be implemented by bus specific device representations, +/// where the underlying bus is DMA capable, such as [`pci::Device`](::kernel::pci::Device) or +/// [`platform::Device`](::kernel::platform::Device). +pub trait Device :AsRef <device::Device <Core >> {} +/// Possible attributes associated with a DMA mapping. /// /// They can be combined with the operators `|`, `&`, and `!`.
@@ -132,7 +139,7 @@ pub mod attrs {// Hence, find a way to revoke the device resources of a `CoherentAllocation`, but not the // entire `CoherentAllocation` including the allocated memory itself. pub struct CoherentAllocation <T :AsBytes +FromBytes > { - dev:ARef <Device >, + dev:ARef <device::Device >, dma_handle: bindings::dma_addr_t , count:usize , cpu_addr: *mut T ,
@@ -154,7 +161,7 @@ impl<T: AsBytes + FromBytes> CoherentAllocation<T> {/// # Ok::<(), Error>(()) } /// ``` pub fn alloc_attrs ( - dev: &Device <Bound >, + dev: &device::Device <Bound >, count:usize , gfp_flags: kernel::alloc::Flags , dma_attrs: Attrs,
@@ -199,7 +206,7 @@ pub fn alloc_attrs(/// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the /// `dma_attrs` is 0 by default. pub fn alloc_coherent ( - dev: &Device <Bound >, + dev: &device::Device <Bound >, count:usize , gfp_flags: kernel::alloc::Flags , ) ->Result <CoherentAllocation <T >> {
Signature · 1 line
2.50.0
Danilo Krummrich
maintainerPATCH v2 2/5 rust: dma: add DMA addressing capabilities
In reply to Danilo KrummrichImplement dma_set_mask(), dma_set_coherent_mask() and dma_set_mask_and_coherent() in the dma::Device trait.
Those methods are used to set up the device's DMA addressing capabilities.
--- rust/helpers/dma.c | 5 ++ rust/kernel/dma.rs | 112 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 4 deletions(-)
rust/helpers/dma.c +5 -0
@@ -14,3 +14,8 @@ void rust_helper_dma_free_attrs(struct device *dev, size_t size, void *cpu_addr, {dma_free_attrs (dev, size, cpu_addr, dma_handle, attrs); } + +int rust_helper_dma_set_mask_and_coherent (struct device *dev,u64 mask) +{ +return dma_set_mask_and_coherent (dev, mask); +}
rust/kernel/dma.rs +108 -4
124 lines not on this page · raw message
Signature · 1 line
2.50.0
Daniel Almeida
reviewerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Hi Danilo,
rust/kernel/dma.rs line 119
Quoted 9 lines
rust/kernel/dma.rs +9
-0+#[inline] +pub const fn new (n:usize ) ->Result <Self > { +Ok (Self (match n { +0 =>0 , +1 ..=64 => u64::MAX >> (64 - n), + _ =>return Err (EINVAL ), + })) + } +
Isn’t this equivalent to genmask_u64(0..=n) ? See [0].
You should also get a compile-time failure if n is out of bounds by default using genmask.
— Daniel
[0]: https://lore.kernel.org/rust-for-linux/[email protected]/#r
Daniel Almeida
reviewerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Quoted 16 lines · Daniel Almeida
Hi Danilo,
rust/kernel/dma.rs line 119
Quoted 9 lines
rust/kernel/dma.rs +9
-0+#[inline] +pub const fn new (n:usize ) ->Result <Self > { +Ok (Self (match n { +0 =>0 , +1 ..=64 => u64::MAX >> (64 - n), + _ =>return Err (EINVAL ), + })) + } +
Quoted 8 lines
Isn’t this equivalent to
genmask_u64(0..=n)? See [0].You should also get a compile-time failure if n is out of bounds by default using genmask.
— Daniel
[0]: https://lore.kernel.org/rust-for-linux/[email protected]/#r
Or genmask_u64(0..=n-1), if we disregard the previous off-by-one error
— Daniel
Danilo Krummrich
maintainerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
In reply to Daniel AlmeidaQuoted 12 lines · Daniel Almeida
Hi Danilo,
rust/kernel/dma.rs line 119
Quoted 9 lines
rust/kernel/dma.rs +9
-0+#[inline] +pub const fn new (n:usize ) ->Result <Self > { +Ok (Self (match n { +0 =>0 , +1 ..=64 => u64::MAX >> (64 - n), + _ =>return Err (EINVAL ), + })) + } +
Quoted 1 line
Isn’t this equivalent to
genmask_u64(0..=n)? See [0].
Instead of the match this can use genmask_checked_u64() and convert the Option to a Result, once genmask is upstream.
Quoted 2 lines
You should also get a compile-time failure if n is out of bounds by default using genmask.
No, we can't use genmask_u64(), n is not guaranteed to be known at compile time, so we'd need to use genmask_checked_u64().
Of course, we could have a separate DmaMask constructor, e.g. with a const generic -- not sure that's worth though.
Danilo Krummrich
maintainerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Quoted 18 lines · Danilo Krummrich
Quoted 12 lines · Daniel Almeida
Hi Danilo,
rust/kernel/dma.rs line 119
Quoted 9 lines
rust/kernel/dma.rs +9
-0+#[inline] +pub const fn new (n:usize ) ->Result <Self > { +Ok (Self (match n { +0 =>0 , +1 ..=64 => u64::MAX >> (64 - n), + _ =>return Err (EINVAL ), + })) + } +Quoted 1 line
Isn’t this equivalent to
genmask_u64(0..=n)? See [0].Instead of the match this can use
genmask_checked_u64()and convert the Option to a Result, once genmask is upstream.
Quoted 8 lines
Quoted 2 lines
You should also get a compile-time failure if n is out of bounds by default using genmask.
No, we can't use
genmask_u64(),nis not guaranteed to be known at compile time, so we'd need to usegenmask_checked_u64().Of course, we could have a separate DmaMask constructor, e.g. with a const generic -- not sure that's worth though.
On the other hand, it doesn't hurt. Guess I will add another constructor with a const generic. :)
I also quickly tried genmask and I have a few questions:
(1) Why does genmask not use a const generic? I think this makes it more obvious that it's only intended to be used from const context.
(2) Why is there no build_assert() when the range exceeds the number of bits of the target type? I would expect genmask_u64(0..100) to fail.
(3) OOC, why did you choose u32 as argument type?
Danilo Krummrich
maintainerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Quoted 22 lines · Danilo Krummrich
Quoted 21 lines · Danilo Krummrich
Quoted 12 lines · Daniel Almeida
Hi Danilo,
Review
Quoted 9 lines
(quoted diff) +9
-0+ #[inline] + pub const fn new(n: usize) -> Result<Self> { + Ok(Self(match n { + 0 => 0, + 1..=64 => u64::MAX >> (64 - n), + _ => return Err(EINVAL), + })) + } +Quoted 1 line
Isn’t this equivalent to
genmask_u64(0..=n)? See [0].Instead of the match this can use
genmask_checked_u64()and convert the Option to a Result, once genmask is upstream.Quoted 2 lines
You should also get a compile-time failure if n is out of bounds by default using genmask.
Quoted 8 lines
Quoted 5 lines
No, we can't use
genmask_u64(),nis not guaranteed to be known at compile time, so we'd need to usegenmask_checked_u64().Of course, we could have a separate DmaMask constructor, e.g. with a const generic -- not sure that's worth though.
On the other hand, it doesn't hurt. Guess I will add another constructor with a const generic. :)
rust/kernel/dma.rs +52 -20
@@ -81,25 +81,6 @@ unsafe fn dma_set_mask_and_coherent(&self, mask: DmaMask) -> Result {/// never exceed the bit width of `u64`. /// /// This is the Rust equivalent of the C macro `DMA_BIT_MASK()`. -/// -/// # Examples -/// -/// ``` -/// use kernel::dma::DmaMask; -/// -/// let mask0 = DmaMask::new(0)?; -/// assert_eq!(mask0.value(), 0); -/// -/// let mask1 = DmaMask::new(1)?; -/// assert_eq!(mask1.value(), 0b1); -/// -/// let mask64 = DmaMask::new(64)?; -/// assert_eq!(mask64.value(), u64::MAX); -/// -/// let mask_overflow = DmaMask::new(100); -/// assert!(mask_overflow.is_err()); -/// # Ok::<(), Error>(()) -/// ``` #[derive( Debug , Clone , Copy , PartialEq , Eq )] pub struct DmaMask (u64 );
@@ -108,8 +89,27 @@ impl DmaMask {/// /// For `n <= 64`, sets exactly the lowest `n` bits. /// For `n > 64`, returns [`EINVAL`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::dma::DmaMask; +/// +/// let mask0 = DmaMask::try_new(0)?; +/// assert_eq!(mask0.value(), 0); +/// +/// let mask1 = DmaMask::try_new(1)?; +/// assert_eq!(mask1.value(), 0b1); +/// +/// let mask64 = DmaMask::try_new(64)?; +/// assert_eq!(mask64.value(), u64::MAX); +/// +/// let mask_overflow = DmaMask::try_new(100); +/// assert!(mask_overflow.is_err()); +/// # Ok::<(), Error>(()) +/// ``` #[inline] -pub const fn new (n:usize ) ->Result <Self > { +pub const fn try_new (n:u32 ) ->Result <Self > { Ok(Self(match n { 0 => 0, 1..=64 => u64::MAX >> (64 - n),
@@ -117,6 +117,38 @@ pub const fn new(n: usize) -> Result<Self> { })) } +/// Constructs a `DmaMask` with the lowest `n` bits set to `1`. +/// +/// For `n <= 64`, sets exactly the lowest `n` bits. +/// For `n > 64`, results in a build error. +/// +/// # Examples +/// +/// ``` +/// use kernel::dma::DmaMask; +/// use kernel::bits::genmask_u64; +/// +/// let mask0 = DmaMask::new::<0>(); +/// assert_eq!(mask0.value(), 0); +/// +/// let mask1 = DmaMask::new::<1>(); +/// assert_eq!(mask1.value(), 0b1); +/// +/// let mask64 = DmaMask::new::<64>(); +/// assert_eq!(mask64.value(), u64::MAX); +/// +/// // Build failure. +/// // let mask_overflow = DmaMask::new::<100>(); +/// ``` +#[inline(always)] +pub const fn new <const N :u32 >() ->Self { +let Ok (mask) =Self ::try_new (N )else { +build_error! ("Invalid DMA Mask." ); + }; + + mask + } +/// Returns the underlying `u64` bitmask value. #[inline] pub const fn value (&self) ->u64 {
samples/rust/rust_dma.rs +1 -1
@@ -58,7 +58,7 @@ impl pci::Driver for DmaSampleDriver {fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->Result <Pin <KBox <Self >>> {dev_info! (pdev.as_ref (),"Probe DMA test driver.\n" ); -let mask =DmaMask ::new (64 )?; +let mask =DmaMask ::new ::<64 >();// SAFETY: There are no concurrent calls to DMA allocation and mapping primitives. unsafe { pdev.dma_set_mask_and_coherent (mask)? };
Daniel Almeida
reviewerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
For some reason this diff did not apply on top of your latest commit (corrupt patch on line 6)
Quoted 64 lines
rust/kernel/dma.rs +20
-20@@ -81,25 +81,6 @@ unsafe fn dma_set_mask_and_coherent(&self, mask: DmaMask) -> Result {/// never exceed the bit width of `u64`. /// /// This is the Rust equivalent of the C macro `DMA_BIT_MASK()`. -/// -/// # Examples -/// -/// ``` -/// use kernel::dma::DmaMask; -/// -/// let mask0 = DmaMask::new(0)?; -/// assert_eq!(mask0.value(), 0); -/// -/// let mask1 = DmaMask::new(1)?; -/// assert_eq!(mask1.value(), 0b1); -/// -/// let mask64 = DmaMask::new(64)?; -/// assert_eq!(mask64.value(), u64::MAX); -/// -/// let mask_overflow = DmaMask::new(100); -/// assert!(mask_overflow.is_err()); -/// # Ok::<(), Error>(()) -/// ``` #[derive( Debug , Clone , Copy , PartialEq , Eq )] pub struct DmaMask (u64 );@@ -108,8 +89,27 @@ impl DmaMask {/// /// For `n <= 64`, sets exactly the lowest `n` bits. /// For `n > 64`, returns [`EINVAL`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::dma::DmaMask; +/// +/// let mask0 = DmaMask::try_new(0)?; +/// assert_eq!(mask0.value(), 0); +/// +/// let mask1 = DmaMask::try_new(1)?; +/// assert_eq!(mask1.value(), 0b1); +/// +/// let mask64 = DmaMask::try_new(64)?; +/// assert_eq!(mask64.value(), u64::MAX); +/// +/// let mask_overflow = DmaMask::try_new(100); +/// assert!(mask_overflow.is_err()); +/// # Ok::<(), Error>(()) +/// ``` #[inline] -pub const fn new (n:usize ) ->Result <Self > { +pub const fn try_new (n:u32 ) ->Result <Self > { Ok(Self(match n { 0 => 0, 1..=64 => u64::MAX >> (64 - n),@@ -117,6 +117,38 @@ pub const fn new(n: usize) -> Result<Self> { })) }
rust/kernel/dma.rs pub const fn new(n: usize) -> Result<Self> { · line 129
Quoted 10 lines
rust/kernel/dma.rs +10
-0+/// Constructs a `DmaMask` with the lowest `n` bits set to `1`. +/// +/// For `n <= 64`, sets exactly the lowest `n` bits. +/// For `n > 64`, results in a build error. +/// +/// # Examples +/// +/// ``` +/// use kernel::dma::DmaMask; +/// use kernel::bits::genmask_u64;
There is also this, which is not upstream (nor used in the patch)
Quoted 32 lines
rust/kernel/dma.rs +22
-0+/// +/// let mask0 = DmaMask::new::<0>(); +/// assert_eq!(mask0.value(), 0); +/// +/// let mask1 = DmaMask::new::<1>(); +/// assert_eq!(mask1.value(), 0b1); +/// +/// let mask64 = DmaMask::new::<64>(); +/// assert_eq!(mask64.value(), u64::MAX); +/// +/// // Build failure. +/// // let mask_overflow = DmaMask::new::<100>(); +/// ``` +#[inline(always)] +pub const fn new <const N :u32 >() ->Self { +let Ok (mask) =Self ::try_new (N )else { +build_error! ("Invalid DMA Mask." ); + }; + + mask + } +/// Returns the underlying `u64` bitmask value. #[inline] pub const fn value (&self) ->u64 {samples/rust/rust_dma.rs +0
-0
samples/rust/rust_dma.rs impl pci::Driver for DmaSampleDriver { · line 64
Quoted 8 lines
samples/rust/rust_dma.rs +1
-1@@ -58,7 +58,7 @@ impl pci::Driver for DmaSampleDriver {fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->Result <Pin <KBox <Self >>> {dev_info! (pdev.as_ref (),"Probe DMA test driver.\n" ); -let mask =DmaMask ::new (64 )?; +let mask =DmaMask ::new ::<64 >();// SAFETY: There are no concurrent calls to DMA allocation and mapping primitives. unsafe { pdev.dma_set_mask_and_coherent (mask)? };
I hand-applied this and fixed the issue above. All doctests pass FYI.
— Daniel
Danilo Krummrich
maintainerRe: [PATCH v2 2/5] rust: dma: add DMA addressing capabilities
Quoted 1 line · Daniel Almeida
I hand-applied this and fixed the issue above. All doctests pass FYI.
Yes, I noticed this in my testing as well, yet it seems I somehow managed to post the wrong diff. In my branch [1] everything should be correct.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/dma-mask
Danilo Krummrich
maintainerPATCH v2 3/5 rust: pci: implement the `dma::Device` trait
In reply to Danilo KrummrichThe PCI bus is potentially capable of performing DMA, hence implement the dma:Device trait for pci::Device.
--- rust/kernel/pci.rs | 2 ++ 1 file changed, 2 insertions(+)
rust/kernel/pci.rs +2 -0
@@ -434,6 +434,8 @@ pub fn set_master(&self) { kernel::impl_device_context_deref!(unsafe {Device }); kernel::impl_device_context_into_aref!(Device ); +impl crate ::dma::Device for Device <device::Core > {} +// SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate ::types::AlwaysRefCounted for Device {fn inc_ref (&self) {
Signature · 1 line
2.50.0
Danilo Krummrich
maintainerPATCH v2 4/5 rust: platform: implement the `dma::Device` trait
In reply to Danilo KrummrichThe platform bus is potentially capable of performing DMA, hence implement the dma:Device trait for platform::Device.
--- rust/kernel/platform.rs | 2 ++ 1 file changed, 2 insertions(+)
rust/kernel/platform.rs +2 -0
@@ -195,6 +195,8 @@ fn as_raw(&self) -> *mut bindings::platform_device { kernel::impl_device_context_deref!(unsafe {Device }); kernel::impl_device_context_into_aref!(Device ); +impl crate ::dma::Device for Device <device::Core > {} +// SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate ::types::AlwaysRefCounted for Device {fn inc_ref (&self) {
Signature · 1 line
2.50.0
Danilo Krummrich
maintainerPATCH v2 5/5 rust: samples: dma: set DMA mask
In reply to Danilo KrummrichSet a DMA mask for the pci::Device in the Rust DMA sample driver.
--- samples/rust/rust_dma.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
samples/rust/rust_dma.rs +13 -1
@@ -4,7 +4,14 @@//! //! To make this driver probe, QEMU must be run with `-device pci-testdev`. -use kernel::{bindings, device::Core , dma::CoherentAllocation , pci, prelude::*, types::ARef }; +use kernel::{ + bindings, + device::Core , + dma::{CoherentAllocation ,Device ,DmaMask }, + pci, + prelude::*, + types::ARef , +};struct DmaSampleDriver { pdev:ARef <pci::Device >,
@@ -51,6 +58,11 @@ impl pci::Driver for DmaSampleDriver {fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->Result <Pin <KBox <Self >>> {dev_info! (pdev.as_ref (),"Probe DMA test driver.\n" ); +let mask =DmaMask ::new (64 )?; + +// SAFETY: There are no concurrent calls to DMA allocation and mapping primitives. +unsafe { pdev.dma_set_mask_and_coherent (mask)? }; +let ca:CoherentAllocation <MyStruct > =CoherentAllocation ::alloc_coherent (pdev.as_ref (),TEST_VALUES .len (),GFP_KERNEL )?;
Signature · 1 line
2.50.0
Quoted 12 lines · Danilo Krummrich
This patch series adds the
dma::Devicetrait to be implemented by bus devices on DMA capable busses.The
dma::Devicetrait implements methods to set the DMA mask for for such devices.The first two bus devices implementing the trait are PCI and platform.
Unfortunately, the DMA mask setters have to be unsafe for now, since, with reasonable effort, we can't prevent drivers from data races writing and reading the DMA mask fields concurrently (see also [1]).
Quoted 8 lines
Changes in v2:
- replacedma_bit_mask()with a new type DmaMask
- mention that DmaMask is the Rust equivalent of the C macroDMA_BIT_MASK()
- makeDmaMask::new()fallible
- inline DmaMask methods
I like the DmaMask stuff, nice!
Danilo Krummrich
maintainerQuoted 2 lines · Danilo Krummrich
This patch series adds the
dma::Devicetrait to be implemented by bus devices on DMA capable busses.
Applied to driver-core-testing, thanks!