diff mbox series

[1/3] rust: add static_call support

Message ID 20240606-tracepoint-v1-1-6551627bf51b@google.com (mailing list archive)
State Superseded
Headers show
Series Tracepoints and static branch/call in Rust | expand

Commit Message

Alice Ryhl June 6, 2024, 3:05 p.m. UTC
Add static_call support by mirroring how C does. When the platform does
not support static calls (right now, that means that it is not x86),
then the function pointer is loaded from a global and called. Otherwise,
we generate a call to a trampoline function, and objtool is used to make
these calls patchable at runtime.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/lib.rs         |  1 +
 rust/kernel/static_call.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

Comments

Peter Zijlstra June 6, 2024, 5:18 p.m. UTC | #1
On Thu, Jun 06, 2024 at 03:05:24PM +0000, Alice Ryhl wrote:
> Add static_call support by mirroring how C does. When the platform does
> not support static calls (right now, that means that it is not x86),
> then the function pointer is loaded from a global and called. Otherwise,
> we generate a call to a trampoline function, and objtool is used to make
> these calls patchable at runtime.

This is absolutely unreadable gibberish -- how am I supposed to keep
this in sync with the rest of the static_call infrastructure?

> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/lib.rs         |  1 +
>  rust/kernel/static_call.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index fbd91a48ff8b..d534b1178955 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -38,6 +38,7 @@
>  pub mod prelude;
>  pub mod print;
>  mod static_assert;
> +pub mod static_call;
>  #[doc(hidden)]
>  pub mod std_vendor;
>  pub mod str;
> diff --git a/rust/kernel/static_call.rs b/rust/kernel/static_call.rs
> new file mode 100644
> index 000000000000..f7b8ba7bf1fb
> --- /dev/null
> +++ b/rust/kernel/static_call.rs
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Logic for static calls.
> +
> +#[macro_export]
> +#[doc(hidden)]
> +macro_rules! ty_underscore_for {
> +    ($arg:expr) => {
> +        _
> +    };
> +}
> +
> +#[doc(hidden)]
> +#[repr(transparent)]
> +pub struct AddressableStaticCallKey {
> +    _ptr: *const bindings::static_call_key,
> +}
> +unsafe impl Sync for AddressableStaticCallKey {}
> +impl AddressableStaticCallKey {
> +    pub const fn new(ptr: *const bindings::static_call_key) -> Self {
> +        Self { _ptr: ptr }
> +    }
> +}
> +
> +#[cfg(CONFIG_HAVE_STATIC_CALL)]
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! _static_call {
> +    ($name:ident($($args:expr),* $(,)?)) => {{
> +        // Symbol mangling will give this symbol a unique name.
> +        #[cfg(CONFIG_HAVE_STATIC_CALL_INLINE)]
> +        #[link_section = ".discard.addressable"]
> +        #[used]
> +        static __ADDRESSABLE: $crate::static_call::AddressableStaticCallKey = unsafe {
> +            $crate::static_call::AddressableStaticCallKey::new(::core::ptr::addr_of!(
> +                $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
> +            ))
> +        };
> +
> +        let fn_ptr: unsafe extern "C" fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
> +            $crate::macros::paste! { $crate::bindings:: [<__SCT__ $name >]; };
> +        (fn_ptr)($($args),*)
> +    }};
> +}
> +
> +#[cfg(not(CONFIG_HAVE_STATIC_CALL))]
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! _static_call {
> +    ($name:ident($($args:expr),* $(,)?)) => {{
> +        let void_ptr_fn: *mut ::core::ffi::c_void =
> +            $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }.func;
> +
> +        let fn_ptr: unsafe extern "C" fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
> +            if true {
> +                ::core::mem::transmute(void_ptr_fn)
> +            } else {
> +                // This is dead code, but it influences type inference on `fn_ptr` so that we
> +                // transmute the function pointer to the right type.
> +                $crate::macros::paste! { $crate::bindings:: [<__SCT__ $name >]; }
> +            };
> +
> +        (fn_ptr)($($args),*)
> +    }};
> +}
> +
> +/// Statically call a global function.
> +///
> +/// # Safety
> +///
> +/// This macro will call the provided function. It is up to the caller to uphold the safety
> +/// guarantees of the function.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// fn call_static() {
> +///     unsafe {
> +///         static_call! { your_static_call() };
> +///     }
> +/// }
> +/// ```
> +#[macro_export]
> +macro_rules! static_call {
> +    // Forward to the real implementation. Separated like this so that we don't have to duplicate
> +    // the documentation.
> +    ($($args:tt)*) => { $crate::static_call::_static_call! { $($args)* } };
> +}
> +
> +pub use {_static_call, static_call, ty_underscore_for};
> 
> -- 
> 2.45.2.505.gda0bf45e8d-goog
>
Miguel Ojeda June 6, 2024, 7:09 p.m. UTC | #2
On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> This is absolutely unreadable gibberish -- how am I supposed to keep
> this in sync with the rest of the static_call infrastructure?

