rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] docs: rust: extend abstraction and binding documentation
@ 2024-02-09  7:05 Dirk Behme
  2024-02-10  7:37 ` Trevor Gross
  2024-02-11  7:54 ` Valentin Obst
  0 siblings, 2 replies; 3+ messages in thread
From: Dirk Behme @ 2024-02-09  7:05 UTC (permalink / raw)
  To: rust-for-linux; +Cc: Dirk Behme, Valentin Obst, Miguel Ojeda

Add some basics explained by Miguel in [1] to the documentation.
And connect it with some hints where this is implemented in the
kernel.

[1] https://www.linuxfoundation.org/webinars/rust-for-linux-writing-abstractions-and-drivers

Cc: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
Changes in v2: Try to incorporate the v1 comments from Valentin and Miguel.

 Documentation/rust/general-information.rst | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/Documentation/rust/general-information.rst b/Documentation/rust/general-information.rst
index 081397827a7e..91a5a69c7c74 100644
--- a/Documentation/rust/general-information.rst
+++ b/Documentation/rust/general-information.rst
@@ -64,6 +64,54 @@ but it is intended that coverage is expanded as time goes on. "Leaf" modules
 (e.g. drivers) should not use the C bindings directly. Instead, subsystems
 should provide as-safe-as-possible abstractions as needed.
 
+.. code-block::
+
+	                                                rust/bindings/
+                                                       (rust/helpers.c)
+
+                                                          include/ -----+ <-+
+                                                                        |   |
+         drivers/              rust/kernel/              +----------+ <-+   |
+           fs/                                           | bindgen  |       |
+          .../            +-------------------+          +----------+ --+   |
+                          |    Abstractions   |                         |   |
+       +---------+        | +------+ +------+ |          +----------+   |   |
+       | my_foo  | -----> | | foo  | | bar  | | -------> | Bindings | <-+   |
+       | driver  |  Safe  | | sub- | | sub- | |  Unsafe  |          |       |
+       +---------+        | |system| |system| |          | bindings | <-----+
+            |             | +------+ +------+ |          |  crate   |       |
+            |             |   kernel crate    |          +----------+       |
+            |             +-------------------+                             |
+            |                                                               |
+            +------------------# FORBIDDEN #--------------------------------+
+
+The main idea is to encapsulate all unsafe handling in carefully reviewed
+and documented abstractions. These are then considered to be sound. With this model
+it is ensured that users of the abstractions ("my_foo driver") can't do anything
+unsound if
+
+#. the abstractions are sound
+#. they don't use ``unsafe()``
+
+Bindings
+~~~~~~~~
+
+By including a C header from ``include/`` into ``rust/bindings/bindings_helper.h``
+the ``bindgen`` tool will auto-generate the bindings for the included subsystem.
+See the ``*_generated.rs`` output files in the ``rust/bindings/`` directory.
+
+For parts of the C header ``bindgen`` doesn't auto generate, e.g. C ``inline``
+functions or macros, there is the option to add a small wrapper function
+to ``rust/helpers.c`` to make it available for the Rust side as well.
+
+Abstractions
+~~~~~~~~~~~~
+
+Abstractions are the layer between the bindings and the in-kernel users. For example
+the drivers or file systems written in Rust. They are located in ``rust/kernel/``
+and their role is to encapsulate the unsafe access to the bindings into a safe API
+that they expose to their users.
+
 
 Conditional compilation
 -----------------------
-- 
2.28.0


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

* Re: [PATCH v2] docs: rust: extend abstraction and binding documentation
  2024-02-09  7:05 [PATCH v2] docs: rust: extend abstraction and binding documentation Dirk Behme
@ 2024-02-10  7:37 ` Trevor Gross
  2024-02-11  7:54 ` Valentin Obst
  1 sibling, 0 replies; 3+ messages in thread
From: Trevor Gross @ 2024-02-10  7:37 UTC (permalink / raw)
  To: Dirk Behme; +Cc: rust-for-linux, Valentin Obst, Miguel Ojeda

