linux-kernel - Re: [PATCH v14 09/11] rust: dma: use `dma::Device` in `CoherentAllocation`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87y0x2igjz.fsf@kernel.org>
Date: Tue, 18 Mar 2025 15:01:20 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Abdiel Janulgue <abdiel.janulgue@...il.com>
Cc: rust-for-linux@...r.kernel.org,  daniel.almeida@...labora.com,
  dakr@...nel.org,  robin.murphy@....com,  aliceryhl@...gle.com,  Miguel
 Ojeda <ojeda@...nel.org>,  Alex Gaynor <alex.gaynor@...il.com>,  Boqun
 Feng <boqun.feng@...il.com>,  Gary Guo <gary@...yguo.net>,  Björn Roy
 Baron <bjorn3_gh@...tonmail.com>,  Benno Lossin <benno.lossin@...ton.me>,
  Trevor Gross <tmgross@...ch.edu>,  Valentin Obst
 <kernel@...entinobst.de>,  linux-kernel@...r.kernel.org (open list),
  Christoph Hellwig <hch@....de>,  Marek Szyprowski
 <m.szyprowski@...sung.com>,  airlied@...hat.com,  iommu@...ts.linux.dev
 (open list:DMA MAPPING HELPERS)
Subject: Re: [PATCH v14 09/11] rust: dma: use `dma::Device` in
 `CoherentAllocation`

Abdiel Janulgue <abdiel.janulgue@...il.com> writes:

> From: Danilo Krummrich <dakr@...nel.org>
>
> Require a `&dyn dma::Device` in `CoherentAllocation`, such that DMA
> memory can only be allocated with devices on potentially DMA capable
> busses.
>
> Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@...il.com>
> ---
>  rust/kernel/dma.rs       | 16 ++++++++--------
>  samples/rust/rust_dma.rs |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index ac3ec0042327..d0c6569cfc70 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -57,10 +57,9 @@ fn dma_set_mask(&mut self, mask: u64) -> Result {
>  /// # Examples
>  ///
>  /// ```
> -/// use kernel::device::Device;
> -/// use kernel::dma::{attrs::*, CoherentAllocation};
> +/// use kernel::dma::{attrs::*, Device, CoherentAllocation};
>  ///
> -/// # fn test(dev: &Device) -> Result {
> +/// # fn test(dev: &dyn Device) -> Result {
>  /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN;
>  /// let c: CoherentAllocation<u64> =
>  ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?;
> @@ -178,16 +177,15 @@ impl<T: AsBytes + FromBytes> CoherentAllocation<T> {
>      /// # Examples
>      ///
>      /// ```
> -    /// use kernel::device::Device;
> -    /// use kernel::dma::{attrs::*, CoherentAllocation};
> +    /// use kernel::dma::{attrs::*, Device, CoherentAllocation};
>      ///
> -    /// # fn test(dev: &Device) -> Result {
> +    /// # fn test(dev: &dyn Device) -> Result {
>      /// let c: CoherentAllocation<u64> =
>      ///     CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?;
>      /// # Ok::<(), Error>(()) }
>      /// ```
>      pub fn alloc_attrs(
> -        dev: &device::Device,
> +        dev: &dyn Device,
>          count: usize,
>          gfp_flags: kernel::alloc::Flags,
>          dma_attrs: Attrs,
> @@ -197,6 +195,8 @@ pub fn alloc_attrs(
>              "It doesn't make sense for the allocated type to be a ZST"
>          );
>  
> +        let dev = dev.as_ref();
> +
>          let size = count
>              .checked_mul(core::mem::size_of::<T>())
>              .ok_or(EOVERFLOW)?;
> @@ -229,7 +229,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::Device,
> +        dev: &dyn Device,

Is it possible (and would it make sense?) to make these methods, or even
`CoherentAllocation` generic over the `D: Device` to get rid of the
dynamic dispatch:

    pub fn alloc_coherent(
        dev: &impl Device,
        count: usize,
        gfp_flags: kernel::alloc::Flags,
    ) -> Result<CoherentAllocation<T>> {


Best regards,
Andreas Hindborg