Rust support
Subsystems (26)
Miguel Ojeda
maintainerv9 cover Rust support
Rust support
This is the patch series (v9) to add support for Rust as a second language to the Linux kernel.
If you are interested in following this effort, please join us in the mailing list at:
and take a look at the project itself at:
https://github.com/Rust-for-Linux
As usual, special thanks go to ISRG (Internet Security Research Group) and Google for their financial support on this endeavor.
Cheers,
Miguel
--
# Rust support
This cover letter explains the major changes and updates done since the previous ones. For those, please see:
RFC: https://lore.kernel.org/lkml/[email protected]/ v1: https://lore.kernel.org/lkml/[email protected]/ v2: https://lore.kernel.org/lkml/[email protected]/ v3: https://lore.kernel.org/lkml/[email protected]/ v4: https://lore.kernel.org/lkml/[email protected]/ v5: https://lore.kernel.org/lkml/[email protected]/ v6: https://lore.kernel.org/lkml/[email protected]/ v7: https://lore.kernel.org/lkml/[email protected]/ v8: https://lore.kernel.org/lkml/[email protected]/
This is a trimmed down v8. It has enough support to compile a minimal Rust kernel module, with the aim to get the "core" support in first and then start upstreaming the rest piece by piece.
The kernel module is able to create a Vec<i32> (a contiguous, growable array type), push some numbers and print them on unload to the kernel log with the pr_info! macro.
The patch series could be made even more minimal by removing some of that too, but this seemed like a good balance.
After the diet regime, 3% of the kernel crate remains (down to 500 lines), 60% of alloc, and the "adapt alloc" commit is 100 lines. Overall, we went from 40 to 13 klines.
Most of the code has been around in linux-next for some months now, but to trim down things I had to do minor changes. In any case, please let me know if this selection is in line with what you expected.
## Patch series status
The Rust support is still to be considered experimental. However, support is good enough that kernel developers can start working on the Rust abstractions for subsystems and write drivers and other modules.
The current series will appear in the next linux-next, as usual.
I have kept the docs as they were in v8 since they showcase best what the docs would eventually look like:
https://rust-for-linux.github.io/docs/kernel/
As usual, please see the following link for the live list of unstable Rust features we are using (note that this trimmed version does not require all of them):
https://github.com/Rust-for-Linux/linux/issues/2
## Conferences, meetings and liaisons
Join us in LPC 2022 (Linux Plumbers Conference) for the Rust MC (microconference)! The schedule is available at:
https://lpc.events/event/16/sessions/150/
We will be talking about GCC Rust (the Rust frontend for GCC), rustc_codegen_gcc (the GCC backend for rustc), Rust for Linux, the Rust NVMe driver, the integration of Rust with the Kernel Testing Service and Rust in the Kernel (via eBPF).
In addition, I would like to personally thank Google and ISRG (Internet Security Research Group) for sponsoring Kangrejos, the Rust for Linux workshop:
## Acknowledgements
The signatures in the main commits correspond to the people that wrote code that has ended up in them at the present time. For details on contributions to code and discussions, please see our repository:
https://github.com/Rust-for-Linux/linux
However, we would like to give credit to everyone that has contributed in one way or another to the Rust for Linux project. Since the previous cover letter:
- Boqun Feng, Konstantin Shelekhin and David Laight for their reviews of some of the v8 patches.
- Matthew Wilcox, Greg Kroah-Hartman, Rasmus Villemoes and Christoph Hellwig for their feedback on v8.
- Björn Roy Baron, Gary Guo and Boqun Feng for stepping up as reviewers in MAINTAINERS.
- Jon Olson for working on a cpu module with utilities for SMP systems.
- Alice Ryhl for working on adding BINDER_TYPE_PTR support.
- FUJITA Tomonori for working on PCI support for a NVMe Rust driver.
- Andreas Reindl for working on adding missing SAFETY comments.
- Wei Liu for taking the time to answer questions from newcomers in Zulip.
- Philip Li, Yujie Liu et al. for continuing their work on adding Rust support to the Intel 0DAY/LKP kernel test robot.
- Philip Herron and Arthur Cohen (and his supporters Open Source Security and Embecosm) et al. for their ongoing work on GCC Rust.
- Antoni Boucher (and his supporters) et al. for their ongoing work on rustc_codegen_gcc.
- Emilio Cobos Álvarez et al. for their work on bindgen, including on issues that affect the kernel.
- Mats Larsen, Marc Poulhiès et al. for their ongoing work on improving Rust support in Compiler Explorer.
- Many folks that have reported issues, tested the project, helped spread the word, joined discussions and contributed in other ways!
Please see also the acknowledgements on the previous cover letters.
Boqun Feng (2):
kallsyms: use sizeof instead of hardcoded size
kallsyms: avoid hardcoding buffer size
Daniel Xu (1):
scripts: add is_rust_module.sh
Gary Guo (1):
vsprintf: add new %pA format specifier
Miguel Ojeda (22):
kallsyms: add static relationship between KSYM_NAME_LEN{,_BUFFER}
kallsyms: support "big" kernel symbols
kallsyms: increase maximum kernel symbol length to 512
rust: add C helpers
rust: import upstream alloc crate
rust: adapt alloc crate to the kernel
rust: add compiler_builtins crate
rust: add macros crate
rust: add bindings crate
rust: export generated symbols
scripts: checkpatch: diagnose uses of %pA in the C side as errors
scripts: checkpatch: enable language-independent checks for Rust
scripts: decode_stacktrace: demangle Rust symbols
scripts: add generate_rust_analyzer.py
scripts: add generate_rust_target.rs
scripts: add rust_is_available.sh
rust: add .rustfmt.toml
Kbuild: add Rust support
docs: add Rust documentation
x86: enable initial Rust support
samples: add first Rust examples
MAINTAINERS: Rust
Wedson Almeida Filho (1):
rust: add kernel crate
.gitignore | 6 + .rustfmt.toml | 12 + Documentation/core-api/printk-formats.rst | 10 + Documentation/doc-guide/kernel-doc.rst | 3 + Documentation/index.rst | 1 + Documentation/kbuild/kbuild.rst | 17 + Documentation/kbuild/makefiles.rst | 50 +- Documentation/process/changes.rst | 41 + Documentation/rust/arch-support.rst | 19 + Documentation/rust/coding-guidelines.rst | 216 ++ Documentation/rust/general-information.rst | 79 + Documentation/rust/index.rst | 22 + Documentation/rust/quick-start.rst | 232 ++ MAINTAINERS | 18 + Makefile | 172 +- arch/Kconfig | 6 + arch/x86/Kconfig | 1 + arch/x86/Makefile | 10 + include/linux/compiler_types.h | 6 +- include/linux/kallsyms.h | 2 +- init/Kconfig | 46 +- kernel/configs/rust.config | 1 + kernel/kallsyms.c | 26 +- kernel/livepatch/core.c | 4 +- lib/Kconfig.debug | 34 + lib/vsprintf.c | 13 + rust/.gitignore | 8 + rust/Makefile | 381 +++ rust/alloc/README.md | 33 + rust/alloc/alloc.rs | 440 +++ rust/alloc/borrow.rs | 498 +++ rust/alloc/boxed.rs | 2028 +++++++++++ rust/alloc/collections/mod.rs | 156 + rust/alloc/lib.rs | 244 ++ rust/alloc/raw_vec.rs | 527 +++ rust/alloc/slice.rs | 1204 +++++++ rust/alloc/vec/drain.rs | 186 ++ rust/alloc/vec/drain_filter.rs | 145 + rust/alloc/vec/into_iter.rs | 366 ++ rust/alloc/vec/is_zero.rs | 120 + rust/alloc/vec/mod.rs | 3140 ++++++++++++++++++ rust/alloc/vec/partial_eq.rs | 49 + rust/bindgen_parameters | 21 + rust/bindings/bindings_helper.h | 13 + rust/bindings/lib.rs | 53 + rust/compiler_builtins.rs | 63 + rust/exports.c | 21 + rust/helpers.c | 51 + rust/kernel/allocator.rs | 64 + rust/kernel/error.rs | 59 + rust/kernel/lib.rs | 78 + rust/kernel/prelude.rs | 20 + rust/kernel/print.rs | 198 ++ rust/kernel/str.rs | 72 + rust/macros/helpers.rs | 51 + rust/macros/lib.rs | 72 + rust/macros/module.rs | 282 ++ samples/Kconfig | 2 + samples/Makefile | 1 + samples/rust/Kconfig | 30 + samples/rust/Makefile | 5 + samples/rust/hostprogs/.gitignore | 3 + samples/rust/hostprogs/Makefile | 5 + samples/rust/hostprogs/a.rs | 7 + samples/rust/hostprogs/b.rs | 5 + samples/rust/hostprogs/single.rs | 12 + samples/rust/rust_minimal.rs | 38 + scripts/.gitignore | 1 + scripts/Kconfig.include | 6 +- scripts/Makefile | 3 + scripts/Makefile.build | 60 + scripts/Makefile.debug | 10 + scripts/Makefile.host | 34 +- scripts/Makefile.lib | 12 + scripts/Makefile.modfinal | 8 +- scripts/cc-version.sh | 12 +- scripts/checkpatch.pl | 12 +- scripts/decode_stacktrace.sh | 14 + scripts/generate_rust_analyzer.py | 135 + scripts/generate_rust_target.rs | 182 + scripts/is_rust_module.sh | 16 + scripts/kallsyms.c | 47 +- scripts/kconfig/confdata.c | 75 + scripts/min-tool-version.sh | 6 + scripts/rust_is_available.sh | 160 + scripts/rust_is_available_bindgen_libclang.h | 2 + tools/include/linux/kallsyms.h | 2 +- tools/lib/perf/include/perf/event.h | 2 +- tools/lib/symbol/kallsyms.h | 2 +- 89 files changed, 12548 insertions(+), 51 deletions(-) create mode 100644 .rustfmt.toml create mode 100644 Documentation/rust/arch-support.rst create mode 100644 Documentation/rust/coding-guidelines.rst create mode 100644 Documentation/rust/general-information.rst create mode 100644 Documentation/rust/index.rst create mode 100644 Documentation/rust/quick-start.rst create mode 100644 kernel/configs/rust.config create mode 100644 rust/.gitignore create mode 100644 rust/Makefile create mode 100644 rust/alloc/README.md create mode 100644 rust/alloc/alloc.rs create mode 100644 rust/alloc/borrow.rs create mode 100644 rust/alloc/boxed.rs create mode 100644 rust/alloc/collections/mod.rs create mode 100644 rust/alloc/lib.rs create mode 100644 rust/alloc/raw_vec.rs create mode 100644 rust/alloc/slice.rs create mode 100644 rust/alloc/vec/drain.rs create mode 100644 rust/alloc/vec/drain_filter.rs create mode 100644 rust/alloc/vec/into_iter.rs create mode 100644 rust/alloc/vec/is_zero.rs create mode 100644 rust/alloc/vec/mod.rs create mode 100644 rust/alloc/vec/partial_eq.rs create mode 100644 rust/bindgen_parameters create mode 100644 rust/bindings/bindings_helper.h create mode 100644 rust/bindings/lib.rs create mode 100644 rust/compiler_builtins.rs create mode 100644 rust/exports.c create mode 100644 rust/helpers.c create mode 100644 rust/kernel/allocator.rs create mode 100644 rust/kernel/error.rs create mode 100644 rust/kernel/lib.rs create mode 100644 rust/kernel/prelude.rs create mode 100644 rust/kernel/print.rs create mode 100644 rust/kernel/str.rs create mode 100644 rust/macros/helpers.rs create mode 100644 rust/macros/lib.rs create mode 100644 rust/macros/module.rs create mode 100644 samples/rust/Kconfig create mode 100644 samples/rust/Makefile create mode 100644 samples/rust/hostprogs/.gitignore create mode 100644 samples/rust/hostprogs/Makefile create mode 100644 samples/rust/hostprogs/a.rs create mode 100644 samples/rust/hostprogs/b.rs create mode 100644 samples/rust/hostprogs/single.rs create mode 100644 samples/rust/rust_minimal.rs create mode 100755 scripts/generate_rust_analyzer.py create mode 100644 scripts/generate_rust_target.rs create mode 100755 scripts/is_rust_module.sh create mode 100755 scripts/rust_is_available.sh create mode 100644 scripts/rust_is_available_bindgen_libclang.h
base-commit: 3d7cb6b04c3f3115719235cc6866b10326de34cd
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 1/27 kallsyms: use `sizeof` instead of hardcoded size
From: Boqun Feng <[email protected]>
This removes one place where the 500 constant is hardcoded.
--- scripts/kallsyms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/kallsyms.c +1 -1
@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }
Signature · 1 line
2.37.1
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 27 lines · Miguel Ojeda
From: Boqun Feng <[email protected]>
This removes one place where the
500constant is hardcoded.Signed-off-by Boqun Feng <[email protected]>Co-developed-by Miguel Ojeda <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- scripts/kallsyms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)scripts/kallsyms.c +1
-1@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }Signature · 1 line
2.37.1
And I think that this patch and all other "rust" kallsyms patches allready should have been accepted in the v3 or v5 series.
Regards
Geert Stappers
Signature · 1 line
Silence is hard to parse
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 2 lines · Geert Stappers
Signed-off-by Geert Stappers <[email protected]>
Thanks for the message and the support, but please note that since you are not in the path of the patch, you cannot use this tag; instead look into Reviewed-by etc. See
https://www.kernel.org/doc/html/v5.19/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
and the following section for details.
Quoted 2 lines
And I think that this patch and all other "rust" kallsyms patches allready should have been accepted in the v3 or v5 series.
Yeah, it could be a good idea to get the prerequisites in first. Let's see if the patches get some Reviewed-bys (e.g. I had to remove Kees' one because I had to split the patch).
Cheers,
Miguel
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
In reply to Miguel Ojedascripts/kallsyms.c static struct sym_entry *read_symbol(FILE *in) · line 212
Quoted 13 lines
scripts/kallsyms.c +1
-1@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }
Might be another nit, but IMO it's better to use ARRAY_SIZE() here.
Kees Cook
Reviewed-byRe: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 6 lines · Konstantin Shelekhin
Quoted 6 lines
scripts/kallsyms.c +0
-0@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in)
Quoted 9 lines
scripts/kallsyms.c static struct sym_entry *read_symbol(FILE *in) · line 212
Quoted 7 lines
scripts/kallsyms.c +1
-1rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }Might be another nit, but IMO it's better to use
ARRAY_SIZE()here.
I'm not sure I see a benefit for char arrays. It'll produce the same result, and the tradition for string functions is to use sizeof(). *shrug*
Either way:
Signature · 1 line
Kees Cook
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 17 lines · Kees Cook
Quoted 6 lines · Konstantin Shelekhin
Quoted 6 lines
scripts/kallsyms.c +0
-0@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in)Quoted 9 lines
scripts/kallsyms.c static struct sym_entry *read_symbol(FILE *in) · line 212
Quoted 7 lines
scripts/kallsyms.c +1
-1rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }Might be another nit, but IMO it's better to use
ARRAY_SIZE()here.
Quoted 3 lines
I'm not sure I see a benefit for char arrays. It'll produce the same result, and the tradition for string functions is to use
sizeof(). *shrug*
ARRAY_SIZE() (though not this one) can catch this:
- char array[16];
+ char *array;
Saves me some.
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 22 lines · Konstantin Shelekhin
Quoted 17 lines · Kees Cook
Quoted 6 lines · Konstantin Shelekhin
Quoted 6 lines
scripts/kallsyms.c +0
-0@@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in)Quoted 9 lines
scripts/kallsyms.c static struct sym_entry *read_symbol(FILE *in) · line 212
Quoted 7 lines
scripts/kallsyms.c +1
-1rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name);if (rc !=3 ) { -if (rc !=EOF &&fgets (name,500 , in) ==NULL ) +if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );return NULL ; }Might be another nit, but IMO it's better to use
ARRAY_SIZE()here.Quoted 3 lines
I'm not sure I see a benefit for char arrays. It'll produce the same result, and the tradition for string functions is to use
sizeof(). *shrug*
Quoted 6 lines
ARRAY_SIZE()(though not this one) can catch this:- char array[16];
+ char *array;Saves me some.
Oh, that's an excellent point; I forgot it'll actually compile-time error if the var is a pointer. +1
Signature · 1 line
Kees Cook
Re: [PATCH v9 01/27] kallsyms: use `sizeof` instead of hardcoded size
Quoted 3 lines · Kees Cook
Oh, that's an excellent point; I forgot it'll actually compile-time error if the var is a pointer. +1
In this case it doesn't, because scripts/kallsyms.c defines its own ARRAY_SIZE that doesn't have the __must_be_array.
I have changed it for v10 anyway, since that way we may benefit in the future if the ARRAY_SIZE here gains the check.
Cheers,
Miguel
Miguel Ojeda
maintainerPATCH v9 2/27 kallsyms: avoid hardcoding buffer size
In reply to Miguel OjedaFrom: Boqun Feng <[email protected]>
This introduces KSYM_NAME_LEN_BUFFER in place of the previously hardcoded size of the input buffer.
It will also make it easier to update the size in a single place in a later patch.
--- scripts/kallsyms.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
scripts/kallsyms.c +8 -2
@@ -27,8 +27,14 @@#define ARRAY_SIZE (arr) (sizeof(arr) / sizeof(arr[0])) +#define _stringify_1 (x) #x +#define _stringify (x) _stringify_1(x) +#define KSYM_NAME_LEN 128 +/* A substantially bigger size than the current maximum. */ +#define KSYM_NAME_LEN_BUFFER 499 +struct sym_entry {unsigned long long addr;unsigned int len;
@@ -198,13 +204,13 @@ static void check_symbol_range(const char *sym, unsigned long long addr,static struct sym_entry *read_symbol (FILE *in) { -char name[500 ], type; +char name[KSYM_NAME_LEN_BUFFER +1 ], type;unsigned long long addr;unsigned int len;struct sym_entry *sym;int rc; - rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name); + rc = fscanf(in, "%llx %c %" _stringify(KSYM_NAME_LEN_BUFFER) "s\n", &addr, &type, name);if (rc !=3 ) {if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );
Signature · 1 line
2.37.1
Re: [PATCH v9 02/27] kallsyms: avoid hardcoding buffer size
Quoted 11 lines · Miguel Ojeda
From: Boqun Feng <[email protected]>
This introduces
KSYM_NAME_LEN_BUFFERin place of the previously hardcoded size of the input buffer.It will also make it easier to update the size in a single place in a later patch.
Signed-off-by Boqun Feng <[email protected]>Co-developed-by Miguel Ojeda <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>
Does someone want to commit to taking these "prereq" patches? These clean-ups are nice even without adding Rust.
-Kees
Quoted 42 lines
--- scripts/kallsyms.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)scripts/kallsyms.c +8
-2@@ -27,8 +27,14 @@#define ARRAY_SIZE (arr) (sizeof(arr) / sizeof(arr[0])) +#define _stringify_1 (x) #x +#define _stringify (x) _stringify_1(x) +#define KSYM_NAME_LEN 128 +/* A substantially bigger size than the current maximum. */ +#define KSYM_NAME_LEN_BUFFER 499 +struct sym_entry {unsigned long long addr;unsigned int len;@@ -198,13 +204,13 @@ static void check_symbol_range(const char *sym, unsigned long long addr,static struct sym_entry *read_symbol (FILE *in) { -char name[500 ], type; +char name[KSYM_NAME_LEN_BUFFER +1 ], type;unsigned long long addr;unsigned int len;struct sym_entry *sym;int rc; - rc =fscanf (in,"%llx %c %499s\n" , &addr, &type, name); + rc = fscanf(in, "%llx %c %" _stringify(KSYM_NAME_LEN_BUFFER) "s\n", &addr, &type, name);if (rc !=3 ) {if (rc !=EOF &&fgets (name,sizeof (name), in) ==NULL )fprintf (stderr,"Read error or end of file.\n" );Signature · 1 line
2.37.1
Signature · 1 line
Kees Cook
Re: [PATCH v9 02/27] kallsyms: avoid hardcoding buffer size
Quoted 9 lines · Kees Cook
Quoted 8 lines · Miguel Ojeda
From: Boqun Feng <[email protected]>
This introduces
KSYM_NAME_LEN_BUFFERin place of the previously hardcoded size of the input buffer.It will also make it easier to update the size in a single place in a later patch.
Quoted 8 lines
Quoted 3 lines
Signed-off-by Boqun Feng <[email protected]>Co-developed-by Miguel Ojeda <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>Reviewed-by Kees Cook <[email protected]>Does someone want to commit to taking these "prereq" patches? These clean-ups are nice even without adding Rust.
Qouting Message-ID: <CANiq72mXDne_WkUCo2oRe+sip7nQWESnouOJrcCYzyJMkG8F6A@mail.gmail.com>
https://lore.kernel.org/lkml/CANiq72mXDne_WkUCo2oRe+sip7nQWESnouOJrcCYzyJMkG8F6A@mail.gmail.com/
Miguel Ojeda, 2022-08-05:
| > And I think that this patch and all other "rust" kallsyms patches | > allready should have been accepted in the v3 or v5 series. | | Yeah, it could be a good idea to get the prerequisites in first. | Let's see if the patches get some Reviewed-bys
Now that there is a 'Reviewed-by: Kees Cook <[email protected]>'
Regards
Geert Stappers
In an attempt to help making Rust for Linux happen.
Signature · 1 line
Silence is hard to parse
Miguel Ojeda
maintainerPATCH v9 3/27 kallsyms: add static relationship between `KSYM_NAME_LEN{,_BUFFER}`
In reply to Miguel OjedaThis adds a static assert to ensure KSYM_NAME_LEN_BUFFER gets updated when KSYM_NAME_LEN changes.
The relationship used is one that keeps the new size (512+1) close to the original buffer size (500).
--- scripts/kallsyms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
scripts/kallsyms.c +5 -1
@@ -33,7 +33,11 @@#define KSYM_NAME_LEN 128/* A substantially bigger size than the current maximum. */ -#define KSYM_NAME_LEN_BUFFER 499 +#define KSYM_NAME_LEN_BUFFER 512 +_Static_assert ( +KSYM_NAME_LEN_BUFFER ==KSYM_NAME_LEN *4 , +"Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN" +);struct sym_entry {unsigned long long addr;
Signature · 1 line
2.37.1
Re: [PATCH v9 03/27] kallsyms: add static relationship between `KSYM_NAME_LEN{,_BUFFER}`
Quoted 18 lines · Miguel Ojeda
This adds a static assert to ensure
KSYM_NAME_LEN_BUFFERgets updated whenKSYM_NAME_LENchanges.The relationship used is one that keeps the new size (512+1) close to the original buffer size (500).
Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- scripts/kallsyms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)scripts/kallsyms.c +0
-0
scripts/kallsyms.c line 40
Quoted 9 lines
scripts/kallsyms.c +5
-1@@ -33,7 +33,11 @@#define KSYM_NAME_LEN 128/* A substantially bigger size than the current maximum. */ -#define KSYM_NAME_LEN_BUFFER 499 +#define KSYM_NAME_LEN_BUFFER 512 +_Static_assert ( +KSYM_NAME_LEN_BUFFER ==KSYM_NAME_LEN *4 , +"Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN" +);
Why not just make this define:
#define KSYM_NAME_LEN_BUFFER (KSYM_NAME_LEN * 4)
? If there's a good reason not it, please put it in the commit log.
-Kees
Signature · 1 line
Kees Cook
Re: [PATCH v9 03/27] kallsyms: add static relationship between `KSYM_NAME_LEN{,_BUFFER}`
Quoted 31 lines · Kees Cook
Quoted 18 lines · Miguel Ojeda
This adds a static assert to ensure
KSYM_NAME_LEN_BUFFERgets updated whenKSYM_NAME_LENchanges.The relationship used is one that keeps the new size (512+1) close to the original buffer size (500).
Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- scripts/kallsyms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)scripts/kallsyms.c +0
-0scripts/kallsyms.c line 40
Quoted 9 lines
scripts/kallsyms.c +5
-1@@ -33,7 +33,11 @@#define KSYM_NAME_LEN 128/* A substantially bigger size than the current maximum. */ -#define KSYM_NAME_LEN_BUFFER 499 +#define KSYM_NAME_LEN_BUFFER 512 +_Static_assert ( +KSYM_NAME_LEN_BUFFER ==KSYM_NAME_LEN *4 , +"Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN" +);Why not just make this define:
Quoted 4 lines
#define
KSYM_NAME_LEN_BUFFER(KSYM_NAME_LEN* 4)? If there's a good reason not it, please put it in the commit log.
Because KSYM_NAME_LEN_BUFFER is used as a string by stringify() in fscanf(), defining it as (KSYM_NAME_LEN * 4) will produce a string
"128 * 4"
after stringify() and that doesn't work with fscanf().
Miguel, maybe we can add something below in the commit log?
KSYM_NAME_LEN_BUFFER cannot be defined as an expression, because it gets stringified in the fscanf() format. Therefore a _Static_assert() is needed.
Thoughts?
Regards,
Boqun
Quoted 4 lines
-Kees
Signature · 1 line
Kees Cook
Re: [PATCH v9 03/27] kallsyms: add static relationship between `KSYM_NAME_LEN{,_BUFFER}`
Quoted 37 lines · Boqun Feng
Quoted 31 lines · Kees Cook
Quoted 18 lines · Miguel Ojeda
This adds a static assert to ensure
KSYM_NAME_LEN_BUFFERgets updated whenKSYM_NAME_LENchanges.The relationship used is one that keeps the new size (512+1) close to the original buffer size (500).
Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- scripts/kallsyms.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)scripts/kallsyms.c +0
-0scripts/kallsyms.c line 40
Quoted 9 lines
scripts/kallsyms.c +5
-1@@ -33,7 +33,11 @@#define KSYM_NAME_LEN 128/* A substantially bigger size than the current maximum. */ -#define KSYM_NAME_LEN_BUFFER 499 +#define KSYM_NAME_LEN_BUFFER 512 +_Static_assert ( +KSYM_NAME_LEN_BUFFER ==KSYM_NAME_LEN *4 , +"Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN" +);Why not just make this define:
Quoted 4 lines
#define
KSYM_NAME_LEN_BUFFER(KSYM_NAME_LEN* 4)? If there's a good reason not it, please put it in the commit log.
Quoted 6 lines
Because
KSYM_NAME_LEN_BUFFERis used as a string bystringify()infscanf(), defining it as (KSYM_NAME_LEN* 4) will produce a string"128 * 4"after
stringify()and that doesn't work withfscanf().
Ah yeah. Thanks!
Quoted 5 lines
Miguel, maybe we can add something below in the commit log?
KSYM_NAME_LEN_BUFFERcannot be defined as an expression, because it gets stringified in thefscanf()format. Therefore a_Static_assert()is needed.
Yeah, please add a source comment for that. :)
Signature · 1 line
Kees Cook
Re: [PATCH v9 03/27] kallsyms: add static relationship between `KSYM_NAME_LEN{,_BUFFER}`
Quoted 2 lines · Kees Cook
Yeah, please add a source comment for that. :)
I agree, I think this sort of explanation should be in the source vs. the commit message, since it explains something about the code, not the change itself.
Cheers,
Miguel
Miguel Ojeda
maintainerPATCH v9 4/27 kallsyms: support "big" kernel symbols
In reply to Miguel OjedaRust symbols can become quite long due to namespacing introduced by modules, types, traits, generics, etc.
Increasing to 255 is not enough in some cases, therefore introduce longer lengths to the symbol table.
In order to avoid increasing all lengths to 2 bytes (since most of them are small, including many Rust ones), use ULEB128 to keep smaller symbols in 1 byte, with the rest in 2 bytes.
--- kernel/kallsyms.c | 26 ++++++++++++++++++++++---- scripts/kallsyms.c | 29 ++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-)
kernel/kallsyms.c +22 -4
@@ -70,12 +70,20 @@ static unsigned int kallsyms_expand_symbol(unsigned int off, data = &kallsyms_names[off]; len = *data; data++; + off++; + +/* If MSB is 1, it is a "big" symbol, so needs an additional byte. */ +if ((len &0x80 ) !=0 ) { + len = (len &0x7F ) | (*data <<7 ); + data++; + off++; + }/* * Update the offset to return the offset for the next symbol on * the compressed stream. */ - off += len +1 ; + off += len;/* * For every byte on the compressed symbol data, copy the table
@@ -128,7 +136,7 @@ static char kallsyms_get_symbol_type(unsigned int off)static unsigned int get_symbol_offset (unsigned long pos) {const u8 *name; -int i; +int i, len;/* * Use the closest marker we have. We have markers every 256 positions,
@@ -142,8 +150,18 @@ static unsigned int get_symbol_offset(unsigned long pos)* so we just need to add the len to the current pointer for every * symbol we wish to skip. */ -for (i =0 ; i < (pos &0xFF ); i++) - name = name + (*name) +1 ; +for (i =0 ; i < (pos &0xFF ); i++) { + len = *name; + +/* +* If MSB is 1, it is a "big" symbol, so we need to look into +* the next byte (and skip it, too). +*/ +if ((len &0x80 ) !=0 ) + len = ((len &0x7F ) | (name[1 ] <<7 )) +1 ; + + name = name + len +1 ; + }return name - kallsyms_names; }
scripts/kallsyms.c +26 -3
@@ -481,12 +481,35 @@ static void write_src(void)if ((i &0xFF ) ==0 ) markers[i >>8 ] = off; -printf ("\t.byte 0x%02x" , table[i]->len); +/* There cannot be any symbol of length zero. */ +if (table[i]->len ==0 ) { +fprintf (stderr,"kallsyms failure: " +"unexpected zero symbol length\n" ); +exit (EXIT_FAILURE ); + } + +/* Only lengths that fit in up-to-two-byte ULEB128 are supported. */ +if (table[i]->len >0x3FFF ) { +fprintf (stderr,"kallsyms failure: " +"unexpected huge symbol length\n" ); +exit (EXIT_FAILURE ); + } + +/* Encode length with ULEB128. */ +if (table[i]->len <=0x7F ) { +/* Most symbols use a single byte for the length. */ +printf ("\t.byte 0x%02x" , table[i]->len); + off += table[i]->len +1 ; + }else { +/* "Big" symbols use two bytes. */ +printf ("\t.byte 0x%02x, 0x%02x" , + (table[i]->len &0x7F ) |0x80 , + (table[i]->len >>7 ) &0x7F ); + off += table[i]->len +2 ; + }for (k =0 ; k < table[i]->len; k++)printf (", 0x%02x" , table[i]->sym[k]);printf ("\n" ); - - off += table[i]->len +1 ; }printf ("\n" );
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 5/27 kallsyms: increase maximum kernel symbol length to 512
In reply to Miguel OjedaRust symbols can become quite long due to namespacing introduced by modules, types, traits, generics, etc. For instance, the following code:
pub mod my_module {
pub struct MyType;
pub struct MyGenericType<T>(T);
pub trait MyTrait {
fn my_method() -> u32;
}
impl MyTrait for MyGenericType<MyType> {
fn my_method() -> u32 {
42
}
}
}generates a symbol of length 96 when using the upcoming v0 mangling scheme:
_RNvXNtCshGpAVYOtgW1_7example9my_moduleINtB2_13MyGenericTypeNtB2_6MyTypeENtB2_7MyTrait9my_method
At the moment, Rust symbols may reach up to 300 in length. Setting 512 as the maximum seems like a reasonable choice to keep some headroom.
--- include/linux/kallsyms.h | 2 +- kernel/livepatch/core.c | 4 ++-- scripts/kallsyms.c | 4 ++-- tools/include/linux/kallsyms.h | 2 +- tools/lib/perf/include/perf/event.h | 2 +- tools/lib/symbol/kallsyms.h | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
include/linux/kallsyms.h +1 -1
@@ -15,7 +15,7 @@#include <asm/sections.h> -#define KSYM_NAME_LEN 128 +#define KSYM_NAME_LEN 512#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s %s]") + \ (KSYM_NAME_LEN - 1) + \ 2*(BITS_PER_LONG*3/10) + (MODULE_NAME_LEN - 1) + \
kernel/livepatch/core.c +2 -2
@@ -213,7 +213,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,* we use the smallest/strictest upper bound possible (56, based on * the current definition of MODULE_NAME_LEN) to prevent overflows. */ -BUILD_BUG_ON (MODULE_NAME_LEN <56 ||KSYM_NAME_LEN !=128 ); +BUILD_BUG_ON (MODULE_NAME_LEN <56 ||KSYM_NAME_LEN !=512 ); relas = (Elf_Rela *) relasec->sh_addr;/* For each rela in this klp relocation section */
@@ -227,7 +227,7 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab,/* Format: .klp.sym.sym_objname.sym_name,sympos */ cnt =sscanf (strtab + sym->st_name, -".klp.sym.%55[^.].%127[^,],%lu" , +".klp.sym.%55[^.].%511[^,],%lu" , sym_objname, sym_name, &sympos);if (cnt !=3 ) { pr_err("symbol %s has an incorrectly formatted name\n",
scripts/kallsyms.c +2 -2
@@ -30,10 +30,10 @@#define _stringify_1 (x) #x#define _stringify (x) _stringify_1(x) -#define KSYM_NAME_LEN 128 +#define KSYM_NAME_LEN 512/* A substantially bigger size than the current maximum. */ -#define KSYM_NAME_LEN_BUFFER 512 +#define KSYM_NAME_LEN_BUFFER 2048_Static_assert (KSYM_NAME_LEN_BUFFER ==KSYM_NAME_LEN *4 , "Please keep KSYM_NAME_LEN_BUFFER in sync with KSYM_NAME_LEN"
tools/include/linux/kallsyms.h +1 -1
@@ -6,7 +6,7 @@#include <stdio.h> #include <unistd.h> -#define KSYM_NAME_LEN 128 +#define KSYM_NAME_LEN 512struct module ;
tools/lib/perf/include/perf/event.h +1 -1
@@ -95,7 +95,7 @@ struct perf_record_throttle { };#ifndef KSYM_NAME_LEN -#define KSYM_NAME_LEN 256 +#define KSYM_NAME_LEN 512#endif struct perf_record_ksymbol {
tools/lib/symbol/kallsyms.h +1 -1
@@ -7,7 +7,7 @@#include <linux/types.h> #ifndef KSYM_NAME_LEN -#define KSYM_NAME_LEN 256 +#define KSYM_NAME_LEN 512#endif static inline u8 kallsyms2elf_binding (char type)
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 6/27 rust: add C helpers
In reply to Miguel OjedaThis source file contains forwarders to C macros and inlined functions.
--- rust/helpers.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 rust/helpers.c
rust/helpers.c new file +51 -0
@@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Non-trivial C macros cannot be used in Rust. Similarly, inlined C functions +* cannot be called either. This file explicitly creates functions ("helpers") +* that wrap those so that they can be called from Rust. +* +* Even though Rust kernel modules should never use directly the bindings, some +* of these helpers need to be exported because Rust generics and inlined +* functions may not get their code generated in the crate where they are +* defined. Other helpers, called from non-inline functions, may not be +* exported, in principle. However, in general, the Rust compiler does not +* guarantee codegen will be performed for a non-inline function either. +* Therefore, this file exports all the helpers. In the future, this may be +* revisited to reduce the number of exports after the compiler is informed +* about the places codegen is required. +* +* All symbols are exported as GPL-only to guarantee no GPL-only feature is +* accidentally exposed. +*/ + +#include <linux/bug.h> +#include <linux/build_bug.h> + +__noreturn void rust_helper_BUG (void ) +{ +BUG (); +} +EXPORT_SYMBOL_GPL (rust_helper_BUG); + +/* +* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type +* as the Rust `usize` type, so we can use it in contexts where Rust +* expects a `usize` like slice (array) indices. `usize` is defined to be +* the same as C's `uintptr_t` type (can hold any pointer) but not +* necessarily the same as `size_t` (can hold the size of any single +* object). Most modern platforms use the same concrete integer type for +* both of them, but in case we find ourselves on a platform where +* that's not true, fail early instead of risking ABI or +* integer-overflow issues. +* +* If your platform fails this assertion, it means that you are in +* danger of integer-overflow bugs (even if you attempt to remove +* `--size_t-is-usize`). It may be easiest to change the kernel ABI on +* your platform such that `size_t` matches `uintptr_t` (i.e., to increase +* `size_t`, because `uintptr_t` has to be at least as big as `size_t`). +*/ +static_assert ( +sizeof (size_t ) ==sizeof (uintptr_t ) && + __alignof__(size_t ) == __alignof__(uintptr_t ), +"Rust code expects C `size_t` to match Rust `usize`" +);
Signature · 1 line
2.37.1
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 2 lines · Miguel Ojeda
This source file contains forwarders to C macros and inlined functions.
Perhaps:
"Introduce the source file that will contain forwarders to common C macros as inlined Rust functions. Initially this only contains type size asserts, but will gain more helpers in subsequent patches."
Quoted 47 lines
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Geoffrey Thomas <[email protected]>Signed-off-by Geoffrey Thomas <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Sven Van Asbroeck <[email protected]>Signed-off-by Sven Van Asbroeck <[email protected]>Co-developed-by Gary Guo <[email protected]>Signed-off-by Gary Guo <[email protected]>Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Co-developed-by Maciej Falkowski <[email protected]>Signed-off-by Maciej Falkowski <[email protected]>Co-developed-by Wei Liu <[email protected]>Signed-off-by Wei Liu <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- rust/helpers.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 rust/helpers.crust/helpers.c new file +18
-0@@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Non-trivial C macros cannot be used in Rust. Similarly, inlined C functions +* cannot be called either. This file explicitly creates functions ("helpers") +* that wrap those so that they can be called from Rust. +* +* Even though Rust kernel modules should never use directly the bindings, some +* of these helpers need to be exported because Rust generics and inlined +* functions may not get their code generated in the crate where they are +* defined. Other helpers, called from non-inline functions, may not be +* exported, in principle. However, in general, the Rust compiler does not +* guarantee codegen will be performed for a non-inline function either. +* Therefore, this file exports all the helpers. In the future, this may be +* revisited to reduce the number of exports after the compiler is informed +* about the places codegen is required. +* +* All symbols are exported as GPL-only to guarantee no GPL-only feature is +* accidentally exposed.
rust/helpers.c line 28
Quoted 10 lines
rust/helpers.c new file +10
-0+*/ + +#include <linux/bug.h> +#include <linux/build_bug.h> + +__noreturn void rust_helper_BUG (void ) +{ +BUG (); +} +EXPORT_SYMBOL_GPL (rust_helper_BUG);
Given the distaste for ever using BUG()[1], why does this helper exist?
Quoted 13 lines
rust/helpers.c new file +13
-0+ +/* +* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type +* as the Rust `usize` type, so we can use it in contexts where Rust +* expects a `usize` like slice (array) indices. `usize` is defined to be +* the same as C's `uintptr_t` type (can hold any pointer) but not +* necessarily the same as `size_t` (can hold the size of any single +* object). Most modern platforms use the same concrete integer type for +* both of them, but in case we find ourselves on a platform where +* that's not true, fail early instead of risking ABI or +* integer-overflow issues. +* +* If your platform fails this assertion, it means that you are in
rust/helpers.c line 51
Quoted 10 lines
rust/helpers.c new file +10
-0+* danger of integer-overflow bugs (even if you attempt to remove +* `--size_t-is-usize`). It may be easiest to change the kernel ABI on +* your platform such that `size_t` matches `uintptr_t` (i.e., to increase +* `size_t`, because `uintptr_t` has to be at least as big as `size_t`). +*/ +static_assert ( +sizeof (size_t ) ==sizeof (uintptr_t ) && + __alignof__(size_t ) == __alignof__(uintptr_t ), +"Rust code expects C `size_t` to match Rust `usize`" +);
-Kees
[1] https://docs.kernel.org/process/deprecated.html#bug-and-bug-on
Signature · 1 line
Kees Cook
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 4 lines · Kees Cook
"Introduce the source file that will contain forwarders to common C macros as inlined Rust functions. Initially this only contains type size asserts, but will gain more helpers in subsequent patches."
Yeah, I will reword it, it doesn't make as much sense now that it is trimmed.
Quoted 1 line
Given the distaste for ever using BUG()[1], why does this helper exist?
We use it exclusively for the Rust panic handler, which does not return (we use fallible operations as much as possible, of course, but we need to provide a panic handler nevertheless).
Killing the entire machine is definitely too aggressive for some setups/situations, so at some point last year we discussed potential alternatives (e.g. make_task_dead() or similar) with, if I recall correctly, Greg. Maybe we want to make it configurable too. We are open to suggestions!
Cheers,
Miguel
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 6 lines · Miguel Ojeda
Quoted 1 line · Kees Cook
Given the distaste for ever using BUG()[1], why does this helper exist?
We use it exclusively for the Rust panic handler, which does not return (we use fallible operations as much as possible, of course, but we need to provide a panic handler nevertheless).
Gotcha -- it's for the implicit situations (e.g. -C overflow-checks=on), nothing is expected to explicitly call the Rust panic handler?
Quoted 5 lines
Killing the entire machine is definitely too aggressive for some setups/situations, so at some point last year we discussed potential alternatives (e.g.
make_task_dead()or similar) with, if I recall correctly, Greg. Maybe we want to make it configurable too. We are open to suggestions!
I suffer the same problems trying to fix C and the old "can never fail" interfaces. Mainly we've just been systematically replacing such APIs with APIs that return error codes, allowing the error to bubble back up. (Which I know is exactly what you've already done with the allocator, etc. Yay!)
-Kees
Signature · 1 line
Kees Cook
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 2 lines · Kees Cook
Gotcha -- it's for the implicit situations (e.g. -C overflow-checks=on),
Yeah, exactly.
Quoted 1 line
nothing is expected to explicitly call the Rust panic handler?
If by explicitly you mean calling panic!(), then in the kernel crate in the v9 patches there is none.
Though we may want to call it in the future (we have 4 instances in the full code not submitted here, e.g. for mismatching an independent lock guard with its owner). They can be avoided depending on how we want the design to be and, I guess, what the "Rust panic" policy will finally be (i.e. BUG() or something softer).
Outside the kernel crate, there are also instances in proc macros and Rust hostprogs/scripts (compilation-time in the host), in the alloc crate (compiled-out) and in the compiler_builtins crate (for e.g. u128 support that eventually we would like to not see compiled-in).
Cheers,
Miguel
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 10 lines · Miguel Ojeda
Quoted 2 lines · Kees Cook
Gotcha -- it's for the implicit situations (e.g. -C overflow-checks=on),
Yeah, exactly.
Quoted 1 line
nothing is expected to explicitly call the Rust panic handler?
If by explicitly you mean calling
panic!(), then in thekernelcrate in the v9 patches there is none.
Perfect. It may be worth stating this explicitly with the helper. i.e. "This is for handling any panic!() calls in core Rust, but should not ever be used in the 'kernel' create; failures should be handled."
Quoted 11 lines
Though we may want to call it in the future (we have 4 instances in the full code not submitted here, e.g. for mismatching an independent lock guard with its owner). They can be avoided depending on how we want the design to be and, I guess, what the "Rust panic" policy will finally be (i.e.
BUG()or something softer).Outside the
kernelcrate, there are also instances in proc macros and Rust hostprogs/scripts (compilation-time in the host), in thealloccrate (compiled-out) and in thecompiler_builtinscrate (for e.g.u128support that eventually we would like to not see compiled-in).
Sounds good!
-Kees
Signature · 1 line
Kees Cook
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 4 lines · Kees Cook
Perfect. It may be worth stating this explicitly with the helper. i.e. "This is for handling any panic!() calls in core Rust, but should not ever be used in the 'kernel' create; failures should be handled."
I am not sure we should say "ever", because there are sometimes situations where we statically know a situation is impossible. Of course, "impossible" in practice is possible -- even if it is due to a single-event upset.
For the "statically impossible" cases, we could simply trigger UB instead of panicking. However, while developing and debugging one would like to detect bugs as soon as possible. Moreover, in production, people may have use cases where killing the world is better as soon as anything "funny" is detected, no matter what.
So we could make it configurable, so that "Rust statically impossible panics" can be defined as UB, make_task_dead() or a full BUG().
By the way, I should have mentioned the unwrap()s too, since they are pretty much explicit panics. We don't have any in v9 either, but we do have a couple dozens in the full code (in the 97% not submitted) in non-test or examples code. Many are of the "statically impossible" kind, but any that is not merits some discussion, which we can do as we upstream the different pieces.
Cheers,
Miguel
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 11 lines · Miguel Ojeda
Quoted 4 lines · Kees Cook
Perfect. It may be worth stating this explicitly with the helper. i.e. "This is for handling any panic!() calls in core Rust, but should not ever be used in the 'kernel' create; failures should be handled."
I am not sure we should say "ever", because there are sometimes situations where we statically know a situation is impossible. Of course, "impossible" in practice is possible -- even if it is due to a single-event upset.
Quoted 5 lines
For the "statically impossible" cases, we could simply trigger UB instead of panicking. However, while developing and debugging one would like to detect bugs as soon as possible. Moreover, in production, people may have use cases where killing the world is better as soon as anything "funny" is detected, no matter what.
Please, no UB. I will take a panic over UB any day. It'd be best to handle things with some error path, but those are the rare exception.
Quoted 2 lines
So we could make it configurable, so that "Rust statically impossible panics" can be defined as UB,
make_task_dead()or a fullBUG().
C is riddled with UB and it's just terrible. Let's make sure we don't continue that mistake. :)
Quoted 6 lines
By the way, I should have mentioned the
unwrap()stoo, since they are pretty much explicit panics. We don't have any in v9 either, but we do have a couple dozens in the full code (in the 97% not submitted) in non-test or examples code. Many are of the "statically impossible" kind, but any that is not merits some discussion, which we can do as we upstream the different pieces.
The simple answer is that if an "impossible" situation can be recovered from, it should error instead of panic. As long as that's the explicit design goal, I think we're good. Yes there will be cases where it is really and truly unrecoverable, but those will be rare and can be well documented.
Signature · 1 line
Kees Cook
Re: [PATCH v9 06/27] rust: add C helpers
Quoted 6 lines · Kees Cook
Please, no UB. I will take a panic over UB any day. It'd be best to handle things with some error path, but those are the rare exception.
C is riddled with UB and it's just terrible. Let's make sure we don't continue that mistake. :)
I definitely agree on avoiding UB :)
Quoted 5 lines
The simple answer is that if an "impossible" situation can be recovered from, it should error instead of panic. As long as that's the explicit design goal, I think we're good. Yes there will be cases where it is really and truly unrecoverable, but those will be rare and can be well documented.
Yeah, that is the goal and we always take that into account, but there are always tricky cases which is best to consider case-by-case.
Cheers,
Miguel
Miguel Ojeda
maintainerPATCH v9 7/27 rust: import upstream `alloc` crate
In reply to Miguel OjedaThis is a subset of the Rust standard library alloc crate, version 1.62.0, licensed under "Apache-2.0 OR MIT", from:
https://github.com/rust-lang/rust/tree/1.62.0/library/alloc/src
The files are copied as-is, with no modifications whatsoever (not even adding the SPDX identifiers).
For copyright details, please see:
https://github.com/rust-lang/rust/blob/1.62.0/COPYRIGHT
The next patch modifies these files as needed for use within the kernel. This patch split allows reviewers to double-check the import and to clearly see the differences introduced.
Vendoring alloc, at least for the moment, allows us to have fallible allocations support (i.e. the try_* versions of methods which return a Result instead of panicking) early on. It also gives a bit more freedom to experiment with new interfaces and to iterate quickly.
Eventually, the goal is to have everything the kernel needs in upstream alloc and drop it from the kernel tree.
For a summary of work on alloc happening upstream, please see:
https://github.com/Rust-for-Linux/linux/issues/408
--- rust/alloc/alloc.rs | 438 +++++ rust/alloc/borrow.rs | 496 +++++ rust/alloc/boxed.rs | 2024 +++++++++++++++++++++ rust/alloc/collections/mod.rs | 154 ++ rust/alloc/lib.rs | 236 +++ rust/alloc/raw_vec.rs | 518 ++++++ rust/alloc/slice.rs | 1202 ++++++++++++ rust/alloc/vec/drain.rs | 184 ++ rust/alloc/vec/drain_filter.rs | 143 ++ rust/alloc/vec/into_iter.rs | 362 ++++ rust/alloc/vec/is_zero.rs | 118 ++ rust/alloc/vec/mod.rs | 3115 ++++++++++++++++++++++++++++++++ rust/alloc/vec/partial_eq.rs | 47 + 13 files changed, 9037 insertions(+) create mode 100644 rust/alloc/alloc.rs create mode 100644 rust/alloc/borrow.rs create mode 100644 rust/alloc/boxed.rs create mode 100644 rust/alloc/collections/mod.rs create mode 100644 rust/alloc/lib.rs create mode 100644 rust/alloc/raw_vec.rs create mode 100644 rust/alloc/slice.rs create mode 100644 rust/alloc/vec/drain.rs create mode 100644 rust/alloc/vec/drain_filter.rs create mode 100644 rust/alloc/vec/into_iter.rs create mode 100644 rust/alloc/vec/is_zero.rs create mode 100644 rust/alloc/vec/mod.rs create mode 100644 rust/alloc/vec/partial_eq.rs
rust/alloc/alloc.rs new file +438 -0
@@ -0,0 +1,438 @@ +//! Memory allocation APIs + +#![stable(feature = "alloc_module" , since = "1.28.0" )] + +#[cfg(not(test))] +use core::intrinsics; +use core::intrinsics::{min_align_of_val, size_of_val}; + +use core::ptr::Unique ; +#[cfg(not(test))] +use core::ptr::{self,NonNull }; + +#[stable(feature = "alloc_module" , since = "1.28.0" )] +#[doc(inline)] +pub use core::alloc::*; + +use core::marker::Destruct ; + +#[cfg(test)] +mod tests; + +extern "Rust" { +// These are the magic symbols to call the global allocator. rustc generates +// them to call `__rg_alloc` etc. if there is a `#[global_allocator]` attribute +// (the code expanding that attribute macro generates those functions), or to call +// the default implementations in libstd (`__rdl_alloc` etc. in `library/std/src/alloc.rs`) +// otherwise. +// The rustc fork of LLVM also special-cases these function names to be able to optimize them +// like `malloc`, `realloc`, and `free`, respectively. +#[rustc_allocator] +#[rustc_allocator_nounwind] +fn __rust_alloc (size:usize , align:usize ) -> *mut u8 ; +#[rustc_allocator_nounwind] +fn __rust_dealloc (ptr: *mut u8 , size:usize , align:usize ); +#[rustc_allocator_nounwind] +fn __rust_realloc (ptr: *mut u8 , old_size:usize , align:usize , new_size:usize ) -> *mut u8 ; +#[rustc_allocator_nounwind] +fn __rust_alloc_zeroed (size:usize , align:usize ) -> *mut u8 ; +} + +/// The global memory allocator. +/// +/// This type implements the [`Allocator`] trait by forwarding calls +/// to the allocator registered with the `#[global_allocator]` attribute +/// if there is one, or the `std` crate’s default. +/// +/// Note: while this type is unstable, the functionality it provides can be +/// accessed through the [free functions in `alloc`](self#functions). +#[unstable(feature = "allocator_api" , issue = "32838" )] +#[derive( Copy , Clone , Default , Debug )] +#[cfg(not(test))] +pub struct Global ; + +#[cfg(test)] +pub use std::alloc::Global ; + +/// Allocate memory with the global allocator. +/// +/// This function forwards calls to the [`GlobalAlloc::alloc`] method +/// of the allocator registered with the `#[global_allocator]` attribute +/// if there is one, or the `std` crate’s default. +/// +/// This function is expected to be deprecated in favor of the `alloc` method +/// of the [`Global`] type when it and the [`Allocator`] trait become stable. +/// +/// # Safety +/// +/// See [`GlobalAlloc::alloc`]. +/// +/// # Examples +/// +/// ``` +/// use std::alloc::{alloc, dealloc, Layout}; +/// +/// unsafe { +/// let layout = Layout::new::<u16>(); +/// let ptr = alloc(layout); +/// +/// *(ptr as *mut u16) = 42; +/// assert_eq!(*(ptr as *mut u16), 42); +/// +/// dealloc(ptr, layout); +/// } +/// ``` +#[stable(feature = "global_alloc" , since = "1.28.0" )] +#[must_use = "losing the pointer will leak memory" ] +#[inline] +pub unsafe fn alloc (layout:Layout ) -> *mut u8 { +unsafe {__rust_alloc (layout.size (), layout.align ()) } +} + +/// Deallocate memory with the global allocator. +/// +/// This function forwards calls to the [`GlobalAlloc::dealloc`] method +/// of the allocator registered with the `#[global_allocator]` attribute +/// if there is one, or the `std` crate’s default. +/// +/// This function is expected to be deprecated in favor of the `dealloc` method +/// of the [`Global`] type when it and the [`Allocator`] trait become stable. +/// +/// # Safety +/// +/// See [`GlobalAlloc::dealloc`]. +#[stable(feature = "global_alloc" , since = "1.28.0" )] +#[inline] +pub unsafe fn dealloc (ptr: *mut u8 , layout:Layout ) { +unsafe {__rust_dealloc (ptr, layout.size (), layout.align ()) } +} + +/// Reallocate memory with the global allocator. +/// +/// This function forwards calls to the [`GlobalAlloc::realloc`] method +/// of the allocator registered with the `#[global_allocator]` attribute +/// if there is one, or the `std` crate’s default. +/// +/// This function is expected to be deprecated in favor of the `realloc` method +/// of the [`Global`] type when it and the [`Allocator`] trait become stable. +/// +/// # Safety +/// +/// See [`GlobalAlloc::realloc`]. +#[stable(feature = "global_alloc" , since = "1.28.0" )] +#[must_use = "losing the pointer will leak memory" ] +#[inline] +pub unsafe fn realloc (ptr: *mut u8 , layout:Layout , new_size:usize ) -> *mut u8 { +unsafe {__rust_realloc (ptr, layout.size (), layout.align (), new_size) } +} + +/// Allocate zero-initialized memory with the global allocator. +/// +/// This function forwards calls to the [`GlobalAlloc::alloc_zeroed`] method +/// of the allocator registered with the `#[global_allocator]` attribute +/// if there is one, or the `std` crate’s default. +/// +/// This function is expected to be deprecated in favor of the `alloc_zeroed` method +/// of the [`Global`] type when it and the [`Allocator`] trait become stable. +/// +/// # Safety +/// +/// See [`GlobalAlloc::alloc_zeroed`]. +/// +/// # Examples +/// +/// ``` +/// use std::alloc::{alloc_zeroed, dealloc, Layout}; +/// +/// unsafe { +/// let layout = Layout::new::<u16>(); +/// let ptr = alloc_zeroed(layout); +/// +/// assert_eq!(*(ptr as *mut u16), 0); +/// +/// dealloc(ptr, layout); +/// } +/// ``` +#[stable(feature = "global_alloc" , since = "1.28.0" )] +#[must_use = "losing the pointer will leak memory" ] +#[inline] +pub unsafe fn alloc_zeroed (layout:Layout ) -> *mut u8 { +unsafe {__rust_alloc_zeroed (layout.size (), layout.align ()) } +} + +#[cfg(not(test))] +impl Global { +#[inline] +fn alloc_impl (&self, layout:Layout , zeroed:bool ) ->Result <NonNull <[u8 ]>,AllocError > { +match layout.size () { +0 =>Ok (NonNull ::slice_from_raw_parts (layout.dangling (),0 )), +// SAFETY: `layout` is non-zero in size, + size =>unsafe { +let raw_ptr =if zeroed {alloc_zeroed (layout) }else {alloc (layout) }; +let ptr =NonNull ::new (raw_ptr).ok_or (AllocError )?; +Ok (NonNull ::slice_from_raw_parts (ptr, size)) + }, + } + } + +// SAFETY: Same as `Allocator::grow` +#[inline] +unsafe fn grow_impl ( + &self, + ptr:NonNull <u8 >, + old_layout:Layout , + new_layout:Layout , + zeroed:bool , + ) ->Result <NonNull <[u8 ]>,AllocError > { +debug_assert! ( + new_layout.size () >= old_layout.size (), +"`new_layout.size()` must be greater than or equal to `old_layout.size()`" + ); + +match old_layout.size () { +0 => self.alloc_impl (new_layout, zeroed), + +// SAFETY: `new_size` is non-zero as `old_size` is greater than or equal to `new_size` +// as required by safety conditions. Other conditions must be upheld by the caller + old_sizeif old_layout.align () == new_layout.align () =>unsafe { +let new_size = new_layout.size (); + +// `realloc` probably checks for `new_size >= old_layout.size()` or something similar. + intrinsics::assume (new_size >= old_layout.size ()); + +let raw_ptr =realloc (ptr.as_ptr (), old_layout, new_size); +let ptr =NonNull ::new (raw_ptr).ok_or (AllocError )?; +if zeroed { + raw_ptr.add (old_size).write_bytes (0 , new_size - old_size); + } +Ok (NonNull ::slice_from_raw_parts (ptr, new_size)) + }, + +// SAFETY: because `new_layout.size()` must be greater than or equal to `old_size`, +// both the old and new memory allocation are valid for reads and writes for `old_size` +// bytes. Also, because the old allocation wasn't yet deallocated, it cannot overlap +// `new_ptr`. Thus, the call to `copy_nonoverlapping` is safe. The safety contract +// for `dealloc` must be upheld by the caller. + old_size =>unsafe { +let new_ptr = self.alloc_impl (new_layout, zeroed)?; + ptr::copy_nonoverlapping (ptr.as_ptr (), new_ptr.as_mut_ptr (), old_size); + self.deallocate (ptr, old_layout); +Ok (new_ptr) + }, + } + } +} + +#[unstable(feature = "allocator_api" , issue = "32838" )] +#[cfg(not(test))] +unsafe impl Allocator for Global { +#[inline] +fn allocate (&self, layout:Layout ) ->Result <NonNull <[u8 ]>,AllocError > { + self.alloc_impl (layout,false ) + } + +#[inline] +fn allocate_zeroed (&self, layout:Layout ) ->Result <NonNull <[u8 ]>,AllocError > { + self.alloc_impl (layout,true ) + } + +#[inline] +unsafe fn deallocate (&self, ptr:NonNull <u8 >, layout:Layout ) { +if layout.size () !=0 { +// SAFETY: `layout` is non-zero in size, +// other conditions must be upheld by the caller +unsafe {dealloc (ptr.as_ptr (), layout) } + } + } + +#[inline] +unsafe fn grow ( + &self, + ptr:NonNull <u8 >, + old_layout:Layout , + new_layout:Layout , + ) ->Result <NonNull <[u8 ]>,AllocError > { +// SAFETY: all conditions must be upheld by the caller +unsafe { self.grow_impl (ptr, old_layout, new_layout,false ) } + } + +#[inline] +unsafe fn grow_zeroed ( + &self, + ptr:NonNull <u8 >, + old_layout:Layout , + new_layout:Layout , + ) ->Result <NonNull <[u8 ]>,AllocError > { +// SAFETY: all conditions must be upheld by the caller +unsafe { self.grow_impl (ptr, old_layout, new_layout,true ) } + } + +#[inline] +unsafe fn shrink ( + &self, + ptr:NonNull <u8 >, + old_layout:Layout , + new_layout:Layout , + ) ->Result <NonNull <[u8 ]>,AllocError > { +debug_assert! ( + new_layout.size () <= old_layout.size (), +"`new_layout.size()` must be smaller than or equal to `old_layout.size()`" + ); + +match new_layout.size () { +// SAFETY: conditions must be upheld by the caller +0 =>unsafe { + self.deallocate (ptr, old_layout); +Ok (NonNull ::slice_from_raw_parts (new_layout.dangling (),0 )) + }, + +// SAFETY: `new_size` is non-zero. Other conditions must be upheld by the caller + new_sizeif old_layout.align () == new_layout.align () =>unsafe { +// `realloc` probably checks for `new_size <= old_layout.size()` or something similar. + intrinsics::assume (new_size <= old_layout.size ()); + +let raw_ptr =realloc (ptr.as_ptr (), old_layout, new_size); +let ptr =NonNull ::new (raw_ptr).ok_or (AllocError )?; +Ok (NonNull ::slice_from_raw_parts (ptr, new_size)) + }, + +// SAFETY: because `new_size` must be smaller than or equal to `old_layout.size()`, +// both the old and new memory allocation are valid for reads and writes for `new_size` +// bytes. Also, because the old allocation wasn't yet deallocated, it cannot overlap +// `new_ptr`. Thus, the call to `copy_nonoverlapping` is safe. The safety contract +// for `dealloc` must be upheld by the caller. + new_size =>unsafe { +let new_ptr = self.allocate (new_layout)?; + ptr::copy_nonoverlapping (ptr.as_ptr (), new_ptr.as_mut_ptr (), new_size); + self.deallocate (ptr, old_layout); +Ok (new_ptr) + }, + } + } +} + +/// The allocator for unique pointers. +#[cfg(all(not(no_global_oom_handling), not(test)))] +#[lang = "exchange_malloc" ] +#[inline] +unsafe fn exchange_malloc (size:usize , align:usize ) -> *mut u8 { +let layout =unsafe {Layout ::from_size_align_unchecked (size, align) }; +match Global .allocate (layout) { +Ok (ptr) => ptr.as_mut_ptr (), +Err (_) =>handle_alloc_error (layout), + } +} + +#[cfg_attr(not(test), lang = "box_free" )] +#[inline] +#[rustc_const_unstable(feature = "const_box" , issue = "92521" )] +// This signature has to be the same as `Box`, otherwise an ICE will happen. +// When an additional parameter to `Box` is added (like `A: Allocator`), this has to be added here as +// well. +// For example if `Box` is changed to `struct Box<T: ?Sized, A: Allocator>(Unique<T>, A)`, +// this function has to be changed to `fn box_free<T: ?Sized, A: Allocator>(Unique<T>, A)` as well. +pub(crate) const unsafe fn box_free<T: ?Sized, A: ~const Allocator + ~const Destruct>( + ptr:Unique <T >, + alloc:A , +) { +unsafe { +let size =size_of_val (ptr.as_ref ()); +let align =min_align_of_val (ptr.as_ref ()); +let layout =Layout ::from_size_align_unchecked (size, align); + alloc.deallocate (From ::from (ptr.cast ()), layout) + } +} + +// # Allocation error handler + +#[cfg(not(no_global_oom_handling))] +extern "Rust" { +// This is the magic symbol to call the global alloc error handler. rustc generates +// it to call `__rg_oom` if there is a `#[alloc_error_handler]`, or to call the +// default implementations below (`__rdl_oom`) otherwise. +fn __rust_alloc_error_handler (size:usize , align:usize ) -> !; +} + +/// Abort on memory allocation error or failure. +/// +/// Callers of memory allocation APIs wishing to abort computation +/// in response to an allocation error are encouraged to call this function, +/// rather than directly invoking `panic!` or similar. +/// +/// The default behavior of this function is to print a message to standard error +/// and abort the process. +/// It can be replaced with [`set_alloc_error_hook`] and [`take_alloc_error_hook`]. +/// +/// [`set_alloc_error_hook`]: ../../std/alloc/fn.set_alloc_error_hook.html +/// [`take_alloc_error_hook`]: ../../std/alloc/fn.take_alloc_error_hook.html +#[stable(feature = "global_alloc" , since = "1.28.0" )] +#[rustc_const_unstable(feature = "const_alloc_error" , issue = "92523" )] +#[cfg(all(not(no_global_oom_handling), not(test)))] +#[cold] +pub const fn handle_alloc_error (layout:Layout ) -> ! { +const fn ct_error (_:Layout ) -> ! { +panic! ("allocation failed" ); + } + +fn rt_error (layout:Layout ) -> ! { +unsafe { +__rust_alloc_error_handler (layout.size (), layout.align ()); + } + } + +unsafe { core::intrinsics::const_eval_select ((layout,), ct_error, rt_error) } +} + +// For alloc test `std::alloc::handle_alloc_error` can be used directly. +#[cfg(all(not(no_global_oom_handling), test))] +pub use std::alloc::handle_alloc_error; + +#[cfg(all(not(no_global_oom_handling), not(test)))] +#[doc(hidden)] +#[allow(unused_attributes)] +#[unstable(feature = "alloc_internals" , issue = "none" )] +pub mod __alloc_error_handler { +use crate ::alloc::Layout ; + +// called via generated `__rust_alloc_error_handler` + +// if there is no `#[alloc_error_handler]` +#[rustc_std_internal_symbol] +pub unsafe extern "C-unwind" fn __rdl_oom (size:usize , _align:usize ) -> ! { +panic! ("memory allocation of {size} bytes failed" ) + } + +// if there is an `#[alloc_error_handler]` +#[rustc_std_internal_symbol] +pub unsafe extern "C-unwind" fn __rg_oom (size:usize , align:usize ) -> ! { +let layout =unsafe {Layout ::from_size_align_unchecked (size, align) }; +extern "Rust" { +#[lang = "oom" ] +fn oom_impl (layout:Layout ) -> !; + } +unsafe {oom_impl (layout) } + } +} + +/// Specialize clones into pre-allocated, uninitialized memory. +/// Used by `Box::clone` and `Rc`/`Arc::make_mut`. +pub (crate )trait WriteCloneIntoRaw :Sized { +unsafe fn write_clone_into_raw (&self, target: *mut Self ); +} + +impl <T :Clone >WriteCloneIntoRaw for T { +#[inline] +default unsafe fn write_clone_into_raw (&self, target: *mut Self ) { +// Having allocated *first* may allow the optimizer to create +// the cloned value in-place, skipping the local and move. +unsafe { target.write (self.clone ()) }; + } +} + +impl <T :Copy >WriteCloneIntoRaw for T { +#[inline] +unsafe fn write_clone_into_raw (&self, target: *mut Self ) { +// We can always copy in-place, without ever involving a local value. +unsafe { target.copy_from_nonoverlapping (self,1 ) }; + } +}
rust/alloc/borrow.rs new file +496 -0
496 lines not on this page · raw message
rust/alloc/boxed.rs new file +2024 -0
2,024 lines not on this page · raw message
rust/alloc/collections/mod.rs new file +154 -0
42 lines not on this page · raw message
rust/alloc/lib.rs new file +236 -0
236 lines not on this page · raw message
rust/alloc/raw_vec.rs new file +518 -0
518 lines not on this page · raw message
rust/alloc/slice.rs new file +1202 -0
1,202 lines not on this page · raw message
rust/alloc/vec/drain.rs new file +184 -0
184 lines not on this page · raw message
rust/alloc/vec/drain_filter.rs new file +143 -0
143 lines not on this page · raw message
rust/alloc/vec/into_iter.rs new file +362 -0
362 lines not on this page · raw message
rust/alloc/vec/is_zero.rs new file +118 -0
118 lines not on this page · raw message
rust/alloc/vec/mod.rs new file +3115 -0
3,115 lines not on this page · raw message
rust/alloc/vec/partial_eq.rs new file +47 -0
47 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.37.1
Part of the diff is not shown here. View the raw message.
Re: [PATCH v9 07/27] rust: import upstream `alloc` crate
Quoted 8 lines · Miguel Ojeda
This is a subset of the Rust standard library
alloccrate, version 1.62.0, licensed under "Apache-2.0 OR MIT", from:https://github.com/rust-lang/rust/tree/1.62.0/library/alloc/srcThe files are copied as-is, with no modifications whatsoever (not even adding the SPDX identifiers).
Quoted 7 lines
For copyright details, please see:
https://github.com/rust-lang/rust/blob/1.62.0/COPYRIGHTThe next patch modifies these files as needed for use within the kernel. This patch split allows reviewers to double-check the import and to clearly see the differences introduced.
Can you include an easy script (in the commit log) for this kind of verification?
For this, I have done:
$ git am 0001-rust-import-upstream-alloc-crate.patch $ for i in $(diffstat -lp3 0001-rust-import-upstream-alloc-crate.patch); do wget --quiet -O check.rs \ https://github.com/rust-lang/rust/raw/1.62.0/library/alloc/src/$i diff -up rust/alloc/$i check.rs && echo $i: ok rm -f check.rs done
Signature · 1 line
Kees Cook
Re: [PATCH v9 07/27] rust: import upstream `alloc` crate
Quoted 3 lines · Kees Cook
Can you include an easy script (in the commit log) for this kind of verification?
Of course, good idea!
Quoted 9 lines
For this, I have done:
$ git am 0001-rust-import-upstream-alloc-crate.patch $ for i in $(diffstat -lp3 0001-rust-import-upstream-alloc-crate.patch); do wget --quiet -O check.rs \ https://github.com/rust-lang/rust/raw/1.62.0/library/alloc/src/$i diff -up rust/alloc/$i check.rs && echo $i: ok rm -f check.rs done
Thanks for providing the steps like in the other version. This one is even fancier :)
Cheers,
Miguel
Miguel Ojeda
maintainerPATCH v9 8/27 rust: adapt `alloc` crate to the kernel
In reply to Miguel OjedaThis customizes the subset of the Rust standard library alloc that was just imported as-is, mainly by:
- Adding SPDX license identifiers.
- Skipping modules (e.g. rc and sync) via new cfgs.
- Adding fallible (try_*) versions of existing infallible methods (i.e. returning a Result instead of panicking).
Since the standard library requires stable/unstable attributes, these additions are annotated with:
#[stable(feature = "kernel", since = "1.0.0")]
Using "kernel" as the feature allows to have the additions clearly marked. The "1.0.0" version is just a placeholder.
(At the moment, only one is needed, but in the future more fallible methods will be added).
--- rust/alloc/README.md | 33 +++++++++++++++++++++++++++++++++ rust/alloc/alloc.rs | 2 ++ rust/alloc/borrow.rs | 4 +++- rust/alloc/boxed.rs | 4 ++++ rust/alloc/collections/mod.rs | 2 ++ rust/alloc/lib.rs | 8 ++++++++ rust/alloc/raw_vec.rs | 9 +++++++++ rust/alloc/slice.rs | 2 ++ rust/alloc/vec/drain.rs | 2 ++ rust/alloc/vec/drain_filter.rs | 2 ++ rust/alloc/vec/into_iter.rs | 4 ++++ rust/alloc/vec/is_zero.rs | 2 ++ rust/alloc/vec/mod.rs | 25 +++++++++++++++++++++++++ rust/alloc/vec/partial_eq.rs | 2 ++ 14 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 rust/alloc/README.md
rust/alloc/README.md new file +33 -0
@@ -0,0 +1,33 @@ +# `alloc` + +These source files come from the Rust standard library, hosted in +the <https://github.com/rust-lang/rust> repository, licensed under +"Apache-2.0 OR MIT" and adapted for kernel use. For copyright details, +see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>. + +Please note that these files should be kept as close as possible to +upstream. In general, only additions should be performed (e.g. new +methods). Eventually, changes should make it into upstream so that, +at some point, this fork can be dropped from the kernel tree. + + +## Rationale + +On one hand, kernel folks wanted to keep `alloc` in-tree to have more +freedom in both workflow and actual features if actually needed +(e.g. receiver types if we ended up using them), which is reasonable. + +On the other hand, Rust folks wanted to keep `alloc` as close as +upstream as possible and avoid as much divergence as possible, which +is also reasonable. + +We agreed on a middle-ground: we would keep a subset of `alloc` +in-tree that would be as small and as close as possible to upstream. +Then, upstream can start adding the functions that we add to `alloc` +etc., until we reach a point where the kernel already knows exactly +what it needs in `alloc` and all the new methods are merged into +upstream, so that we can drop `alloc` from the kernel tree and go back +to using the upstream one. + +By doing this, the kernel can go a bit faster now, and Rust can +slowly incorporate and discuss the changes as needed.
rust/alloc/alloc.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! Memory allocation APIs #![stable(feature = "alloc_module" , since = "1.28.0" )]
rust/alloc/borrow.rs +3 -1
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! A module for working with borrowed data. #![stable(feature = "rust1" , since = "1.0.0" )]
@@ -11,7 +13,7 @@ use core::ops::{Add, AddAssign};#[stable(feature = "rust1" , since = "1.0.0" )] pub use core::borrow::{Borrow ,BorrowMut }; -use crate ::fmt; +use core::fmt;#[cfg(not(no_global_oom_handling))] use crate ::string::String ;
rust/alloc/boxed.rs +4 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! A pointer type for heap allocation. //! //! [`Box<T>`], casually referred to as a 'box', provides the simplest form of
@@ -163,9 +165,11 @@ use crate::str::from_boxed_utf8_unchecked;#[cfg(not(no_global_oom_handling))] use crate ::vec::Vec ; +#[cfg(not(no_thin))] #[unstable(feature = "thin_box" , issue = "92791" )] pub use thin::ThinBox ; +#[cfg(not(no_thin))] mod thin;/// A pointer type for heap allocation.
rust/alloc/collections/mod.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! Collection types. #![stable(feature = "rust1" , since = "1.0.0" )]
rust/alloc/lib.rs +8 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! # The Rust core allocation and collections library //! //! This library provides smart pointers and collections for managing
@@ -192,6 +194,7 @@ extern crate std;extern crate test;// Module with internal macros used by other modules (needs to be included before other modules). +#[cfg(not(no_macros))] #[macro_use] mod macros;
@@ -216,11 +219,16 @@ pub mod borrow;pub mod collections;#[cfg(not(no_global_oom_handling))] pub mod ffi; +#[cfg(not(no_fmt))] pub mod fmt; +#[cfg(not(no_rc))] pub mod rc;pub mod slice; +#[cfg(not(no_str))] pub mod str; +#[cfg(not(no_string))] pub mod string; +#[cfg(not(no_sync))] #[cfg(target_has_atomic = "ptr" )] pub mod sync;#[cfg(all(not(no_global_oom_handling), target_has_atomic = "ptr" ))]
rust/alloc/raw_vec.rs +9 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +#![unstable(feature = "raw_vec_internals" , reason = "unstable const warnings" , issue = "none" )] use core::alloc::LayoutError ;
@@ -307,6 +309,12 @@ impl<T, A: Allocator> RawVec<T, A> { } } +/// The same as `reserve_for_push`, but returns on errors instead of panicking or aborting. +#[inline(never)] +pub fn try_reserve_for_push (&mut self, len:usize ) ->Result <(),TryReserveError > { + self.grow_amortized (len,1 ) + } +/// Ensures that the buffer contains at least enough space to hold `len + /// additional` elements. If it doesn't already, will reallocate the /// minimum possible amount of memory necessary. Generally this will be
@@ -421,6 +429,7 @@ impl<T, A: Allocator> RawVec<T, A> {Ok (()) } +#[allow(dead_code)] fn shrink (&mut self, cap:usize ) ->Result <(),TryReserveError > {assert! (cap <= self.capacity (),"Tried to shrink to a larger capacity" );
rust/alloc/slice.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! A dynamically-sized view into a contiguous sequence, `[T]`. //! //! *[See also the slice primitive type](slice).*
rust/alloc/vec/drain.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +use crate ::alloc::{Allocator ,Global };use core::fmt;use core::iter::{FusedIterator ,TrustedLen };
rust/alloc/vec/drain_filter.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +use crate ::alloc::{Allocator ,Global };use core::ptr::{self};use core::slice::{self};
rust/alloc/vec/into_iter.rs +4 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +#[cfg(not(no_global_oom_handling))] use super ::AsVecIntoIter ;use crate ::alloc::{Allocator ,Global };
@@ -9,6 +11,7 @@ use core::iter::{ };use core::marker::PhantomData ;use core::mem::{self,ManuallyDrop }; +#[cfg(not(no_global_oom_handling))] use core::ops::Deref ;use core::ptr::{self,NonNull };use core::slice::{self};
@@ -123,6 +126,7 @@ impl<T, A: Allocator> IntoIter<T, A> { }/// Forgets to Drop the remaining elements while still allowing the backing allocation to be freed. +#[allow(dead_code)] pub (crate )fn forget_remaining_elements (&mut self) { self.ptr = self.end; }
rust/alloc/vec/is_zero.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +use crate ::boxed::Box ;#[rustc_specialization_trait]
rust/alloc/vec/mod.rs +25 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +//! A contiguous growable array type with heap-allocated contents, written //! `Vec<T>`. //!
@@ -1739,6 +1741,29 @@ impl<T, A: Allocator> Vec<T, A> { } } +/// Tries to append an element to the back of a collection. +/// +/// # Examples +/// +/// ``` +/// let mut vec = vec![1, 2]; +/// vec.try_push(3).unwrap(); +/// assert_eq!(vec, [1, 2, 3]); +/// ``` +#[inline] +#[stable(feature = "kernel" , since = "1.0.0" )] +pub fn try_push (&mut self, value:T ) ->Result <(),TryReserveError > { +if self.len == self.buf.capacity () { + self.buf.try_reserve_for_push (self.len)?; + } +unsafe { +let end = self.as_mut_ptr ().add (self.len); + ptr::write (end, value); + self.len +=1 ; + } +Ok (()) + } +/// Removes the last element from a vector and returns it, or [`None`] if it /// is empty. ///
rust/alloc/vec/partial_eq.rs +2 -0
@@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +use crate ::alloc::Allocator ;#[cfg(not(no_global_oom_handling))] use crate ::borrow::Cow ;
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 9/27 rust: add `compiler_builtins` crate
In reply to Miguel OjedaRust provides compiler_builtins as a port of LLVM's compiler-rt. Since we do not need the vast majority of them, we avoid the dependency by providing our own crate.
--- rust/compiler_builtins.rs | 63 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 rust/compiler_builtins.rs
rust/compiler_builtins.rs new file +63 -0
@@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Our own `compiler_builtins`. +//! +//! Rust provides [`compiler_builtins`] as a port of LLVM's [`compiler-rt`]. +//! Since we do not need the vast majority of them, we avoid the dependency +//! by providing this file. +//! +//! At the moment, some builtins are required that should not be. For instance, +//! [`core`] has 128-bit integers functionality which we should not be compiling +//! in. We will work with upstream [`core`] to provide feature flags to disable +//! the parts we do not need. For the moment, we define them to [`panic!`] at +//! runtime for simplicity to catch mistakes, instead of performing surgery +//! on `core.o`. +//! +//! In any case, all these symbols are weakened to ensure we do not override +//! those that may be provided by the rest of the kernel. +//! +//! [`compiler_builtins`]: https://github.com/rust-lang/compiler-builtins +//! [`compiler-rt`]: https://compiler-rt.llvm.org/ + +#![feature(compiler_builtins)] +#![compiler_builtins] +#![no_builtins] +#![no_std] + +macro_rules! define_panicking_intrinsics( + ($reason: tt, { $($ident: ident, )* }) => { + $( +#[doc(hidden)] +#[no_mangle] +pub extern"C" fn $ident() { +panic! ($reason); + } + )* + } +); + +define_panicking_intrinsics! ("`f32` should not be used" , { + __eqsf2, + __gesf2, + __lesf2, + __nesf2, + __unordsf2, +}); + +define_panicking_intrinsics! ("`f64` should not be used" , { + __unorddf2, +}); + +define_panicking_intrinsics! ("`i128` should not be used" , { + __ashrti3, + __muloti4, + __multi3, +}); + +define_panicking_intrinsics! ("`u128` should not be used" , { + __ashlti3, + __lshrti3, + __udivmodti4, + __udivti3, + __umodti3, +});
Signature · 1 line
2.37.1
Re: [PATCH v9 09/27] rust: add `compiler_builtins` crate
Quoted 6 lines · Miguel Ojeda
Rust provides
compiler_builtinsas a port of LLVM'scompiler-rt. Since we do not need the vast majority of them, we avoid the dependency by providing our own crate.Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Re: [PATCH v9 09/27] rust: add `compiler_builtins` crate
In reply to Miguel OjedaQuoted 5 lines · Miguel Ojeda
Rust provides
compiler_builtinsas a port of LLVM'scompiler-rt. Since we do not need the vast majority of them, we avoid the dependency by providing our own crate.
Quoted 9 lines
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Sven Van Asbroeck <[email protected]>Signed-off-by Sven Van Asbroeck <[email protected]>Co-developed-by Gary Guo <[email protected]>Signed-off-by Gary Guo <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>
I still really really do not like this patch. Thoughts below.
Quoted 11 lines
--- rust/compiler_builtins.rs | 63 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 rust/compiler_builtins.rsrust/compiler_builtins.rs new file +0
-0
rust/compiler_builtins.rs line 10
Quoted 10 lines
rust/compiler_builtins.rs new file +10
-0@@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Our own `compiler_builtins`. +//! +//! Rust provides [`compiler_builtins`] as a port of LLVM's [`compiler-rt`]. +//! Since we do not need the vast majority of them, we avoid the dependency +//! by providing this file. +//! +//! At the moment, some builtins are required that should not be. For instance, +//! [`core`] has 128-bit integers functionality which we should not be compiling
It's not just 128b, these are for double word sizes, so for 32b targets (I recall an earlier version of series supporting armv6) these symbols are for 64b types.
rust/compiler_builtins.rs line 12
Quoted 2 lines
rust/compiler_builtins.rs new file +2
-0+//! in. We will work with upstream [`core`] to provide feature flags to disable +//! the parts we do not need. For the moment, we define them to [`panic!`] at
Where is the bug filed upstream (or in your issue tracker)?
rust/compiler_builtins.rs line 14
Quoted 2 lines
rust/compiler_builtins.rs new file +2
-0+//! runtime for simplicity to catch mistakes, instead of performing surgery +//! on `core.o`.
Wait, so Kbuild performs surgery on alloc, but core is off limits? bah
Core is huge!
$ du -h rust/core.o 1.3M rust/core.o
Quoted 16 lines
rust/compiler_builtins.rs new file +16
-0+//! +//! In any case, all these symbols are weakened to ensure we do not override +//! those that may be provided by the rest of the kernel. +//! +//! [`compiler_builtins`]: https://github.com/rust-lang/compiler-builtins +//! [`compiler-rt`]: https://compiler-rt.llvm.org/ + +#![feature(compiler_builtins)] +#![compiler_builtins] +#![no_builtins] +#![no_std] + +macro_rules! define_panicking_intrinsics( + ($reason: tt, { $($ident: ident, )* }) => { + $( + #[doc(hidden)]
rust/compiler_builtins.rs line 40
Quoted 10 lines
rust/compiler_builtins.rs new file +10
-0+#[no_mangle] +pub extern "C" fn $ident() { +panic! ($reason); + } + )* + } +); + +define_panicking_intrinsics! ("`f32` should not be used" , { + __eqsf2,
I don't see ^ this symbol in core.
$ llvm-nm rust/core.o | grep "U __" | sort | tr -s " " U __gesf2 U __lesf2 U __udivti3 U __umodti3 U __unorddf2 U __unordsf2 U __x86_indirect_thunk_r11
so a few of these seem extraneous. Are you sure they're coming from core? (or was it the different arch support that was removed from v8 to facilitate v9?)
__gesf2, __lesf2, and __unordsf2 all seem to be coming from <<f32>::classify> and <<f32>::to_bits::ct_f32_to_u32>. Surely we can avoid more f32 support by relying on objcopy -N/--strip-symbol=/--strip-symbols=filename to slice out/drop symbols from core.o?
<core::fmt::num::exp_u128> uses __umodti3+__udivti3
__unorddf2 is referenced by <<f64>::classify> and
<<f64>::to_bits::ct_f64_to_u64>.
Can we slice those 5 symbols out from core, or are they further referenced from within core? If we can, we can drop this patch outright, and avoid a false-negative when C code generates references to these symbols from the compiler runtime, for architectures where don't want to provide a full compiler runtime. (--rtlib=none is a pipe dream; first we need to get to the point where we're not -ffreestanding for all targets...)
I suspect someone can iteratively whittle down core to avoid these symbols?
```
diff --git a/rust/Makefile b/rust/Makefile
index 7700d3853404..e64a82c21156 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -359,6 +359,7 @@ $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs
$(obj)/target.json FORCE
$(obj)/compiler_builtins.o: private rustc_objcopy = -w -W '__*'
$(obj)/compiler_builtins.o: $(src)/compiler_builtins.rs $(obj)/core.o FORCE
$(call if_changed_dep,rustc_library)
+ @$(OBJCOPY) --strip-symbols=$(obj)/strip-symbols.txt $(obj)/core.o
$(obj)/alloc.o: private skip_clippy = 1
$(obj)/alloc.o: private skip_flags = -Dunreachable_pub
```then define symbols in rust/strip-symbols.txt?
I'm getting the sense that no, there are many functions like <usize as core::fmt::UpperExp>::fmt
<i64 as core::fmt::UpperExp>::fmt
<u64 as core::fmt::UpperExp>::fmt
that call many of these functions...so if you call format! on an i64/u64/usize you'll potentially panic the kernel? Seems kinda dangerous.
I'm also seeing disassemblers having trouble completely disassembling core.o ... the more I look to see who's calling what, the more instances I find...
Quoted 26 lines
rust/compiler_builtins.rs new file +23
-1+ __gesf2, + __lesf2, + __nesf2, + __unordsf2, +}); + +define_panicking_intrinsics! ("`f64` should not be used" , { + __unorddf2, +}); + +define_panicking_intrinsics! ("`i128` should not be used" , { + __ashrti3, + __muloti4, + __multi3, +}); + +define_panicking_intrinsics! ("`u128` should not be used" , { + __ashlti3, + __lshrti3, + __udivmodti4, + __udivti3, + __umodti3, +}); --2.37.1
Signature · 2 lines
Thanks, ~Nick Desaulniers
Re: [PATCH v9 09/27] rust: add `compiler_builtins` crate
Quoted 132 lines · Nick Desaulniers
Quoted 5 lines · Miguel Ojeda
Rust provides
compiler_builtinsas a port of LLVM'scompiler-rt. Since we do not need the vast majority of them, we avoid the dependency by providing our own crate.Quoted 9 lines
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Sven Van Asbroeck <[email protected]>Signed-off-by Sven Van Asbroeck <[email protected]>Co-developed-by Gary Guo <[email protected]>Signed-off-by Gary Guo <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>I still really really do not like this patch. Thoughts below.
Quoted 11 lines
--- rust/compiler_builtins.rs | 63 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 rust/compiler_builtins.rsrust/compiler_builtins.rs new file +0
-0rust/compiler_builtins.rs line 10
Quoted 10 lines
rust/compiler_builtins.rs new file +10
-0@@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Our own `compiler_builtins`. +//! +//! Rust provides [`compiler_builtins`] as a port of LLVM's [`compiler-rt`]. +//! Since we do not need the vast majority of them, we avoid the dependency +//! by providing this file. +//! +//! At the moment, some builtins are required that should not be. For instance, +//! [`core`] has 128-bit integers functionality which we should not be compiling It's not just 128b, these are for double word sizes, so for 32b targets (I recall an earlier version of series supporting armv6) these symbols are for 64b types.
rust/compiler_builtins.rs line 14
Quoted 4 lines
rust/compiler_builtins.rs new file +4
-0+//! in. We will work with upstream [`core`] to provide feature flags to disable +//! the parts we do not need. For the moment, we define them to [`panic!`] at +//! runtime for simplicity to catch mistakes, instead of performing surgery +//! on `core.o`. Wait, so
Kbuildperforms surgery on alloc, but core is off limits? bahQuoted 16 lines
rust/compiler_builtins.rs new file +16
-0+//! +//! In any case, all these symbols are weakened to ensure we do not override +//! those that may be provided by the rest of the kernel. +//! +//! [`compiler_builtins`]: https://github.com/rust-lang/compiler-builtins +//! [`compiler-rt`]: https://compiler-rt.llvm.org/ + +#![feature(compiler_builtins)] +#![compiler_builtins] +#![no_builtins] +#![no_std] + +macro_rules! define_panicking_intrinsics( + ($reason: tt, { $($ident: ident, )* }) => { + $( + #[doc(hidden)]rust/compiler_builtins.rs line 40
Quoted 10 lines
rust/compiler_builtins.rs new file +10
-0+#[no_mangle] +pub extern "C" fn $ident() { +panic! ($reason); + } + )* + } +); + +define_panicking_intrinsics! ("`f32` should not be used" , { + __eqsf2,I don't see ^ this symbol in core.
$ llvm-nm rust/core.o | grep "U __" | sort | tr -s " " U __gesf2 U __lesf2 U __udivti3 U __umodti3 U __unorddf2 U __unordsf2 U __x86_indirect_thunk_r11so a few of these seem extraneous. Are you sure they're coming from core? (or was it the different arch support that was removed from v8 to facilitate v9?)
__gesf2,__lesf2, and__unordsf2all seem to be coming from <<f32>::classify> and <<f32>::to_bits::ct_f32_to_u32>. Surely we can avoid more f32 support by relying on objcopy -N/--strip-symbol=/--strip-symbols=filename to slice out/drop symbols from core.o?<
core::fmt::num::exp_u128> uses __umodti3+__udivti3
__unorddf2is referenced by <<f64>::classify> and
<<f64>::to_bits::ct_f64_to_u64>.Can we slice those 5 symbols out from core, or are they further referenced from within core? If we can, we can drop this patch outright, and avoid a false-negative when C code generates references to these symbols from the compiler runtime, for architectures where don't want to provide a full compiler runtime. (--rtlib=none is a pipe dream; first we need to get to the point where we're not -ffreestanding for all targets...)
I suspect someone can iteratively whittle down core to avoid these symbols?
```
rust/Makefile +0
-0@@ -359,6 +359,7 @@ $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs$(obj)/target.json FORCE
$(obj)/compiler_builtins.o: privaterustc_objcopy= -w -W '__*' $(obj)/compiler_builtins.o: $(src)/compiler_builtins.rs $(obj)/core.o FORCE$(call if_changed_dep,rustc_library)+ @$(OBJCOPY) --strip-symbols=$(obj)/strip-symbols.txt $(obj)/core.o
$(obj)/alloc.o: private
skip_clippy= 1 $(obj)/alloc.o: privateskip_flags= -Dunreachable_pub
```
then define symbols inrust/strip-symbols.txt?
Quoted 4 lines
I'm getting the sense that no, there are many functions like <usize as core::fmt::UpperExp>::fmt
<i64 as core::fmt::UpperExp>::fmt
<u64 as core::fmt::UpperExp>::fmt
Another idea I had, and Ard mentioned to me this morning that efistub does something similar:
I think objcopy can rename symbol references. So instead of calling foo, I think you can use objcopy to call bar instead.
To avoid the case of Rust core refering to symbols typically provided by the --rtlib={libgcc|compiler-rt}, what if we:
1. build core.o (with reference to the unfavorable symbols)
2. use objcopy to rename references in core.o to the symbols listed in this file; maybe prepend them with rust_core_ or something.
3. provide this crate with panic'ing definitions, but for the renamed symbols.
That way, enabling CONFIG_RUST=y won't hide the case where C code is triggering such libcalls from the C compiler?
To restate the concern, I don't want CONFIG_RUST=y to hide that fact that someone wrote float x, y; if (x != y) ... in C code, where the C compiler will likely lower that to a libcall to __nesf2 since with the current approach in this patch, linkage would proceed. The current behavior is to intentionally fail to link. I think what I describe above may work to keep this intended behavior of the kernel's build system.
Ah, looks like there's objcopy flags:
--redefine-sym old=new
Change the name of a symbol old, to new. This can be
useful when one is trying link
two things together for which you have no source, and there
are name collisions.
--redefine-syms=filename
Apply --redefine-sym to each symbol pair "old new" listed
in the file filename.
filename is simply a flat file, with one symbol pair per
line. Line comments may be
introduced by the hash character. This option may be given
more than once.
So probably could start with my diff from above, but use --redefine-syms=filename instead of --strip-symbols=.
Quoted 24 lines
Quoted 24 lines
rust/compiler_builtins.rs new file +23
-0+ __gesf2, + __lesf2, + __nesf2, + __unordsf2, +}); + +define_panicking_intrinsics! ("`f64` should not be used" , { + __unorddf2, +}); + +define_panicking_intrinsics! ("`i128` should not be used" , { + __ashrti3, + __muloti4, + __multi3, +}); + +define_panicking_intrinsics! ("`u128` should not be used" , { + __ashlti3, + __lshrti3, + __udivmodti4, + __udivti3, + __umodti3, +});
Quoted 8 lines
Quoted 3 lines
--
2.37.1--
Thanks,
~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
Gary Guo
reviewerRe: [PATCH v9 09/27] rust: add `compiler_builtins` crate
Quoted 40 lines · On Wed, 24 Aug 2022 11:38:46 -0700 Nick Desaulniers <[email protected]>…
Another idea I had, and Ard mentioned to me this morning that efistub does something similar:
I think objcopy can rename symbol references. So instead of calling foo, I think you can use objcopy to call bar instead.
To avoid the case of Rust core refering to symbols typically provided by the --rtlib={libgcc|compiler-rt}, what if we:
1. build core.o (with reference to the unfavorable symbols)
2. use objcopy to rename references in core.o to the symbols listed in this file; maybe prepend them withrust_core_or something.
3. provide this crate with panic'ing definitions, but for the renamed symbols.That way, enabling
CONFIG_RUST=ywon't hide the case where C code is triggering such libcalls from the C compiler?To restate the concern, I don't want
CONFIG_RUST=yto hide that fact that someone wrotefloat x, y; if (x != y) ...in C code, where the C compiler will likely lower that to a libcall to__nesf2since with the current approach in this patch, linkage would proceed. The current behavior is to intentionally fail to link. I think what I describe above may work to keep this intended behavior of the kernel's build system.Ah, looks like there's objcopy flags:
--redefine-sym old=new
Change the name of a symbol old, to new. This can be
useful when one is trying link
two things together for which you have no source, and there
are name collisions.--redefine-syms=filename
Apply --redefine-sym to each symbol pair "old new" listed
in the file filename.
filename is simply a flat file, with one symbol pair per
line. Line comments may be
introduced by the hash character. This option may be given
more than once.
Quoted 6 lines
So probably could start with my diff from above, but use --redefine-syms=filename instead of --strip-symbols=.
--
Thanks,
~Nick Desaulniers
This is the approach we are about to take, see
https://github.com/Rust-for-Linux/linux/pull/779.
It's easy for just one symbol that is known to be not defined, but it can be more complex as some of these symbols can be defined on a platform but not another, so we would have to generate a dynamic list of what symbols to "hijack" depending on the config. Currently we avoid this issue by having all symbols in compiler_builtins crate weak, but we couldn't weakly redefine a symbol.
It's certainly doable, though, but it require some effort. I am not quite available recently so hadn't further extend and polish my patch.
Best,
Gary
Miguel Ojeda
maintainerPATCH v9 10/27 rust: add `macros` crate
In reply to Miguel OjedaThis crate contains all the procedural macros ("proc macros") shared by all the kernel.
Procedural macros allow to create syntax extensions. They run at compile-time and can consume as well as produce Rust syntax.
For instance, the module! macro that is used by Rust modules is implemented here. It allows to easily declare the equivalent information to the MODULE_* macros in C modules, e.g.:
module! {
type: RustMinimal,
name: b"rust_minimal",
author: b"Rust for Linux Contributors",
description: b"Rust minimal sample",
license: b"GPL",
}
--- rust/macros/helpers.rs | 51 ++++++++ rust/macros/lib.rs | 72 +++++++++++ rust/macros/module.rs | 282 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 405 insertions(+) create mode 100644 rust/macros/helpers.rs create mode 100644 rust/macros/lib.rs create mode 100644 rust/macros/module.rs
rust/macros/helpers.rs new file +51 -0
@@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0 + +use proc_macro::{token_stream,TokenTree }; + +pub (crate )fn try_ident (it: &mut token_stream::IntoIter ) ->Option <String > { +if let Some (TokenTree ::Ident (ident)) = it.next () { +Some (ident.to_string ()) + }else { +None + } +} + +pub (crate )fn try_literal (it: &mut token_stream::IntoIter ) ->Option <String > { +if let Some (TokenTree ::Literal (literal)) = it.next () { +Some (literal.to_string ()) + }else { +None + } +} + +pub (crate )fn try_byte_string (it: &mut token_stream::IntoIter ) ->Option <String > { +try_literal (it).and_then (|byte_string| { +if byte_string.starts_with ("b\"" ) && byte_string.ends_with ('\"' ) { +Some (byte_string[2 ..byte_string.len () -1 ].to_string ()) + }else { +None + } + }) +} + +pub (crate )fn expect_ident (it: &mut token_stream::IntoIter ) ->String { +try_ident (it).expect ("Expected Ident" ) +} + +pub (crate )fn expect_punct (it: &mut token_stream::IntoIter ) ->char { +if let TokenTree ::Punct (punct) = it.next ().expect ("Reached end of token stream for Punct" ) { + punct.as_char () + }else { +panic! ("Expected Punct" ); + } +} + +pub (crate )fn expect_byte_string (it: &mut token_stream::IntoIter ) ->String { +try_byte_string (it).expect ("Expected byte string" ) +} + +pub (crate )fn expect_end (it: &mut token_stream::IntoIter ) { +if it.next ().is_some () { +panic! ("Expected end" ); + } +}
rust/macros/lib.rs new file +72 -0
72 lines not on this page · raw message
rust/macros/module.rs new file +282 -0
282 lines not on this page · raw message
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 11/27 rust: add `bindings` crate
In reply to Miguel OjedaThis crate contains the bindings to the C side of the kernel.
Calling C (in general, FFI) is assumed to be unsafe in Rust and, in many cases, this is accurate. For instance, virtually all C functions that take a pointer are unsafe since, typically, it will be dereferenced at some point (and in most cases there is no way for the callee to check its validity beforehand).
Since one of the goals of using Rust in the kernel is precisely to avoid unsafe code in "leaf" kernel modules (e.g. drivers), these bindings should not be used directly by them.
Instead, these bindings need to be wrapped into safe abstractions. These abstractions provide a safe API that kernel modules can use. In this way, unsafe code in kernel modules is minimized.
--- rust/bindings/bindings_helper.h | 13 ++++++++ rust/bindings/lib.rs | 53 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 rust/bindings/bindings_helper.h create mode 100644 rust/bindings/lib.rs
rust/bindings/bindings_helper.h new file +13 -0
@@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Header that contains the code (mostly headers) for which Rust bindings +* will be automatically generated by `bindgen`. +* +* Sorted alphabetically. +*/ + +#include <linux/slab.h> + +/* `bindgen` gets confused at certain things. */ +const gfp_t BINDINGS_GFP_KERNEL =GFP_KERNEL ; +const gfp_t BINDINGS___GFP_ZERO = __GFP_ZERO;
rust/bindings/lib.rs new file +53 -0
53 lines not on this page · raw message
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 12/27 rust: add `kernel` crate
In reply to Miguel OjedaFrom: Wedson Almeida Filho <[email protected]>
The kernel crate currently includes all the abstractions that wrap kernel features written in C.
These abstractions call the C side of the kernel via the generated bindings with the bindgen tool. Modules developed in Rust should never call the bindings themselves.
In the future, as the abstractions grow in number, we may need to split this crate into several, possibly following a similar subdivision in subsystems as the kernel itself and/or moving the code to the actual subsystems.
--- rust/kernel/allocator.rs | 64 +++++++++++++ rust/kernel/error.rs | 59 ++++++++++++ rust/kernel/lib.rs | 78 +++++++++++++++ rust/kernel/prelude.rs | 20 ++++ rust/kernel/print.rs | 198 +++++++++++++++++++++++++++++++++++++++ rust/kernel/str.rs | 72 ++++++++++++++ 6 files changed, 491 insertions(+) create mode 100644 rust/kernel/allocator.rs create mode 100644 rust/kernel/error.rs create mode 100644 rust/kernel/lib.rs create mode 100644 rust/kernel/prelude.rs create mode 100644 rust/kernel/print.rs create mode 100644 rust/kernel/str.rs
rust/kernel/allocator.rs new file +64 -0
@@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Allocator support. + +use core::alloc::{GlobalAlloc ,Layout }; +use core::ptr; + +use crate ::bindings; + +struct KernelAllocator ; + +unsafe impl GlobalAlloc for KernelAllocator { +unsafe fn alloc (&self, layout:Layout ) -> *mut u8 { +// `krealloc()` is used instead of `kmalloc()` because the latter is +// an inline function and cannot be bound to as a result. +unsafe { bindings::krealloc (ptr::null (), layout.size (), bindings::GFP_KERNEL )as *mut u8 } + } + +unsafe fn dealloc (&self, ptr: *mut u8 , _layout:Layout ) { +unsafe { + bindings::kfree (ptras *const core::ffi::c_void ); + } + } +} + +#[global_allocator] +static ALLOCATOR :KernelAllocator =KernelAllocator ; + +// `rustc` only generates these for some crate types. Even then, we would need +// to extract the object file that has them from the archive. For the moment, +// let's generate them ourselves instead. +// +// Note that `#[no_mangle]` implies exported too, nowadays. +#[no_mangle] +fn __rust_alloc (size:usize , _align:usize ) -> *mut u8 { +unsafe { bindings::krealloc (core::ptr::null (), size, bindings::GFP_KERNEL )as *mut u8 } +} + +#[no_mangle] +fn __rust_dealloc (ptr: *mut u8 , _size:usize , _align:usize ) { +unsafe { bindings::kfree (ptras *const core::ffi::c_void ) }; +} + +#[no_mangle] +fn __rust_realloc (ptr: *mut u8 , _old_size:usize , _align:usize , new_size:usize ) -> *mut u8 { +unsafe { + bindings::krealloc ( + ptras *const core::ffi::c_void , + new_size, + bindings::GFP_KERNEL , + )as *mut u8 + } +} + +#[no_mangle] +fn __rust_alloc_zeroed (size:usize , _align:usize ) -> *mut u8 { +unsafe { + bindings::krealloc ( + core::ptr::null (), + size, + bindings::GFP_KERNEL | bindings::__GFP_ZERO, + )as *mut u8 + } +}
rust/kernel/error.rs new file +59 -0
59 lines not on this page · raw message
rust/kernel/lib.rs new file +78 -0
78 lines not on this page · raw message
rust/kernel/prelude.rs new file +20 -0
@@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! The `kernel` prelude. +//! +//! These are the most common items used by Rust code in the kernel, +//! intended to be imported by all Rust code, for convenience. +//! +//! # Examples +//! +//! ``` +//! use kernel::prelude::*; +//! ``` + +pub use super ::{ + error::{Error ,Result }, + pr_emerg, pr_info,ThisModule , +}; +pub use alloc::{boxed::Box , vec::Vec }; +pub use core::pin::Pin ; +pub use macros::module;
rust/kernel/print.rs new file +198 -0
198 lines not on this page · raw message
rust/kernel/str.rs new file +72 -0
72 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [PATCH v9 12/27] rust: add `kernel` crate
rust/kernel/allocator.rs line 24
Quoted 13 lines
rust/kernel/allocator.rs new file +13
-0+unsafe impl GlobalAlloc for KernelAllocator { +unsafe fn alloc (&self, layout:Layout ) -> *mut u8 { +// `krealloc()` is used instead of `kmalloc()` because the latter is +// an inline function and cannot be bound to as a result. +unsafe { bindings::krealloc (ptr::null (), layout.size (), bindings::GFP_KERNEL )as *mut u8 } + } + +unsafe fn dealloc (&self, ptr: *mut u8 , _layout:Layout ) { +unsafe { + bindings::kfree (ptras *const core::ffi::c_void ); + } + } +}
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
For example:
struct bio *bio;
for (...) {
bio = bio_alloc_bioset(bdev, nr_vecs, opf, GFP_NOIO, bs);
if (!bio)
return -ENOMEM;
}Without GFP_NOIO we can run into a deadlock, because the kernel will try give us free memory by flushing the dirty pages and we need the memory to actually do it and boom, deadlock.
Or we can be allocating some structs under spinlock (yeah, that happens too):
struct efc_vport *vport;
spin_lock_irqsave(...);
vport = kzalloc(sizeof(*vport), GFP_ATOMIC);
if (!vport) {
spin_unlock_irqrestore(...);
return NULL;
}
spin_unlock(...);Same can (and probably will) happen to e.g. Vec elements. So some form of flags passing should be supported in try_* variants:
let mut vec = Vec::try_new(GFP_ATOMIC)?; vec.try_push(GFP_ATOMIC, 1)?; vec.try_push(GFP_ATOMIC, 2)?; vec.try_push(GFP_ATOMIC, 3)?;
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 3 lines · Konstantin Shelekhin
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's alloc could support our use cases, etc. has been on discussion for a long time.
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;
Cheers,
Miguel
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 7 lines · Miguel Ojeda
Quoted 2 lines
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's
alloccould support our use cases, etc. has been on discussion for a long time.
Quoted 5 lines
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;
IMO it's just easier to always pass flags like this:
let a = Box::try_new(GFP_KERNEL | GFP_DMA, 101)?;
But if allocate_with_flags() will be somehow present in the API that's just what we need.
P.S. Thanks for a quick reply!
Re: [PATCH v9 12/27] rust: add `kernel` crate
In reply to Miguel OjedaQuoted 10 lines · Miguel Ojeda
Quoted 3 lines · Konstantin Shelekhin
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's
alloccould support our use cases, etc. has been on discussion for a long time.
Quoted 5 lines
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;
Something I've been wondering about for a while is ...
struct task_struct {
...
+ gfp_t gfp_flags;
...
};
We've already done some work towards this with the scoped allocation API for NOIO and NOFS, but having spin_lock() turn current->gfp_flags into GFP_ATOMIC might not be the worst idea in the world.
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 23 lines · Matthew Wilcox
Quoted 10 lines · Miguel Ojeda
Quoted 3 lines · Konstantin Shelekhin
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's
alloccould support our use cases, etc. has been on discussion for a long time.Quoted 5 lines
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;Something I've been wondering about for a while is ...
struct
task_struct{
...(quoted diff) +1
-0+ gfp_t gfp_flags;
Quoted 1 line
};
For GFP_ATOMIC, we could use preempt_count except that it isn't always enabled. Conveniently, it is already separated out into its own config. How do people feel about removing CONFIG_PREEMPT_COUNT and having the count always enabled?
We would then have a way to reliably detect when we are in atomic context and we could catch other scenarios beyond allocation. For example, I recently noticed that the following code (as a minimal reproduction) does _not_ lead to a deadlock when CONFIG_PREEMPT=n:
rcu_read_lock();synchronize_rcu();
Boqun explained to me that the reason is that synchronize_rcu() is not supposed to be called from an rcu read-side critical section and the current implementation takes advantage of this fact plus there is no way to detect if we're in atomic context. This is all well and good, but if one makes this mistake, the result is a potential user-after-free. Always having preempt_count would allow us to behave differently here -- this is another conversation but at least we'll have to option to choose what to do. (And it seems to me that detecting this and deadlocking or BUG'ing would be preferable over the alternative [CC'ing Kees].)
Anyway, objections to a patch series that would amount to the changes below?
Thanks,
-Wedson
arch/arm/include/asm/assembler.h +0 -11
@@ -224,7 +224,6 @@ THUMB( fpreg .req r7 )/* * Increment/decrement the preempt count. */ -#ifdef CONFIG_PREEMPT_COUNT .macro inc_preempt_count, ti, tmp ldr \tmp, [\ti, #TI_PREEMPT] @ get preempt count add \tmp, \tmp, #1 @ increment it
@@ -241,16 +240,6 @@ THUMB( fpreg .req r7 ) get_thread_info \ti dec_preempt_count \ti, \tmp .endm -#else - .macro inc_preempt_count, ti, tmp - .endm - - .macro dec_preempt_count, ti, tmp - .endm - - .macro dec_preempt_count_ti, ti, tmp - .endm -#endif #define USERL(l, x...) \ 9999: x; \
arch/arm/kernel/iwmmxt.S +0 -2
@@ -95,9 +95,7 @@ ENTRY(iwmmxt_task_enable) mov r2, r2 @ cpwait bl concan_save -#ifdef CONFIG_PREEMPT_COUNT get_thread_info r10 -#endif 4: dec_preempt_count r10, r3 ret r9 @ normal exit from exception
arch/xtensa/kernel/entry.S +1 -1
@@ -832,7 +832,7 @@ ENTRY(debug_exception) * preemption if we have HW breakpoints to preserve DEBUGCAUSE.DBNUM * meaning. */ -#if defined(CONFIG_PREEMPT_COUNT) && defined(CONFIG_HAVE_HW_BREAKPOINT) +#if defined(CONFIG_HAVE_HW_BREAKPOINT) GET_THREAD_INFO(a2, a1) l32i a3, a2, TI_PRE_COUNT addi a3, a3, 1
drivers/gpu/drm/i915/Kconfig.debug +0 -1
@@ -22,7 +22,6 @@ config DRM_I915_DEBUG depends on EXPERT # only for developers depends on !COMPILE_TEST # never built by robots select DEBUG_FS - select PREEMPT_COUNT select I2C_CHARDEV select STACKDEPOT select DRM_DP_AUX_CHARDEV
drivers/gpu/drm/i915/i915_utils.h +1 -2
@@ -293,8 +293,7 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms) (Wmax))#define wait_for (COND ,MS ) _wait_for((COND), (MS) * 1000, 10, 1000) -/* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */ -#if defined(CONFIG_DRM_I915_DEBUG ) && defined(CONFIG_PREEMPT_COUNT ) +#if defined(CONFIG_DRM_I915_DEBUG )# define _WAIT_FOR_ATOMIC_CHECK (ATOMIC ) WARN_ON_ONCE((ATOMIC) && !in_atomic())#else # define _WAIT_FOR_ATOMIC_CHECK (ATOMIC ) do { } while (0)
include/linux/bit_spinlock.h +1 -3
@@ -90,10 +90,8 @@ static inline int bit_spin_is_locked(int bitnum, unsigned long *addr) {#if defined(CONFIG_SMP ) || defined(CONFIG_DEBUG_SPINLOCK )return test_bit (bitnum, addr); -#elif definedCONFIG_PREEMPT_COUNT -return preempt_count ();#else -return 1 ; +return preempt_count ();#endif }
include/linux/lockdep.h +2 -4
@@ -600,16 +600,14 @@ do { \#define lockdep_assert_preemption_enabled () \ do { \ - WARN_ON_ONCE(IS_ENABLED(CONFIG_PREEMPT_COUNT) && \ - __lockdep_enabled && \ + WARN_ON_ONCE(__lockdep_enabled && \ (preempt_count() != 0 || \ !this_cpu_read(hardirqs_enabled))); \ } while (0)#define lockdep_assert_preemption_disabled () \ do { \ - WARN_ON_ONCE(IS_ENABLED(CONFIG_PREEMPT_COUNT) && \ - __lockdep_enabled && \ + WARN_ON_ONCE(__lockdep_enabled && \ (preempt_count() == 0 && \ this_cpu_read(hardirqs_enabled))); \ } while (0)
include/linux/page_ref.h +0 -2
@@ -271,9 +271,7 @@ static inline bool folio_ref_try_add_rcu(struct folio *folio, int count)* context, so (on !SMP) we only need preemption to be disabled * and TINY_RCU does that for us. */ -# ifdef CONFIG_PREEMPT_COUNT VM_BUG_ON (!in_atomic () && !irqs_disabled ()); -# endif VM_BUG_ON_FOLIO (folio_ref_count (folio) ==0 , folio);folio_ref_add (folio, count);#else
include/linux/preempt.h +3 -32
@@ -56,8 +56,7 @@#define PREEMPT_DISABLED (PREEMPT_DISABLE_OFFSET + PREEMPT_ENABLED)/* -* Disable preemption until the scheduler is running -- use an unconditional -* value so that it also works on !PREEMPT_COUNT kernels. +* Disable preemption until the scheduler is running. * * Reset by start_kernel()->sched_init()->init_idle()->init_idle_preempt_count(). */
@@ -69,7 +68,6 @@* * preempt_count() == 2*PREEMPT_DISABLE_OFFSET * -* Note: PREEMPT_DISABLE_OFFSET is 0 for !PREEMPT_COUNT kernels. * Note: See finish_task_switch(). */ #define FORK_PREEMPT_COUNT (2*PREEMPT_DISABLE_OFFSET + PREEMPT_ENABLED)
@@ -133,11 +131,7 @@ static __always_inline unsigned char interrupt_context_level(void)/* * The preempt_count offset after preempt_disable(); */ -#if defined(CONFIG_PREEMPT_COUNT ) -# define PREEMPT_DISABLE_OFFSET PREEMPT_OFFSET -#else -# define PREEMPT_DISABLE_OFFSET 0 -#endif +#define PREEMPT_DISABLE_OFFSET PREEMPT_OFFSET/* * The preempt_count offset after spin_lock()
@@ -154,7 +148,7 @@ static __always_inline unsigned char interrupt_context_level(void)* * spin_lock_bh() * -* Which need to disable both preemption (CONFIG_PREEMPT_COUNT) and +* Which need to disable both preemption and * softirqs, such that unlock sequences of: * * spin_unlock();
@@ -196,8 +190,6 @@ extern void preempt_count_sub(int val);#define preempt_count_inc () preempt_count_add(1)#define preempt_count_dec () preempt_count_sub(1) -#ifdef CONFIG_PREEMPT_COUNT -#define preempt_disable () \ do { \ preempt_count_inc(); \
@@ -263,27 +255,6 @@ do { \__preempt_count_dec (); \ }while (0 ) -#else /* !CONFIG_PREEMPT_COUNT */ - -/* -* Even if we don't have any preemption, we need preempt disable/enable -* to be barriers, so that we don't have things like get_user/put_user -* that can cause faults and scheduling migrate into our preempt-protected -* region. -*/ -#define preempt_disable () barrier() -#define sched_preempt_enable_no_resched () barrier() -#define preempt_enable_no_resched () barrier() -#define preempt_enable () barrier() -#define preempt_check_resched () do { } while (0) - -#define preempt_disable_notrace () barrier() -#define preempt_enable_no_resched_notrace () barrier() -#define preempt_enable_notrace () barrier() -#define preemptible () 0 - -#endif /* CONFIG_PREEMPT_COUNT */ - #ifdef MODULE/* * Modules have no business playing preemption tricks.
include/linux/uaccess.h +0 -3
@@ -225,9 +225,6 @@ static inline bool pagefault_disabled(void)* * This function should only be used by the fault handlers. Other users should * stick to pagefault_disabled(). -* Please NEVER use preempt_disable() to disable the fault handler. With -* !CONFIG_PREEMPT_COUNT, this is like a NOP. So the handler won't be disabled. -* in_atomic() will report different values based on !CONFIG_PREEMPT_COUNT. */ #define faulthandler_disabled () (pagefault_disabled() || in_atomic())
kernel/Kconfig.preempt +0 -4
@@ -86,12 +86,8 @@ config PREEMPT_RT endchoice -config PREEMPT_COUNT - bool - config PREEMPTION bool - select PREEMPT_COUNT config PREEMPT_DYNAMIC bool "Preemption behaviour defined on boot"
kernel/rcu/Kconfig.debug +0 -1
@@ -122,7 +122,6 @@ config RCU_STRICT_GRACE_PERIOD bool "Provide debug RCU implementation with short grace periods" depends on DEBUG_KERNEL && RCU_EXPERT && NR_CPUS <= 4 && !TINY_RCU default n - select PREEMPT_COUNT if PREEMPT=n help Select this option to build an RCU variant that is strict about grace periods, making them as short as it can. This limits
kernel/rcu/tree.c +1 -1
@@ -2478,7 +2478,7 @@ static __latent_entropy void rcu_core(void)WARN_ON_ONCE (!rdp->beenonline);/* Report any deferred quiescent states if preemption enabled. */ -if (IS_ENABLED (CONFIG_PREEMPT_COUNT ) && (!(preempt_count () &PREEMPT_MASK ))) { +if (!(preempt_count () &PREEMPT_MASK )) {rcu_preempt_deferred_qs (current); }else if (rcu_preempt_need_deferred_qs (current)) {set_tsk_need_resched (current);
kernel/rcu/tree_plugin.h +1 -1
@@ -29,7 +29,7 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) (IS_ENABLED (CONFIG_HOTPLUG_CPU ) &&lockdep_is_cpus_held ()) ||rcu_lockdep_is_held_nocb (rdp) || (rdp ==this_cpu_ptr (&rcu_data) && - !(IS_ENABLED (CONFIG_PREEMPT_COUNT ) &&preemptible ())) || + !preemptible ()) || rcu_current_is_nocb_kthread(rdp)),"Unsafe read of RCU_NOCB offloaded state" );
kernel/sched/core.c +1 -8
@@ -5111,8 +5111,7 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)* finish_task_switch() for details. * * finish_task_switch() will drop rq->lock() and lower preempt_count -* and the preempt_enable() will end up enabling preemption (on -* PREEMPT_COUNT kernels). +* and the preempt_enable() will end up enabling preemption. */ finish_task_switch (prev);
@@ -9901,9 +9900,6 @@ void __cant_sleep(const char *file, int line, int preempt_offset)if (irqs_disabled ())return ; -if (!IS_ENABLED (CONFIG_PREEMPT_COUNT )) -return ; -if (preempt_count () > preempt_offset)return ;
@@ -9933,9 +9929,6 @@ void __cant_migrate(const char *file, int line)if (is_migration_disabled (current))return ; -if (!IS_ENABLED (CONFIG_PREEMPT_COUNT )) -return ; -if (preempt_count () >0 )return ;
lib/Kconfig.debug +0 -2
@@ -1230,7 +1230,6 @@ config PROVE_LOCKING select DEBUG_RWSEMS select DEBUG_WW_MUTEX_SLOWPATH select DEBUG_LOCK_ALLOC - select PREEMPT_COUNT if !ARCH_NO_PREEMPT select TRACE_IRQFLAGS default n help
@@ -1431,7 +1430,6 @@ config DEBUG_LOCKDEP config DEBUG_ATOMIC_SLEEP bool "Sleep inside atomic section checking" - select PREEMPT_COUNT depends on DEBUG_KERNEL depends on !ARCH_NO_PREEMPT help
mm/slub.c +1 -5
@@ -3106,19 +3106,15 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, {void *p; -#ifdef CONFIG_PREEMPT_COUNT /* * We may have been preempted and rescheduled on a different * cpu before disabling preemption. Need to reload cpu area * pointer. */ c =slub_get_cpu_ptr (s->cpu_slab); -#endif - p =___slab_alloc (s, gfpflags, node, addr, c); -#ifdef CONFIG_PREEMPT_COUNT slub_put_cpu_ptr (s->cpu_slab); -#endif +return p; }
tools/testing/selftests/rcutorture/configs/rcu/SRCU-T +0 -1
@@ -9,4 +9,3 @@ CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_OBJECTS_RCU_HEAD=n CONFIG_DEBUG_ATOMIC_SLEEP=y -#CHECK#CONFIG_PREEMPT_COUNT=y
tools/testing/selftests/rcutorture/configs/rcu/SRCU-U +0 -1
@@ -7,4 +7,3 @@ CONFIG_PREEMPT_DYNAMIC=n CONFIG_RCU_TRACE=n CONFIG_DEBUG_LOCK_ALLOC=n CONFIG_DEBUG_OBJECTS_RCU_HEAD=n -CONFIG_PREEMPT_COUNT=n
tools/testing/selftests/rcutorture/configs/rcu/TINY01 +0 -1
@@ -11,4 +11,3 @@ CONFIG_RCU_TRACE=n #CHECK#CONFIG_RCU_STALL_COMMON=n CONFIG_DEBUG_LOCK_ALLOC=n CONFIG_DEBUG_OBJECTS_RCU_HEAD=n -CONFIG_PREEMPT_COUNT=n
tools/testing/selftests/rcutorture/doc/TINY_RCU.txt +0 -1
@@ -4,7 +4,6 @@ This document gives a brief rationale for the TINY_RCU test cases. Kconfig Parameters: CONFIG_DEBUG_LOCK_ALLOC -- Do all three and none of the three. -CONFIG_PREEMPT_COUNT CONFIG_RCU_TRACE The theory here is that randconfig testing will hit the other six possible
tools/testing/selftests/rcutorture/doc/TREE_RCU-kconfig.txt +0 -1
@@ -42,7 +42,6 @@ CONFIG_64BIT Used only to check CONFIG_RCU_FANOUT value, inspection suffices. -CONFIG_PREEMPT_COUNT CONFIG_PREEMPT_RCU Redundant with CONFIG_PREEMPT, ignore.
tools/testing/selftests/rcutorture/formal/srcu-cbmc/src/config.h +0 -1
@@ -8,7 +8,6 @@#undef CONFIG_HOTPLUG_CPU#undef CONFIG_MODULES#undef CONFIG_NO_HZ_FULL_SYSIDLE -#undef CONFIG_PREEMPT_COUNT#undef CONFIG_PREEMPT_RCU#undef CONFIG_PROVE_RCU#undef CONFIG_RCU_NOCB_CPU
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 8 lines · Wedson Almeida Filho
For
GFP_ATOMIC, we could usepreempt_countexcept that it isn't always enabled. Conveniently, it is already separated out into its own config. How do people feel about removingCONFIG_PREEMPT_COUNTand having the count always enabled?We would then have a way to reliably detect when we are in atomic context [..]
No.
First off, it's not true. There are non-preempt atomic contexts too, like interrupts disabled etc. Can you enumerate all those? Possibly.
But more importantly, doing "depending on context, I silently and automatically do different things" is simply WRONG. Don't do it. It's a disaster.
Doing that for *debugging* is fine. So having a
WARN_ON_ONCE(in_atomic_context());
is a perfectly fine debug check to find people who do bad bad things, and we have lots of variations of that theme (ie might_sleep(), but also things like lockdep_assert_held() and friends that assert other constraints entirely).
But having *behavior changes* depending on context is a total disaster. And that's invariably why people want this disgusting thing.
They want to do broken things like "I want to allocate memory, and I don't want to care where I am, so I want the memory allocator to just do the whole GFP_ATOMIC for me".
And that is FUNDAMENTALLY BROKEN.
If you want to allocate memory, and you don't want to care about what context you are in, or whether you are holding spinlocks etc, then you damn well shouldn't be doing kernel programming. Not in C, and not in Rust.
It really is that simple. Contexts like this ("I am in a critical region, I must not do memory allocation or use sleeping locks") is *fundamental* to kernel programming. It has nothing to do with the language, and everything to do with the problem space.
So don't go down this "let's have the allocator just know if you're in an atomic context automatically" path. It's wrong. It's complete garbage. It may generate kernel code that superficially "works", but one that is fundamentally broken, and will fail and becaome unreliable under memory pressure.
The thing is, when you do kernel programming, and you're holding a spinlock and need to allocate memory, you generally shouldn't allocate memory at all, you should go "Oh, maybe I need to do the allocation *before* getting the lock".
And it's not just locks. It's also "I'm in a hardware interrupt", but now the solution is fundamentally different. Maybe you still want to do pre-allocation, but now you're a driver interrupt and the pre-allocation has to happen in another code sequence entirely, because obviously the interrupt itself is asynchronous.
But more commonly, you just want to use GFP_ATOMIC, and go "ok, I know the VM layer tries to keep a _limited_ set of pre-allocated buffers around".
But it needs to be explicit, because that GFP_ATOMIC pool of allocations really is very limited, and you as the allocator need to make it *explicit* that yeah, now you're not just doing a random allocation, you are doing one of these *special* allocations that will eat into that very limited global pool of allocations.
So no, you cannot and MUST NOT have an allocator that silently just dips into that special pool, without the user being aware or requesting it.
That really is very very fundamental. Allocators that "just work" in different contexts are broken garbage within the context of a kernel.
Please just accept this, and really *internalize* it. Because this isn't actually just about allocators. Allocators may be one very common special case of this kind of issue, and they come up quite often as a result, but this whole "your code needs to *understand* the special restrictions that the kernel is under" is something that is quite fundamental in general.
It shows up in various other situations too, like "Oh, this code can run in an interrupt handler" (which is *different* from the atomicity of just "while holding a lock", because it implies a level of random nesting that is very relevant for locking).
Or sometimes it's subtler things than just correctness, ie "I'm running under a critical lock, so I must be very careful because there are scalability issues". The code may *work* just fine, but things like tasklist_lock are very very special.
So embrace that kernel requirement. Kernels are special. We do things, and we have constraints that pretty much no other environment has.
It is often what makes kernel programming interesting - because it's challenging. We have a very limited stack. We have some very direct and deep interactions with the CPU, with things like IO and interrupts. Some constraints are always there, and others are context-dependent.
The whole "really know what context this code is running within" is really important. You may want to write very explicit comments about it.
And you definitely should always write your code knowing about it.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 4 lines · Linus Torvalds
The whole "really know what context this code is running within" is really important. You may want to write very explicit comments about it.
Side note: a corollary of this is that people should avoid "dynamic context" things like the plague, because it makes for such pain when the context isn't statically obvious.
So things like conditional locking should generally be avoided as much as humanly possible. Either you take the lock or you don't - don't write code where the lock context depends on some argument value or flag, for example.
Code like this is fine:
if (some_condition) {
spin_lock(&mylock);
xyz();
spin_unlock(&mylock);
}because 'xyz()' is always run in the same context. But avoid patterns like
if (some_condition)
spin_lock(&mylock);
xyz();
if (same_condition)
spin_unlock(&mylock);where now 'xyz()' sometimes does something with the lock held, and sometimes not. That way lies insanity.
Now, obviously, the context for helper functions (like the Rust kernel crate is, pretty much by definition) obviously depends on the context of the callers of said helpers, so in that sense the above kind of "sometimes in locked context, sometimes not" will always be the case.
So those kinds of helper functions will generally need to be either insensitive to context and usable in all contexts (best), or documented - and verify with debug code like 'might_sleep()' - that they only run in certain contexts.
And then in the worst case there's a gfp_flag that says "you can only do these kinds of allocations" or whatever, but even then you should strive to never have other dynamic behavior (ie please try to avoid behavior like having a "already locked" argument and then taking a lock depending on that).
Because if you follow those rules, at least you can statically see the context from a call chain (so, for example, the stack trace of an oops will make the context unambiguous, because there's hopefully no lock or interrupt disabling or similar that has some dynamic behavior like in that second example of "xyz()".
Do we have places in the kernel that do conditional locking? Yes we do. Examples like that second case do exist. It's bad. Sometimes you can't avoid it. But you can always *strive* to avoid it, and minimizing those kinds of "context depends on other things" situations.
And we should strive very hard to make those kinds of contexts very clear and explicit and not dynamic exactly because it's so important in the kernel, and it has subtle implications wrt other locking, and memory allocations.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 10 lines · Linus Torvalds
Quoted 4 lines · Linus Torvalds
The whole "really know what context this code is running within" is really important. You may want to write very explicit comments about it.
Side note: a corollary of this is that people should avoid "dynamic context" things like the plague, because it makes for such pain when the context isn't statically obvious.
As you know, we're trying to guarantee the absence of undefined behaviour for code written in Rust. And the context is _really_ important, so important that leaving it up to comments isn't enough.
I don't care as much about allocation flags as I do about sleeping in an rcu read-side critical region. When CONFIG_PREEMPT=n, if some CPU makes the mistake of sleeping between rcu_read_lock()/rcu_read_unlock(), RCU will take that as a quiescent state, which may cause unsuspecting code waiting for a grace period to wake up too early and potentially free memory that is still in use, which is obviously undefined behaviour.
We generally have two routes to avoid undefined behaviour: detect at compile time (and fail compilation) or at runtime (and stop things before they go too far). The former, while feasible, would require some static analysi or passing tokens as arguments to guarantee that we're in sleepable context when sleeping (all ellided at compile time, so zero-cost in terms of run-time performance), but likely painful to program use.
Always having preempt_count would allow us to detect such issues in RCU at runtime (for both C and Rust) and prevent user-after-frees.
Do you have an opinion on the above?
Cheers,
-Wedson
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 4 lines · Wedson Almeida Filho
As you know, we're trying to guarantee the absence of undefined behaviour for code written in Rust. And the context is _really_ important, so important that leaving it up to comments isn't enough.
You need to realize that
(a) reality trumps fantasy
(b) kernel needs trump any Rust needs
And the *reality* is that there are no absolute guarantees. Ever. The "Rust is safe" is not some kind of absolute guarantee of code safety. Never has been. Anybody who believes that should probably re-take their kindergarten year, and stop believing in the Easter bunny and Santa Claus.
Even "safe" rust code in user space will do things like panic when things go wrong (overflows, allocation failures, etc). If you don't realize that that is NOT some kind of true safely, I don't know what to say.
Not completing the operation at all, is *not* really any better than getting the wrong answer, it's only more debuggable.
In the kernel, "panic and stop" is not an option (it's actively worse than even the wrong answer, since it's really not debugable), so the kernel version of "panic" is "WARN_ON_ONCE()" and continue with the wrong answer.
So this is something that I really *need* the Rust people to understand. That whole reality of "safe" not being some absolute thing, and the reality that the kernel side *requires* slightly different rules than user space traditionally does.
Quoted 6 lines
I don't care as much about allocation flags as I do about sleeping in an rcu read-side critical region. When
CONFIG_PREEMPT=n, if some CPU makes the mistake of sleeping between rcu_read_lock()/rcu_read_unlock(), RCU will take that as a quiescent state, which may cause unsuspecting code waiting for a grace period to wake up too early and potentially free memory that is still in use, which is obviously undefined behaviour.
So?
You had a bug. Shit happens. We have a lot of debugging tools that will give you a *HUGE* warning when said shit happens, including sending automated reports to the distro maker. And then you fix the bug.
Think of that "debugging tools give a huge warning" as being the equivalent of std::panic in standard rust. Yes, the kernel will continue (unless you have panic-on-warn set), because the kernel *MUST* continue in order for that "report to upstream" to have a chance of happening.
So it's technically a veryu different implementation from std:panic, but you should basically see it as exactly that: a *technical* difference, not a conceptual one. The rules for how the kernel deals with bugs is just different, because we don't have core-files and debuggers in the general case.
(And yes, you can have a kernel debugger, and you can just have the WARN_ON_ONCE trigger the debugger, but think of all those billions of devices that are in normal users hands).
And yes, in certain configurations, even those warnings will be turned off because the state tracking isn't done. Again, that's just reality. You don't need to use those configurations yourself if you don't like them, but that does *NOT* mean that you get to say "nobody else gets to use those configurations either".
Deal with it.
Or, you know, if you can't deal with the rules that the kernel requires, then just don't do kernel programming.
Because in the end it really is that simple. I really need you to understand that Rust in the kernel is dependent on *kernel* rules. Not some other random rules that exist elsewhere.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 19 lines · Linus Torvalds
Quoted 4 lines · Wedson Almeida Filho
As you know, we're trying to guarantee the absence of undefined behaviour for code written in Rust. And the context is _really_ important, so important that leaving it up to comments isn't enough.
You need to realize that
(a) reality trumps fantasy
(b) kernel needs trump any Rust needs
And the *reality* is that there are no absolute guarantees. Ever. The "Rust is safe" is not some kind of absolute guarantee of code safety. Never has been. Anybody who believes that should probably re-take their kindergarten year, and stop believing in the Easter bunny and Santa Claus.
Quoted 4 lines
Even "safe" rust code in user space will do things like panic when things go wrong (overflows, allocation failures, etc). If you don't realize that that is NOT some kind of true safely, I don't know what to say.
No one is talking about absolute safety guarantees. I am talking about specific ones that Rust makes: these are well-documented and formally defined.
Quoted 43 lines
Not completing the operation at all, is *not* really any better than getting the wrong answer, it's only more debuggable.
In the kernel, "panic and stop" is not an option (it's actively worse than even the wrong answer, since it's really not debugable), so the kernel version of "panic" is "
WARN_ON_ONCE()" and continue with the wrong answer.So this is something that I really *need* the Rust people to understand. That whole reality of "safe" not being some absolute thing, and the reality that the kernel side *requires* slightly different rules than user space traditionally does.
Quoted 6 lines
I don't care as much about allocation flags as I do about sleeping in an rcu read-side critical region. When
CONFIG_PREEMPT=n, if some CPU makes the mistake of sleeping between rcu_read_lock()/rcu_read_unlock(), RCU will take that as a quiescent state, which may cause unsuspecting code waiting for a grace period to wake up too early and potentially free memory that is still in use, which is obviously undefined behaviour.So?
You had a bug. Shit happens. We have a lot of debugging tools that will give you a *HUGE* warning when said shit happens, including sending automated reports to the distro maker. And then you fix the bug.
Think of that "debugging tools give a huge warning" as being the equivalent of
std::panicin standard rust. Yes, the kernel will continue (unless you have panic-on-warn set), because the kernel *MUST* continue in order for that "report to upstream" to have a chance of happening.So it's technically a veryu different implementation from std:panic, but you should basically see it as exactly that: a *technical* difference, not a conceptual one. The rules for how the kernel deals with bugs is just different, because we don't have core-files and debuggers in the general case.
(And yes, you can have a kernel debugger, and you can just have the
WARN_ON_ONCEtrigger the debugger, but think of all those billions of devices that are in normal users hands).
Quoted 7 lines
And yes, in certain configurations, even those warnings will be turned off because the state tracking isn't done. Again, that's just reality. You don't need to use those configurations yourself if you don't like them, but that does *NOT* mean that you get to say "nobody else gets to use those configurations either".
Deal with it.
While I disagree with some of what you write, the point is taken.
But I won't give up on Rust guarantees just yet, I'll try to find ergonomic ways to enforce them at compile time.
Thanks,
-Wedson
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 4 lines · Wedson Almeida Filho
No one is talking about absolute safety guarantees. I am talking about specific ones that Rust makes: these are well-documented and formally defined.
If you cannot get over the fact that the kernel may have other requirements that trump any language standards, we really can't work together.
Those Rust rules may make sense in other environments. But the kernel really does have hard requirements that you continue to limp along even if some fundamental rule has been violated. Exactly because there's often no separate environment outside the kernel that can deal with it.
End result: a compiler - or language infrastructure - that says "my rules are so ingrained that I cannot do that" is not one that is valid for kernel work.
This is not really any different from the whole notion of "allocation failures cannot panic" that Rust people seemed to readily understand is a major kernel requirement, and that the kernel needed a graceful failure return instead of a hard panic.
Also note that the kernel is perfectly willing to say "I will use compiler flags that disable certain guarantees". We do it all the time.
For example, the C standard has a lot of "the compiler is allowed to make this assumption". And then we disagree with those, and so "kernel C" is different.
For example, the standard says that dereferencing a NULL pointer is undefined behavior, so a C compiler can see a dereference of a pointer to be a guarantee that said pointer isn't NULL, and remove any subsequent NULL pointer tests.
That turns out to be one of those "obviously true in a perfect world, but problematic in a real world with bugs", and we tell the compiler to not do that by passing it the '-fno-delete-null-pointer-checks' flag, because the compiler _depending_ on undefined behavior and changing code generation in the build ends up being a really bad idea from a security standpoint.
Now, in C, most of these kinds of things come from the C standard being very lax, and having much too many "this is undefined behavior" rules. So in almost all cases we end up saying "we want the well-defined implementation, not the 'strictly speaking, the language specs allows the compiler to do Xyz".
Rust comes from a different direction than C, and it may well be that we very much need some of the rules to be relaxed.
And hey, Rust people do know about "sometimes the rules have to be relaxed". When it comes to integer overflows etc, there's a "overflow-checks" flag, typically used for debug vs release builds.
The kernel has similar issues where sometimes you might want the strict checking (lockdep etc), and sometimes you may end up being less strict and miss a few rules (eg "we don't maintain a preempt count for this config, so we can't check RCU mode violations").
Quoted 2 lines
But I won't give up on Rust guarantees just yet, I'll try to find ergonomic ways to enforce them at compile time.
I think that compile-time static checking is wonderful, and as much as possible should be done 100% statically so that people cannot write incorrect programs.
But we all know that static checking is limited, and then the amount of dynamic checking for violations is often something that will have to depend on environment flags, because it may come with an exorbitant price in the checking.
Exactly like integer overflow checking.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
I think there is some amount of talking past each other here.
Rust's rules are that a function that's safe must not exhibit UB, no matter what arguments they're called with. This can be done with static checking or dynamic checking, with obvious trade offs between the two.
We've had pretty good success, thus far, modeling various kernel subsystems with APIs that follow this rule. But when there's not a good way, consistent with the kernel's idioms, to expose a kernel API in Rust that's safe, that doesn't mean it's impossible! In those cases we expose an unsafe fn in Rust. This means that the caller (e.g., driver code) needs to ensure it meets the required pre-conditions for calling that function.
This is how we square the circle of: How do we prioritize the kernel's goals, while also staying consistent with Rust's notion of safety?
Wedson's point is that, when possible, finding ways to expose safe functions is better, since it puts less of a burden on driver authors. But, sadly, we all know we won't be able to find them in all circumstances -- we just want to have it as a goal to find them whenever possible.
Regards,
Alex
Quoted 79 lines · Linus Torvalds
Quoted 4 lines · Wedson Almeida Filho
No one is talking about absolute safety guarantees. I am talking about specific ones that Rust makes: these are well-documented and formally defined.
If you cannot get over the fact that the kernel may have other requirements that trump any language standards, we really can't work together.
Those Rust rules may make sense in other environments. But the kernel really does have hard requirements that you continue to limp along even if some fundamental rule has been violated. Exactly because there's often no separate environment outside the kernel that can deal with it.
End result: a compiler - or language infrastructure - that says "my rules are so ingrained that I cannot do that" is not one that is valid for kernel work.
This is not really any different from the whole notion of "allocation failures cannot panic" that Rust people seemed to readily understand is a major kernel requirement, and that the kernel needed a graceful failure return instead of a hard panic.
Also note that the kernel is perfectly willing to say "I will use compiler flags that disable certain guarantees". We do it all the time.
For example, the C standard has a lot of "the compiler is allowed to make this assumption". And then we disagree with those, and so "kernel C" is different.
For example, the standard says that dereferencing a NULL pointer is undefined behavior, so a C compiler can see a dereference of a pointer to be a guarantee that said pointer isn't NULL, and remove any subsequent NULL pointer tests.
That turns out to be one of those "obviously true in a perfect world, but problematic in a real world with bugs", and we tell the compiler to not do that by passing it the '-fno-delete-null-pointer-checks' flag, because the compiler _depending_ on undefined behavior and changing code generation in the build ends up being a really bad idea from a security standpoint.
Now, in C, most of these kinds of things come from the C standard being very lax, and having much too many "this is undefined behavior" rules. So in almost all cases we end up saying "we want the well-defined implementation, not the 'strictly speaking, the language specs allows the compiler to do Xyz".
Rust comes from a different direction than C, and it may well be that we very much need some of the rules to be relaxed.
And hey, Rust people do know about "sometimes the rules have to be relaxed". When it comes to integer overflows etc, there's a "overflow-checks" flag, typically used for debug vs release builds.
The kernel has similar issues where sometimes you might want the strict checking (lockdep etc), and sometimes you may end up being less strict and miss a few rules (eg "we don't maintain a preempt count for this config, so we can't check RCU mode violations").
Quoted 2 lines
But I won't give up on Rust guarantees just yet, I'll try to find ergonomic ways to enforce them at compile time.
I think that compile-time static checking is wonderful, and as much as possible should be done 100% statically so that people cannot write incorrect programs.
But we all know that static checking is limited, and then the amount of dynamic checking for violations is often something that will have to depend on environment flags, because it may come with an exorbitant price in the checking.
Exactly like integer overflow checking.
Linus
Signature · 1 line
All that is necessary for evil to succeed is for good people to do nothing.
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 5 lines · Alex Gaynor
Rust's rules are that a function that's safe must not exhibit UB, no matter what arguments they're called with. This can be done with static checking or dynamic checking, with obvious trade offs between the two.
I think you are missing just how many things are "unsafe" in certain contexts and *cannot* be validated.
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Those things are simply not "safe" if you hold a spinlock, or if you are in a RCU read-locked region.
And there is literally no way to check for it in certain configurations. None.
So are you going to mark every single function that takes a mutex as being "unsafe"?
Or are you just going to accept and understand that "hey, exactly like with integer overflows, sometimes it will be checked, and sometimes it just won't be".
Because that is literally the reality of the kernel. Sometimes you WILL NOT have the checks, and you literally CANNOT have the checks.
This is just how reality is. You don't get to choose the universe you live in.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 5 lines · Linus Torvalds
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Examples of "number of other things" ends up being things like "accessing user memory", which depending on what you are doing may be very common too.
And btw, it's not only about the (multiple kinds of) atomic regions.
We have other context rules in the kernel too, like "does floating point or vector unit calculations". Which you can actually do, but only in a kernel_fpu_begin/kernel_fpu_end region.
Now, the floating point thing is rare enough that it's probably fine to just say "no floating point at all in Rust code". It tends to be very special code, so you'd write it in C or inline assembly, because you're doing special things like using the vector unit for crypto hashes using special CPU instructions.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 18 lines · Linus Torvalds
Quoted 5 lines · Linus Torvalds
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Examples of "number of other things" ends up being things like "accessing user memory", which depending on what you are doing may be very common too.
And btw, it's not only about the (multiple kinds of) atomic regions.
We have other context rules in the kernel too, like "does floating point or vector unit calculations". Which you can actually do, but only in a kernel_fpu_begin/kernel_fpu_end region.
Quoted 5 lines
Now, the floating point thing is rare enough that it's probably fine to just say "no floating point at all in Rust code". It tends to be very special code, so you'd write it in C or inline assembly, because you're doing special things like using the vector unit for crypto hashes using special CPU instructions.
I just want to point out that there are ways of representing things like the context you are running in during compile time. I won't argue they are necessarily practical, but there are ways and by exploring those ways some practical solutions may result.
Instead of saying:spin_lock(&lock);do_something();spin_unlock(&lock);
It is possible to say:with_spin_lock(&lock, do_something());
This can be taken a step farther and with_spin_lock can pass a ``token'' say a structure with no members that disappears at compile time that let's the code know it has the spinlock held.
In C I would do:
struct have_spin_lock_x {
// nothing
};do_something(struct have_spin_lock_x context_guarantee)
{
...;
}I think most of the special contexts in the kernel can be represented in a similar manner. A special parameter that can be passed and will compile out.
I don't recall seeing anything like that tried in the kernel so I don't know if it makes sense or if it would get too wordy to live, but it is possible. If passing a free context parameter is not too wordy it would catch silly errors, and would hopefully leave more mental space for developers to pay attention to the other details of the problems they are solving.
*Shrug* I don't know if rust allows for free parameters like that and if it does I don't know if it would be cheap enough to live.
Eric
Gary Guo
reviewerRe: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 64 lines · On Tue, 20 Sep 2022 10:55:27 -0500 "Eric W. Biederman" <[email protected]>…
Quoted 18 lines · Linus Torvalds
Quoted 5 lines · Linus Torvalds
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Examples of "number of other things" ends up being things like "accessing user memory", which depending on what you are doing may be very common too.
And btw, it's not only about the (multiple kinds of) atomic regions.
We have other context rules in the kernel too, like "does floating point or vector unit calculations". Which you can actually do, but only in a kernel_fpu_begin/kernel_fpu_end region.
Quoted 5 lines
Now, the floating point thing is rare enough that it's probably fine to just say "no floating point at all in Rust code". It tends to be very special code, so you'd write it in C or inline assembly, because you're doing special things like using the vector unit for crypto hashes using special CPU instructions.
I just want to point out that there are ways of representing things like the context you are running in during compile time. I won't argue they are necessarily practical, but there are ways and by exploring those ways some practical solutions may result.
Instead of saying:
spin_lock(&lock);do_something();spin_unlock(&lock);It is possible to say:
with_spin_lock(&lock, do_something());This can be taken a step farther and
with_spin_lockcan pass a ``token'' say a structure with no members that disappears at compile time that let's the code know it has the spinlock held.In C I would do:
struct have_spin_lock_x { // nothing };
do_something(struct have_spin_lock_x context_guarantee){ ...; }I think most of the special contexts in the kernel can be represented in a similar manner. A special parameter that can be passed and will compile out.
I don't recall seeing anything like that tried in the kernel so I don't know if it makes sense or if it would get too wordy to live, but it is possible. If passing a free context parameter is not too wordy it would catch silly errors, and would hopefully leave more mental space for developers to pay attention to the other details of the problems they are solving.
Quoted 2 lines
*Shrug* I don't know if rust allows for free parameters like that and if it does I don't know if it would be cheap enough to live.
I believe this was mentioned by Wedson in one of his previous emails. This pattern is quite common in Rust code. It looks like this:
#[derive(Clone, Copy)]
pub struct Token<'a>(PhantomData<&'a ()>);
pub fn with_token<T>(f: impl for<'a> FnOnce(Token<'a>) -> T) ->
T { f(Token(PhantomData))
}Any function that requires something can just take token by value, e.g. with token: Token<'_>. Since Token is a zero-sized type (ZST), this parameter will be omitted during code generation, so it won't affect the ABI and this has no runtime cost.
Example on godbolt: https://godbolt.org/z/9n954cG4d, showing that the token is actually all optimised out.
It should be noted however, atomic context is not something that a token can represent. You can only use tokens to restrict what you *can* do, but not what you *can't* do. There is no negative reasoning with tokens, you can't create a function that can only be called when you don't have token.
You can use tokens to represent non-atomic contexts, but that'll be really painful because this requires carrying a token in almost all functions. This kind of API also works well for FPU contexts.
Best,
Gary
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 9 lines · Gary Guo
It should be noted however, atomic context is not something that a token can represent. You can only use tokens to restrict what you *can* do, but not what you *can't* do. There is no negative reasoning with tokens, you can't create a function that can only be called when you don't have token.
On the other hand, it ought to be feasible to implement that kind of ’negative reasoning' as a custom lint. It might not work as well as something built into the language, but it should work decently well, and could serve as a prototype for a future built-in feature.
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 10 lines · comex
Quoted 9 lines · Gary Guo
It should be noted however, atomic context is not something that a token can represent. You can only use tokens to restrict what you *can* do, but not what you *can't* do. There is no negative reasoning with tokens, you can't create a function that can only be called when you don't have token.
Quoted 4 lines
On the other hand, it ought to be feasible to implement that kind of ’negative reasoning' as a custom lint. It might not work as well as something built into the language, but it should work decently well, and could serve as a prototype for a future built-in feature.
Interesting, do you have an example somewhere?
Regards,
Boqun
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 10 lines
Quoted 5 lines
On the other hand, it ought to be feasible to implement that kind of ’negative reasoning' as a custom lint. It might not work as well as something built into the language, but it should work decently well, and could serve as a prototype for a future built-in feature.
Interesting, do you have an example somewhere?
Regards,
Boqun
After some searching, I found this, which someone wrote several years ago for a very similar purpose:
https://github.com/thepowersgang/tag_safe/
Quoted 9 lines
This is a linter designed originally for use with a kernel, where functions need to be marked as "IRQ safe" (meaning they are safe to call within an IRQ handler, and handle the case where they may interrupt themselves).
If a function is annotated with #[req_safe(ident)] (where ident can be anything, and defines the type of safety) this linter will check that all functions called by that function are either annotated with the same annotation or #[is_safe(ident)], OR they do not call functions with the reverse #[is_unsafe(ident)] annotation.
Note that the code won't work as-is with recent rustc. rustc's API for custom lints is not stable, and in fact rustc has deprecated linter plugins entirely [1], though there are alternative approaches to using custom lints [2]. Still, it's a good example of the approach.
One fundamental caveat is that it doesn't seem to have the sophistication needed to be sound with respect to indirect calls.
For example, suppose you have a function that fetches a callback from some structure and calls it. Whether this function is IRQ-safe depends on whether the callback is expected to be IRQ-safe, so in order to safety-check this, you would need an annotation on either the callback field or the function pointer type. This is more complex than just putting annotations on function definitions.
Or suppose you have the following code:
fn foo() {
bar(|| do_something_not_irq_safe());
}If foo is expected to be IRQ-safe, this may or may not be sound, depending on whether bar calls the callback immediately or saves it for later. If bar saves it for later, then it could be marked unconditionally IRQ-safe. But if bar calls it immediately, then it's neither IRQ-safe nor IRQ-unsafe, but effectively generic over IRQ safety. You could pessimistically mark it IRQ-unsafe, but Rust has tons of basic helper methods that accept callbacks and call them immediately; not being able to use any of them in an IRQ-safe context would be quite limiting.
In short, a fully sound approach requires not just checking which functions call which, but having some kind of integration with the type system. This is the kind of issue that I was thinking of when I said a custom lint may not work as well as something built into the language.
However, I do think it's *possible* to handle it soundly from a lint, especially if it focuses on typical use cases and relies on manual annotations for the rest. Alternately, even an unsound lint would be a good first step. It wouldn't really comport with Rust's ethos of making safety guarantees ironclad rather than heuristic, but it would serve as a good proof of concept for a future language feature, while likely being helpful in practice in the short term.
[1] https://github.com/rust-lang/rust/pull/64675/files
[2] https://www.trailofbits.com/post/write-rust-lints-without-forking-clippy
Re: [PATCH v9 12/27] rust: add `kernel` crate
In reply to Linus TorvaldsQuoted 15 lines · Linus Torvalds
Quoted 5 lines · Alex Gaynor
Rust's rules are that a function that's safe must not exhibit UB, no matter what arguments they're called with. This can be done with static checking or dynamic checking, with obvious trade offs between the two.
I think you are missing just how many things are "unsafe" in certain contexts and *cannot* be validated.
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Quoted 8 lines
Those things are simply not "safe" if you hold a spinlock, or if you are in a RCU read-locked region.
And there is literally no way to check for it in certain configurations. None.
So are you going to mark every single function that takes a mutex as being "unsafe"?
In this early experiment stage, if something is unsafe per Rust safety requirements, maybe we should mark it as "unsafe"? Not because Rust safety needs trump kernel needs, but because we need showcases to the Rust langauge people the things that are not working very well in Rust today.
I definitely agree that we CANNOT change kernel behaviors to solely fulfil Rust safety requirements, we (Rust-for-Linux people) should either find a way to check in compiler time or just mark it as "unsafe".
Maybe I'm naive ;-) But keeping Rust safety requirements as they are helps communication with the people on the other side (Rust langauge/compiler): "Hey, I did everything per your safety requirements, and it ends like this, I'm not happy about it, could you figure out something helpful? After all, Rust is a *system programming" language, it should be able to handle things like these".
Or we want to say "kernel is special, so please give me a option so that I don't need to worry about these UBs and deal with my real problems"? I don't have the first hand experience, but seems this is what we have been doing with C for many years. Do we want to try a new strategy? ;-) But perhaps it's not new, maybe it's done a few times already but didn't end well..
Anyway, if I really want to teach Rust language/compiler people "I know what I'm doing, the problem is that the language is not ready". What should I do?
Regards,
Boqun
Quoted 10 lines
Or are you just going to accept and understand that "hey, exactly like with integer overflows, sometimes it will be checked, and sometimes it just won't be".
Because that is literally the reality of the kernel. Sometimes you WILL NOT have the checks, and you literally CANNOT have the checks.
This is just how reality is. You don't get to choose the universe you live in.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
I don't know if there was ever a direct answer to this question; I was hoping someone with more involvement in the project would jump in.
Quoted 9 lines
In this early experiment stage, if something is unsafe per Rust safety requirements, maybe we should mark it as "unsafe"? Not because Rust safety needs trump kernel needs, but because we need showcases to the Rust langauge people the things that are not working very well in Rust today.
I definitely agree that we CANNOT change kernel behaviors to solely fulfil Rust safety requirements, we (Rust-for-Linux people) should either find a way to check in compiler time or just mark it as "unsafe".
From the perspective of a Linux outsider, marking huge swaths of Rust code "unsafe" doesn't actually sound incorrect to me. If a function may exhibit undefined behavior, it's unsafe; thus, it should be marked as such. True, you lose some of Rust's guarantees within the code marked "unsafe", but this is what permits Rust to enforce those guarantees elsewhere. Having lots of uses of "unsafe" in kernel code is probably to be expected, and (in my opinion) not actually a sign that things "are not working very well in Rust." It's not even particularly verbose, if the callers of unsafe functions are generally also marked "unsafe"; in these cases, only the signatures are affected.
The last sentence of Boqun's quote above captures the intent of "unsafe" perfectly: it simply indicates that code cannot be proven at compile time not to exhibit undefined behavior. It's important for systems languages to enable such code to be written, which is precisely why Rust has the "unsafe" keyword.
Quoted 67 lines · Boqun Feng
Quoted 15 lines · Linus Torvalds
Quoted 5 lines · Alex Gaynor
Rust's rules are that a function that's safe must not exhibit UB, no matter what arguments they're called with. This can be done with static checking or dynamic checking, with obvious trade offs between the two.
I think you are missing just how many things are "unsafe" in certain contexts and *cannot* be validated.
This is not some kind of "a few special things".
This is things like absolutely _anything_ that allocates memory, or takes a lock, or does a number of other things.
Quoted 8 lines
Those things are simply not "safe" if you hold a spinlock, or if you are in a RCU read-locked region.
And there is literally no way to check for it in certain configurations. None.
So are you going to mark every single function that takes a mutex as being "unsafe"?
In this early experiment stage, if something is unsafe per Rust safety requirements, maybe we should mark it as "unsafe"? Not because Rust safety needs trump kernel needs, but because we need showcases to the Rust langauge people the things that are not working very well in Rust today.
I definitely agree that we CANNOT change kernel behaviors to solely fulfil Rust safety requirements, we (Rust-for-Linux people) should either find a way to check in compiler time or just mark it as "unsafe".
Maybe I'm naive ;-) But keeping Rust safety requirements as they are helps communication with the people on the other side (Rust langauge/compiler): "Hey, I did everything per your safety requirements, and it ends like this, I'm not happy about it, could you figure out something helpful? After all, Rust is a *system programming" language, it should be able to handle things like these".
Or we want to say "kernel is special, so please give me a option so that I don't need to worry about these UBs and deal with my real problems"? I don't have the first hand experience, but seems this is what we have been doing with C for many years. Do we want to try a new strategy? ;-) But perhaps it's not new, maybe it's done a few times already but didn't end well..
Anyway, if I really want to teach Rust language/compiler people "I know what I'm doing, the problem is that the language is not ready". What should I do?
Regards,
BoqunQuoted 10 lines
Or are you just going to accept and understand that "hey, exactly like with integer overflows, sometimes it will be checked, and sometimes it just won't be".
Because that is literally the reality of the kernel. Sometimes you WILL NOT have the checks, and you literally CANNOT have the checks.
This is just how reality is. You don't get to choose the universe you live in.
Linus
Re: [PATCH v9 12/27] rust: add `kernel` crate
In reply to Linus TorvaldsQuoted 16 lines · Linus Torvalds
Quoted 4 lines · Wedson Almeida Filho
No one is talking about absolute safety guarantees. I am talking about specific ones that Rust makes: these are well-documented and formally defined.
If you cannot get over the fact that the kernel may have other requirements that trump any language standards, we really can't work together.
Those Rust rules may make sense in other environments. But the kernel really does have hard requirements that you continue to limp along even if some fundamental rule has been violated. Exactly because there's often no separate environment outside the kernel that can deal with it.
Quoted 8 lines
End result: a compiler - or language infrastructure - that says "my rules are so ingrained that I cannot do that" is not one that is valid for kernel work.
This is not really any different from the whole notion of "allocation failures cannot panic" that Rust people seemed to readily understand is a major kernel requirement, and that the kernel needed a graceful failure return instead of a hard panic.
I am not a programming language/compiler person. My background is in kernel programming (Linux is not the only kernel in existence) so I have no difficulty whatsoever with kernel requirements; the graceful handling of allocation failures that you brought up is in fact something I added soon after I started started working on rust-for-linux and realised (in admittedly a bit of a shock) that userland Rust didn't have this at the time.
Quoted 21 lines
Also note that the kernel is perfectly willing to say "I will use compiler flags that disable certain guarantees". We do it all the time.
For example, the C standard has a lot of "the compiler is allowed to make this assumption". And then we disagree with those, and so "kernel C" is different.
For example, the standard says that dereferencing a NULL pointer is undefined behavior, so a C compiler can see a dereference of a pointer to be a guarantee that said pointer isn't NULL, and remove any subsequent NULL pointer tests.
That turns out to be one of those "obviously true in a perfect world, but problematic in a real world with bugs", and we tell the compiler to not do that by passing it the '-fno-delete-null-pointer-checks' flag, because the compiler _depending_ on undefined behavior and changing code generation in the build ends up being a really bad idea from a security standpoint.
Quoted 8 lines
Now, in C, most of these kinds of things come from the C standard being very lax, and having much too many "this is undefined behavior" rules. So in almost all cases we end up saying "we want the well-defined implementation, not the 'strictly speaking, the language specs allows the compiler to do Xyz".
Rust comes from a different direction than C, and it may well be that we very much need some of the rules to be relaxed.
Sure. In fact, we are likely to be able to influence the Rust language more and more quickly than C. So if things don't make sense, we may be able to change them.
There are also opportunities here, for example, a compatible memory model and guaranteed honouring of dependency chains for more efficient synchronisation primitives. I'm not claiming this is surely going to happen or that it's easy, just that there's an opportunity to do better than C here.
Quoted 12 lines
And hey, Rust people do know about "sometimes the rules have to be relaxed". When it comes to integer overflows etc, there's a "overflow-checks" flag, typically used for debug vs release builds.
The kernel has similar issues where sometimes you might want the strict checking (lockdep etc), and sometimes you may end up being less strict and miss a few rules (eg "we don't maintain a preempt count for this config, so we can't check RCU mode violations").
Quoted 2 lines
But I won't give up on Rust guarantees just yet, I'll try to find ergonomic ways to enforce them at compile time.
Quoted 8 lines
I think that compile-time static checking is wonderful, and as much as possible should be done 100% statically so that people cannot write incorrect programs.
But we all know that static checking is limited, and then the amount of dynamic checking for violations is often something that will have to depend on environment flags, because it may come with an exorbitant price in the checking.
If we can't find an ergonomic way to enforce safety guarantees, we have fallbacks like unsafe functions or runtime enforcement. And as you point out, if the runtime cost is too high in certain scenarios, we do allow users to give up safety with a config as in the integer overflow case you mentioned. We do try to minimise those.
Cheers,
-Wedson
Re: [PATCH v9 12/27] rust: add `kernel` crate
In reply to Miguel OjedaQuoted 10 lines · Miguel Ojeda
Quoted 3 lines · Konstantin Shelekhin
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's
alloccould support our use cases, etc. has been on discussion for a long time.
Quoted 5 lines
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;
In my opinion, the rest of the thread clearly shows that the conservative approach is currently the only solid option. I suggest the following explicit API:
let a = Box::try_new(size, flags)?; Vec::try_push(item, flags)?;
etc. Whadda you think?
Re: [PATCH v9 12/27] rust: add `kernel` crate
Quoted 17 lines · Konstantin Shelekhin
Quoted 10 lines · Miguel Ojeda
Quoted 3 lines · Konstantin Shelekhin
I sense possible problems here. It's common for a kernel code to pass flags during memory allocations.
Yes, of course. We will support this, but how exactly it will look like, to what extent upstream Rust's
alloccould support our use cases, etc. has been on discussion for a long time.Quoted 5 lines
For instance, see https://github.com/Rust-for-Linux/linux/pull/815 for a potential extension trait approach with no allocator carried on the type that Andreas wrote after a discussion in the last informal call:
let a = Box::try_new_atomic(101)?;
Quoted 8 lines
In my opinion, the rest of the thread clearly shows that the conservative approach is currently the only solid option. I suggest the following explicit API:
let a = Box::try_new(size, flags)?; Vec::try_push(item, flags)?;etc. Whadda you think?
Please, yes. This fits the current kernel memory allocation pattern and allows for proper propagation of the allocation flags as needed through the system. This is going to be required in any non-trivial kernel code anyway, might as well do it correct from the beginning.
It also allows for flags to change over time, which also happens.
thanks,
greg k-h
Miguel Ojeda
maintainerPATCH v9 13/27 rust: export generated symbols
In reply to Miguel OjedaAll symbols are reexported reusing the EXPORT_SYMBOL_GPL macro from C. The lists of symbols are generated on the fly.
There are three main sets of symbols to distinguish:
- The ones from the core and alloc crates (from the Rust standard library). The code is licensed as Apache/MIT.
- The ones from our abstractions in the kernel crate.
- The helpers (already exported since they are not generated).
We export everything as GPL. This ensures we do not mistakenly expose GPL kernel symbols/features as non-GPL, even indirectly.
--- rust/exports.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 rust/exports.c
rust/exports.c new file +21 -0
@@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* A hack to export Rust symbols for loadable modules without having to redo +* the entire `include/linux/export.h` logic in Rust. +* +* This requires the Rust's new/future `v0` mangling scheme because the default +* one ("legacy") uses invalid characters for C identifiers (thus we cannot use +* the `EXPORT_SYMBOL_*` macros). +* +* All symbols are exported as GPL-only to guarantee no GPL-only feature is +* accidentally exposed. +*/ + +#include <linux/module.h> + +#define EXPORT_SYMBOL_RUST_GPL (sym) extern int sym; EXPORT_SYMBOL_GPL(sym) + +#include "exports_core_generated.h" +#include "exports_alloc_generated.h" +#include "exports_bindings_generated.h" +#include "exports_kernel_generated.h"
Signature · 1 line
2.37.1
Re: [PATCH v9 13/27] rust: export generated symbols
Quoted 10 lines · Miguel Ojeda
All symbols are reexported reusing the
EXPORT_SYMBOL_GPLmacro from C. The lists of symbols are generated on the fly.There are three main sets of symbols to distinguish:
- The ones from the
coreandalloccrates (from the Rust standard library). The code is licensed as Apache/MIT.- The ones from our abstractions in the
kernelcrate.
Quoted 7 lines
- The helpers (already exported since they are not generated).
We export everything as GPL. This ensures we do not mistakenly expose GPL kernel symbols/features as non-GPL, even indirectly.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 14/27 vsprintf: add new `%pA` format specifier
In reply to Miguel OjedaFrom: Gary Guo <[email protected]>
This patch adds a format specifier %pA to vsprintf which formats a pointer as core::fmt::Arguments. Doing so allows us to directly format to the internal buffer of printf, so we do not have to use a temporary buffer on the stack to pre-assemble the message on the Rust side.
This specifier is intended only to be used from Rust and not for C, so checkpatch.pl is intentionally unchanged to catch any misuse.
--- Documentation/core-api/printk-formats.rst | 10 ++++++++++ lib/vsprintf.c | 13 +++++++++++++ 2 files changed, 23 insertions(+)
Documentation/core-api/printk-formats.rst +10 -0
@@ -625,6 +625,16 @@ Examples:: %p4cc Y10 little-endian (0x20303159) %p4cc NV12 big-endian (0xb231564e) +Rust +---- + +:: + + %pA + +Only intended to be used from Rust code to format ``core::fmt::Arguments``. +Do *not* use it from C. + Thanks ======
lib/vsprintf.c +13 -0
@@ -2246,6 +2246,9 @@ int __init no_hash_pointers_enable(char *str) }early_param ("no_hash_pointers" , no_hash_pointers_enable); +/* Used for Rust formatting ('%pA'). */ +char *rust_fmt_argument (char *buf,char *end,void *ptr); +/* * Show a '%p' thing. A kernel extension is that the '%p' is followed * by an extra set of alphanumeric characters that are extended format
@@ -2372,6 +2375,10 @@ early_param("no_hash_pointers", no_hash_pointers_enable);* * Note: The default behaviour (unadorned %p) is to hash the address, * rendering it useful as a unique identifier. +* +* There is also a '%pA' format specifier, but it is only intended to be used +* from Rust code to format core::fmt::Arguments. Do *not* use it from C. +* See rust/kernel/print.rs for details. */ static noinline_for_stack char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -2444,6 +2451,12 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,return device_node_string (buf, end, ptr, spec, fmt +1 );case 'f' :return fwnode_string (buf, end, ptr, spec, fmt +1 ); +case 'A' : +if (!IS_ENABLED (CONFIG_RUST )) { +WARN_ONCE (1 ,"Please remove %%pA from non-Rust code\n" ); +return error_string (buf, end,"(%pA?)" , spec); + } +return rust_fmt_argument (buf, end, ptr);case 'x' :return pointer_string (buf, end, ptr, spec);case 'e' :
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 15/27 scripts: checkpatch: diagnose uses of `%pA` in the C side as errors
In reply to Miguel OjedaThe %pA format specifier is only intended to be used from Rust.
checkpatch.pl already gives a warning for invalid specificers:
WARNING: Invalid vsprintf pointer extension '%pA'
This makes it an error and introduces an explanatory message:
ERROR: Invalid vsprintf pointer extension '%pA' - '%pA' is only intended to be used from Rust code
--- scripts/checkpatch.pl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +6 -2
@@ -6782,15 +6782,19 @@ sub process { } if ($bad_specifier ne "") { my $stat_real = get_stat_real($linenr, $lc); + my $msg_level = \&WARN; my $ext_type = "Invalid"; my $use = ""; if ($bad_specifier =~ /p[Ff]/) { $use = " - use %pS instead"; $use =~ s/pS/ps/ if ($bad_specifier =~ /pf/); + } elsif ($bad_specifier =~ /pA/) { + $use = " - '%pA' is only intended to be used from Rust code"; + $msg_level = \&ERROR; } - WARN("VSPRINTF_POINTER_EXTENSION", - "$ext_type vsprintf pointer extension '$bad_specifier'$use\n" . "$here\n$stat_real\n"); + &{$msg_level}("VSPRINTF_POINTER_EXTENSION", + "$ext_type vsprintf pointer extension '$bad_specifier'$use\n" . "$here\n$stat_real\n"); } } }
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 16/27 scripts: checkpatch: enable language-independent checks for Rust
In reply to Miguel OjedaInclude Rust in the "source code files" category, so that the language-independent tests are checked for Rust too, and teach checkpatch about the comment style for Rust files.
This enables the malformed SPDX check, the misplaced SPDX license tag check, the long line checks, the lines without a newline check and the embedded filename check.
--- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +2 -2
@@ -3615,7 +3615,7 @@ sub process { my $comment = ""; if ($realfile =~ /\.(h|s|S)$/) { $comment = '/*'; - } elsif ($realfile =~ /\.(c|dts|dtsi)$/) { + } elsif ($realfile =~ /\.(c|rs|dts|dtsi)$/) { $comment = '//'; } elsif (($checklicenseline == 2) || $realfile =~ /\.(sh|pl|py|awk|tc|yaml)$/) { $comment = '#';
@@ -3663,7 +3663,7 @@ sub process { } # check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); + next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/); # check for using SPDX-License-Identifier on the wrong line number if ($realline != $checklicenseline &&
Signature · 1 line
2.37.1
Re: [PATCH v9 16/27] scripts: checkpatch: enable language-independent checks for Rust
Quoted 10 lines · Miguel Ojeda
Include Rust in the "source code files" category, so that the language-independent tests are checked for Rust too, and teach
checkpatchabout the comment style for Rust files.This enables the malformed SPDX check, the misplaced SPDX license tag check, the long line checks, the lines without a newline check and the embedded filename check.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 17/27 scripts: decode_stacktrace: demangle Rust symbols
In reply to Miguel OjedaRecent versions of both Binutils (c++filt) and LLVM (llvm-cxxfilt) provide Rust v0 mangling support.
--- scripts/decode_stacktrace.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
scripts/decode_stacktrace.sh +14 -0
@@ -8,6 +8,14 @@ usage() { echo " $0 -r <release> | <vmlinux> [<base path>|auto] [<modules path>]" } +# Try to find a Rust demangler +if type llvm-cxxfilt >/dev/null 2>&1 ; then + cppfilt=llvm-cxxfilt +elif type c++filt >/dev/null 2>&1 ; then + cppfilt=c++filt + cppfilt_opts=-i +fi + if [[ $1 == "-r" ]] ; then vmlinux="" basepath="auto"
@@ -180,6 +188,12 @@ parse_symbol() { # In the case of inlines, move everything to same line code=${code//$'\n'/' '} + # Demangle if the name looks like a Rust symbol and if + # we got a Rust demangler + if [[ $name =~ ^_R && $cppfilt != "" ]] ; then + name=$("$cppfilt" "$cppfilt_opts" "$name") + fi + # Replace old address with pretty line numbers symbol="$segment$name ($code)" }
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 18/27 scripts: add `generate_rust_analyzer.py`
In reply to Miguel OjedaThe generate_rust_analyzer.py script generates the configuration file (rust-project.json) for rust-analyzer.
rust-analyzer is a modular compiler frontend for the Rust language. It provides an LSP server which can be used in editors such as VS Code, Emacs or Vim.
--- .gitignore | 3 + scripts/generate_rust_analyzer.py | 135 ++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100755 scripts/generate_rust_analyzer.py
.gitignore +3 -0
@@ -162,3 +162,6 @@ x509.genkey # Documentation toolchain sphinx_*/ + +# Rust analyzer configuration +/rust-project.json
scripts/generate_rust_analyzer.py new file +135 -0
135 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [PATCH v9 18/27] scripts: add `generate_rust_analyzer.py`
Quoted 9 lines · Miguel Ojeda
The
generate_rust_analyzer.pyscript generates the configuration file (rust-project.json) for rust-analyzer.rust-analyzer is a modular compiler frontend for the Rust language. It provides an LSP server which can be used in editors such as VS Code, Emacs or Vim.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 19/27 scripts: add `generate_rust_target.rs`
In reply to Miguel OjedaThis script takes care of generating the custom target specification file for rustc, based on the kernel configuration.
It also serves as an example of a Rust host program.
A dummy architecture is kept in this patch so that a later patch adds x86 support on top with as few changes as possible.
--- scripts/.gitignore | 1 + scripts/generate_rust_target.rs | 171 ++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 scripts/generate_rust_target.rs
scripts/.gitignore +1 -0
@@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only /asn1_compiler /bin2c +/generate_rust_target /insert-sys-cert /kallsyms /module.lds
scripts/generate_rust_target.rs new file +171 -0
171 lines not on this page · raw message
Signature · 1 line
2.37.1
Re: [PATCH v9 19/27] scripts: add `generate_rust_target.rs`
Quoted 10 lines · Miguel Ojeda
This script takes care of generating the custom target specification file for
rustc, based on the kernel configuration.It also serves as an example of a Rust host program.
A dummy architecture is kept in this patch so that a later patch adds x86 support on top with as few changes as possible.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 20/27 scripts: add `rust_is_available.sh`
In reply to Miguel OjedaThis script tests whether the Rust toolchain requirements are in place to enable Rust support.
The build system will call it to set CONFIG_RUST_IS_AVAILABLE in a later patch.
It also has an option (-v) to explain what is missing, which is useful to set up the development environment. This is used via the make rustavailable target added in a later patch.
--- scripts/rust_is_available.sh | 160 +++++++++++++++++++ scripts/rust_is_available_bindgen_libclang.h | 2 + 2 files changed, 162 insertions(+) create mode 100755 scripts/rust_is_available.sh create mode 100644 scripts/rust_is_available_bindgen_libclang.h
scripts/rust_is_available.sh new file +160 -0
@@ -0,0 +1,160 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Tests whether a suitable Rust toolchain is available. +# +# Pass `-v` for human output and more checks (as warnings). + +set -e + +min_tool_version=$(dirname $0)/min-tool-version.sh + +# Convert the version string x.y.z to a canonical up-to-7-digits form. +# +# Note that this function uses one more digit (compared to other +# instances in other version scripts) to give a bit more space to +# `rustc` since it will reach 1.100.0 in late 2026. +get_canonical_version() +{ + IFS=. + set -- $1 + echo $((100000 * $1 + 100 * $2 + $3)) +} + +# Check that the Rust compiler exists. +if ! command -v "$RUSTC" >/dev/null; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' could not be found." + echo >&2 "***" + fi + exit 1 +fi + +# Check that the Rust bindings generator exists. +if ! command -v "$BINDGEN" >/dev/null; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' could not be found." + echo >&2 "***" + fi + exit 1 +fi + +# Check that the Rust compiler version is suitable. +# +# Non-stable and distributions' versions may have a version suffix, e.g. `-dev`. +rust_compiler_version=$( \ + LC_ALL=C "$RUSTC" --version 2>/dev/null \ + | head -n 1 \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ +) +rust_compiler_min_version=$($min_tool_version rustc) +rust_compiler_cversion=$(get_canonical_version $rust_compiler_version) +rust_compiler_min_cversion=$(get_canonical_version $rust_compiler_min_version) +if [ "$rust_compiler_cversion" -lt "$rust_compiler_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' is too old." + echo >&2 "*** Your version: $rust_compiler_version" + echo >&2 "*** Minimum version: $rust_compiler_min_version" + echo >&2 "***" + fi + exit 1 +fi +if [ "$1" = -v ] && [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' is too new. This may or may not work." + echo >&2 "*** Your version: $rust_compiler_version" + echo >&2 "*** Expected version: $rust_compiler_min_version" + echo >&2 "***" +fi + +# Check that the Rust bindings generator is suitable. +# +# Non-stable and distributions' versions may have a version suffix, e.g. `-dev`. +rust_bindings_generator_version=$( \ + LC_ALL=C "$BINDGEN" --version 2>/dev/null \ + | head -n 1 \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ +) +rust_bindings_generator_min_version=$($min_tool_version bindgen) +rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version) +rust_bindings_generator_min_cversion=$(get_canonical_version $rust_bindings_generator_min_version) +if [ "$rust_bindings_generator_cversion" -lt "$rust_bindings_generator_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' is too old." + echo >&2 "*** Your version: $rust_bindings_generator_version" + echo >&2 "*** Minimum version: $rust_bindings_generator_min_version" + echo >&2 "***" + fi + exit 1 +fi +if [ "$1" = -v ] && [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cversion" ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' is too new. This may or may not work." + echo >&2 "*** Your version: $rust_bindings_generator_version" + echo >&2 "*** Expected version: $rust_bindings_generator_min_version" + echo >&2 "***" +fi + +# Check that the `libclang` used by the Rust bindings generator is suitable. +bindgen_libclang_version=$( \ + LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null \ + | grep -F 'clang version ' \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ + | head -n 1 \ +) +bindgen_libclang_min_version=$($min_tool_version llvm) +bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version) +bindgen_libclang_min_cversion=$(get_canonical_version $bindgen_libclang_min_version) +if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN') is too old." + echo >&2 "*** Your version: $bindgen_libclang_version" + echo >&2 "*** Minimum version: $bindgen_libclang_min_version" + echo >&2 "***" + fi + exit 1 +fi + +# If the C compiler is Clang, then we can also check whether its version +# matches the `libclang` version used by the Rust bindings generator. +# +# In the future, we might be able to perform a full version check, see +# https://github.com/rust-lang/rust-bindgen/issues/2138. +if [ "$1" = -v ]; then + cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ') + if [ "$cc_name" = Clang ]; then + clang_version=$( \ + LC_ALL=C "$CC" --version 2>/dev/null \ + | sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p' + ) + if [ "$clang_version" != "$bindgen_libclang_version" ]; then + echo >&2 "***" + echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN')" + echo >&2 "*** version does not match Clang's. This may be a problem." + echo >&2 "*** libclang version: $bindgen_libclang_version" + echo >&2 "*** Clang version: $clang_version" + echo >&2 "***" + fi + fi +fi + +# Check that the source code for the `core` standard library exists. +# +# `$KRUSTFLAGS` is passed in case the user added `--sysroot`. +rustc_sysroot=$("$RUSTC" $KRUSTFLAGS --print sysroot) +rustc_src=${RUST_LIB_SRC:-"$rustc_sysroot/lib/rustlib/src/rust/library"} +rustc_src_core="$rustc_src/core/src/lib.rs" +if [ ! -e "$rustc_src_core" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Source code for the 'core' standard library could not be found" + echo >&2 "*** at '$rustc_src_core'." + echo >&2 "***" + fi + exit 1 +fi
scripts/rust_is_available_bindgen_libclang.h new file +2 -0
@@ -0,0 +1,2 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#pragma message("clang version " __clang_version__)
Signature · 1 line
2.37.1
Re: [PATCH v9 20/27] scripts: add `rust_is_available.sh`
Quoted 76 lines · Miguel Ojeda
This script tests whether the Rust toolchain requirements are in place to enable Rust support.
The build system will call it to set
CONFIG_RUST_IS_AVAILABLEin a later patch.It also has an option (
-v) to explain what is missing, which is useful to set up the development environment. This is used via themake rustavailabletarget added in a later patch.Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Finn Behrens <[email protected]>Signed-off-by Finn Behrens <[email protected]>Co-developed-by Miguel Cano <[email protected]>Signed-off-by Miguel Cano <[email protected]>Co-developed-by Tiago Lam <[email protected]>Signed-off-by Tiago Lam <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- scripts/rust_is_available.sh | 160 +++++++++++++++++++ scripts/rust_is_available_bindgen_libclang.h | 2 + 2 files changed, 162 insertions(+) create mode 100755 scripts/rust_is_available.sh create mode 100644 scripts/rust_is_available_bindgen_libclang.hscripts/rust_is_available.sh new file +42
-0@@ -0,0 +1,160 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Tests whether a suitable Rust toolchain is available. +# +# Pass `-v` for human output and more checks (as warnings). + +set -e + +min_tool_version=$(dirname $0)/min-tool-version.sh + +# Convert the version string x.y.z to a canonical up-to-7-digits form. +# +# Note that this function uses one more digit (compared to other +# instances in other version scripts) to give a bit more space to +# `rustc` since it will reach 1.100.0 in late 2026. +get_canonical_version() +{ + IFS=. + set -- $1 + echo $((100000 * $1 + 100 * $2 + $3)) +} + +# Check that the Rust compiler exists. +if ! command -v "$RUSTC" >/dev/null; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' could not be found." + echo >&2 "***" + fi + exit 1 +fi + +# Check that the Rust bindings generator exists. +if ! command -v "$BINDGEN" >/dev/null; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' could not be found." + echo >&2 "***" + fi + exit 1 +fi
scripts/rust_is_available.sh line 52
Quoted 10 lines
scripts/rust_is_available.sh new file +10
-0+ +# Check that the Rust compiler version is suitable. +# +# Non-stable and distributions' versions may have a version suffix, e.g. `-dev`. +rust_compiler_version=$( \ + LC_ALL=C "$RUSTC" --version 2>/dev/null \ + | head -n 1 \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ +) +rust_compiler_min_version=$($min_tool_version rustc)
I think the min-tool-version.sh changes from patch 23 should be moved into this patch.
With that:
-Kees
Quoted 119 lines
scripts/rust_is_available.sh new file +108
-0+rust_compiler_cversion=$(get_canonical_version $rust_compiler_version) +rust_compiler_min_cversion=$(get_canonical_version $rust_compiler_min_version) +if [ "$rust_compiler_cversion" -lt "$rust_compiler_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' is too old." + echo >&2 "*** Your version: $rust_compiler_version" + echo >&2 "*** Minimum version: $rust_compiler_min_version" + echo >&2 "***" + fi + exit 1 +fi +if [ "$1" = -v ] && [ "$rust_compiler_cversion" -gt "$rust_compiler_min_cversion" ]; then + echo >&2 "***" + echo >&2 "*** Rust compiler '$RUSTC' is too new. This may or may not work." + echo >&2 "*** Your version: $rust_compiler_version" + echo >&2 "*** Expected version: $rust_compiler_min_version" + echo >&2 "***" +fi + +# Check that the Rust bindings generator is suitable. +# +# Non-stable and distributions' versions may have a version suffix, e.g. `-dev`. +rust_bindings_generator_version=$( \ + LC_ALL=C "$BINDGEN" --version 2>/dev/null \ + | head -n 1 \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ +) +rust_bindings_generator_min_version=$($min_tool_version bindgen) +rust_bindings_generator_cversion=$(get_canonical_version $rust_bindings_generator_version) +rust_bindings_generator_min_cversion=$(get_canonical_version $rust_bindings_generator_min_version) +if [ "$rust_bindings_generator_cversion" -lt "$rust_bindings_generator_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' is too old." + echo >&2 "*** Your version: $rust_bindings_generator_version" + echo >&2 "*** Minimum version: $rust_bindings_generator_min_version" + echo >&2 "***" + fi + exit 1 +fi +if [ "$1" = -v ] && [ "$rust_bindings_generator_cversion" -gt "$rust_bindings_generator_min_cversion" ]; then + echo >&2 "***" + echo >&2 "*** Rust bindings generator '$BINDGEN' is too new. This may or may not work." + echo >&2 "*** Your version: $rust_bindings_generator_version" + echo >&2 "*** Expected version: $rust_bindings_generator_min_version" + echo >&2 "***" +fi + +# Check that the `libclang` used by the Rust bindings generator is suitable. +bindgen_libclang_version=$( \ + LC_ALL=C "$BINDGEN" $(dirname $0)/rust_is_available_bindgen_libclang.h 2>&1 >/dev/null \ + | grep -F 'clang version ' \ + | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' \ + | head -n 1 \ +) +bindgen_libclang_min_version=$($min_tool_version llvm) +bindgen_libclang_cversion=$(get_canonical_version $bindgen_libclang_version) +bindgen_libclang_min_cversion=$(get_canonical_version $bindgen_libclang_min_version) +if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN') is too old." + echo >&2 "*** Your version: $bindgen_libclang_version" + echo >&2 "*** Minimum version: $bindgen_libclang_min_version" + echo >&2 "***" + fi + exit 1 +fi + +# If the C compiler is Clang, then we can also check whether its version +# matches the `libclang` version used by the Rust bindings generator. +# +# In the future, we might be able to perform a full version check, see +# https://github.com/rust-lang/rust-bindgen/issues/2138. +if [ "$1" = -v ]; then + cc_name=$($(dirname $0)/cc-version.sh "$CC" | cut -f1 -d' ') + if [ "$cc_name" = Clang ]; then + clang_version=$( \ + LC_ALL=C "$CC" --version 2>/dev/null \ + | sed -nE '1s:.*version ([0-9]+\.[0-9]+\.[0-9]+).*:\1:p' + ) + if [ "$clang_version" != "$bindgen_libclang_version" ]; then + echo >&2 "***" + echo >&2 "*** libclang (used by the Rust bindings generator '$BINDGEN')" + echo >&2 "*** version does not match Clang's. This may be a problem." + echo >&2 "*** libclang version: $bindgen_libclang_version" + echo >&2 "*** Clang version: $clang_version" + echo >&2 "***" + fi + fi +fi + +# Check that the source code for the `core` standard library exists. +# +# `$KRUSTFLAGS` is passed in case the user added `--sysroot`. +rustc_sysroot=$("$RUSTC" $KRUSTFLAGS --print sysroot) +rustc_src=${RUST_LIB_SRC:-"$rustc_sysroot/lib/rustlib/src/rust/library"} +rustc_src_core="$rustc_src/core/src/lib.rs" +if [ ! -e "$rustc_src_core" ]; then + if [ "$1" = -v ]; then + echo >&2 "***" + echo >&2 "*** Source code for the 'core' standard library could not be found" + echo >&2 "*** at '$rustc_src_core'." + echo >&2 "***" + fi + exit 1 +fiscripts/rust_is_available_bindgen_libclang.h new file +2
-0@@ -0,0 +1,2 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#pragma message("clang version " __clang_version__)Signature · 1 line
2.37.1
Signature · 1 line
Kees Cook
Re: [PATCH v9 20/27] scripts: add `rust_is_available.sh`
Quoted 3 lines · Kees Cook
I think the
min-tool-version.shchanges from patch 23 should be moved into this patch.
Yeah, it calls into that script with a "rustc" argument, which doesn't work if somebody happened to run it by hand, so this makes sense.
However, the intention was to move each "big" new file into their own patch to make a smaller Kbuild one (and only do that "new file" additions in each patch), and then finally do all the modifications and enable the Rust support in the Kbuild patch (which has the callers into this script). Also the script is not intended to be called directly anyway (since you need the Make environment, but you definitely can), there is a Make target to help with that.
Hopefully that explains the logic behind the arrangement of the patches, which was to make the patches as simple as possible. I am happy to rearrange if you think it is more understandable that way.
Cheers,
Miguel
Re: [PATCH v9 20/27] scripts: add `rust_is_available.sh`
In reply to Miguel OjedaQuoted 3 lines · Miguel Ojeda
This script tests whether the Rust toolchain requirements are in place to enable Rust support.
With this, I get:
$ make LLVM=1 rustavailable *** *** libclang (used by the Rust bindings generator 'bindgen') *** version does not match Clang's. This may be a problem. *** libclang version: 15.0.0 *** Clang version: 16.0.0 *** Rust is available!
because I'm using clang built from source from ToT. Is this supposed to mean that I can't use clang-16, clang-14, clang-13, clang-12, or clang-11 (in the kernel we support clang-11+) in order to use rust? I'm guessing that's going to hinder adoption. Is there a way to specify which libclang version bindgen should be using?
I have libclang built in my clang sources, llvm-project/llvm/build/lib/libclang.so. I also tried:
$ CLANG_PATH=/android0/llvm-project/llvm/build/lib/libclang.so.15 make LLVM=1 -j72 rustavailable $ CLANG_PATH=/android0/llvm-project/llvm/build/lib/libclang.so make LLVM=1 -j72 rustavailable
Signature · 2 lines
Thanks, ~Nick Desaulniers
Re: [PATCH v9 20/27] scripts: add `rust_is_available.sh`
Quoted 5 lines · Nick Desaulniers
because I'm using clang built from source from ToT. Is this supposed to mean that I can't use clang-16, clang-14, clang-13, clang-12, or clang-11 (in the kernel we support clang-11+) in order to use rust? I'm guessing that's going to hinder adoption. Is there a way to
No, it is only a warning, so you can proceed if you want to risk it.
However, is there a reason you would like to mix the versions?
If not, please point bindgen to your newer libclang (see below).
Quoted 1 line
specify which libclang version bindgen should be using?
Yes, see below.
Quoted 5 lines
I have libclang built in my clang sources, llvm-project/llvm/build/lib/libclang.so. I also tried:
$ CLANG_PATH=/android0/llvm-project/llvm/build/lib/libclang.so.15 make LLVM=1 -j72 rustavailable
CLANG_PATH is for pointing to a clang executable, not libclang.
Instead, please try LIBCLANG_PATH (note the LIB there).
For instance, using the test header to print the libclang version, this works for me:
$ bindgen scripts/rust_is_available_bindgen_libclang.h
... clang version 14.0.6 (https://github.com/llvm/llvm-project.git ... $ LIBCLANG_PATH=.../libclang-6.0.so.1 \
bindgen scripts/rust_is_available_bindgen_libclang.h
... clang version 6.0.0 (tags/RELEASE_600/final) ...If the above does not work, for details on how the dependency is resolved, please see:
https://github.com/KyleMayes/clang-sys#linking
https://github.com/KyleMayes/clang-sys#dependencies
https://github.com/KyleMayes/clang-sys#environment-variables
https://github.com/rust-lang/rust-bindgen#environment-variables
I will add a note about this to the docs.
Cheers,
Miguel
Re: [PATCH v9 20/27] scripts: add `rust_is_available.sh`
Quoted 10 lines · Miguel Ojeda
For instance, using the test header to print the libclang version, this works for me:
$ bindgen scripts/rust_is_available_bindgen_libclang.h ... clang version 14.0.6 (https://github.com/llvm/llvm-project.git ...$ LIBCLANG_PATH=.../libclang-6.0.so.1 \ bindgen scripts/rust_is_available_bindgen_libclang.h ... clang version 6.0.0 (tags/RELEASE_600/final) ...
By the way, a while ago I requested an easier way to check the libclang version directly from bindgen, and Emilio quickly added it (thanks a lot!): since bindgen 0.60.0 you can do:
$ bindgen --version --verbose
bindgen 0.60.0
Clang: clang version 14.0.6 (https://github.com/llvm/ ...Cheers,
Miguel
Miguel Ojeda
maintainerPATCH v9 21/27 scripts: add `is_rust_module.sh`
In reply to Miguel OjedaFrom: Daniel Xu <[email protected]>
This script is used to detect whether a kernel module is written in Rust.
It will later be used to disable BTF generation on Rust modules as BTF does not yet support Rust.
--- scripts/is_rust_module.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 scripts/is_rust_module.sh
scripts/is_rust_module.sh new file +16 -0
@@ -0,0 +1,16 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# is_rust_module.sh module.ko +# +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. + +set -e + +# Using the `16_` prefix ensures other symbols with the same substring +# are not picked up (even if it would be unlikely). The last part is +# used just in case LLVM decides to use the `.` suffix. +# +# In the future, checking for the `.comment` section may be another +# option, see https://github.com/rust-lang/rust/pull/97550. +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$'
Signature · 1 line
2.37.1
Re: [PATCH v9 21/27] scripts: add `is_rust_module.sh`
Quoted 10 lines · Miguel Ojeda
From: Daniel Xu <[email protected]>
This script is used to detect whether a kernel module is written in Rust.
It will later be used to disable BTF generation on Rust modules as BTF does not yet support Rust.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 22/27 rust: add `.rustfmt.toml`
In reply to Miguel OjedaThis is the configuration file for the rustfmt tool.
rustfmt is a tool for formatting Rust code according to style guidelines. It is very commonly used across Rust projects.
The default configuration options are used.
--- .gitignore | 1 + .rustfmt.toml | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 .rustfmt.toml
.gitignore +1 -0
@@ -97,6 +97,7 @@ modules.order !.gitattributes !.gitignore !.mailmap +!.rustfmt.toml # # Generated include files
.rustfmt.toml new file +12 -0
@@ -0,0 +1,12 @@ +edition = "2021" +newline_style = "Unix" + +# Unstable options that help catching some mistakes in formatting and that we may want to enable +# when they become stable. +# +# They are kept here since they are useful to run from time to time. +#format_code_in_doc_comments = true +#reorder_impl_items = true +#comment_width = 100 +#wrap_comments = true +#normalize_comments = true
Signature · 1 line
2.37.1
Re: [PATCH v9 22/27] rust: add `.rustfmt.toml`
Quoted 9 lines · Miguel Ojeda
This is the configuration file for the
rustfmttool.
rustfmtis a tool for formatting Rust code according to style guidelines. It is very commonly used across Rust projects.The default configuration options are used.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 23/27 Kbuild: add Rust support
In reply to Miguel OjedaHaving most of the new files in place, we now enable Rust support in the build system, including Kconfig entries related to Rust, the Rust configuration printer and a few other bits.
--- .gitignore | 2 + Makefile | 172 ++++++++++++++- arch/Kconfig | 6 + include/linux/compiler_types.h | 6 +- init/Kconfig | 46 +++- kernel/configs/rust.config | 1 + lib/Kconfig.debug | 34 +++ rust/.gitignore | 8 + rust/Makefile | 381 +++++++++++++++++++++++++++++++++ rust/bindgen_parameters | 21 ++ scripts/Kconfig.include | 6 +- scripts/Makefile | 3 + scripts/Makefile.build | 60 ++++++ scripts/Makefile.debug | 10 + scripts/Makefile.host | 34 ++- scripts/Makefile.lib | 12 ++ scripts/Makefile.modfinal | 8 +- scripts/cc-version.sh | 12 +- scripts/kconfig/confdata.c | 75 +++++++ scripts/min-tool-version.sh | 6 + 20 files changed, 877 insertions(+), 26 deletions(-) create mode 100644 kernel/configs/rust.config create mode 100644 rust/.gitignore create mode 100644 rust/Makefile create mode 100644 rust/bindgen_parameters
.gitignore +2 -0
@@ -37,6 +37,8 @@ *.o *.o.* *.patch +*.rmeta +*.rsi *.s *.so *.so.dbg
Makefile +163 -9
342 lines not on this page · raw message
arch/Kconfig +6 -0
@@ -353,6 +353,12 @@ config HAVE_RSEQ This symbol should be selected by an architecture if it supports an implementation of restartable sequences. +config HAVE_RUST + bool + help + This symbol should be selected by an architecture if it + supports Rust. + config HAVE_FUNCTION_ARG_ACCESS_API bool help
include/linux/compiler_types.h +5 -1
@@ -4,8 +4,12 @@#ifndef __ASSEMBLY__ +/* +* Skipped when running bindgen due to a libclang issue; +* see https://github.com/rust-lang/rust-bindgen/issues/2244. +*/ #if defined(CONFIG_DEBUG_INFO_BTF ) && defined(CONFIG_PAHOLE_HAS_BTF_TAG ) && \ -__has_attribute (btf_type_tag) +__has_attribute (btf_type_tag) && !defined(__BINDGEN__)# define BTF_TYPE_TAG (value) __attribute__((btf_type_tag(#value)))#else # define BTF_TYPE_TAG (value)/* nothing */
init/Kconfig +45 -1
64 lines not on this page · raw message
kernel/configs/rust.config new file +1 -0
@@ -0,0 +1 @@ +CONFIG_RUST=y
lib/Kconfig.debug +34 -0
@@ -2688,6 +2688,40 @@ config HYPERV_TESTING endmenu # "Kernel Testing and Coverage" +menu "Rust hacking" + +config RUST_DEBUG_ASSERTIONS + bool "Debug assertions" + depends on RUST + help + Enables rustc's `-Cdebug-assertions` codegen option. + + This flag lets you turn `cfg(debug_assertions)` conditional + compilation on or off. This can be used to enable extra debugging + code in development but not in production. For example, it controls + the behavior of the standard library's `debug_assert!` macro. + + Note that this will apply to all Rust code, including `core`. + + If unsure, say N. + +config RUST_OVERFLOW_CHECKS + bool "Overflow checks" + default y + depends on RUST + help + Enables rustc's `-Coverflow-checks` codegen option. + + This flag allows you to control the behavior of runtime integer + overflow. When overflow-checks are enabled, a Rust panic will occur + on overflow. + + Note that this will apply to all Rust code, including `core`. + + If unsure, say Y. + +endmenu # "Rust" + source "Documentation/Kconfig" endmenu # Kernel hacking
rust/.gitignore new file +8 -0
@@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0 + +target.json +bindings_generated.rs +bindings_helpers_generated.rs +exports_*_generated.h +doc/ +test/
rust/Makefile new file +381 -0
381 lines not on this page · raw message
rust/bindgen_parameters new file +21 -0
@@ -0,0 +1,21 @@ +# SPDX-License-Identifier: GPL-2.0 + +--opaque-type xregs_state +--opaque-type desc_struct +--opaque-type arch_lbr_state +--opaque-type local_apic + +# Packed type cannot transitively contain a `#[repr(align)]` type. +--opaque-type x86_msi_data +--opaque-type x86_msi_addr_lo + +# `try` is a reserved keyword since Rust 2018; solved in `bindgen` v0.59.2, +# commit 2aed6b021680 ("context: Escape the try keyword properly"). +--opaque-type kunit_try_catch + +# If SMP is disabled, `arch_spinlock_t` is defined as a ZST which triggers a Rust +# warning. We don't need to peek into it anyway. +--opaque-type spinlock + +# `seccomp`'s comment gets understood as a doctest +--no-doc-comments
scripts/Kconfig.include +3 -3
@@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) # check if $(CC) and $(LD) exist -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) $(error-if,$(failure,command -v $(LD)),linker '$(LD)' not found) -# Get the compiler name, version, and error out if it is not supported. +# Get the C compiler name, version, and error out if it is not supported. cc-info := $(shell,$(srctree)/scripts/cc-version.sh $(CC)) -$(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not supported.) +$(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this C compiler is not supported.) cc-name := $(shell,set -- $(cc-info) && echo $1) cc-version := $(shell,set -- $(cc-info) && echo $2)
scripts/Makefile +3 -0
@@ -10,6 +10,9 @@ hostprogs-always-$(CONFIG_BUILDTIME_TABLE_SORT) += sorttable hostprogs-always-$(CONFIG_ASN1) += asn1_compiler hostprogs-always-$(CONFIG_MODULE_SIG_FORMAT) += sign-file hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert +hostprogs-always-$(CONFIG_RUST) += generate_rust_target + +generate_rust_target-rust := y HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include HOSTLDLIBS_sorttable = -lpthread
scripts/Makefile.build +60 -0
72 lines not on this page · raw message
scripts/Makefile.debug +10 -0
@@ -1,4 +1,5 @@ DEBUG_CFLAGS := +DEBUG_RUSTFLAGS := ifdef CONFIG_DEBUG_INFO_SPLIT DEBUG_CFLAGS += -gsplit-dwarf
@@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM KBUILD_AFLAGS += -Wa,-gdwarf-2 endif +ifdef CONFIG_DEBUG_INFO_REDUCED +DEBUG_RUSTFLAGS += -Cdebuginfo=1 +else +DEBUG_RUSTFLAGS += -Cdebuginfo=2 +endif + ifndef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4 dwarf-version-$(CONFIG_DEBUG_INFO_DWARF5) := 5
@@ -31,3 +38,6 @@ endif KBUILD_CFLAGS += $(DEBUG_CFLAGS) export DEBUG_CFLAGS + +KBUILD_RUSTFLAGS += $(DEBUG_RUSTFLAGS) +export DEBUG_RUSTFLAGS
scripts/Makefile.host +31 -3
@@ -22,6 +22,8 @@ $(obj)/%.tab.c $(obj)/%.tab.h: $(src)/%.y FORCE # to preprocess a data file. # # Both C and C++ are supported, but preferred language is C for such utilities. +# Rust is also supported, but it may only be used in scenarios where a Rust +# toolchain is required to be available (e.g. when `CONFIG_RUST` is enabled). # # Sample syntax (see Documentation/kbuild/makefiles.rst for reference) # hostprogs := bin2hex
@@ -37,15 +39,20 @@ $(obj)/%.tab.c $(obj)/%.tab.h: $(src)/%.y FORCE # qconf-objs := menu.o # Will compile qconf as a C++ program, and menu as a C program. # They are linked as C++ code to the executable qconf +# +# hostprogs := target +# target-rust := y +# Will compile `target` as a Rust program, using `target.rs` as the crate root. +# The crate may consist of several source files. # C code # Executables compiled from a single .c file host-csingle := $(foreach m,$(hostprogs), \ - $(if $($(m)-objs)$($(m)-cxxobjs),,$(m))) + $(if $($(m)-objs)$($(m)-cxxobjs)$($(m)-rust),,$(m))) # C executables linked based on several .o files host-cmulti := $(foreach m,$(hostprogs),\ - $(if $($(m)-cxxobjs),,$(if $($(m)-objs),$(m)))) + $(if $($(m)-cxxobjs)$($(m)-rust),,$(if $($(m)-objs),$(m)))) # Object (.o) files compiled from .c files host-cobjs := $(sort $(foreach m,$(hostprogs),$($(m)-objs)))
@@ -58,11 +65,17 @@ host-cxxmulti := $(foreach m,$(hostprogs),$(if $($(m)-cxxobjs),$(m))) # C++ Object (.o) files compiled from .cc files host-cxxobjs := $(sort $(foreach m,$(host-cxxmulti),$($(m)-cxxobjs))) +# Rust code +# Executables compiled from a single Rust crate (which may consist of +# one or more .rs files) +host-rust := $(foreach m,$(hostprogs),$(if $($(m)-rust),$(m))) + host-csingle := $(addprefix $(obj)/,$(host-csingle)) host-cmulti := $(addprefix $(obj)/,$(host-cmulti)) host-cobjs := $(addprefix $(obj)/,$(host-cobjs)) host-cxxmulti := $(addprefix $(obj)/,$(host-cxxmulti)) host-cxxobjs := $(addprefix $(obj)/,$(host-cxxobjs)) +host-rust := $(addprefix $(obj)/,$(host-rust)) ##### # Handle options to gcc. Support building with separate output directory
@@ -71,6 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \ $(HOSTCFLAGS_$(target-stem).o) _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \ $(HOSTCXXFLAGS_$(target-stem).o) +_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ + $(HOSTRUSTFLAGS_$(target-stem)) # $(objtree)/$(obj) for including generated headers from checkin source files ifeq ($(KBUILD_EXTMOD),)
@@ -82,6 +97,7 @@ endif hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) +hostrust_flags = $(_hostrust_flags) ##### # Compile programs on the host
@@ -128,5 +144,17 @@ quiet_cmd_host-cxxobjs = HOSTCXX $@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE $(call if_changed_dep,host-cxxobjs) +# Create executable from a single Rust crate (which may consist of +# one or more `.rs` files) +# host-rust -> Executable +quiet_cmd_host-rust = HOSTRUSTC $@ + cmd_host-rust = \ + $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \ + --out-dir=$(obj)/ $<; \ + mv $(obj)/$(target-stem).d $(depfile); \ + sed -i '/^\#/d' $(depfile) +$(host-rust): $(obj)/%: $(src)/%.rs FORCE + $(call if_changed_dep,host-rust) + targets += $(host-csingle) $(host-cmulti) $(host-cobjs) \ - $(host-cxxmulti) $(host-cxxobjs) + $(host-cxxmulti) $(host-cxxobjs) $(host-rust)
scripts/Makefile.lib +12 -0
@@ -8,6 +8,7 @@ ldflags-y += $(EXTRA_LDFLAGS) # flags that take effect in current and sub directories KBUILD_AFLAGS += $(subdir-asflags-y) KBUILD_CFLAGS += $(subdir-ccflags-y) +KBUILD_RUSTFLAGS += $(subdir-rustflags-y) # Figure out what we need to build from the various variables # ===========================================================================
@@ -128,6 +129,10 @@ _c_flags = $(filter-out $(CFLAGS_REMOVE_$(target-stem).o), \ $(filter-out $(ccflags-remove-y), \ $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(ccflags-y)) \ $(CFLAGS_$(target-stem).o)) +_rust_flags = $(filter-out $(RUSTFLAGS_REMOVE_$(target-stem).o), \ + $(filter-out $(rustflags-remove-y), \ + $(KBUILD_RUSTFLAGS) $(rustflags-y)) \ + $(RUSTFLAGS_$(target-stem).o)) _a_flags = $(filter-out $(AFLAGS_REMOVE_$(target-stem).o), \ $(filter-out $(asflags-remove-y), \ $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(asflags-y)) \
@@ -202,6 +207,11 @@ modkern_cflags = \ $(KBUILD_CFLAGS_MODULE) $(CFLAGS_MODULE), \ $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL) $(modfile_flags)) +modkern_rustflags = \ + $(if $(part-of-module), \ + $(KBUILD_RUSTFLAGS_MODULE) $(RUSTFLAGS_MODULE), \ + $(KBUILD_RUSTFLAGS_KERNEL) $(RUSTFLAGS_KERNEL)) + modkern_aflags = $(if $(part-of-module), \ $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE), \ $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL))
@@ -211,6 +221,8 @@ c_flags = -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ $(_c_flags) $(modkern_cflags) \ $(basename_flags) $(modname_flags) +rust_flags = $(_rust_flags) $(modkern_rustflags) @$(objtree)/include/generated/rustc_cfg + a_flags = -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ $(_a_flags) $(modkern_aflags)
scripts/Makefile.modfinal +5 -3
@@ -39,11 +39,13 @@ quiet_cmd_ld_ko_o = LD [M] $@ quiet_cmd_btf_ko = BTF [M] $@ cmd_btf_ko = \ - if [ -f vmlinux ]; then \ + if [ ! -f vmlinux ]; then \ + printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ + elif [ -n "$(CONFIG_RUST)" ] && $(srctree)/scripts/is_rust_module.sh $@; then \ + printf "Skipping BTF generation for %s because it's a Rust module\n" $@ 1>&2; \ + else \ LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ $(RESOLVE_BTFIDS) -b vmlinux $@; \ - else \ - printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ fi; # Same as newer-prereqs, but allows to exclude specified extra dependencies
scripts/cc-version.sh +6 -6
@@ -1,13 +1,13 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 # -# Print the compiler name and its version in a 5 or 6-digit form. +# Print the C compiler name and its version in a 5 or 6-digit form. # Also, perform the minimum version check. set -e -# Print the compiler name and some version components. -get_compiler_info() +# Print the C compiler name and some version components. +get_c_compiler_info() { cat <<- EOF | "$@" -E -P -x c - 2>/dev/null #if defined(__clang__)
@@ -32,7 +32,7 @@ get_canonical_version() # $@ instead of $1 because multiple words might be given, e.g. CC="ccache gcc". orig_args="$@" -set -- $(get_compiler_info "$@") +set -- $(get_c_compiler_info "$@") name=$1
@@ -52,7 +52,7 @@ ICC) min_version=$($min_tool_version icc) ;; *) - echo "$orig_args: unknown compiler" >&2 + echo "$orig_args: unknown C compiler" >&2 exit 1 ;; esac
@@ -62,7 +62,7 @@ min_cversion=$(get_canonical_version $min_version) if [ "$cversion" -lt "$min_cversion" ]; then echo >&2 "***" - echo >&2 "*** Compiler is too old." + echo >&2 "*** C compiler is too old." echo >&2 "*** Your $name version: $version" echo >&2 "*** Minimum $name version: $min_version" echo >&2 "***"
scripts/kconfig/confdata.c +75 -0
99 lines not on this page · raw message
scripts/min-tool-version.sh +6 -0
@@ -30,6 +30,12 @@ llvm) echo 11.0.0 fi ;; +rustc) + echo 1.62.0 + ;; +bindgen) + echo 0.56.0 + ;; *) echo "$1: unknown tool" >&2 exit 1
Signature · 1 line
2.37.1
Re: [PATCH v9 23/27] Kbuild: add Rust support
Quoted 27 lines · Miguel Ojeda
Having most of the new files in place, we now enable Rust support in the build system, including
Kconfigentries related to Rust, the Rust configuration printer and a few other bits.Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Finn Behrens <[email protected]>Signed-off-by Finn Behrens <[email protected]>Co-developed-by Adam Bratschi-Kaye <[email protected]>Signed-off-by Adam Bratschi-Kaye <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Michael Ellerman <[email protected]>Signed-off-by Michael Ellerman <[email protected]>Co-developed-by Sven Van Asbroeck <[email protected]>Signed-off-by Sven Van Asbroeck <[email protected]>Co-developed-by Gary Guo <[email protected]>Signed-off-by Gary Guo <[email protected]>Co-developed-by Boris-Chengbiao Zhou <[email protected]>Signed-off-by Boris-Chengbiao Zhou <[email protected]>Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Co-developed-by Douglas Su <[email protected]>Signed-off-by Douglas Su <[email protected]>Co-developed-by Dariusz Sosnowski <[email protected]>Signed-off-by Dariusz Sosnowski <[email protected]>Co-developed-by Antonio Terceiro <[email protected]>
Quoted 8 lines
Signed-off-by Antonio Terceiro <[email protected]>Co-developed-by Daniel Xu <[email protected]>Signed-off-by Daniel Xu <[email protected]>Co-developed-by Björn Roy Baron <[email protected]>Signed-off-by Björn Roy Baron <[email protected]>Co-developed-by Martin Rodriguez Reboredo <[email protected]>Signed-off-by Martin Rodriguez Reboredo <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>
It'd be nice to split this up into maybe per-target patches, but given how long this has been getting developed, I'd prefer to avoid blocking on that, and just get this series landed so we can move to incremental fixes/additions.
-Kees
Signature · 1 line
Kees Cook
Re: [PATCH v9 23/27] Kbuild: add Rust support
Quoted 5 lines · Kees Cook
It'd be nice to split this up into maybe per-target patches, but given how long this has been getting developed, I'd prefer to avoid blocking on that, and just get this series landed so we can move to incremental fixes/additions.
When I started the series, I considered several approaches, and in the end opted for "setup as much as possible before the flag-patch that enables the Kbuild support", which was simple, though indeed the Kbuild patch remained fairly big.
Then some people commented on that patch over several versions, so I kept the simple strategy :)
What I can look into is moving the optional targets out of this patch, putting them after the "flag-patch", since they may be considered "additional features" anyway. In fact, for the trimming down in v9, I thought about removing them entirely. It will not change things too much, since they are small, but it may help.
For v9 I nevertheless attempted to simplify this patch by doing the "move all new, big files to their own patch" (see my reply at [1]), so it is fairly smaller than in previous versions.
[1] https://lore.kernel.org/lkml/CANiq72nzOQSd2vsh2_=0YpGNpY=7agokbgi_vBc5_GF4-02rsA@mail.gmail.com/
Cheers,
Miguel
Re: [PATCH v9 23/27] Kbuild: add Rust support
In reply to Miguel OjedaQuoted 4 lines · Miguel Ojeda
Having most of the new files in place, we now enable Rust support in the build system, including
Kconfigentries related to Rust, the Rust configuration printer and a few other bits.
Cool, I'm finally happy with this patch.
I built and boot tested with CONFIG_RUST enabled, played with CLIPPY=1, and built all of the new make targets for rust.
Thanks for all of the work that went into these series from all of the authors and Miguel for your determination. Keep it up!
I've left comments on other patches (and I will leave one on rust/compiler_builtins.rs because I still don't like that approach) and there may be small cleanups we can do here or there, but I think we're in good shape to land something and start iterating on it upstream. I'll file bugs in your issue tracker for small nits I come across, but so far, it's been more-so questions.
---
LWN recently demonstrated that most fixes are in relatively younger
code: https://lwn.net/Articles/902854/
An analysis of 0day exploits found in the wild in 2021 showed that for Android, researchers are attacking drivers:
https://googleprojectzero.blogspot.com/2022/04/the-more-you-know-more-you-know-you.html
Multiple independent reports cite high numbers (70% or more) of memory safety issues in native code:
https://www.memorysafety.org/docs/memory-safety/
I have colleagues that are developing a microkernel (they then use a memory unsafe language for their kernel as well :^P ) to move as much functionality as possible into lower levels of privilege. It's interesting and I wish them well, but I also prefer more incremental approaches to existing solutions, and suspect the way of the monolith to still give us the best performance.
I learned an interesting word the other day: Corten Steel from the YouTube channel Practical Engineering (it's a great channel on Civil Engineering):
https://www.youtube.com/watch?v=2RbiCOFffRs&t=523s
transcript:
Quoted 1 line
Quoted 1 line
I should also note that there are even steel alloys whose rust is protective! Weathering steel (sometimes known by its trade name of Corten Steel) is a group of alloys that are naturally resilient against rust because of passivation. A special blend of elements, including manganese, nickel, silicon, and chromium don’t keep the steel from rusting, but they allow the layer of rust to stay attached, forming a protective layer that significantly slows corrosion.
My hope is that Rust may provide a layer of Corten Steel to the Linux kernel to help us protect newly written driver code from memory safety related issues, so that Linux remains the best option for developing products for the next 30 years. I also suspect it may bring in a whole new generation of hackers to the kernel ecosystem. That is my blessing for Rust in the Linux kernel.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v9 23/27] Kbuild: add Rust support
In reply to Miguel OjedaQuoted 122 lines · Miguel Ojeda
Having most of the new files in place, we now enable Rust support in the build system, including
Kconfigentries related to Rust, the Rust configuration printer and a few other bits.Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>Co-developed-by Finn Behrens <[email protected]>Signed-off-by Finn Behrens <[email protected]>Co-developed-by Adam Bratschi-Kaye <[email protected]>Signed-off-by Adam Bratschi-Kaye <[email protected]>Co-developed-by Wedson Almeida Filho <[email protected]>Signed-off-by Wedson Almeida Filho <[email protected]>Co-developed-by Michael Ellerman <[email protected]>Signed-off-by Michael Ellerman <[email protected]>Co-developed-by Sven Van Asbroeck <[email protected]>Signed-off-by Sven Van Asbroeck <[email protected]>Co-developed-by Gary Guo <[email protected]>Signed-off-by Gary Guo <[email protected]>Co-developed-by Boris-Chengbiao Zhou <[email protected]>Signed-off-by Boris-Chengbiao Zhou <[email protected]>Co-developed-by Boqun Feng <[email protected]>Signed-off-by Boqun Feng <[email protected]>Co-developed-by Douglas Su <[email protected]>Signed-off-by Douglas Su <[email protected]>Co-developed-by Dariusz Sosnowski <[email protected]>Signed-off-by Dariusz Sosnowski <[email protected]>Co-developed-by Antonio Terceiro <[email protected]>Signed-off-by Antonio Terceiro <[email protected]>Co-developed-by Daniel Xu <[email protected]>Signed-off-by Daniel Xu <[email protected]>Co-developed-by Björn Roy Baron <[email protected]>Signed-off-by Björn Roy Baron <[email protected]>Co-developed-by Martin Rodriguez Reboredo <[email protected]>Signed-off-by Martin Rodriguez Reboredo <[email protected]>Signed-off-by Miguel Ojeda <[email protected]>--- .gitignore | 2 + Makefile | 172 ++++++++++++++- arch/Kconfig | 6 + include/linux/compiler_types.h | 6 +- init/Kconfig | 46 +++- kernel/configs/rust.config | 1 + lib/Kconfig.debug | 34 +++ rust/.gitignore | 8 + rust/Makefile | 381 +++++++++++++++++++++++++++++++++ rust/bindgen_parameters | 21 ++ scripts/Kconfig.include | 6 +- scripts/Makefile | 3 + scripts/Makefile.build | 60 ++++++ scripts/Makefile.debug | 10 + scripts/Makefile.host | 34 ++- scripts/Makefile.lib | 12 ++ scripts/Makefile.modfinal | 8 +- scripts/cc-version.sh | 12 +- scripts/kconfig/confdata.c | 75 +++++++ scripts/min-tool-version.sh | 6 + 20 files changed, 877 insertions(+), 26 deletions(-) create mode 100644 kernel/configs/rust.config create mode 100644 rust/.gitignore create mode 100644 rust/Makefile create mode 100644 rust/bindgen_parameters.gitignore +2
-0@@ -37,6 +37,8 @@ *.o *.o.* *.patch +*.rmeta +*.rsi *.s *.so *.so.dbgMakefile +12
-2@@ -120,6 +120,15 @@ endif export KBUILD_CHECKSRC +# Enable "clippy" (a linter) as part of the Rust compilation. +# +# Use 'make CLIPPY=1' to enable it. +ifeq ("$(origin CLIPPY)", "command line") + KBUILD_CLIPPY := $(CLIPPY) +endif + +export KBUILD_CLIPPY + # Use make M=dir or set the environment variable KBUILD_EXTMOD to specify the # directory of external module to build. Setting M= takes precedence. ifeq ("$(origin M)", "command line")@@ -267,14 +276,14 @@ no-dot-config-targets := $(clean-targets) \ cscope gtags TAGS tags help% %docs check% coccicheck \ $(version_h) headers headers_% archheaders archscripts \ %asm-generic kernelversion %src-pkg dt_binding_check \ - outputmakefile + outputmakefile rustavailable rustfmt rustfmtcheck # Installation targets should not require compiler. Unfortunately, vdso_install # is an exception where build artifacts may be updated. This must be fixed. no-compiler-targets := $(no-dot-config-targets) install dtbs_install \ headers_install modules_install kernelrelease image_name no-sync-config-targets := $(no-dot-config-targets) %install kernelrelease \ image_name -single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.s %.symtypes %/ +single-targets := %.a %.i %.rsi %.ko %.lds %.ll %.lst %.mod %.o %.s %.symtypes %/ config-build := mixed-build :=@@ -436,6 +445,7 @@ else HOSTCC = gcc HOSTCXX = g++ endif +HOSTRUSTC = rustc HOSTPKG_CONFIG = pkg-config KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
Makefile KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ · line 460
Quoted 7 lines
Makefile +4
-0@@ -444,8 +454,26 @@ KBUILD_USERHOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \ KBUILD_USERCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(USERCFLAGS) KBUILD_USERLDFLAGS := $(USERLDFLAGS) +# These flags apply to all Rust code in the tree, including the kernel and +# host programs. +export rust_common_flags := --edition=2021 \ + -Zbinary_dep_depinfo=y \
Let me ask a question about the host_rust rule.
If I directly run the build command from the command line, I get an error, like follows:
$ rustc --edition=2021 -Zbinary_dep_depinfo=y -Dunsafe_op_in_unsafe_fn -Drust_2018_idioms -Dunreachable_pub -Dnon_ascii_idents -Wmissing_docs -Drustdoc::missing_crate_level_docs -Dclippy::correctness -Dclippy::style -Dclippy::suspicious -Dclippy::complexity -Dclippy::perf -Dclippy::let_unit_value -Dclippy::mut_mut -Dclippy::needless_bitwise_bool -Dclippy::needless_continue -Wclippy::dbg_macro -O -Cstrip=debuginfo -Zallow-features= --emit=dep-info,link --out-dir=scripts/ ../scripts/generate_rust_target.rs error: the option `Z` is only accepted on the nightly compiler
This is the same command recorded as inscripts/.generate_rust_target.cmd
Why no error when it is invoked from Makefile?
I have not figured out where this difference comes from.
Signature · 2 lines
Best Regards Masahiro Yamada
Re: [PATCH v9 23/27] Kbuild: add Rust support
Hi Masahiro,
Quoted 2 lines · Masahiro Yamada
I have not figured out where this difference comes from.
It is the RUSTC_BOOTSTRAP environment variable: it allows to use unstable featuers in the stable compiler.
We currently set it in the global Makefile, but we could be more explicit and do it on each command if you think that would be better.
If you want that we keep using the global export, then we can add a comment explaining this to clarify.
Cheers,
Miguel
Re: [PATCH v9 23/27] Kbuild: add Rust support
Quoted 9 lines · Miguel Ojeda
Hi Masahiro,
Quoted 2 lines · Masahiro Yamada
I have not figured out where this difference comes from.
It is the
RUSTC_BOOTSTRAPenvironment variable: it allows to use unstable featuers in the stable compiler.
Thanks, I learned a new thing.
Quoted 3 lines
We currently set it in the global
Makefile, but we could be more explicit and do it on each command if you think that would be better.
I checked "make compile_commands.json", but it collects build commands only for C.
I did not see any bad scenario with the current approach.
Quoted 6 lines
If you want that we keep using the global export, then we can add a comment explaining this to clarify.
Cheers,
Miguel
--
Best Regards
Masahiro Yamada
Miguel Ojeda
maintainerPATCH v9 24/27 docs: add Rust documentation
In reply to Miguel OjedaMost of the documentation for Rust is written within the source code itself, as it is idiomatic for Rust projects. This applies to both the shared infrastructure at rust/ as well as any other Rust module (e.g. drivers) written across the kernel.
However, these documents contain general information that does not fit particularly well in the source code, like the Quick Start guide.
It also contains a few other small changes elsewhere in the documentation folder.
--- Documentation/doc-guide/kernel-doc.rst | 3 + Documentation/index.rst | 1 + Documentation/kbuild/kbuild.rst | 17 ++ Documentation/kbuild/makefiles.rst | 50 ++++- Documentation/process/changes.rst | 41 ++++ Documentation/rust/arch-support.rst | 18 ++ Documentation/rust/coding-guidelines.rst | 216 +++++++++++++++++++ Documentation/rust/general-information.rst | 79 +++++++ Documentation/rust/index.rst | 22 ++ Documentation/rust/quick-start.rst | 232 +++++++++++++++++++++ 10 files changed, 675 insertions(+), 4 deletions(-) create mode 100644 Documentation/rust/arch-support.rst create mode 100644 Documentation/rust/coding-guidelines.rst create mode 100644 Documentation/rust/general-information.rst create mode 100644 Documentation/rust/index.rst create mode 100644 Documentation/rust/quick-start.rst
Documentation/doc-guide/kernel-doc.rst +3 -0
@@ -12,6 +12,9 @@ when it is embedded in source files. reasons. The kernel source contains tens of thousands of kernel-doc comments. Please stick to the style described here. +.. note:: kernel-doc does not cover Rust code: please see + Documentation/rust/general-information.rst instead. + The kernel-doc structure is extracted from the comments, and proper `Sphinx C Domain`_ function and type descriptions with anchors are generated from them. The descriptions are filtered for special kernel-doc
Documentation/index.rst +1 -0
@@ -82,6 +82,7 @@ merged much easier. maintainer/index fault-injection/index livepatch/index + rust/index Kernel API documentation
Documentation/kbuild/kbuild.rst +17 -0
@@ -48,6 +48,10 @@ KCFLAGS ------- Additional options to the C compiler (for built-in and modules). +KRUSTFLAGS +---------- +Additional options to the Rust compiler (for built-in and modules). + CFLAGS_KERNEL ------------- Additional options for $(CC) when used to compile
@@ -57,6 +61,15 @@ CFLAGS_MODULE ------------- Additional module specific options to use for $(CC). +RUSTFLAGS_KERNEL +---------------- +Additional options for $(RUSTC) when used to compile +code that is compiled as built-in. + +RUSTFLAGS_MODULE +---------------- +Additional module specific options to use for $(RUSTC). + LDFLAGS_MODULE -------------- Additional options used for $(LD) when linking modules.
@@ -69,6 +82,10 @@ HOSTCXXFLAGS ------------ Additional flags to be passed to $(HOSTCXX) when building host programs. +HOSTRUSTFLAGS +------------- +Additional flags to be passed to $(HOSTRUSTC) when building host programs. + HOSTLDFLAGS ----------- Additional flags to be passed when linking host programs.
Documentation/kbuild/makefiles.rst +46 -4
80 lines not on this page · raw message
Documentation/process/changes.rst +41 -0
65 lines not on this page · raw message
Documentation/rust/arch-support.rst new file +18 -0
@@ -0,0 +1,18 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Arch Support +============ + +Currently, the Rust compiler (``rustc``) uses LLVM for code generation, +which limits the supported architectures that can be targeted. In addition, +support for building the kernel with LLVM/Clang varies (please see +Documentation/kbuild/llvm.rst). This support is needed for ``bindgen`` +which uses ``libclang``. + +Below is a general summary of architectures that currently work. Level of +support corresponds to ``S`` values in the ``MAINTAINERS`` file. + +============ ================ ============================================== +Architecture Level of support Constraints +============ ================ ============================================== +============ ================ ==============================================
Documentation/rust/coding-guidelines.rst new file +216 -0
216 lines not on this page · raw message
Documentation/rust/general-information.rst new file +79 -0
79 lines not on this page · raw message
Documentation/rust/index.rst new file +22 -0
@@ -0,0 +1,22 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Rust +==== + +Documentation related to Rust within the kernel. To start using Rust +in the kernel, please read the quick-start.rst guide. + +.. toctree:: + :maxdepth: 1 + + quick-start + general-information + coding-guidelines + arch-support + +.. only:: subproject and html + + Indices + ======= + + * :ref:`genindex`
Documentation/rust/quick-start.rst new file +232 -0
232 lines not on this page · raw message
Signature · 1 line
2.37.1
Miguel Ojeda
maintainerPATCH v9 25/27 x86: enable initial Rust support
In reply to Miguel OjedaNote that only x86_64 is covered and not all features nor mitigations are handled, but it is enough as a starting point and showcases the basics needed to add Rust support for a new architecture.
--- Documentation/rust/arch-support.rst | 1 + arch/x86/Kconfig | 1 + arch/x86/Makefile | 10 ++++++++++ scripts/generate_rust_target.rs | 15 +++++++++++++-- 4 files changed, 25 insertions(+), 2 deletions(-)
Documentation/rust/arch-support.rst +1 -0
@@ -15,4 +15,5 @@ support corresponds to ``S`` values in the ``MAINTAINERS`` file. ============ ================ ============================================== Architecture Level of support Constraints ============ ================ ============================================== +``x86`` Maintained ``x86_64`` only. ============ ================ ==============================================
arch/x86/Kconfig +1 -0
@@ -258,6 +258,7 @@ config X86 select HAVE_STATIC_CALL_INLINE if HAVE_OBJTOOL select HAVE_PREEMPT_DYNAMIC_CALL select HAVE_RSEQ + select HAVE_RUST if X86_64 select HAVE_SYSCALL_TRACEPOINTS select HAVE_UACCESS_VALIDATION if HAVE_OBJTOOL select HAVE_UNSTABLE_SCHED_CLOCK
arch/x86/Makefile +10 -0
@@ -68,6 +68,7 @@ export BITS # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53383 # KBUILD_CFLAGS += -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx +KBUILD_RUSTFLAGS += -Ctarget-feature=-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-avx,-avx2 ifeq ($(CONFIG_X86_KERNEL_IBT),y) #
@@ -155,8 +156,17 @@ else cflags-$(CONFIG_GENERIC_CPU) += -mtune=generic KBUILD_CFLAGS += $(cflags-y) + rustflags-$(CONFIG_MK8) += -Ctarget-cpu=k8 + rustflags-$(CONFIG_MPSC) += -Ctarget-cpu=nocona + rustflags-$(CONFIG_MCORE2) += -Ctarget-cpu=core2 + rustflags-$(CONFIG_MATOM) += -Ctarget-cpu=atom + rustflags-$(CONFIG_GENERIC_CPU) += -Ztune-cpu=generic + KBUILD_RUSTFLAGS += $(rustflags-y) + KBUILD_CFLAGS += -mno-red-zone KBUILD_CFLAGS += -mcmodel=kernel + KBUILD_RUSTFLAGS += -Cno-redzone=y + KBUILD_RUSTFLAGS += -Ccode-model=kernel endif #
scripts/generate_rust_target.rs +13 -2
@@ -148,8 +148,19 @@ fn main() {let mut ts =TargetSpec ::new ();// `llvm-target`s are taken from `scripts/Makefile.clang`. -if cfg.has ("DUMMY_ARCH" ) { - ts.push ("arch" ,"dummy_arch" ); +if cfg.has ("X86_64" ) { + ts.push ("arch" ,"x86_64" ); + ts.push ( +"data-layout" , +"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" , + ); +let mut features ="-3dnow,-3dnowa,-mmx,+soft-float" .to_string (); +if cfg.has ("RETPOLINE" ) { + features +=",+retpoline-external-thunk" ; + } + ts.push ("features" , features); + ts.push ("llvm-target" ,"x86_64-linux-gnu" ); + ts.push ("target-pointer-width" ,"64" ); }else {panic! ("Unsupported architecture" ); }
Signature · 1 line
2.37.1
Re: [PATCH v9 25/27] x86: enable initial Rust support
Quoted 6 lines · Miguel Ojeda
Note that only
x86_64is covered and not all features nor mitigations are handled, but it is enough as a starting point and showcases the basics needed to add Rust support for a new architecture.Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 26/27 samples: add first Rust examples
In reply to Miguel OjedaThe beginning of a set of Rust modules that showcase how Rust modules look like and how to use the abstracted kernel features.
It also includes an example of a Rust host program with several modules.
These samples also double as tests in the CI.
--- samples/Kconfig | 2 ++ samples/Makefile | 1 + samples/rust/Kconfig | 30 ++++++++++++++++++++++++ samples/rust/Makefile | 5 ++++ samples/rust/hostprogs/.gitignore | 3 +++ samples/rust/hostprogs/Makefile | 5 ++++ samples/rust/hostprogs/a.rs | 7 ++++++ samples/rust/hostprogs/b.rs | 5 ++++ samples/rust/hostprogs/single.rs | 12 ++++++++++ samples/rust/rust_minimal.rs | 38 +++++++++++++++++++++++++++++++ 10 files changed, 108 insertions(+) create mode 100644 samples/rust/Kconfig create mode 100644 samples/rust/Makefile create mode 100644 samples/rust/hostprogs/.gitignore create mode 100644 samples/rust/hostprogs/Makefile create mode 100644 samples/rust/hostprogs/a.rs create mode 100644 samples/rust/hostprogs/b.rs create mode 100644 samples/rust/hostprogs/single.rs create mode 100644 samples/rust/rust_minimal.rs
samples/Kconfig +2 -0
@@ -263,6 +263,8 @@ config SAMPLE_CORESIGHT_SYSCFG This demonstrates how a user may create their own CoreSight configurations and easily load them into the system at runtime. +source "samples/rust/Kconfig" + endif # SAMPLES config HAVE_SAMPLE_FTRACE_DIRECT
samples/Makefile +1 -0
@@ -35,3 +35,4 @@ subdir-$(CONFIG_SAMPLE_WATCH_QUEUE) += watch_queue obj-$(CONFIG_DEBUG_KMEMLEAK_TEST) += kmemleak/ obj-$(CONFIG_SAMPLE_CORESIGHT_SYSCFG) += coresight/ obj-$(CONFIG_SAMPLE_FPROBE) += fprobe/ +obj-$(CONFIG_SAMPLES_RUST) += rust/
samples/rust/Kconfig new file +30 -0
@@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0 + +menuconfig SAMPLES_RUST + bool "Rust samples" + depends on RUST + help + You can build sample Rust kernel code here. + + If unsure, say N. + +if SAMPLES_RUST + +config SAMPLE_RUST_MINIMAL + tristate "Minimal" + help + This option builds the Rust minimal module sample. + + To compile this as a module, choose M here: + the module will be called rust_minimal. + + If unsure, say N. + +config SAMPLE_RUST_HOSTPROGS + bool "Host programs" + help + This option builds the Rust host program samples. + + If unsure, say N. + +endif # SAMPLES_RUST
samples/rust/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o + +subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
samples/rust/hostprogs/.gitignore new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +single
samples/rust/hostprogs/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +hostprogs-always-y := single + +single-rust := y
samples/rust/hostprogs/a.rs new file +7 -0
@@ -0,0 +1,7 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust single host program sample: module `a`. + +pub (crate )fn f (x:i32 ) { +println! ("The number is {}." , x); +}
samples/rust/hostprogs/b.rs new file +5 -0
@@ -0,0 +1,5 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust single host program sample: module `b`. + +pub (crate )const CONSTANT :i32 =42 ;
samples/rust/hostprogs/single.rs new file +12 -0
@@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust single host program sample. + +mod a; +mod b; + +fn main () { +println! ("Hello world!" ); + + a::f (b::CONSTANT ); +}
samples/rust/rust_minimal.rs new file +38 -0
@@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust minimal sample. + +use kernel::prelude::*; + +module! { +type :RustMinimal , +name :b"rust_minimal" , + author:b"Rust for Linux Contributors" , + description:b"Rust minimal sample" , + license:b"GPL" , +} + +struct RustMinimal { + numbers:Vec <i32 >, +} + +impl kernel::Module for RustMinimal { +fn init (_module: &'staticThisModule ) ->Result <Self > { +pr_info! ("Rust minimal sample (init)\n" ); +pr_info! ("Am I built-in? {}\n" , !cfg! (MODULE )); + +let mut numbers =Vec ::new (); + numbers.try_push (72 )?; + numbers.try_push (108 )?; + numbers.try_push (200 )?; + +Ok (RustMinimal { numbers }) + } +} + +impl Drop for RustMinimal { +fn drop (&mut self) { +pr_info! ("My numbers are {:?}\n" , self.numbers); +pr_info! ("Rust minimal sample (exit)\n" ); + } +}
Signature · 1 line
2.37.1
Re: [PATCH v9 26/27] samples: add first Rust examples
samples/rust/rust_minimal.rs line 38
Quoted 6 lines
samples/rust/rust_minimal.rs new file +6
-0+impl Drop for RustMinimal { +fn drop (&mut self) { +pr_info! ("My numbers are {:?}\n" , self.numbers); +pr_info! ("Rust minimal sample (exit)\n" ); + } +}
I wonder if it would make more sense to implement exit() in kernel::Module, just for the sake of uniformity.
Re: [PATCH v9 26/27] samples: add first Rust examples
Quoted 3 lines · Konstantin Shelekhin
I wonder if it would make more sense to implement
exit()inkernel::Module, just for the sake of uniformity.
Do you mean uniformity with respect to the C side?
Thanks for taking a look!
Cheers,
Miguel
Re: [PATCH v9 26/27] samples: add first Rust examples
Quoted 7 lines · Miguel Ojeda
Quoted 3 lines · Konstantin Shelekhin
I wonder if it would make more sense to implement
exit()inkernel::Module, just for the sake of uniformity.Do you mean uniformity with respect to the C side?
Yeah. It's weird that entry point is implemented in Module while exit point in Drop.
Re: [PATCH v9 26/27] samples: add first Rust examples
In reply to Miguel OjedaQuoted 10 lines · Miguel Ojeda
The beginning of a set of Rust modules that showcase how Rust modules look like and how to use the abstracted kernel features.
It also includes an example of a Rust host program with several modules.
These samples also double as tests in the CI.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Miguel Ojeda
maintainerPATCH v9 27/27 MAINTAINERS: Rust
In reply to Miguel OjedaMiguel, Alex and Wedson will be maintaining the Rust support.
Boqun, Gary and Björn will be reviewers.
--- MAINTAINERS | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
MAINTAINERS +18 -0
@@ -17477,6 +17477,24 @@ L: [email protected] S: Maintained F: drivers/infiniband/ulp/rtrs/ +RUST +M: Miguel Ojeda <[email protected]> +M: Alex Gaynor <[email protected]> +M: Wedson Almeida Filho <[email protected]> +R: Boqun Feng <[email protected]> +R: Gary Guo <[email protected]> +R: Björn Roy Baron <[email protected]> +L: [email protected] +S: Supported +W: https://github.com/Rust-for-Linux/linux +B: https://github.com/Rust-for-Linux/linux/issues +T: git https://github.com/Rust-for-Linux/linux.git rust-next +F: Documentation/rust/ +F: rust/ +F: samples/rust/ +F: scripts/*rust* +K: \b(?i:rust)\b + RXRPC SOCKETS (AF_RXRPC) M: David Howells <[email protected]> M: Marc Dionne <[email protected]>
Signature · 1 line
2.37.1
Re: [PATCH v9 27/27] MAINTAINERS: Rust
Quoted 6 lines · Miguel Ojeda
Miguel, Alex and Wedson will be maintaining the Rust support.
Boqun, Gary and Björn will be reviewers.
Co-developed-by Alex Gaynor <[email protected]>Signed-off-by Alex Gaynor <[email protected]>
Signature · 1 line
Kees Cook
Re: [PATCH v9 27/27] MAINTAINERS: Rust
Quoted 2 lines · Kees Cook
Reviewed-by Kees Cook <[email protected]>
Thanks a lot for all the reviews Kees!
In case you are wondering, I removed your previous Review of this one too since we got some new R lines. I guessed you would have been OK either way given new names are trivial, but it did change a fair % of the patch, so I played it safe... :-)
Cheers,
Miguel