Re: [RFC PATCH v2 2/5] rust: add bindgen step as a meson dependency

[Paolo Bonzini]

Just one somewhat larger request, otherwise just a collection of ideas.

On Tue, Jun 11, 2024 at 12:34 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> diff --git a/rust/meson.build b/rust/meson.build
> new file mode 100644
> index 0000000000..e9660a3045
> --- /dev/null
> +++ b/rust/meson.build
> @@ -0,0 +1,91 @@
> +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(),
> +  '--meson-build-dir', meson.current_build_dir(),
> +  '--meson-source-dir', meson.current_source_dir(),
> +]
>
> +
> +# TODO: verify rust_target_triple if given as an option
> +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
> +endif
> +
> +if get_option('optimization') in ['0', '1', 'g']
> +  rs_build_type = 'debug'
> +else
> +  rs_build_type = 'release'
> +endif
> +
> +rust_hw_target_list = {}
> +
> +foreach rust_hw_target, rust_hws: rust_hw_target_list
> +  foreach rust_hw_dev: rust_hws
> +    output = meson.current_build_dir() / rust_target_triple / rs_build_type 
> / rust_hw_dev['output']
> +    crate_metadata = {
> +      'name': rust_hw_dev['name'],
> +      'output': [rust_hw_dev['output']],
> +      'output-path': output,
> +      'command': [cargo_wrapper,
> +        '--crate-dir',
> +        meson.current_source_dir() / rust_hw_dev['dirname'],
> +        '--profile',
> +        rs_build_type,
> +        '--target-triple',
> +        rust_target_triple,
> +        '--outdir',
> +        '@OUTDIR@',
> +        'build-lib'

Probably needs to add config-devices.h as well to --config-headers? I
think that we can have just one crate that is built per target,
instead of many small crates like your rust_pl011_cargo. And then that
crate is built many times with a rust/src/ hierarchy that resembles
the meson.build files we use for C.

rust/src/mod.rs:
pub mod hw;

rust/src/hw/mod.rs:
pub mod char;

rust/src/hw/char/mod.rs:
#[cfg(feature = "CONFIG_PL011")] pub mod pl011;

(perhaps just src/? And we could even move all sources down one level
to src/ for QEMU as well? But that can be done later, for now it can
be rust/src/). This basically gives Kconfig integration for free if it
works, so I'd ask you to try doing this for the next version?

Also, sooner or later we'll have to tackle building a set of common
dependencies (similar to common_ss), and then bringing those as a
dependency to the build of the per-target crates. Even if we don't get
to the point of building devices once for all targets, I'd rather at
least avoid having to build a dozen times the common deps of
procedural macros.

This is not trivial, but should not be hard either, by using build.rs
(cargo::rustc-link-search) to add the -Ldependency=/path/to/rlibs
option to the per-target rustc invocations. This may also require
grabbing the compilation log via "cargo build
--message-format=json-render-diagnostics", but that's not hard to do
in cargo_wrapper.py. And unlike the previous request about Kconfig, it
can be done after the first merge.

> +#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"

Please check for any headers included indirectly and whether it's
worth including them here (e.g. it seems that qapi/error.h,
qom/object.h, hw/qdev-core.h should be there).

> diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> index d338effdaa..a36a4fc86d 100644
> --- a/scripts/cargo_wrapper.py
> +++ b/scripts/cargo_wrapper.py
> @@ -94,6 +94,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)
>
> @@ -164,6 +166,14 @@ def main() -> None:
>          required=True,
>      )
>      parser.add_argument(
> +        "--meson-build-root",
> +        metavar="BUILD_ROOT",
> +        help="meson.project_build_root()",
> +        type=pathlib.Path,
> +        dest="meson_build_root",
> +        required=True,
> +    )
> +    parser.add_argument(
>          "--meson-source-dir",
>          metavar="SOURCE_DIR",
>          help="meson.current_source_dir()",

Probably all of cargo_wrapper.py can be moved to this patch.

Paolo