mbox series

[v7,0/7] Add Rust build support, ARM PL011 device impl

Message ID 20240815-rust-pl011-v7-0-975135e98831@linaro.org (mailing list archive)
Headers show
Series Add Rust build support, ARM PL011 device impl | expand

Message

Manos Pitsidianakis Aug. 15, 2024, 11:42 a.m. UTC
Changes
=======

- Incorporated changes by Paolo Bonzini and Junjie Mao as a result of
  discussion on the previous patch series version
- Included two squash patches from
  <20240814090820.1251026-1-junjie.mao@intel.com>
  Junjie Mao (2):
    meson: subprojects: Specify Rust edition by rust_std=20XX
    rust: Specify Rust edition by rust_std=20XX

Outstanding issues
==================

Outstanding issues that are not blocking for merge are:

- Cross-compilation for aarch64 is not possible out-of-the-box because of this bug:
  <https://github.com/rust-lang/rust/issues/125619> in llvm which when
  fixed, must be ported to upstream rust's llvm fork. Since the problem
  is an extraneous symbol we could strip it with objcopy -N|--strip-symbol
- Adding more than one Rust device ends up with duplicate symbols from
  rust std library because we are linking as whole archives because...
  constructors are stripped by the linker otherwise :( It can be worked
  around if a single Rust library is built with all the devices as
  dependencies which is then linked to qemu. The fix is a small change
  which I will add either in a next version or when a new Rust device is
  added.

Previous version was: <rust-pl011-rfc-v6.git.manos.pitsidianakis@linaro.org>

---
Hello everyone,

This series adds:

- build system support for the Rust compiler
- a small Rust library, qemu-api, which includes bindings to QEMU's C
  interface generated with bindgen
- a proof of concept ARM PL011 device implementation in Rust, chosen for
  its low complexity. The device is used in the arm virt machine if qemu
  is compiled with rust enabled (./configure --enable-rust [...])

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

---
Manos Pitsidianakis (5):
      build-sys: Add rust feature option
      rust: add bindgen step as a meson dependency
      .gitattributes: add Rust diff and merge attributes
      rust: add crate to expose bindings and interfaces
      rust: add PL011 device model

Paolo Bonzini (2):
      Require meson version 1.5.0
      configure, meson: detect Rust toolchain

 MAINTAINERS                                        |  20 +
 configure                                          |  50 +-
 meson.build                                        |  77 ++-
 rust/wrapper.h                                     |  39 ++
 .gitattributes                                     |   3 +
 Kconfig                                            |   1 +
 Kconfig.host                                       |   3 +
 hw/arm/Kconfig                                     |  33 +-
 meson_options.txt                                  |   3 +
 python/scripts/vendor.py                           |   4 +-
 python/wheels/meson-1.2.3-py3-none-any.whl         | Bin 964928 -> 0 bytes
 python/wheels/meson-1.5.0-py3-none-any.whl         | Bin 0 -> 959846 bytes
 pythondeps.toml                                    |   2 +-
 rust/.gitignore                                    |   3 +
 rust/Kconfig                                       |   1 +
 rust/hw/Kconfig                                    |   2 +
 rust/hw/char/Kconfig                               |   3 +
 rust/hw/char/meson.build                           |   1 +
 rust/hw/char/pl011/.gitignore                      |   2 +
 rust/hw/char/pl011/Cargo.lock                      | 125 +++++
 rust/hw/char/pl011/Cargo.toml                      |  26 +
 rust/hw/char/pl011/README.md                       |  31 ++
 rust/hw/char/pl011/meson.build                     |  21 +
 rust/hw/char/pl011/rustfmt.toml                    |   1 +
 rust/hw/char/pl011/src/definitions.rs              |  26 +
 rust/hw/char/pl011/src/device.rs                   | 586 +++++++++++++++++++++
 rust/hw/char/pl011/src/device_class.rs             |  58 ++
 rust/hw/char/pl011/src/lib.rs                      | 584 ++++++++++++++++++++
 rust/hw/char/pl011/src/memory_ops.rs               |  56 ++
 rust/hw/meson.build                                |   1 +
 rust/meson.build                                   |  11 +
 rust/qemu-api/.gitignore                           |   2 +
 rust/qemu-api/Cargo.lock                           |   7 +
 rust/qemu-api/Cargo.toml                           |  23 +
 rust/qemu-api/README.md                            |  17 +
 rust/qemu-api/build.rs                             |  13 +
 rust/qemu-api/meson.build                          |  17 +
 rust/qemu-api/rustfmt.toml                         |   1 +
 rust/qemu-api/src/bindings.rs                      |   7 +
 rust/qemu-api/src/definitions.rs                   | 108 ++++
 rust/qemu-api/src/device_class.rs                  | 128 +++++
 rust/qemu-api/src/lib.rs                           | 100 ++++
 rust/qemu-api/src/tests.rs                         |  48 ++
 rust/rustfmt.toml                                  |   7 +
 scripts/archive-source.sh                          |   5 +-
 scripts/make-release                               |   5 +-
 scripts/meson-buildoptions.sh                      |   3 +
 scripts/rustc_args.py                              |  84 +++
 subprojects/.gitignore                             |  11 +
 subprojects/arbitrary-int-1-rs.wrap                |   7 +
 subprojects/bilge-0.2-rs.wrap                      |   7 +
 subprojects/bilge-impl-0.2-rs.wrap                 |   7 +
 subprojects/either-1-rs.wrap                       |   7 +
 subprojects/itertools-0.11-rs.wrap                 |   7 +
 .../packagefiles/arbitrary-int-1-rs/meson.build    |  19 +
 subprojects/packagefiles/bilge-0.2-rs/meson.build  |  29 +
 .../packagefiles/bilge-impl-0.2-rs/meson.build     |  45 ++
 subprojects/packagefiles/either-1-rs/meson.build   |  24 +
 .../packagefiles/itertools-0.11-rs/meson.build     |  30 ++
 .../packagefiles/proc-macro-error-1-rs/meson.build |  40 ++
 .../proc-macro-error-attr-1-rs/meson.build         |  32 ++
 .../packagefiles/proc-macro2-1-rs/meson.build      |  31 ++
 subprojects/packagefiles/quote-1-rs/meson.build    |  29 +
 subprojects/packagefiles/syn-2-rs/meson.build      |  40 ++
 .../packagefiles/unicode-ident-1-rs/meson.build    |  20 +
 subprojects/proc-macro-error-1-rs.wrap             |   7 +
 subprojects/proc-macro-error-attr-1-rs.wrap        |   7 +
 subprojects/proc-macro2-1-rs.wrap                  |   7 +
 subprojects/quote-1-rs.wrap                        |   7 +
 subprojects/syn-2-rs.wrap                          |   7 +
 subprojects/unicode-ident-1-rs.wrap                |   7 +
 tests/lcitool/mappings.yml                         |   2 +-
 72 files changed, 2756 insertions(+), 21 deletions(-)
---
base-commit: a733f37aef3b7d1d33bfe2716af88cdfd67ba64e
change-id: 20240814-rust-pl011-v7

Best regards,
--
γαῖα πυρί μιχθήτω

Comments

Peter Maydell Aug. 15, 2024, 5:10 p.m. UTC | #1
On Thu, 15 Aug 2024 at 12:42, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> Outstanding issues that are not blocking for merge are:
>
> - Cross-compilation for aarch64 is not possible out-of-the-box because of this bug:
>   <https://github.com/rust-lang/rust/issues/125619> in llvm which when
>   fixed, must be ported to upstream rust's llvm fork. Since the problem
>   is an extraneous symbol we could strip it with objcopy -N|--strip-symbol

I asked around internally and found somebody willing to do
code review on the proposed LLVM compiler-rt pullreq that fixes
the root of this bug, so hopefully we can get that fixed (though
presumably we might want the workaround anyway given it'll take a
while to percolate through to a rust release).

-- PMM
Junjie Mao Aug. 16, 2024, 8:06 a.m. UTC | #2
On 8/15/2024 7:42 PM, Manos Pitsidianakis wrote:
> Outstanding issues
> ==================
> 
> Outstanding issues that are not blocking for merge are:
> 
> - Cross-compilation for aarch64 is not possible out-of-the-box because of this bug:
>    <https://github.com/rust-lang/rust/issues/125619> in llvm which when
>    fixed, must be ported to upstream rust's llvm fork. Since the problem
>    is an extraneous symbol we could strip it with objcopy -N|--strip-symbol
> - Adding more than one Rust device ends up with duplicate symbols from
>    rust std library because we are linking as whole archives because...
>    constructors are stripped by the linker otherwise :( It can be worked
>    around if a single Rust library is built with all the devices as
>    dependencies which is then linked to qemu. The fix is a small change
>    which I will add either in a next version or when a new Rust device is
>    added.
> 

Hi Manos,

I also noticed that when I tried adding a second device. Some other projects met 
similar issues [1], but no clean solution seems to be available yet. The options 
are:

1) Combining all crates into one staticlib which is linked to the final 
executable. That requires generating one .rs with extern crate decls of all 
enabled crates. In the context of QEMU, different targets may enable different 
set of crates (e.g., some crates have arch constraints), thus one .rs for each 
target will be needed in general.

2) Linking rlibs (or emitted objects) directly with other C objects using the C 
linker. That somehow works (with some tricks) but is not officially supported 
and may break in the future.

