[PATCH 0/2] rust: Add uapi crate

[PATCH 0/2] rust: Add uapi crate

[Asahi Lina]

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 modifies the ioctl crate (which I sent as a
separate series) to use the new crate. Miguel: if/once everything looks
good, feel free to merge just patch #1 and I can squash #2 into the ioctl
crate, or you can squash it yourself, or just merge both series in
sequence, whatever works ^^

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
Asahi Lina (2):
      rust: uapi: Add UAPI crate
      rust: ioctl: Move to the uapi crate
 rust/.gitignore         |  1 +
 rust/Makefile           | 18 ++++++++++++++++--
 rust/kernel/ioctl.rs    | 32 ++++++++++++++++----------------
 rust/kernel/lib.rs      |  1 +
 rust/uapi/lib.rs        | 27 +++++++++++++++++++++++++++
 rust/uapi/uapi_helper.h |  9 +++++++++
 6 files changed, 70 insertions(+), 18 deletions(-)
---
base-commit: 5aa20836252a607ce5d3fa96f9807b56e9f6b1ca
change-id: 20230329-rust-uapi-894421e9a5d2

Thank you,
~~ Lina

[PATCH 1/2] rust: uapi: Add UAPI crate

[Asahi Lina]

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").

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 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..59a327f0fa1a 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 7610b18ee642..63f796781b7c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -43,6 +43,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

[PATCH 2/2] rust: ioctl: Move to the uapi crate

[Asahi Lina]

