[PATCH 1/9] rust: dma: rename CoherentAllocation fallible methods

[Eliot Courtney <ecourtney@nvidia.com>]

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@nvidia.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