[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZyEgbhWdFa66p-G3@Boquns-Mac-mini.local>
Date: Tue, 29 Oct 2024 10:50:38 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Peter Zijlstra <peterz@...radead.org>,
Josh Poimboeuf <jpoimboe@...nel.org>, Jason Baron <jbaron@...mai.com>,
Ard Biesheuvel <ardb@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>,
linux-trace-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
linux-arch@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>,
Sean Christopherson <seanjc@...gle.com>, Uros Bizjak <ubizjak@...il.com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>,
Mark Rutland <mark.rutland@....com>, Ryan Roberts <ryan.roberts@....com>,
Fuad Tabba <tabba@...gle.com>, linux-arm-kernel@...ts.infradead.org,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Anup Patel <apatel@...tanamicro.com>,
Andrew Jones <ajones@...tanamicro.com>,
Alexandre Ghiti <alexghiti@...osinc.com>,
Conor Dooley <conor.dooley@...rochip.com>,
Samuel Holland <samuel.holland@...ive.com>,
linux-riscv@...ts.infradead.org, Huacai Chen <chenhuacai@...nel.org>,
WANG Xuerui <kernel@...0n.name>, Bibo Mao <maobibo@...ngson.cn>,
Tiezhu Yang <yangtiezhu@...ngson.cn>,
Andrew Morton <akpm@...ux-foundation.org>,
Tianrui Zhao <zhaotianrui@...ngson.cn>, loongarch@...ts.linux.dev
Subject: Re: [PATCH v11 5/5] rust: add arch_static_branch
On Tue, Oct 15, 2024 at 01:14:59PM +0000, Alice Ryhl wrote:
> To allow the Rust implementation of static_key_false to use runtime code
> patching instead of the generic implementation, pull in the relevant
> inline assembly from the jump_label.h header by running the C
> preprocessor on a .rs.S file. Build rules are added for .rs.S files.
>
> Since the relevant inline asm has been adjusted to export the inline asm
> via the ARCH_STATIC_BRANCH_ASM macro in a consistent way, the Rust side
> does not need architecture specific code to pull in the asm.
>
> It is not possible to use the existing C implementation of
> arch_static_branch via a Rust helper because it passes the argument
> `key` to inline assembly as an 'i' parameter. Any attempt to add a C
> helper for this function will fail to compile because the value of `key`
> must be known at compile-time.
>
> Suggested-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Co-developed-by: Miguel Ojeda <ojeda@...nel.org>
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
> rust/Makefile | 6 +++++
> rust/kernel/.gitignore | 3 +++
> rust/kernel/arch_static_branch_asm.rs.S | 7 +++++
> rust/kernel/jump_label.rs | 46 ++++++++++++++++++++++++++++++++-
> rust/kernel/lib.rs | 35 +++++++++++++++++++++++++
> scripts/Makefile.build | 9 ++++++-
> 6 files changed, 104 insertions(+), 2 deletions(-)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index b5e0a73b78f3..c532f48b79de 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -36,6 +36,8 @@ always-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.c
> obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
> obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
>
> +always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/arch_static_branch_asm.rs
> +
> # Avoids running `$(RUSTC)` for the sysroot when it may not be available.
> ifdef CONFIG_RUST
>
> @@ -424,4 +426,8 @@ $(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)
>
> +ifneq ($(CONFIG_JUMP_LABEL),)
> +$(obj)/kernel.o: $(obj)/kernel/arch_static_branch_asm.rs
> +endif
> +
> endif # CONFIG_RUST
> diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
> new file mode 100644
> index 000000000000..d082731007c6
> --- /dev/null
> +++ b/rust/kernel/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +/arch_static_branch_asm.rs
> diff --git a/rust/kernel/arch_static_branch_asm.rs.S b/rust/kernel/arch_static_branch_asm.rs.S
> new file mode 100644
> index 000000000000..2afb638708db
> --- /dev/null
> +++ b/rust/kernel/arch_static_branch_asm.rs.S
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <linux/jump_label.h>
> +
> +// Cut here.
> +
> +::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}", "{l_yes}"))
> diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
> index 4b7655b2a022..2f2df03a3275 100644
> --- a/rust/kernel/jump_label.rs
> +++ b/rust/kernel/jump_label.rs
> @@ -24,7 +24,51 @@ macro_rules! static_branch_unlikely {
> let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
> let _key: *const $crate::bindings::static_key = _key.cast();
>
> - $crate::bindings::static_key_count(_key.cast_mut()) > 0
> + #[cfg(not(CONFIG_JUMP_LABEL))]
> + {
> + $crate::bindings::static_key_count(_key) > 0
> + }
> +
> + #[cfg(CONFIG_JUMP_LABEL)]
> + $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
> }};
> }
> pub use static_branch_unlikely;
> +
> +/// Assert that the assembly block evaluates to a string literal.
> +#[cfg(CONFIG_JUMP_LABEL)]
> +const _: &str = include!(concat!(
> + env!("OBJTREE"),
> + "/rust/kernel/arch_static_branch_asm.rs"
> +));
> +
> +#[macro_export]
> +#[doc(hidden)]
> +#[cfg(CONFIG_JUMP_LABEL)]
> +macro_rules! arch_static_branch {
> + ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> + $crate::asm!(
> + include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
> + l_yes = label {
> + break 'my_label true;
> + },
> + symb = sym $key,
> + off = const ::core::mem::offset_of!($keytyp, $field),
> + branch = const $crate::jump_label::bool_to_int($branch),
> + );
> +
> + break 'my_label false;
> + }};
> +}
> +
> +#[cfg(CONFIG_JUMP_LABEL)]
> +pub use arch_static_branch;
> +
> +/// A helper used by inline assembly to pass a boolean to as a `const` parameter.
> +///
> +/// Using this function instead of a cast lets you assert that the input is a boolean, and not some
> +/// other type that can also be cast to an integer.
> +#[doc(hidden)]
> +pub const fn bool_to_int(b: bool) -> i32 {
> + b as i32
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 55f81f49024e..c0ae9ddd9468 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -148,3 +148,38 @@ macro_rules! container_of {
> ptr.sub(offset) as *const $type
> }}
> }
> +
> +/// Helper for `.rs.S` files.
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! concat_literals {
> + ($( $asm:literal )* ) => {
> + ::core::concat!($($asm),*)
> + };
> +}
> +
> +/// Wrapper around `asm!` configured for use in the kernel.
> +///
> +/// Uses a semicolon to avoid parsing ambiguities, even though this does not match native `asm!`
> +/// syntax.
> +// For x86, `asm!` uses intel syntax by default, but we want to use at&t syntax in the kernel.
> +#[cfg(target_arch = "x86_64")]
This should be:
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
for future proofing and um (usermode linux) support.
> +#[macro_export]
> +macro_rules! asm {
> + ($($asm:expr),* ; $($rest:tt)*) => {
> + ::core::arch::asm!( $($asm)*, options(att_syntax), $($rest)* )
> + };
> +}
> +
> +/// Wrapper around `asm!` configured for use in the kernel.
> +///
> +/// Uses a semicolon to avoid parsing ambiguities, even though this does not match native `asm!`
> +/// syntax.
> +// For non-x86 arches we just pass through to `asm!`.
> +#[cfg(not(target_arch = "x86_64"))]
Ditto.
With these fixed, feel free to add:
Reviewed-by: Boqun Feng <boqun.feng@...il.com>
Regards,
Boqun
> +#[macro_export]
> +macro_rules! asm {
> + ($($asm:expr),* ; $($rest:tt)*) => {
> + ::core::arch::asm!( $($asm)*, $($rest)* )
> + };
> +}
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 8f423a1faf50..03ee558fcd4d 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -248,12 +248,13 @@ $(obj)/%.lst: $(obj)/%.c FORCE
> # Compile Rust sources (.rs)
> # ---------------------------------------------------------------------------
>
> -rust_allowed_features := new_uninit
> +rust_allowed_features := asm_const,asm_goto,new_uninit
>
> # `--out-dir` is required to avoid temporaries being created by `rustc` in the
> # current working directory, which may be not accessible in the out-of-tree
> # modules case.
> rust_common_cmd = \
> + OBJTREE=$(abspath $(objtree)) \
> RUST_MODFILE=$(modfile) $(RUSTC_OR_CLIPPY) $(rust_flags) \
> -Zallow-features=$(rust_allowed_features) \
> -Zcrate-attr=no_std \
> @@ -303,6 +304,12 @@ quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> $(obj)/%.ll: $(obj)/%.rs FORCE
> +$(call if_changed_dep,rustc_ll_rs)
>
> +quiet_cmd_rustc_rs_rs_S = RSCPP $(quiet_modtag) $@
> + cmd_rustc_rs_rs_S = $(CPP) $(c_flags) -xc -C -P $< | sed '1,/^\/\/ Cut here.$$/d' >$@
> +
> +$(obj)/%.rs: $(obj)/%.rs.S FORCE
> + +$(call if_changed_dep,rustc_rs_rs_S)
> +
> # Compile assembler sources (.S)
> # ---------------------------------------------------------------------------
>
>
> --
> 2.47.0.rc1.288.g06298d1525-goog
>
Powered by blists - more mailing lists