Yeah, they are macros, which look different from "normal" Rust code.

Is there something we could do to help here? I think Alice and others
would be happy to explain how it works and/or help maintain it in the
future if you prefer.

Cheers,
Miguel
Peter Zijlstra June 6, 2024, 7:33 p.m. UTC | #3
On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > This is absolutely unreadable gibberish -- how am I supposed to keep
> > this in sync with the rest of the static_call infrastructure?
> 
> Yeah, they are macros, which look different from "normal" Rust code.

Macros like CPP ?

> Is there something we could do to help here? I think Alice and others
> would be happy to explain how it works and/or help maintain it in the
> future if you prefer.

Write a new language that looks more like C -- pretty please ? :-)

Mostly I would just really like you to just use arm/jump_label.h,
they're inline functions and should be doable with IR, no weirdo CPP
involved in this case.
Alice Ryhl June 7, 2024, 9:43 a.m. UTC | #4
Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > this in sync with the rest of the static_call infrastructure?
> > 
> > Yeah, they are macros, which look different from "normal" Rust code.
> 
> Macros like CPP ?

Yes, this patch series uses declarative macros, which are the closest
that Rust has to the C preprocessor. They are powerful, but just like
CPP, they can become pretty complicated and hard to read if you are
doing something non-trivial.

The macro_rules! block is how you define a new declarative macro.

The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
the declarative macro. This syntax means:

1. The input starts with any identifier, which we call $name.
2. Then there must be a ( token.
3. Then the $(),* part defines a repetition group. The contents inside
   the parenthesises are what is being repeated. The comma means that
   repetitions are separated by commas. The asterisk means that the
   contents may be repeated any number of times, including zero times.
   (Alternatives are + which means repeated at least once, and ? which
   means that the contents may be repeated zero or one time.)
4. The contents of the repetition group will be an expression, which we
   call $args.
5. After the last expression, we have $(,)? which means that you can
   have zero or one commas after the last expression. Rust usually
   allows trailing commas in lists, which is why I included it.
6. And finally, you must close the input with a ) token.

So for example, you might invoke the macro like this:

static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));

Here, $name will be tp_func_my_tracepoint, and the repetition group is
repeated twice, with $args first corresponding to `__data` and then to
`&mut my_task_struct` when the macro is expanded. The $(,)? group is
repeated zero times.


Inside the macro, you will see things such as:
$crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }

The Rust syntax for invoking a macro has an exclamation mark after the
name, so you know that $crate::macros::paste is a macro. The `paste`
macro just emits its input unchanged, except that any identifiers
between [< and >] are concatenated into a single identifier. So if $name
is my_static_key, then the above invocation of paste! emits:

	$crate::bindings::__SCK__my_static_key;

The $crate::bindings module is where the output of bindgen goes, so this
should correspond to the C symbol called __SCK__my_static_key.

> > Is there something we could do to help here? I think Alice and others
> > would be happy to explain how it works and/or help maintain it in the
> > future if you prefer.
> 
> Write a new language that looks more like C -- pretty please ? :-)
> 
> Mostly I would just really like you to just use arm/jump_label.h,
> they're inline functions and should be doable with IR, no weirdo CPP
> involved in this case.

I assume that you're referring to static_key_false here? I don't think
that function can be exposed using IR because it passes the function
argument called key as an "i" argument to an inline assembly block. Any
attempt to compile static_key_false without knowing the value of key at
compile time will surely fail to compile with the

	invalid operand for inline asm constraint 'i'

