Re: [PATCH v2] docs: rust: extend abstraction and binding documentation

[Trevor Gross <tmgross@umich.edu>]

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