I'm working on (1), but would like to have your thoughts and preference on those 
options.

[1] https://github.com/rust-lang/rust/issues/73632

---
Best Regards
Junjie Mao
Manos Pitsidianakis Aug. 16, 2024, 8:17 a.m. UTC | #3
On Fri, 16 Aug 2024, 11:06 Junjie Mao, <junjie.mao@intel.com> wrote:

> On 8/15/2024 7:42 PM, Manos Pitsidianakis wrote:
> > Outstanding issues
> > ==================
> >
> > Outstanding issues that are not blocking for merge are:
> >
> > - Cross-compilation for aarch64 is not possible out-of-the-box because
> of this bug:
> >    <https://github.com/rust-lang/rust/issues/125619> in llvm which when
> >    fixed, must be ported to upstream rust's llvm fork. Since the problem
> >    is an extraneous symbol we could strip it with objcopy
> -N|--strip-symbol
> > - Adding more than one Rust device ends up with duplicate symbols from
> >    rust std library because we are linking as whole archives because...
> >    constructors are stripped by the linker otherwise :( It can be worked
> >    around if a single Rust library is built with all the devices as
> >    dependencies which is then linked to qemu. The fix is a small change
> >    which I will add either in a next version or when a new Rust device is
> >    added.
> >
>
> Hi Manos,
>
> I also noticed that when I tried adding a second device. Some other
> projects met
> similar issues [1], but no clean solution seems to be available yet. The
> options
> are:
>
> 1) Combining all crates into one staticlib which is linked to the final
> executable. That requires generating one .rs with extern crate decls of
> all
> enabled crates. In the context of QEMU, different targets may enable
> different
> set of crates (e.g., some crates have arch constraints), thus one .rs for
> each
> target will be needed in general.
>
> 2) Linking rlibs (or emitted objects) directly with other C objects using
> the C
> linker. That somehow works (with some tricks) but is not officially
> supported
> and may break in the future.
>
> I'm working on (1), but would like to have your thoughts and preference on
> those
> options.
>

