2023-04-03 09:52:51

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 0/6] rust: error: Add missing wrappers to convert to/from kernel error codes

Hi everyone!

This series is part of the set of dependencies for the drm/asahi
Apple M1/M2 GPU driver.

It adds a bunch of missing wrappers in kernel::error, which are useful
to convert to/from kernel error codes. Since these will be used by many
abstractions coming up soon, I think it makes sense to merge them as
soon as possible instead of bundling them with the first user. Hence,
they have allow() tags to silence dead code warnings. These can be
removed as soon as the first user is in the kernel crate.

Getting this in first allows the subsequent abstractions to be merged in
any order, so we don't have to worry about piecewise rebasing and fixing
conflicts in the Error wrappers. See [1] for a complete tree with the DRM
abstractions and all other miscellaneous work-in-progress prerequisites
rebased on top of mainline.

Most of these have been extracted from the rust-for-linux/rust branch,
with author attribution to the first/primary author and Co-developed-by:
for everyone else who touched the code.

Attribution changes:
- One of the patches had Miguel's old email in the tags, updated that per
his request.
- Wedson's email changed from @google.com to @gmail.com (I understand
this is the current one).

Sven: There is one patch from you in this series, do you want to send it
yourself directly? I understand Wedson and Miguel are okay with me
sending stuff on their behalf.

[1] https://github.com/Rust-for-Linux/linux/pull/969/commits

Signed-off-by: Asahi Lina <[email protected]>
---
Changes in v3:
- Re-introduced from_errno_unchecked() (but private to this module since
it has no outside users) and used it in from_err_ptr().
- Updated safety comment in to_ptr().
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Removed the redundant _kernel from various function names.
- Replaced the from_result!{} macro with a from_result() function taking a closure.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Asahi Lina (2):
rust: error: Rename to_kernel_errno() -> to_errno()
rust: error: Add Error::to_ptr()

Miguel Ojeda (1):
rust: error: Add Error::from_errno{_unchecked}()

Sven Van Asbroeck (1):
rust: error: Add a helper to convert a C ERR_PTR to a `Result`

Wedson Almeida Filho (2):
rust: error: Add to_result() helper
rust: error: Add from_result() helper
rust/helpers.c | 19 +++++++
rust/kernel/error.rs | 137 +++++++++++++++++++++++++++++++++++++++++++++++++-
rust/macros/module.rs | 2 +-
3 files changed, 156 insertions(+), 2 deletions(-)
---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230224-rust-error-f2871fbc907f

Thank you,
~~ Lina


2023-04-03 09:52:52

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 1/6] rust: error: Rename to_kernel_errno() -> to_errno()

This is kernel code, so specifying "kernel" is redundant. Let's simplify
things and just call it to_errno().

Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/kernel/error.rs | 2 +-
rust/macros/module.rs | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 5b9751d7ff1d..35894fa35efe 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -73,7 +73,7 @@ pub struct Error(core::ffi::c_int);

impl Error {
/// Returns the kernel error code.
- pub fn to_kernel_errno(self) -> core::ffi::c_int {
+ pub fn to_errno(self) -> core::ffi::c_int {
self.0
}
}
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index a7e363c2b044..143336543866 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -258,7 +258,7 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
return 0;
}}
Err(e) => {{
- return e.to_kernel_errno();
+ return e.to_errno();
}}
}}
}}

--
2.40.0

2023-04-03 09:53:15

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 4/6] rust: error: Add to_result() helper

From: Wedson Almeida Filho <[email protected]>

Add a to_result() helper to convert kernel C return values to a Rust
Result, mapping >=0 values to Ok(()) and negative values to Err(...),
with Error::from_errno() ensuring that the errno is within range.