error.

Alice
Peter Zijlstra June 7, 2024, 10:52 a.m. UTC | #5
On Fri, Jun 07, 2024 at 09:43:29AM +0000, Alice Ryhl wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > > this in sync with the rest of the static_call infrastructure?
> > > 
> > > Yeah, they are macros, which look different from "normal" Rust code.
> > 
> > Macros like CPP ?
> 
> Yes, this patch series uses declarative macros, which are the closest
> that Rust has to the C preprocessor. They are powerful, but just like
> CPP, they can become pretty complicated and hard to read if you are
> doing something non-trivial.
> 
> The macro_rules! block is how you define a new declarative macro.

I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going
to go away. So I'm reading your macros do NOT rule.

> The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
> the declarative macro. This syntax means:
> 
> 1. The input starts with any identifier, which we call $name.
> 2. Then there must be a ( token.

The above exaple fails, because the next token is :ident, whatever the
heck that might be. Also, extra points for line-noise due to lack of
whitespace.

> So for example, you might invoke the macro like this:
> 
> static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));

static_call NOT (blah dog blah);

> Inside the macro, you will see things such as:
> $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
> 
> The Rust syntax for invoking a macro has an exclamation mark after the

Like I said before, the creator of Rust must've been an esoteric
language freak and must've wanted to make this unreadable on purpose :/

Also, why the white space beteen the :: scope operator and the [< thing?
that's just weird. I would then expect the output to be:

  ...::bindings:: __SCK__my_static_key

> name, so you know that $crate::macros::paste is a macro. The `paste`
> macro just emits its input unchanged, except that any identifiers
> between [< and >] are concatenated into a single identifier. So if $name
> is my_static_key, then the above invocation of paste! emits:
> 
> 	$crate::bindings::__SCK__my_static_key;

But it doesn't, so it isn't unmodified, it seems to strip whitespace.

> The $crate::bindings module is where the output of bindgen goes, so this
> should correspond to the C symbol called __SCK__my_static_key.
> 
> > > Is there something we could do to help here? I think Alice and others
> > > would be happy to explain how it works and/or help maintain it in the
> > > future if you prefer.
> > 
> > Write a new language that looks more like C -- pretty please ? :-)
> > 
> > Mostly I would just really like you to just use arm/jump_label.h,
> > they're inline functions and should be doable with IR, no weirdo CPP
> > involved in this case.
> 
> I assume that you're referring to static_key_false here? I don't think
> that function can be exposed using IR because it passes the function
> argument called key as an "i" argument to an inline assembly block. Any
> attempt to compile static_key_false without knowing the value of key at
> compile time will surely fail to compile with the
> 
> 	invalid operand for inline asm constraint 'i'
> 
> error.

You can have clang read the header files and compile them into
Intermediate-Representation, and have it splice the lot into the Rust
crap's IR and voila, compile time.

You just need to extend the rust thing to be able to consume C header
files.
Alice Ryhl June 7, 2024, 11:08 a.m. UTC | #6
On Fri, Jun 7, 2024 at 12:52 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Jun 07, 2024 at 09:43:29AM +0000, Alice Ryhl wrote:
> > Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > > > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > > >
> > > > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > > > this in sync with the rest of the static_call infrastructure?
> > > >
> > > > Yeah, they are macros, which look different from "normal" Rust code.
> > >
> > > Macros like CPP ?
> >
> > Yes, this patch series uses declarative macros, which are the closest
> > that Rust has to the C preprocessor. They are powerful, but just like
> > CPP, they can become pretty complicated and hard to read if you are
> > doing something non-trivial.
> >
> > The macro_rules! block is how you define a new declarative macro.
>
> I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going
> to go away. So I'm reading your macros do NOT rule.

So you already understand ! in two ways, but you don't want to add a
third? That seems to violate the Zero One Infinity rule. :)

https://en.wikipedia.org/wiki/Zero_one_infinity_rule

