qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v3 0/5] Implement ARM PL011 in Rust
@ 2024-06-19 20:13 Manos Pitsidianakis
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
                   ` (4 more replies)
  0 siblings, 5 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson

Changes from v2->v3:
- Addressed minor mistakes (thanks Stefan)
- Setup supported version checks for cargo, rustc and bindgen (thanks 
  everyone who pointed it out / suggested it)
- Fixed problem with bindgen failing if certain system headers where 
  needed by defining an allowlist for headers instead of a blocklist for 
  what we don't want (thanks Alex Bennée for reporting it)
- Cleaned up bindgen target/dependendy definition in meson.build by 
  removing unnecessary bits

Changes from v1->v2:
- Create bindgen target first, then add commit for device (thanks 
  Pierrick)
- Create a special named generated.rs for each target as compilation 
  would fail if more than one targets were defined. The generated.rs 
  target names would clash.
- Add more descriptive commit messages
- Update MAINTAINERS
- Cleanup patch order for better review, hopefully

v2 was:
<rust-pl011-rfc-v2.git.manos.pitsidianakis@linaro.org>

v1 was:
<cover.rust-pl011-rfc-v1.git.manos.pitsidianakis@linaro.org>

Patches can be found online at 
https://gitlab.com/epilys/rust-for-qemu/-/tags

Tag/refs:
- rust-pl011-rfc-v3
- rust-pl011-rfc-v2
- rust-pl011-rfc-v1

Manos Pitsidianakis (5):
  build-sys: Add rust feature option
  rust: add bindgen step as a meson dependency
  rust: add PL011 device model
  DO NOT MERGE: add rustdoc build for gitlab pages
  DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine

 .gitlab-ci.d/buildtest.yml     |  64 ++--
 MAINTAINERS                    |  15 +
 configure                      |  11 +
 hw/arm/virt.c                  |   4 +
 meson.build                    |  71 ++++
 meson_options.txt              |   4 +
 rust/.cargo/config.toml        |   2 +
 rust/.gitignore                |   3 +
 rust/meson.build               | 131 ++++++++
 rust/pl011/.gitignore          |   2 +
 rust/pl011/Cargo.lock          | 120 +++++++
 rust/pl011/Cargo.toml          |  66 ++++
 rust/pl011/README.md           |  42 +++
 rust/pl011/build.rs            |  44 +++
 rust/pl011/deny.toml           |  57 ++++
 rust/pl011/meson.build         |   7 +
 rust/pl011/rustfmt.toml        |   1 +
 rust/pl011/src/definitions.rs  |  95 ++++++
 rust/pl011/src/device.rs       | 531 ++++++++++++++++++++++++++++++
 rust/pl011/src/device_class.rs |  95 ++++++
 rust/pl011/src/generated.rs    |   5 +
 rust/pl011/src/lib.rs          | 581 +++++++++++++++++++++++++++++++++
 rust/pl011/src/memory_ops.rs   |  38 +++
 rust/rustfmt.toml              |   7 +
 rust/wrapper.h                 |  39 +++
 scripts/cargo_wrapper.py       | 289 ++++++++++++++++
 scripts/meson-buildoptions.sh  |   6 +
 27 files changed, 2311 insertions(+), 19 deletions(-)
 create mode 100644 rust/.cargo/config.toml
 create mode 100644 rust/.gitignore
 create mode 100644 rust/meson.build
 create mode 100644 rust/pl011/.gitignore
 create mode 100644 rust/pl011/Cargo.lock
 create mode 100644 rust/pl011/Cargo.toml
 create mode 100644 rust/pl011/README.md
 create mode 100644 rust/pl011/build.rs
 create mode 100644 rust/pl011/deny.toml
 create mode 100644 rust/pl011/meson.build
 create mode 120000 rust/pl011/rustfmt.toml
 create mode 100644 rust/pl011/src/definitions.rs
 create mode 100644 rust/pl011/src/device.rs
 create mode 100644 rust/pl011/src/device_class.rs
 create mode 100644 rust/pl011/src/generated.rs
 create mode 100644 rust/pl011/src/lib.rs
 create mode 100644 rust/pl011/src/memory_ops.rs
 create mode 100644 rust/rustfmt.toml
 create mode 100644 rust/wrapper.h
 create mode 100644 scripts/cargo_wrapper.py


base-commit: 01782d6b294f95bcde334386f0aaac593cd28c0d
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply	[flat|nested] 44+ messages in thread

* [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
@ 2024-06-19 20:13 ` Manos Pitsidianakis
  2024-06-20 13:21   ` Paolo Bonzini
                     ` (2 more replies)
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.

`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.

A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.

https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 MAINTAINERS                   |   5 +
 configure                     |  11 ++
 meson.build                   |  11 ++
 meson_options.txt             |   4 +
 scripts/cargo_wrapper.py      | 279 ++++++++++++++++++++++++++++++++++
 scripts/meson-buildoptions.sh |   6 +
 6 files changed, 316 insertions(+)
 create mode 100644 scripts/cargo_wrapper.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 1b79767d61..431010ddbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4223,6 +4223,11 @@ F: docs/sphinx/
 F: docs/_templates/
 F: docs/devel/docs.rst
 
+Rust build system integration
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Maintained
+F: scripts/cargo_wrapper.py
+
 Miscellaneous
 -------------
 Performance Tools and Tests
diff --git a/configure b/configure
index 38ee257701..6894d7c2d1 100755
--- a/configure
+++ b/configure
@@ -302,6 +302,9 @@ else
   objcc="${objcc-${cross_prefix}clang}"
 fi
 
+with_rust="auto"
+with_rust_target_triple=""
+
 ar="${AR-${cross_prefix}ar}"
 as="${AS-${cross_prefix}as}"
 ccas="${CCAS-$cc}"
@@ -760,6 +763,12 @@ for opt do
   ;;
   --gdb=*) gdb_bin="$optarg"
   ;;
+  --enable-with-rust) with_rust=enabled
+  ;;
+  --disable-with-rust) with_rust=disabled
+  ;;
+  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
+  ;;
   # everything else has the same name in configure and meson
   --*) meson_option_parse "$opt" "$optarg"
   ;;
@@ -1796,6 +1805,8 @@ if test "$skip_meson" = no; then
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
   test "$plugins" = yes && meson_option_add "-Dplugins=true"
   test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
+  test "$with_rust" != enabled && meson_option_add "-Dwith_rust=$with_rust"
+  test "$with_rust_target_triple" != "" && meson_option_add "-Dwith_rust_target_triple=$with_rust_target_triple"
   run_meson() {
     NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
   }
diff --git a/meson.build b/meson.build
index a9de71d450..3533889852 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,12 @@ foreach lang : all_languages
   endif
 endforeach
 
+cargo = not_found
+if get_option('with_rust').allowed()
+  cargo = find_program('cargo', required: get_option('with_rust'))
+endif
+with_rust = cargo.found()
+
 # default flags for all hosts
 # We use -fwrapv to tell the compiler that we require a C dialect where
 # left shift of signed integers is well defined and has the expected
@@ -2066,6 +2072,7 @@ endif
 
 config_host_data = configuration_data()
 
+config_host_data.set('CONFIG_WITH_RUST', with_rust)
 audio_drivers_selected = []
 if have_system
   audio_drivers_available = {
@@ -4190,6 +4197,10 @@ if 'objc' in all_languages
 else
   summary_info += {'Objective-C compiler': false}
 endif
+summary_info += {'Rust support':      with_rust}
+if with_rust and get_option('with_rust_target_triple') != ''
+  summary_info += {'Rust target':     get_option('with_rust_target_triple')}
+endif
 option_cflags = (get_option('debug') ? ['-g'] : [])
 if get_option('optimization') != 'plain'
   option_cflags += ['-O' + get_option('optimization')]
diff --git a/meson_options.txt b/meson_options.txt
index 4c1583eb40..223491b731 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -366,3 +366,7 @@ option('qemu_ga_version', type: 'string', value: '',
 
 option('hexagon_idef_parser', type : 'boolean', value : true,
        description: 'use idef-parser to automatically generate TCG code for the Hexagon frontend')
+option('with_rust', type: 'feature', value: 'auto',
+       description: 'Enable Rust support')
+option('with_rust_target_triple', type : 'string', value: '',
+       description: 'Rust target triple')
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
new file mode 100644
index 0000000000..927336f80e
--- /dev/null
+++ b/scripts/cargo_wrapper.py
@@ -0,0 +1,279 @@
+#!/usr/bin/env python3
+
+"""Wrap cargo builds for meson integration
+
+This program builds Rust library crates and makes sure:
+ - They receive the correct --cfg compile flags from the QEMU build that calls
+   it.
+ - They receive the generated Rust bindings path so that they can copy it
+   inside their output subdirectories.
+ - Cargo puts all its build artifacts in the appropriate meson build directory.
+ - The produced static libraries are copied to the path the caller (meson)
+   defines.
+
+Copyright (c) 2020 Red Hat, Inc.
+Copyright (c) 2024 Linaro Ltd.
+
+Authors:
+ Marc-André Lureau <marcandre.lureau@redhat.com>
+ Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""
+
+
+import argparse
+import json
+import logging
+import os
+import subprocess
+import sys
+import shutil
+
+from pathlib import Path
+from typing import Any, Dict, List
+
+
+def generate_cfg_flags(header: str) -> List[str]:
+    """Converts defines from config[..].h headers to rustc --cfg flags."""
+
+    def cfg_name(name: str) -> str:
+        """Filter function for C #defines"""
+        if (
+            name.startswith("CONFIG_")
+            or name.startswith("TARGET_")
+            or name.startswith("HAVE_")
+        ):
+            return name
+        return ""
+
+    with open(header, encoding="utf-8") as cfg:
+        config = [l.split()[1:] for l in cfg if l.startswith("#define")]
+
+    cfg_list = []
+    for cfg in config:
+        name = cfg_name(cfg[0])
+        if not name:
+            continue
+        if len(cfg) >= 2 and cfg[1] != "1":
+            continue
+        cfg_list.append("--cfg")
+        cfg_list.append(name)
+    return cfg_list
+
+
+def cargo_target_dir(args: argparse.Namespace) -> Path:
+    """Place cargo's build artifacts into meson's build directory"""
+    return args.meson_build_dir
+
+
+def manifest_path(args: argparse.Namespace) -> Path:
+    """Returns the Cargo.toml manifest path"""
+    return args.crate_dir / "Cargo.toml"
+
+
+def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
+    """Returns the appropriate cargo invocation and environment"""
+
+    # See https://doc.rust-lang.org/cargo/reference/environment-variables.html
+    # Item `CARGO_ENCODED_RUSTFLAGS — A list of custom flags separated by
+    # 0x1f (ASCII Unit Separator) to pass to all compiler invocations that Cargo
+    # performs`
+    cfg = chr(0x1F).join(
+        [c for h in args.config_headers for c in generate_cfg_flags(h)]
+    )
+    target_dir = cargo_target_dir(args)
+    cargo_path = manifest_path(args)
+
+    cargo_cmd = [
+        "cargo",
+        "build",
+        "--target-dir",
+        str(target_dir),
+        "--manifest-path",
+        str(cargo_path),
+    ]
+    if args.target_triple:
+        cargo_cmd += ["--target", args.target_triple]
+    if args.profile == "release":
+        cargo_cmd += ["--release"]
+
+    env = os.environ
+    env["CARGO_ENCODED_RUSTFLAGS"] = cfg
+
+    return (env, cargo_cmd)
+
+
+def run_cargo(env: Dict[str, Any], cargo_cmd: List[str]) -> str:
+    """Calls cargo build invocation."""
+    envlog = " ".join([f"{k}={v}" for k, v in env.items()])
+    cmdlog = " ".join(cargo_cmd)
+    logging.debug("Running %s %s", envlog, cmdlog)
+    try:
+        out = subprocess.check_output(
+            cargo_cmd,
+            env=dict(os.environ, **env),
+            stderr=subprocess.STDOUT,
+            universal_newlines=True,
+        )
+    except subprocess.CalledProcessError as err:
+        print("Environment: " + envlog)
+        print("Command: " + cmdlog)
+        print(err.output)
+        sys.exit(1)
+
+    return out
+
+
+def get_package_name(cargo_toml_path: Path) -> str:
+    """Attempts to get package name from cargo manifest file with toml parsing libraries."""
+    # pylint: disable=import-outside-toplevel
+
+    try:
+        import tomllib
+    except ImportError:
+        import tomli as tomllib
+    with open(cargo_toml_path, "rb") as toml_file:
+        config = tomllib.load(toml_file)
+
+    package_name = config["package"]["name"].strip('"').replace("-", "_")
+    return package_name
+
+
+def get_package_name_json(cargo_toml_path: Path) -> str:
+    """Attempts to get package name from cargo-metadata output which has a standard JSON format."""
+
+    cmd = [
+        "cargo",
+        "metadata",
+        "--format-version",
+        "1",
+        "--no-deps",
+        "--manifest-path",
+        str(cargo_toml_path),
+        "--offline",
+    ]
+    try:
+        out = subprocess.check_output(
+            cmd,
+            env=os.environ,
+            stderr=subprocess.STDOUT,
+            universal_newlines=True,
+        )
+    except subprocess.CalledProcessError as err:
+        print("Command: ", " ".join(cmd))
+        print(err.output)
+        raise err
+    package_name = json.loads(out)["packages"][0]["name"].strip('"').replace("-", "_")
+    return package_name
+
+
+def build_lib(args: argparse.Namespace) -> None:
+    """Builds Rust lib given by command line arguments."""
+
+    logging.debug("build-lib")
+    target_dir = cargo_target_dir(args)
+    cargo_toml_path = manifest_path(args)
+
+    try:
+        # If we have tomllib or tomli, parse the .toml file
+        package_name = get_package_name(cargo_toml_path)
+    except ImportError as import_exc:
+        try:
+            # Parse the json output of cargo-metadata as a fallback
+            package_name = get_package_name_json(cargo_toml_path)
+        except Exception as exc:
+            raise exc from import_exc
+
+    liba_filename = "lib" + package_name + ".a"
+    liba = target_dir / args.target_triple / args.profile / liba_filename
+
+    env, cargo_cmd = get_cargo_rustc(args)
+    out = run_cargo(env, cargo_cmd)
+    logging.debug("cargo output: %s", out)
+    logging.debug("cp %s %s", liba, args.outdir)
+    shutil.copy2(liba, args.outdir)
+
+
+def main() -> None:
+    # pylint: disable=missing-function-docstring
+    parser = argparse.ArgumentParser()
+    parser.add_argument("-v", "--verbose", action="store_true")
+    parser.add_argument(
+        "--color",
+        metavar="WHEN",
+        choices=["auto", "always", "never"],
+        default="auto",
+        help="Coloring: auto, always, never",
+    )
+    parser.add_argument(
+        "--config-headers",
+        metavar="CONFIG_HEADER",
+        action="append",
+        dest="config_headers",
+        required=False,
+        default=[],
+    )
+    parser.add_argument(
+        "--meson-build-dir",
+        metavar="BUILD_DIR",
+        help="meson.current_build_dir()",
+        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_source_dir",
+        required=True,
+    )
+    parser.add_argument(
+        "--crate-dir",
+        metavar="CRATE_DIR",
+        type=Path,
+        dest="crate_dir",
+        help="Absolute path that contains the manifest file of the crate to compile",
+        required=True,
+    )
+    parser.add_argument(
+        "--outdir",
+        metavar="OUTDIR",
+        type=Path,
+        dest="outdir",
+        help="Path to copy compiled artifacts to for Meson to use.",
+        required=True,
+    )
+    parser.add_argument(
+        "--profile", type=str, choices=["release", "debug"], required=True
+    )
+    parser.add_argument("--target-triple", type=str, required=True)
+
+    subparsers = parser.add_subparsers()
+
+    buildlib = subparsers.add_parser("build-lib")
+    buildlib.set_defaults(func=build_lib)
+
+    args = parser.parse_args()
+    if args.verbose:
+        logging.basicConfig(level=logging.DEBUG)
+    logging.debug("args: %s", args)
+
+    args.func(args)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 6ce5a8b72a..fdcf17129e 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -79,6 +79,8 @@ meson_options_help() {
   printf "%s\n" '                           auto/sigaltstack/ucontext/windows)'
   printf "%s\n" '  --with-pkgversion=VALUE  use specified string as sub-version of the'
   printf "%s\n" '                           package'
+  printf "%s\n" '  --with-rust-target-triple=VALUE'
+  printf "%s\n" '                           Specify Rust host target triple'
   printf "%s\n" '  --with-suffix=VALUE      Suffix for QEMU data/modules/config directories'
   printf "%s\n" '                           (can be empty) [qemu]'
   printf "%s\n" '  --with-trace-file=VALUE  Trace file prefix for simple backend [trace]'
@@ -215,6 +217,7 @@ meson_options_help() {
   printf "%s\n" '  vvfat           vvfat image format support'
   printf "%s\n" '  werror          Treat warnings as errors'
   printf "%s\n" '  whpx            WHPX acceleration support'
+  printf "%s\n" '  with-rust       Enable Rust support'
   printf "%s\n" '  xen             Xen backend support'
   printf "%s\n" '  xen-pci-passthrough'
   printf "%s\n" '                  Xen PCI passthrough support'
@@ -550,6 +553,9 @@ _meson_option_parse() {
     --disable-werror) printf "%s" -Dwerror=false ;;
     --enable-whpx) printf "%s" -Dwhpx=enabled ;;
     --disable-whpx) printf "%s" -Dwhpx=disabled ;;
+    --enable-with-rust) printf "%s" -Dwith_rust=enabled ;;
+    --disable-with-rust) printf "%s" -Dwith_rust=disabled ;;
+    --with-rust-target-triple=*) quote_sh "-Dwith_rust_target_triple=$2" ;;
     --enable-xen) printf "%s" -Dxen=enabled ;;
     --disable-xen) printf "%s" -Dxen=disabled ;;
     --enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
@ 2024-06-19 20:13 ` Manos Pitsidianakis
  2024-06-20 11:10   ` Alex Bennée
                     ` (3 more replies)
  2024-06-19 20:14 ` [RFC PATCH v3 3/5] rust: add PL011 device model Manos Pitsidianakis
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:13 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 431010ddbf..0f36bb3e9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4227,6 +4227,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 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:
+       # > 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'
+}
+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
+
+# 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
+
+# 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'
+      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
+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(),
+  '--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
+
+# 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()",
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [RFC PATCH v3 3/5] rust: add PL011 device model
  2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
