[RFC,v3,2/5] rust: add bindgen step as a meson dependency

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              |  56 +++++++++++++++++
 rust/.gitignore          |   3 +
 rust/meson.build         | 129 +++++++++++++++++++++++++++++++++++++++
 rust/wrapper.h           |  39 ++++++++++++
 scripts/cargo_wrapper.py |  10 +++
 6 files changed, 240 insertions(+)
 create mode 100644 rust/.gitignore
 create mode 100644 rust/meson.build
 create mode 100644 rust/wrapper.h

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
>
<snip>
> +
> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> +msrv = {
> +  'rustc': '1.79.0',
> +  'cargo': '1.79.0',
> +  'bindgen': '0.69.4',
> +}

So for Debian Bookworm this comes out as:

  msrv = {
    'rustc': '1.79.0',
    'cargo': '1.79.0',
    'bindgen': '0.69.4',
  }

I shall have to see how close Trixie is ;-)

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
>
<snip>
> +
> +
> +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(),
> +]

I'm unclear what the difference between meson-build-root and
meson-build-dir is?

We also end up defining crate-dir and outdir. Aren't these all
derivable from whatever module we are building?

> +
> +if get_option('b_colorout') != 'never'
> +  cargo_wrapper += ['--color', 'always']
> +endif
> +
> +if get_option('optimization') in ['0', '1', 'g']
> +  rs_build_type = 'debug'
> +else
> +  rs_build_type = 'release'
> +endif
> +
> +# 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
> +    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'
> +        ]
> +      }
> +    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..bcf808c8d7
> --- /dev/null
> +++ b/rust/wrapper.h
> @@ -0,0 +1,39 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2020 Fabrice Bellard
> + *
> + * 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 927336f80e..833e0e55f8 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)
>  
> @@ -234,6 +236,14 @@ def main() -> None:
>          required=True,
>      )
>      parser.add_argument(
> +        "--meson-build-root",
> +        metavar="BUILD_ROOT",
> +        help="meson.project_build_root()",
> +        type=Path,
> +        dest="meson_build_root",
> +        required=True,
> +    )
> +    parser.add_argument(
>          "--meson-source-dir",
>          metavar="SOURCE_DIR",
>          help="meson.current_source_dir()",

On Thu, Jun 20, 2024 at 1:10 PM Alex Bennée <alex.bennee@linaro.org> wrote:
> > +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> > +msrv = {
> > +  'rustc': '1.79.0',
> > +  'cargo': '1.79.0',
> > +  'bindgen': '0.69.4',
> > +}
>
> So for Debian Bookworm this comes out as:
>
>   msrv = {
>     'rustc': '1.79.0',
>     'cargo': '1.79.0',
>     'bindgen': '0.69.4',
>   }

I think it's 0.60.1 bindgen and 1.63.0 rustc/cargo? That means we
don't have generic associated types (1.65), which are nice to have but
not absolutely necessary.

The only other one with an old version is Ubuntu 22.04 (1.58.1), but
it has 1.75.0 in updates

Paolo

On 6/19/24 13:13, Manos Pitsidianakis wrote:
> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> +msrv = {
> +  'rustc': '1.79.0',
> +  'cargo': '1.79.0',
> +  'bindgen': '0.69.4',
> +}

A note for other rust newbies:

These versions are pretty darn close to actual minima.  Ubuntu 24.04 packages rust 1.77, 
which does not support (but has a warning reserving syntax for)

> +    println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");

Since even the newest distros will not have current enough rust versions, we must rely on 
'rustup'.  This may be available even on older distros; for instance Ubuntu 22.04 has 
rustup via 'snap'.

I think this is good enough for rust development within qemu, but it may require that the 
configure switch be opt-in: default no rather than default auto.


r~

On Thu, 20 Jun 2024 15:34, Paolo Bonzini <pbonzini@redhat.com> wrote:
>On Thu, Jun 20, 2024 at 1:10 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>> > +# FIXME: These are the latest stable versions, refine to actual minimum ones.
>> > +msrv = {
>> > +  'rustc': '1.79.0',
>> > +  'cargo': '1.79.0',
>> > +  'bindgen': '0.69.4',
>> > +}
>>
>> So for Debian Bookworm this comes out as:
>>
>>   msrv = {
>>     'rustc': '1.79.0',
>>     'cargo': '1.79.0',
>>     'bindgen': '0.69.4',
>>   }
>
>I think it's 0.60.1 bindgen and 1.63.0 rustc/cargo? That means we
>don't have generic associated types (1.65), which are nice to have but
>not absolutely necessary.
>
>The only other one with an old version is Ubuntu 22.04 (1.58.1), but
>it has 1.75.0 in updates
>
>Paolo

1.63 is definitely old at this point but we might still be in luck, 
(Except for bindgen). I will try running cargo-msrv[0] which finds the 
actual minimum supported version of a codebase with a binary search and 
see if we can give up features if necessary.

[0]: https://github.com/foresterre/cargo-msrv

On Thu, 20 Jun 2024 15:32, Alex Bennée <alex.bennee@linaro.org> wrote:
>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
>>
><snip>
>> +
>> +
>> +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(),
>> +]
>
>I'm unclear what the difference between meson-build-root and
>meson-build-dir is?

Build-dir is the subdir of the current subdir(...) meson.build file

So if we are building under qemu/build, meson_build_root is qemu/build 
and meson_build_dir is qemu/build/rust