Lina: Imported from rust-for-linux/rust, originally developed by Wedson
as part of the AMBA device driver support.

Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Andreas Hindborg <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/kernel/error.rs | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 1af0d75d3a73..e8697ad6b62e 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -178,3 +178,13 @@ impl From<core::convert::Infallible> for Error {
/// it should still be modeled as returning a `Result` rather than
/// just an [`Error`].
pub type Result<T = ()> = core::result::Result<T, Error>;
+
+/// Converts an integer as returned by a C kernel function to an error if it's negative, and
+/// `Ok(())` otherwise.
+pub fn to_result(err: core::ffi::c_int) -> Result {
+ if err < 0 {
+ Err(Error::from_errno(err))
+ } else {
+ Ok(())
+ }
+}

--
2.40.0

2023-04-03 09:53:20

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 2/6] rust: error: Add Error::to_ptr()

This is the Rust equivalent to ERR_PTR(), for use in C callbacks.
Marked as #[allow(dead_code)] for now, since it does not have any
consumers yet.

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/helpers.c | 7 +++++++
rust/kernel/error.rs | 7 +++++++
2 files changed, 14 insertions(+)

diff --git a/rust/helpers.c b/rust/helpers.c
index 09a4d93f9d62..89f4cd1e0df3 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -20,6 +20,7 @@

#include <linux/bug.h>
#include <linux/build_bug.h>
+#include <linux/err.h>
#include <linux/refcount.h>

__noreturn void rust_helper_BUG(void)
@@ -46,6 +47,12 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
}
EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);

