* [PATCH 0/5] use custom FFI integer types
@ 2024-09-13 21:29 Gary Guo
2024-09-13 21:29 ` [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins Gary Guo
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross
Cc: rust-for-linux
This patch series aim to reduce the unnecessary type casts needed in
Rust code doing FFI calls.
With this series, we can ensure:
* `size_t` is mapped to usize. Currently this is mostly true except for
C builtin functions, which gets translated by bindgen to c_ulong/c_uint.
* `__kernel_size_t` is mapped to usize. Currently this is mapped to
c_ulong/c_uint.
* `unsigned long` is mapped to usize. Currently this is mapped by Rust
libcore to either u32 or u64.
* `char` is mapped to `u8`. Currently this is mapped by Rust libcore to
either i8 or u8.
After this series, FFI code needs to use `kernel::ffi` types instead of
`core::ffi`.
Gary Guo (5):
rust: fix size_t in bindgen prototypes of C builtins
rust: map `__kernel_size_t` and friends also to usize/isize
rust: use custom FFI integer types
rust: map `long` to `isize` and `char` to `u8`
rust: cleanup unnecessary casts
rust/Makefile | 24 ++++++++++-----
rust/bindgen_parameters | 5 ++++
rust/bindings/lib.rs | 5 ++++
rust/ffi.rs | 48 ++++++++++++++++++++++++++++++
rust/kernel/alloc/allocator.rs | 4 +--
rust/kernel/block/mq/operations.rs | 18 +++++------
rust/kernel/block/mq/raw_writer.rs | 2 +-
rust/kernel/block/mq/tag_set.rs | 2 +-
rust/kernel/error.rs | 25 +++++++---------
rust/kernel/firmware.rs | 2 +-
rust/kernel/init.rs | 2 +-
rust/kernel/kunit.rs | 10 ++-----
rust/kernel/lib.rs | 2 ++
rust/kernel/net/phy.rs | 14 ++++-----
rust/kernel/print.rs | 4 +--
rust/kernel/str.rs | 10 +++----
rust/kernel/sync/arc.rs | 6 ++--
rust/kernel/sync/condvar.rs | 2 +-
rust/kernel/sync/lock.rs | 2 +-
rust/kernel/sync/lock/mutex.rs | 2 +-
rust/kernel/sync/lock/spinlock.rs | 2 +-
rust/kernel/task.rs | 8 ++---
rust/kernel/time.rs | 4 +--
rust/kernel/types.rs | 26 ++++++++--------
rust/kernel/uaccess.rs | 31 ++++++-------------
rust/macros/module.rs | 8 ++---
rust/uapi/lib.rs | 5 ++++
27 files changed, 161 insertions(+), 112 deletions(-)
create mode 100644 rust/ffi.rs
base-commit: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2
--
2.44.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo @ 2024-09-13 21:29 ` Gary Guo 2024-09-29 21:00 ` Trevor Gross 2024-09-13 21:29 ` [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize Gary Guo ` (5 subsequent siblings) 6 siblings, 1 reply; 20+ messages in thread From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt Cc: rust-for-linux, linux-kernel, llvm Without `-fno-builtin`, for functions like memcpy/memmove (and many others), bindgen seems to be using the clang-provided prototype. This prototype is ABI-wise compatible, but the issue is that it does not have the same information as the source code w.r.t. typedefs. For example, bindgen generates the following: extern "C" { pub fn strlen(s: *const core::ffi::c_char) -> core::ffi::c_ulong; } note that the return type is `c_ulong` (i.e. unsigned long), despite the size_t-is-usize behavior (this is default, and we have not opted out from it using --no-size_t-is-usize). Similarly, memchr's size argument should be of type `__kernel_size_t`, but bindgen generates `c_ulong` directly. We want to ensure any `size_t` is translated to Rust `usize` so that we can avoid having them be different type on 32-bit and 64-bit architectures, and hence would require a lot of excessive type casts when calling FFI functions. I found that this bindgen behavior (which probably is caused by libclang) can be disabled by `-fno-builtin`. Using the flag for compiled code can result in less optimisation because compiler cannot assume about their properties anymore, but this should not affect bindgen. Signed-off-by: Gary Guo <gary@garyguo.net> --- rust/Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rust/Makefile b/rust/Makefile index 4eae318f36ff7..863d87ad0ce68 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -265,7 +265,11 @@ else bindgen_c_flags_lto = $(bindgen_c_flags) endif -bindgen_c_flags_final = $(bindgen_c_flags_lto) -D__BINDGEN__ +# `-fno-builtin` is passed to avoid bindgen from using clang builtin prototypes +# for functions like `memcpy` -- if this flag is not passed, bindgen-generated +# prototypes use `c_ulong` or `c_uint` depending on architecture instead of +# generating `usize`. +bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__ quiet_cmd_bindgen = BINDGEN $@ cmd_bindgen = \ -- 2.44.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins 2024-09-13 21:29 ` [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins Gary Guo @ 2024-09-29 21:00 ` Trevor Gross 2024-10-05 22:10 ` Gary Guo 0 siblings, 1 reply; 20+ messages in thread From: Trevor Gross @ 2024-09-29 21:00 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, rust-for-linux, linux-kernel, llvm On Fri, Sep 13, 2024 at 5:32 PM Gary Guo <gary@garyguo.net> wrote: > -bindgen_c_flags_final = $(bindgen_c_flags_lto) -D__BINDGEN__ > +# `-fno-builtin` is passed to avoid bindgen from using clang builtin prototypes > +# for functions like `memcpy` -- if this flag is not passed, bindgen-generated > +# prototypes use `c_ulong` or `c_uint` depending on architecture instead of > +# generating `usize`. > +bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__ I wonder how reliable this behavior is. Maybe bindgen could do a better job controlling this, is there an open issue? - Trevor ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins 2024-09-29 21:00 ` Trevor Gross @ 2024-10-05 22:10 ` Gary Guo 0 siblings, 0 replies; 20+ messages in thread From: Gary Guo @ 2024-10-05 22:10 UTC (permalink / raw) To: Trevor Gross Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, rust-for-linux, linux-kernel, llvm On Sun, 29 Sep 2024 17:00:29 -0400 Trevor Gross <tmgross@umich.edu> wrote: > On Fri, Sep 13, 2024 at 5:32 PM Gary Guo <gary@garyguo.net> wrote: > > > -bindgen_c_flags_final = $(bindgen_c_flags_lto) -D__BINDGEN__ > > +# `-fno-builtin` is passed to avoid bindgen from using clang builtin prototypes > > +# for functions like `memcpy` -- if this flag is not passed, bindgen-generated > > +# prototypes use `c_ulong` or `c_uint` depending on architecture instead of > > +# generating `usize`. > > +bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__ > > I wonder how reliable this behavior is. Maybe bindgen could do a > better job controlling this, is there an open issue? > > - Trevor I didn't search for bindgen issues when made this change, but apparently this is indeed the suggested approach in https://github.com/rust-lang/rust-bindgen/issues/1770 Best, Gary ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo 2024-09-13 21:29 ` [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins Gary Guo @ 2024-09-13 21:29 ` Gary Guo 2024-09-23 9:20 ` Alice Ryhl 2024-09-29 21:02 ` Trevor Gross 2024-09-13 21:29 ` [PATCH 3/5] rust: use custom FFI integer types Gary Guo ` (4 subsequent siblings) 6 siblings, 2 replies; 20+ messages in thread From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Finn Behrens, Martin Rodriguez Reboredo Cc: rust-for-linux, linux-kernel Currently bindgen has special logic to recognise `size_t` and `ssize_t` and map them to Rust `usize` and `isize`. Similarly, `ptrdiff_t` is mapped to `isize`. However this falls short for `__kernel_size_t`, `__kernel_ssize_t` and `__kernel_ptrdiff_t`. To ensure that they are mapped to usize/isize rather than 32/64 integers depending on platform, blocklist them in bindgen parameters and manually provide their definition. Signed-off-by: Gary Guo <gary@garyguo.net> --- rust/bindgen_parameters | 5 +++++ rust/bindings/lib.rs | 5 +++++ rust/uapi/lib.rs | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters index a721d466bee4b..756307dd59350 100644 --- a/rust/bindgen_parameters +++ b/rust/bindgen_parameters @@ -1,5 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 +# We want to map these types to isize/usize manually, instead of +# define them as int/long depending on platform bitwidth. +--blocklist-type __kernel_s?size_t +--blocklist-type __kernel_ptrdiff_t + --opaque-type xregs_state --opaque-type desc_struct --opaque-type arch_lbr_state diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs index 93a1a3fc97bc9..9999c90e8db14 100644 --- a/rust/bindings/lib.rs +++ b/rust/bindings/lib.rs @@ -26,6 +26,11 @@ #[allow(dead_code)] mod bindings_raw { + // Manual definition for blocklisted types. + type __kernel_size_t = usize; + type __kernel_ssize_t = isize; + type __kernel_ptrdiff_t = isize; + // Use glob import here to expose all helpers. // Symbols defined within the module will take precedence to the glob import. pub use super::bindings_helper::*; diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs index 80a00260e3e7a..0eca836c0dbbb 100644 --- a/rust/uapi/lib.rs +++ b/rust/uapi/lib.rs @@ -24,4 +24,9 @@ unsafe_op_in_unsafe_fn )] +// Manual definition of blocklisted types. +type __kernel_size_t = usize; +type __kernel_ssize_t = isize; +type __kernel_ptrdiff_t = isize; + include!(concat!(env!("OBJTREE"), "/rust/uapi/uapi_generated.rs")); -- 2.44.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize 2024-09-13 21:29 ` [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize Gary Guo @ 2024-09-23 9:20 ` Alice Ryhl 2024-09-29 21:02 ` Trevor Gross 1 sibling, 0 replies; 20+ messages in thread From: Alice Ryhl @ 2024-09-23 9:20 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Trevor Gross, Finn Behrens, Martin Rodriguez Reboredo, rust-for-linux, linux-kernel On Fri, Sep 13, 2024 at 11:32 PM Gary Guo <gary@garyguo.net> wrote: > > Currently bindgen has special logic to recognise `size_t` and `ssize_t` > and map them to Rust `usize` and `isize`. Similarly, `ptrdiff_t` is > mapped to `isize`. > > However this falls short for `__kernel_size_t`, `__kernel_ssize_t` and > `__kernel_ptrdiff_t`. To ensure that they are mapped to usize/isize > rather than 32/64 integers depending on platform, blocklist them in > bindgen parameters and manually provide their definition. > > Signed-off-by: Gary Guo <gary@garyguo.net> Reviewed-by: Alice Ryhl <aliceryhl@google.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize 2024-09-13 21:29 ` [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize Gary Guo 2024-09-23 9:20 ` Alice Ryhl @ 2024-09-29 21:02 ` Trevor Gross 1 sibling, 0 replies; 20+ messages in thread From: Trevor Gross @ 2024-09-29 21:02 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Finn Behrens, Martin Rodriguez Reboredo, rust-for-linux, linux-kernel On Fri, Sep 13, 2024 at 5:32 PM Gary Guo <gary@garyguo.net> wrote: > > Currently bindgen has special logic to recognise `size_t` and `ssize_t` > and map them to Rust `usize` and `isize`. Similarly, `ptrdiff_t` is > mapped to `isize`. > > However this falls short for `__kernel_size_t`, `__kernel_ssize_t` and > `__kernel_ptrdiff_t`. To ensure that they are mapped to usize/isize > rather than 32/64 integers depending on platform, blocklist them in > bindgen parameters and manually provide their definition. > > Signed-off-by: Gary Guo <gary@garyguo.net> Reviewed-by: Trevor Gross <tmgross@umich.edu> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/5] rust: use custom FFI integer types 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo 2024-09-13 21:29 ` [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins Gary Guo 2024-09-13 21:29 ` [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize Gary Guo @ 2024-09-13 21:29 ` Gary Guo 2024-09-13 21:29 ` [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` Gary Guo ` (3 subsequent siblings) 6 siblings, 0 replies; 20+ messages in thread From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, FUJITA Tomonori, Martin Rodriguez Reboredo, Vlastimil Babka, Valentin Obst, Jens Axboe, Alex Mantel, Danilo Krummrich, Yutaro Ohno, Tiago Lam, Charalampos Mitrodimas, Thomas Gleixner, Kartik Prajapati, Obei Sideg, Thomas Bertschinger Cc: Wedson Almeida Filho, Finn Behrens, Manmohan Shukla, Asahi Lina, linux-kernel, rust-for-linux, linux-block, netdev Currently FFI integer types are defined in libcore. This commit creates the `ffi` crate and asks bindgen to use that crate for FFI integer types instead of `core::ffi`. This commit is preparatory and no type changes are made in this commit yet. Signed-off-by: Gary Guo <gary@garyguo.net> --- rust/Makefile | 18 ++++++++++++------ rust/ffi.rs | 13 +++++++++++++ rust/kernel/alloc/allocator.rs | 4 ++-- rust/kernel/block/mq/operations.rs | 18 +++++++++--------- rust/kernel/block/mq/raw_writer.rs | 2 +- rust/kernel/block/mq/tag_set.rs | 2 +- rust/kernel/error.rs | 20 ++++++++++---------- rust/kernel/init.rs | 2 +- rust/kernel/lib.rs | 2 ++ rust/kernel/net/phy.rs | 14 +++++++------- rust/kernel/str.rs | 4 ++-- rust/kernel/sync/arc.rs | 6 +++--- rust/kernel/sync/condvar.rs | 2 +- rust/kernel/sync/lock.rs | 2 +- rust/kernel/sync/lock/mutex.rs | 2 +- rust/kernel/sync/lock/spinlock.rs | 2 +- rust/kernel/task.rs | 8 ++------ rust/kernel/time.rs | 4 ++-- rust/kernel/types.rs | 26 +++++++++++++------------- rust/kernel/uaccess.rs | 6 +++--- rust/macros/module.rs | 8 ++++---- 21 files changed, 91 insertions(+), 74 deletions(-) create mode 100644 rust/ffi.rs diff --git a/rust/Makefile b/rust/Makefile index 863d87ad0ce68..041ebc5db50b9 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -3,7 +3,7 @@ # Where to place rustdoc generated documentation rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc -obj-$(CONFIG_RUST) += core.o compiler_builtins.o +obj-$(CONFIG_RUST) += core.o compiler_builtins.o ffi.o always-$(CONFIG_RUST) += exports_core_generated.h # Missing prototypes are expected in the helpers since these are exported @@ -114,7 +114,7 @@ rustdoc-alloc: private rustc_target_flags = $(alloc-cfgs) \ rustdoc-alloc: $(RUST_LIB_SRC)/alloc/src/lib.rs rustdoc-core rustdoc-compiler_builtins FORCE +$(call if_changed,rustdoc) -rustdoc-kernel: private rustc_target_flags = --extern alloc \ +rustdoc-kernel: private rustc_target_flags = --extern alloc --extern ffi \ --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \ --extern bindings --extern uapi rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \ @@ -274,7 +274,7 @@ bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__ quiet_cmd_bindgen = BINDGEN $@ cmd_bindgen = \ $(BINDGEN) $< $(bindgen_target_flags) \ - --use-core --with-derive-default --ctypes-prefix core::ffi --no-layout-tests \ + --use-core --with-derive-default --ctypes-prefix ffi --no-layout-tests \ --no-debug '.*' --enable-function-attribute-detection \ -o $@ -- $(bindgen_c_flags_final) -DMODULE \ $(bindgen_target_cflags) $(bindgen_target_extra) @@ -411,19 +411,25 @@ $(obj)/alloc.o: $(RUST_LIB_SRC)/alloc/src/lib.rs $(obj)/compiler_builtins.o FORC $(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE +$(call if_changed_rule,rustc_library) +$(obj)/ffi.o: $(src)/ffi.rs $(obj)/compiler_builtins.o FORCE + +$(call if_changed_rule,rustc_library) + +$(obj)/bindings.o: private rustc_target_flags = --extern ffi $(obj)/bindings.o: $(src)/bindings/lib.rs \ - $(obj)/compiler_builtins.o \ + $(obj)/ffi.o \ $(obj)/bindings/bindings_generated.rs \ $(obj)/bindings/bindings_helpers_generated.rs FORCE +$(call if_changed_rule,rustc_library) +$(obj)/uapi.o: private rustc_target_flags = --extern ffi $(obj)/uapi.o: $(src)/uapi/lib.rs \ - $(obj)/compiler_builtins.o \ + $(obj)/ffi.o \ $(obj)/uapi/uapi_generated.rs FORCE +$(call if_changed_rule,rustc_library) $(obj)/kernel.o: private rustc_target_flags = --extern alloc \ - --extern build_error --extern macros --extern bindings --extern uapi + --extern build_error --extern macros --extern ffi \ + --extern bindings --extern uapi $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \ $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE +$(call if_changed_rule,rustc_library) diff --git a/rust/ffi.rs b/rust/ffi.rs new file mode 100644 index 0000000000000..1894a511ee171 --- /dev/null +++ b/rust/ffi.rs @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Foreign function interface (FFI) types. +//! +//! This crate provides mapping from C primitive types to Rust ones. +//! +//! Rust core crate provides [`core::ffi`], which maps integer types to platform default C ABI. +//! Kernel does not use `core::ffi` so it can customise the mapping that deviates from platform +//! default. + +#![no_std] + +pub use core::ffi::*; diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs index e6ea601f38c6d..e7a5937a98b09 100644 --- a/rust/kernel/alloc/allocator.rs +++ b/rust/kernel/alloc/allocator.rs @@ -28,7 +28,7 @@ pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F // function safety requirement. // - `size` is greater than 0 since it's from `layout.size()` (which cannot be zero according // to the function safety requirement) - unsafe { bindings::krealloc(ptr as *const core::ffi::c_void, size, flags.0) as *mut u8 } + unsafe { bindings::krealloc(ptr as *const crate::ffi::c_void, size, flags.0) as *mut u8 } } unsafe impl GlobalAlloc for KernelAllocator { @@ -40,7 +40,7 @@ unsafe fn alloc(&self, layout: Layout) -> *mut u8 { unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) { unsafe { - bindings::kfree(ptr as *const core::ffi::c_void); + bindings::kfree(ptr as *const crate::ffi::c_void); } } diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs index 9ba7fdfeb4b22..c8646d0d98669 100644 --- a/rust/kernel/block/mq/operations.rs +++ b/rust/kernel/block/mq/operations.rs @@ -131,7 +131,7 @@ impl<T: Operations> OperationsVTable<T> { unsafe extern "C" fn poll_callback( _hctx: *mut bindings::blk_mq_hw_ctx, _iob: *mut bindings::io_comp_batch, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { T::poll().into() } @@ -145,9 +145,9 @@ impl<T: Operations> OperationsVTable<T> { /// for the same context. unsafe extern "C" fn init_hctx_callback( _hctx: *mut bindings::blk_mq_hw_ctx, - _tagset_data: *mut core::ffi::c_void, - _hctx_idx: core::ffi::c_uint, - ) -> core::ffi::c_int { + _tagset_data: *mut crate::ffi::c_void, + _hctx_idx: crate::ffi::c_uint, + ) -> crate::ffi::c_int { from_result(|| Ok(0)) } @@ -159,7 +159,7 @@ impl<T: Operations> OperationsVTable<T> { /// This function may only be called by blk-mq C infrastructure. unsafe extern "C" fn exit_hctx_callback( _hctx: *mut bindings::blk_mq_hw_ctx, - _hctx_idx: core::ffi::c_uint, + _hctx_idx: crate::ffi::c_uint, ) { } @@ -176,9 +176,9 @@ impl<T: Operations> OperationsVTable<T> { unsafe extern "C" fn init_request_callback( _set: *mut bindings::blk_mq_tag_set, rq: *mut bindings::request, - _hctx_idx: core::ffi::c_uint, - _numa_node: core::ffi::c_uint, - ) -> core::ffi::c_int { + _hctx_idx: crate::ffi::c_uint, + _numa_node: crate::ffi::c_uint, + ) -> crate::ffi::c_int { from_result(|| { // SAFETY: By the safety requirements of this function, `rq` points // to a valid allocation. @@ -203,7 +203,7 @@ impl<T: Operations> OperationsVTable<T> { unsafe extern "C" fn exit_request_callback( _set: *mut bindings::blk_mq_tag_set, rq: *mut bindings::request, - _hctx_idx: core::ffi::c_uint, + _hctx_idx: crate::ffi::c_uint, ) { // SAFETY: The tagset invariants guarantee that all requests are allocated with extra memory // for the request data. diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs index 9222465d670bf..7e2159e4f6a6f 100644 --- a/rust/kernel/block/mq/raw_writer.rs +++ b/rust/kernel/block/mq/raw_writer.rs @@ -25,7 +25,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> { } pub(crate) fn from_array<const N: usize>( - a: &'a mut [core::ffi::c_char; N], + a: &'a mut [crate::ffi::c_char; N], ) -> Result<RawWriter<'a>> { Self::new( // SAFETY: the buffer of `a` is valid for read and write as `u8` for diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs index f9a1ca655a35b..d7f175a05d992 100644 --- a/rust/kernel/block/mq/tag_set.rs +++ b/rust/kernel/block/mq/tag_set.rs @@ -53,7 +53,7 @@ pub fn new( queue_depth: num_tags, cmd_size, flags: bindings::BLK_MQ_F_SHOULD_MERGE, - driver_data: core::ptr::null_mut::<core::ffi::c_void>(), + driver_data: core::ptr::null_mut::<crate::ffi::c_void>(), nr_maps: num_maps, ..tag_set } diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 6f1587a2524e8..1090a13c2bd17 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -88,14 +88,14 @@ macro_rules! declare_err { /// /// The value is a valid `errno` (i.e. `>= -MAX_ERRNO && < 0`). #[derive(Clone, Copy, PartialEq, Eq)] -pub struct Error(core::ffi::c_int); +pub struct Error(crate::ffi::c_int); impl Error { /// Creates an [`Error`] from a kernel error code. /// /// It is a bug to pass an out-of-range `errno`. `EINVAL` would /// be returned in such a case. - pub(crate) fn from_errno(errno: core::ffi::c_int) -> Error { + pub(crate) fn from_errno(errno: crate::ffi::c_int) -> Error { if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 { // TODO: Make it a `WARN_ONCE` once available. crate::pr_warn!( @@ -115,14 +115,14 @@ pub(crate) fn from_errno(errno: core::ffi::c_int) -> Error { /// # Safety /// /// `errno` must be within error code range (i.e. `>= -MAX_ERRNO && < 0`). - unsafe fn from_errno_unchecked(errno: core::ffi::c_int) -> Error { + unsafe fn from_errno_unchecked(errno: crate::ffi::c_int) -> Error { // INVARIANT: The contract ensures the type invariant // will hold. Error(errno) } /// Returns the kernel error code. - pub fn to_errno(self) -> core::ffi::c_int { + pub fn to_errno(self) -> crate::ffi::c_int { self.0 } @@ -239,7 +239,7 @@ fn from(e: core::convert::Infallible) -> 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 { +pub fn to_result(err: crate::ffi::c_int) -> Result { if err < 0 { Err(Error::from_errno(err)) } else { @@ -262,7 +262,7 @@ pub fn to_result(err: core::ffi::c_int) -> Result { /// fn devm_platform_ioremap_resource( /// pdev: &mut PlatformDevice, /// index: u32, -/// ) -> Result<*mut core::ffi::c_void> { +/// ) -> Result<*mut kernel::ffi::c_void> { /// // SAFETY: `pdev` points to a valid platform device. There are no safety requirements /// // on `index`. /// from_err_ptr(unsafe { bindings::devm_platform_ioremap_resource(pdev.to_ptr(), index) }) @@ -271,8 +271,8 @@ pub fn to_result(err: core::ffi::c_int) -> Result { // 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(); + // CAST: Casting a pointer to `*const crate::ffi::c_void` is always valid. + let const_ptr: *const crate::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. @@ -287,7 +287,7 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> { // 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) }); + return Err(unsafe { Error::from_errno_unchecked(err as crate::ffi::c_int) }); } Ok(ptr) } @@ -307,7 +307,7 @@ pub(crate) fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> { /// # use kernel::bindings; /// unsafe extern "C" fn probe_callback( /// pdev: *mut bindings::platform_device, -/// ) -> core::ffi::c_int { +/// ) -> kernel::ffi::c_int { /// from_result(|| { /// let ptr = devm_alloc(pdev)?; /// bindings::platform_set_drvdata(pdev, ptr); diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index a17ac8762d8f9..e0a757bd42f0c 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -133,7 +133,7 @@ //! # } //! # // `Error::from_errno` is `pub(crate)` in the `kernel` crate, thus provide a workaround. //! # trait FromErrno { -//! # fn from_errno(errno: core::ffi::c_int) -> Error { +//! # fn from_errno(errno: kernel::ffi::c_int) -> Error { //! # // Dummy error that can be constructed outside the `kernel` crate. //! # Error::from(core::fmt::Error) //! # } diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index f10b06a78b9d5..fbce57c09fe0c 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -26,6 +26,8 @@ // Allow proc-macros to refer to `::kernel` inside the `kernel` crate (this crate). extern crate self as kernel; +pub use ffi; + pub mod alloc; #[cfg(CONFIG_BLOCK)] pub mod block; diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index fd40b703d2244..d1a44ab2a408d 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -327,7 +327,7 @@ impl<T: Driver> Adapter<T> { /// `phydev` must be passed by the corresponding callback in `phy_driver`. unsafe extern "C" fn soft_reset_callback( phydev: *mut bindings::phy_device, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -343,7 +343,7 @@ impl<T: Driver> Adapter<T> { /// `phydev` must be passed by the corresponding callback in `phy_driver`. unsafe extern "C" fn get_features_callback( phydev: *mut bindings::phy_device, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -357,7 +357,7 @@ impl<T: Driver> Adapter<T> { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int { + unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int { from_result(|| { // SAFETY: The C core code ensures that the accessors on // `Device` are okay to call even though `phy_device->lock` @@ -371,7 +371,7 @@ impl<T: Driver> Adapter<T> { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> core::ffi::c_int { + unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int { from_result(|| { // SAFETY: The C core code ensures that the accessors on // `Device` are okay to call even though `phy_device->lock` @@ -387,7 +387,7 @@ impl<T: Driver> Adapter<T> { /// `phydev` must be passed by the corresponding callback in `phy_driver`. unsafe extern "C" fn config_aneg_callback( phydev: *mut bindings::phy_device, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -403,7 +403,7 @@ impl<T: Driver> Adapter<T> { /// `phydev` must be passed by the corresponding callback in `phy_driver`. unsafe extern "C" fn read_status_callback( phydev: *mut bindings::phy_device, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -419,7 +419,7 @@ impl<T: Driver> Adapter<T> { /// `phydev` must be passed by the corresponding callback in `phy_driver`. unsafe extern "C" fn match_phy_device_callback( phydev: *mut bindings::phy_device, - ) -> core::ffi::c_int { + ) -> crate::ffi::c_int { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on // `Device` are okay to call. diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index bb8d4f41475b5..3980d37bd4b29 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -185,7 +185,7 @@ pub const fn is_empty(&self) -> bool { /// last at least `'a`. When `CStr` is alive, the memory pointed by `ptr` /// must not be mutated. #[inline] - pub unsafe fn from_char_ptr<'a>(ptr: *const core::ffi::c_char) -> &'a Self { + pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self { // SAFETY: The safety precondition guarantees `ptr` is a valid pointer // to a `NUL`-terminated C string. let len = unsafe { bindings::strlen(ptr) } + 1; @@ -248,7 +248,7 @@ pub unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut CStr { /// Returns a C pointer to the string. #[inline] - pub const fn as_char_ptr(&self) -> *const core::ffi::c_char { + pub const fn as_char_ptr(&self) -> *const crate::ffi::c_char { self.0.as_ptr() as _ } diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 3021f30fd822f..ec95bf6157561 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -336,11 +336,11 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> { impl<T: 'static> ForeignOwnable for Arc<T> { type Borrowed<'a> = ArcBorrow<'a, T>; - fn into_foreign(self) -> *const core::ffi::c_void { + fn into_foreign(self) -> *const crate::ffi::c_void { ManuallyDrop::new(self).ptr.as_ptr() as _ } - unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow<'a>(ptr: *const crate::ffi::c_void) -> ArcBorrow<'a, T> { // SAFETY: By the safety requirement of this function, we know that `ptr` came from // a previous call to `Arc::into_foreign`. let inner = NonNull::new(ptr as *mut ArcInner<T>).unwrap(); @@ -350,7 +350,7 @@ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> ArcBorrow<'a, T> { unsafe { ArcBorrow::new(inner) } } - unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *const crate::ffi::c_void) -> Self { // SAFETY: By the safety requirement of this function, we know that `ptr` came from // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and // holds a reference count increment that is transferrable to us. diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index 2b306afbe56d9..fd9a67f5a2efd 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -7,6 +7,7 @@ use super::{lock::Backend, lock::Guard, LockClassKey}; use crate::{ + ffi::{c_int, c_long}, init::PinInit, pin_init, str::CStr, @@ -14,7 +15,6 @@ time::Jiffies, types::Opaque, }; -use core::ffi::{c_int, c_long}; use core::marker::PhantomPinned; use core::ptr; use macros::pin_data; diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index f6c34ca4d819f..9f6a5d2373bbc 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -46,7 +46,7 @@ pub unsafe trait Backend { /// remain valid for read indefinitely. unsafe fn init( ptr: *mut Self::State, - name: *const core::ffi::c_char, + name: *const crate::ffi::c_char, key: *mut bindings::lock_class_key, ); diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs index 30632070ee670..abd7701578255 100644 --- a/rust/kernel/sync/lock/mutex.rs +++ b/rust/kernel/sync/lock/mutex.rs @@ -96,7 +96,7 @@ unsafe impl super::Backend for MutexBackend { unsafe fn init( ptr: *mut Self::State, - name: *const core::ffi::c_char, + name: *const crate::ffi::c_char, key: *mut bindings::lock_class_key, ) { // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs index ea5c5bc1ce12e..e4891670e9f3f 100644 --- a/rust/kernel/sync/lock/spinlock.rs +++ b/rust/kernel/sync/lock/spinlock.rs @@ -95,7 +95,7 @@ unsafe impl super::Backend for SpinLockBackend { unsafe fn init( ptr: *mut Self::State, - name: *const core::ffi::c_char, + name: *const crate::ffi::c_char, key: *mut bindings::lock_class_key, ) { // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 55dff7e088bf5..5bce090a38697 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -4,13 +4,9 @@ //! //! C header: [`include/linux/sched.h`](srctree/include/linux/sched.h). +use crate::ffi::{c_int, c_long, c_uint}; use crate::types::Opaque; -use core::{ - ffi::{c_int, c_long, c_uint}, - marker::PhantomData, - ops::Deref, - ptr, -}; +use core::{marker::PhantomData, ops::Deref, ptr}; /// A sentinel value used for infinite timeouts. pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX; diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index e3bb5e89f88da..379c0f5772e57 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -12,10 +12,10 @@ pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64; /// The time unit of Linux kernel. One jiffy equals (1/HZ) second. -pub type Jiffies = core::ffi::c_ulong; +pub type Jiffies = crate::ffi::c_ulong; /// The millisecond time unit. -pub type Msecs = core::ffi::c_uint; +pub type Msecs = crate::ffi::c_uint; /// Converts milliseconds to jiffies. #[inline] diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 9e7ca066355cd..452328c675b7d 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -31,7 +31,7 @@ pub trait ForeignOwnable: Sized { /// For example, it might be invalid, dangling or pointing to uninitialized memory. Using it in /// any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`], /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior. - fn into_foreign(self) -> *const core::ffi::c_void; + fn into_foreign(self) -> *const crate::ffi::c_void; /// Borrows a foreign-owned object. /// @@ -39,7 +39,7 @@ pub trait ForeignOwnable: Sized { /// /// `ptr` must have been returned by a previous call to [`ForeignOwnable::into_foreign`] for /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. - unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Self::Borrowed<'a>; + unsafe fn borrow<'a>(ptr: *const crate::ffi::c_void) -> Self::Borrowed<'a>; /// Converts a foreign-owned object back to a Rust-owned one. /// @@ -49,7 +49,7 @@ pub trait ForeignOwnable: Sized { /// which a previous matching [`ForeignOwnable::from_foreign`] hasn't been called yet. /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] for /// this object must have been dropped. - unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self; + unsafe fn from_foreign(ptr: *const crate::ffi::c_void) -> Self; /// Tries to convert a foreign-owned object back to a Rust-owned one. /// @@ -60,7 +60,7 @@ pub trait ForeignOwnable: Sized { /// /// `ptr` must either be null or satisfy the safety requirements for /// [`ForeignOwnable::from_foreign`]. - unsafe fn try_from_foreign(ptr: *const core::ffi::c_void) -> Option<Self> { + unsafe fn try_from_foreign(ptr: *const crate::ffi::c_void) -> Option<Self> { if ptr.is_null() { None } else { @@ -74,11 +74,11 @@ unsafe fn try_from_foreign(ptr: *const core::ffi::c_void) -> Option<Self> { impl<T: 'static> ForeignOwnable for Box<T> { type Borrowed<'a> = &'a T; - fn into_foreign(self) -> *const core::ffi::c_void { + fn into_foreign(self) -> *const crate::ffi::c_void { Box::into_raw(self) as _ } - unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T { + unsafe fn borrow<'a>(ptr: *const crate::ffi::c_void) -> &'a T { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for @@ -86,7 +86,7 @@ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> &'a T { unsafe { &*ptr.cast() } } - unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *const crate::ffi::c_void) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. unsafe { Box::from_raw(ptr as _) } @@ -96,12 +96,12 @@ unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { impl<T: 'static> ForeignOwnable for Pin<Box<T>> { type Borrowed<'a> = Pin<&'a T>; - fn into_foreign(self) -> *const core::ffi::c_void { + fn into_foreign(self) -> *const crate::ffi::c_void { // SAFETY: We are still treating the box as pinned. Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }) as _ } - unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Pin<&'a T> { + unsafe fn borrow<'a>(ptr: *const crate::ffi::c_void) -> Pin<&'a T> { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for @@ -112,7 +112,7 @@ unsafe fn borrow<'a>(ptr: *const core::ffi::c_void) -> Pin<&'a T> { unsafe { Pin::new_unchecked(r) } } - unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *const crate::ffi::c_void) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. unsafe { Pin::new_unchecked(Box::from_raw(ptr as _)) } @@ -122,13 +122,13 @@ unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self { impl ForeignOwnable for () { type Borrowed<'a> = (); - fn into_foreign(self) -> *const core::ffi::c_void { + fn into_foreign(self) -> *const crate::ffi::c_void { core::ptr::NonNull::dangling().as_ptr() } - unsafe fn borrow<'a>(_: *const core::ffi::c_void) -> Self::Borrowed<'a> {} + unsafe fn borrow<'a>(_: *const crate::ffi::c_void) -> Self::Borrowed<'a> {} - unsafe fn from_foreign(_: *const core::ffi::c_void) -> Self {} + unsafe fn from_foreign(_: *const crate::ffi::c_void) -> Self {} } /// Runs a cleanup function/closure when dropped. diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index e9347cff99ab2..c746a1f1bb5ad 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -8,11 +8,11 @@ alloc::Flags, bindings, error::Result, + ffi::{c_ulong, c_void}, prelude::*, types::{AsBytes, FromBytes}, }; use alloc::vec::Vec; -use core::ffi::{c_ulong, c_void}; use core::mem::{size_of, MaybeUninit}; /// The type used for userspace addresses. @@ -47,7 +47,7 @@ /// /// ```no_run /// use alloc::vec::Vec; -/// use core::ffi::c_void; +/// use kernel::ffi::c_void; /// use kernel::error::Result; /// use kernel::uaccess::{UserPtr, UserSlice}; /// @@ -70,7 +70,7 @@ /// /// ```no_run /// use alloc::vec::Vec; -/// use core::ffi::c_void; +/// use kernel::ffi::c_void; /// use kernel::error::{code::EINVAL, Result}; /// use kernel::uaccess::{UserPtr, UserSlice}; /// diff --git a/rust/macros/module.rs b/rust/macros/module.rs index 571ffa2e189ca..6f6c8d18ca2d1 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -249,7 +249,7 @@ mod __module_init {{ #[doc(hidden)] #[no_mangle] #[link_section = \".init.text\"] - pub unsafe extern \"C\" fn init_module() -> core::ffi::c_int {{ + pub unsafe extern \"C\" fn init_module() -> kernel::ffi::c_int {{ // SAFETY: This function is inaccessible to the outside due to the double // module wrapping it. It is called exactly once by the C side via its // unique name. @@ -288,7 +288,7 @@ mod __module_init {{ #[doc(hidden)] #[link_section = \"{initcall_section}\"] #[used] - pub static __{name}_initcall: extern \"C\" fn() -> core::ffi::c_int = __{name}_init; + pub static __{name}_initcall: extern \"C\" fn() -> kernel::ffi::c_int = __{name}_init; #[cfg(not(MODULE))] #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)] @@ -303,7 +303,7 @@ mod __module_init {{ #[cfg(not(MODULE))] #[doc(hidden)] #[no_mangle] - pub extern \"C\" fn __{name}_init() -> core::ffi::c_int {{ + pub extern \"C\" fn __{name}_init() -> kernel::ffi::c_int {{ // SAFETY: This function is inaccessible to the outside due to the double // module wrapping it. It is called exactly once by the C side via its // placement above in the initcall section. @@ -326,7 +326,7 @@ mod __module_init {{ /// # Safety /// /// This function must only be called once. - unsafe fn __init() -> core::ffi::c_int {{ + unsafe fn __init() -> kernel::ffi::c_int {{ match <{type_} as kernel::Module>::init(&super::super::THIS_MODULE) {{ Ok(m) => {{ // SAFETY: No data race, since `__MOD` can only be accessed by this -- 2.44.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo ` (2 preceding siblings ...) 2024-09-13 21:29 ` [PATCH 3/5] rust: use custom FFI integer types Gary Guo @ 2024-09-13 21:29 ` Gary Guo 2024-09-23 9:20 ` Alice Ryhl 2024-09-13 21:29 ` [PATCH 5/5] rust: cleanup unnecessary casts Gary Guo ` (2 subsequent siblings) 6 siblings, 1 reply; 20+ messages in thread From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Luis Chamberlain, Russ Weight, Danilo Krummrich, Martin Rodriguez Reboredo, Finn Behrens, Valentin Obst Cc: Wedson Almeida Filho, rust-for-linux, linux-kernel The following FFI types are replaced compared to `core::ffi`: 1. `char` type is now always mapped to `u8`, since kernel uses `-funsigned-char` on the C code. `core::ffi` maps it to platform default ABI, which can be either signed or unsigned. 2. `long` is now always mapped to `isize`. It's very common in the kernel to use `long` to represent a pointer-sized integer, and in fact `intptr_t` is a typedef of `long` in the kernel. Enforce this mapping rather than mapping to `i32/i64` depending on platform can save us a lot of unnecessary casts. Signed-off-by: Gary Guo <gary@garyguo.net> --- rust/ffi.rs | 37 ++++++++++++++++++++++++++++++++++++- rust/kernel/error.rs | 5 +---- rust/kernel/firmware.rs | 2 +- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/rust/ffi.rs b/rust/ffi.rs index 1894a511ee171..814d10106b5a8 100644 --- a/rust/ffi.rs +++ b/rust/ffi.rs @@ -10,4 +10,39 @@ #![no_std] -pub use core::ffi::*; +macro_rules! alias { + ($($name:ident = $ty:ty;)*) => {$( + #[allow(non_camel_case_types, missing_docs)] + pub type $name = $ty; + + // Check size compatibility with libcore. + const _: () = assert!( + core::mem::size_of::<$name>() == core::mem::size_of::<core::ffi::$name>() + ); + )*} +} + +alias! { + // `core::ffi::c_char` is either i8/u8 depending on architecture. In kernel, we use + // `-funsigned-char` so it's always mapped to u8. + c_char = u8; + + c_schar = i8; + c_uchar = u8; + + c_short = i16; + c_ushort = u16; + + c_int = i32; + c_uint = u32; + + // In kernel, `intptr_t` is defined to be `long` in all platforms, so we can map the type to + // `isize`. + c_long = isize; + c_ulong = usize; + + c_longlong = i64; + c_ulonglong = u64; +} + +pub use core::ffi::c_void; diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 1090a13c2bd17..b9ad9ff25cddb 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -135,11 +135,8 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t { /// Returns the error encoded as a pointer. #[allow(dead_code)] pub(crate) fn to_ptr<T>(self) -> *mut T { - #[cfg_attr(target_pointer_width = "32", allow(clippy::useless_conversion))] // SAFETY: `self.0` is a valid error due to its invariant. - unsafe { - bindings::ERR_PTR(self.0.into()) as *mut _ - } + unsafe { bindings::ERR_PTR(self.0 as _) as *mut _ } } /// Returns a string representing the error, if one exists. diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index dee5b4b18aec4..6f77945016579 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -12,7 +12,7 @@ /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`, /// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`. struct FwFunc( - unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32, + unsafe extern "C" fn(*mut *const bindings::firmware, *const u8, *mut bindings::device) -> i32, ); impl FwFunc { -- 2.44.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` 2024-09-13 21:29 ` [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` Gary Guo @ 2024-09-23 9:20 ` Alice Ryhl 0 siblings, 0 replies; 20+ messages in thread From: Alice Ryhl @ 2024-09-23 9:20 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Trevor Gross, Luis Chamberlain, Russ Weight, Danilo Krummrich, Martin Rodriguez Reboredo, Finn Behrens, Valentin Obst, Wedson Almeida Filho, rust-for-linux, linux-kernel On Fri, Sep 13, 2024 at 11:33 PM Gary Guo <gary@garyguo.net> wrote: > > The following FFI types are replaced compared to `core::ffi`: > > 1. `char` type is now always mapped to `u8`, since kernel uses > `-funsigned-char` on the C code. `core::ffi` maps it to platform > default ABI, which can be either signed or unsigned. > > 2. `long` is now always mapped to `isize`. It's very common in the > kernel to use `long` to represent a pointer-sized integer, and in > fact `intptr_t` is a typedef of `long` in the kernel. Enforce this > mapping rather than mapping to `i32/i64` depending on platform can > save us a lot of unnecessary casts. > > Signed-off-by: Gary Guo <gary@garyguo.net> Reviewed-by: Alice Ryhl <aliceryhl@google.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/5] rust: cleanup unnecessary casts 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo ` (3 preceding siblings ...) 2024-09-13 21:29 ` [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` Gary Guo @ 2024-09-13 21:29 ` Gary Guo 2024-09-23 9:19 ` Alice Ryhl 2024-09-13 21:42 ` [PATCH 0/5] use custom FFI integer types Benno Lossin 2024-11-11 0:14 ` Miguel Ojeda 6 siblings, 1 reply; 20+ messages in thread From: Gary Guo @ 2024-09-13 21:29 UTC (permalink / raw) To: Brendan Higgins, David Gow, Rae Moar, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Martin Rodriguez Reboredo, Michael Vetter, Finn Behrens, Valentin Obst, Danilo Krummrich, Yutaro Ohno Cc: Wedson Almeida Filho, Asahi Lina, linux-kselftest, kunit-dev, rust-for-linux, linux-kernel With `long` mapped to `isize`, `size_t`/`__kernel_size_t` mapped to usize and `char` mapped to `u8`, many of the existing casts are no longer necessary. Signed-off-by: Gary Guo <gary@garyguo.net> --- rust/kernel/kunit.rs | 10 ++-------- rust/kernel/print.rs | 4 ++-- rust/kernel/str.rs | 6 +++--- rust/kernel/uaccess.rs | 27 +++++++-------------------- 4 files changed, 14 insertions(+), 33 deletions(-) diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 0ba77276ae7ef..766aeb1c6aea8 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -17,10 +17,7 @@ pub fn err(args: fmt::Arguments<'_>) { // are passing. #[cfg(CONFIG_PRINTK)] unsafe { - bindings::_printk( - b"\x013%pA\0".as_ptr() as _, - &args as *const _ as *const c_void, - ); + bindings::_printk(b"\x013%pA\0".as_ptr(), &args as *const _ as *const c_void); } } @@ -33,10 +30,7 @@ pub fn info(args: fmt::Arguments<'_>) { // are passing. #[cfg(CONFIG_PRINTK)] unsafe { - bindings::_printk( - b"\x016%pA\0".as_ptr() as _, - &args as *const _ as *const c_void, - ); + bindings::_printk(b"\x016%pA\0".as_ptr(), &args as *const _ as *const c_void); } } diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs index 508b0221256c9..90ae4f2568910 100644 --- a/rust/kernel/print.rs +++ b/rust/kernel/print.rs @@ -104,7 +104,7 @@ pub unsafe fn call_printk( #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - format_string.as_ptr() as _, + format_string.as_ptr(), module_name.as_ptr(), &args as *const _ as *const c_void, ); @@ -125,7 +125,7 @@ pub fn call_printk_cont(args: fmt::Arguments<'_>) { #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - format_strings::CONT.as_ptr() as _, + format_strings::CONT.as_ptr(), &args as *const _ as *const c_void, ); } diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 3980d37bd4b29..2d30bca079e37 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -190,7 +190,7 @@ pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self { // to a `NUL`-terminated C string. let len = unsafe { bindings::strlen(ptr) } + 1; // SAFETY: Lifetime guaranteed by the safety precondition. - let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len as _) }; + let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len) }; // SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`. // As we have added 1 to `len`, the last byte is known to be `NUL`. unsafe { Self::from_bytes_with_nul_unchecked(bytes) } @@ -249,7 +249,7 @@ pub unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut CStr { /// Returns a C pointer to the string. #[inline] pub const fn as_char_ptr(&self) -> *const crate::ffi::c_char { - self.0.as_ptr() as _ + self.0.as_ptr() } /// Convert the string to a byte slice without the trailing `NUL` byte. @@ -817,7 +817,7 @@ pub fn try_from_fmt(args: fmt::Arguments<'_>) -> Result<Self, Error> { // SAFETY: The buffer is valid for read because `f.bytes_written()` is bounded by `size` // (which the minimum buffer size) and is non-zero (we wrote at least the `NUL` terminator) // so `f.bytes_written() - 1` doesn't underflow. - let ptr = unsafe { bindings::memchr(buf.as_ptr().cast(), 0, (f.bytes_written() - 1) as _) }; + let ptr = unsafe { bindings::memchr(buf.as_ptr().cast(), 0, f.bytes_written() - 1) }; if !ptr.is_null() { return Err(EINVAL); } diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index c746a1f1bb5ad..eb72fbcf152a1 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -8,7 +8,7 @@ alloc::Flags, bindings, error::Result, - ffi::{c_ulong, c_void}, + ffi::c_void, prelude::*, types::{AsBytes, FromBytes}, }; @@ -227,13 +227,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result { if len > self.length { return Err(EFAULT); } - let Ok(len_ulong) = c_ulong::try_from(len) else { - return Err(EFAULT); - }; - // SAFETY: `out_ptr` points into a mutable slice of length `len_ulong`, so we may write + // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write // that many bytes to it. - let res = - unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len_ulong) }; + let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) }; if res != 0 { return Err(EFAULT); } @@ -262,9 +258,6 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> { if len > self.length { return Err(EFAULT); } - let Ok(len_ulong) = c_ulong::try_from(len) else { - return Err(EFAULT); - }; let mut out: MaybeUninit<T> = MaybeUninit::uninit(); // SAFETY: The local variable `out` is valid for writing `size_of::<T>()` bytes. // @@ -275,7 +268,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> { bindings::_copy_from_user( out.as_mut_ptr().cast::<c_void>(), self.ptr as *const c_void, - len_ulong, + len, ) }; if res != 0 { @@ -338,12 +331,9 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result { if len > self.length { return Err(EFAULT); } - let Ok(len_ulong) = c_ulong::try_from(len) else { - return Err(EFAULT); - }; - // SAFETY: `data_ptr` points into an immutable slice of length `len_ulong`, so we may read + // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read // that many bytes from it. - let res = unsafe { bindings::copy_to_user(self.ptr as *mut c_void, data_ptr, len_ulong) }; + let res = unsafe { bindings::copy_to_user(self.ptr as *mut c_void, data_ptr, len) }; if res != 0 { return Err(EFAULT); } @@ -362,9 +352,6 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result { if len > self.length { return Err(EFAULT); } - let Ok(len_ulong) = c_ulong::try_from(len) else { - return Err(EFAULT); - }; // SAFETY: The reference points to a value of type `T`, so it is valid for reading // `size_of::<T>()` bytes. // @@ -375,7 +362,7 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result { bindings::_copy_to_user( self.ptr as *mut c_void, (value as *const T).cast::<c_void>(), - len_ulong, + len, ) }; if res != 0 { -- 2.44.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 5/5] rust: cleanup unnecessary casts 2024-09-13 21:29 ` [PATCH 5/5] rust: cleanup unnecessary casts Gary Guo @ 2024-09-23 9:19 ` Alice Ryhl 0 siblings, 0 replies; 20+ messages in thread From: Alice Ryhl @ 2024-09-23 9:19 UTC (permalink / raw) To: Gary Guo Cc: Brendan Higgins, David Gow, Rae Moar, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Trevor Gross, Martin Rodriguez Reboredo, Michael Vetter, Finn Behrens, Valentin Obst, Danilo Krummrich, Yutaro Ohno, Wedson Almeida Filho, Asahi Lina, linux-kselftest, kunit-dev, rust-for-linux, linux-kernel On Fri, Sep 13, 2024 at 11:33 PM Gary Guo <gary@garyguo.net> wrote: > > With `long` mapped to `isize`, `size_t`/`__kernel_size_t` mapped to > usize and `char` mapped to `u8`, many of the existing casts are no > longer necessary. > > Signed-off-by: Gary Guo <gary@garyguo.net> This is great! Reviewed-by: Alice Ryhl <aliceryhl@google.com> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo ` (4 preceding siblings ...) 2024-09-13 21:29 ` [PATCH 5/5] rust: cleanup unnecessary casts Gary Guo @ 2024-09-13 21:42 ` Benno Lossin [not found] ` <CAOcBZOS6BAJ1FTFkB3x6jdag_hL7zrLbFy7TxkvZWKxRZ_+ggA@mail.gmail.com> 2024-11-11 0:14 ` Miguel Ojeda 6 siblings, 1 reply; 20+ messages in thread From: Benno Lossin @ 2024-09-13 21:42 UTC (permalink / raw) To: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross Cc: rust-for-linux On 13.09.24 23:29, Gary Guo wrote: > This patch series aim to reduce the unnecessary type casts needed in > Rust code doing FFI calls. > > With this series, we can ensure: > > * `size_t` is mapped to usize. Currently this is mostly true except for > C builtin functions, which gets translated by bindgen to c_ulong/c_uint. > > * `__kernel_size_t` is mapped to usize. Currently this is mapped to > c_ulong/c_uint. > > * `unsigned long` is mapped to usize. Currently this is mapped by Rust > libcore to either u32 or u64. > > * `char` is mapped to `u8`. Currently this is mapped by Rust libcore to > either i8 or u8. Great work! > After this series, FFI code needs to use `kernel::ffi` types instead of > `core::ffi`. I think we should have a lint that checks for this. Can probably be a checkpatch.pl check and a good-first-issue. --- Cheers, Benno > Gary Guo (5): > rust: fix size_t in bindgen prototypes of C builtins > rust: map `__kernel_size_t` and friends also to usize/isize > rust: use custom FFI integer types > rust: map `long` to `isize` and `char` to `u8` > rust: cleanup unnecessary casts > > rust/Makefile | 24 ++++++++++----- > rust/bindgen_parameters | 5 ++++ > rust/bindings/lib.rs | 5 ++++ > rust/ffi.rs | 48 ++++++++++++++++++++++++++++++ > rust/kernel/alloc/allocator.rs | 4 +-- > rust/kernel/block/mq/operations.rs | 18 +++++------ > rust/kernel/block/mq/raw_writer.rs | 2 +- > rust/kernel/block/mq/tag_set.rs | 2 +- > rust/kernel/error.rs | 25 +++++++--------- > rust/kernel/firmware.rs | 2 +- > rust/kernel/init.rs | 2 +- > rust/kernel/kunit.rs | 10 ++----- > rust/kernel/lib.rs | 2 ++ > rust/kernel/net/phy.rs | 14 ++++----- > rust/kernel/print.rs | 4 +-- > rust/kernel/str.rs | 10 +++---- > rust/kernel/sync/arc.rs | 6 ++-- > rust/kernel/sync/condvar.rs | 2 +- > rust/kernel/sync/lock.rs | 2 +- > rust/kernel/sync/lock/mutex.rs | 2 +- > rust/kernel/sync/lock/spinlock.rs | 2 +- > rust/kernel/task.rs | 8 ++--- > rust/kernel/time.rs | 4 +-- > rust/kernel/types.rs | 26 ++++++++-------- > rust/kernel/uaccess.rs | 31 ++++++------------- > rust/macros/module.rs | 8 ++--- > rust/uapi/lib.rs | 5 ++++ > 27 files changed, 161 insertions(+), 112 deletions(-) > create mode 100644 rust/ffi.rs > > > base-commit: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2 > -- > 2.44.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <CAOcBZOS6BAJ1FTFkB3x6jdag_hL7zrLbFy7TxkvZWKxRZ_+ggA@mail.gmail.com>]
* Re: [PATCH 0/5] use custom FFI integer types [not found] ` <CAOcBZOS6BAJ1FTFkB3x6jdag_hL7zrLbFy7TxkvZWKxRZ_+ggA@mail.gmail.com> @ 2024-09-14 2:51 ` Ramon de C Valle 2024-09-14 8:52 ` Alice Ryhl 2024-09-14 23:52 ` Gary Guo 0 siblings, 2 replies; 20+ messages in thread From: Ramon de C Valle @ 2024-09-14 2:51 UTC (permalink / raw) To: Benno Lossin Cc: Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux Since we're doing this, may I suggest that we use https://crates.io/crates/cfi-types by reexporting them as kernel::ffi instead? By using those, we would also avoid the granularity loss of normalizing integers and using the integer normalization option wouldn't be necessary. I could accept any contributions/kernel-specific changes that would make it easier for the kernel to use them. On Fri, Sep 13, 2024 at 7:48 PM Ramon de C Valle <rcvalle@google.com> wrote: > > Since we're doing this, may I suggest that we use https://crates.io/crates/cfi-types by reexporting them as kernel::ffi instead? By using those, we would also avoid the granularity loss of normalizing integers and using the integer normalization option wouldn't be necessary. I could accept any contributions/kernel-specific changes that would make it easier for the kernel to use them. > > On Fri, Sep 13, 2024 at 2:43 PM Benno Lossin <benno.lossin@proton.me> wrote: >> >> On 13.09.24 23:29, Gary Guo wrote: >> > This patch series aim to reduce the unnecessary type casts needed in >> > Rust code doing FFI calls. >> > >> > With this series, we can ensure: >> > >> > * `size_t` is mapped to usize. Currently this is mostly true except for >> > C builtin functions, which gets translated by bindgen to c_ulong/c_uint. >> > >> > * `__kernel_size_t` is mapped to usize. Currently this is mapped to >> > c_ulong/c_uint. >> > >> > * `unsigned long` is mapped to usize. Currently this is mapped by Rust >> > libcore to either u32 or u64. >> > >> > * `char` is mapped to `u8`. Currently this is mapped by Rust libcore to >> > either i8 or u8. >> >> Great work! >> >> > After this series, FFI code needs to use `kernel::ffi` types instead of >> > `core::ffi`. >> >> I think we should have a lint that checks for this. Can probably be a >> checkpatch.pl check and a good-first-issue. >> >> --- >> Cheers, >> Benno >> >> > Gary Guo (5): >> > rust: fix size_t in bindgen prototypes of C builtins >> > rust: map `__kernel_size_t` and friends also to usize/isize >> > rust: use custom FFI integer types >> > rust: map `long` to `isize` and `char` to `u8` >> > rust: cleanup unnecessary casts >> > >> > rust/Makefile | 24 ++++++++++----- >> > rust/bindgen_parameters | 5 ++++ >> > rust/bindings/lib.rs | 5 ++++ >> > rust/ffi.rs | 48 ++++++++++++++++++++++++++++++ >> > rust/kernel/alloc/allocator.rs | 4 +-- >> > rust/kernel/block/mq/operations.rs | 18 +++++------ >> > rust/kernel/block/mq/raw_writer.rs | 2 +- >> > rust/kernel/block/mq/tag_set.rs | 2 +- >> > rust/kernel/error.rs | 25 +++++++--------- >> > rust/kernel/firmware.rs | 2 +- >> > rust/kernel/init.rs | 2 +- >> > rust/kernel/kunit.rs | 10 ++----- >> > rust/kernel/lib.rs | 2 ++ >> > rust/kernel/net/phy.rs | 14 ++++----- >> > rust/kernel/print.rs | 4 +-- >> > rust/kernel/str.rs | 10 +++---- >> > rust/kernel/sync/arc.rs | 6 ++-- >> > rust/kernel/sync/condvar.rs | 2 +- >> > rust/kernel/sync/lock.rs | 2 +- >> > rust/kernel/sync/lock/mutex.rs | 2 +- >> > rust/kernel/sync/lock/spinlock.rs | 2 +- >> > rust/kernel/task.rs | 8 ++--- >> > rust/kernel/time.rs | 4 +-- >> > rust/kernel/types.rs | 26 ++++++++-------- >> > rust/kernel/uaccess.rs | 31 ++++++------------- >> > rust/macros/module.rs | 8 ++--- >> > rust/uapi/lib.rs | 5 ++++ >> > 27 files changed, 161 insertions(+), 112 deletions(-) >> > create mode 100644 rust/ffi.rs >> > >> > >> > base-commit: 93dc3be19450447a3a7090bd1dfb9f3daac3e8d2 >> > -- >> > 2.44.1 >> > >> >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-14 2:51 ` Ramon de C Valle @ 2024-09-14 8:52 ` Alice Ryhl 2024-09-14 23:52 ` Gary Guo 1 sibling, 0 replies; 20+ messages in thread From: Alice Ryhl @ 2024-09-14 8:52 UTC (permalink / raw) To: Ramon de C Valle Cc: Benno Lossin, Gary Guo, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Trevor Gross, rust-for-linux On Sat, Sep 14, 2024 at 4:51 AM Ramon de C Valle <rcvalle@google.com> wrote: > > Since we're doing this, may I suggest that we use > https://crates.io/crates/cfi-types by reexporting them as kernel::ffi > instead? By using those, we would also avoid the granularity loss of > normalizing integers and using the integer normalization option > wouldn't be necessary. I could accept any > contributions/kernel-specific changes that would make it easier for > the kernel to use them. I've been wondering about this for some time now. Is there any way we could improve the language to allow #[cfi_encoding] annotations on type aliases? It would go a long way to make this kind of thing more convenient to use. Alice ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-14 2:51 ` Ramon de C Valle 2024-09-14 8:52 ` Alice Ryhl @ 2024-09-14 23:52 ` Gary Guo 2024-09-16 17:17 ` Ramon de C Valle 1 sibling, 1 reply; 20+ messages in thread From: Gary Guo @ 2024-09-14 23:52 UTC (permalink / raw) To: Ramon de C Valle Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux On Fri, 13 Sep 2024 19:51:01 -0700 Ramon de C Valle <rcvalle@google.com> wrote: > Since we're doing this, may I suggest that we use > https://crates.io/crates/cfi-types by reexporting them as kernel::ffi > instead? By using those, we would also avoid the granularity loss of > normalizing integers and using the integer normalization option > wouldn't be necessary. I could accept any > contributions/kernel-specific changes that would make it easier for > the kernel to use them. I don't think integer normalization is a big issue. Sami said the granularity loss only results in ~1% reduction of unique type hashes in https://lore.kernel.org/all/CABCJKuc8ue1y7WBPo3YRRoDeGUFpRon4at=Wa1rQjrXzOGpt9w@mail.gmail.com/. As Alice said, if the CFI annotation works with type aliases then we should switch to use them. Without it though, I think it's quite inconvenient if we have to use new types for all FFI integer primitives and add a ton of conversions. I'd prefer we still use type aliases. Best, Gary ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-14 23:52 ` Gary Guo @ 2024-09-16 17:17 ` Ramon de C Valle 2024-09-16 17:26 ` Miguel Ojeda 0 siblings, 1 reply; 20+ messages in thread From: Ramon de C Valle @ 2024-09-16 17:17 UTC (permalink / raw) To: Gary Guo Cc: Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux Makes sense. Unfortunately, the cfi_encoding attribute doesn't work with aliases because when types are encoded all type aliases are already resolved to their respective aliased types--maybe if in the future aliases have their own ty::Ty representation it'll be possible (e.g., as in https://github.com/rust-lang/compiler-team/issues/504), and the encoding could even be deduced from the aliased type. On Sat, Sep 14, 2024 at 4:52 PM Gary Guo <gary@garyguo.net> wrote: > > On Fri, 13 Sep 2024 19:51:01 -0700 > Ramon de C Valle <rcvalle@google.com> wrote: > > > Since we're doing this, may I suggest that we use > > https://crates.io/crates/cfi-types by reexporting them as kernel::ffi > > instead? By using those, we would also avoid the granularity loss of > > normalizing integers and using the integer normalization option > > wouldn't be necessary. I could accept any > > contributions/kernel-specific changes that would make it easier for > > the kernel to use them. > > I don't think integer normalization is a big issue. Sami said the > granularity loss only results in ~1% reduction of unique type hashes in > https://lore.kernel.org/all/CABCJKuc8ue1y7WBPo3YRRoDeGUFpRon4at=Wa1rQjrXzOGpt9w@mail.gmail.com/. > > As Alice said, if the CFI annotation works with type aliases then we > should switch to use them. Without it though, I think it's quite > inconvenient if we have to use new types for all FFI integer > primitives and add a ton of conversions. I'd prefer we still use type > aliases. > > Best, > Gary ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-16 17:17 ` Ramon de C Valle @ 2024-09-16 17:26 ` Miguel Ojeda 0 siblings, 0 replies; 20+ messages in thread From: Miguel Ojeda @ 2024-09-16 17:26 UTC (permalink / raw) To: Ramon de C Valle Cc: Gary Guo, Benno Lossin, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux On Mon, Sep 16, 2024 at 7:17 PM Ramon de C Valle <rcvalle@google.com> wrote: > > Makes sense. Unfortunately, the cfi_encoding attribute doesn't work > with aliases because when types are encoded all type aliases are > already resolved to their respective aliased types--maybe if in the > future aliases have their own ty::Ty representation it'll be possible > (e.g., as in https://github.com/rust-lang/compiler-team/issues/504), > and the encoding could even be deduced from the aliased type. Yeah, it would be best to do something in the compiler for this, rather than force the source code to change. Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo ` (5 preceding siblings ...) 2024-09-13 21:42 ` [PATCH 0/5] use custom FFI integer types Benno Lossin @ 2024-11-11 0:14 ` Miguel Ojeda 2024-12-15 22:25 ` Miguel Ojeda 6 siblings, 1 reply; 20+ messages in thread From: Miguel Ojeda @ 2024-11-11 0:14 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux On Fri, Sep 13, 2024 at 11:32 PM Gary Guo <gary@garyguo.net> wrote: > > This patch series aim to reduce the unnecessary type casts needed in > Rust code doing FFI calls. > > With this series, we can ensure: > > * `size_t` is mapped to usize. Currently this is mostly true except for > C builtin functions, which gets translated by bindgen to c_ulong/c_uint. > > * `__kernel_size_t` is mapped to usize. Currently this is mapped to > c_ulong/c_uint. > > * `unsigned long` is mapped to usize. Currently this is mapped by Rust > libcore to either u32 or u64. > > * `char` is mapped to `u8`. Currently this is mapped by Rust libcore to > either i8 or u8. > > After this series, FFI code needs to use `kernel::ffi` types instead of > `core::ffi`. Applied to `rust-next` -- thanks everyone! Gary: please double-check if you have time! [ Trevor asked: "I wonder how reliable this behavior is. Maybe bindgen could do a better job controlling this, is there an open issue?". Gary replied: ..."apparently this is indeed the suggested approach in https://github.com/rust-lang/rust-bindgen/issues/1770". - Miguel ] [ Formatted comment. - Miguel ] [ Formatted comment. - Miguel ] [ Added `rustdoc`, `rusttest` and KUnit tests support. Rebased on top of `rust-next` (e.g. migrated more `core::ffi` cases). Reworded crate docs slightly and formatted. - Miguel ] [ Moved `uaccess` changes from the next commit, since they were irrefutable patterns that Rust >= 1.82.0 warns about. Reworded slightly and reformatted a few documentation comments. Rebased on top of `rust-next`. - Miguel ] [ Moved `uaccess` changes to the previous commit, since they were irrefutable patterns that Rust >= 1.82.0 warns about. Removed a couple casts that now use `c""` literals. Rebased on top of `rust-next`. - Miguel ] Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] use custom FFI integer types 2024-11-11 0:14 ` Miguel Ojeda @ 2024-12-15 22:25 ` Miguel Ojeda 0 siblings, 0 replies; 20+ messages in thread From: Miguel Ojeda @ 2024-12-15 22:25 UTC (permalink / raw) To: Gary Guo Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux On Mon, Nov 11, 2024 at 1:14 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Applied to `rust-next` -- thanks everyone! Applied to `rust-next` (the rest) -- thanks everyone! To clarify: last cycle we ended up applying only the first part, i.e. the addition of the `ffi` crate and the move of all (now, most) of the `core::ffi` uses, without the actual remapping. This time around I am applying the rest (with an initial commit to migrate the few remaining/new `core::ffi` uses like Gary's third patch here). Gary: please double-check if you have time! Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-12-15 22:25 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-13 21:29 [PATCH 0/5] use custom FFI integer types Gary Guo
2024-09-13 21:29 ` [PATCH 1/5] rust: fix size_t in bindgen prototypes of C builtins Gary Guo
2024-09-29 21:00 ` Trevor Gross
2024-10-05 22:10 ` Gary Guo
2024-09-13 21:29 ` [PATCH 2/5] rust: map `__kernel_size_t` and friends also to usize/isize Gary Guo
2024-09-23 9:20 ` Alice Ryhl
2024-09-29 21:02 ` Trevor Gross
2024-09-13 21:29 ` [PATCH 3/5] rust: use custom FFI integer types Gary Guo
2024-09-13 21:29 ` [PATCH 4/5] rust: map `long` to `isize` and `char` to `u8` Gary Guo
2024-09-23 9:20 ` Alice Ryhl
2024-09-13 21:29 ` [PATCH 5/5] rust: cleanup unnecessary casts Gary Guo
2024-09-23 9:19 ` Alice Ryhl
2024-09-13 21:42 ` [PATCH 0/5] use custom FFI integer types Benno Lossin
[not found] ` <CAOcBZOS6BAJ1FTFkB3x6jdag_hL7zrLbFy7TxkvZWKxRZ_+ggA@mail.gmail.com>
2024-09-14 2:51 ` Ramon de C Valle
2024-09-14 8:52 ` Alice Ryhl
2024-09-14 23:52 ` Gary Guo
2024-09-16 17:17 ` Ramon de C Valle
2024-09-16 17:26 ` Miguel Ojeda
2024-11-11 0:14 ` Miguel Ojeda
2024-12-15 22:25 ` Miguel Ojeda
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).