>
>We also end up defining crate-dir and outdir. Aren't these all
>derivable from whatever module we are building?

Crate dir is the source directory (i.e. qemu/rust/pl011) that contains 
the crate's manifest file Cargo.toml.

Outdir is where to put the final build artifact for meson to find. We 
could derive that from the build directories and package names somehow 
but I chose to be explicit instead of doing indirect logic to make the 
process less magic.

I know it's a lot so I'm open to simplifications. The only problem is 
that all of these directories, except the crate source code, are defined 
from meson and can change with any refactor we do from the meson side of 
things.

On Thu, 20 Jun 2024 17:01, Richard Henderson <richard.henderson@linaro.org> wrote:
>On 6/19/24 13:13, Manos Pitsidianakis wrote:
>> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
>> +msrv = {
>> +  'rustc': '1.79.0',
>> +  'cargo': '1.79.0',
>> +  'bindgen': '0.69.4',
>> +}
>
>A note for other rust newbies:
>
>These versions are pretty darn close to actual minima.  Ubuntu 24.04 packages rust 1.77, 
>which does not support (but has a warning reserving syntax for)
>
>> +    println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
>


rerun-if-env-changed is not new, I think, but the `cargo::` instead of 
`cargo:` syntax is. Is this what the warning is saying?

Source 
<https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script>:

  Note: The old invocation prefix cargo: (one colon only) is deprecated
  and won’t get any new features. To migrate, use two-colons prefix
  cargo::, which was added in Rust 1.77. If you were using
  cargo:KEY=VALUE for arbitrary links manifest key-value pairs, it is
  encouraged to switch to cargo::metadata=KEY=VALUE. Stick to cargo:
  only if the support of Rust version older than 1.77 is required.

But this is not in any way necessary for us, we can ignore cargo's stale 
build detection and force it from meson.

On 6/20/24 11:36, Manos Pitsidianakis wrote:
> On Thu, 20 Jun 2024 17:01, Richard Henderson <richard.henderson@linaro.org> wrote:
>> On 6/19/24 13:13, Manos Pitsidianakis wrote:
>>> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
>>> +msrv = {
>>> +  'rustc': '1.79.0',
>>> +  'cargo': '1.79.0',
>>> +  'bindgen': '0.69.4',
>>> +}
>>
>> A note for other rust newbies:
>>
>> These versions are pretty darn close to actual minima.  Ubuntu 24.04 packages rust 1.77, 
>> which does not support (but has a warning reserving syntax for)
>>
>>> +    println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
>>
> 
> 
> rerun-if-env-changed is not new, I think, but the `cargo::` instead of `cargo:` syntax is. 
> Is this what the warning is saying?
> 
> Source 
> <https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script>:
> 
>   Note: The old invocation prefix cargo: (one colon only) is deprecated
>   and won’t get any new features. To migrate, use two-colons prefix
>   cargo::, which was added in Rust 1.77. If you were using
>   cargo:KEY=VALUE for arbitrary links manifest key-value pairs, it is
>   encouraged to switch to cargo::metadata=KEY=VALUE. Stick to cargo:
>   only if the support of Rust version older than 1.77 is required.
> 
> But this is not in any way necessary for us, we can ignore cargo's stale build detection 
> and force it from meson.

Yes indeed.  The exact error reported is

error: unsupported output in build script of `pl011 v0.1.0 
(/home/rth/qemu/src/rust/pl011)`: `cargo::rerun-if-env-changed=MESON_BUILD_DIR`
Found a `cargo::key=value` build directive which is reserved for future use.
Either change the directive to `cargo:key=value` syntax (note the single `:`) or upgrade 
your version of Rust.


r~

Hello Zhao!

On Mon, 24 Jun 2024 13:12, Zhao Liu <zhao1.liu@intel.com> wrote:
>Hi Manos,
>
>Thanks for your patch. I have a few comments below:
>
>> diff --git a/meson.build b/meson.build
>> index 3533889852..2b305e745a 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3876,6 +3876,62 @@ foreach target : target_dirs
>>      lib_deps += dep.partial_dependency(compile_args: true, includes: true)
>>    endforeach
>>  
>> +  if with_rust and target_type == 'system'
>> +       # FIXME:
>
>Just nit, FIXME looks a bit ambiguous, is it intended to fix the
>following warning? It could be a bit more specific.


Yes it could, that's true. Basically meson prints this warning when it 
reconfigures the build targets. So to use these newer features in our 
minimum supported meson version the code should be fixed to be backwards 
compatible.

I will add a sentence explaining that this is meson's output.

>
>> +       # > 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 generated_rs target per target, so give them
>> +      # target-specific names.
>> +      copy = fs.copyfile('rust/wrapper.h',
>> +                         target + '_wrapper.h')
>> +      generated_rs = rust_bindgen.bindgen(
>> +        input: copy,
>> +        dependencies: arch_deps + lib_deps,
>> +        output: target + '-generated.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 = custom_target(t['name'],
>> +                                       output: t['output'],
>> +                                       depends: [generated_rs],
>> +                                       build_always_stale: true,
>> +                                       command: t['command'])
>> +          rust_dep = declare_dependency(link_args: [
>> +                                          '-Wl,--whole-archive',
>> +                                          t['output-path'],
>> +                                          '-Wl,--no-whole-archive'
>> +                                          ],
>> +                                          sources: [rust_device_cargo])
>> +          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..435abd3e1c
>> --- /dev/null
>> +++ b/rust/meson.build
>> @@ -0,0 +1,129 @@
>> +# Supported hosts
>> +rust_supported_oses = {
>> +  'linux': '-unknown-linux-gnu',
>> +  'darwin': '-apple-darwin',
>> +  'windows': '-pc-windows-gnu'
>> +}
>
>Does the current test cover windows and apple? If not, I think we can
>add these two platforms later on.