@ 2024-06-19 20:14 ` Manos Pitsidianakis
  2024-06-19 20:14 ` [RFC PATCH v3 4/5] DO NOT MERGE: add rustdoc build for gitlab pages Manos Pitsidianakis
  2024-06-19 20:14 ` [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
  4 siblings, 0 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini

This commit adds a re-implementation of hw/char/pl011.c in Rust.

It uses generated Rust bindings (produced by `ninja
aarch64-softmmu-generated.rs`) to
register itself as a QOM type/class.

How to build:

1. Make sure rust, cargo and bindgen (cargo install bindgen-cli) are
   installed
2. Configure a QEMU build with:
   --enable-system --target-list=aarch64-softmmu --enable-with-rust
3. Launching a VM with qemu-system-aarch64 should use the Rust version
   of the pl011 device (unless it is not set up so in hw/arm/virt.c; the
   type of the UART device is hardcoded).

   To confirm, inspect `info qom-tree` in the monitor and look for an
   `x-pl011-rust` device.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 MAINTAINERS                    |   7 +
 meson.build                    |   4 +
 rust/.cargo/config.toml        |   2 +
 rust/meson.build               |   2 +
 rust/pl011/.gitignore          |   2 +
 rust/pl011/Cargo.lock          | 120 +++++++
 rust/pl011/Cargo.toml          |  66 ++++
 rust/pl011/README.md           |  42 +++
 rust/pl011/build.rs            |  44 +++
 rust/pl011/deny.toml           |  57 ++++
 rust/pl011/meson.build         |   7 +
 rust/pl011/rustfmt.toml        |   1 +
 rust/pl011/src/definitions.rs  |  95 ++++++
 rust/pl011/src/device.rs       | 531 ++++++++++++++++++++++++++++++
 rust/pl011/src/device_class.rs |  95 ++++++
 rust/pl011/src/generated.rs    |   5 +
 rust/pl011/src/lib.rs          | 581 +++++++++++++++++++++++++++++++++
 rust/pl011/src/memory_ops.rs   |  38 +++
 rust/rustfmt.toml              |   7 +
 19 files changed, 1706 insertions(+)
 create mode 100644 rust/.cargo/config.toml
 create mode 100644 rust/pl011/.gitignore
 create mode 100644 rust/pl011/Cargo.lock
 create mode 100644 rust/pl011/Cargo.toml
 create mode 100644 rust/pl011/README.md
 create mode 100644 rust/pl011/build.rs
 create mode 100644 rust/pl011/deny.toml
 create mode 100644 rust/pl011/meson.build
 create mode 120000 rust/pl011/rustfmt.toml
 create mode 100644 rust/pl011/src/definitions.rs
 create mode 100644 rust/pl011/src/device.rs
 create mode 100644 rust/pl011/src/device_class.rs
 create mode 100644 rust/pl011/src/generated.rs
 create mode 100644 rust/pl011/src/lib.rs
 create mode 100644 rust/pl011/src/memory_ops.rs
 create mode 100644 rust/rustfmt.toml

diff --git a/MAINTAINERS b/MAINTAINERS
index 0f36bb3e9a..1420d83402 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1181,6 +1181,11 @@ F: include/hw/*/microbit*.h
 F: tests/qtest/microbit-test.c
 F: docs/system/arm/nrf.rst
 
+ARM PL011 Rust device
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Maintained
+F: rust/pl011/
+
 AVR Machines
 -------------
 
@@ -4230,6 +4235,8 @@ F: scripts/cargo_wrapper.py
 F: rust/meson.build
 F: rust/wrapper.h
 F: rust/.gitignore
+F: rust/rustfmt.toml
+F: rust/.cargo/config.toml
 
 Miscellaneous
 -------------
diff --git a/meson.build b/meson.build
index 2b305e745a..ca40a39ad7 100644
--- a/meson.build
+++ b/meson.build
@@ -296,6 +296,10 @@ if get_option('with_rust').allowed()
 endif
 with_rust = cargo.found()
 
+if with_rust
+  subdir('rust')
+endif
+
 # default flags for all hosts
 # We use -fwrapv to tell the compiler that we require a C dialect where
 # left shift of signed integers is well defined and has the expected
diff --git a/rust/.cargo/config.toml b/rust/.cargo/config.toml
new file mode 100644
index 0000000000..241210ffa7
--- /dev/null
+++ b/rust/.cargo/config.toml
@@ -0,0 +1,2 @@
+[build]
+rustflags = ["-Crelocation-model=pic", "-Ctarget-feature=+crt-static"]
diff --git a/rust/meson.build b/rust/meson.build
index 435abd3e1c..e21309d922 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -109,6 +109,8 @@ endif
 # bindgen dependency is declared.
 rust_hw_target_list = {}
 
+subdir('pl011')
+
 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']
diff --git a/rust/pl011/.gitignore b/rust/pl011/.gitignore
new file mode 100644
index 0000000000..d8db38b44e
--- /dev/null
+++ b/rust/pl011/.gitignore
@@ -0,0 +1,2 @@
+# Ignore generated bindings file overrides.
+src/generated.rs.inc
diff --git a/rust/pl011/Cargo.lock b/rust/pl011/Cargo.lock
new file mode 100644
index 0000000000..d0fa46f9f5
--- /dev/null
+++ b/rust/pl011/Cargo.lock
@@ -0,0 +1,120 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "arbitrary-int"
+version = "1.2.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d"
+
+[[package]]
+name = "bilge"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dc707ed8ebf81de5cd6c7f48f54b4c8621760926cdf35a57000747c512e67b57"
+dependencies = [
+ "arbitrary-int",
+ "bilge-impl",
+]
+
+[[package]]
+name = "bilge-impl"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8"
+dependencies = [
+ "itertools",
+ "proc-macro-error",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "either"
+version = "1.12.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
+
+[[package]]
+name = "itertools"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57"
+dependencies = [
+ "either",
+]
+
+[[package]]
+name = "pl011"
+version = "0.1.0"
+dependencies = [
+ "arbitrary-int",
+ "bilge",
+ "bilge-impl",
+]
+
+[[package]]
+name = "proc-macro-error"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
+dependencies = [
+ "proc-macro-error-attr",
+ "proc-macro2",
+ "quote",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro-error-attr"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.84"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6"
+dependencies = [
+ "unicode-ident",
+]
+
+[[package]]
+name = "quote"
+version = "1.0.36"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7"
+dependencies = [
+ "proc-macro2",
+]
+
+[[package]]
+name = "syn"
+version = "2.0.66"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "unicode-ident"
+version = "1.0.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b"
+
+[[package]]
+name = "version_check"
+version = "0.9.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
diff --git a/rust/pl011/Cargo.toml b/rust/pl011/Cargo.toml
new file mode 100644
index 0000000000..db74f2b59f
--- /dev/null
+++ b/rust/pl011/Cargo.toml
@@ -0,0 +1,66 @@
+[package]
+name = "pl011"
+version = "0.1.0"
+edition = "2021"
+authors = ["Manos Pitsidianakis <manos.pitsidianakis@linaro.org>"]
+license = "GPL-2.0 OR GPL-3.0-or-later"
+readme = "README.md"
+homepage = "https://www.qemu.org"
+description = "pl011 device model for QEMU"
+repository = "https://gitlab.com/epilys/rust-for-qemu"
+resolver = "2"
+publish = false
+keywords = []
+categories = []
+
+[lib]
+crate-type = ["staticlib"]
+
+# bilge deps included here to include them with docs
+[dependencies]
+arbitrary-int = { version = "1.2.7" }
+bilge = { version = "0.2.0" }
+bilge-impl = { version = "0.2.0" }
+
+[lints]
+[lints.rustdoc]
+broken_intra_doc_links = "deny"
+redundant_explicit_links = "deny"
+[lints.clippy]
+# lint groups
+correctness = { level = "deny", priority = -1 }
+suspicious = { level = "deny", priority = -1 }
+complexity = { level = "deny", priority = -1 }
+perf = { level = "deny", priority = -1 }
+cargo = { level = "deny", priority = -1 }
+nursery = { level = "deny", priority = -1 }
+style = { level = "deny", priority = -1 }
+# restriction group
+dbg_macro = "deny"
+rc_buffer = "deny"
+as_underscore = "deny"
+assertions_on_result_states = "deny"
+# pedantic group
+doc_markdown = "deny"
+expect_fun_call = "deny"
+borrow_as_ptr = "deny"
+case_sensitive_file_extension_comparisons = "deny"
+cast_lossless = "deny"
+cast_ptr_alignment = "allow"
+large_futures = "deny"
+waker_clone_wake = "deny"
+unused_enumerate_index = "deny"
+unnecessary_fallible_conversions = "deny"
+struct_field_names = "deny"
+manual_hash_one = "deny"
+into_iter_without_iter = "deny"
+option_if_let_else = "deny"
+missing_const_for_fn = "deny"
+significant_drop_tightening = "deny"
+multiple_crate_versions = "deny"
+significant_drop_in_scrutinee = "deny"
+cognitive_complexity = "deny"
+missing_safety_doc = "allow"
+
+# Do not include in any global workspace
+[workspace]
diff --git a/rust/pl011/README.md b/rust/pl011/README.md
new file mode 100644
index 0000000000..54efe3c0cb
--- /dev/null
+++ b/rust/pl011/README.md
@@ -0,0 +1,42 @@
+# PL011 QEMU Device Model
+
+This library implements a device model for the PrimeCell® UART (PL011)
+device in QEMU.
+
+The C bindings were generated for commit `01782d6b29`:
+
+```console
+$ git describe 01782d6b29
+v9.0.0-769-g01782d6b29
+```
+
+with `bindgen`, using this build target:
+
+```console
+$ ninja aarch64-softmmu-generated.rs
+```
+
+## Build static lib
+
+```sh
+cargo build --target x86_64-unknown-linux-gnu
+```
+
+Replace host target triplet if necessary.
+
+## Generate Rust documentation
+
+To generate docs for this crate, including private items:
+
+```sh
+cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+```
+
+To include direct dependencies like `bilge` (bitmaps for register types):
+
+```sh
+cargo tree --depth 1 -e normal --prefix none \
+ | cut -d' ' -f1 \
+ | xargs printf -- '-p %s\n' \
+ | xargs cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+```
diff --git a/rust/pl011/build.rs b/rust/pl011/build.rs
new file mode 100644
index 0000000000..169516d8ab
--- /dev/null
+++ b/rust/pl011/build.rs
@@ -0,0 +1,44 @@
+use std::{env, path::Path};
+
+fn main() {
+    println!("cargo::rerun-if-env-changed=MESON_BUILD_DIR");
+    println!("cargo::rerun-if-env-changed=MESON_BUILD_ROOT");
+    println!("cargo::rerun-if-changed=src/generated.rs.inc");
+
+    let out_dir = env::var_os("OUT_DIR").unwrap();
+
+    if let Some(build_dir) = std::env::var_os("MESON_BUILD_ROOT") {
+        let mut build_dir = Path::new(&build_dir).to_path_buf();
+        let mut out_dir = Path::new(&out_dir).to_path_buf();
+        assert!(
+            build_dir.exists(),
+            "MESON_BUILD_ROOT value does not exist on filesystem: {}",
+            build_dir.display()
+        );
+        assert!(
+            build_dir.is_dir(),
+            "MESON_BUILD_ROOT value is not actually a directory: {}",
+            build_dir.display()
+        );
+        build_dir.push("aarch64-softmmu-generated.rs");
+        let generated_rs = build_dir;
+        assert!(
+            generated_rs.exists(),
+            "MESON_BUILD_ROOT/aarch64-softmmu-generated.rs does not exist on filesystem: {}",
+            generated_rs.display()
+        );
+        assert!(
+            generated_rs.is_file(),
+            "MESON_BUILD_ROOT/aarch64-softmmu-generated.rs is not a file: {}",
+            generated_rs.display()
+        );
+        out_dir.push("generated.rs");
+        std::fs::copy(generated_rs, out_dir).unwrap();
+        println!("cargo::rustc-cfg=MESON_GENERATED_RS");
+    } else if !Path::new("src/generated.rs.inc").exists() {
+        panic!(
+            "No generated C bindings found! Either build them manually with bindgen or with meson \
+             (`ninja aarch64-softmmu-generated.rs`) and copy them to src/generated.rs.inc, or build through meson."
+        );
+    }
+}
diff --git a/rust/pl011/deny.toml b/rust/pl011/deny.toml
new file mode 100644
index 0000000000..3992380509
--- /dev/null
+++ b/rust/pl011/deny.toml
@@ -0,0 +1,57 @@
+# cargo-deny configuration file
+
+[graph]
+targets = [
+    "aarch64-unknown-linux-gnu",
+    "x86_64-unknown-linux-gnu",
+    "x86_64-apple-darwin",
+    "aarch64-apple-darwin",
+    "x86_64-pc-windows-gnu",
+    "aarch64-pc-windows-gnullvm",
+]
+#exclude = []
+all-features = false
+no-default-features = false
+#features = []
+
+[output]
+feature-depth = 1
+
+[advisories]
+db-path = "$CARGO_HOME/advisory-dbs"
+db-urls = ["https://github.com/rustsec/advisory-db"]
+ignore = []
+
+[licenses]
+allow = [
+  "GPL-2.0",
+  "MIT",
+  "Apache-2.0",
+  "Unicode-DFS-2016",
+]
+confidence-threshold = 0.8
+exceptions = []
+
+[licenses.private]
+ignore = false
+registries = []
+
+[bans]
+multiple-versions = "warn"
+wildcards = "deny"
+# The graph highlighting used when creating dotgraphs for crates
+# with multiple versions
+# * lowest-version - The path to the lowest versioned duplicate is highlighted
+# * simplest-path - The path to the version with the fewest edges is highlighted
+# * all - Both lowest-version and simplest-path are used
+highlight = "all"
+workspace-default-features = "allow"
+external-default-features = "allow"
+allow = []
+deny = []
+
+[sources]
+unknown-registry = "deny"
+unknown-git = "deny"
+allow-registry = ["https://github.com/rust-lang/crates.io-index"]
+allow-git = []
diff --git a/rust/pl011/meson.build b/rust/pl011/meson.build
new file mode 100644
index 0000000000..70d1ca1148
--- /dev/null
+++ b/rust/pl011/meson.build
@@ -0,0 +1,7 @@
+rust_pl011 = {
+  'name': 'rust_pl011_cargo',
+  'dirname': 'pl011',
+  'output': 'libpl011.a',
+  }
+
+rust_hw_target_list += {'aarch64-softmmu': [rust_pl011]}
diff --git a/rust/pl011/rustfmt.toml b/rust/pl011/rustfmt.toml
new file mode 120000
index 0000000000..39f97b043b
--- /dev/null
+++ b/rust/pl011/rustfmt.toml
@@ -0,0 +1 @@
+../rustfmt.toml
\ No newline at end of file
diff --git a/rust/pl011/src/definitions.rs b/rust/pl011/src/definitions.rs
new file mode 100644
index 0000000000..f02604c114
--- /dev/null
+++ b/rust/pl011/src/definitions.rs
@@ -0,0 +1,95 @@
+//! Definitions required by QEMU when registering the device.
+
+use core::{ffi::CStr, mem::MaybeUninit, ptr::NonNull};
+
+use crate::{device::PL011State, device_class::pl011_class_init, generated::*};
+
+pub const TYPE_PL011: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"x-pl011-rust\0") };
+
+pub const PL011_ARM_INFO: TypeInfo = TypeInfo {
+    name: TYPE_PL011.as_ptr(),
+    parent: TYPE_SYS_BUS_DEVICE.as_ptr(),
+    instance_size: core::mem::size_of::<PL011State>(),
+    instance_align: core::mem::align_of::<PL011State>(),
+    instance_init: Some(pl011_init),
+    instance_post_init: None,
+    instance_finalize: None,
+    abstract_: false,
+    class_size: 0,
+    class_init: Some(pl011_class_init),
+    class_base_init: None,
+    class_data: core::ptr::null_mut(),
+    interfaces: core::ptr::null_mut(),
+};
+
+pub const VMSTATE_PL011: VMStateDescription = VMStateDescription {
+    name: PL011_ARM_INFO.name,
+    unmigratable: true,
+    ..unsafe { MaybeUninit::<VMStateDescription>::zeroed().assume_init() }
+};
+//version_id : 2,
+//minimum_version_id : 2,
+//post_load : pl011_post_load,
+//fields = (const VMStateField[]) {
+//    VMSTATE_UINT32(readbuff, PL011State),
+//    VMSTATE_UINT32(flags, PL011State),
+//    VMSTATE_UINT32(lcr, PL011State),
+//    VMSTATE_UINT32(rsr, PL011State),
+//    VMSTATE_UINT32(cr, PL011State),
+//    VMSTATE_UINT32(dmacr, PL011State),
+//    VMSTATE_UINT32(int_enabled, PL011State),
+//    VMSTATE_UINT32(int_level, PL011State),
+//    VMSTATE_UINT32_ARRAY(read_fifo, PL011State, PL011_FIFO_DEPTH),
+//    VMSTATE_UINT32(ilpr, PL011State),
+//    VMSTATE_UINT32(ibrd, PL011State),
+//    VMSTATE_UINT32(fbrd, PL011State),
+//    VMSTATE_UINT32(ifl, PL011State),
+//    VMSTATE_INT32(read_pos, PL011State),
+//    VMSTATE_INT32(read_count, PL011State),
+//    VMSTATE_INT32(read_trigger, PL011State),
+//    VMSTATE_END_OF_LIST()
+//},
+//.subsections = (const VMStateDescription * const []) {
+//    &vmstate_pl011_clock,
+//    NULL
+//}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
+    assert!(!obj.is_null());
+    let mut state = NonNull::new_unchecked(obj.cast::<PL011State>());
+    state.as_mut().init();
+}
+
+use crate::generated::module_init_type_MODULE_INIT_QOM;
+
+#[macro_export]
+macro_rules! module_init {
+    ($func:expr, $type:expr) => {
+        #[cfg_attr(target_os = "linux", link_section = ".ctors")]
+        #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
+        #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
+        pub static LOAD_MODULE: extern "C" fn() = {
+            assert!($type < $crate::generated::module_init_type_MODULE_INIT_MAX);
+
+            extern "C" fn __load() {
+                // ::std::panic::set_hook(::std::boxed::Box::new(|_| {}));
+
+                unsafe {
+                    $crate::generated::register_module_init(Some($func), $type);
+                }
+            }
+
+            __load
+        };
+    };
+}
+
+#[no_mangle]
+unsafe extern "C" fn register_type() {
+    crate::generated::type_register_static(&PL011_ARM_INFO);
+}
+
+module_init! {
+    register_type, module_init_type_MODULE_INIT_QOM
+}
diff --git a/rust/pl011/src/device.rs b/rust/pl011/src/device.rs
new file mode 100644
index 0000000000..405f590f03
--- /dev/null
+++ b/rust/pl011/src/device.rs
@@ -0,0 +1,531 @@
+use core::{
+    ffi::{c_int, c_uchar, c_uint, c_void, CStr},
+    mem::MaybeUninit,
+    ptr::{addr_of, addr_of_mut, NonNull},
+};
+
+use crate::{
+    definitions::PL011_ARM_INFO,
+    generated::{self, *},
+    memory_ops::PL011_OPS,
+    registers::{self, Interrupt},
+    RegisterOffset,
+};
+
+static PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1];
+
+const DATA_BREAK: u32 = 1 << 10;
+
+/// QEMU sourced constant.
+pub const PL011_FIFO_DEPTH: usize = 16_usize;
+
+#[repr(C)]
+#[derive(Debug)]
+/// PL011 Device Model in QEMU
+pub struct PL011State {
+    pub parent_obj: SysBusDevice,
+    pub iomem: MemoryRegion,
+    pub readbuff: u32,
+    #[doc(alias = "fr")]
+    pub flags: registers::Flags,
+    #[doc(alias = "lcr")]
+    pub line_control: registers::LineControl,
+    #[doc(alias = "rsr")]
+    pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
+    #[doc(alias = "cr")]
+    pub control: registers::Control,
+    pub dmacr: u32,
+    pub int_enabled: u32,
+    pub int_level: u32,
+    pub read_fifo: [u32; PL011_FIFO_DEPTH],
+    pub ilpr: u32,
+    pub ibrd: u32,
+    pub fbrd: u32,
+    pub ifl: u32,
+    pub read_pos: usize,
+    pub read_count: usize,
+    pub read_trigger: usize,
+    #[doc(alias = "chr")]
+    pub char_backend: CharBackend,
+    /// QEMU interrupts
+    ///
+    /// ```text
+    ///  * sysbus MMIO region 0: device registers
+    ///  * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
+    ///  * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
+    ///  * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
+    ///  * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
+    ///  * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
+    ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
+    /// ```
+    #[doc(alias = "irq")]
+    pub interrupts: [qemu_irq; 6usize],
+    #[doc(alias = "clk")]
+    pub clock: NonNull<Clock>,
+    #[doc(alias = "migrate_clk")]
+    pub migrate_clock: bool,
+}
+
+impl PL011State {
+    pub fn init(&mut self) {
+        unsafe {
+            memory_region_init_io(
+                addr_of_mut!(self.iomem),
+                addr_of_mut!(*self).cast::<Object>(),
+                &PL011_OPS,
+                addr_of_mut!(*self).cast::<c_void>(),
+                PL011_ARM_INFO.name,
+                0x1000,
+            );
+            let sbd = addr_of_mut!(*self).cast::<SysBusDevice>();
+            let dev = addr_of_mut!(*self).cast::<DeviceState>();
+            sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
+            for irq in self.interrupts.iter_mut() {
+                sysbus_init_irq(sbd, irq);
+            }
+            const CLK_NAME: &CStr = unsafe { CStr::from_bytes_with_nul_unchecked(b"clk\0") };
+            self.clock = NonNull::new(qdev_init_clock_in(
+                dev,
+                CLK_NAME.as_ptr(),
+                None, /* pl011_clock_update */
+                addr_of_mut!(*self).cast::<c_void>(),
+                ClockEvent_ClockUpdate,
+            ))
+            .unwrap();
+        }
+    }
+
+    pub fn read(&mut self, offset: hwaddr, _size: core::ffi::c_uint) -> u64 {
+        use RegisterOffset::*;
+
+        match RegisterOffset::try_from(offset) {
+            Err(v) if (0x3f8..0x400).contains(&v) => {
+                u64::from(PL011_ID_ARM[((offset - 0xfe0) >> 2) as usize])
+            }
+            Err(_) => {
+                // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
+                0
+            }
+            Ok(DR) => {
+                // s->flags &= ~PL011_FLAG_RXFF;
+                self.flags.set_receive_fifo_full(false);
+                let c = self.read_fifo[self.read_pos];
+                if self.read_count > 0 {
+                    self.read_count -= 1;
+                    self.read_pos = (self.read_pos + 1) & (self.fifo_depth() - 1);
+                }
+                if self.read_count == 0 {
+                    // self.flags |= PL011_FLAG_RXFE;
+                    self.flags.set_receive_fifo_empty(true);
+                }
+                if self.read_count + 1 == self.read_trigger {
+                    //self.int_level &= ~ INT_RX;
+                    self.int_level &= !registers::INT_RX;
+                }
+                // Update error bits.
+                self.receive_status_error_clear = c.to_be_bytes()[3].into();
+                self.update();
+                unsafe { qemu_chr_fe_accept_input(&mut self.char_backend) };
+                c.into()
+            }
+            Ok(RSR) => u8::from(self.receive_status_error_clear).into(),
+            Ok(FR) => u16::from(self.flags).into(),
+            Ok(FBRD) => self.fbrd.into(),
+            Ok(ILPR) => self.ilpr.into(),
+            Ok(IBRD) => self.ibrd.into(),
+            Ok(LCR_H) => u16::from(self.line_control).into(),
+            Ok(CR) => {
+                // We exercise our self-control.
+                u16::from(self.control).into()
+            }
+            Ok(FLS) => self.ifl.into(),
+            Ok(IMSC) => self.int_enabled.into(),
+            Ok(RIS) => self.int_level.into(),
+            Ok(MIS) => u64::from(self.int_level & self.int_enabled),
+            Ok(ICR) => {
+                // "The UARTICR Register is the interrupt clear register and is write-only"
+                // Source: ARM DDI 0183G 3.3.13 Interrupt Clear Register, UARTICR
+                0
+            }
+            Ok(DMACR) => self.dmacr.into(),
+        }
+    }
+
+    pub fn write(&mut self, offset: hwaddr, value: u64) {
+        // eprintln!("write offset {offset} value {value}");
+        use RegisterOffset::*;
+        let value: u32 = value as u32;
+        match RegisterOffset::try_from(offset) {
+            Err(_bad_offset) => {
+                eprintln!("write bad offset {offset} value {value}");
+            }
+            Ok(DR) => {
+                // ??? Check if transmitter is enabled.
+                let ch: u8 = value as u8;
+                // XXX this blocks entire thread. Rewrite to use
+                // qemu_chr_fe_write and background I/O callbacks
+                unsafe {
+                    qemu_chr_fe_write_all(addr_of_mut!(self.char_backend), &ch, 1);
+                }
+                self.loopback_tx(value);
+                self.int_level |= registers::INT_TX;
+                self.update();
+            }
+            Ok(RSR) => {
+                self.receive_status_error_clear = 0.into();
+            }
+            Ok(FR) => {
+                // flag writes are ignored
+            }
+            Ok(ILPR) => {
+                self.ilpr = value;
+            }
+            Ok(IBRD) => {
+                self.ibrd = value;
+            }
+            Ok(FBRD) => {
+                self.fbrd = value;
+            }
+            Ok(LCR_H) => {
+                let value = value as u16;
+                let new_val: registers::LineControl = value.into();
+                // Reset the FIFO state on FIFO enable or disable
+                if bool::from(self.line_control.fifos_enabled())
+                    ^ bool::from(new_val.fifos_enabled())
+                {
+                    self.reset_fifo();
+                }
+                if self.line_control.send_break() ^ new_val.send_break() {
+                    let mut break_enable: c_int = new_val.send_break().into();
+                    unsafe {
+                        qemu_chr_fe_ioctl(
+                            addr_of_mut!(self.char_backend),
+                            CHR_IOCTL_SERIAL_SET_BREAK as i32,
+                            addr_of_mut!(break_enable).cast::<c_void>(),
+                        );
+                    }
+                    self.loopback_break(break_enable > 0);
+                }
+                self.line_control = new_val;
+                self.set_read_trigger();
+            }
+            Ok(CR) => {
+                // ??? Need to implement the enable bit.
+                let value = value as u16;
+                self.control = value.into();
+                self.loopback_mdmctrl();
+            }
+            Ok(FLS) => {
+                self.ifl = value;
+                self.set_read_trigger();
+            }
+            Ok(IMSC) => {
+                self.int_enabled = value;
+                self.update();
+            }
+            Ok(RIS) => {}
+            Ok(MIS) => {}
+            Ok(ICR) => {
+                self.int_level &= !value;
+                self.update();
+            }
+            Ok(DMACR) => {
+                self.dmacr = value;
+                if value & 3 > 0 {
+                    // qemu_log_mask(LOG_UNIMP, "pl011: DMA not implemented\n");
+                    eprintln!("pl011: DMA not implemented");
+                }
+            }
+        }
+    }
+
+    #[inline]
+    fn loopback_tx(&mut self, value: u32) {
+        if !self.loopback_enabled() {
+            return;
+        }
+
+        // Caveat:
+        //
+        // In real hardware, TX loopback happens at the serial-bit level
+        // and then reassembled by the RX logics back into bytes and placed
+        // into the RX fifo. That is, loopback happens after TX fifo.
+        //
+        // Because the real hardware TX fifo is time-drained at the frame
+        // rate governed by the configured serial format, some loopback
+        // bytes in TX fifo may still be able to get into the RX fifo
+        // that could be full at times while being drained at software
+        // pace.
+        //
+        // In such scenario, the RX draining pace is the major factor
+        // deciding which loopback bytes get into the RX fifo, unless
+        // hardware flow-control is enabled.
+        //
+        // For simplicity, the above described is not emulated.
+        self.put_fifo(value);
+    }
+
+    fn loopback_mdmctrl(&mut self) {
+        if !self.loopback_enabled() {
+            return;
+        }
+
+        /*
+         * Loopback software-driven modem control outputs to modem status inputs:
+         *   FR.RI  <= CR.Out2
+         *   FR.DCD <= CR.Out1
+         *   FR.CTS <= CR.RTS
+         *   FR.DSR <= CR.DTR
+         *
+         * The loopback happens immediately even if this call is triggered
+         * by setting only CR.LBE.
+         *
+         * CTS/RTS updates due to enabled hardware flow controls are not
+         * dealt with here.
+         */
+
+        //fr = s->flags & ~(PL011_FLAG_RI | PL011_FLAG_DCD |
+        //                  PL011_FLAG_DSR | PL011_FLAG_CTS);
+        //fr |= (cr & CR_OUT2) ? PL011_FLAG_RI  : 0;
+        //fr |= (cr & CR_OUT1) ? PL011_FLAG_DCD : 0;
+        //fr |= (cr & CR_RTS)  ? PL011_FLAG_CTS : 0;
+        //fr |= (cr & CR_DTR)  ? PL011_FLAG_DSR : 0;
+        //
+        self.flags.set_ring_indicator(self.control.out_2());
+        self.flags.set_data_carrier_detect(self.control.out_1());
+        self.flags.set_clear_to_send(self.control.request_to_send());
+        self.flags
+            .set_data_set_ready(self.control.data_transmit_ready());
+
+        // Change interrupts based on updated FR
+        let mut il = self.int_level;
+
+        il &= !Interrupt::MS;
+        //il |= (fr & PL011_FLAG_DSR) ? INT_DSR : 0;
+        //il |= (fr & PL011_FLAG_DCD) ? INT_DCD : 0;
+        //il |= (fr & PL011_FLAG_CTS) ? INT_CTS : 0;
+        //il |= (fr & PL011_FLAG_RI)  ? INT_RI  : 0;
+
+        if self.flags.data_set_ready() {
+            il |= Interrupt::DSR as u32;
+        }
+        if self.flags.data_carrier_detect() {
+            il |= Interrupt::DCD as u32;
+        }
+        if self.flags.clear_to_send() {
+            il |= Interrupt::CTS as u32;
+        }
+        if self.flags.ring_indicator() {
+            il |= Interrupt::RI as u32;
+        }
+        self.int_level = il;
+        self.update();
+    }
+
+    fn loopback_break(&mut self, enable: bool) {
+        if enable {
+            self.loopback_tx(DATA_BREAK);
+        }
+    }
+
+    fn set_read_trigger(&mut self) {
+        //#if 0
+        //    /* The docs say the RX interrupt is triggered when the FIFO exceeds
+        //       the threshold.  However linux only reads the FIFO in response to an
+        //       interrupt.  Triggering the interrupt when the FIFO is non-empty seems
+        //       to make things work.  */
+        //    if (s->lcr & LCR_FEN)
+        //        s->read_trigger = (s->ifl >> 1) & 0x1c;
+        //    else
+        //#endif
+        self.read_trigger = 1;
+    }
+
+    pub fn realize(&mut self) {
+        unsafe {
+            qemu_chr_fe_set_handlers(
+                addr_of_mut!(self.char_backend),
+                Some(pl011_can_receive),
+                Some(pl011_receive),
+                Some(pl011_event),
+                None,
+                addr_of_mut!(*self).cast::<c_void>(),
+                core::ptr::null_mut(),
+                true,
+            );
+        }
+    }
+
+    pub fn reset(&mut self) {
+        self.line_control.reset();
+        self.receive_status_error_clear.reset();
+        self.dmacr = 0;
+        self.int_enabled = 0;
+        self.int_level = 0;
+        self.ilpr = 0;
+        self.ibrd = 0;
+        self.fbrd = 0;
+        self.read_trigger = 1;
+        self.ifl = 0x12;
+        self.control.reset();
+        self.flags = 0.into();
+        self.reset_fifo();
+    }
+
+    pub fn reset_fifo(&mut self) {
+        self.read_count = 0;
+        self.read_pos = 0;
+
+        /* Reset FIFO flags */
+        self.flags.reset();
+    }
+
+    pub fn can_receive(&self) -> bool {
+        // trace_pl011_can_receive(s->lcr, s->read_count, r);
+        self.read_count < self.fifo_depth()
+    }
+
+    pub fn event(&mut self, event: QEMUChrEvent) {
+        if event == generated::QEMUChrEvent_CHR_EVENT_BREAK && !self.fifo_enabled() {
+            self.put_fifo(DATA_BREAK);
+            self.receive_status_error_clear.set_break_error(true);
+        }
+    }
+
+    #[inline]
+    pub fn fifo_enabled(&self) -> bool {
+        matches!(self.line_control.fifos_enabled(), registers::Mode::FIFO)
+    }
+
+    #[inline]
+    pub fn loopback_enabled(&self) -> bool {
+        self.control.enable_loopback()
+    }
+
+    #[inline]
+    pub fn fifo_depth(&self) -> usize {
+        // Note: FIFO depth is expected to be power-of-2
+        if self.fifo_enabled() {
+            return PL011_FIFO_DEPTH;
+        }
+        1
+    }
+
+    pub fn put_fifo(&mut self, value: c_uint) {
+        let depth = self.fifo_depth();
+        assert!(depth > 0);
+        let slot = (self.read_pos + self.read_count) & (depth - 1);
+        self.read_fifo[slot] = value;
+        self.read_count += 1;
+        // s->flags &= ~PL011_FLAG_RXFE;
+        self.flags.set_receive_fifo_empty(false);
+        if self.read_count == depth {
+            //s->flags |= PL011_FLAG_RXFF;
+            self.flags.set_receive_fifo_full(true);
+        }
+
+        if self.read_count == self.read_trigger {
+            self.int_level |= registers::INT_RX;
+            self.update();
+        }
+    }
+
+    pub fn update(&mut self) {
+        let flags = self.int_level & self.int_enabled;
+        for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
+            unsafe { qemu_set_irq(*irq, i32::from(flags & i != 0)) };
+        }
+    }
+}
+
+/// Which bits in the interrupt status matter for each outbound IRQ line ?
+pub const IRQMASK: [u32; 6] = [
+    /* combined IRQ */
+    Interrupt::E
+        | Interrupt::MS
+        | Interrupt::RT as u32
+        | Interrupt::TX as u32
+        | Interrupt::RX as u32,
+    Interrupt::RX as u32,
+    Interrupt::TX as u32,
+    Interrupt::RT as u32,
+    Interrupt::MS,
+    Interrupt::E,
+];
+
+pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int {
+    assert!(!opaque.is_null());
+    let state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+    state.as_ref().can_receive().into()
+}
+pub unsafe extern "C" fn pl011_receive(
+    opaque: *mut core::ffi::c_void,
+    buf: *const u8,
+    size: core::ffi::c_int,
+) {
+    assert!(!opaque.is_null());
+    let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+    if state.as_ref().loopback_enabled() {
+        return;
+    }
+    if size > 0 {
+        assert!(!buf.is_null());
+        state.as_mut().put_fifo(*buf.cast::<c_uint>())
+    }
+}
+
+pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) {
+    assert!(!opaque.is_null());
+    let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+    state.as_mut().event(event)
+}
+
+pub const VMSTATE_PL011: VMStateDescription = VMStateDescription {
+    name: PL011_ARM_INFO.name,
+    unmigratable: true,
+    ..unsafe { MaybeUninit::<VMStateDescription>::zeroed().assume_init() }
+};
+//version_id : 2,
+//minimum_version_id : 2,
+//post_load : pl011_post_load,
+//fields = (const VMStateField[]) {
+//    VMSTATE_UINT32(readbuff, PL011State),
+//    VMSTATE_UINT32(flags, PL011State),
+//    VMSTATE_UINT32(lcr, PL011State),
+//    VMSTATE_UINT32(rsr, PL011State),
+//    VMSTATE_UINT32(cr, PL011State),
+//    VMSTATE_UINT32(dmacr, PL011State),
+//    VMSTATE_UINT32(int_enabled, PL011State),
+//    VMSTATE_UINT32(int_level, PL011State),
+//    VMSTATE_UINT32_ARRAY(read_fifo, PL011State, PL011_FIFO_DEPTH),
+//    VMSTATE_UINT32(ilpr, PL011State),
+//    VMSTATE_UINT32(ibrd, PL011State),
+//    VMSTATE_UINT32(fbrd, PL011State),
+//    VMSTATE_UINT32(ifl, PL011State),
+//    VMSTATE_INT32(read_pos, PL011State),
+//    VMSTATE_INT32(read_count, PL011State),
+//    VMSTATE_INT32(read_trigger, PL011State),
+//    VMSTATE_END_OF_LIST()
+//},
+//.subsections = (const VMStateDescription * const []) {
+//    &vmstate_pl011_clock,
+//    NULL
+//}
+
+pub unsafe extern "C" fn pl011_create(
+    addr: u64,
+    irq: qemu_irq,
+    chr: *mut Chardev,
+) -> *mut DeviceState {
+    let dev: *mut DeviceState = unsafe { qdev_new(PL011_ARM_INFO.name) };
+    assert!(!dev.is_null());
+    let sysbus: *mut SysBusDevice = dev as *mut SysBusDevice;
+
+    unsafe {
+        qdev_prop_set_chr(dev, generated::TYPE_CHARDEV.as_ptr(), chr);
+        sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error);
+        sysbus_mmio_map(sysbus, 0, addr);
+        sysbus_connect_irq(sysbus, 0, irq);
+    }
+    dev
+}
diff --git a/rust/pl011/src/device_class.rs b/rust/pl011/src/device_class.rs
new file mode 100644
index 0000000000..50d91eb527
--- /dev/null
+++ b/rust/pl011/src/device_class.rs
@@ -0,0 +1,95 @@
+use core::{mem::MaybeUninit, ptr::NonNull};
+use std::sync::OnceLock;
+
+use crate::{
+    device::{PL011State, VMSTATE_PL011},
+    generated::*,
+};
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_class_init(klass: *mut ObjectClass, _: *mut core::ffi::c_void) {
+    let mut dc = NonNull::new(klass.cast::<DeviceClass>()).unwrap();
+    dc.as_mut().realize = Some(pl011_realize);
+    dc.as_mut().reset = Some(pl011_reset);
+    dc.as_mut().vmsd = &VMSTATE_PL011;
+    _ = PL011_PROPERTIES.get_or_init(make_pl011_properties);
+    device_class_set_props(
+        dc.as_mut(),
+        PL011_PROPERTIES.get_mut().unwrap().as_mut_ptr(),
+    );
+}
+
+#[macro_export]
+macro_rules! define_property {
+    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr) => {
+        $crate::generated::Property {
+            name: $name,
+            info: $prop,
+            offset: ::core::mem::offset_of!($state, $field) as _,
+            bitnr: 0,
+            bitmask: 0,
+            set_default: true,
+            defval: $crate::generated::Property__bindgen_ty_1 { u: $defval.into() },
+            arrayoffset: 0,
+            arrayinfo: ::core::ptr::null(),
+            arrayfieldsize: 0,
+            link_type: ::core::ptr::null(),
+        }
+    };
+    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr) => {
+        $crate::generated::Property {
+            name: $name,
+            info: $prop,
+            offset: ::core::mem::offset_of!($state, $field) as _,
+            bitnr: 0,
+            bitmask: 0,
+            set_default: false,
+            defval: $crate::generated::Property__bindgen_ty_1 { i: 0 },
+            arrayoffset: 0,
+            arrayinfo: ::core::ptr::null(),
+            arrayfieldsize: 0,
+            link_type: ::core::ptr::null(),
+        }
+    };
+}
+
+#[no_mangle]
+pub static mut PL011_PROPERTIES: OnceLock<[Property; 3]> = OnceLock::new();
+
+unsafe impl Send for Property {}
+unsafe impl Sync for Property {}
+
+#[no_mangle]
+fn make_pl011_properties() -> [Property; 3] {
+    [
+        define_property!(
+            c"chardev".as_ptr(),
+            PL011State,
+            char_backend,
+            unsafe { &qdev_prop_chr },
+            CharBackend
+        ),
+        define_property!(
+            c"migrate-clk".as_ptr(),
+            PL011State,
+            migrate_clock,
+            unsafe { &qdev_prop_bool },
+            bool
+        ),
+        unsafe { MaybeUninit::<Property>::zeroed().assume_init() },
+    ]
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) {
+    assert!(!dev.is_null());
+    let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
+    state.as_mut().realize();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) {
+    assert!(!dev.is_null());
+    let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
+    state.as_mut().reset();
+}
diff --git a/rust/pl011/src/generated.rs b/rust/pl011/src/generated.rs
new file mode 100644
index 0000000000..670e7b541d
--- /dev/null
+++ b/rust/pl011/src/generated.rs
@@ -0,0 +1,5 @@
+#[cfg(MESON_GENERATED_RS)]
+include!(concat!(env!("OUT_DIR"), "/generated.rs"));
+
+#[cfg(not(MESON_GENERATED_RS))]
+include!("generated.rs.inc");
diff --git a/rust/pl011/src/lib.rs b/rust/pl011/src/lib.rs
new file mode 100644
index 0000000000..bb2899dbc2
--- /dev/null
+++ b/rust/pl011/src/lib.rs
@@ -0,0 +1,581 @@
+// PL011 QEMU Device Model
+//
+// This library implements a device model for the PrimeCell® UART (PL011)
+// device in QEMU.
+//
+#![doc = include_str!("../README.md")]
+//! # Library crate
+//!
+//! See [`PL011State`](crate::device::PL011State) for the device model type and
+//! the [`registers`] module for register types.
+
+// FIXME: remove improper_ctypes
+#[allow(
+    improper_ctypes_definitions,
+    improper_ctypes,
+    non_camel_case_types,
+    non_snake_case,
+    non_upper_case_globals
+)]
+#[allow(
+    clippy::missing_const_for_fn,
+    clippy::useless_transmute,
+    clippy::too_many_arguments,
+    clippy::approx_constant,
+    clippy::use_self,
+    clippy::cast_lossless,
+)]
+#[rustfmt::skip]
+pub mod generated;
+
+pub mod definitions;
+pub mod device;
+pub mod device_class;
+pub mod memory_ops;
+
+/// Offset of each register from the base memory address of the device.
+///
+/// # Source
+/// ARM DDI 0183G, Table 3-1 p.3-3
+#[doc(alias = "offset")]
+#[allow(non_camel_case_types)]
+#[repr(u64)]
+#[derive(Debug)]
+pub enum RegisterOffset {
+    /// Data Register
+    ///
+    /// A write to this register initiates the actual data transmission
+    #[doc(alias = "UARTDR")]
+    DR = 0x000,
+    /// Receive Status Register or Error Clear Register
+    #[doc(alias = "UARTRSR")]
+    #[doc(alias = "UARTECR")]
+    RSR = 0x004,
+    /// Flag Register
+    ///
+    /// A read of this register shows if transmission is complete
+    #[doc(alias = "UARTFR")]
+    FR = 0x018,
+    /// Fractional Baud Rate Register
+    ///
+    /// responsible for baud rate speed
+    #[doc(alias = "UARTFBRD")]
+    FBRD = 0x028,
+    /// `IrDA` Low-Power Counter Register
+    #[doc(alias = "UARTILPR")]
+    ILPR = 0x020,
+    /// Integer Baud Rate Register
+    ///
+    /// Responsible for baud rate speed
+    #[doc(alias = "UARTIBRD")]
+    IBRD = 0x024,
+    /// line control register (data frame format)
+    #[doc(alias = "UARTLCR_H")]
+    LCR_H = 0x02C,
+    /// Toggle UART, transmission or reception
+    #[doc(alias = "UARTCR")]
+    CR = 0x030,
+    /// Interrupt FIFO Level Select Register
+    #[doc(alias = "UARTIFLS")]
+    FLS = 0x034,
+    /// Interrupt Mask Set/Clear Register
+    #[doc(alias = "UARTIMSC")]
+    IMSC = 0x038,
+    /// Raw Interrupt Status Register
+    #[doc(alias = "UARTRIS")]
+    RIS = 0x03C,
+    /// Masked Interrupt Status Register
+    #[doc(alias = "UARTMIS")]
+    MIS = 0x040,
+    /// Interrupt Clear Register
+    #[doc(alias = "UARTICR")]
+    ICR = 0x044,
+    /// DMA control Register
+    #[doc(alias = "UARTDMACR")]
+    DMACR = 0x048,
+    ///// Reserved, offsets `0x04C` to `0x07C`.
+    //Reserved = 0x04C,
+}
+
+impl core::convert::TryFrom<u64> for RegisterOffset {
+    type Error = u64;
+
+    fn try_from(value: u64) -> Result<Self, Self::Error> {
+        macro_rules! case {
+            ($($discriminant:ident),*$(,)*) => {
+                /* check that matching on all macro arguments compiles, which means we are not
+                 * missing any enum value; if the type definition ever changes this will stop
+                 * compiling.
+                 */
+                const fn _assert_exhaustive(val: RegisterOffset) {
+                    match val {
+                        $(RegisterOffset::$discriminant => (),)*
+                    }
+                }
+
+                match value {
+                    $(x if x == Self::$discriminant as u64 => Ok(Self::$discriminant),)*
+                     _ => Err(value),
+                }
+            }
+        }
+        case! { DR, RSR, FR, FBRD, ILPR, IBRD, LCR_H, CR, FLS, IMSC, RIS, MIS, ICR, DMACR }
+    }
+}
+
+pub mod registers {
+    //! Device registers exposed as typed structs which are backed by arbitrary
+    //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
+    //!
+    //! All PL011 registers are essentially 32-bit wide, but are typed here as
+    //! bitmaps with only the necessary width. That is, if a struct bitmap
+    //! in this module is for example 16 bits long, it should be conceived
+    //! as a 32-bit register where the unmentioned higher bits are always
+    //! unused thus treated as zero when read or written.
+    use bilge::prelude::*;
+
+    // TODO: FIFO Mode has different semantics
+    /// Data Register, `UARTDR`
+    ///
+    /// The `UARTDR` register is the data register.
+    ///
+    /// For words to be transmitted:
+    ///
+    /// - if the FIFOs are enabled, data written to this location is pushed onto
+    ///   the transmit
+    /// FIFO
+    /// - if the FIFOs are not enabled, data is stored in the transmitter
+    ///   holding register (the
+    /// bottom word of the transmit FIFO).
+    ///
+    /// The write operation initiates transmission from the UART. The data is
+    /// prefixed with a start bit, appended with the appropriate parity bit
+    /// (if parity is enabled), and a stop bit. The resultant word is then
+    /// transmitted.
+    ///
+    /// For received words:
+    ///
+    /// - if the FIFOs are enabled, the data byte and the 4-bit status (break,
+    ///   frame, parity,
+    /// and overrun) is pushed onto the 12-bit wide receive FIFO
+    /// - if the FIFOs are not enabled, the data byte and status are stored in
+    ///   the receiving
+    /// holding register (the bottom word of the receive FIFO).
+    ///
+    /// The received data byte is read by performing reads from the `UARTDR`
+    /// register along with the corresponding status information. The status
+    /// information can also be read by a read of the `UARTRSR/UARTECR`
+    /// register.
+    ///
+    /// # Note
+    ///
+    /// You must disable the UART before any of the control registers are
+    /// reprogrammed. When the UART is disabled in the middle of
+    /// transmission or reception, it completes the current character before
+    /// stopping.
+    ///
+    /// # Source
+    /// ARM DDI 0183G 3.3.1 Data Register, UARTDR
+    #[bitsize(16)]
+    #[derive(Clone, Copy, DebugBits, FromBits)]
+    #[doc(alias = "UARTDR")]
+    pub struct Data {
+        _reserved: u4,
+        pub data: u8,
+        pub framing_error: bool,
+        pub parity_error: bool,
+        pub break_error: bool,
+        pub overrun_error: bool,
+    }
+
+    // TODO: FIFO Mode has different semantics
+    /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
+    ///
+    /// The UARTRSR/UARTECR register is the receive status register/error clear
+    /// register. Receive status can also be read from the `UARTRSR`
+    /// register. If the status is read from this register, then the status
+    /// information for break, framing and parity corresponds to the
+    /// data character read from the [Data register](Data), `UARTDR` prior to
+    /// reading the UARTRSR register. The status information for overrun is
+    /// set immediately when an overrun condition occurs.
+    ///
+    ///
+    /// # Note
+    /// The received data character must be read first from the [Data
+    /// Register](Data), `UARTDR` before reading the error status associated
+    /// with that data character from the `UARTRSR` register. This read
+    /// sequence cannot be reversed, because the `UARTRSR` register is
+    /// updated only when a read occurs from the `UARTDR` register. However,
+    /// the status information can also be obtained by reading the `UARTDR`
+    /// register
+    ///
+    /// # Source
+    /// ARM DDI 0183G 3.3.2 Receive Status Register/Error Clear Register,
+    /// UARTRSR/UARTECR
+    #[bitsize(8)]
+    #[derive(Clone, Copy, DebugBits, FromBits)]
+    pub struct ReceiveStatusErrorClear {
+        pub framing_error: bool,
+        pub parity_error: bool,
+        pub break_error: bool,
+        pub overrun_error: bool,
+        _reserved_unpredictable: u4,
+    }
+
+    impl ReceiveStatusErrorClear {
+        pub fn reset(&mut self) {
+            // All the bits are cleared to 0 on reset.
+            *self = 0.into();
+        }
+    }
+
+    impl Default for ReceiveStatusErrorClear {
+        fn default() -> Self {
+            0.into()
+        }
+    }
+
+    #[bitsize(16)]
+    #[derive(Clone, Copy, DebugBits, FromBits)]
+    /// Flag Register, `UARTFR`
+    #[doc(alias = "UARTFR")]
+    pub struct Flags {
+        /// CTS Clear to send. This bit is the complement of the UART clear to
+        /// send, `nUARTCTS`, modem status input. That is, the bit is 1
+        /// when `nUARTCTS` is LOW.
+        pub clear_to_send: bool,
+        /// DSR Data set ready. This bit is the complement of the UART data set
+        /// ready, `nUARTDSR`, modem status input. That is, the bit is 1 when
+        /// `nUARTDSR` is LOW.
+        pub data_set_ready: bool,
+        /// DCD Data carrier detect. This bit is the complement of the UART data
+        /// carrier detect, `nUARTDCD`, modem status input. That is, the bit is
+        /// 1 when `nUARTDCD` is LOW.
+        pub data_carrier_detect: bool,
+        /// BUSY UART busy. If this bit is set to 1, the UART is busy
+        /// transmitting data. This bit remains set until the complete
+        /// byte, including all the stop bits, has been sent from the
+        /// shift register. This bit is set as soon as the transmit FIFO
+        /// becomes non-empty, regardless of whether the UART is enabled
+        /// or not.
+        pub busy: bool,
+        /// RXFE Receive FIFO empty. The meaning of this bit depends on the
+        /// state of the FEN bit in the UARTLCR_H register. If the FIFO
+        /// is disabled, this bit is set when the receive holding
+        /// register is empty. If the FIFO is enabled, the RXFE bit is
+        /// set when the receive FIFO is empty.
+        pub receive_fifo_empty: bool,
+        /// TXFF Transmit FIFO full. The meaning of this bit depends on the
+        /// state of the FEN bit in the UARTLCR_H register. If the FIFO
+        /// is disabled, this bit is set when the transmit holding
+        /// register is full. If the FIFO is enabled, the TXFF bit is
+        /// set when the transmit FIFO is full.
+        pub transmit_fifo_full: bool,
+        /// RXFF Receive FIFO full. The meaning of this bit depends on the state
+        /// of the FEN bit in the UARTLCR_H register. If the FIFO is
+        /// disabled, this bit is set when the receive holding register
+        /// is full. If the FIFO is enabled, the RXFF bit is set when
+        /// the receive FIFO is full.
+        pub receive_fifo_full: bool,
+        /// Transmit FIFO empty. The meaning of this bit depends on the state of
+        /// the FEN bit in the [Line Control register](LineControl),
+        /// `UARTLCR_H`. If the FIFO is disabled, this bit is set when the
+        /// transmit holding register is empty. If the FIFO is enabled,
+        /// the TXFE bit is set when the transmit FIFO is empty. This
+        /// bit does not indicate if there is data in the transmit shift
+        /// register.
+        pub transmit_fifo_empty: bool,
+        /// `RI`, is `true` when `nUARTRI` is `LOW`.
+        pub ring_indicator: bool,
+        _reserved_zero_no_modify: u7,
+    }
+
+    impl Flags {
+        pub fn reset(&mut self) {
+            // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
+            self.set_receive_fifo_full(false);
+            self.set_transmit_fifo_full(false);
+            self.set_busy(false);
+            self.set_receive_fifo_empty(true);
+            self.set_transmit_fifo_empty(true);
+        }
+    }
+
+    impl Default for Flags {
+        fn default() -> Self {
+            let mut ret: Self = 0.into();
+            ret.reset();
+            ret
+        }
+    }
+
+    #[bitsize(16)]
+    #[derive(Clone, Copy, DebugBits, FromBits)]
+    /// Line Control Register, `UARTLCR_H`
+    #[doc(alias = "UARTLCR_H")]
+    pub struct LineControl {
+        /// 15:8 - Reserved, do not modify, read as zero.
+        _reserved_zero_no_modify: u8,
+        /// 7 SPS Stick parity select.
+        /// 0 = stick parity is disabled
+        /// 1 = either:
+        /// • if the EPS bit is 0 then the parity bit is transmitted and checked
+        /// as a 1 • if the EPS bit is 1 then the parity bit is
+        /// transmitted and checked as a 0. This bit has no effect when
+        /// the PEN bit disables parity checking and generation. See Table 3-11
+        /// on page 3-14 for the parity truth table.
+        pub sticky_parity: bool,
+        /// WLEN Word length. These bits indicate the number of data bits
+        /// transmitted or received in a frame as follows: b11 = 8 bits
+        /// b10 = 7 bits
+        /// b01 = 6 bits
+        /// b00 = 5 bits.
+        pub word_length: WordLength,
+        /// FEN Enable FIFOs:
+        /// 0 = FIFOs are disabled (character mode) that is, the FIFOs become
+        /// 1-byte-deep holding registers 1 = transmit and receive FIFO
+        /// buffers are enabled (FIFO mode).
+        pub fifos_enabled: Mode,
+        /// 3 STP2 Two stop bits select. If this bit is set to 1, two stop bits
+        /// are transmitted at the end of the frame. The receive
+        /// logic does not check for two stop bits being received.
+        pub two_stops_bits: bool,
+        /// EPS Even parity select. Controls the type of parity the UART uses
+        /// during transmission and reception:
+        /// - 0 = odd parity. The UART generates or checks for an odd number of
+        ///   1s in the data and parity bits.
+        /// - 1 = even parity. The UART generates or checks for an even number
+        ///   of 1s in the data and parity bits.
+        /// This bit has no effect when the `PEN` bit disables parity checking
+        /// and generation. See Table 3-11 on page 3-14 for the parity
+        /// truth table.
+        pub parity: Parity,
+        /// 1 PEN Parity enable:
+        ///
+        /// - 0 = parity is disabled and no parity bit added to the data frame
+        /// - 1 = parity checking and generation is enabled.
+        ///
+        /// See Table 3-11 on page 3-14 for the parity truth table.
+        pub parity_enabled: bool,
+        /// BRK Send break.
+        ///
+        /// If this bit is set to `1`, a low-level is continually output on the
+        /// `UARTTXD` output, after completing transmission of the
+        /// current character. For the proper execution of the break command,
+        /// the software must set this bit for at least two complete
+        /// frames. For normal use, this bit must be cleared to `0`.
+        pub send_break: bool,
+    }
+
+    impl LineControl {
+        pub fn reset(&mut self) {
+            // All the bits are cleared to 0 when reset.
+            *self = 0.into();
+        }
+    }
+
+    impl Default for LineControl {
+        fn default() -> Self {
+            0.into()
+        }
+    }
+
+    #[bitsize(1)]
+    #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+    /// `EPS` "Even parity select", field of [Line Control
+    /// register](LineControl).
+    pub enum Parity {
+        /// - 0 = odd parity. The UART generates or checks for an odd number of
+        ///   1s in the data and parity bits.
+        Odd = 0,
+        /// - 1 = even parity. The UART generates or checks for an even number
+        ///   of 1s in the data and parity bits.
+        Even = 1,
+    }
+
+    #[bitsize(1)]
+    #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+    /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
+    /// register](LineControl).
+    pub enum Mode {
+        /// 0 = FIFOs are disabled (character mode) that is, the FIFOs become
+        /// 1-byte-deep holding registers
+        Character = 0,
+        /// 1 = transmit and receive FIFO buffers are enabled (FIFO mode).
+        FIFO = 1,
+    }
+
+    impl From<Mode> for bool {
+        fn from(val: Mode) -> Self {
+            matches!(val, Mode::FIFO)
+        }
+    }
+
+    #[bitsize(2)]
+    #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+    /// `WLEN` Word length, field of [Line Control register](LineControl).
+    ///
+    /// These bits indicate the number of data bits transmitted or received in a
+    /// frame as follows:
+    pub enum WordLength {
+        /// b11 = 8 bits
+        _8Bits = 0b11,
+        /// b10 = 7 bits
+        _7Bits = 0b10,
+        /// b01 = 6 bits
+        _6Bits = 0b01,
+        /// b00 = 5 bits.
+        _5Bits = 0b00,
+    }
+
+    /// Control Register, `UARTCR`
+    ///
+    /// The `UARTCR` register is the control register. All the bits are cleared
+    /// to `0` on reset except for bits `9` and `8` that are set to `1`.
+    ///
+    /// # Source
+    /// ARM DDI 0183G, 3.3.8 Control Register, `UARTCR`, Table 3-12
+    #[bitsize(16)]
+    #[doc(alias = "UARTCR")]
+    #[derive(Clone, Copy, DebugBits, FromBits)]
+    pub struct Control {
+        /// `UARTEN` UART enable: 0 = UART is disabled. If the UART is disabled
+        /// in the middle of transmission or reception, it completes the current
+        /// character before stopping. 1 = the UART is enabled. Data
+        /// transmission and reception occurs for either UART signals or SIR
+        /// signals depending on the setting of the SIREN bit.
+        pub enable_uart: bool,
+        /// `SIREN` `SIR` enable: 0 = IrDA SIR ENDEC is disabled. `nSIROUT`
+        /// remains LOW (no light pulse generated), and signal transitions on
+        /// SIRIN have no effect. 1 = IrDA SIR ENDEC is enabled. Data is
+        /// transmitted and received on nSIROUT and SIRIN. UARTTXD remains HIGH,
+        /// in the marking state. Signal transitions on UARTRXD or modem status
+        /// inputs have no effect. This bit has no effect if the UARTEN bit
+        /// disables the UART.
+        pub enable_sir: bool,
+        /// `SIRLP` SIR low-power IrDA mode. This bit selects the IrDA encoding
+        /// mode. If this bit is cleared to 0, low-level bits are transmitted as
+        /// an active high pulse with a width of 3/ 16th of the bit period. If
+        /// this bit is set to 1, low-level bits are transmitted with a pulse
+        /// width that is 3 times the period of the IrLPBaud16 input signal,
+        /// regardless of the selected bit rate. Setting this bit uses less
+        /// power, but might reduce transmission distances.
+        pub sir_lowpower_irda_mode: u1,
+        /// Reserved, do not modify, read as zero.
+        _reserved_zero_no_modify: u4,
+        /// `LBE` Loopback enable. If this bit is set to 1 and the SIREN bit is
+        /// set to 1 and the SIRTEST bit in the Test Control register, UARTTCR
+        /// on page 4-5 is set to 1, then the nSIROUT path is inverted, and fed
+        /// through to the SIRIN path. The SIRTEST bit in the test register must
+        /// be set to 1 to override the normal half-duplex SIR operation. This
+        /// must be the requirement for accessing the test registers during
+        /// normal operation, and SIRTEST must be cleared to 0 when loopback
+        /// testing is finished. This feature reduces the amount of external
+        /// coupling required during system test. If this bit is set to 1, and
+        /// the SIRTEST bit is set to 0, the UARTTXD path is fed through to the
+        /// UARTRXD path. In either SIR mode or UART mode, when this bit is set,
+        /// the modem outputs are also fed through to the modem inputs. This bit
+        /// is cleared to 0 on reset, to disable loopback.
+        pub enable_loopback: bool,
+        /// `TXE` Transmit enable. If this bit is set to 1, the transmit section
+        /// of the UART is enabled. Data transmission occurs for either UART
+        /// signals, or SIR signals depending on the setting of the SIREN bit.
+        /// When the UART is disabled in the middle of transmission, it
+        /// completes the current character before stopping.
+        pub enable_transmit: bool,
+        /// `RXE` Receive enable. If this bit is set to 1, the receive section
+        /// of the UART is enabled. Data reception occurs for either UART
+        /// signals or SIR signals depending on the setting of the SIREN bit.
+        /// When the UART is disabled in the middle of reception, it completes
+        /// the current character before stopping.
+        pub enable_receive: bool,
+        /// `DTR` Data transmit ready. This bit is the complement of the UART
+        /// data transmit ready, `nUARTDTR`, modem status output. That is, when
+        /// the bit is programmed to a 1 then `nUARTDTR` is LOW.
+        pub data_transmit_ready: bool,
+        /// `RTS` Request to send. This bit is the complement of the UART
+        /// request to send, `nUARTRTS`, modem status output. That is, when the
+        /// bit is programmed to a 1 then `nUARTRTS` is LOW.
+        pub request_to_send: bool,
+        /// `Out1` This bit is the complement of the UART Out1 (`nUARTOut1`)
+        /// modem status output. That is, when the bit is programmed to a 1 the
+        /// output is 0. For DTE this can be used as Data Carrier Detect (DCD).
+        pub out_1: bool,
+        /// `Out2` This bit is the complement of the UART Out2 (`nUARTOut2`)
+        /// modem status output. That is, when the bit is programmed to a 1, the
+        /// output is 0. For DTE this can be used as Ring Indicator (RI).
+        pub out_2: bool,
+        /// `RTSEn` RTS hardware flow control enable. If this bit is set to 1,
+        /// RTS hardware flow control is enabled. Data is only requested when
+        /// there is space in the receive FIFO for it to be received.
+        pub rts_hardware_flow_control_enable: bool,
+        /// `CTSEn` CTS hardware flow control enable. If this bit is set to 1,
+        /// CTS hardware flow control is enabled. Data is only transmitted when
+        /// the `nUARTCTS` signal is asserted.
+        pub cts_hardware_flow_control_enable: bool,
+    }
+
+    impl Control {
+        pub fn reset(&mut self) {
+            *self = 0.into();
+            self.set_enable_receive(true);
+            self.set_enable_transmit(true);
+        }
+    }
+
+    impl Default for Control {
+        fn default() -> Self {
+            let mut ret: Self = 0.into();
+            ret.reset();
+            ret
+        }
+    }
+
+    /// Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC
+    pub const INT_OE: u32 = 1 << 10;
+    pub const INT_BE: u32 = 1 << 9;
+    pub const INT_PE: u32 = 1 << 8;
+    pub const INT_FE: u32 = 1 << 7;
+    pub const INT_RT: u32 = 1 << 6;
+    pub const INT_TX: u32 = 1 << 5;
+    pub const INT_RX: u32 = 1 << 4;
+    pub const INT_DSR: u32 = 1 << 3;
+    pub const INT_DCD: u32 = 1 << 2;
+    pub const INT_CTS: u32 = 1 << 1;
+    pub const INT_RI: u32 = 1 << 0;
+    pub const INT_E: u32 = INT_OE | INT_BE | INT_PE | INT_FE;
+    pub const INT_MS: u32 = INT_RI | INT_DSR | INT_DCD | INT_CTS;
+
+    #[repr(u32)]
+    pub enum Interrupt {
+        OE = 1 << 10,
+        BE = 1 << 9,
+        PE = 1 << 8,
+        FE = 1 << 7,
+        RT = 1 << 6,
+        TX = 1 << 5,
+        RX = 1 << 4,
+        DSR = 1 << 3,
+        DCD = 1 << 2,
+        CTS = 1 << 1,
+        RI = 1 << 0,
+    }
+
+    impl Interrupt {
+        pub const E: u32 = INT_OE | INT_BE | INT_PE | INT_FE;
+        pub const MS: u32 = INT_RI | INT_DSR | INT_DCD | INT_CTS;
+    }
+}
+
+// State
+// TODO You must disable the UART before any of the control registers are
+// reprogrammed. When the UART is disabled in the middle of transmission or
+// reception, it completes the current character before stopping
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {}
+}
diff --git a/rust/pl011/src/memory_ops.rs b/rust/pl011/src/memory_ops.rs
new file mode 100644
index 0000000000..4c7d4577c7
--- /dev/null
+++ b/rust/pl011/src/memory_ops.rs
@@ -0,0 +1,38 @@
+use core::{mem::MaybeUninit, ptr::NonNull};
+
+use crate::{device::PL011State, generated::*};
+
+pub const PL011_OPS: MemoryRegionOps = MemoryRegionOps {
+    read: Some(pl011_read),
+    write: Some(pl011_write),
+    read_with_attrs: None,
+    write_with_attrs: None,
+    endianness: device_endian_DEVICE_NATIVE_ENDIAN,
+    valid: unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_1>::zeroed().assume_init() },
+    impl_: MemoryRegionOps__bindgen_ty_2 {
+        min_access_size: 4,
+        max_access_size: 4,
+        ..unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_2>::zeroed().assume_init() }
+    },
+};
+
+unsafe extern "C" fn pl011_read(
+    opaque: *mut core::ffi::c_void,
+    addr: hwaddr,
+    size: core::ffi::c_uint,
+) -> u64 {
+    assert!(!opaque.is_null());
+    let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+    state.as_mut().read(addr, size)
+}
+
+unsafe extern "C" fn pl011_write(
+    opaque: *mut core::ffi::c_void,
+    addr: hwaddr,
+    data: u64,
+    _size: core::ffi::c_uint,
+) {
+    assert!(!opaque.is_null());
+    let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+    state.as_mut().write(addr, data)
+}
diff --git a/rust/rustfmt.toml b/rust/rustfmt.toml
new file mode 100644
index 0000000000..ebecb99fe0
--- /dev/null
+++ b/rust/rustfmt.toml
@@ -0,0 +1,7 @@
+edition = "2021"
+format_generated_files = false
+format_code_in_doc_comments = true
+format_strings = true
+imports_granularity = "Crate"
+group_imports = "StdExternalCrate"
+wrap_comments = true
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [RFC PATCH v3 4/5] DO NOT MERGE: add rustdoc build for gitlab pages
  2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
                   ` (2 preceding siblings ...)
  2024-06-19 20:14 ` [RFC PATCH v3 3/5] rust: add PL011 device model Manos Pitsidianakis
