Add mechanism to generate rust hw targets that depend on a custom
bindgen target for rust bindings to C.

This way bindings will be created before the rust crate is compiled.

The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
as a target:

ninja aarch64-softmmu-generated.rs

The way the bindings are generated is:

1. All required C headers are included in a single file, in our case
   rust/wrapper.h for convenience. Otherwise we'd have to provide a list
   of headers every time to the bindgen tool.

2. Meson creates a generated_rs target that runs bindgen making sure
   the architecture etc header dependencies are present.

3. The generated_rs target takes a list of files, type symbols,
   function symbols to block from being generated. This is not necessary
   for the bindings to work, but saves us time and space.

4. Meson creates rust hardware target dependencies from the rust_targets
   dictionary defined in rust/meson.build.

   Since we cannot declare a dependency on generated_rs before it is
   declared in meson.build, the rust crate targets must be defined after
   the generated_rs target for each target architecture is defined. This
   way meson sets up the dependency tree properly.

5. After compiling each rust crate with the cargo_wrapper.py script,
   its static library artifact is linked as a `whole-archive` with the
   final binary.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 MAINTAINERS              |   3 ++
 meson.build              |  57 ++++++++++++++++++++
 rust/.gitignore          |   3 ++
 rust/meson.build         | 112 +++++++++++++++++++++++++++++++++++++++
 rust/wrapper.h           |  39 ++++++++++++++
 scripts/cargo_wrapper.py |  18 +++----
 6 files changed, 220 insertions(+), 12 deletions(-)
 create mode 100644 rust/.gitignore
 create mode 100644 rust/meson.build
 create mode 100644 rust/wrapper.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d01bd06ab7..6e7b8207fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4230,6 +4230,9 @@ Rust build system integration
 M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 S: Maintained
 F: scripts/cargo_wrapper.py
+F: rust/meson.build
+F: rust/wrapper.h
+F: rust/.gitignore
 
 Miscellaneous
 -------------
diff --git a/meson.build b/meson.build
index 11b8b146da..71011fd3b3 100644
--- a/meson.build
+++ b/meson.build
@@ -3929,6 +3929,63 @@ foreach target : target_dirs
     lib_deps += dep.partial_dependency(compile_args: true, includes: true)
   endforeach
 
