Re: [PATCH 1/2] rust: Add dma_set_mask() and dma_set_coherent_mask() bindings

["Alexandre Courbot" <acourbot@nvidia.com>]

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@nvidia.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Alex Gaynor <alex.gaynor@gmail.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: "Björn Roy Baron" <bjorn3_gh@protonmail.com>
> Cc: Benno Lossin <lossin@kernel.org>
> Cc: Andreas Hindborg <a.hindborg@kernel.org>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Trevor Gross <tmgross@umich.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-kernel@vger.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.