On Fri, Feb 9, 2024 at 1:06 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Add some basics explained by Miguel in [1] to the documentation.
> And connect it with some hints where this is implemented in the
> kernel.
>
> [1] https://www.linuxfoundation.org/webinars/rust-for-linux-writing-abstractions-and-drivers
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> Changes in v2: Try to incorporate the v1 comments from Valentin and Miguel.
>
>  Documentation/rust/general-information.rst | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/Documentation/rust/general-information.rst b/Documentation/rust/general-information.rst
> index 081397827a7e..91a5a69c7c74 100644
> --- a/Documentation/rust/general-information.rst
> +++ b/Documentation/rust/general-information.rst
> @@ -64,6 +64,54 @@ but it is intended that coverage is expanded as time goes on. "Leaf" modules
>  (e.g. drivers) should not use the C bindings directly. Instead, subsystems
>  should provide as-safe-as-possible abstractions as needed.
>
> +.. code-block::
> +
> +                                                       rust/bindings/
> +                                                       (rust/helpers.c)
> +
> +                                                          include/ -----+ <-+
> +                                                                        |   |
> +         drivers/              rust/kernel/              +----------+ <-+   |
> +           fs/                                           | bindgen  |       |
> +          .../            +-------------------+          +----------+ --+   |
> +                          |    Abstractions   |                         |   |
> +       +---------+        | +------+ +------+ |          +----------+   |   |
> +       | my_foo  | -----> | | foo  | | bar  | | -------> | Bindings | <-+   |
> +       | driver  |  Safe  | | sub- | | sub- | |  Unsafe  |          |       |
> +       +---------+        | |system| |system| |          | bindings | <-----+
> +            |             | +------+ +------+ |          |  crate   |       |
> +            |             |   kernel crate    |          +----------+       |
> +            |             +-------------------+                             |
> +            |                                                               |
> +            +------------------# FORBIDDEN #--------------------------------+

Awesome ascii flowchart :)

> +The main idea is to encapsulate all unsafe handling in carefully reviewed

in -> into

> +and documented abstractions. These are then considered to be sound. With this model
> +it is ensured that users of the abstractions ("my_foo driver") can't do anything
> +unsound if

"unsound if" -> "unsound if:"

> +
> +#. the abstractions are sound
> +#. they don't use ``unsafe()``

Technically there is no `unsafe()` at this time, only `unsafe { ... }`
:) (and `unsafe impl`)

> +Bindings
> +~~~~~~~~
> +
> +By including a C header from ``include/`` into ``rust/bindings/bindings_helper.h``
> +the ``bindgen`` tool will auto-generate the bindings for the included subsystem.
> +See the ``*_generated.rs`` output files in the ``rust/bindings/`` directory.

Worth mentioning `rust/bindings/*_generated` is only available after building

> +For parts of the C header ``bindgen`` doesn't auto generate, e.g. C ``inline``

".. C header +that ``bindgen`` doesn't autogenerate"

> +functions or macros, there is the option to add a small wrapper function
> +to ``rust/helpers.c`` to make it available for the Rust side as well.

I would say "it is acceptable to add a small wrapper function". "there
is an option to ..." sounds a bit like there's a kconfig option to
enable.

> +Abstractions
> +~~~~~~~~~~~~
> +
> +Abstractions are the layer between the bindings and the in-kernel users. For example
> +the drivers or file systems written in Rust. They are located in ``rust/kernel/``
> +and their role is to encapsulate the unsafe access to the bindings into a safe API
> +that they expose to their users.

I would move the "For example ... in Rust" to the end as something
like "Users of abstractions include things like drivers or file
systems written in Rust" to make it more clear what is in
`rust/kernel`.

This looks great, the diagram adds a lot to making the organization more clear.

Thanks,
Trevor

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

* Re: [PATCH v2] docs: rust: extend abstraction and binding documentation
  2024-02-09  7:05 [PATCH v2] docs: rust: extend abstraction and binding documentation Dirk Behme
  2024-02-10  7:37 ` Trevor Gross
@ 2024-02-11  7:54 ` Valentin Obst
  1 sibling, 0 replies; 3+ messages in thread
