linux-kernel - Re: [PATCH v4 1/2] rust: add static_key

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH5fLghYodekhH-1A0BWZVwgbqkWbP3WP70-us2FtHqvOqD_Hw@mail.gmail.com>
Date: Wed, 31 Jul 2024 23:34:13 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 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>, 
	Boqun Feng <boqun.feng@...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@...sung.com>, 
	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>, "Peter Zijlstra (Intel)" <peterz@...radaed.org>, 
	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 v4 1/2] rust: add static_key_false

On Wed, Jul 31, 2024 at 7:05 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Fri, Jun 28, 2024 at 01:23:31PM +0000, Alice Ryhl wrote:
>
> >  rust/kernel/arch/arm64/jump_label.rs     | 34 ++++++++++++++++++++++++++++
> >  rust/kernel/arch/loongarch/jump_label.rs | 35 +++++++++++++++++++++++++++++
> >  rust/kernel/arch/mod.rs                  | 24 ++++++++++++++++++++
> >  rust/kernel/arch/riscv/jump_label.rs     | 38 ++++++++++++++++++++++++++++++++
> >  rust/kernel/arch/x86/jump_label.rs       | 35 +++++++++++++++++++++++++++++
> >  rust/kernel/lib.rs                       |  2 ++
> >  rust/kernel/static_key.rs                | 32 +++++++++++++++++++++++++++
> >  scripts/Makefile.build                   |  2 +-
> >  8 files changed, 201 insertions(+), 1 deletion(-)
>
> So I really find the amount of duplicated asm offensive. Is is far too
> easy for any of this to get out of sync.
>
> > diff --git a/rust/kernel/arch/x86/jump_label.rs b/rust/kernel/arch/x86/jump_label.rs
> > new file mode 100644
> > index 000000000000..383bed273c50
> > --- /dev/null
> > +++ b/rust/kernel/arch/x86/jump_label.rs
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2024 Google LLC.
> > +
> > +//! X86 Rust implementation of jump_label.h
> > +
> > +/// x86 implementation of arch_static_branch
> > +#[macro_export]
> > +#[cfg(target_arch = "x86_64")]
> > +macro_rules! arch_static_branch {
> > +    ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> > +        core::arch::asm!(
> > +            r#"
> > +            1: .byte 0x0f,0x1f,0x44,0x00,0x00
> > +
> > +            .pushsection __jump_table,  "aw"
> > +            .balign 8
> > +            .long 1b - .
> > +            .long {0} - .
> > +            .quad {1} + {2} + {3} - .
> > +            .popsection
> > +            "#,
> > +            label {
> > +                break 'my_label true;
> > +            },
> > +            sym $key,
> > +            const ::core::mem::offset_of!($keytyp, $field),
> > +            const $crate::arch::bool_to_int($branch),
> > +        );
> > +
> > +        break 'my_label false;
> > +    }};
> > +}
>
> Note that this uses the forced 5 byte version, and not the dynamic sized
> one. On top of that it hard-codes the nop5 string :/
>
> Please work harder to not have to duplicate stuff like this.

I really didn't want to duplicate it, but it's very hard to find a
performant alternative. Is there any way we could accept duplication
only in the cases where an 'i' parameter is used? I don't have the
choice of using a Rust helper for 'i' parameters.

Perhaps one option could be to put the Rust code inside jump_label.h
and have the header file evaluate to either C or Rust depending on the
value of some #ifdefs?

#ifndef RUST_ASM
/* existing C code goes here */
#endif
#ifdef RUST_ASM
// rust code goes here
#endif

That way the duplication is all in a single file. It would also avoid
the need for duplicating the nop5 string, as the Rust case is still
going through the C preprocessor and can use the existing #define.

I'm also open to other alternatives. But I don't have infinite
resources to drive major language changes.

Alice