On Tue, Jul 08, 2025 at 04:01:19PM +0900, Alexandre Courbot wrote:
> 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@xxxxxxxxxx>
> > Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Cc: "Krzysztof Wilczyński" <kwilczynski@xxxxxxxxxx>
> > Cc: Miguel Ojeda <ojeda@xxxxxxxxxx>
> > Cc: Alex Gaynor <alex.gaynor@xxxxxxxxx>
> > Cc: Boqun Feng <boqun.feng@xxxxxxxxx>
> > Cc: Gary Guo <gary@xxxxxxxxxxx>
> > Cc: "Björn Roy Baron" <bjorn3_gh@xxxxxxxxxxxxxx>
> > Cc: Benno Lossin <lossin@xxxxxxxxxx>
> > Cc: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
> > Cc: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> > Cc: Trevor Gross <tmgross@xxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> > Cc: Alexandre Courbot <acourbot@xxxxxxxxxx>
> > Cc: linux-pci@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > ---
> > 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?
Ok.
> > + }
> > +
> > + /// 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.
Right you are. Will drop the `From` implementation for the next version, thanks.