Do you mean the test for supported OSes?

This is for future-proofing the Rust integration in general. I haven't 
been able to compile under macos yet because bindgen cannot find the 
system clang header. I also don't have a windows pc to test it on. But 
it should work theoretically under all three.

>
>> +rust_supported_cpus = ['x86_64', 'aarch64']
>
>Similarly, here I have another question, x-pl011-rust in arm virt machine
>I understand should only run on ARM platform, right? So compiling to
>x86_64 doesn't seem necessary at the moment?

This refers to the host cpu not the virtualized ones, you can run 
qemu-system-aarch64 under x86_64


>> +# 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
>> +
>> +# FIXME: retrieve target triple from meson and construct rust_target_triple
>> +if meson.is_cross_build()
>> +  message()
>> +  error('QEMU does not support cross compiling with Rust enabled.')
>> +endif
>
>Is the check here to avoid inconsistencies between cross-build's target
>and rust_target_triple?
>
>If target consistency is not guaranteed, then it seems custom rust_target_triple
>is useless, right? please educate me if I am wrong ;-)

It is, correct. I was hoping someone on the list would have advice on 
how to best do this (see the FIXME comment right above)

>
>> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
>> +msrv = {
>> +  'rustc': '1.79.0',
>> +  'cargo': '1.79.0',
>> +  'bindgen': '0.69.4',
>> +}
>> +
>> +# rustup = find_program('rustup', required: false)
>> +foreach bin_dep: msrv.keys()
>> +  bin = find_program(bin_dep, required: true)
>> +  if bin.version() < msrv[bin_dep]
>> +    if bin_dep == 'bindgen' and get_option('wrap_mode') != 'nodownload'
>
>What about adding a download info here?
>
>It took a long time for my bindgen-cli to update quietly, and for a
>moment I thought it was stuck here...
>
>e.g., message('Installing bindgen-cli...')
>
>Or can we just report the error and let users install bindgen-cli
>themselves?

Now that I think about it again I agree it's better to not mess with 
user/system installations without asking.

>> +      run_command(cargo, 'install', 'bindgen-cli', capture: true, check: true)
>> +      bin = find_program(bin_dep, required: true)
>> +      if bin.version() >= msrv[bin_dep]
>> +        continue
>> +      endif
>> +    endif
>> +    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
>
>It seems rust_target_triple missed support check, we should use rust_supported_cpus
>+ rust_supported_oses to check rust_target_triple, right? ;-)
>
>> +endif
>> +
>> +
>
>An extra blank line :-)
>
>> +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(),
>> +]
>> +
>> +if get_option('b_colorout') != 'never'
>> +  cargo_wrapper += ['--color', 'always']
>> +endif
>> +
>> +if get_option('optimization') in ['0', '1', 'g']
>> +  rs_build_type = 'debug'
>> +else
>> +  rs_build_type = 'release'
>> +endif
>
>Could we decouple 'optimization' with cargo's opt-level (maybe in the future)?
>
>Or we could add new option to specify custom rust opt-level, which will
>set the environment varialbe CARGO_PROFILE_<profile_name>_OPT_LEVEL and
>override the default opt-level in Cargo.toml.