+  if with_rust and target_type == 'system'
+       # FIXME: meson outputs the following warnings, which should be resolved
+       # before merging:
+       # > WARNING: Project specifies a minimum meson_version '>=0.63.0' but
+       # > uses features which were added in newer versions:
+       # > * 0.64.0: {'fs.copyfile'}
+       # > * 1.0.0: {'dependencies arg in rust.bindgen', 'module rust as stable module'}
+      rust_bindgen = import('rust')
+
+      # We need one bindings_rs build target per arch target, so give them
+      # arch-specific names.
+      copy = fs.copyfile('rust/wrapper.h',
+                         target + '_wrapper.h')
+      bindings_rs = rust_bindgen.bindgen(
+        input: copy,
+        dependencies: arch_deps + lib_deps,
+        output: 'bindings-' + target + '.rs',
+        include_directories: include_directories('.', 'include'),
+        args: [
+          '--ctypes-prefix', 'core::ffi',
+          '--formatter', 'rustfmt',
+          '--generate-block',
+          '--generate-cstr',
+          '--impl-debug',
+          '--merge-extern-blocks',
+          '--no-doc-comments',
+          '--no-include-path-detection',
+          '--use-core',
+          '--with-derive-default',
+          '--allowlist-file', meson.project_source_root() + '/include/.*',
+          '--allowlist-file', meson.project_source_root() + '/.*',
+          '--allowlist-file', meson.project_build_root() + '/.*'
+        ],
+      )
+
+      if target in rust_targets
+        rust_hw = ss.source_set()
+        foreach t: rust_targets[target]
+          rust_device_cargo_build = custom_target(t['name'],
+                                       output: t['output'],
+                                       depends: [bindings_rs],
+                                       build_always_stale: true,
+                                       command: t['command'])
+          rust_dep = declare_dependency(link_args: [
+                                          '-Wl,--whole-archive',
+                                          t['output'],
+                                          '-Wl,--no-whole-archive'
+                                          ],
+                                          sources: [rust_device_cargo_build])
+          rust_hw.add(rust_dep)
+        endforeach
+        rust_hw_config = rust_hw.apply(config_target, strict: false)
+        arch_srcs += rust_hw_config.sources()
+        arch_deps += rust_hw_config.dependencies()
+      endif
+  endif
+
   lib = static_library('qemu-' + target,
                  sources: arch_srcs + genh,
                  dependencies: lib_deps,
diff --git a/rust/.gitignore b/rust/.gitignore
new file mode 100644
index 0000000000..1bf71b1f68
--- /dev/null
+++ b/rust/.gitignore
@@ -0,0 +1,3 @@
+# Ignore any cargo development build artifacts; for qemu-wide builds, all build
+# artifacts will go to the meson build directory.
+target
diff --git a/rust/meson.build b/rust/meson.build
new file mode 100644
index 0000000000..5fdc2621a3
--- /dev/null
+++ b/rust/meson.build
@@ -0,0 +1,112 @@
+# Supported hosts
+rust_supported_oses = {
+  'linux': '-unknown-linux-gnu',
+  # 'darwin': '-apple-darwin',
+  # 'windows': '-pc-windows-gnu'
+}
+rust_supported_cpus = ['x86_64', 'aarch64']
+
+# Future-proof the above definitions against any change in the root meson.build file:
+foreach rust_os: rust_supported_oses.keys()
+  if not supported_oses.contains(rust_os)
+    message()
+    warning('UNSUPPORTED OS VALUES IN ' + meson.current_source_dir() + '/meson.build')
+    message()
+    message('This meson.build file claims OS `+' + rust_os + '` is supported but')
+    message('it is not included in the global supported OSes list in')
+    message(meson.global_source_root() + '/meson.build.')
+  endif
+endforeach
+foreach rust_cpu: rust_supported_cpus
+  if not supported_cpus.contains(rust_cpu)
+    message()
+    warning('UNSUPPORTED CPU VALUES IN ' + meson.current_source_dir() + '/meson.build')
+    message()
+    message('This meson.build file claims CPU `+' + rust_cpu + '` is supported but')
+    message('it is not included in the global supported CPUs list in')
+    message(meson.global_source_root() + '/meson.build.')
+  endif
+endforeach
+
+msrv = {
+  'rustc': '1.77.2',
+  'cargo': '1.77.2',
+  'bindgen': '0.69.4',
+}
+
+foreach bin_dep: msrv.keys()
+  bin = find_program(bin_dep, required: true)
+  if bin.version() < msrv[bin_dep]
+    message()
+    error(bin_dep + ' version ' + bin.version() + ' is unsupported: Please upgrade to at least ' + msrv[bin_dep])
+  endif
+endforeach
+
+rust_target_triple = get_option('with_rust_target_triple')
+
+if rust_target_triple == ''
+  if not supported_oses.contains(host_os)
+    message()
+    error('QEMU does not support `' + host_os +'` as a Rust platform.')
+  elif not supported_cpus.contains(host_arch)
+    message()
+    error('QEMU does not support `' + host_arch +'` as a Rust architecture.')
+  endif
+  rust_target_triple = host_arch + rust_supported_oses[host_os]
+  # if host_os == 'windows' and host_arch == 'aarch64'
+  #   rust_target_triple += 'llvm'
+  # endif
+else
+  # verify rust_target_triple if given as an option
+  rustc = find_program('rustc', required: true)
+  rustc_targets = run_command(rustc, '--print', 'target-list', capture: true, check: true).stdout().strip().split()
+  if not rustc_targets.contains(rust_target_triple)
+    message()
+    error('Given rust_target_triple ' + rust_target_triple + ' is not listed in rustc --print target-list output')
+  endif
+endif
+
+rust_targets = {}
+
+cargo_wrapper = [
+  find_program(meson.global_source_root() / 'scripts/cargo_wrapper.py'),
+  '--config-headers', meson.project_build_root() / 'config-host.h',
+  '--meson-build-root', meson.project_build_root(),
+]
+
+if get_option('b_colorout') != 'never'
+  cargo_wrapper += ['--color', 'always']
+endif
+
+if get_option('optimization') in ['0', '1', 'g']
+  rs_build_profile = 'dev'
+else
+  rs_build_profile = 'release'
+endif
+
+subdir('qemu-api')
+
+# Collect metadata for each (crate,qemu-target,compiler-target) combination.
+# Rust meson targets cannot be defined a priori because they depend on bindgen
+# generation that is created for each emulation target separately. Thus Rust
+# meson targets will be defined for each target after the target-specific
+# bindgen dependency is declared.
+rust_hw_target_list = {}
+
+foreach rust_hw_target, rust_hws: rust_hw_target_list
+  foreach rust_hw_dev: rust_hws
+    crate_metadata = {
+      'name': rust_hw_dev['name'],
+      'output': [rust_hw_dev['output']],
+      'command': [cargo_wrapper,
+        '--crate-dir', meson.current_source_dir() / rust_hw_dev['dirname'],
+        '--profile', rs_build_profile,
+        '--target-triple', rust_target_triple,
+        '--private-dir', '@PRIVATE_DIR@',
+        '--outdir', '@OUTDIR@',
+        'build-lib'
+        ]
+      }
+    rust_targets += { rust_hw_target: [crate_metadata] }
+  endforeach
+endforeach
diff --git a/rust/wrapper.h b/rust/wrapper.h
new file mode 100644
index 0000000000..51985f0ef1
--- /dev/null
+++ b/rust/wrapper.h
@@ -0,0 +1,39 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu-io.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "exec/memory.h"
+#include "chardev/char-fe.h"
+#include "hw/clock.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/irq.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "chardev/char-serial.h"
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
index d2c7265461..e7d9238c16 100644
--- a/scripts/cargo_wrapper.py
+++ b/scripts/cargo_wrapper.py
@@ -111,6 +111,8 @@ def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]
 
     env = os.environ
     env["CARGO_ENCODED_RUSTFLAGS"] = cfg
