Commit fe584659 authored by Abdiel Janulgue's avatar Abdiel Janulgue Committed by Danilo Krummrich
Browse files

rust: dma: convert the read/write macros to return Result 



We could do better here by having the macros return `Result`,
so that we don't have to wrap these calls in a closure for
validation which is confusing.

Co-developed-by: default avatarAndreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: default avatarAndreas Hindborg <a.hindborg@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/87h63qhz4q.fsf@kernel.org/


Reviewed-by: default avatarAndreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: default avatarAbdiel Janulgue <abdiel.janulgue@gmail.com>
Link: https://lore.kernel.org/r/20250602085444.1925053-3-abdiel.janulgue@gmail.com


[ Fix line length in dma_read!(). - Danilo ]
Signed-off-by: default avatarDanilo Krummrich <dakr@kernel.org>
parent 9863f774

rust/kernel/dma.rs

+33 −23
Original line number Diff line number Diff line
@@ -328,20 +328,24 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {} 
#[macro_export]

macro_rules! 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.

            unsafe {

                let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);

                ::core::result::Result::Ok(

                    $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field)

                )

            }

        })()

    }};

    ($dma:ident [ $idx:expr ] $($field:tt)* ) => {

        $crate::dma_read!($dma, $idx, $($field)*);

        $crate::dma_read!($dma, $idx, $($field)*)

    };

    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {

        $crate::dma_read!($($dma).*, $idx, $($field)*);

        $crate::dma_read!($($dma).*, $idx, $($field)*)

    };

}
@@ -368,17 +372,21 @@ macro_rules! dma_read { 
#[macro_export]

macro_rules! dma_write {

    ($dma:ident [ $idx:expr ] $($field:tt)*) => {{

        $crate::dma_write!($dma, $idx, $($field)*);

        $crate::dma_write!($dma, $idx, $($field)*)

    }};

    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{

        $crate::dma_write!($($dma).*, $idx, $($field)*);

        $crate::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.

            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`
@@ -387,5 +395,7 @@ macro_rules! dma_write { 
                let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);

                $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)

            }

            ::core::result::Result::Ok(())

        })()

    };

}

samples/rust/rust_dma.rs

+15 −13
Original line number Diff line number Diff line
@@ -54,14 +54,10 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> Result<Pin<KBox<Self 
        let ca: CoherentAllocation<MyStruct> =

            CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;



        || -> Result {

        for (i, value) in TEST_VALUES.into_iter().enumerate() {

                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1));

            kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;

        }



            Ok(())

        }()?;



        let drvdata = KBox::new(

            Self {

                pdev: pdev.into(),
@@ -78,13 +74,19 @@ impl Drop for DmaSampleDriver { 
    fn drop(&mut self) {

        dev_info!(self.pdev.as_ref(), "Unload DMA test driver.\n");



        let _ = || -> Result {

        for (i, value) in TEST_VALUES.into_iter().enumerate() {

                assert_eq!(kernel::dma_read!(self.ca[i].h), value.0);

                assert_eq!(kernel::dma_read!(self.ca[i].b), value.1);

            let val0 = kernel::dma_read!(self.ca[i].h);

            let val1 = kernel::dma_read!(self.ca[i].b);

            assert!(val0.is_ok());

            assert!(val1.is_ok());



            if let Ok(val0) = val0 {

                assert_eq!(val0, value.0);

            }

            if let Ok(val1) = val1 {

                assert_eq!(val1, value.1);

            }

        }

            Ok(())

        }();

    }

}