2023-04-03 09:38:53

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v2 0/2] rust: Add uapi crate

In general, direct bindgen bindings for C kernel APIs are not intended
to be used by drivers outside of the `kernel` crate. However, some
drivers do need to interact directly with UAPI definitions to implement
userspace APIs.

Instead of making this an exception to the use of the `bindings` crate,
introduce a new `uapi` crate that will contain only these publicly
usable definitions. The build logic mirrors the `bindings` crate, but
there is no helper support since UAPIs are only intended to contain
constant and type definitions, not function prototypes.

In the future, we would like to extend this to also auto-derive and
validate certain safety properties for UAPI structure definitions, such
as that they are safely castable to/from "bag of bits" buffers.

The first patch introduces the `uapi` crate proper and stands on its own,
while the second patch introduces the `ioctl` module in the kernel crate
that uses it.

(Miguel: this series should apply stand-alone now)

Signed-off-by: Asahi Lina <[email protected]>
---
Changes in v2:
- Fixed the trailing '\' warning (mirroring the fix from Vicenzo
Palazzo)
- Pulled in the ioctl module patch into this series. Per the discussion
in the meeting, this could be moved into the uapi crate in principle,
but we can decide to do that later since the whole buildsystem is
likely to undergo major changes anyway.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Asahi Lina (2):
rust: uapi: Add UAPI crate
rust: ioctl: Add ioctl number manipulation functions
rust/.gitignore | 1 +
rust/Makefile | 18 +++++++++++--
rust/kernel/ioctl.rs | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 ++
rust/uapi/lib.rs | 27 +++++++++++++++++++
rust/uapi/uapi_helper.h | 9 +++++++
6 files changed, 126 insertions(+), 2 deletions(-)
---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230329-rust-uapi-894421e9a5d2

Thank you,
~~ Lina


2023-04-03 09:39:52

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v2 2/2] rust: ioctl: Add ioctl number manipulation functions

Add simple 1:1 wrappers of the C ioctl number manipulation functions.
Since these are macros we cannot bindgen them directly, and since they
should be usable in const context we cannot use helper wrappers, so
we'll have to reimplement them in Rust. Thankfully, the C headers do
declare defines for the relevant bitfield positions, so we don't need
to duplicate that.

Signed-off-by: Asahi Lina <[email protected]>
---
rust/kernel/ioctl.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
2 files changed, 72 insertions(+)

diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
new file mode 100644
index 000000000000..007437959395
--- /dev/null
+++ b/rust/kernel/ioctl.rs
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+#![allow(non_snake_case)]
+
+//! ioctl() number definitions
+//!
+//! C header: [`include/asm-generic/ioctl.h`](../../../../include/asm-generic/ioctl.h)
+
+use crate::build_assert;
+
+/// Build an ioctl number, analogous to the C macro of the same name.
+#[inline(always)]
+const fn _IOC(dir: u32, ty: u32, nr: u32, size: usize) -> u32 {
+ build_assert!(dir <= uapi::_IOC_DIRMASK);
+ build_assert!(ty <= uapi::_IOC_TYPEMASK);
+ build_assert!(nr <= uapi::_IOC_NRMASK);
+ build_assert!(size <= (uapi::_IOC_SIZEMASK as usize));
+
+ (dir << uapi::_IOC_DIRSHIFT)
+ | (ty << uapi::_IOC_TYPESHIFT)
+ | (nr << uapi::_IOC_NRSHIFT)
+ | ((size as u32) << uapi::_IOC_SIZESHIFT)
+}
+
+/// Build an ioctl number for an argumentless ioctl.
+#[inline(always)]
+pub const fn _IO(ty: u32, nr: u32) -> u32 {
+ _IOC(uapi::_IOC_NONE, ty, nr, 0)
+}
+
+/// Build an ioctl number for an read-only ioctl.
+#[inline(always)]
+pub const fn _IOR<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(uapi::_IOC_READ, ty, nr, core::mem::size_of::<T>())
+}
+
+/// Build an ioctl number for an write-only ioctl.
+#[inline(always)]
+pub const fn _IOW<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(uapi::_IOC_WRITE, ty, nr, core::mem::size_of::<T>())
+}
+
+/// Build an ioctl number for a read-write ioctl.
+#[inline(always)]
+pub const fn _IOWR<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(
+ uapi::_IOC_READ | uapi::_IOC_WRITE,
+ ty,
+ nr,
+ core::mem::size_of::<T>(),
+ )
+}
+
+/// Get the ioctl direction from an ioctl number.
+pub const fn _IOC_DIR(nr: u32) -> u32 {
+ (nr >> uapi::_IOC_DIRSHIFT) & uapi::_IOC_DIRMASK
+}
+
+/// Get the ioctl type from an ioctl number.
+pub const fn _IOC_TYPE(nr: u32) -> u32 {
+ (nr >> uapi::_IOC_TYPESHIFT) & uapi::_IOC_TYPEMASK
+}
+
+/// Get the ioctl number from an ioctl number.
+pub const fn _IOC_NR(nr: u32) -> u32 {
+ (nr >> uapi::_IOC_NRSHIFT) & uapi::_IOC_NRMASK
+}
+
+/// Get the ioctl size from an ioctl number.
+pub const fn _IOC_SIZE(nr: u32) -> usize {
+ ((nr >> uapi::_IOC_SIZESHIFT) & uapi::_IOC_SIZEMASK) as usize
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index afec0792d982..63f796781b7c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -30,6 +30,7 @@ compile_error!("Missing kernel configuration for conditional compilation");
mod allocator;
mod build_assert;
pub mod error;
+pub mod ioctl;
pub mod prelude;
pub mod print;
mod static_assert;

--
2.40.0

2023-04-03 09:41:02

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v2 1/2] rust: uapi: Add UAPI crate

