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 <[email protected]> --- 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