+    env["MESON_BUILD_DIR"] = str(target_dir)
+    env["MESON_BUILD_ROOT"] = str(args.meson_build_root)
 
     return (env, cargo_cmd)
 
@@ -231,19 +233,11 @@ def main() -> None:
         default=[],
     )
     parser.add_argument(
-        "--meson-build-dir",
-        metavar="BUILD_DIR",
-        help="meson.current_build_dir()",
+        "--meson-build-root",
+        metavar="BUILD_ROOT",
+        help="meson.project_build_root(): the root build directory. Example: '/path/to/qemu/build'",
         type=Path,
-        dest="meson_build_dir",
-        required=True,
-    )
-    parser.add_argument(
-        "--meson-source-dir",
-        metavar="SOURCE_DIR",
-        help="meson.current_source_dir()",
-        type=Path,
-        dest="meson_build_dir",
+        dest="meson_build_root",
         required=True,
     )
     parser.add_argument(
-- 
γαῖα πυρί μιχθήτω

Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:

> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
<snip>
> +      # We need one bindings_rs build target per arch target, so give them
> +      # arch-specific names.
> +      copy = fs.copyfile('rust/wrapper.h',
> +                         target + '_wrapper.h')
> +      bindings_rs = rust_bindgen.bindgen(
> +        input: copy,
> +        dependencies: arch_deps + lib_deps,
> +        output: 'bindings-' + target + '.rs',
> +        include_directories: include_directories('.', 'include'),
> +        args: [
> +          '--ctypes-prefix', 'core::ffi',
> +          '--formatter', 'rustfmt',

I guess we also need to detect rustfmt, although it seems to be
non-fatal:

  15:59:27 [alex@debian-arm64:~/l/q/b/rust] review/rust-pl011-v4 + ninja
  [831/3041] Generating bindings for Rust rustmod-bindgen-aarch64-softmmu_wrapper.h
  Failed to run rustfmt: cannot find binary path (non-fatal, continuing)
  [3041/3041] Linking target tests/qtest/qos-test

<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:

> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
> as a target:
>
> ninja aarch64-softmmu-generated.rs
>
> The way the bindings are generated is:
>
> 1. All required C headers are included in a single file, in our case
>    rust/wrapper.h for convenience. Otherwise we'd have to provide a list
>    of headers every time to the bindgen tool.
>
> 2. Meson creates a generated_rs target that runs bindgen making sure
>    the architecture etc header dependencies are present.
>
> 3. The generated_rs target takes a list of files, type symbols,
>    function symbols to block from being generated. This is not necessary
>    for the bindings to work, but saves us time and space.
>
> 4. Meson creates rust hardware target dependencies from the rust_targets
>    dictionary defined in rust/meson.build.
>
>    Since we cannot declare a dependency on generated_rs before it is
>    declared in meson.build, the rust crate targets must be defined after
>    the generated_rs target for each target architecture is defined. This
>    way meson sets up the dependency tree properly.
>
> 5. After compiling each rust crate with the cargo_wrapper.py script,
>    its static library artifact is linked as a `whole-archive` with the
>    final binary.
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
<snip>
> +
> +msrv = {
> +  'rustc': '1.77.2',
> +  'cargo': '1.77.2',
> +  'bindgen': '0.69.4',
> +}

This is still pretty bleeding edge (it even tripped up on the
.cargo/bin/cargo I have installed). This needs to be set to the
baseline which from:

  https://wiki.qemu.org/RustInQemu/2022

Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
says 0.66.0). While it might make sense to delay merging if we are
waiting for one distro to produce a new LTS we shouldn't be needing
rustup by default.


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Alex Bennée <alex.bennee@linaro.org> writes:

> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>
>> Add mechanism to generate rust hw targets that depend on a custom
>> bindgen target for rust bindings to C.
>>
>> This way bindings will be created before the rust crate is compiled.
>>
>> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
>> as a target:
>>
>> ninja aarch64-softmmu-generated.rs
>>
>> The way the bindings are generated is:
>>
>> 1. All required C headers are included in a single file, in our case
>>    rust/wrapper.h for convenience. Otherwise we'd have to provide a list
>>    of headers every time to the bindgen tool.
>>
>> 2. Meson creates a generated_rs target that runs bindgen making sure
>>    the architecture etc header dependencies are present.
>>
>> 3. The generated_rs target takes a list of files, type symbols,
>>    function symbols to block from being generated. This is not necessary
>>    for the bindings to work, but saves us time and space.
>>
>> 4. Meson creates rust hardware target dependencies from the rust_targets
>>    dictionary defined in rust/meson.build.
>>
>>    Since we cannot declare a dependency on generated_rs before it is
>>    declared in meson.build, the rust crate targets must be defined after
>>    the generated_rs target for each target architecture is defined. This
>>    way meson sets up the dependency tree properly.
>>
>> 5. After compiling each rust crate with the cargo_wrapper.py script,
>>    its static library artifact is linked as a `whole-archive` with the
>>    final binary.
>>
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> <snip>
>> +
>> +msrv = {
>> +  'rustc': '1.77.2',
>> +  'cargo': '1.77.2',
>> +  'bindgen': '0.69.4',
>> +}
>
> This is still pretty bleeding edge (it even tripped up on the
> .cargo/bin/cargo I have installed). This needs to be set to the
> baseline which from:
>
>   https://wiki.qemu.org/RustInQemu/2022
>
> Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
> says 0.66.0). While it might make sense to delay merging if we are
> waiting for one distro to produce a new LTS we shouldn't be needing
> rustup by default.

Also bindgen, do we need such a new one? On Trixie:

  Message:

  ../../rust/meson.build:41:4: ERROR: Problem encountered: bindgen version 0.66.1 is unsupported: Please upgrade to at least 0.69.4

  A full log can be found at /home/alex/lsrc/qemu.git/builds/rust/meson-logs/meson-log.txt

  ERROR: meson setup failed

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On Tue, 9 Jul 2024 at 11:53, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> > +msrv = {
> > +  'rustc': '1.77.2',
> > +  'cargo': '1.77.2',
> > +  'bindgen': '0.69.4',
> > +}
>
> This is still pretty bleeding edge (it even tripped up on the
> .cargo/bin/cargo I have installed). This needs to be set to the
> baseline which from:
>
>   https://wiki.qemu.org/RustInQemu/2022
>
> Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
> says 0.66.0). While it might make sense to delay merging if we are
> waiting for one distro to produce a new LTS we shouldn't be needing
> rustup by default.