Hello Junjie, I have also implemented (1) already (the fix I mentioned in
the cover letter). In general I'd like to do it on a standalone patch so
that it can be separated from the other changes instead of squashing it.

If you have something already too, please share here! I will send mine as a
reply to this thread when I am able. I am not familiar with meson so my
version could be lacking!

Manos



[1] https://github.com/rust-lang/rust/issues/73632
>
> ---
> Best Regards
> Junjie Mao
>
Junjie Mao Aug. 19, 2024, 4:09 a.m. UTC | #4
On 8/16/2024 4:17 PM, Manos Pitsidianakis wrote:
> 
> 
> On Fri, 16 Aug 2024, 11:06 Junjie Mao, <junjie.mao@intel.com 
> <mailto:junjie.mao@intel.com>> wrote:
> 
>     On 8/15/2024 7:42 PM, Manos Pitsidianakis wrote:
>      > Outstanding issues
>      > ==================
>      >
>      > Outstanding issues that are not blocking for merge are:
>      >
>      > - Cross-compilation for aarch64 is not possible out-of-the-box because of
>     this bug:
>      >    <https://github.com/rust-lang/rust/issues/125619
>     <https://github.com/rust-lang/rust/issues/125619>> in llvm which when
>      >    fixed, must be ported to upstream rust's llvm fork. Since the problem
>      >    is an extraneous symbol we could strip it with objcopy -N|--strip-symbol
>      > - Adding more than one Rust device ends up with duplicate symbols from
>      >    rust std library because we are linking as whole archives because...
>      >    constructors are stripped by the linker otherwise :( It can be worked
>      >    around if a single Rust library is built with all the devices as
>      >    dependencies which is then linked to qemu. The fix is a small change
>      >    which I will add either in a next version or when a new Rust device is
>      >    added.
>      >
> 
>     Hi Manos,
> 
>     I also noticed that when I tried adding a second device. Some other projects
>     met
>     similar issues [1], but no clean solution seems to be available yet. The
>     options
>     are:
> 
>     1) Combining all crates into one staticlib which is linked to the final
>     executable. That requires generating one .rs with extern crate decls of all
>     enabled crates. In the context of QEMU, different targets may enable different
>     set of crates (e.g., some crates have arch constraints), thus one .rs for each
>     target will be needed in general.
> 
>     2) Linking rlibs (or emitted objects) directly with other C objects using the C
>     linker. That somehow works (with some tricks) but is not officially supported
>     and may break in the future.
> 
>     I'm working on (1), but would like to have your thoughts and preference on
>     those
>     options.
> 
> 
> Hello Junjie, I have also implemented (1) already (the fix I mentioned in the 
> cover letter). In general I'd like to do it on a standalone patch so that it can 
> be separated from the other changes instead of squashing it.
> 
> If you have something already too, please share here! I will send mine as a 
> reply to this thread when I am able. I am not familiar with meson so my version 
> could be lacking!

