rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] rust: ioctl: Add ioctl number manipulation functions
@ 2023-03-23 12:33 Asahi Lina
  2023-03-23 13:05 ` Miguel Ojeda
  0 siblings, 1 reply; 3+ messages in thread
From: Asahi Lina @ 2023-03-23 12:33 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Arnd Bergmann
  Cc: linux-kernel, rust-for-linux, asahi, linux-arch, Asahi Lina

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 <lina@asahilina.net>
---
Changes in v3:
- Actually made the change intended in v2.
- Link to v2: https://lore.kernel.org/r/20230224-rust-ioctl-v2-1-5325e76a92df@asahilina.net

Changes in v2:
- Changed from assert!() to build_assert!() (static_assert!() can't work
  here)
- Link to v1: https://lore.kernel.org/r/20230224-rust-ioctl-v1-1-5142d365a934@asahilina.net
---
 rust/bindings/bindings_helper.h |  3 +-
 rust/kernel/ioctl.rs            | 71 +++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |  1 +
 3 files changed, 74 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..b2076113b6a8
--- /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 <= bindings::_IOC_DIRMASK);
+    build_assert!(ty <= bindings::_IOC_TYPEMASK);
+    build_assert!(nr <= bindings::_IOC_NRMASK);
+    build_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.
+#[inline(always)]
+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.
+#[inline(always)]
+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.
+#[inline(always)]
+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.
+#[inline(always)]
+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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] rust: ioctl: Add ioctl number manipulation functions
  2023-03-23 12:33 [PATCH v3] rust: ioctl: Add ioctl number manipulation functions Asahi Lina
@ 2023-03-23 13:05 ` Miguel Ojeda
  2023-03-23 14:00   ` Asahi Lina
  0 siblings, 1 reply; 3+ messages in thread
From: Miguel Ojeda @ 2023-03-23 13:05 UTC (permalink / raw)
  To: Asahi Lina
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Arnd Bergmann, linux-kernel,
	rust-for-linux, asahi, linux-arch

On Thu, Mar 23, 2023 at 1:34 PM Asahi Lina <lina@asahilina.net> wrote:
>
> Changes in v3:
> - Actually made the change intended in v2.
> - Link to v2: https://lore.kernel.org/r/20230224-rust-ioctl-v2-1-5325e76a92df@asahilina.net
>
> Changes in v2:
> - Changed from assert!() to build_assert!() (static_assert!() can't work
>   here)
> - Link to v1: https://lore.kernel.org/r/20230224-rust-ioctl-v1-1-5142d365a934@asahilina.net

It seems `#[inline(always)]` got added to a few of those, right? (The
addition looks fine to me, but just to understand if is it an omission
in the changelog, or an unintended change, or intended for another
reason).

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] rust: ioctl: Add ioctl number manipulation functions
  2023-03-23 13:05 ` Miguel Ojeda
@ 2023-03-23 14:00   ` Asahi Lina
  0 siblings, 0 replies; 3+ messages in thread
From: Asahi Lina @ 2023-03-23 14:00 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Arnd Bergmann, linux-kernel,
	rust-for-linux, asahi, linux-arch

On 23/03/2023 22.05, Miguel Ojeda wrote:
> On Thu, Mar 23, 2023 at 1:34 PM Asahi Lina <lina@asahilina.net> wrote:
>>
>> Changes in v3:
>> - Actually made the change intended in v2.
>> - Link to v2: https://lore.kernel.org/r/20230224-rust-ioctl-v2-1-5325e76a92df@asahilina.net
>>
>> Changes in v2:
>> - Changed from assert!() to build_assert!() (static_assert!() can't work
>>    here)
>> - Link to v1: https://lore.kernel.org/r/20230224-rust-ioctl-v1-1-5142d365a934@asahilina.net
> 
> It seems `#[inline(always)]` got added to a few of those, right? (The
> addition looks fine to me, but just to understand if is it an omission
> in the changelog, or an unintended change, or intended for another
> reason).

Ah yes, I should've mentioned that! build_assert!() only works for stuff 
that can be const-optimized at build time, which requires inlining. 
Otherwise this change immediately causes build failures since generic 
variants of the functions get compiled (and since they're public 
functions, cannot be optimized out at link time).

> 
> Thanks!
> 
> Cheers,
> Miguel
> 

~~ Lina


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-23 14:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23 12:33 [PATCH v3] rust: ioctl: Add ioctl number manipulation functions Asahi Lina
2023-03-23 13:05 ` Miguel Ojeda
2023-03-23 14:00   ` Asahi Lina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).