I suspect that some of the older distro releases in that chart
have already fallen off the end of our supported-platforms set,
so the minimum is probably newer than 1.24.0 now.

My take on this one is that (at some point, not necessarily
right now) we want to look at:

 * what is the actual baseline requirement? We definitely want
   to support "using rustup on an older system" (should be no
   problem) and "current distro building QEMU using the distro's
   rust", I assume. It would certainly be nice to have "building
   QEMU on the older-but-still-in-our-support-list distro releases
   with that distro's rust", but this probably implies not just
   a minimum rust version but also a limited set of crates.
   I think (but forget the details) that some of what we've done
   with Python where we accept that the older-but-still-supported
   distro will end up taking the "download at build time" path
   in the build system might apply also for Rust.
 * what, on the Rust side, is the version requirement? Presumably
   there's some features that are pretty much "we really need
   this and can't do without it" and some which are "this would
   be pretty awkward not to have but if we have to we can implement
   some kind of fallback/alternative with a TODO note to come
   back and clean up when our baseline moves forwards".

At that point we have more information to figure out what
if any tradeoff we want to make.

thanks
-- PMM

On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>  * what is the actual baseline requirement? We definitely want
>    to support "using rustup on an older system" (should be no
>    problem) and "current distro building QEMU using the distro's
>    rust", I assume. It would certainly be nice to have "building
>    QEMU on the older-but-still-in-our-support-list distro releases
>    with that distro's rust", but this probably implies not just
>    a minimum rust version but also a limited set of crates.