optimization here refers to LLVM's opt flags directly, so I think it 
makes sense to follow the project-wide compiler settings?
>
>> +# 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
>> +    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'
>> +        ]
>> +      }
>> +    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..bcf808c8d7
>> --- /dev/null
>> +++ b/rust/wrapper.h
>> @@ -0,0 +1,39 @@
>> +/*
>> + * QEMU System Emulator
>> + *
>> + * Copyright (c) 2003-2020 Fabrice Bellard
>
>Here should the author be yourself?

Technically I copy pasted this file from the main.c executable so it's a 
derived work :D

>
>> + *
>> + * 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"
>
>Thanks,
>Zhao
>

Hi Manos,

Thanks for your patch. I have a few comments below:

> diff --git a/meson.build b/meson.build
> index 3533889852..2b305e745a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3876,6 +3876,62 @@ foreach target : target_dirs
>      lib_deps += dep.partial_dependency(compile_args: true, includes: true)
>    endforeach
>  
> +  if with_rust and target_type == 'system'
> +       # FIXME:

Just nit, FIXME looks a bit ambiguous, is it intended to fix the
following warning? It could be a bit more specific.

> +       # > 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 generated_rs target per target, so give them
> +      # target-specific names.
> +      copy = fs.copyfile('rust/wrapper.h',
> +                         target + '_wrapper.h')
> +      generated_rs = rust_bindgen.bindgen(
> +        input: copy,
> +        dependencies: arch_deps + lib_deps,
> +        output: target + '-generated.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 = custom_target(t['name'],
> +                                       output: t['output'],
> +                                       depends: [generated_rs],
> +                                       build_always_stale: true,
> +                                       command: t['command'])
> +          rust_dep = declare_dependency(link_args: [
> +                                          '-Wl,--whole-archive',
> +                                          t['output-path'],
> +                                          '-Wl,--no-whole-archive'
> +                                          ],
> +                                          sources: [rust_device_cargo])
> +          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..435abd3e1c
> --- /dev/null
> +++ b/rust/meson.build
> @@ -0,0 +1,129 @@
> +# Supported hosts
> +rust_supported_oses = {
> +  'linux': '-unknown-linux-gnu',
> +  'darwin': '-apple-darwin',
> +  'windows': '-pc-windows-gnu'
> +}

Does the current test cover windows and apple? If not, I think we can
add these two platforms later on.

> +rust_supported_cpus = ['x86_64', 'aarch64']

Similarly, here I have another question, x-pl011-rust in arm virt machine
I understand should only run on ARM platform, right? So compiling to
x86_64 doesn't seem necessary at the moment?

> +# 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
> +
> +# FIXME: retrieve target triple from meson and construct rust_target_triple
> +if meson.is_cross_build()
> +  message()
> +  error('QEMU does not support cross compiling with Rust enabled.')
> +endif

Is the check here to avoid inconsistencies between cross-build's target
and rust_target_triple?

If target consistency is not guaranteed, then it seems custom rust_target_triple
is useless, right? please educate me if I am wrong ;-)

> +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> +msrv = {
> +  'rustc': '1.79.0',
> +  'cargo': '1.79.0',
> +  'bindgen': '0.69.4',
> +}
> +
> +# rustup = find_program('rustup', required: false)
> +foreach bin_dep: msrv.keys()
> +  bin = find_program(bin_dep, required: true)
> +  if bin.version() < msrv[bin_dep]
> +    if bin_dep == 'bindgen' and get_option('wrap_mode') != 'nodownload'

What about adding a download info here?

It took a long time for my bindgen-cli to update quietly, and for a
moment I thought it was stuck here...

e.g., message('Installing bindgen-cli...')

Or can we just report the error and let users install bindgen-cli
themselves?

> +      run_command(cargo, 'install', 'bindgen-cli', capture: true, check: true)
> +      bin = find_program(bin_dep, required: true)
> +      if bin.version() >= msrv[bin_dep]
> +        continue
> +      endif
> +    endif
> +    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

It seems rust_target_triple missed support check, we should use rust_supported_cpus
+ rust_supported_oses to check rust_target_triple, right? ;-)

> +endif
> +
> +

An extra blank line :-)

> +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(),
> +]
> +
> +if get_option('b_colorout') != 'never'
> +  cargo_wrapper += ['--color', 'always']
> +endif
> +
> +if get_option('optimization') in ['0', '1', 'g']
> +  rs_build_type = 'debug'
> +else
> +  rs_build_type = 'release'
> +endif

Could we decouple 'optimization' with cargo's opt-level (maybe in the future)?

Or we could add new option to specify custom rust opt-level, which will
set the environment varialbe CARGO_PROFILE_<profile_name>_OPT_LEVEL and
override the default opt-level in Cargo.toml.

> +# 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
> +    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'
> +        ]
> +      }
> +    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..bcf808c8d7
> --- /dev/null
> +++ b/rust/wrapper.h
> @@ -0,0 +1,39 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2020 Fabrice Bellard

Here should the author be yourself?

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

Thanks,
Zhao

On Thu, 20 Jun 2024 at 14:35, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Thu, 20 Jun 2024 15:32, Alex Bennée <alex.bennee@linaro.org> wrote:
> >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
> >>
> ><snip>
> >> +
> >> +
> >> +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(),
> >> +]
> >
> >I'm unclear what the difference between meson-build-root and
> >meson-build-dir is?
>
> Build-dir is the subdir of the current subdir(...) meson.build file
>
> So if we are building under qemu/build, meson_build_root is qemu/build
> and meson_build_dir is qemu/build/rust
>
> >
> >We also end up defining crate-dir and outdir. Aren't these all
> >derivable from whatever module we are building?
>
> Crate dir is the source directory (i.e. qemu/rust/pl011) that contains
> the crate's manifest file Cargo.toml.
>
> Outdir is where to put the final build artifact for meson to find. We
> could derive that from the build directories and package names somehow
> but I chose to be explicit instead of doing indirect logic to make the
> process less magic.
>
> I know it's a lot so I'm open to simplifications. The only problem is
> that all of these directories, except the crate source code, are defined
> from meson and can change with any refactor we do from the meson side of
> things.

Expanding the help text for these command-line options would make it
easier to understand. It would be great to include an example path
too.

Stefan

> > > +       # > 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 generated_rs target per target, so give them
> > > +      # target-specific names.
> > > +      copy = fs.copyfile('rust/wrapper.h',
> > > +                         target + '_wrapper.h')
> > > +      generated_rs = rust_bindgen.bindgen(
> > > +        input: copy,
> > > +        dependencies: arch_deps + lib_deps,
> > > +        output: target + '-generated.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 = custom_target(t['name'],
> > > +                                       output: t['output'],
> > > +                                       depends: [generated_rs],
> > > +                                       build_always_stale: true,
> > > +                                       command: t['command'])
> > > +          rust_dep = declare_dependency(link_args: [
> > > +                                          '-Wl,--whole-archive',
> > > +                                          t['output-path'],
> > > +                                          '-Wl,--no-whole-archive'
> > > +                                          ],
> > > +                                          sources: [rust_device_cargo])
> > > +          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..435abd3e1c
> > > --- /dev/null
> > > +++ b/rust/meson.build
> > > @@ -0,0 +1,129 @@
> > > +# Supported hosts
> > > +rust_supported_oses = {
> > > +  'linux': '-unknown-linux-gnu',
> > > +  'darwin': '-apple-darwin',
> > > +  'windows': '-pc-windows-gnu'
> > > +}
> > 
> > Does the current test cover windows and apple? If not, I think we can
> > add these two platforms later on.
> 
> Do you mean the test for supported OSes?

Yes.

> This is for future-proofing the Rust integration in general. I haven't been
> able to compile under macos yet because bindgen cannot find the system clang
> header. I also don't have a windows pc to test it on. But it should work
> theoretically under all three.

Yes, they should work. EMM, but there is no particular need for them at
the moment, so just to be safe, we can put these two platforms on hold
for now, and they can be easily added when the tests are covered.

A TODO can remind support for them.

> > 
> > > +rust_supported_cpus = ['x86_64', 'aarch64']
> > 
> > Similarly, here I have another question, x-pl011-rust in arm virt machine
> > I understand should only run on ARM platform, right? So compiling to
> > x86_64 doesn't seem necessary at the moment?
> 
> This refers to the host cpu not the virtualized ones, you can run
> qemu-system-aarch64 under x86_64

Thanks, I see.

> > > +# 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
> > > +
> > > +# FIXME: retrieve target triple from meson and construct rust_target_triple
> > > +if meson.is_cross_build()
> > > +  message()
> > > +  error('QEMU does not support cross compiling with Rust enabled.')
> > > +endif
> > 
> > Is the check here to avoid inconsistencies between cross-build's target
> > and rust_target_triple?
> > 
> > If target consistency is not guaranteed, then it seems custom rust_target_triple
> > is useless, right? please educate me if I am wrong ;-)
> 
> It is, correct. I was hoping someone on the list would have advice on how to
> best do this (see the FIXME comment right above)

Yes, Daniel has said what I would say in his reply.

> > 
> > > +# FIXME: These are the latest stable versions, refine to actual minimum ones.
> > > +msrv = {
> > > +  'rustc': '1.79.0',
> > > +  'cargo': '1.79.0',
> > > +  'bindgen': '0.69.4',
> > > +}
> > > +
> > > +# rustup = find_program('rustup', required: false)
> > > +foreach bin_dep: msrv.keys()
> > > +  bin = find_program(bin_dep, required: true)
> > > +  if bin.version() < msrv[bin_dep]
> > > +    if bin_dep == 'bindgen' and get_option('wrap_mode') != 'nodownload'
> > 
> > What about adding a download info here?
> > 
> > It took a long time for my bindgen-cli to update quietly, and for a
> > moment I thought it was stuck here...
> > 
> > e.g., message('Installing bindgen-cli...')
> > 
> > Or can we just report the error and let users install bindgen-cli
> > themselves?
> 
> Now that I think about it again I agree it's better to not mess with
> user/system installations without asking.

I agree. Thanks!

> > > +      run_command(cargo, 'install', 'bindgen-cli', capture: true, check: true)
> > > +      bin = find_program(bin_dep, required: true)
> > > +      if bin.version() >= msrv[bin_dep]
> > > +        continue
> > > +      endif
> > > +    endif
> > > +    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
> > 
> > It seems rust_target_triple missed support check, we should use rust_supported_cpus
> > + rust_supported_oses to check rust_target_triple, right? ;-)
> > 
> > > +endif
> > > +
> > > +
> > 
> > An extra blank line :-)
> > 
> > > +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(),
> > > +]
> > > +
> > > +if get_option('b_colorout') != 'never'
> > > +  cargo_wrapper += ['--color', 'always']
> > > +endif
> > > +
> > > +if get_option('optimization') in ['0', '1', 'g']
> > > +  rs_build_type = 'debug'
> > > +else
> > > +  rs_build_type = 'release'
> > > +endif
> > 
> > Could we decouple 'optimization' with cargo's opt-level (maybe in the future)?
> > 
> > Or we could add new option to specify custom rust opt-level, which will
> > set the environment varialbe CARGO_PROFILE_<profile_name>_OPT_LEVEL and
> > override the default opt-level in Cargo.toml.
> 
> optimization here refers to LLVM's opt flags directly, so I think it makes
> sense to follow the project-wide compiler settings?

Oh, yes. But I think the choice of debug or release is best based on the
debug info. What about this?

if get_option('debug')
    rs_build_type = 'debug'
else
    rs_build_type = 'release'
endif

get_option('debug') would apply -g flag, and that flag is mapped to debuginfo=2,
which is the default debuginfo level for debug version.

> > > +# 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
> > > +    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'
> > > +        ]
> > > +      }
> > > +    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..bcf808c8d7
> > > --- /dev/null
> > > +++ b/rust/wrapper.h
> > > @@ -0,0 +1,39 @@
> > > +/*
> > > + * QEMU System Emulator
> > > + *
> > > + * Copyright (c) 2003-2020 Fabrice Bellard
> > 
> > Here should the author be yourself?
> 
> Technically I copy pasted this file from the main.c executable so it's a
> derived work :D
>

It's a totally new file, and it is used to derive other files. These are
all significant changes. At least for 2024, I think you can declare
copyright here. :-)

Thanks,
Zhao

On Tue, 25 Jun 2024 19:00, Zhao Liu <zhao1.liu@intel.com> wrote:
>[snip]
>> This is for future-proofing the Rust integration in general. I 
>> haven't been
>> able to compile under macos yet because bindgen cannot find the system clang
>> header. I also don't have a windows pc to test it on. But it should work
>> theoretically under all three.
>
>Yes, they should work. EMM, but there is no particular need for them at
>the moment, so just to be safe, we can put these two platforms on hold
>for now, and they can be easily added when the tests are covered.
>
>A TODO can remind support for them.

I'm still trying to figure out why bindgen doesn't find the /Library/*** 
include paths.. it's frustrating! I will remove them if I don't succeed 
and also no one volunteers to attempt a windows build. :)

>[snip]
>> > 
>> > Could we decouple 'optimization' with cargo's opt-level (maybe in the future)?
>> > 
>> > Or we could add new option to specify custom rust opt-level, which will
>> > set the environment varialbe CARGO_PROFILE_<profile_name>_OPT_LEVEL and
>> > override the default opt-level in Cargo.toml.
>> 
>> optimization here refers to LLVM's opt flags directly, so I think it makes
>> sense to follow the project-wide compiler settings?
>
>Oh, yes. But I think the choice of debug or release is best based on the
>debug info. What about this?
>
>if get_option('debug')
>    rs_build_type = 'debug'
>else
>    rs_build_type = 'release'
>endif
>
>get_option('debug') would apply -g flag, and that flag is mapped to debuginfo=2,
>which is the default debuginfo level for debug version.

But wait, I just noticed rs_build_type should be 'dev' not 'debug'. You 
can build with optimizations and keep debuginfo if you override the 
'release' profile or use a different profile altogether.

I will correct the optimization and debuginfo mappings in the next 
version.

>It's a totally new file, and it is used to derive other files. These 
>are
>all significant changes. At least for 2024, I think you can declare
>copyright here. :-)

Heh yes :)

>
>Thanks,
>Zhao
>

On 6/25/24 11:08, Manos Pitsidianakis wrote:
> On Tue, 25 Jun 2024 19:00, Zhao Liu <zhao1.liu@intel.com> wrote:
>> [snip]
>>> This is for future-proofing the Rust integration in general. I
>>> haven't been
>>> able to compile under macos yet because bindgen cannot find the system clang
>>> header. I also don't have a windows pc to test it on. But it should work
>>> theoretically under all three.
>>
>> Yes, they should work. EMM, but there is no particular need for them at
>> the moment, so just to be safe, we can put these two platforms on hold
>> for now, and they can be easily added when the tests are covered.
>>
>> A TODO can remind support for them.
> 
> I'm still trying to figure out why bindgen doesn't find the /Library/***
> include paths.. it's frustrating! I will remove them if I don't succeed
> and also no one volunteers to attempt a windows build. :)
> 

I'm currently doing it, and managed to run until bindgen step. Same 
problem that you found on MacOS, it can't locate some headers 
(strings.h, included from osdep.h). I'll try to dig into this, but if 
you found a solution already, you're welcome to share it.

'gcc | grep' command you used should work, but should be adapted because 
windows paths start with C:/ instead of /.

>> [snip]
>>>>
>>>> Could we decouple 'optimization' with cargo's opt-level (maybe in the future)?
>>>>
>>>> Or we could add new option to specify custom rust opt-level, which will
>>>> set the environment varialbe CARGO_PROFILE_<profile_name>_OPT_LEVEL and
>>>> override the default opt-level in Cargo.toml.
>>>
>>> optimization here refers to LLVM's opt flags directly, so I think it makes
>>> sense to follow the project-wide compiler settings?
>>
>> Oh, yes. But I think the choice of debug or release is best based on the
>> debug info. What about this?
>>
>> if get_option('debug')
>>     rs_build_type = 'debug'
>> else
>>     rs_build_type = 'release'
>> endif
>>
>> get_option('debug') would apply -g flag, and that flag is mapped to debuginfo=2,
>> which is the default debuginfo level for debug version.
> 
> But wait, I just noticed rs_build_type should be 'dev' not 'debug'. You
> can build with optimizations and keep debuginfo if you override the
> 'release' profile or use a different profile altogether.
> 
> I will correct the optimization and debuginfo mappings in the next
> version.
> 
>> It's a totally new file, and it is used to derive other files. These
>> are
>> all significant changes. At least for 2024, I think you can declare
>> copyright here. :-)
> 
> Heh yes :)
> 
>>
>> Thanks,
>> Zhao
>>

On 6/27/24 16:47, Pierrick Bouvier wrote:
> On 6/25/24 11:08, Manos Pitsidianakis wrote:
>> On Tue, 25 Jun 2024 19:00, Zhao Liu <zhao1.liu@intel.com> wrote:
>>> [snip]
>>>> This is for future-proofing the Rust integration in general. I
>>>> haven't been
>>>> able to compile under macos yet because bindgen cannot find the system clang
>>>> header. I also don't have a windows pc to test it on. But it should work
>>>> theoretically under all three.
>>>
>>> Yes, they should work. EMM, but there is no particular need for them at
>>> the moment, so just to be safe, we can put these two platforms on hold
>>> for now, and they can be easily added when the tests are covered.
>>>
>>> A TODO can remind support for them.
>>
>> I'm still trying to figure out why bindgen doesn't find the /Library/***
>> include paths.. it's frustrating! I will remove them if I don't succeed
>> and also no one volunteers to attempt a windows build. :)
>>
> 
> I'm currently doing it, and managed to run until bindgen step. Same
> problem that you found on MacOS, it can't locate some headers
> (strings.h, included from osdep.h). I'll try to dig into this, but if
> you found a solution already, you're welcome to share it.
> 
> 'gcc | grep' command you used should work, but should be adapted because
> windows paths start with C:/ instead of /.
> 

I've been able to build rust device on windows, with a few tweaks needed.

- specificy the target for libclang (used by bindgen), which targets 
MSVC by default (so different set of headers)
- additional headers (libclang searches its own header with a relative 
path instead of absolute)
- additional windows libs that must be linked in final executable

However, even tough I can build the executable, I get this error:
$ ./build/qemu-system-aarch64 -M virt
C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'

Any idea of what could be missing here?

By the way, I noticed configure --enable-with-rust does not trigger 
error when not finding cargo, it just deactivates rust support, which is 
probably not what is expected.

---

QEMU Build instructions for windows are here:
https://wiki.qemu.org/Hosts/W32#Native_builds_with_MSYS2

Additional steps needed:
$ cargo install bindgen-cli
$ export PATH=/c/Users/user/.cargo/bin/:$PATH
$ wget 
https://github.com/llvm/llvm-project/releases/download/llvmorg-18.1.6/LLVM-18.1.6-win64.exe 
# for libclang.dll
$ pacman -S p7zip
$ mkdir llvm && cd llvm && 7z x ../LLVM-18.1.6-win64.exe && cd ..
$ export LIBCLANG_PATH=$(cygpath -m $(pwd)/llvm/bin/libclang.dll)

Additional libs to link can be found with:
$ touch empty.rs
$ rustc empty.rs --print=native-static-libs --crate-type=staticlib
note: Link against the following native artifacts when linking against 
this static library. The order and any duplication can be significant on so
me platforms.
note: native-static-libs: -lkernel32 -ladvapi32 -lkernel32 -lntdll 
-luserenv -lws2_32 -lkernel32 -lws2_32 -lkernel32

---

diff --git a/meson.build b/meson.build
index ca40a39ad7e..98faa4777b7 100644
--- a/meson.build
+++ b/meson.build
@@ -3896,7 +3896,8 @@ foreach target : target_dirs
          input: copy,
          dependencies: arch_deps + lib_deps,
          output: target + '-generated.rs',
-        include_directories: include_directories('.', 'include'),
+        include_directories: include_directories('.', 'include',
+        'llvm/lib/clang/18/include/'),
          args: [
            '--ctypes-prefix', 'core::ffi',
            '--formatter', 'rustfmt',
@@ -3910,7 +3911,10 @@ foreach target : target_dirs
            '--with-derive-default',
            '--allowlist-file', meson.project_source_root() + '/include/.*',
            '--allowlist-file', meson.project_source_root() + '/.*',
-          '--allowlist-file', meson.project_build_root() + '/.*'
+          '--allowlist-file', meson.project_build_root() + '/.*',
+        ],
+        c_args: [
+          '--target=x86_64-pc-windows-gnu'
          ],
        )

@@ -3925,7 +3929,12 @@ foreach target : target_dirs
            rust_dep = declare_dependency(link_args: [
                                            '-Wl,--whole-archive',
                                            t['output-path'],
-                                          '-Wl,--no-whole-archive'
+                                          '-Wl,--no-whole-archive',
+                                          '-lkernel32',
+                                          '-ladvapi32',
+                                          '-lntdll',
+                                          '-luserenv',
+                                          '-lws2_32',
                                            ],
                                            sources: [rust_device_cargo])
            rust_hw.add(rust_dep)

On Fri, Jun 28, 2024 at 9:12 PM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
> However, even tough I can build the executable, I get this error:
> $ ./build/qemu-system-aarch64 -M virt
> C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>
> Any idea of what could be missing here?

Maybe the underlying mechanism to invoke constructors is different?

Perhaps we could use https://crates.io/crates/ctor instead?

Paolo

On Fri, 28 Jun 2024 22:12, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>I've been able to build rust device on windows, with a few tweaks 
>needed.
>
>- specificy the target for libclang (used by bindgen), which targets 
>MSVC by default (so different set of headers)
>- additional headers (libclang searches its own header with a relative 
>path instead of absolute)
>- additional windows libs that must be linked in final executable
>
>However, even tough I can build the executable, I get this error:
>$ ./build/qemu-system-aarch64 -M virt
>C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>
>Any idea of what could be missing here?

Sounds like either the rust lib is not linked to the final binary (which 
you can confirm if you look at the included symbols and don't see 
anything with rust origin) or the constructor decorated with     
#[cfg_attr(target_os = "windows", link_section = ".CRT$XCU"] is not 
linked properly (you can add a puts() and see if it's invoked or add a 
breakpoint in gdb)

Seems like the using cfg_attr was taken from this crate, so it should be 
equivalent to using it.

On 6/28/24 14:50, Paolo Bonzini wrote:
> On Fri, Jun 28, 2024 at 9:12 PM Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>> However, even tough I can build the executable, I get this error:
>> $ ./build/qemu-system-aarch64 -M virt
>> C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>>
>> Any idea of what could be missing here?
> 
> Maybe the underlying mechanism to invoke constructors is different?
> 
> Perhaps we could use https://crates.io/crates/ctor instead?
> 
> Paolo
>

The ctor is not run, but, interesting point, there is the same problem 
on Linux :)

Can you please confirm with a clean build this work on your machine with 
this v3, and report exact configure and run commands you use?

Thanks,
Pierrick

On 6/29/24 01:06, Manos Pitsidianakis wrote:
> On Fri, 28 Jun 2024 22:12, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> I've been able to build rust device on windows, with a few tweaks
>> needed.
>>
>> - specificy the target for libclang (used by bindgen), which targets
>> MSVC by default (so different set of headers)
>> - additional headers (libclang searches its own header with a relative
>> path instead of absolute)
>> - additional windows libs that must be linked in final executable
>>
>> However, even tough I can build the executable, I get this error:
>> $ ./build/qemu-system-aarch64 -M virt
>> C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>>
>> Any idea of what could be missing here?
> 
> Sounds like either the rust lib is not linked to the final binary (which
> you can confirm if you look at the included symbols and don't see
> anything with rust origin) or the constructor decorated with
> #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU"] is not
> linked properly (you can add a puts() and see if it's invoked or add a
> breakpoint in gdb)

On Mon, 01 Jul 2024 21:54, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>The ctor is not run, but, interesting point, there is the same problem 
>on Linux :)
>
>Can you please confirm with a clean build this work on your machine with 
>this v3, and report exact configure and run commands you use?

On a clean checkout, I did exactly the following:

```
mkdir ./build && cd ./build
../configure --enable-system --enable-debug \
 --target-list=aarch64-softmmu --enable-with-rust
ninja qemu-system-aarch64
```

which selected the default cc/gcc on debian,
gcc (Debian 12.2.0-14) 12.2.0

and ran QEMU with like this:

```
./qemu-system-aarch64 -M virt \
  -machine virtualization=true -machine virt,gic-version=3 \
  -cpu max,pauth-impdef=on -smp 2 -m 4096 \
  [.. drive and img arguments ..] \
  -nographic
```


>
>Thanks,
>Pierrick
>
>On 6/29/24 01:06, Manos Pitsidianakis wrote:
>> On Fri, 28 Jun 2024 22:12, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>>> I've been able to build rust device on windows, with a few tweaks
>>> needed.
>>>
>>> - specificy the target for libclang (used by bindgen), which targets
>>> MSVC by default (so different set of headers)
>>> - additional headers (libclang searches its own header with a relative
>>> path instead of absolute)
>>> - additional windows libs that must be linked in final executable
>>>
>>> However, even tough I can build the executable, I get this error:
>>> $ ./build/qemu-system-aarch64 -M virt
>>> C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>>>
>>> Any idea of what could be missing here?
>> 
>> Sounds like either the rust lib is not linked to the final binary (which
>> you can confirm if you look at the included symbols and don't see
>> anything with rust origin) or the constructor decorated with
>> #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU"] is not
>> linked properly (you can add a puts() and see if it's invoked or add a
>> breakpoint in gdb)

On 7/2/24 05:25, Manos Pitsidianakis wrote:
> On Mon, 01 Jul 2024 21:54, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> The ctor is not run, but, interesting point, there is the same problem
>> on Linux :)
>>
>> Can you please confirm with a clean build this work on your machine with
>> this v3, and report exact configure and run commands you use?
> 
> On a clean checkout, I did exactly the following:
> 
> ```
> mkdir ./build && cd ./build
> ../configure --enable-system --enable-debug \
>   --target-list=aarch64-softmmu --enable-with-rust
> ninja qemu-system-aarch64
> ```
> 

Debug flag was the difference.
After looking with Manos, we identified an issue with release builds 
(unused symbols gets dropped, including device ctor), that will be 
solved in v4.

> which selected the default cc/gcc on debian,
> gcc (Debian 12.2.0-14) 12.2.0
> 
> and ran QEMU with like this:
> 
> ```
> ./qemu-system-aarch64 -M virt \
>    -machine virtualization=true -machine virt,gic-version=3 \
>    -cpu max,pauth-impdef=on -smp 2 -m 4096 \
>    [.. drive and img arguments ..] \
>    -nographic
> ```
> 
> 
>>
>> Thanks,
>> Pierrick
>>
>> On 6/29/24 01:06, Manos Pitsidianakis wrote:
>>> On Fri, 28 Jun 2024 22:12, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>>>> I've been able to build rust device on windows, with a few tweaks
>>>> needed.
>>>>
>>>> - specificy the target for libclang (used by bindgen), which targets
>>>> MSVC by default (so different set of headers)
>>>> - additional headers (libclang searches its own header with a relative
>>>> path instead of absolute)
>>>> - additional windows libs that must be linked in final executable
>>>>
>>>> However, even tough I can build the executable, I get this error:
>>>> $ ./build/qemu-system-aarch64 -M virt
>>>> C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
>>>>
>>>> Any idea of what could be missing here?
>>>
>>> Sounds like either the rust lib is not linked to the final binary (which
>>> you can confirm if you look at the included symbols and don't see
>>> anything with rust origin) or the constructor decorated with
>>> #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU"] is not
>>> linked properly (you can add a puts() and see if it's invoked or add a
>>> breakpoint in gdb)