Now that we have the uapi crate, this abstraction can use that instead
of bindings.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 rust/kernel/ioctl.rs | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
index b2076113b6a8..007437959395 100644
--- a/rust/kernel/ioctl.rs
+++ b/rust/kernel/ioctl.rs
@@ -10,40 +10,40 @@ 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 <= bindings::_IOC_DIRMASK);
-    build_assert!(ty <= bindings::_IOC_TYPEMASK);
-    build_assert!(nr <= bindings::_IOC_NRMASK);
-    build_assert!(size <= (bindings::_IOC_SIZEMASK as usize));
+    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 << bindings::_IOC_DIRSHIFT)
-        | (ty << bindings::_IOC_TYPESHIFT)
-        | (nr << bindings::_IOC_NRSHIFT)
-        | ((size as u32) << bindings::_IOC_SIZESHIFT)
+    (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(bindings::_IOC_NONE, ty, nr, 0)
+    _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(bindings::_IOC_READ, ty, nr, core::mem::size_of::<T>())
+    _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(bindings::_IOC_WRITE, ty, nr, core::mem::size_of::<T>())
+    _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(
-        bindings::_IOC_READ | bindings::_IOC_WRITE,
+        uapi::_IOC_READ | uapi::_IOC_WRITE,
         ty,
         nr,
         core::mem::size_of::<T>(),
@@ -52,20 +52,20 @@ pub const fn _IOWR<T>(ty: u32, nr: u32) -> u32 {
 
 /// Get the ioctl direction from an ioctl number.
 pub const fn _IOC_DIR(nr: u32) -> u32 {
-    (nr >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK
+    (nr >> uapi::_IOC_DIRSHIFT) & uapi::_IOC_DIRMASK
 }
 
 /// Get the ioctl type from an ioctl number.
 pub const fn _IOC_TYPE(nr: u32) -> u32 {
-    (nr >> bindings::_IOC_TYPESHIFT) & bindings::_IOC_TYPEMASK
+    (nr >> uapi::_IOC_TYPESHIFT) & uapi::_IOC_TYPEMASK
 }
 
 /// Get the ioctl number from an ioctl number.
 pub const fn _IOC_NR(nr: u32) -> u32 {
-    (nr >> bindings::_IOC_NRSHIFT) & bindings::_IOC_NRMASK
+    (nr >> uapi::_IOC_NRSHIFT) & uapi::_IOC_NRMASK
 }
 
 /// Get the ioctl size from an ioctl number.
 pub const fn _IOC_SIZE(nr: u32) -> usize {
-    ((nr >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK) as usize
+    ((nr >> uapi::_IOC_SIZESHIFT) & uapi::_IOC_SIZEMASK) as usize
 }

-- 
2.40.0

Re: [PATCH 1/2] rust: uapi: Add UAPI crate

[Martin Rodriguez Reboredo]

On 3/29/23 08:40, Asahi Lina wrote:
> 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").
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  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..59a327f0fa1a 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)

This emits a warning with grep about a numeral unnecessarily escaped as
seen in a patch sent by Vicenzo Palazzo [1].

```
grep: warning: stray \ before #
```

> +$(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 7610b18ee642..63f796781b7c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -43,6 +43,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>
> 

Link: https://lore.kernel.org/rust-for-linux/20230302132107.210502-1-vincenzopalazzodev@gmail.com/ [1]

Reviewed-by: Martin Rodriguez Reboredo

Re: [PATCH 2/2] rust: ioctl: Move to the uapi crate

[Martin Rodriguez Reboredo]

On 3/29/23 08:40, Asahi Lina wrote:
> Now that we have the uapi crate, this abstraction can use that instead
> of bindings.
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  rust/kernel/ioctl.rs | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
> index b2076113b6a8..007437959395 100644
> --- a/rust/kernel/ioctl.rs
> +++ b/rust/kernel/ioctl.rs
> @@ -10,40 +10,40 @@ 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 <= bindings::_IOC_DIRMASK);
> -    build_assert!(ty <= bindings::_IOC_TYPEMASK);
> -    build_assert!(nr <= bindings::_IOC_NRMASK);
> -    build_assert!(size <= (bindings::_IOC_SIZEMASK as usize));
> +    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 << bindings::_IOC_DIRSHIFT)
> -        | (ty << bindings::_IOC_TYPESHIFT)
> -        | (nr << bindings::_IOC_NRSHIFT)
> -        | ((size as u32) << bindings::_IOC_SIZESHIFT)
> +    (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(bindings::_IOC_NONE, ty, nr, 0)
> +    _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(bindings::_IOC_READ, ty, nr, core::mem::size_of::<T>())
> +    _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(bindings::_IOC_WRITE, ty, nr, core::mem::size_of::<T>())
> +    _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(
> -        bindings::_IOC_READ | bindings::_IOC_WRITE,
> +        uapi::_IOC_READ | uapi::_IOC_WRITE,
>          ty,
>          nr,
>          core::mem::size_of::<T>(),
> @@ -52,20 +52,20 @@ pub const fn _IOWR<T>(ty: u32, nr: u32) -> u32 {
>  
>  /// Get the ioctl direction from an ioctl number.
>  pub const fn _IOC_DIR(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK
> +    (nr >> uapi::_IOC_DIRSHIFT) & uapi::_IOC_DIRMASK
>  }
>  
>  /// Get the ioctl type from an ioctl number.
>  pub const fn _IOC_TYPE(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_TYPESHIFT) & bindings::_IOC_TYPEMASK
> +    (nr >> uapi::_IOC_TYPESHIFT) & uapi::_IOC_TYPEMASK
>  }
>  
>  /// Get the ioctl number from an ioctl number.
>  pub const fn _IOC_NR(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_NRSHIFT) & bindings::_IOC_NRMASK
> +    (nr >> uapi::_IOC_NRSHIFT) & uapi::_IOC_NRMASK
>  }
>  
>  /// Get the ioctl size from an ioctl number.
>  pub const fn _IOC_SIZE(nr: u32) -> usize {
> -    ((nr >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK) as usize
> +    ((nr >> uapi::_IOC_SIZESHIFT) & uapi::_IOC_SIZEMASK) as usize
>  }
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH 1/2] rust: uapi: Add UAPI crate

[Gary Guo]

On Wed, 29 Mar 2023 20:40:18 +0900
Asahi Lina <lina@asahilina.net> wrote:

> 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").
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  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..59a327f0fa1a 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 7610b18ee642..63f796781b7c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -43,6 +43,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))]

Looks like that we should bump our bindgen version.

> +#![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>
> 

Best,
Gary

Re: [PATCH 2/2] rust: ioctl: Move to the uapi crate

[Gary Guo]

On Wed, 29 Mar 2023 20:40:19 +0900
Asahi Lina <lina@asahilina.net> wrote:

> Now that we have the uapi crate, this abstraction can use that instead
> of bindings.

These functions really just reflect macros in uapi ioctl.h, maybe
these should just be part of the uapi crate?

Best,
Gary

> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
>  rust/kernel/ioctl.rs | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
> index b2076113b6a8..007437959395 100644
> --- a/rust/kernel/ioctl.rs
> +++ b/rust/kernel/ioctl.rs
> @@ -10,40 +10,40 @@ 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 <= bindings::_IOC_DIRMASK);
> -    build_assert!(ty <= bindings::_IOC_TYPEMASK);
> -    build_assert!(nr <= bindings::_IOC_NRMASK);
> -    build_assert!(size <= (bindings::_IOC_SIZEMASK as usize));
> +    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 << bindings::_IOC_DIRSHIFT)
> -        | (ty << bindings::_IOC_TYPESHIFT)
> -        | (nr << bindings::_IOC_NRSHIFT)
> -        | ((size as u32) << bindings::_IOC_SIZESHIFT)
> +    (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(bindings::_IOC_NONE, ty, nr, 0)
> +    _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(bindings::_IOC_READ, ty, nr, core::mem::size_of::<T>())
> +    _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(bindings::_IOC_WRITE, ty, nr, core::mem::size_of::<T>())
> +    _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(
> -        bindings::_IOC_READ | bindings::_IOC_WRITE,
> +        uapi::_IOC_READ | uapi::_IOC_WRITE,
>          ty,
>          nr,
>          core::mem::size_of::<T>(),
> @@ -52,20 +52,20 @@ pub const fn _IOWR<T>(ty: u32, nr: u32) -> u32 {
>  
>  /// Get the ioctl direction from an ioctl number.
>  pub const fn _IOC_DIR(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK
> +    (nr >> uapi::_IOC_DIRSHIFT) & uapi::_IOC_DIRMASK
>  }
>  
>  /// Get the ioctl type from an ioctl number.
>  pub const fn _IOC_TYPE(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_TYPESHIFT) & bindings::_IOC_TYPEMASK
> +    (nr >> uapi::_IOC_TYPESHIFT) & uapi::_IOC_TYPEMASK
>  }
>  
>  /// Get the ioctl number from an ioctl number.
>  pub const fn _IOC_NR(nr: u32) -> u32 {
> -    (nr >> bindings::_IOC_NRSHIFT) & bindings::_IOC_NRMASK
> +    (nr >> uapi::_IOC_NRSHIFT) & uapi::_IOC_NRMASK
>  }
>  
>  /// Get the ioctl size from an ioctl number.
>  pub const fn _IOC_SIZE(nr: u32) -> usize {
> -    ((nr >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK) as usize
> +    ((nr >> uapi::_IOC_SIZESHIFT) & uapi::_IOC_SIZEMASK) as usize
>  }
>