I don't think limiting ourselves to the set of crates in the distro is
feasible. It's not the way the language works, for example I tried
checking if the "cstr" crate exists and I didn't find it in Debian.

We can define a known-good set of versions:
* either via Cargo.lock (i.e. recording the versions and downloading
them) or by including sources in the tree
* at any time in qemu.git, at branching time (three times a year), or
on every release

(That's six possibilities overall).

>    I think (but forget the details) that some of what we've done
>    with Python where we accept that the older-but-still-supported
>    distro will end up taking the "download at build time" path
>    in the build system might apply also for Rust.

Yes, and --without-download may be changed to the "--frozen" flag of cargo.

>  * what, on the Rust side, is the version requirement? Presumably
>    there's some features that are pretty much "we really need
>    this and can't do without it" and some which are "this would
>    be pretty awkward not to have but if we have to we can implement
>    some kind of fallback/alternative with a TODO note to come
>    back and clean up when our baseline moves forwards".

Here are the stopping points that I found over the last couple weeks:

1.56.0: 2021 edition
1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
crate, see below)
1.64.0: std::ffi::c_char
1.65.0: Generic Associated Types
1.74.0: Clippy can be configured in Cargo.toml
1.77.0: C string literals, offset_of!

I think 1.59.0 is pretty much the lower bound. Not having offset_of!
will be a bit painful, but it can be worked around (and pretty much
has to be, because 1.77.0 is really new).

As far as I understand, Debian bullseye has 1.63.0 these days and it's
the oldest platform we have.

Paolo

> At that point we have more information to figure out what
> if any tradeoff we want to make.
>
> thanks
> -- PMM
>

On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> 
> Here are the stopping points that I found over the last couple weeks:
> 
> 1.56.0: 2021 edition
> 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> crate, see below)
> 1.64.0: std::ffi::c_char
> 1.65.0: Generic Associated Types
> 1.74.0: Clippy can be configured in Cargo.toml
> 1.77.0: C string literals, offset_of!
> 
> I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> will be a bit painful, but it can be worked around (and pretty much
> has to be, because 1.77.0 is really new).
>

An additional question: does our minimum rust version requirement
indicate that users with this rust version can compile other
dependencies that satisfy QEMU requirements, such as bindgen?

Because I find 1.59.0 can only go to compile bindgen 0.63.0 [1].

[1]: bindgen 0.63.0 MSRV: https://github.com/rust-lang/rust-bindgen/tree/v0.63.0?tab=readme-ov-file#msrv

-Zhao

