[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20260130-coherent-array-v1-1-bcd672dacc70@nvidia.com>
Date: Fri, 30 Jan 2026 17:34:04 +0900
From: Eliot Courtney <ecourtney@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>,
Alexandre Courbot <acourbot@...dia.com>, Alice Ryhl <aliceryhl@...gle.com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Abdiel Janulgue <abdiel.janulgue@...il.com>,
Daniel Almeida <daniel.almeida@...labora.com>,
Robin Murphy <robin.murphy@....com>,
Andreas Hindborg <a.hindborg@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>, Trevor Gross <tmgross@...ch.edu>
Cc: nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, driver-core@...ts.linux.dev,
rust-for-linux@...r.kernel.org, Eliot Courtney <ecourtney@...dia.com>
Subject: [PATCH 1/9] rust: dma: rename CoherentAllocation fallible methods
Prefix fallible methods in CoherentAllocation with try_. Prefix
dma_write! and dma_read! macros with try_ to better indicate they
can fail.
Signed-off-by: Eliot Courtney <ecourtney@...dia.com>
---
drivers/gpu/nova-core/dma.rs | 2 +-
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/firmware/fwsec.rs | 4 +-
drivers/gpu/nova-core/gsp.rs | 16 +++----
drivers/gpu/nova-core/gsp/boot.rs | 6 +--
drivers/gpu/nova-core/gsp/cmdq.rs | 14 +++---
rust/kernel/dma.rs | 85 +++++++++++++++++----------------
samples/rust/rust_dma.rs | 6 +--
8 files changed, 69 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
index 7215398969da..f77754f12f02 100644
--- a/drivers/gpu/nova-core/dma.rs
+++ b/drivers/gpu/nova-core/dma.rs
@@ -33,7 +33,7 @@ pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Res
Self::new(dev, data.len()).and_then(|mut dma_obj| {
// SAFETY: We have just allocated the DMA memory, we are the only users and
// we haven't made the device aware of the handle yet.
- unsafe { dma_obj.write(data, 0)? }
+ unsafe { dma_obj.try_write(data, 0)? }
Ok(dma_obj)
})
}
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 82c661aef594..9cd271de0554 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -460,7 +460,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
FalconMem::Imem => (load_offsets.src_start, fw.dma_handle()),
FalconMem::Dmem => (
0,
- fw.dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
+ fw.try_dma_handle_with_offset(load_offsets.src_start.into_safe_cast())?,
),
};
if dma_start % DmaAddress::from(DMA_LEN) > 0 {
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index b28e34d279f4..515b19926b49 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -191,7 +191,7 @@ unsafe fn transmute<T: Sized + FromBytes>(fw: &DmaObject, offset: usize) -> Resu
// SAFETY: The safety requirements of the function guarantee the device won't read
// or write to memory while the reference is alive and that this call won't race
// with writes to the same memory region.
- T::from_bytes(unsafe { fw.as_slice(offset, size_of::<T>())? }).ok_or(EINVAL)
+ T::from_bytes(unsafe { fw.try_as_slice(offset, size_of::<T>())? }).ok_or(EINVAL)
}
/// Reinterpret the area starting from `offset` in `fw` as a mutable instance of `T` (which must
@@ -210,7 +210,7 @@ unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
// SAFETY: The safety requirements of the function guarantee the device won't read
// or write to memory while the reference is alive and that this call won't race
// with writes or reads to the same memory region.
- T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())? }).ok_or(EINVAL)
+ T::from_bytes_mut(unsafe { fw.try_as_slice_mut(offset, size_of::<T>())? }).ok_or(EINVAL)
}
/// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index fb6f74797178..43bc35fd3b55 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -8,10 +8,10 @@
CoherentAllocation,
DmaAddress, //
},
- dma_write,
pci,
prelude::*,
- transmute::AsBytes, //
+ transmute::AsBytes,
+ try_dma_write, //
};
pub(crate) mod cmdq;
@@ -92,7 +92,7 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
unsafe {
// Copy the self-mapping PTE at the expected location.
obj.0
- .as_slice_mut(size_of::<u64>(), size_of_val(&ptes))?
+ .try_as_slice_mut(size_of::<u64>(), size_of_val(&ptes))?
.copy_from_slice(ptes.as_bytes())
};
@@ -131,13 +131,13 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
// _kgspInitLibosLoggingStructures (allocates memory for buffers)
// kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array)
let loginit = LogBuffer::new(dev)?;
- dma_write!(libos[0] = LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0))?;
+ try_dma_write!(libos[0] = LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0))?;
let logintr = LogBuffer::new(dev)?;
- dma_write!(libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0))?;
+ try_dma_write!(libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0))?;
let logrm = LogBuffer::new(dev)?;
- dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
+ try_dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
let cmdq = Cmdq::new(dev)?;
@@ -146,8 +146,8 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
1,
GFP_KERNEL | __GFP_ZERO,
)?;
- dma_write!(rmargs[0] = fw::GspArgumentsCached::new(&cmdq))?;
- dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
+ try_dma_write!(rmargs[0] = fw::GspArgumentsCached::new(&cmdq))?;
+ try_dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
Ok(try_pin_init!(Self {
libos,
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..69e2fb064220 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -3,11 +3,11 @@
use kernel::{
device,
dma::CoherentAllocation,
- dma_write,
io::poll::read_poll_timeout,
pci,
prelude::*,
- time::Delta, //
+ time::Delta,
+ try_dma_write, //
};
use crate::{
@@ -160,7 +160,7 @@ pub(crate) fn boot(
let wpr_meta =
CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
- dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
+ try_dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
self.cmdq
.send_command(bar, commands::SetSystemInfo::new(pdev))?;
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 3991ccc0c10f..9c94f4c6ff6d 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -15,7 +15,6 @@
CoherentAllocation,
DmaAddress, //
},
- dma_write,
io::poll::read_poll_timeout,
prelude::*,
sync::aref::ARef,
@@ -24,6 +23,7 @@
AsBytes,
FromBytes, //
},
+ try_dma_write, //
};
use crate::{
@@ -201,9 +201,11 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
let gsp_mem =
CoherentAllocation::<GspMem>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
- dma_write!(gsp_mem[0].ptes = PteArray::new(gsp_mem.dma_handle())?)?;
- dma_write!(gsp_mem[0].cpuq.tx = MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES))?;
- dma_write!(gsp_mem[0].cpuq.rx = MsgqRxHeader::new())?;
+ try_dma_write!(gsp_mem[0].ptes = PteArray::new(gsp_mem.dma_handle())?)?;
+ try_dma_write!(
+ gsp_mem[0].cpuq.tx = MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES)
+ )?;
+ try_dma_write!(gsp_mem[0].cpuq.rx = MsgqRxHeader::new())?;
Ok(Self(gsp_mem))
}
@@ -221,7 +223,7 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
// - The `CoherentAllocation` contains exactly one object.
// - We will only access the driver-owned part of the shared memory.
// - Per the safety statement of the function, no concurrent access will be performed.
- let gsp_mem = &mut unsafe { self.0.as_slice_mut(0, 1) }.unwrap()[0];
+ let gsp_mem = &mut unsafe { self.0.try_as_slice_mut(0, 1) }.unwrap()[0];
// PANIC: per the invariant of `cpu_write_ptr`, `tx` is `<= MSGQ_NUM_PAGES`.
let (before_tx, after_tx) = gsp_mem.cpuq.msgq.data.split_at_mut(tx);
@@ -256,7 +258,7 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
// - The `CoherentAllocation` contains exactly one object.
// - We will only access the driver-owned part of the shared memory.
// - Per the safety statement of the function, no concurrent access will be performed.
- let gsp_mem = &unsafe { self.0.as_slice(0, 1) }.unwrap()[0];
+ let gsp_mem = &unsafe { self.0.try_as_slice(0, 1) }.unwrap()[0];
// PANIC: per the invariant of `cpu_read_ptr`, `xx` is `<= MSGQ_NUM_PAGES`.
let (before_rx, after_rx) = gsp_mem.gspq.msgq.data.split_at(rx);
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 909d56fd5118..02321d5f3f06 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -482,7 +482,7 @@ pub fn dma_handle(&self) -> DmaAddress {
/// device as the DMA address base of the region.
///
/// Returns `EINVAL` if `offset` is not within the bounds of the allocation.
- pub fn dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
+ pub fn try_dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
if offset >= self.count {
Err(EINVAL)
} else {
@@ -494,7 +494,7 @@ pub fn dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
/// Common helper to validate a range applied from the allocated region in the CPU's virtual
/// address space.
- fn validate_range(&self, offset: usize, count: usize) -> Result {
+ fn try_validate_range(&self, offset: usize, count: usize) -> Result {
if offset.checked_add(count).ok_or(EOVERFLOW)? > self.count {
return Err(EINVAL);
}
@@ -514,8 +514,8 @@ fn validate_range(&self, offset: usize, count: usize) -> Result {
/// slice is live.
/// * Callers must ensure that this call does not race with a write to the same region while
/// the returned slice is live.
- pub unsafe fn as_slice(&self, offset: usize, count: usize) -> Result<&[T]> {
- self.validate_range(offset, count)?;
+ pub unsafe fn try_as_slice(&self, offset: usize, count: usize) -> Result<&[T]> {
+ self.try_validate_range(offset, count)?;
// SAFETY:
// - The pointer is valid due to type invariant on `CoherentAllocation`,
// we've just checked that the range and index is within bounds. The immutability of the
@@ -525,8 +525,8 @@ pub unsafe fn as_slice(&self, offset: usize, count: usize) -> Result<&[T]> {
Ok(unsafe { core::slice::from_raw_parts(self.start_ptr().add(offset), count) })
}
- /// Performs the same functionality as [`CoherentAllocation::as_slice`], except that a mutable
- /// slice is returned.
+ /// Performs the same functionality as [`CoherentAllocation::try_as_slice`], except that a
+ /// mutable slice is returned.
///
/// # Safety
///
@@ -534,8 +534,8 @@ pub unsafe fn as_slice(&self, offset: usize, count: usize) -> Result<&[T]> {
/// slice is live.
/// * Callers must ensure that this call does not race with a read or write to the same region
/// while the returned slice is live.
- pub unsafe fn as_slice_mut(&mut self, offset: usize, count: usize) -> Result<&mut [T]> {
- self.validate_range(offset, count)?;
+ pub unsafe fn try_as_slice_mut(&mut self, offset: usize, count: usize) -> Result<&mut [T]> {
+ self.try_validate_range(offset, count)?;
// SAFETY:
// - The pointer is valid due to type invariant on `CoherentAllocation`,
// we've just checked that the range and index is within bounds. The immutability of the
@@ -561,11 +561,11 @@ pub unsafe fn as_slice_mut(&mut self, offset: usize, count: usize) -> Result<&mu
/// let buf: &[u8] = &somedata;
/// // SAFETY: There is no concurrent HW operation on the device and no other R/W access to the
/// // region.
- /// unsafe { alloc.write(buf, 0)?; }
+ /// unsafe { alloc.try_write(buf, 0)?; }
/// # Ok::<(), Error>(()) }
/// ```
- pub unsafe fn write(&mut self, src: &[T], offset: usize) -> Result {
- self.validate_range(offset, src.len())?;
+ pub unsafe fn try_write(&mut self, src: &[T], offset: usize) -> Result {
+ self.try_validate_range(offset, src.len())?;
// SAFETY:
// - The pointer is valid due to type invariant on `CoherentAllocation`
// and we've just checked that the range and index is within bounds.
@@ -581,12 +581,13 @@ pub unsafe fn write(&mut self, src: &[T], offset: usize) -> Result {
Ok(())
}
- /// Returns a pointer to an element from the region with bounds checking. `offset` is in
- /// units of `T`, not the number of bytes.
+ /// Returns a pointer to an element from the region with bounds checking. `offset` is in units
+ /// of `T`, not the number of bytes.
///
- /// Public but hidden since it should only be used from [`dma_read`] and [`dma_write`] macros.
+ /// Public but hidden since it should only be used from [`try_dma_read`] and [`try_dma_write`]
+ /// macros.
#[doc(hidden)]
- pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
+ pub fn try_item_from_index(&self, offset: usize) -> Result<*mut T> {
if offset >= self.count {
return Err(EINVAL);
}
@@ -602,10 +603,10 @@ pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
///
/// # Safety
///
- /// This must be called from the [`dma_read`] macro which ensures that the `field` pointer is
- /// validated beforehand.
+ /// This must be called from the [`try_dma_read`] macro which ensures that the `field` pointer
+ /// is validated beforehand.
///
- /// Public but hidden since it should only be used from [`dma_read`] macro.
+ /// Public but hidden since it should only be used from [`try_dma_read`] macro.
#[doc(hidden)]
pub unsafe fn field_read<F: FromBytes>(&self, field: *const F) -> F {
// SAFETY:
@@ -625,10 +626,10 @@ pub unsafe fn field_read<F: FromBytes>(&self, field: *const F) -> F {
///
/// # Safety
///
- /// This must be called from the [`dma_write`] macro which ensures that the `field` pointer is
- /// validated beforehand.
+ /// This must be called from the [`try_dma_write`] macro which ensures that the `field` pointer
+ /// is validated beforehand.
///
- /// Public but hidden since it should only be used from [`dma_write`] macro.
+ /// Public but hidden since it should only be used from [`try_dma_write`] macro.
#[doc(hidden)]
pub unsafe fn field_write<F: AsBytes>(&self, field: *mut F, val: F) {
// SAFETY:
@@ -684,18 +685,18 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
///
/// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// let whole = kernel::dma_read!(alloc[2]);
-/// let field = kernel::dma_read!(alloc[1].field);
+/// let whole = kernel::try_dma_read!(alloc[2]);
+/// let field = kernel::try_dma_read!(alloc[1].field);
/// # Ok::<(), Error>(()) }
/// ```
#[macro_export]
-macro_rules! dma_read {
- ($dma:expr, $idx: expr, $($field:tt)*) => {{
+macro_rules! try_dma_read {
+ ($dma:expr, $idx:expr, $($field:tt)*) => {{
(|| -> ::core::result::Result<_, $crate::error::Error> {
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
- // dereferenced. The compiler also further validates the expression on whether `field`
- // is a member of `item` when expanded by the macro.
+ let item = $crate::dma::CoherentAllocation::try_item_from_index(&$dma, $idx)?;
+ // SAFETY: `try_item_from_index` ensures that `item` is always a valid pointer
+ // and can be dereferenced. The compiler also further validates the expression
+ // on whether `field` is a member of `item` when expanded by the macro.
unsafe {
let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);
::core::result::Result::Ok(
@@ -705,10 +706,10 @@ macro_rules! dma_read {
})()
}};
($dma:ident [ $idx:expr ] $($field:tt)* ) => {
- $crate::dma_read!($dma, $idx, $($field)*)
+ $crate::try_dma_read!($dma, $idx, $($field)*)
};
($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {
- $crate::dma_read!($($dma).*, $idx, $($field)*)
+ $crate::try_dma_read!($($dma).*, $idx, $($field)*)
};
}
@@ -728,32 +729,32 @@ macro_rules! dma_read {
/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
///
/// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// kernel::dma_write!(alloc[2].member = 0xf);
-/// kernel::dma_write!(alloc[1] = MyStruct { member: 0xf });
+/// kernel::try_dma_write!(alloc[2].member = 0xf);
+/// kernel::try_dma_write!(alloc[1] = MyStruct { member: 0xf });
/// # Ok::<(), Error>(()) }
/// ```
#[macro_export]
-macro_rules! dma_write {
+macro_rules! try_dma_write {
($dma:ident [ $idx:expr ] $($field:tt)*) => {{
- $crate::dma_write!($dma, $idx, $($field)*)
+ $crate::try_dma_write!($dma, $idx, $($field)*)
}};
($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{
- $crate::dma_write!($($dma).*, $idx, $($field)*)
+ $crate::try_dma_write!($($dma).*, $idx, $($field)*)
}};
($dma:expr, $idx: expr, = $val:expr) => {
(|| -> ::core::result::Result<_, $crate::error::Error> {
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid item.
+ let item = $crate::dma::CoherentAllocation::try_item_from_index(&$dma, $idx)?;
+ // SAFETY: `try_item_from_index` ensures that `item` is always a valid item.
unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) }
::core::result::Result::Ok(())
})()
};
($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => {
(|| -> ::core::result::Result<_, $crate::error::Error> {
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
- // dereferenced. The compiler also further validates the expression on whether `field`
- // is a member of `item` when expanded by the macro.
+ let item = $crate::dma::CoherentAllocation::try_item_from_index(&$dma, $idx)?;
+ // SAFETY: `try_item_from_index` ensures that `item` is always a valid pointer
+ // and can be dereferenced. The compiler also further validates the expression
+ // on whether `field` is a member of `item` when expanded by the macro.
unsafe {
let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);
$crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9c45851c876e..7a87048575df 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
for (i, value) in TEST_VALUES.into_iter().enumerate() {
- kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
+ kernel::try_dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
}
let size = 4 * page::PAGE_SIZE;
@@ -91,8 +91,8 @@ fn drop(self: Pin<&mut Self>) {
dev_info!(self.pdev, "Unload DMA test driver.\n");
for (i, value) in TEST_VALUES.into_iter().enumerate() {
- let val0 = kernel::dma_read!(self.ca[i].h);
- let val1 = kernel::dma_read!(self.ca[i].b);
+ let val0 = kernel::try_dma_read!(self.ca[i].h);
+ let val1 = kernel::try_dma_read!(self.ca[i].b);
assert!(val0.is_ok());
assert!(val1.is_ok());
--
2.52.0
Powered by blists - more mailing lists