2023-03-23 12:16:37

by Asahi Lina

[permalink] [raw]
Subject: [PATCH v2] 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]>
---
Changes in v2:
- Changed from assert!() to build_assert!() (static_assert!() can't work
here)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
rust/bindings/bindings_helper.h | 3 +-
rust/kernel/ioctl.rs | 64 +++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 75d85bd6c592..aef60f300be0 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,8 +6,9 @@
* Sorted alphabetically.
*/

-#include <linux/slab.h>
+#include <linux/ioctl.h>
#include <linux/refcount.h>
+#include <linux/slab.h>

/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs
new file mode 100644
index 000000000000..6cd8e5738b91
--- /dev/null
+++ b/rust/kernel/ioctl.rs
@@ -0,0 +1,64 @@
+// 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)
+
+/// Build an ioctl number, analogous to the C macro of the same name.
+const fn _IOC(dir: u32, ty: u32, nr: u32, size: usize) -> u32 {
+ core::assert!(dir <= bindings::_IOC_DIRMASK);
+ core::assert!(ty <= bindings::_IOC_TYPEMASK);
+ core::assert!(nr <= bindings::_IOC_NRMASK);
+ core::assert!(size <= (bindings::_IOC_SIZEMASK as usize));
+
+ (dir << bindings::_IOC_DIRSHIFT)
+ | (ty << bindings::_IOC_TYPESHIFT)
+ | (nr << bindings::_IOC_NRSHIFT)
+ | ((size as u32) << bindings::_IOC_SIZESHIFT)
+}
+
+/// Build an ioctl number for an argumentless ioctl.
+pub const fn _IO(ty: u32, nr: u32) -> u32 {
+ _IOC(bindings::_IOC_NONE, ty, nr, 0)
+}
+
+/// Build an ioctl number for an read-only ioctl.
+pub const fn _IOR<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(bindings::_IOC_READ, ty, nr, core::mem::size_of::<T>())
+}
+
+/// Build an ioctl number for an write-only ioctl.
+pub const fn _IOW<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(bindings::_IOC_WRITE, ty, nr, core::mem::size_of::<T>())
+}
+
+/// Build an ioctl number for a read-write ioctl.
+pub const fn _IOWR<T>(ty: u32, nr: u32) -> u32 {
+ _IOC(
+ bindings::_IOC_READ | bindings::_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 >> bindings::_IOC_DIRSHIFT) & bindings::_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
+}
+
+/// Get the ioctl number from an ioctl number.
+pub const fn _IOC_NR(nr: u32) -> u32 {
+ (nr >> bindings::_IOC_NRSHIFT) & bindings::_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
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 223564f9f0cc..7610b18ee642 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;

---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230224-rust-ioctl-a520f3eb3aa8

Thank you,
~~ Lina


2023-03-23 12:22:22

by Arnd Bergmann

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

On Thu, Mar 23, 2023, at 13:08, Asahi Lina wrote:
> Changes in v2:
> - Changed from assert!() to build_assert!() (static_assert!() can't work here)
...
> +/// Build an ioctl number, analogous to the C macro of the same name.
> +const fn _IOC(dir: u32, ty: u32, nr: u32, size: usize) -> u32 {
> + core::assert!(dir <= bindings::_IOC_DIRMASK);
> + core::assert!(ty <= bindings::_IOC_TYPEMASK);
> + core::assert!(nr <= bindings::_IOC_NRMASK);
> + core::assert!(size <= (bindings::_IOC_SIZEMASK as usize));

Just to make sure: did you actually change it according
to the changelog? It still looks like a runtime assertion
to me, but I don't really understand any rust.

Arnd

2023-03-23 12:32:59

by Asahi Lina

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

On 23/03/2023 21.18, Arnd Bergmann wrote:
> On Thu, Mar 23, 2023, at 13:08, Asahi Lina wrote:
>> Changes in v2:
>> - Changed from assert!() to build_assert!() (static_assert!() can't work here)
> ...
>> +/// Build an ioctl number, analogous to the C macro of the same name.
>> +const fn _IOC(dir: u32, ty: u32, nr: u32, size: usize) -> u32 {
>> + core::assert!(dir <= bindings::_IOC_DIRMASK);
>> + core::assert!(ty <= bindings::_IOC_TYPEMASK);
>> + core::assert!(nr <= bindings::_IOC_NRMASK);
>> + core::assert!(size <= (bindings::_IOC_SIZEMASK as usize));
>
> Just to make sure: did you actually change it according
> to the changelog? It still looks like a runtime assertion
> to me, but I don't really understand any rust.

Umm... I'm not sure what happened there.

Sorry, I'll resend it. I ran into some unrelated pain with bindgen
versions while trying to compile-test this, and along the way I must
have somehow dropped the actual v2 change...

~~ Lina