On Wed, Jul 10, 2024 at 4:48 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> >
> > Here are the stopping points that I found over the last couple weeks:
> >
> > 1.56.0: 2021 edition
> > 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> > crate, see below)
> > 1.64.0: std::ffi::c_char
> > 1.65.0: Generic Associated Types
> > 1.74.0: Clippy can be configured in Cargo.toml
> > 1.77.0: C string literals, offset_of!
> >
> > I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> > will be a bit painful, but it can be worked around (and pretty much
> > has to be, because 1.77.0 is really new).
> >
>
> An additional question: does our minimum rust version requirement
> indicate that users with this rust version can compile other
> dependencies that satisfy QEMU requirements, such as bindgen?

Yes (though in the case of bindgen, like cargo and rustc, we'll use it
from the distro; Debian bookworm has 0.60.1 so that's what we'll have
to stick with).

Paolo

On Wed, Jul 10, 2024 at 04:50:10PM +0200, Paolo Bonzini wrote:
> Date: Wed, 10 Jul 2024 16:50:10 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
> 
> On Wed, Jul 10, 2024 at 4:48 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> > >
> > > Here are the stopping points that I found over the last couple weeks:
> > >
> > > 1.56.0: 2021 edition
> > > 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> > > crate, see below)
> > > 1.64.0: std::ffi::c_char
> > > 1.65.0: Generic Associated Types
> > > 1.74.0: Clippy can be configured in Cargo.toml
> > > 1.77.0: C string literals, offset_of!
> > >
> > > I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> > > will be a bit painful, but it can be worked around (and pretty much
> > > has to be, because 1.77.0 is really new).
> > >
> >
> > An additional question: does our minimum rust version requirement
> > indicate that users with this rust version can compile other
> > dependencies that satisfy QEMU requirements, such as bindgen?
> 
> Yes (though in the case of bindgen, like cargo and rustc, we'll use it
> from the distro; Debian bookworm has 0.60.1 so that's what we'll have
> to stick with).
>

I just did some build tests to hopefully help Manos clarify the gap.

With 1.59.0 rust and 0.60.1 bindgen, I found the big problem is this
version can't support current external crates ("bilge" and
"arbitrary-int").

The lowest possible version of bilge is 0.1.1 (because 0.1.0 contains the
removed unstable feature), but v0.1.1 bilge depends on arbitrary-int
(lowest bound is 1.2.4), which is not supported by v1.59.0 rust. So for
this case either we have to keep raising the minimum rust version, or we
have to reconsider the external crates.

And with 1.63.0, the main problem is core_ffi_c is unstable (once_cell
doesn't matter since it's unneeded). Roughly speaking, it seems that if
we didn't have to use the c type as parameters, this problem could
possibly be worked around.

So maybe 1.63.0 is the proper minimum version and this is aligned with
Debian bullseye.

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>  * what is the actual baseline requirement? We definitely want
>>    to support "using rustup on an older system" (should be no
>>    problem) and "current distro building QEMU using the distro's
>>    rust", I assume. It would certainly be nice to have "building
>>    QEMU on the older-but-still-in-our-support-list distro releases
>>    with that distro's rust", but this probably implies not just
>>    a minimum rust version but also a limited set of crates.
>
> I don't think limiting ourselves to the set of crates in the distro is
> feasible. It's not the way the language works, for example I tried
> checking if the "cstr" crate exists and I didn't find it in Debian.
>
> We can define a known-good set of versions:
> * either via Cargo.lock (i.e. recording the versions and downloading
> them) or by including sources in the tree
> * at any time in qemu.git, at branching time (three times a year), or
> on every release
>
> (That's six possibilities overall).
>
>>    I think (but forget the details) that some of what we've done
>>    with Python where we accept that the older-but-still-supported
>>    distro will end up taking the "download at build time" path
>>    in the build system might apply also for Rust.
>
> Yes, and --without-download may be changed to the "--frozen" flag of cargo.
>
>>  * what, on the Rust side, is the version requirement? Presumably
>>    there's some features that are pretty much "we really need
>>    this and can't do without it" and some which are "this would
>>    be pretty awkward not to have but if we have to we can implement
>>    some kind of fallback/alternative with a TODO note to come
>>    back and clean up when our baseline moves forwards".
>
> Here are the stopping points that I found over the last couple weeks:
>
> 1.56.0: 2021 edition
> 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> crate, see below)
> 1.64.0: std::ffi::c_char
> 1.65.0: Generic Associated Types
> 1.74.0: Clippy can be configured in Cargo.toml
> 1.77.0: C string literals, offset_of!
>
> I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> will be a bit painful, but it can be worked around (and pretty much
> has to be, because 1.77.0 is really new).
>
> As far as I understand, Debian bullseye has 1.63.0 these days and it's
> the oldest platform we have.