This crate mirrors the `bindings` crate, but will contain only UAPI
bindings. Unlike the bindings crate, drivers may directly use this crate
if they have to interface with userspace.

Initially, just bind the generic ioctl stuff.

In the future, we would also like to add additional checks to ensure
that all types exposed by this crate satisfy UAPI-safety guarantees
(that is, they are safely castable to/from a "bag of bits").

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
Signed-off-by: Asahi Lina <[email protected]>
---
rust/.gitignore | 1 +
rust/Makefile | 18 ++++++++++++++++--
rust/kernel/lib.rs | 1 +
rust/uapi/lib.rs | 27 +++++++++++++++++++++++++++
rust/uapi/uapi_helper.h | 9 +++++++++
5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/rust/.gitignore b/rust/.gitignore
index 168cb26a31b9..21552992b401 100644
--- a/rust/.gitignore
+++ b/rust/.gitignore
@@ -2,6 +2,7 @@

bindings_generated.rs
bindings_helpers_generated.rs
+uapi_generated.rs
exports_*_generated.h
doc/
test/
diff --git a/rust/Makefile b/rust/Makefile
index f88d108fbef0..f81fdefec0ab 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -16,6 +16,9 @@ obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
always-$(CONFIG_RUST) += exports_alloc_generated.h exports_bindings_generated.h \
exports_kernel_generated.h

+always-$(CONFIG_RUST) += uapi/uapi_generated.rs
+obj-$(CONFIG_RUST) += uapi.o
+
ifdef CONFIG_RUST_BUILD_ASSERT_ALLOW
obj-$(CONFIG_RUST) += build_error.o
else
@@ -288,6 +291,12 @@ $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \
$(src)/bindgen_parameters FORCE
$(call if_changed_dep,bindgen)

+$(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \
+ $(shell grep -v '^#\|^$$' $(srctree)/$(src)/bindgen_parameters)
+$(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \
+ $(src)/bindgen_parameters FORCE
+ $(call if_changed_dep,bindgen)
+
# See `CFLAGS_REMOVE_helpers.o` above. In addition, Clang on C does not warn
# with `-Wmissing-declarations` (unlike GCC), so it is not strictly needed here
# given it is `libclang`; but for consistency, future Clang changes and/or
@@ -388,10 +397,15 @@ $(obj)/bindings.o: $(src)/bindings/lib.rs \
$(obj)/bindings/bindings_helpers_generated.rs FORCE
$(call if_changed_dep,rustc_library)

+$(obj)/uapi.o: $(src)/uapi/lib.rs \
+ $(obj)/compiler_builtins.o \
+ $(obj)/uapi/uapi_generated.rs FORCE
+ $(call if_changed_dep,rustc_library)
+
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
- --extern build_error --extern macros --extern bindings
+ --extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
- $(obj)/libmacros.so $(obj)/bindings.o FORCE
+ $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
$(call if_changed_dep,rustc_library)

endif # CONFIG_RUST
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 223564f9f0cc..afec0792d982 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -42,6 +42,7 @@ pub mod types;
#[doc(hidden)]
pub use bindings;
pub use macros;
+pub use uapi;

#[doc(hidden)]
pub use build_error::build_error;
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
new file mode 100644
index 000000000000..29f69f3a52de
--- /dev/null
+++ b/rust/uapi/lib.rs
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! UAPI Bindings.
+//!
+//! Contains the bindings generated by `bindgen` for UAPI interfaces.
+//!
+//! This crate may be used directly by drivers that need to interact with
+//! userspace APIs.
+
+#![no_std]
+#![feature(core_ffi_c)]
+// See <https://github.com/rust-lang/rust-bindgen/issues/1651>.
+#![cfg_attr(test, allow(deref_nullptr))]
+#![cfg_attr(test, allow(unaligned_references))]
+#![cfg_attr(test, allow(unsafe_op_in_unsafe_fn))]
+#![allow(
+ clippy::all,
+ missing_docs,
+ non_camel_case_types,
+ non_upper_case_globals,
+ non_snake_case,
+ improper_ctypes,
+ unreachable_pub,
+ unsafe_op_in_unsafe_fn
+)]
+
+include!(concat!(env!("OBJTREE"), "/rust/uapi/uapi_generated.rs"));
diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
new file mode 100644
index 000000000000..301f5207f023
--- /dev/null
+++ b/rust/uapi/uapi_helper.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header that contains the headers for which Rust UAPI bindings
+ * will be automatically generated by `bindgen`.
+ *
+ * Sorted alphabetically.
+ */
+
+#include <uapi/asm-generic/ioctl.h>

--
2.40.0

2023-04-21 23:51:00

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] rust: Add uapi crate

On Mon, Apr 3, 2023 at 11:34 AM Asahi Lina <[email protected]> wrote:
>
> The first patch introduces the `uapi` crate proper and stands on its own,
> while the second patch introduces the `ioctl` module in the kernel crate
> that uses it.

Applied to `rust-next`, with added support for the `rustdoc` and
`rusttest` targets, since we want to keep those working. Thanks!

Cheers,
Miguel