> > The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
> > the declarative macro. This syntax means:
> >
> > 1. The input starts with any identifier, which we call $name.
> > 2. Then there must be a ( token.
>
> The above exaple fails, because the next token is :ident, whatever the
> heck that might be. Also, extra points for line-noise due to lack of
> whitespace.

The :ident part means that $name should be parsed as an identifier.
Similarly, the :expr part means that $args should be parsed as an
expression. It doesn't mean that the input should literally contain
":ident".

> > So for example, you might invoke the macro like this:
> >
> > static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));
>
> static_call NOT (blah dog blah);
>
> > Inside the macro, you will see things such as:
> > $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
> >
> > The Rust syntax for invoking a macro has an exclamation mark after the
>
> Like I said before, the creator of Rust must've been an esoteric
> language freak and must've wanted to make this unreadable on purpose :/
>
> Also, why the white space beteen the :: scope operator and the [< thing?
> that's just weird. I would then expect the output to be:
>
>   ...::bindings:: __SCK__my_static_key

Sorry, you are right. There is a space in the output.

> > name, so you know that $crate::macros::paste is a macro. The `paste`
> > macro just emits its input unchanged, except that any identifiers
> > between [< and >] are concatenated into a single identifier. So if $name
> > is my_static_key, then the above invocation of paste! emits:
> >
> >       $crate::bindings::__SCK__my_static_key;
>
> But it doesn't, so it isn't unmodified, it seems to strip whitespace.

Thanks for the correction. The actual output is:

$crate::bindings:: __SCK__my_static_key;

However, although whitespace is generally not used here, the syntax allows it.

> > The $crate::bindings module is where the output of bindgen goes, so this
> > should correspond to the C symbol called __SCK__my_static_key.
> >
> > > > Is there something we could do to help here? I think Alice and others
> > > > would be happy to explain how it works and/or help maintain it in the
> > > > future if you prefer.
> > >
> > > Write a new language that looks more like C -- pretty please ? :-)
> > >
> > > Mostly I would just really like you to just use arm/jump_label.h,
> > > they're inline functions and should be doable with IR, no weirdo CPP
> > > involved in this case.
> >
> > I assume that you're referring to static_key_false here? I don't think
> > that function can be exposed using IR because it passes the function
> > argument called key as an "i" argument to an inline assembly block. Any
> > attempt to compile static_key_false without knowing the value of key at
> > compile time will surely fail to compile with the
> >
> >       invalid operand for inline asm constraint 'i'
> >
> > error.
>
> You can have clang read the header files and compile them into
> Intermediate-Representation, and have it splice the lot into the Rust
> crap's IR and voila, compile time.
>
> You just need to extend the rust thing to be able to consume C header
> files.

I wish! There are people, including me, who want this. See e.g. this
very recent document:
https://rust-lang.github.io/rust-project-goals/2024h2/Seamless-C-Support.html

But there are also people who dislike the idea, so it does not have
unanimous support yet, unfortunately.

Ultimately, I have to work with what exists today.

Alice
Miguel Ojeda June 7, 2024, 11:46 a.m. UTC | #7
On Fri, Jun 7, 2024 at 12:52 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going
> to go away. So I'm reading your macros do NOT rule.

It makes it clear what is macro call or not. They could have gone for
UPPERCASE names (for instance), yes. On the other hand, they do not
work like C macros and are ~hygienic, so it also makes sense to avoid
confusion here.

I mean, I am not suggesting to do a CPP-pass to Rust files, but if
someone really, really wanted to mix them in a single file, it would
be nice to not confuse the two kinds. :)

Generally they feel "closer" to the language (given what they
do/support) compared to the CPP ones, so it also makes sense they
don't "shout" so much compared to UPPERCASE, if that makes sense.

> The above exaple fails, because the next token is :ident, whatever the
> heck that might be. Also, extra points for line-noise due to lack of
> whitespace.

$name:ident means "match what Rust would consider an identifier here
and call it $name for the purposes of this macro".

So, for instance, $x:ident matches:

    a
    a2
    a_b

But it would not match:

    2a
    a-b
    a _b

For the usual reasons why those are not identifiers.

