[v2,1/2] rust: add scatterlist support

Message ID 20230405201416.395840-2-daniel.almeida@collabora.com
State New
Headers
Series rust: virtio: add virtio support |

Commit Message

Daniel Almeida April 5, 2023, 8:14 p.m. UTC
  This patch adds a scatterlist abstraction. It is then used and tested
by the new rust virtio sample module.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/lib.rs         |  1 +
 rust/kernel/scatterlist.rs | 40 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)
 create mode 100644 rust/kernel/scatterlist.rs
  

Comments

Martin Rodriguez Reboredo April 6, 2023, 2:21 p.m. UTC | #1
On 4/5/23 17:14, Daniel Almeida wrote:
> This patch adds a scatterlist abstraction. It is then used and tested
> by the new rust virtio sample module.
> 
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> ---
>  rust/kernel/lib.rs         |  1 +
>  rust/kernel/scatterlist.rs | 40 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 rust/kernel/scatterlist.rs
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c20b37e88ab2..b23be69919cc 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -71,6 +71,7 @@ pub mod net;
>  pub mod pages;
>  pub mod power;
>  pub mod revocable;
> +pub mod scatterlist;
>  pub mod security;
>  pub mod task;
>  pub mod workqueue;
> diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs
> new file mode 100644
> index 000000000000..fe036af13995
> --- /dev/null
> +++ b/rust/kernel/scatterlist.rs
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Scatterlist abstractions.
> +//!
> +//! C header: [`include/linux/virtio.h`](../../../../include/linux/scatterlist.h)

Little typo, I think that you've meant this to be scatterlist.h instead
of virtio.h.

> +
> +use core::cell::UnsafeCell;
> +
> +/// Scatterlist abstraction over the C side `struct scatterlist`.
> +pub struct Scatterlist {
> +    /// The C side `struct scatterlist`.
> +    inner: UnsafeCell<bindings::scatterlist>,
> +}
> +

Wouldn't be better to use a transparent tuple struct [1] since this only
has one non-zero sized field and could be easily transmuted into its
inner type.

> +impl Scatterlist {
> +    /// A wrapper over the C-side `sg_init_one()`. Initialize a single entry sg
> +    /// list.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Caller must ensure that `buf` is valid and `buflen` really
> +    /// represents the size of `buf`.
> +    pub unsafe fn init_one(buf: *const core::ffi::c_void, buflen: u32) -> Self {
> +        // SAFETY: There are no references or function pointers in this
> +        // `Scatterlist`.
> +        let mut sg: Scatterlist = unsafe { core::mem::zeroed() };
> +        // SAFETY: we pass a valid sg entry, which points to stack-allocated
> +        // variable above. `buf` and `buflen` are presumed valid as per this
> +        // function's safety requirements.
> +        unsafe {
> +            bindings::sg_init_one(sg.inner.get_mut(), buf, buflen);
> +        }
> +
> +        sg
> +    }

If this function was meant to be used for internal use only then it
should be `pub(crate)`, or else, a function that takes an slice should
be provided as a safe alternative.

> +
> +    pub(crate) fn inner(&self) -> &UnsafeCell<bindings::scatterlist> {
> +        &self.inner
> +    }
> +}

Link: https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent [1]
Regards
-> Martin
  

Patch

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c20b37e88ab2..b23be69919cc 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -71,6 +71,7 @@  pub mod net;
 pub mod pages;
 pub mod power;
 pub mod revocable;
+pub mod scatterlist;
 pub mod security;
 pub mod task;
 pub mod workqueue;
diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs
new file mode 100644
index 000000000000..fe036af13995
--- /dev/null
+++ b/rust/kernel/scatterlist.rs
@@ -0,0 +1,40 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! Scatterlist abstractions.
+//!
+//! C header: [`include/linux/virtio.h`](../../../../include/linux/scatterlist.h)
+
+use core::cell::UnsafeCell;
+
+/// Scatterlist abstraction over the C side `struct scatterlist`.
+pub struct Scatterlist {
+    /// The C side `struct scatterlist`.
+    inner: UnsafeCell<bindings::scatterlist>,
+}
+
+impl Scatterlist {
+    /// A wrapper over the C-side `sg_init_one()`. Initialize a single entry sg
+    /// list.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure that `buf` is valid and `buflen` really
+    /// represents the size of `buf`.
+    pub unsafe fn init_one(buf: *const core::ffi::c_void, buflen: u32) -> Self {
+        // SAFETY: There are no references or function pointers in this
+        // `Scatterlist`.
+        let mut sg: Scatterlist = unsafe { core::mem::zeroed() };
+        // SAFETY: we pass a valid sg entry, which points to stack-allocated
+        // variable above. `buf` and `buflen` are presumed valid as per this
+        // function's safety requirements.
+        unsafe {
+            bindings::sg_init_one(sg.inner.get_mut(), buf, buflen);
+        }
+
+        sg
+    }
+
+    pub(crate) fn inner(&self) -> &UnsafeCell<bindings::scatterlist> {
+        &self.inner
+    }
+}