I was going to say Bookworm has now superseded Bullseye which will reach
its release + 3 year support point in August. However the version you
mention in the Bookworm one!

>
> Paolo
>
>> At that point we have more information to figure out what
>> if any tradeoff we want to make.
>>
>> thanks
>> -- PMM
>>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >  * what is the actual baseline requirement? We definitely want
> >    to support "using rustup on an older system" (should be no
> >    problem) and "current distro building QEMU using the distro's
> >    rust", I assume. It would certainly be nice to have "building
> >    QEMU on the older-but-still-in-our-support-list distro releases
> >    with that distro's rust", but this probably implies not just
> >    a minimum rust version but also a limited set of crates.
> 
> I don't think limiting ourselves to the set of crates in the distro is
> feasible. It's not the way the language works, for example I tried
> checking if the "cstr" crate exists and I didn't find it in Debian.

Yep, Rust is new enough that it is highly likely that crates will be
missing in multiple distros.

For ordinary users, cargo will happily download the missing pieces
so its not an issue.

For distro packagers, they'll just have to either package up the crates,
or bundle them in their QEMU build. Cargo makes the latter easy at
least. If distros don't want bundling, they'll need to go the more
involved route of packaging deps.

IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
versions we need as the minimum bar.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 7/9/24 06:00, Daniel P. Berrangé wrote:
> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
>> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>>   * what is the actual baseline requirement? We definitely want
>>>     to support "using rustup on an older system" (should be no
>>>     problem) and "current distro building QEMU using the distro's
>>>     rust", I assume. It would certainly be nice to have "building
>>>     QEMU on the older-but-still-in-our-support-list distro releases
>>>     with that distro's rust", but this probably implies not just
>>>     a minimum rust version but also a limited set of crates.
>>
>> I don't think limiting ourselves to the set of crates in the distro is
>> feasible. It's not the way the language works, for example I tried
>> checking if the "cstr" crate exists and I didn't find it in Debian.
> 
> Yep, Rust is new enough that it is highly likely that crates will be
> missing in multiple distros.
> 
> For ordinary users, cargo will happily download the missing pieces
> so its not an issue.
> 
> For distro packagers, they'll just have to either package up the crates,
> or bundle them in their QEMU build. Cargo makes the latter easy at
> least. If distros don't want bundling, they'll need to go the more
> involved route of packaging deps.
> 
> IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
> versions we need as the minimum bar.
> 
> With regards,
> Daniel

I would like to add that, contrary to pip packages for python, it can be 
*trusted* that Rust crates.io will keep packages and their different 
versions, as it was designed to be an immutable registry.

Indeed, users can't remove their package, except for strong security 
issues, to the opposite of pip.

Besides offline compilation scenario, are there any other reason for 
wanting to use distro packages instead of letting cargo do the work?

On Fri, 12 Jul 2024 00:23, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>On 7/9/24 06:00, Daniel P. Berrangé wrote:
>> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
>>> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>>>   * what is the actual baseline requirement? We definitely want
>>>>     to support "using rustup on an older system" (should be no
>>>>     problem) and "current distro building QEMU using the distro's
>>>>     rust", I assume. It would certainly be nice to have "building
>>>>     QEMU on the older-but-still-in-our-support-list distro releases
>>>>     with that distro's rust", but this probably implies not just
>>>>     a minimum rust version but also a limited set of crates.
>>>
>>> I don't think limiting ourselves to the set of crates in the distro is
>>> feasible. It's not the way the language works, for example I tried
>>> checking if the "cstr" crate exists and I didn't find it in Debian.
>> 
>> Yep, Rust is new enough that it is highly likely that crates will be
>> missing in multiple distros.
>> 
>> For ordinary users, cargo will happily download the missing pieces
>> so its not an issue.
>> 
>> For distro packagers, they'll just have to either package up the crates,
>> or bundle them in their QEMU build. Cargo makes the latter easy at
>> least. If distros don't want bundling, they'll need to go the more
>> involved route of packaging deps.
>> 
>> IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
>> versions we need as the minimum bar.
>> 
>> With regards,
>> Daniel
>
>I would like to add that, contrary to pip packages for python, it can be 
>*trusted* that Rust crates.io will keep packages and their different 
>versions, as it was designed to be an immutable registry.
>
>Indeed, users can't remove their package, except for strong security 
>issues, to the opposite of pip.
>
>Besides offline compilation scenario, are there any other reason for 
>wanting to use distro packages instead of letting cargo do the work?