@ 2024-06-19 20:14 ` Manos Pitsidianakis
  2024-06-19 20:14 ` [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
  4 siblings, 0 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Wainer dos Santos Moschetta, Beraldo Leal

Deploy the generated rustdocs for my personal rust qemu fork on gitlab.

The URL is:

https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 .gitlab-ci.d/buildtest.yml | 64 +++++++++++++++++++++++++++-----------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91c57efded..380c24897d 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -715,31 +715,57 @@ build-tools-and-docs-debian:
 # For contributor forks we want to publish from any repo so
 # that users can see the results of their commits, regardless
 # of what topic branch they're currently using
+# pages:
+#   extends: .base_job_template
+#   image: $CI_REGISTRY_IMAGE/qemu/debian:$QEMU_CI_CONTAINER_TAG
+#   stage: test
+#   needs:
+#     - job: build-tools-and-docs-debian
+#   script:
+#     - mkdir -p public
+#     # HTML-ised source tree
+#     - make gtags
+#     # We unset variables to work around a bug in some htags versions
+#     # which causes it to fail when the environment is large
+#     - CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
+#         -anT --tree-view=filetree -m qemu_init
+#         -t "Welcome to the QEMU sourcecode"
+#     - mv HTML public/src
+#     # Project documentation
+#     - make -C build install DESTDIR=$(pwd)/temp-install
+#     - mv temp-install/usr/local/share/doc/qemu/* public/
+#   artifacts:
+#     when: on_success
+#     paths:
+#       - public
+#   variables:
+#     QEMU_JOB_PUBLISH: 1
+# The Docker image that will be used to build your app
 pages:
-  extends: .base_job_template
-  image: $CI_REGISTRY_IMAGE/qemu/debian:$QEMU_CI_CONTAINER_TAG
-  stage: test
-  needs:
-    - job: build-tools-and-docs-debian
+  image: rust:latest
   script:
-    - mkdir -p public
-    # HTML-ised source tree
-    - make gtags
-    # We unset variables to work around a bug in some htags versions
-    # which causes it to fail when the environment is large
-    - CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
-        -anT --tree-view=filetree -m qemu_init
-        -t "Welcome to the QEMU sourcecode"
-    - mv HTML public/src
-    # Project documentation
-    - make -C build install DESTDIR=$(pwd)/temp-install
-    - mv temp-install/usr/local/share/doc/qemu/* public/
+    - rustup component add rustfmt
+    - DEBIAN_FRONTEND=noninteractive apt-get update -y
+    - DEBIAN_FRONTEND=noninteractive apt-get install -y python3-venv meson libgcrypt20-dev zlib1g-dev autoconf automake libtool bison flex git libglib2.0-dev libfdt-dev libpixman-1-dev ninja-build make libclang-14-dev
+    - cargo install bindgen-cli
+    - mkdir ./build/
+    - cd ./build/
+    - ../configure --enable-system --disable-kvm --target-list=aarch64-softmmu --enable-with-rust
+    - ninja "aarch64-softmmu-generated.rs"
+    - cp ./aarch64-softmmu-generated.rs ../rust/pl011/src/generated.rs.inc
+    - cd ../rust/pl011/
+    - cargo tree --depth 1 -e normal --prefix none | cut -d' ' -f1  | xargs
+      printf -- '-p %s\n'  | xargs cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+    - cd ./../..
+    - mv ./rust/pl011/target/x86_64-unknown-linux-gnu/doc ./public
   artifacts:
     when: on_success
     paths:
       - public
-  variables:
-    QEMU_JOB_PUBLISH: 1
+  rules:
+    # This ensures that only pushes to the default branch will trigger
+    # a pages deploy
+    - if: $CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH
 
 coverity:
   image: $CI_REGISTRY_IMAGE/qemu/fedora:$QEMU_CI_CONTAINER_TAG
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
  2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
                   ` (3 preceding siblings ...)
  2024-06-19 20:14 ` [RFC PATCH v3 4/5] DO NOT MERGE: add rustdoc build for gitlab pages Manos Pitsidianakis
@ 2024-06-19 20:14 ` Manos Pitsidianakis
  2024-06-25 16:18   ` Zhao Liu
  4 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-19 20:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	qemu-arm

Convenience patch for testing the rust device.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 hw/arm/virt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3c93c0c0a6..f33b58ae0d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
     int irq = vms->irqmap[uart];
     const char compat[] = "arm,pl011\0arm,primecell";
     const char clocknames[] = "uartclk\0apb_pclk";
+#ifdef CONFIG_WITH_RUST
+    DeviceState *dev = qdev_new("x-pl011-rust");
+#else
     DeviceState *dev = qdev_new(TYPE_PL011);
+#endif
     SysBusDevice *s = SYS_BUS_DEVICE(dev);
     MachineState *ms = MACHINE(vms);
 
-- 
γαῖα πυρί μιχθήτω



^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
@ 2024-06-20 11:10   ` Alex Bennée
  2024-06-20 12:34     ` Paolo Bonzini
  2024-06-20 12:32   ` Alex Bennée
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 44+ messages in thread
From: Alex Bennée @ 2024-06-20 11:10 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

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 ;-)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
  2024-06-20 11:10   ` Alex Bennée
@ 2024-06-20 12:32   ` Alex Bennée
  2024-06-20 18:22     ` Manos Pitsidianakis
  2024-06-20 14:01   ` Richard Henderson
  2024-06-24 10:12   ` Zhao Liu
  3 siblings, 1 reply; 44+ messages in thread
From: Alex Bennée @ 2024-06-20 12:32 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

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()",

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 11:10   ` Alex Bennée
@ 2024-06-20 12:34     ` Paolo Bonzini
  2024-06-20 18:18       ` Manos Pitsidianakis
  0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-20 12:34 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Daniel P. Berrangé, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
	Richard Henderson, John Snow, Cleber Rosa

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



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
@ 2024-06-20 13:21   ` Paolo Bonzini
  2024-06-20 18:06     ` Manos Pitsidianakis
  2024-06-24  8:51     ` Zhao Liu
  2024-06-20 13:41   ` Alex Bennée
  2024-06-24 16:52   ` Daniel P. Berrangé
  2 siblings, 2 replies; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-20 13:21 UTC (permalink / raw)
  To: Manos Pitsidianakis, qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	John Snow, Cleber Rosa

On 6/19/24 22:13, Manos Pitsidianakis wrote:
> Add options for Rust in meson_options.txt, meson.build, configure to
> prepare for adding Rust code in the followup commits.
> 
> `rust` is a reserved meson name, so we have to use an alternative.
> `with_rust` was chosen.
> 
> A cargo_wrapper.py script is added that is heavily based on the work of
> Marc-André Lureau from 2021.
> 
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

The cargo_wrapper.py script is not used yet, so it should be
delayed until it's used.

For the detection of the toolchain, I'd rather do everything in
configure since that's where the cross file is built.  Something like:

diff --git a/configure b/configure
index 8b6a2f16ceb..6412a1021c3 100755
--- a/configure
+++ b/configure
@@ -173,6 +173,8 @@ fi
  
  # default parameters
  container_engine="auto"
+rust_target_triple=""
+with_rust="no"
  cpu=""
  cross_compile="no"
  cross_prefix=""
@@ -201,6 +202,8 @@ for opt do
    --cross-prefix=*) cross_prefix="$optarg"
                      cross_compile="yes"
    ;;
+  --cargo=*) CARGO="$optarg"
+  ;;
    --cc=*) CC="$optarg"
    ;;
    --cxx=*) CXX="$optarg"
@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
  pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
  sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
  
+cargo="${CARGO-cargo}"
+
  check_define() {
  cat > $TMPC <<EOF
  #if !defined($1)
@@ -628,6 +635,8 @@ for opt do
    ;;
    --cross-prefix=*)
    ;;
+  --cargo=*)
+  ;;
    --cc=*)
    ;;
    --host-cc=*) host_cc="$optarg"
@@ -755,8 +764,14 @@ for opt do
    ;;
    --container-engine=*) container_engine="$optarg"
    ;;
+  --rust-target-triple=*) rust_target_triple="$optarg"
+  ;;
    --gdb=*) gdb_bin="$optarg"
    ;;
+  --with-rust) with_rust=yes
+  ;;
+  --without-rust) with_rust=no
+  ;;
    # everything else has the same name in configure and meson
    --*) meson_option_parse "$opt" "$optarg"
    ;;
@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
  Advanced options (experts only):
    -Dmesonoptname=val       passthrough option to meson unmodified
    --cross-prefix=PREFIX    use PREFIX for compile tools, PREFIX can be blank [$cross_prefix]
+  --cargo=CARGO            use Cargo binary CARGO [$cargo]
    --cc=CC                  use C compiler CC [$cc]
    --host-cc=CC             when cross compiling, use C compiler CC for code run
                             at build time [$host_cc]
@@ -869,11 +885,13 @@ Advanced options (experts only):
    --python=PYTHON          use specified python [$python]
    --ninja=NINJA            use specified ninja [$ninja]
    --static                 enable static build [$static]
-  --without-default-features default all --enable-* options to "disabled"
-  --without-default-devices  do not include any device that is not needed to
+  --rust-target-triple=TRIPLE  target for Rust cross compilation
+  --without-default-features   default all --enable-* options to "disabled"
+  --without-default-devices    do not include any device that is not needed to
                             start the emulator (only use if you are including
                             desired devices in configs/devices/)
    --with-devices-ARCH=NAME override default configs/devices
+  --with-rust              enable experimental Rust code
    --enable-debug           enable common debug build options
    --cpu=CPU                Build for host CPU [$cpu]
    --disable-containers     don't use containers for cross-building
@@ -1138,6 +1159,20 @@ EOF
    fi
  fi
  
+##########################################
+# detect rust triples
+
+if test "$with_rust" = yes; then
+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
+  if test $? != 0; then
+    error_exit "could not execute cargo binary \"$CARGO\""
+  fi
+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
+  if test "$rust_target_triple" = ""; then
+    rust_target_triple=$rust_host_triple
+  fi
+fi
+
  ##########################################
  # functions to probe cross compilers
  
@@ -1604,6 +1639,10 @@ if test "$container" != no; then
      echo "RUNC=$runc" >> $config_host_mak
  fi
  echo "SUBDIRS=$subdirs" >> $config_host_mak
+if test "$with_rust" = yes; then
+  echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
+  echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
+fi
  echo "PYTHON=$python" >> $config_host_mak
  echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >> $config_host_mak
  echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
    echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
    test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
    test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> $cross
+  if test "$with_rust" = yes; then
+    if test "$rust_host_triple" != "$rust_target_triple"; then
+      echo "cargo = [$(meson_quote $cargo --target "$rust_target_triple")]" >> $cross
+    else
+      echo "cargo = [$(meson_quote $cargo)]" >> $cross
+    fi
+  fi
    echo "ar = [$(meson_quote $ar)]" >> $cross
    echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
    echo "nm = [$(meson_quote $nm)]" >> $cross
diff --git a/meson.build b/meson.build
index c5360fbd299..ad7dbc0d641 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,11 @@ foreach lang : all_languages
    endif
  endforeach
  
+cargo = not_found
+if 'RUST_TARGET_TRIPLE' in config_host
+  cargo = find_program('cargo', required: true)
+endif
+
  # default flags for all hosts
  # We use -fwrapv to tell the compiler that we require a C dialect where
  # left shift of signed integers is well defined and has the expected
@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
  else
    summary_info += {'Objective-C compiler': false}
  endif
+summary_info += {'Rust support':      cargo.found()}
+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != config_host['RUST_HOST_TRIPLE']
+  summary_info += {'Rust target':     config_host['RUST_TARGET_TRIPLE']}
+endif
  option_cflags = (get_option('debug') ? ['-g'] : [])
  if get_option('optimization') != 'plain'
    option_cflags += ['-O' + get_option('optimization')]




^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
  2024-06-20 13:21   ` Paolo Bonzini
@ 2024-06-20 13:41   ` Alex Bennée
  2024-06-20 18:14     ` Manos Pitsidianakis
  2024-06-24 16:52   ` Daniel P. Berrangé
  2 siblings, 1 reply; 44+ messages in thread
From: Alex Bennée @ 2024-06-20 13:41 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

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

> Add options for Rust in meson_options.txt, meson.build, configure to
> prepare for adding Rust code in the followup commits.
>
> `rust` is a reserved meson name, so we have to use an alternative.
> `with_rust` was chosen.
>
> A cargo_wrapper.py script is added that is heavily based on the work of
> Marc-André Lureau from 2021.
>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
<snip>
>  
> +with_rust="auto"
> +with_rust_target_triple=""
> +
>  ar="${AR-${cross_prefix}ar}"
>  as="${AS-${cross_prefix}as}"
>  ccas="${CCAS-$cc}"
> @@ -760,6 +763,12 @@ for opt do
>    ;;
>    --gdb=*) gdb_bin="$optarg"
>    ;;
> +  --enable-with-rust) with_rust=enabled
> +  ;;
> +  --disable-with-rust) with_rust=disabled
> +  ;;
> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
> +  ;;
>    # everything else has the same name in configure and meson
>    --*) meson_option_parse "$opt" "$optarg"
>    ;;
> @@ -1796,6 +1805,8 @@ if test "$skip_meson" = no; then
>    test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>    test "$plugins" = yes && meson_option_add "-Dplugins=true"
>    test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
> +  test "$with_rust" != enabled && meson_option_add "-Dwith_rust=$with_rust"
> +  test "$with_rust_target_triple" != "" && meson_option_add "-Dwith_rust_target_triple=$with_rust_target_triple"
>    run_meson() {
>      NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
>    }
<snip>

> +summary_info += {'Rust support':      with_rust}
> +if with_rust and get_option('with_rust_target_triple') != ''
> +  summary_info += {'Rust target':     get_option('with_rust_target_triple')}
> +endif
<snip>

I wonder if we should display the auto-probed triple here as well, not
just when its been overridden?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
  2024-06-20 11:10   ` Alex Bennée
  2024-06-20 12:32   ` Alex Bennée
@ 2024-06-20 14:01   ` Richard Henderson
  2024-06-20 18:36     ` Manos Pitsidianakis
  2024-06-24 10:12   ` Zhao Liu
  3 siblings, 1 reply; 44+ messages in thread
From: Richard Henderson @ 2024-06-20 14:01 UTC (permalink / raw)
  To: Manos Pitsidianakis, qemu-devel

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~


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-20 13:21   ` Paolo Bonzini
@ 2024-06-20 18:06     ` Manos Pitsidianakis
  2024-06-20 19:44       ` Paolo Bonzini
  2024-06-24  8:51     ` Zhao Liu
  1 sibling, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-20 18:06 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Benné e,
	Daniel P. Berrangé , Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé , Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	John Snow, Cleber Rosa

On Thu, 20 Jun 2024 16:21, Paolo Bonzini <pbonzini@redhat.com> wrote:
>On 6/19/24 22:13, Manos Pitsidianakis wrote:
>> Add options for Rust in meson_options.txt, meson.build, configure to
>> prepare for adding Rust code in the followup commits.
>> 
>> `rust` is a reserved meson name, so we have to use an alternative.
>> `with_rust` was chosen.
>> 
>> A cargo_wrapper.py script is added that is heavily based on the work of
>> Marc-André Lureau from 2021.
>> 
>> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
>> 
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>
>The cargo_wrapper.py script is not used yet, so it should be
>delayed until it's used.

That's true, I just wanted to make review easier by splitting it out. 
Can we squash them later or do you think I should I do it for the next 
series version?



>
>For the detection of the toolchain, I'd rather do everything in
>configure since that's where the cross file is built.  Something like:
>
>diff --git a/configure b/configure
>index 8b6a2f16ceb..6412a1021c3 100755
>--- a/configure
>+++ b/configure
>@@ -173,6 +173,8 @@ fi
>  
>  # default parameters
>  container_engine="auto"
>+rust_target_triple=""
>+with_rust="no"
>  cpu=""
>  cross_compile="no"
>  cross_prefix=""
>@@ -201,6 +202,8 @@ for opt do
>    --cross-prefix=*) cross_prefix="$optarg"
>                      cross_compile="yes"
>    ;;
>+  --cargo=*) CARGO="$optarg"
>+  ;;
>    --cc=*) CC="$optarg"
>    ;;
>    --cxx=*) CXX="$optarg"
>@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
>  pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
>  sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
>  
>+cargo="${CARGO-cargo}"
>+
>  check_define() {
>  cat > $TMPC <<EOF
>  #if !defined($1)
>@@ -628,6 +635,8 @@ for opt do
>    ;;
>    --cross-prefix=*)
>    ;;
>+  --cargo=*)
>+  ;;
>    --cc=*)
>    ;;
>    --host-cc=*) host_cc="$optarg"
>@@ -755,8 +764,14 @@ for opt do
>    ;;
>    --container-engine=*) container_engine="$optarg"
>    ;;
>+  --rust-target-triple=*) rust_target_triple="$optarg"
>+  ;;
>    --gdb=*) gdb_bin="$optarg"
>    ;;
>+  --with-rust) with_rust=yes
>+  ;;
>+  --without-rust) with_rust=no
>+  ;;
>    # everything else has the same name in configure and meson
>    --*) meson_option_parse "$opt" "$optarg"
>    ;;
>@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
>  Advanced options (experts only):
>    -Dmesonoptname=val       passthrough option to meson unmodified
>    --cross-prefix=PREFIX    use PREFIX for compile tools, PREFIX can be blank [$cross_prefix]
>+  --cargo=CARGO            use Cargo binary CARGO [$cargo]
>    --cc=CC                  use C compiler CC [$cc]
>    --host-cc=CC             when cross compiling, use C compiler CC for code run
>                             at build time [$host_cc]
>@@ -869,11 +885,13 @@ Advanced options (experts only):
>    --python=PYTHON          use specified python [$python]
>    --ninja=NINJA            use specified ninja [$ninja]
>    --static                 enable static build [$static]
>-  --without-default-features default all --enable-* options to "disabled"
>-  --without-default-devices  do not include any device that is not needed to
>+  --rust-target-triple=TRIPLE  target for Rust cross compilation
>+  --without-default-features   default all --enable-* options to "disabled"
>+  --without-default-devices    do not include any device that is not needed to
>                             start the emulator (only use if you are including
>                             desired devices in configs/devices/)
>    --with-devices-ARCH=NAME override default configs/devices
>+  --with-rust              enable experimental Rust code
>    --enable-debug           enable common debug build options
>    --cpu=CPU                Build for host CPU [$cpu]
>    --disable-containers     don't use containers for cross-building
>@@ -1138,6 +1159,20 @@ EOF
>    fi
>  fi
>  
>+##########################################
>+# detect rust triples
>+
>+if test "$with_rust" = yes; then
>+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
>+  if test $? != 0; then
>+    error_exit "could not execute cargo binary \"$CARGO\""
>+  fi
>+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
>+  if test "$rust_target_triple" = ""; then
>+    rust_target_triple=$rust_host_triple
>+  fi
>+fi
>+
>  ##########################################
>  # functions to probe cross compilers
>  
>@@ -1604,6 +1639,10 @@ if test "$container" != no; then
>      echo "RUNC=$runc" >> $config_host_mak
>  fi
>  echo "SUBDIRS=$subdirs" >> $config_host_mak
>+if test "$with_rust" = yes; then
>+  echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
>+  echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
>+fi
>  echo "PYTHON=$python" >> $config_host_mak
>  echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >> $config_host_mak
>  echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
>@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
>    echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
>    test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
>    test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> $cross
>+  if test "$with_rust" = yes; then
>+    if test "$rust_host_triple" != "$rust_target_triple"; then
>+      echo "cargo = [$(meson_quote $cargo --target "$rust_target_triple")]" >> $cross
>+    else
>+      echo "cargo = [$(meson_quote $cargo)]" >> $cross
>+    fi
>+  fi


Hm that looks better indeed, thanks!


>    echo "ar = [$(meson_quote $ar)]" >> $cross
>    echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
>    echo "nm = [$(meson_quote $nm)]" >> $cross
>diff --git a/meson.build b/meson.build
>index c5360fbd299..ad7dbc0d641 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -290,6 +290,11 @@ foreach lang : all_languages
>    endif
>  endforeach
>  
>+cargo = not_found
>+if 'RUST_TARGET_TRIPLE' in config_host
>+  cargo = find_program('cargo', required: true)
>+endif
>+
>  # default flags for all hosts
>  # We use -fwrapv to tell the compiler that we require a C dialect where
>  # left shift of signed integers is well defined and has the expected
>@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
>  else
>    summary_info += {'Objective-C compiler': false}
>  endif
>+summary_info += {'Rust support':      cargo.found()}
>+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != config_host['RUST_HOST_TRIPLE']
>+  summary_info += {'Rust target':     config_host['RUST_TARGET_TRIPLE']}
>+endif
>  option_cflags = (get_option('debug') ? ['-g'] : [])
>  if get_option('optimization') != 'plain'
>    option_cflags += ['-O' + get_option('optimization')]
>
>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-20 13:41   ` Alex Bennée
@ 2024-06-20 18:14     ` Manos Pitsidianakis
  0 siblings, 0 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-20 18:14 UTC (permalink / raw)
  To: Alex Benné e; +Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell

On Thu, 20 Jun 2024 16:41, Alex Bennée <alex.bennee@linaro.org> wrote:
>> +summary_info += {'Rust support':      with_rust}
>> +if with_rust and get_option('with_rust_target_triple') != ''
>> +  summary_info += {'Rust target':     get_option('with_rust_target_triple')}
>> +endif
><snip>
>
>I wonder if we should display the auto-probed triple here as well, not
>just when its been overridden?

I agree, once we straighten out host target / cross target detection 
logic the target summary info print should be unconditional.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 12:34     ` Paolo Bonzini
@ 2024-06-20 18:18       ` Manos Pitsidianakis
  0 siblings, 0 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-20 18:18 UTC (permalink / raw)
  To: Paolo Bonzini, Alex Benné e
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Daniel P. Berrangé , Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé ,
	Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
	Richard Henderson, John Snow, Cleber Rosa

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 12:32   ` Alex Bennée
@ 2024-06-20 18:22     ` Manos Pitsidianakis
  2024-06-24 19:52       ` Stefan Hajnoczi
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-20 18:22 UTC (permalink / raw)
  To: Alex Benné e; +Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell

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.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 14:01   ` Richard Henderson
@ 2024-06-20 18:36     ` Manos Pitsidianakis
  2024-06-20 19:30       ` Richard Henderson
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-20 18:36 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

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.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 18:36     ` Manos Pitsidianakis
@ 2024-06-20 19:30       ` Richard Henderson
  0 siblings, 0 replies; 44+ messages in thread
From: Richard Henderson @ 2024-06-20 19:30 UTC (permalink / raw)
  To: Manos Pitsidianakis, qemu-devel

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~


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-20 18:06     ` Manos Pitsidianakis
@ 2024-06-20 19:44       ` Paolo Bonzini
  0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-20 19:44 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Daniel P. Berrangé,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé, Zhao Liu, Gustavo Romero,
	Pierrick Bouvier, rowan.hart, Richard Henderson, John Snow,
	Cleber Rosa

[-- Attachment #1: Type: text/plain, Size: 7804 bytes --]

Il gio 20 giu 2024, 20:13 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:

> On Thu, 20 Jun 2024 16:21, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >On 6/19/24 22:13, Manos Pitsidianakis wrote:
> >> Add options for Rust in meson_options.txt, meson.build, configure to
> >> prepare for adding Rust code in the followup commits.
> >>
> >> `rust` is a reserved meson name, so we have to use an alternative.
> >> `with_rust` was chosen.
> >>
> >> A cargo_wrapper.py script is added that is heavily based on the work of
> >> Marc-André Lureau from 2021.
> >>
> >>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> >
> >The cargo_wrapper.py script is not used yet, so it should be
> >delayed until it's used.
>
> That's true, I just wanted to make review easier by splitting it out.
> Can we squash them later or do you think I should I do it for the next
> series version?
>

I guess it depends on what the next version looks like. If you start
working on the workspace/build tree/Kconfig parts, it might not have a lot
of cargo_wrapper.py code surviving.

Feel free to take this patch and add

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

>For the detection of the toolchain, I'd rather do everything in
> >configure since that's where the cross file is built.  Something like:
> >
> >diff --git a/configure b/configure
> >index 8b6a2f16ceb..6412a1021c3 100755
> >--- a/configure
> >+++ b/configure
> >@@ -173,6 +173,8 @@ fi
> >
> >  # default parameters
> >  container_engine="auto"
> >+rust_target_triple=""
> >+with_rust="no"
> >  cpu=""
> >  cross_compile="no"
> >  cross_prefix=""
> >@@ -201,6 +202,8 @@ for opt do
> >    --cross-prefix=*) cross_prefix="$optarg"
> >                      cross_compile="yes"
> >    ;;
> >+  --cargo=*) CARGO="$optarg"
> >+  ;;
> >    --cc=*) CC="$optarg"
> >    ;;
> >    --cxx=*) CXX="$optarg"
> >@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
> >  pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
> >  sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
> >
> >+cargo="${CARGO-cargo}"
> >+
> >  check_define() {
> >  cat > $TMPC <<EOF
> >  #if !defined($1)
> >@@ -628,6 +635,8 @@ for opt do
> >    ;;
> >    --cross-prefix=*)
> >    ;;
> >+  --cargo=*)
> >+  ;;
> >    --cc=*)
> >    ;;
> >    --host-cc=*) host_cc="$optarg"
> >@@ -755,8 +764,14 @@ for opt do
> >    ;;
> >    --container-engine=*) container_engine="$optarg"
> >    ;;
> >+  --rust-target-triple=*) rust_target_triple="$optarg"
> >+  ;;
> >    --gdb=*) gdb_bin="$optarg"
> >    ;;
> >+  --with-rust) with_rust=yes
> >+  ;;
> >+  --without-rust) with_rust=no
> >+  ;;
> >    # everything else has the same name in configure and meson
> >    --*) meson_option_parse "$opt" "$optarg"
> >    ;;
> >@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
> >  Advanced options (experts only):
> >    -Dmesonoptname=val       passthrough option to meson unmodified
> >    --cross-prefix=PREFIX    use PREFIX for compile tools, PREFIX can be
> blank [$cross_prefix]
> >+  --cargo=CARGO            use Cargo binary CARGO [$cargo]
> >    --cc=CC                  use C compiler CC [$cc]
> >    --host-cc=CC             when cross compiling, use C compiler CC for
> code run
> >                             at build time [$host_cc]
> >@@ -869,11 +885,13 @@ Advanced options (experts only):
> >    --python=PYTHON          use specified python [$python]
> >    --ninja=NINJA            use specified ninja [$ninja]
> >    --static                 enable static build [$static]
> >-  --without-default-features default all --enable-* options to "disabled"
> >-  --without-default-devices  do not include any device that is not
> needed to
> >+  --rust-target-triple=TRIPLE  target for Rust cross compilation
> >+  --without-default-features   default all --enable-* options to
> "disabled"
> >+  --without-default-devices    do not include any device that is not
> needed to
> >                             start the emulator (only use if you are
> including
> >                             desired devices in configs/devices/)
> >    --with-devices-ARCH=NAME override default configs/devices
> >+  --with-rust              enable experimental Rust code
> >    --enable-debug           enable common debug build options
> >    --cpu=CPU                Build for host CPU [$cpu]
> >    --disable-containers     don't use containers for cross-building
> >@@ -1138,6 +1159,20 @@ EOF
> >    fi
> >  fi
> >
> >+##########################################
> >+# detect rust triples
> >+
> >+if test "$with_rust" = yes; then
> >+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
> >+  if test $? != 0; then
> >+    error_exit "could not execute cargo binary \"$CARGO\""
> >+  fi
> >+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
> >+  if test "$rust_target_triple" = ""; then
> >+    rust_target_triple=$rust_host_triple
> >+  fi
> >+fi
> >+
> >  ##########################################
> >  # functions to probe cross compilers
> >
> >@@ -1604,6 +1639,10 @@ if test "$container" != no; then
> >      echo "RUNC=$runc" >> $config_host_mak
> >  fi
> >  echo "SUBDIRS=$subdirs" >> $config_host_mak
> >+if test "$with_rust" = yes; then
> >+  echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
> >+  echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
> >+fi
> >  echo "PYTHON=$python" >> $config_host_mak
> >  echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >>
> $config_host_mak
> >  echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
> >@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
> >    echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
> >    test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >>
> $cross
> >    test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]"
> >> $cross
> >+  if test "$with_rust" = yes; then
> >+    if test "$rust_host_triple" != "$rust_target_triple"; then
> >+      echo "cargo = [$(meson_quote $cargo --target
> "$rust_target_triple")]" >> $cross
> >+    else
> >+      echo "cargo = [$(meson_quote $cargo)]" >> $cross
> >+    fi
> >+  fi
>
>
> Hm that looks better indeed, thanks!
>
>
> >    echo "ar = [$(meson_quote $ar)]" >> $cross
> >    echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
> >    echo "nm = [$(meson_quote $nm)]" >> $cross
> >diff --git a/meson.build b/meson.build
> >index c5360fbd299..ad7dbc0d641 100644
> >--- a/meson.build
> >+++ b/meson.build
> >@@ -290,6 +290,11 @@ foreach lang : all_languages
> >    endif
> >  endforeach
> >
> >+cargo = not_found
> >+if 'RUST_TARGET_TRIPLE' in config_host
> >+  cargo = find_program('cargo', required: true)
> >+endif
> >+
> >  # default flags for all hosts
> >  # We use -fwrapv to tell the compiler that we require a C dialect where
> >  # left shift of signed integers is well defined and has the expected
> >@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
> >  else
> >    summary_info += {'Objective-C compiler': false}
> >  endif
> >+summary_info += {'Rust support':      cargo.found()}
> >+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) !=
> config_host['RUST_HOST_TRIPLE']
> >+  summary_info += {'Rust target':     config_host['RUST_TARGET_TRIPLE']}
> >+endif
> >  option_cflags = (get_option('debug') ? ['-g'] : [])
> >  if get_option('optimization') != 'plain'
> >    option_cflags += ['-O' + get_option('optimization')]
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 10956 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-20 13:21   ` Paolo Bonzini
  2024-06-20 18:06     ` Manos Pitsidianakis
@ 2024-06-24  8:51     ` Zhao Liu
  2024-06-24 16:35       ` Paolo Bonzini
  1 sibling, 1 reply; 44+ messages in thread
From: Zhao Liu @ 2024-06-24  8:51 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Bennée, Daniel P. Berrangé,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, John Snow, Cleber Rosa

[snip]

> diff --git a/meson.build b/meson.build
> index c5360fbd299..ad7dbc0d641 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -290,6 +290,11 @@ foreach lang : all_languages
>    endif
>  endforeach
> +cargo = not_found
> +if 'RUST_TARGET_TRIPLE' in config_host
> +  cargo = find_program('cargo', required: true)
> +endif
> +

As with the original Manos version, it looks like there's no need to
check cargo here? Since patch 2 checks cargo and others in rust/meson.build.

Otherwise, cargo was checked twice.

>  # default flags for all hosts
>  # We use -fwrapv to tell the compiler that we require a C dialect where
>  # left shift of signed integers is well defined and has the expected
> @@ -4239,6 +4244,10 @@ if 'objc' in all_languages
>  else
>    summary_info += {'Objective-C compiler': false}
>  endif
> +summary_info += {'Rust support':      cargo.found()}
> +if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != config_host['RUST_HOST_TRIPLE']
> +  summary_info += {'Rust target':     config_host['RUST_TARGET_TRIPLE']}
> +endif
>  option_cflags = (get_option('debug') ? ['-g'] : [])
>  if get_option('optimization') != 'plain'
>    option_cflags += ['-O' + get_option('optimization')]
> 
> 


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-24 10:12   ` Zhao Liu
@ 2024-06-24 10:02     ` Manos Pitsidianakis
  2024-06-25 16:00       ` Zhao Liu
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-24 10:02 UTC (permalink / raw)
  To: Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Daniel P. Berrangé ,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé , Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, Paolo Bonzini, John Snow,
	Cleber Rosa

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
>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
                     ` (2 preceding siblings ...)
  2024-06-20 14:01   ` Richard Henderson
@ 2024-06-24 10:12   ` Zhao Liu
  2024-06-24 10:02     ` Manos Pitsidianakis
  3 siblings, 1 reply; 44+ messages in thread
From: Zhao Liu @ 2024-06-24 10:12 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Bennée, Daniel P. Berrangé, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

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



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-24  8:51     ` Zhao Liu
@ 2024-06-24 16:35       ` Paolo Bonzini
  0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-24 16:35 UTC (permalink / raw)
  To: Zhao Liu
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Bennée, Daniel P. Berrangé,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, John Snow, Cleber Rosa

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

Il lun 24 giu 2024, 10:36 Zhao Liu <zhao1.liu@intel.com> ha scritto:

> [snip]
>
> > diff --git a/meson.build b/meson.build
> > index c5360fbd299..ad7dbc0d641 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -290,6 +290,11 @@ foreach lang : all_languages
> >    endif
> >  endforeach
> > +cargo = not_found
> > +if 'RUST_TARGET_TRIPLE' in config_host
> > +  cargo = find_program('cargo', required: true)
> > +endif
> > +
>
> As with the original Manos version, it looks like there's no need to
> check cargo here? Since patch 2 checks cargo and others in
> rust/meson.build.
>
> Otherwise, cargo was checked twice.
>

Yes, I would check it here though because it's used in the summary, already
in this patch.

Paolo


> >  # default flags for all hosts
> >  # We use -fwrapv to tell the compiler that we require a C dialect where
> >  # left shift of signed integers is well defined and has the expected
> > @@ -4239,6 +4244,10 @@ if 'objc' in all_languages
> >  else
> >    summary_info += {'Objective-C compiler': false}
> >  endif
> > +summary_info += {'Rust support':      cargo.found()}
> > +if cargo.found() and config_host['RUST_TARGET_TRIPLE']) !=
> config_host['RUST_HOST_TRIPLE']
> > +  summary_info += {'Rust target':     config_host['RUST_TARGET_TRIPLE']}
> > +endif
> >  option_cflags = (get_option('debug') ? ['-g'] : [])
> >  if get_option('optimization') != 'plain'
> >    option_cflags += ['-O' + get_option('optimization')]
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 2409 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
  2024-06-20 13:21   ` Paolo Bonzini
  2024-06-20 13:41   ` Alex Bennée
@ 2024-06-24 16:52   ` Daniel P. Berrangé
  2024-06-24 17:14     ` Paolo Bonzini
  2024-07-02 14:38     ` Manos Pitsidianakis
  2 siblings, 2 replies; 44+ messages in thread
From: Daniel P. Berrangé @ 2024-06-24 16:52 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Bennée, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
> Add options for Rust in meson_options.txt, meson.build, configure to
> prepare for adding Rust code in the followup commits.
> 
> `rust` is a reserved meson name, so we have to use an alternative.
> `with_rust` was chosen.
> 
> A cargo_wrapper.py script is added that is heavily based on the work of
> Marc-André Lureau from 2021.
> 
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  MAINTAINERS                   |   5 +
>  configure                     |  11 ++
>  meson.build                   |  11 ++
>  meson_options.txt             |   4 +
>  scripts/cargo_wrapper.py      | 279 ++++++++++++++++++++++++++++++++++
>  scripts/meson-buildoptions.sh |   6 +
>  6 files changed, 316 insertions(+)
>  create mode 100644 scripts/cargo_wrapper.py

> diff --git a/configure b/configure
> index 38ee257701..6894d7c2d1 100755
> --- a/configure
> +++ b/configure
> @@ -302,6 +302,9 @@ else
>    objcc="${objcc-${cross_prefix}clang}"
>  fi
>  
> +with_rust="auto"

On last week's call one of the things we discussed is the expectations
for consumers of QEMU around the usage of Rust & its optionality (or
not) long term.

If we consider Rust to be an optional feature, then this largely
precludes re-writing existing C code in Rust, as we would be
forced to either remove existing features from users, or maintain
parallel impls in both C & Rust. Neither of these are desirable
situations.

I'm of the view that to be valuable for QEMU, we need to consider
Rust to become a mandatory feature. There's a question of how quickly
we move, however, before declaring it mandatory. The longer we take
to declare it mandatory, the longer we are limiting the value we
can take from Rust.

If we want to make Rust mandatory, then we should set the user
expectations to reflect this from the start.


IOW, with_rust="enabled" should be the default, and require an
explicit  --disable-rust to opt-out on a very time limited
basis.  Any use of --disable-rust ought to print a warning at
the end of configure, and solicit feedback:

    WARNING: Building without Rust is deprecated. QEMU intends
    WARNING: to make Rust a mandatory build dependency in the
    WARNING: 10.0.0 release.
    WARNING:
    WARNING: If you have concerns about this requirement
    WARNING: please contact qemu-devel@nongnu.org

I illustrated '10.0.0' on the assumption that we add Rust
support in 9.1.0, and thus have 2 releases where it is
optional to align with our deprecation policy. Even though
we don't usually apply our deprecation policy to build
dependencies, this is a significant unique situation so
IMHO worth doing.


> +with_rust_target_triple=""
> +
>  ar="${AR-${cross_prefix}ar}"
>  as="${AS-${cross_prefix}as}"
>  ccas="${CCAS-$cc}"
> @@ -760,6 +763,12 @@ for opt do
>    ;;
>    --gdb=*) gdb_bin="$optarg"
>    ;;
> +  --enable-with-rust) with_rust=enabled
> +  ;;
> +  --disable-with-rust) with_rust=disabled
> +  ;;

--enable-with-XXX / --disable-with-XXX is pretty unsual naming.

Normally you'd see either --enable-XXX or --with-XXX and their
corresponding --disable-XXX or --without-XXX.




> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"

As with previous posting, IMHO, this should ideally not exist. We should
honour the --cross-prefix= values, re-mapping to to Rust targets for all
the combos we know about. --with-rust-target-triple should only be needed
as a workaround for where we might have missed a mapping.


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 :|



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-24 16:52   ` Daniel P. Berrangé
@ 2024-06-24 17:14     ` Paolo Bonzini
  2024-06-25 21:47       ` Manos Pitsidianakis
  2024-07-02 14:38     ` Manos Pitsidianakis
  1 sibling, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-24 17:14 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Bennée, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
	Richard Henderson, John Snow, Cleber Rosa

[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]

Il lun 24 giu 2024, 18:52 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:

> On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
> > Add options for Rust in meson_options.txt, meson.build, configure to
> > prepare for adding Rust code in the followup commits.
> >
> > `rust` is a reserved meson name, so we have to use an alternative.
> > `with_rust` was chosen.
> >
> > A cargo_wrapper.py script is added that is heavily based on the work of
> > Marc-André Lureau from 2021.
> >
> >
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> >  MAINTAINERS                   |   5 +
> >  configure                     |  11 ++
> >  meson.build                   |  11 ++
> >  meson_options.txt             |   4 +
> >  scripts/cargo_wrapper.py      | 279 ++++++++++++++++++++++++++++++++++
> >  scripts/meson-buildoptions.sh |   6 +
> >  6 files changed, 316 insertions(+)
> >  create mode 100644 scripts/cargo_wrapper.py
>
> > diff --git a/configure b/configure
> > index 38ee257701..6894d7c2d1 100755
> > --- a/configure
> > +++ b/configure
> > @@ -302,6 +302,9 @@ else
> >    objcc="${objcc-${cross_prefix}clang}"
> >  fi
> >
> > +with_rust="auto"
>
> On last week's call one of the things we discussed is the expectations
> for consumers of QEMU around the usage of Rust & its optionality (or
> not) long term.
>
> I'm of the view that to be valuable for QEMU, we need to consider
> Rust to become a mandatory feature. There's a question of how quickly
> we move, however, before declaring it mandatory. The longer we take
> to declare it mandatory, the longer we are limiting the value we
> can take from Rust.
>

Yes, I agree. However, considering we haven't even checked the situation
with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.

As soon as the PL011 device is a reasonable example of how to write good
Rust device models, however, we should switch to making it default-on, and
--without-rust can go away within one or two releases.

> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
>
As with previous posting, IMHO, this should ideally not exist. We should
> honour the --cross-prefix= values, re-mapping to to Rust targets for all
> the combos we know about


More precisely it should be based not on the cross prefix, but on the same
OS and CPU values that are detected from cpp symbols and used for the meson
cross file. I have already made a rough list of differences between these
and the Rust target triples, but I haven't yet turned them to code.

It would however be the first patch I send after these are in a good shape
for inclusion.

The priority right now should be to investigate the build tree workspace
design that I sketched last week in my reply to Richard. Once that can be
considered a stable build system integration, further improvements can be
done in tree.

--with-rust-target-triple should only be needed
> as a workaround for where we might have missed a mapping.
>

Agreed.

Paolo

[-- Attachment #2: Type: text/html, Size: 5233 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-20 18:22     ` Manos Pitsidianakis
@ 2024-06-24 19:52       ` Stefan Hajnoczi
  0 siblings, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2024-06-24 19:52 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: Alex Benné e, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-24 10:02     ` Manos Pitsidianakis
@ 2024-06-25 16:00       ` Zhao Liu
  2024-06-25 18:08         ` Manos Pitsidianakis
  0 siblings, 1 reply; 44+ messages in thread
From: Zhao Liu @ 2024-06-25 16:00 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Daniel P. Berrangé,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, Paolo Bonzini, John Snow,
	Cleber Rosa

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



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
  2024-06-19 20:14 ` [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
@ 2024-06-25 16:18   ` Zhao Liu
  2024-06-25 18:23     ` Manos Pitsidianakis
  2024-06-25 19:15     ` Daniel P. Berrangé
  0 siblings, 2 replies; 44+ messages in thread
From: Zhao Liu @ 2024-06-25 16:18 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Bennée, Daniel P. Berrangé, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	qemu-arm

Hi Manos,

On Wed, Jun 19, 2024 at 11:14:02PM +0300, Manos Pitsidianakis wrote:
> Date: Wed, 19 Jun 2024 23:14:02 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with
>  x-pl011-rust in arm virt machine
> X-Mailer: git-send-email 2.44.0
> 
> Convenience patch for testing the rust device.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  hw/arm/virt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3c93c0c0a6..f33b58ae0d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
>      int irq = vms->irqmap[uart];
>      const char compat[] = "arm,pl011\0arm,primecell";
>      const char clocknames[] = "uartclk\0apb_pclk";
> +#ifdef CONFIG_WITH_RUST
> +    DeviceState *dev = qdev_new("x-pl011-rust");
> +#else
>      DeviceState *dev = qdev_new(TYPE_PL011);
> +#endif
>      SysBusDevice *s = SYS_BUS_DEVICE(dev);
>      MachineState *ms = MACHINE(vms);
>

I realized that if we want to merge the rust pl011 device, then this
patch or similar enablement support is necessary, otherwise, the rust
code is only used for compilation and cannot actually be run...

This is also an open for the devices that are rewrite in Rust.

I think there should be an option for the user to choose whether to
enable pl011 in C or pl011 in Rust. What do you think?

Perhaps the easiest way to enable rust pl011 is to add an option for
virt machine... But that's certainly not a long-term approach. I think
the ideal way would be to allow rust pl011 to be specified in the
command line via -device, but this approach would mean allowing the
user to create pl011 and would require changes to the current buildin
pl011's creation logic.

-Zhao




^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-25 16:00       ` Zhao Liu
@ 2024-06-25 18:08         ` Manos Pitsidianakis
  2024-06-27 23:47           ` Pierrick Bouvier
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-25 18:08 UTC (permalink / raw)
  To: Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Daniel P. Berrangé ,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé , Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, Paolo Bonzini, John Snow,
	Cleber Rosa

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
>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
  2024-06-25 16:18   ` Zhao Liu
@ 2024-06-25 18:23     ` Manos Pitsidianakis
  2024-06-25 19:15     ` Daniel P. Berrangé
  1 sibling, 0 replies; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-25 18:23 UTC (permalink / raw)
  To: Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Daniel P. Berrangé ,
	Marc-André Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daudé , Gustavo Romero, Pierrick Bouvier,
	rowan.hart, Richard Henderson, qemu-arm

On Tue, 25 Jun 2024 19:18, Zhao Liu <zhao1.liu@intel.com> wrote:
>Hi Manos,
>
>On Wed, Jun 19, 2024 at 11:14:02PM +0300, Manos Pitsidianakis wrote:
>> Date: Wed, 19 Jun 2024 23:14:02 +0300
>> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> Subject: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with
>>  x-pl011-rust in arm virt machine
>> X-Mailer: git-send-email 2.44.0
>> 
>> Convenience patch for testing the rust device.
>> 
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> ---
>>  hw/arm/virt.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 3c93c0c0a6..f33b58ae0d 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
>>      int irq = vms->irqmap[uart];
>>      const char compat[] = "arm,pl011\0arm,primecell";
>>      const char clocknames[] = "uartclk\0apb_pclk";
>> +#ifdef CONFIG_WITH_RUST
>> +    DeviceState *dev = qdev_new("x-pl011-rust");
>> +#else
>>      DeviceState *dev = qdev_new(TYPE_PL011);
>> +#endif
>>      SysBusDevice *s = SYS_BUS_DEVICE(dev);
>>      MachineState *ms = MACHINE(vms);
>>
>
>I realized that if we want to merge the rust pl011 device, then this
>patch or similar enablement support is necessary, otherwise, the rust
>code is only used for compilation and cannot actually be run...
>
>This is also an open for the devices that are rewrite in Rust.
>
>I think there should be an option for the user to choose whether to
>enable pl011 in C or pl011 in Rust. What do you think?
>
>Perhaps the easiest way to enable rust pl011 is to add an option for
>virt machine... But that's certainly not a long-term approach. I think
>the ideal way would be to allow rust pl011 to be specified in the
>command line via -device, but this approach would mean allowing the
>user to create pl011 and would require changes to the current buildin
>pl011's creation logic.
>
>-Zhao
>

We should definitely refer to ARM maintainers. The peculiarity of it 
being a chardev seems to make it not straightforward... I think it might 
have to become a machine property like discussed here:

https://lore.kernel.org/qemu-devel/CAFEAcA94twaBSx--NVXQcRBQ7v9TuK9iTq9kTWP4FYpRzgPbBA@mail.gmail.com/

And that'd be a bigger change not related just to Rust.



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
  2024-06-25 16:18   ` Zhao Liu
  2024-06-25 18:23     ` Manos Pitsidianakis
@ 2024-06-25 19:15     ` Daniel P. Berrangé
  2024-06-25 20:43       ` Peter Maydell
  1 sibling, 1 reply; 44+ messages in thread
From: Daniel P. Berrangé @ 2024-06-25 19:15 UTC (permalink / raw)
  To: Zhao Liu
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Bennée, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	qemu-arm

On Wed, Jun 26, 2024 at 12:18:55AM +0800, Zhao Liu wrote:
> Hi Manos,
> 
> On Wed, Jun 19, 2024 at 11:14:02PM +0300, Manos Pitsidianakis wrote:
> > Date: Wed, 19 Jun 2024 23:14:02 +0300
> > From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > Subject: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with
> >  x-pl011-rust in arm virt machine
> > X-Mailer: git-send-email 2.44.0
> > 
> > Convenience patch for testing the rust device.
> > 
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> >  hw/arm/virt.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 3c93c0c0a6..f33b58ae0d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
> >      int irq = vms->irqmap[uart];
> >      const char compat[] = "arm,pl011\0arm,primecell";
> >      const char clocknames[] = "uartclk\0apb_pclk";
> > +#ifdef CONFIG_WITH_RUST
> > +    DeviceState *dev = qdev_new("x-pl011-rust");
> > +#else
> >      DeviceState *dev = qdev_new(TYPE_PL011);
> > +#endif
> >      SysBusDevice *s = SYS_BUS_DEVICE(dev);
> >      MachineState *ms = MACHINE(vms);
> >
> 
> I realized that if we want to merge the rust pl011 device, then this
> patch or similar enablement support is necessary, otherwise, the rust
> code is only used for compilation and cannot actually be run...
> 
> This is also an open for the devices that are rewrite in Rust.
> 
> I think there should be an option for the user to choose whether to
> enable pl011 in C or pl011 in Rust. What do you think?
> 
> Perhaps the easiest way to enable rust pl011 is to add an option for
> virt machine... But that's certainly not a long-term approach. I think
> the ideal way would be to allow rust pl011 to be specified in the
> command line via -device, but this approach would mean allowing the
> user to create pl011 and would require changes to the current buildin
> pl011's creation logic.

While having both impls present is reasonable for the RFC, IMHO, if
we're to merge this, then the Rust impl of pl011 should fully replace
the C impl. Maintaining 2 different impls of the same device in
parallel is highly undesirable. This would of course impl that the
Rust impls needs to be a feature match for the existing impl. If we
do a full replacement, then the question of how the user chooses
between the 2 impls is then entirely avoided.

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 :|



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
  2024-06-25 19:15     ` Daniel P. Berrangé
@ 2024-06-25 20:43       ` Peter Maydell
  0 siblings, 0 replies; 44+ messages in thread
From: Peter Maydell @ 2024-06-25 20:43 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Zhao Liu, Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi,
	Mads Ynddal, Alex Bennée, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	qemu-arm

On Tue, 25 Jun 2024 at 20:15, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jun 26, 2024 at 12:18:55AM +0800, Zhao Liu wrote:
> > Hi Manos,
> >
> > On Wed, Jun 19, 2024 at 11:14:02PM +0300, Manos Pitsidianakis wrote:
> > > Date: Wed, 19 Jun 2024 23:14:02 +0300
> > > From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > Subject: [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with
> > >  x-pl011-rust in arm virt machine
> > > X-Mailer: git-send-email 2.44.0
> > >
> > > Convenience patch for testing the rust device.
> > >
> > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > ---
> > >  hw/arm/virt.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 3c93c0c0a6..f33b58ae0d 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -912,7 +912,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
> > >      int irq = vms->irqmap[uart];
> > >      const char compat[] = "arm,pl011\0arm,primecell";
> > >      const char clocknames[] = "uartclk\0apb_pclk";
> > > +#ifdef CONFIG_WITH_RUST
> > > +    DeviceState *dev = qdev_new("x-pl011-rust");
> > > +#else
> > >      DeviceState *dev = qdev_new(TYPE_PL011);
> > > +#endif
> > >      SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > >      MachineState *ms = MACHINE(vms);
> > >
> >
> > I realized that if we want to merge the rust pl011 device, then this
> > patch or similar enablement support is necessary, otherwise, the rust
> > code is only used for compilation and cannot actually be run...
> >
> > This is also an open for the devices that are rewrite in Rust.
> >
> > I think there should be an option for the user to choose whether to
> > enable pl011 in C or pl011 in Rust. What do you think?
> >
> > Perhaps the easiest way to enable rust pl011 is to add an option for
> > virt machine... But that's certainly not a long-term approach. I think
> > the ideal way would be to allow rust pl011 to be specified in the
> > command line via -device, but this approach would mean allowing the
> > user to create pl011 and would require changes to the current buildin
> > pl011's creation logic.
>
> While having both impls present is reasonable for the RFC, IMHO, if
> we're to merge this, then the Rust impl of pl011 should fully replace
> the C impl. Maintaining 2 different impls of the same device in
> parallel is highly undesirable. This would of course impl that the
> Rust impls needs to be a feature match for the existing impl. If we
> do a full replacement, then the question of how the user chooses
> between the 2 impls is then entirely avoided.

Yes, I agree this is where we want to go for the long term. For
the short term of reaching consensus on all the earlier parts of
the patch series (build system integration, how to do a rust
device model, etc), I think this patch is fine as a "don't commit this"
way to test the series. In the medium term I guess "feature-parity
both in parallel for a (hopefully) short time and then drop the C
version" ? I definitely feel we don't want a user-facing runtime
option for selecting which one, at any rate.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-24 17:14     ` Paolo Bonzini
@ 2024-06-25 21:47       ` Manos Pitsidianakis
  2024-06-26  9:34         ` Paolo Bonzini
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-25 21:47 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrangé 
  Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Benné e, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé ,
	Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
	Richard Henderson, John Snow, Cleber Rosa

On Mon, 24 Jun 2024 20:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
>Yes, I agree. However, considering we haven't even checked the 
>situation
>with what language features are required by any idiomatic bindings vs the
>1.63 version that we need to support for Debian, I think it's a bit
>premature to make it mandatory.
>

FWIW, I just ran

`cargo msrv -- cargo check --target x86_64-unknown-linux-gnu`

And the result was `The MSRV is: 1.77.2`

and the most important issue was that the mem::offset_of! macro was 
unstable till then.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-25 21:47       ` Manos Pitsidianakis
@ 2024-06-26  9:34         ` Paolo Bonzini
  0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-26  9:34 UTC (permalink / raw)
  To: Manos Pitsidianakis, Daniel P. Berrangé
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Marc-Andr é Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daud é, Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	John Snow, Cleber Rosa

On 6/25/24 23:47, Manos Pitsidianakis wrote:
> On Mon, 24 Jun 2024 20:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Yes, I agree. However, considering we haven't even checked the situation
>> with what language features are required by any idiomatic bindings vs the
>> 1.63 version that we need to support for Debian, I think it's a bit
>> premature to make it mandatory.
>>
> 
> FWIW, I just ran
> 
> `cargo msrv -- cargo check --target x86_64-unknown-linux-gnu`
> 
> And the result was `The MSRV is: 1.77.2`
> 
> and the most important issue was that the mem::offset_of! macro was 
> unstable till then.

I looked for a way to avoid it and the most promising is 
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=10a22a9b8393abd7b541d8fc844bc0df

Basically, you replace

     pub struct Foo {
         foo: i32,
         bar: i32
     }

with

     with_offsets! {
         #[repr(C)]  // mandatory
         pub struct Foo {
             foo: i32,
             bar: i32,
         }
     }

The macro walks the struct twice, once to actually declare it and once 
to determine the offsets using mem::size_of and mem::align_of.  The 
result is something like

         #[repr(C)]  // mandatory
         pub struct Foo {
             foo: i32,
             bar: i32,
         }

         pub struct FooOffsets {
             foo: usize,
             bar: usize,
         }

         impl Foo {
             pub const offset_to: FooOffsets = FooOffsets {
                 foo: 0,
                 bar: 4,
             }
         }

(where 0 and 4 are actually the aforementioned computation based on 
size_of and align_of).

There are some limitations but the trick is really well done; the need 
for #[repr(C)] is not a problem for us (C<->Rust interoperability needs 
it anyway), and the implementation is fully "const".  And though it only 
works for structs that use "with_offsets!", and with just one level of 
fields, the implementation of offset_of is trivial:

     macro_rules! offset_of {
         ($Container:ty, $field:ident) => {
             <$Container>::offset_to.$field
         };
     }

Anyhow, this should _not_ be in the first version that is 
committed---which, as you remarked in the v2, should focus on working 
build system integration.  As long as we know it is doable, it can be 
left for later.

Paolo



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-25 18:08         ` Manos Pitsidianakis
@ 2024-06-27 23:47           ` Pierrick Bouvier
  2024-06-28 19:12             ` Pierrick Bouvier
  0 siblings, 1 reply; 44+ messages in thread
From: Pierrick Bouvier @ 2024-06-27 23:47 UTC (permalink / raw)
  To: Manos Pitsidianakis, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é, Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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
>>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-27 23:47           ` Pierrick Bouvier
@ 2024-06-28 19:12             ` Pierrick Bouvier
  2024-06-28 21:50               ` Paolo Bonzini
  2024-06-29  8:06               ` Manos Pitsidianakis
  0 siblings, 2 replies; 44+ messages in thread
From: Pierrick Bouvier @ 2024-06-28 19:12 UTC (permalink / raw)
  To: Manos Pitsidianakis, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é, Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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)


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-28 19:12             ` Pierrick Bouvier
@ 2024-06-28 21:50               ` Paolo Bonzini
  2024-07-01 18:53                 ` Pierrick Bouvier
  2024-06-29  8:06               ` Manos Pitsidianakis
  1 sibling, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2024-06-28 21:50 UTC (permalink / raw)
  To: Pierrick Bouvier
  Cc: Manos Pitsidianakis, Zhao Liu, qemu-devel, Stefan Hajnoczi,
	Mads Ynddal, Peter Maydell, Alex Benn é e,
	Daniel P. Berrangé, Marc-Andr é Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daud é, Gustavo Romero,
	rowan.hart, Richard Henderson, John Snow, Cleber Rosa

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



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-28 19:12             ` Pierrick Bouvier
  2024-06-28 21:50               ` Paolo Bonzini
@ 2024-06-29  8:06               ` Manos Pitsidianakis
  2024-07-01 18:54                 ` Pierrick Bouvier
  1 sibling, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-06-29  8:06 UTC (permalink / raw)
  To: Pierrick Bouvier, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé ,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é , Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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)


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-28 21:50               ` Paolo Bonzini
@ 2024-07-01 18:53                 ` Pierrick Bouvier
  0 siblings, 0 replies; 44+ messages in thread
From: Pierrick Bouvier @ 2024-07-01 18:53 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Manos Pitsidianakis, Zhao Liu, qemu-devel, Stefan Hajnoczi,
	Mads Ynddal, Peter Maydell, Alex Benn é e,
	Daniel P. Berrangé, Marc-Andr é Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daud é, Gustavo Romero,
	rowan.hart, Richard Henderson, John Snow, Cleber Rosa

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
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-06-29  8:06               ` Manos Pitsidianakis
@ 2024-07-01 18:54                 ` Pierrick Bouvier
  2024-07-02 12:25                   ` Manos Pitsidianakis
  0 siblings, 1 reply; 44+ messages in thread
From: Pierrick Bouvier @ 2024-07-01 18:54 UTC (permalink / raw)
  To: Manos Pitsidianakis, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é, Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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)


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-07-01 18:54                 ` Pierrick Bouvier
@ 2024-07-02 12:25                   ` Manos Pitsidianakis
  2024-07-02 16:07                     ` Pierrick Bouvier
  0 siblings, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-07-02 12:25 UTC (permalink / raw)
  To: Pierrick Bouvier, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé ,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é , Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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)


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-06-24 16:52   ` Daniel P. Berrangé
  2024-06-24 17:14     ` Paolo Bonzini
@ 2024-07-02 14:38     ` Manos Pitsidianakis
  2024-07-02 15:23       ` Paolo Bonzini
  1 sibling, 1 reply; 44+ messages in thread
From: Manos Pitsidianakis @ 2024-07-02 14:38 UTC (permalink / raw)
  To: Daniel P. Berrangé 
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benné e, Marc-André Lureau, Thomas Huth,
	Markus Armbruster, Philippe Mathieu-Daudé , Zhao Liu,
	Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
	Paolo Bonzini, John Snow, Cleber Rosa

Hi Daniel, I missed one of your questions and I just re-read the thread 
today,

On Mon, 24 Jun 2024 19:52, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
>> Add options for Rust in meson_options.txt, meson.build, configure to
>> prepare for adding Rust code in the followup commits.
>> 
>> `rust` is a reserved meson name, so we have to use an alternative.
>> `with_rust` was chosen.
>> 
>> A cargo_wrapper.py script is added that is heavily based on the work of
>> Marc-André Lureau from 2021.
>> 
>> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
>> 
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> ---
>>  MAINTAINERS                   |   5 +
>>  configure                     |  11 ++
>>  meson.build                   |  11 ++
>>  meson_options.txt             |   4 +
>>  scripts/cargo_wrapper.py      | 279 ++++++++++++++++++++++++++++++++++
>>  scripts/meson-buildoptions.sh |   6 +
>>  6 files changed, 316 insertions(+)
>>  create mode 100644 scripts/cargo_wrapper.py
>
>> diff --git a/configure b/configure
>> index 38ee257701..6894d7c2d1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -302,6 +302,9 @@ else
>>    objcc="${objcc-${cross_prefix}clang}"
>>  fi
>>
>>[..snip..]
>> +with_rust_target_triple=""
>> +
>>  ar="${AR-${cross_prefix}ar}"
>>  as="${AS-${cross_prefix}as}"
>>  ccas="${CCAS-$cc}"
>> @@ -760,6 +763,12 @@ for opt do
>>    ;;
>>    --gdb=*) gdb_bin="$optarg"
>>    ;;
>> +  --enable-with-rust) with_rust=enabled
>> +  ;;
>> +  --disable-with-rust) with_rust=disabled
>> +  ;;
>
>--enable-with-XXX / --disable-with-XXX is pretty unsual naming.
>
>Normally you'd see either --enable-XXX or --with-XXX and their
>corresponding --disable-XXX or --without-XXX.

True. As the commit message says, `rust` is a reserved meson feature 
name, so the auto-generated scripts/meson-buildoptions.sh
has the following args:

  --enable-with-rust
  --disable-with-rust

I used the same in `configure` even though it's not autogenerated in 
order to keep the two synced. If there's a way to get around this I'd 
prefer it.



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
  2024-07-02 14:38     ` Manos Pitsidianakis
@ 2024-07-02 15:23       ` Paolo Bonzini
  0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2024-07-02 15:23 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
	Peter Maydell, Alex Benné e, Marc-André Lureau,
	Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
	Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
	Richard Henderson, John Snow, Cleber Rosa

On Tue, Jul 2, 2024 at 4:44 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> >Normally you'd see either --enable-XXX or --with-XXX and their
> >corresponding --disable-XXX or --without-XXX.
>
> True. As the commit message says, `rust` is a reserved meson feature
> name, so the auto-generated scripts/meson-buildoptions.sh
> has the following args:
>
>   --enable-with-rust
>   --disable-with-rust
>
> I used the same in `configure` even though it's not autogenerated in
> order to keep the two synced. If there's a way to get around this I'd
> prefer it.

With the patch I posted, --with-rust/--without-rust is handled
entirely in configure, Meson gleans the result from the presence of
RUST_TARGET_TRIPLE in config-host.mak.

Paolo



^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
  2024-07-02 12:25                   ` Manos Pitsidianakis
@ 2024-07-02 16:07                     ` Pierrick Bouvier
  0 siblings, 0 replies; 44+ messages in thread
From: Pierrick Bouvier @ 2024-07-02 16:07 UTC (permalink / raw)
  To: Manos Pitsidianakis, Zhao Liu
  Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
	Alex Benn é e, Daniel P. Berrangé,
	Marc-Andr é Lureau, Thomas Huth, Markus Armbruster,
	Philippe Mathieu-Daud é, Gustavo Romero, rowan.hart,
	Richard Henderson, Paolo Bonzini, John Snow, Cleber Rosa

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)


^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2024-07-02 16:07 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-19 20:13 [RFC PATCH v3 0/5] Implement ARM PL011 in Rust Manos Pitsidianakis
2024-06-19 20:13 ` [RFC PATCH v3 1/5] build-sys: Add rust feature option Manos Pitsidianakis
2024-06-20 13:21   ` Paolo Bonzini
2024-06-20 18:06     ` Manos Pitsidianakis
2024-06-20 19:44       ` Paolo Bonzini
2024-06-24  8:51     ` Zhao Liu
2024-06-24 16:35       ` Paolo Bonzini
2024-06-20 13:41   ` Alex Bennée
2024-06-20 18:14     ` Manos Pitsidianakis
2024-06-24 16:52   ` Daniel P. Berrangé
2024-06-24 17:14     ` Paolo Bonzini
2024-06-25 21:47       ` Manos Pitsidianakis
2024-06-26  9:34         ` Paolo Bonzini
2024-07-02 14:38     ` Manos Pitsidianakis
2024-07-02 15:23       ` Paolo Bonzini
2024-06-19 20:13 ` [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency Manos Pitsidianakis
2024-06-20 11:10   ` Alex Bennée
2024-06-20 12:34     ` Paolo Bonzini
2024-06-20 18:18       ` Manos Pitsidianakis
2024-06-20 12:32   ` Alex Bennée
2024-06-20 18:22     ` Manos Pitsidianakis
2024-06-24 19:52       ` Stefan Hajnoczi
2024-06-20 14:01   ` Richard Henderson
2024-06-20 18:36     ` Manos Pitsidianakis
2024-06-20 19:30       ` Richard Henderson
2024-06-24 10:12   ` Zhao Liu
2024-06-24 10:02     ` Manos Pitsidianakis
2024-06-25 16:00       ` Zhao Liu
2024-06-25 18:08         ` Manos Pitsidianakis
2024-06-27 23:47           ` Pierrick Bouvier
2024-06-28 19:12             ` Pierrick Bouvier
2024-06-28 21:50               ` Paolo Bonzini
2024-07-01 18:53                 ` Pierrick Bouvier
2024-06-29  8:06               ` Manos Pitsidianakis
2024-07-01 18:54                 ` Pierrick Bouvier
2024-07-02 12:25                   ` Manos Pitsidianakis
2024-07-02 16:07                     ` Pierrick Bouvier
2024-06-19 20:14 ` [RFC PATCH v3 3/5] rust: add PL011 device model Manos Pitsidianakis
2024-06-19 20:14 ` [RFC PATCH v3 4/5] DO NOT MERGE: add rustdoc build for gitlab pages Manos Pitsidianakis
2024-06-19 20:14 ` [RFC PATCH v3 5/5] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
2024-06-25 16:18   ` Zhao Liu
2024-06-25 18:23     ` Manos Pitsidianakis
2024-06-25 19:15     ` Daniel P. Berrangé
2024-06-25 20:43       ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).