+__force void *rust_helper_ERR_PTR(long err)
+{
+ return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(rust_helper_ERR_PTR);
+
/*
* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
* as the Rust `usize` type, so we can use it in contexts where Rust
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 35894fa35efe..154d0ca6e2dc 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -76,6 +76,13 @@ impl Error {
pub fn to_errno(self) -> core::ffi::c_int {
self.0
}
+
+ /// Returns the error encoded as a pointer.
+ #[allow(dead_code)]
+ pub(crate) fn to_ptr<T>(self) -> *mut T {
+ // SAFETY: self.0 is a valid error due to its invariant.
+ unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
+ }
}

impl From<AllocError> for Error {

--
2.40.0

2023-04-03 09:53:23

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 5/6] rust: error: Add a helper to convert a C ERR_PTR to a `Result`

From: Sven Van Asbroeck <[email protected]>

Some kernel C API functions return a pointer which embeds an optional
`errno`. Callers are supposed to check the returned pointer with
`IS_ERR()` and if this returns `true`, retrieve the `errno` using
`PTR_ERR()`.

Create a Rust helper function to implement the Rust equivalent:
transform a `*mut T` to `Result<*mut T>`.

Lina: Imported from rust-for-linux/linux, with subsequent refactoring
and contributions squashed in and attributed below. Renamed the function
to from_err_ptr().

Co-developed-by: Boqun Feng <[email protected]>
Signed-off-by: Boqun Feng <[email protected]>
Co-developed-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Co-developed-by: Fox Chen <[email protected]>
Signed-off-by: Fox Chen <[email protected]>
Co-developed-by: Gary Guo <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
Signed-off-by: Sven Van Asbroeck <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/helpers.c | 12 ++++++++++++
rust/kernel/error.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)

diff --git a/rust/helpers.c b/rust/helpers.c
index 89f4cd1e0df3..04b9be46e887 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -53,6 +53,18 @@ __force void *rust_helper_ERR_PTR(long err)
}
EXPORT_SYMBOL_GPL(rust_helper_ERR_PTR);

+bool rust_helper_IS_ERR(__force const void *ptr)
+{
+ return IS_ERR(ptr);
+}
+EXPORT_SYMBOL_GPL(rust_helper_IS_ERR);
+
+long rust_helper_PTR_ERR(__force const void *ptr)
+{
+ return PTR_ERR(ptr);
+}
+EXPORT_SYMBOL_GPL(rust_helper_PTR_ERR);
+
/*
* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
* as the Rust `usize` type, so we can use it in contexts where Rust
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index e8697ad6b62e..67637b874267 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -188,3 +188,52 @@ pub fn to_result(err: core::ffi::c_int) -> Result {
Ok(())
}
}
+
+/// Transform a kernel "error pointer" to a normal pointer.
+///
+/// Some kernel C API functions return an "error pointer" which optionally
+/// embeds an `errno`. Callers are supposed to check the returned pointer
+/// for errors. This function performs the check and converts the "error pointer"
+/// to a normal pointer in an idiomatic fashion.
+///
+/// # Examples
+///
+/// ```ignore
+/// # use kernel::from_err_ptr;
+/// # use kernel::bindings;
+/// fn devm_platform_ioremap_resource(
+/// pdev: &mut PlatformDevice,
+/// index: u32,
+/// ) -> Result<*mut core::ffi::c_void> {
+/// // SAFETY: FFI call.
+/// unsafe {
+/// from_err_ptr(bindings::devm_platform_ioremap_resource(
+/// pdev.to_ptr(),
+/// index,
+/// ))
+/// }
+/// }
+/// ```
+// TODO: Remove `dead_code` marker once an in-kernel client is available.
+#[allow(dead_code)]
+pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
+ // CAST: Casting a pointer to `*const core::ffi::c_void` is always valid.
+ let const_ptr: *const core::ffi::c_void = ptr.cast();
+ // SAFETY: The FFI function does not deref the pointer.
+ if unsafe { bindings::IS_ERR(const_ptr) } {
+ // SAFETY: The FFI function does not deref the pointer.
+ let err = unsafe { bindings::PTR_ERR(const_ptr) };
+ // CAST: If `IS_ERR()` returns `true`,
+ // then `PTR_ERR()` is guaranteed to return a
+ // negative value greater-or-equal to `-bindings::MAX_ERRNO`,
+ // which always fits in an `i16`, as per the invariant above.
+ // And an `i16` always fits in an `i32`. So casting `err` to
+ // an `i32` can never overflow, and is always valid.
+ //
+ // SAFETY: `IS_ERR()` ensures `err` is a
+ // negative value greater-or-equal to `-bindings::MAX_ERRNO`.
+ #[allow(clippy::unnecessary_cast)]
+ return Err(unsafe { Error::from_errno_unchecked(err as core::ffi::c_int) });
+ }
+ Ok(ptr)
+}

--
2.40.0

2023-04-03 09:53:23

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v3 6/6] rust: error: Add from_result() helper

From: Wedson Almeida Filho <[email protected]>

Add a helper function to easily return C result codes from a Rust function
that calls functions which return a Result<T>.

Lina: Imported from rust-for-linux/rust, originally developed by Wedson
as part of file_operations.rs. Added the allow() flags since there is no
user in the kernel crate yet and fixed a typo in a comment. Replaced the
macro with a function taking a closure, per discussion on the ML.

Co-developed-by: Fox Chen <[email protected]>
Signed-off-by: Fox Chen <[email protected]>
Co-developed-by: Miguel Ojeda <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/kernel/error.rs | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 67637b874267..5f4114b30b94 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -237,3 +237,42 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
}
Ok(ptr)
}
+
+/// Calls a closure returning a [`crate::error::Result<T>`] and converts the result to
+/// a C integer result.
+///
+/// This is useful when calling Rust functions that return [`crate::error::Result<T>`]
+/// from inside `extern "C"` functions that need to return an integer error result.
+///
+/// `T` should be convertible from an `i16` via `From<i16>`.
+///
+/// # Examples
+///
+/// ```ignore
+/// # use kernel::from_result;
+/// # use kernel::bindings;
+/// unsafe extern "C" fn probe_callback(
+/// pdev: *mut bindings::platform_device,
+/// ) -> core::ffi::c_int {
+/// from_result(|| {
+/// let ptr = devm_alloc(pdev)?;
+/// bindings::platform_set_drvdata(pdev, ptr);
+/// Ok(0)
+/// })
+/// }
+/// ```
+// TODO: Remove `dead_code` marker once an in-kernel client is available.
+#[allow(dead_code)]
+pub(crate) fn from_result<T, F>(f: F) -> T
+where
+ T: From<i16>,
+ F: FnOnce() -> Result<T>,
+{
+ match f() {
+ Ok(v) => v,
+ // NO-OVERFLOW: negative `errno`s are no smaller than `-bindings::MAX_ERRNO`,
+ // `-bindings::MAX_ERRNO` fits in an `i16` as per invariant above,
+ // therefore a negative `errno` always fits in an `i16` and will not overflow.
+ Err(e) => T::from(e.to_errno() as i16),
+ }
+}

--
2.40.0

2023-04-03 15:35:40

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] rust: error: Add Error::to_ptr()

On Mon, 03 Apr 2023 18:48:11 +0900
Asahi Lina <[email protected]> wrote:

> This is the Rust equivalent to ERR_PTR(), for use in C callbacks.
> Marked as #[allow(dead_code)] for now, since it does not have any
> consumers yet.
>
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
> Signed-off-by: Asahi Lina <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

> ---
> rust/helpers.c | 7 +++++++
> rust/kernel/error.rs | 7 +++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 09a4d93f9d62..89f4cd1e0df3 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/err.h>
> #include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> @@ -46,6 +47,12 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
> }
> EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
>
> +__force void *rust_helper_ERR_PTR(long err)
> +{
> + return ERR_PTR(err);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ERR_PTR);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 35894fa35efe..154d0ca6e2dc 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -76,6 +76,13 @@ impl Error {
> pub fn to_errno(self) -> core::ffi::c_int {
> self.0
> }
> +
> + /// Returns the error encoded as a pointer.
> + #[allow(dead_code)]
> + pub(crate) fn to_ptr<T>(self) -> *mut T {
> + // SAFETY: self.0 is a valid error due to its invariant.
> + unsafe { bindings::ERR_PTR(self.0.into()) as *mut _ }
> + }
> }
>
> impl From<AllocError> for Error {
>

2023-04-10 03:56:55

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] rust: error: Add missing wrappers to convert to/from kernel error codes

On Mon, Apr 3, 2023 at 11:48 AM Asahi Lina <[email protected]> wrote:
>
> Hi everyone!
>
> This series is part of the set of dependencies for the drm/asahi
> Apple M1/M2 GPU driver.
>
> It adds a bunch of missing wrappers in kernel::error, which are useful
> to convert to/from kernel error codes. Since these will be used by many
> abstractions coming up soon, I think it makes sense to merge them as
> soon as possible instead of bundling them with the first user. Hence,
> they have allow() tags to silence dead code warnings. These can be
> removed as soon as the first user is in the kernel crate.
>
> Getting this in first allows the subsequent abstractions to be merged in
> any order, so we don't have to worry about piecewise rebasing and fixing
> conflicts in the Error wrappers. See [1] for a complete tree with the DRM
> abstractions and all other miscellaneous work-in-progress prerequisites
> rebased on top of mainline.
>
> Most of these have been extracted from the rust-for-linux/rust branch,
> with author attribution to the first/primary author and Co-developed-by:
> for everyone else who touched the code.
>
> Attribution changes:
> - One of the patches had Miguel's old email in the tags, updated that per
> his request.
> - Wedson's email changed from @google.com to @gmail.com (I understand
> this is the current one).
>
> Sven: There is one patch from you in this series, do you want to send it
> yourself directly? I understand Wedson and Miguel are okay with me
> sending stuff on their behalf.
>
> [1] https://github.com/Rust-for-Linux/linux/pull/969/commits

Added (and then removed in later commits) two `#[allow(dead_code)]`s
as needed to make it compile.

Applied to `rust-next`. Thanks!

Cheers,
Miguel