Only distribution packaging is the concern here, where packaged build 
dependencies may be required.

On Thu, Jul 4, 2024 at 2:16 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
> as a target:
>
> ninja aarch64-softmmu-generated.rs

Is there anything target-dependent in these files? You can generate it
just once I think.


> +  if with_rust and target_type == 'system'
> +       # FIXME: meson outputs the following warnings, which should be resolved
> +       # before merging:
> +       # > WARNING: Project specifies a minimum meson_version '>=0.63.0' but
> +       # > uses features which were added in newer versions:
> +       # > * 0.64.0: {'fs.copyfile'}

you can use configure_file() instead.

> +       # > * 1.0.0: {'dependencies arg in rust.bindgen', 'module rust as stable module'}

You can use

if meson.version().version_compare('>=1.0.0')
  ...
else
  error('Rust support requires Meson version 1.0.0')
endif

> +      if target in rust_targets
> +        rust_hw = ss.source_set()
> +        foreach t: rust_targets[target]
> +          rust_device_cargo_build = custom_target(t['name'],
> +                                       output: t['output'],
> +                                       depends: [bindings_rs],
> +                                       build_always_stale: true,
> +                                       command: t['command'])

Also "console: true".

> +          rust_dep = declare_dependency(link_args: [
> +                                          '-Wl,--whole-archive',
> +                                          t['output'],
> +                                          '-Wl,--no-whole-archive'
> +                                          ],

This is not portable, but I think you can use just "link_whole:
rust_device_cargo_build".

> +msrv = {
> +  'rustc': '1.77.2',
> +  'cargo': '1.77.2',

I think rustc and cargo are always matching, so no need to check both of them?

> +foreach rust_hw_target, rust_hws: rust_hw_target_list
> +  foreach rust_hw_dev: rust_hws

Finding the available devices should be done using Kconfig symbols,
probably by passing the path to the config-devices.mak file to
build.rs via an environment variable.

> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu-io.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/sysbus.h"
> +#include "exec/memory.h"
> +#include "chardev/char-fe.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "hw/irq.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "chardev/char-serial.h"

I think all of these should be target-independent?

> diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> index d2c7265461..e7d9238c16 100644
> --- a/scripts/cargo_wrapper.py
> +++ b/scripts/cargo_wrapper.py
> @@ -111,6 +111,8 @@ def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]
>
>      env = os.environ
>      env["CARGO_ENCODED_RUSTFLAGS"] = cfg
> +    env["MESON_BUILD_DIR"] = str(target_dir)
> +    env["MESON_BUILD_ROOT"] = str(args.meson_build_root)
>
>      return (env, cargo_cmd)
>
> @@ -231,19 +233,11 @@ def main() -> None:
>          default=[],
>      )
>      parser.add_argument(
> -        "--meson-build-dir",
> -        metavar="BUILD_DIR",
> -        help="meson.current_build_dir()",
> +        "--meson-build-root",
> +        metavar="BUILD_ROOT",
> +        help="meson.project_build_root(): the root build directory. Example: '/path/to/qemu/build'",
>          type=Path,
> -        dest="meson_build_dir",
> -        required=True,
> -    )
> -    parser.add_argument(
> -        "--meson-source-dir",
> -        metavar="SOURCE_DIR",
> -        help="meson.current_source_dir()",
> -        type=Path,
> -        dest="meson_build_dir",
> +        dest="meson_build_root",
>          required=True,
>      )
>      parser.add_argument(

Please squash in the previous patch.

Paolo