linux-kernel - Re: [PATCH 1/2] rust: Add dma_set_mask() and dma_set_coherent

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: <DB6HB2VCI9CO.LCNRYZD6HWOJ@nvidia.com>
Date: Tue, 08 Jul 2025 16:01:19 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Alistair Popple" <apopple@...dia.com>, <rust-for-linux@...r.kernel.org>
Cc: "Danilo Krummrich" <dakr@...nel.org>, "Bjorn Helgaas"
 <bhelgaas@...gle.com>, Krzysztof Wilczyński
 <kwilczynski@...nel.org>, "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" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 "John Hubbard" <jhubbard@...dia.com>, <linux-pci@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] rust: Add dma_set_mask() and
 dma_set_coherent_mask() bindings

Hi Alistair,

On Tue Jul 8, 2025 at 3:04 PM JST, Alistair Popple wrote:
> Add bindings to allow setting the DMA masks for both a generic device
> and a PCI device.
>
> Signed-off-by: Alistair Popple <apopple@...dia.com>
> Cc: Danilo Krummrich <dakr@...nel.org>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: "Krzysztof Wilczyński" <kwilczynski@...nel.org>
> Cc: Miguel Ojeda <ojeda@...nel.org>
> Cc: Alex Gaynor <alex.gaynor@...il.com>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Gary Guo <gary@...yguo.net>
> Cc: "Björn Roy Baron" <bjorn3_gh@...tonmail.com>
> Cc: Benno Lossin <lossin@...nel.org>
> Cc: Andreas Hindborg <a.hindborg@...nel.org>
> Cc: Alice Ryhl <aliceryhl@...gle.com>
> Cc: Trevor Gross <tmgross@...ch.edu>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> Cc: John Hubbard <jhubbard@...dia.com>
> Cc: Alexandre Courbot <acourbot@...dia.com>
> Cc: linux-pci@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  rust/kernel/device.rs | 25 +++++++++++++++++++++++++
>  rust/kernel/pci.rs    | 23 +++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index dea06b79ecb5..77a1293a1c82 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -10,6 +10,7 @@
>      types::{ARef, Opaque},
>  };
>  use core::{fmt, marker::PhantomData, ptr};
> +use kernel::prelude::*;
>  
>  #[cfg(CONFIG_PRINTK)]
>  use crate::c_str;
> @@ -67,6 +68,30 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
>          self.0.get()
>      }
>  
> +    /// Sets the DMA mask for the device.
> +    pub fn dma_set_mask(&self, mask: u64) -> Result {
> +        // SAFETY: By the type invariant of `device::Device`, `self.as_ref().as_raw()` is valid.
> +        let ret = unsafe { bindings::dma_set_mask(&(*self.as_raw()) as *const _ as *mut _, mask) };
> +        if ret != 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(())
> +        }

I think you want to use `kernel::error::to_result()` here?

> +    }
> +
> +    /// Sets the coherent DMA mask for the device.
> +    pub fn dma_set_coherent_mask(&self, mask: u64) -> Result {
> +        // SAFETY: By the type invariant of `device::Device`, `self.as_ref().as_raw()` is valid.
> +        let ret = unsafe {
> +            bindings::dma_set_coherent_mask(&(*self.as_raw()) as *const _ as *mut _, mask)
> +        };
> +        if ret != 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            Ok(())
> +        }

And here as well.

> +    }
> +
>      /// Returns a reference to the parent device, if any.
>      #[cfg_attr(not(CONFIG_AUXILIARY_BUS), expect(dead_code))]
>      pub(crate) fn parent(&self) -> Option<&Self> {
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 8435f8132e38..7f640ba8f19c 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -369,6 +369,17 @@ fn as_raw(&self) -> *mut bindings::pci_dev {
>      }
>  }
>  
> +impl<'a, Ctx: device::DeviceContext> From<&'a kernel::pci::Device<Ctx>>
> +    for &'a device::Device<Ctx>
> +{
> +    fn from(pdev: &kernel::pci::Device<Ctx>) -> &device::Device<Ctx> {
> +        // SAFETY: The returned reference has the same lifetime as the
> +        // pci::Device which holds a reference on the underlying device
> +        // pointer.
> +        unsafe { device::Device::as_ref(&(*pdev.as_raw()).dev as *const _ as *mut _) }
> +    }
> +}

IIUC pci::Device has an `AsRef<device::Device>` implementation, why not
use that in the code below?

> +
>  impl Device {
>      /// Returns the PCI vendor ID.
>      pub fn vendor_id(&self) -> u16 {
> @@ -393,6 +404,18 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
>          // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`.
>          Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
>      }
> +
> +    /// Set the DMA mask for PCI device.
> +    pub fn dma_set_mask(&self, mask: u64) -> Result {
> +        let dev: &device::Device = self.into();
> +        dev.dma_set_mask(mask)

Yup, I have tried and `self.as_ref().dma_set_mask(mask)` works just
fine, so I don't think we need the `From` implementation above.