https://godbolt.org/z/G7v4j67dc

    fn f(x: i32) -> i32 {
        x * 2
    }

    macro_rules! f {
        ($x:ident) => { $x * 2 }
    }

    fn main() {
        let a = 42;

        let b = f(a);       // Function.
        let c = f!(a);      // Macro.

        //let c = f!(a2);   // Works, but the variable does not exist.
        //let c = f!(2a);   // Error: no rules expected the token `2a`.

        //let c = f!(a_b);  // Works, but the variable does not exist.
        //let c = f!(a-b);  // Error: no rules expected the token `-`.
        //let c = f!(a_ b); // Error: no rules expected the token `b`.

        println!("{a} {b} {c}");
    }

I hope this makes it clearer.

> You just need to extend the rust thing to be able to consume C header
> files.

I agree, because in practice it is quite useful for a language like
Rust that consuming C header files is "natively" supported.

Though it also has downsides and is a big decision, which is why, like
Alice mentioned, some people agree, and some people don't.
Nevertheless, we have been doing our best for a long time to get the
best we can for the kernel -- just 2 days ago we told the Rust project
in one of our meetings that it would be nice to see that particular
"project goal" from that document realized (among others).

Cheers,
Miguel
diff mbox series

Patch

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fbd91a48ff8b..d534b1178955 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@ 
 pub mod prelude;
 pub mod print;
 mod static_assert;
+pub mod static_call;
 #[doc(hidden)]
 pub mod std_vendor;
 pub mod str;
diff --git a/rust/kernel/static_call.rs b/rust/kernel/static_call.rs
new file mode 100644
index 000000000000..f7b8ba7bf1fb
--- /dev/null
+++ b/rust/kernel/static_call.rs
@@ -0,0 +1,92 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static calls.
+
+#[macro_export]
+#[doc(hidden)]
+macro_rules! ty_underscore_for {
+    ($arg:expr) => {
+        _
+    };
+}
+
+#[doc(hidden)]
+#[repr(transparent)]
+pub struct AddressableStaticCallKey {
+    _ptr: *const bindings::static_call_key,
+}
+unsafe impl Sync for AddressableStaticCallKey {}
+impl AddressableStaticCallKey {
+    pub const fn new(ptr: *const bindings::static_call_key) -> Self {
+        Self { _ptr: ptr }
+    }
+}
+
+#[cfg(CONFIG_HAVE_STATIC_CALL)]
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _static_call {
+    ($name:ident($($args:expr),* $(,)?)) => {{
+        // Symbol mangling will give this symbol a unique name.
+        #[cfg(CONFIG_HAVE_STATIC_CALL_INLINE)]
+        #[link_section = ".discard.addressable"]
+        #[used]
+        static __ADDRESSABLE: $crate::static_call::AddressableStaticCallKey = unsafe {
+            $crate::static_call::AddressableStaticCallKey::new(::core::ptr::addr_of!(
+                $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
+            ))
+        };
+
+        let fn_ptr: unsafe extern "C" fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
+            $crate::macros::paste! { $crate::bindings:: [<__SCT__ $name >]; };
+        (fn_ptr)($($args),*)
+    }};
+}
+
+#[cfg(not(CONFIG_HAVE_STATIC_CALL))]
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _static_call {
+    ($name:ident($($args:expr),* $(,)?)) => {{
+        let void_ptr_fn: *mut ::core::ffi::c_void =
+            $crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }.func;
+
+        let fn_ptr: unsafe extern "C" fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
+            if true {
+                ::core::mem::transmute(void_ptr_fn)
+            } else {
+                // This is dead code, but it influences type inference on `fn_ptr` so that we
+                // transmute the function pointer to the right type.
+                $crate::macros::paste! { $crate::bindings:: [<__SCT__ $name >]; }
+            };
+
+        (fn_ptr)($($args),*)
+    }};
+}
+
+/// Statically call a global function.
+///
+/// # Safety
+///
+/// This macro will call the provided function. It is up to the caller to uphold the safety
+/// guarantees of the function.
+///
+/// # Examples
+///
+/// ```ignore
+/// fn call_static() {
+///     unsafe {
+///         static_call! { your_static_call() };
+///     }
+/// }
+/// ```
+#[macro_export]
+macro_rules! static_call {
+    // Forward to the real implementation. Separated like this so that we don't have to duplicate
+    // the documentation.
+    ($($args:tt)*) => { $crate::static_call::_static_call! { $($args)* } };
+}
+
+pub use {_static_call, static_call, ty_underscore_for};