From: Valentin Obst @ 2024-02-11  7:54 UTC (permalink / raw)
  To: dirk.behme; +Cc: kernel, ojeda, rust-for-linux, tmgross

> +.. code-block::
> +
> +	                                                rust/bindings/
> +                                                       (rust/helpers.c)
> +
> +                                                          include/ -----+ <-+
> +                                                                        |   |
> +         drivers/              rust/kernel/              +----------+ <-+   |
> +           fs/                                           | bindgen  |       |
> +          .../            +-------------------+          +----------+ --+   |
> +                          |    Abstractions   |                         |   |
> +       +---------+        | +------+ +------+ |          +----------+   |   |
> +       | my_foo  | -----> | | foo  | | bar  | | -------> | Bindings | <-+   |
> +       | driver  |  Safe  | | sub- | | sub- | |  Unsafe  |          |       |
> +       +---------+        | |system| |system| |          | bindings | <-----+
> +            |             | +------+ +------+ |          |  crate   |       |
> +            |             |   kernel crate    |          +----------+       |
> +            |             +-------------------+                             |
> +            |                                                               |
> +            +------------------# FORBIDDEN #--------------------------------+
> +
> +The main idea is to encapsulate all unsafe handling in carefully reviewed
> +and documented abstractions. These are then considered to be sound. With this model
> +it is ensured that users of the abstractions ("my_foo driver") can't do anything
> +unsound if
> +
> +#. the abstractions are sound
> +#. they don't use ``unsafe()``

I'd like to follow up on some of the things that Miguel mentioned in his
earlier message:

- To make it clearer what is meant by "unsafe handling": One could say
  "... encapsulate all direct interaction with the kernel's C APIs ...".
  This requires using unsafe Rust, but also leaves the door open for
  drivers to use unsafe Rust, e.g., for performance optimizations or to
  call unsafe abstractions.
- To avoid saying that "... [abstractions] are considered to be sound
  [after review] ...": One could say "The goal must be that users of
  these abstractions cannot introduce undefined behavior as long as:
  1. The abstractions are correct, & 2. they uphold the preconditions
  of all unsafe operations that they perform."

> +
> +Bindings
> +~~~~~~~~
> +
> +By including a C header from ``include/`` into ``rust/bindings/bindings_helper.h``
> +the ``bindgen`` tool will auto-generate the bindings for the included subsystem.
> +See the ``*_generated.rs`` output files in the ``rust/bindings/`` directory.
> +
> +For parts of the C header ``bindgen`` doesn't auto generate, e.g. C ``inline``
> +functions or macros, there is the option to add a small wrapper function
> +to ``rust/helpers.c`` to make it available for the Rust side as well.
> +
> +Abstractions
> +~~~~~~~~~~~~
> +
> +Abstractions are the layer between the bindings and the in-kernel users. For example
> +the drivers or file systems written in Rust. They are located in ``rust/kernel/``
> +and their role is to encapsulate the unsafe access to the bindings into a safe API
> +that they expose to their users.

I'd think one should keep the "as-safe-as-possible" aspect here, as
there will always be cases where it is not possible to provide safe
abstractions.

Another aspect about these APIs that I don't see mentioned here is that
they should be "ergonomic", in the sense that they turn "C-ish"
interfaces into something that allows you to write "idiomatic" Rust
code. Basic examples would be to turn resource acquisition and release
into constructors and destructors or integer error codes into Results,
and so on. I'm not entirely sure to which extend this is covered by the
safety aspect (they have a non-zero overlap), but perhaps it is
something that could be mentioned here.

	- Best Valentin

> +

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

end of thread, other threads:[~2024-02-11  7:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-09  7:05 [PATCH v2] docs: rust: extend abstraction and binding documentation Dirk Behme
2024-02-10  7:37 ` Trevor Gross
2024-02-11  7:54 ` Valentin Obst

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).