Here's my version for your reference. There are still a few places yet to be 
improved:

1. Each virtual device is required to write an additional `variables: {'crate': 
'crate_name'}` in dep decl. It duplicates the crate name which is already given 
in the static_libary() call, "abuses" the dep variables originally for cmake or 
pkg-config, but is the only way I found to include the crate name in the dep.

2. Names of variables and scripts are tentative.

diff --git a/meson.build b/meson.build
index 97f90a9a60..07401b379a 100644
--- a/meson.build
+++ b/meson.build
@@ -3879,6 +3879,8 @@ common_all = static_library('common',
                              dependencies: common_ss.all_dependencies())

  if have_rust and have_system
+  rust_root_crate = find_program('scripts/rust_root_crate.sh')
+
    rust_args += run_command(
      meson.global_source_root() / 'scripts/rustc_args.py',
      '--config-headers', meson.project_build_root() / 'config-host.h',
@@ -3916,6 +3918,8 @@ if have_rust and have_system
        '--allowlist-file', meson.project_build_root() + '/.*'
        ],
      )
+
+  rust_ss = ss.source_set()
    subdir('rust')
  endif

@@ -4013,6 +4017,28 @@ foreach target : target_dirs
    arch_srcs += target_specific.sources()
    arch_deps += target_specific.dependencies()

+  if have_rust and have_system
+    target_rust = rust_ss.apply(config_target, strict: false)
+    crates = []
+    foreach dep : target_rust.dependencies()
+      crates += dep.get_variable('crate')
+    endforeach
+    if crates.length() > 0
+      root_crate = custom_target('rust-' + target + '.rs',
+                                 output: 'rust-' + target + '.rs',
+                                 command: [rust_root_crate] + crates,
+                                 capture: true,
+                                 build_by_default: true,
+                                 build_always_stale: true)
+      rust_lib = static_library('rust-' + target,
+                                root_crate,
+                                dependencies: target_rust.dependencies(),
+                                rust_abi: 'c')
+      arch_deps += declare_dependency(link_whole: [rust_lib])
+    endif
+  endif
+
    # allow using headers from the dependencies but do not include the sources,
    # because this emulator only needs those in "objects".  For external
    # dependencies, the full dependency is included below in the executable.
diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
index 518d4924a9..55d68ffb5c 100644
--- a/rust/hw/char/pl011/meson.build
+++ b/rust/hw/char/pl011/meson.build
@@ -8,7 +8,7 @@ _libpl011_rs = static_library(
    'pl011',
    files('src/lib.rs'),
    override_options: ['rust_std=2021', 'build.rust_std=2021'],
-  rust_abi: 'c',
+  rust_abi: 'rust',
    dependencies: [
      bilge_dep,
      bilge_impl_dep,
@@ -16,6 +16,7 @@ _libpl011_rs = static_library(
    ],
  )

-specific_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
+rust_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
    link_whole: [_libpl011_rs],
+  variables: {'crate': 'pl011'},
  )])
diff --git a/scripts/rust_root_crate.sh b/scripts/rust_root_crate.sh
new file mode 100755
index 0000000000..46d7e8728a
--- /dev/null
+++ b/scripts/rust_root_crate.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+set -eu
+
+cat <<EOF
+/* This file is autogenerated by scripts/rust_root_crate.sh. */
+
+EOF
+
+for crate in $*; do
+    echo "extern crate $crate;"
+done

---
Best Regards
Junjie Mao

> 
> Manos
> 
> 
> 
>     [1] https://github.com/rust-lang/rust/issues/73632
>     <https://github.com/rust-lang/rust/issues/73632>
> 
>     ---
>     Best Regards
>     Junjie Mao
>