[RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

Hello everyone,

This is an early draft of my work on implementing a very simple device, 
in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
and is used in hw/arm/virt.c).

The device is functional, with copied logic from the C code but with 
effort not to make a direct C to Rust translation. In other words, do 
not write Rust as a C developer would.

That goal is not complete but a best-effort case. To give a specific 
example, register values are typed but interrupt bit flags are not (but 
could be). I will leave such minutiae for later iterations.

By the way, the wiki page for Rust was revived to keep track of all 
current series on the mailing list https://wiki.qemu.org/RustInQemu

a #qemu-rust IRC channel was also created for rust-specific discussion 
that might flood #qemu

------------------------------------------------------------------------
A request: keep comments to Rust in relation to the QEMU project and no 
debates on the merits of the language itself. These are valid concerns, 
but it'd be better if they were on separate mailing list threads.
------------------------------------------------------------------------

Table of contents: [TOC]

- How can I try it? [howcanItryit]
- What are the most important points to focus on, at this point? 
  [whatarethemostimportant]
  - What are the issues with not using the compiler, rustc, directly? 
    [whataretheissueswith]
    1. Tooling
    2. Rust dependencies

  - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
  - Should QEMU provide wrapping Rust APIs over QEMU internals? 
    [qemuprovidewrappingrustapis]
  - Will QEMU now depend on Rust and thus not build on my XYZ platform? 
    [qemudependonrustnotbuildonxyz]
- How is the compilation structured? [howisthecompilationstructured]
- The generated.rs rust file includes a bunch of junk definitions? 
  [generatedrsincludesjunk]
- The staticlib artifact contains a bunch of mangled .o objects? 
  [staticlibmangledobjects]

How can I try it?
=================
[howcanItryit] Back to [TOC]

Hopefully applying this patches (or checking out `master` branch from 
https://gitlab.com/epilys/rust-for-qemu/ current commit 
de81929e0e9d470deac2c6b449b7a5183325e7ee )

Tag for this RFC is rust-pl011-rfc-v1 

Rustdoc documentation is hosted on

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

If `cargo` and `bindgen` is installed in your system, you should be able 
to build qemu-system-aarch64 with configure flag --enable-rust and 
launch an arm virt VM. One of the patches hardcodes the default UART of 
the machine to the Rust one, so if something goes wrong you will see it 
upon launching qemu-system-aarch64.

To confirm it is there for sure, run e.g. info qom-tree on the monitor 
and look for x-pl011-rust.


What are the most important points to focus on, at this point?
==============================================================
[whatarethemostimportant] Back to [TOC]

In my opinion, integration of the go-to Rust build system (Cargo and 
crates.io) with the build system we use in QEMU. This is "easily" done 
in some definition of the word with a python wrapper script.

What are the issues with not using the compiler, rustc, directly?
-----------------------------------------------------------------
[whataretheissueswith] Back to [TOC]

1. Tooling
   Mostly writing up the build-sys tooling to do so. Ideally we'd 
   compile everything without cargo but rustc directly.

   If we decide we need Rust's `std` library support, we could 
   investigate whether building it from scratch is a good solution. This 
   will only build the bits we need in our devices.

2. Rust dependencies
   We could go without them completely. I chose deliberately to include 
   one dependency in my UART implementation, `bilge`[0], because it has 
   an elegant way of representing typed bitfields for the UART's 
   registers.

[0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
     Crates.io page: https://crates.io/crates/bilge
     Repository: https://github.com/hecatia-elegua/bilge

Should QEMU use third-party dependencies?
-----------------------------------------
[shouldqemuusethirdparty] Back to [TOC]

In my personal opinion, if we need a dependency we need a strong 
argument for it. A dependency needs a trusted upstream source, a QEMU 
maintainer to make sure it us up-to-date in QEMU etc.

We already fetch some projects with meson subprojects, so this is not a 
new reality. Cargo allows you to define "locked" dependencies which is 
the same as only fetching specific commits by SHA. No suspicious 
tarballs, and no disappearing dependencies a la left-pad in npm.

However, I believe it's worth considering vendoring every dependency by 
default, if they prove to be few, for the sake of having a local QEMU 
git clone buildable without network access.

Should QEMU provide wrapping Rust APIs over QEMU internals?
-----------------------------------------------------------
[qemuprovidewrappingrustapis] Back to [TOC]

My personal opinion is no, with the reasoning being that QEMU internals 
are not documented or stable. However I do not see why creating stable 
opt-in interfaces is bad. It just needs someone to volunteer to maintain 
it and ensure there are no breakages through versions.

Will QEMU now depend on Rust and thus not build on my XYZ platform?
-------------------------------------------------------------------
[qemudependonrustnotbuildonxyz] Back to [TOC]

No, worry about this in some years if this experiment takes off. Rust 
has broad platform support and is present in most distro package 
managers. In the future we might have gcc support for it as well.

For now, Rust will have an experimental status, and will be aimed to 
those who wish to try it. I leave it to the project leaders to make 
proper decisions and statements on this if necessary.


How is the compilation structured?
==================================
[howisthecompilationstructured] Back to [TOC]

First, a meson target that runs `bindgen` on a bunch of header files 
(defined in `rust/wrapper.h`) is created as a target and as a dependency 
for any rust hardware device that needs it. You can see the generated 
bindings by running

  ninja generated.rs

inside your build directory.

The devices are defined as dictionaries in rust/meson.build because they 
depend on the bindgen dependency, which is available much later in the 
meson process (when the static qemu lib and target emulator executables 
are defined).

A cargo wrapper python script under scripts/ exists to build the crate 
library, by providing the path to the generated.rs bindings via the 
environment. Then, the qemu-system-aarch64 binary links against the 
staticlib archive (i.e. libpl011.a)

The generated.rs rust file includes a bunch of junk definitions?
================================================================
[generatedrsincludesjunk] Back to [TOC]

Yes, bindgen allows you to block certain types and identifiers from 
being generated but they are simply too many. I have trimmed some of the 
fat but vast improvements can be made.

The staticlib artifact contains a bunch of mangled .o objects?
==============================================================
[staticlibmangledobjects] Back to [TOC]

Yes, until we compile without the `std` module library or we compile it 
manually instead of linking it, we will have some junk in it.

--

Manos Pitsidianakis (6):
  build-sys: Add rust feature option
  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
  rust: add bindgen step as a meson dependency
  DO NOT MERGE: update rustdoc gitlab pages gen

 .gitignore                     |   2 +
 .gitlab-ci.d/buildtest.yml     |  64 ++--
 configure                      |  12 +
 hw/arm/virt.c                  |   2 +-
 meson.build                    |  99 ++++++
 meson_options.txt              |   4 +
 rust/meson.build               |  93 ++++++
 rust/pl011/.cargo/config.toml  |   2 +
 rust/pl011/.gitignore          |   2 +
 rust/pl011/Cargo.lock          | 120 +++++++
 rust/pl011/Cargo.toml          |  26 ++
 rust/pl011/README.md           |  42 +++
 rust/pl011/build.rs            |  44 +++
 rust/pl011/meson.build         |   7 +
 rust/pl011/rustfmt.toml        |  10 +
 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          | 575 +++++++++++++++++++++++++++++++++
 rust/pl011/src/memory_ops.rs   |  38 +++
 rust/wrapper.h                 |  39 +++
 scripts/cargo_wrapper.py       | 221 +++++++++++++
 scripts/meson-buildoptions.sh  |   5 +
 24 files changed, 2113 insertions(+), 20 deletions(-)
 create mode 100644 rust/meson.build
 create mode 100644 rust/pl011/.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/meson.build
 create mode 100644 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/wrapper.h
 create mode 100644 scripts/cargo_wrapper.py


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

[RFC PATCH v1 1/6] build-sys: Add rust feature option

[Manos Pitsidianakis]

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.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
The cargo wrapper script hardcodes some rust target triples. This is 
just temporary.
---
 .gitignore               |   2 +
 configure                |  12 +++
 meson.build              |  11 ++
 meson_options.txt        |   4 +
 scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 240 insertions(+)
 create mode 100644 scripts/cargo_wrapper.py

diff --git a/.gitignore b/.gitignore
index 61fa39967b..f42b0d937e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,8 @@
 /build/
 /.cache/
 /.vscode/
+/target/
+rust/**/target
 *.pyc
 .sdk
 .stgit-*
diff --git a/configure b/configure
index 38ee257701..c195630771 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-rust) with_rust=enabled
+  ;;
+  --disable-rust) with_rust=disabled
+  ;;
+  --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,9 @@ 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" != 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..d338effdaa
--- /dev/null
+++ b/scripts/cargo_wrapper.py
@@ -0,0 +1,211 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 Red Hat, Inc.
+# Copyright (c) 2023 Linaro Ltd.
+#
+# Authors:
+#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+#  Marc-André Lureau <marcandre.lureau@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import argparse
+import configparser
+import distutils.file_util
+import json
+import logging
+import os
+import os.path
+import re
+import subprocess
+import sys
+import pathlib
+import shutil
+import tomllib
+
+from pathlib import Path
+from typing import Any, Dict, List, Tuple
+
+RUST_TARGET_TRIPLES = (
+    "aarch64-unknown-linux-gnu",
+    "x86_64-unknown-linux-gnu",
+    "x86_64-apple-darwin",
+    "aarch64-apple-darwin",
+)
+
+
+def cfg_name(name: str) -> str:
+    if (
+        name.startswith("CONFIG_")
+        or name.startswith("TARGET_")
+        or name.startswith("HAVE_")
+    ):
+        return name
+    return ""
+
+
+def generate_cfg_flags(header: str) -> List[str]:
+    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) -> pathlib.Path:
+    return args.meson_build_dir
+
+
+def manifest_path(args: argparse.Namespace) -> pathlib.Path:
+    return args.crate_dir / "Cargo.toml"
+
+
+def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
+    # 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:
+    envlog = " ".join(["{}={}".format(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 build_lib(args: argparse.Namespace) -> None:
+    logging.debug("build-lib")
+    target_dir = cargo_target_dir(args)
+    cargo_toml_path = manifest_path(args)
+
+    with open(cargo_toml_path, "rb") as f:
+        config = tomllib.load(f)
+
+    package_name = config["package"]["name"].strip('"').replace("-", "_")
+
+    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("cp %s %s", liba, args.outdir)
+    shutil.copy2(liba, args.outdir)
+
+
+def main() -> None:
+    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=pathlib.Path,
+        dest="meson_build_dir",
+        required=True,
+    )
+    parser.add_argument(
+        "--meson-source-dir",
+        metavar="SOURCE_DIR",
+        help="meson.current_source_dir()",
+        type=pathlib.Path,
+        dest="meson_source_dir",
+        required=True,
+    )
+    parser.add_argument(
+        "--crate-dir",
+        metavar="CRATE_DIR",
+        type=pathlib.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=pathlib.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, choices=RUST_TARGET_TRIPLES, 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()
-- 
γαῖα πυρί μιχθήτω

[RFC PATCH v1 3/6] DO NOT MERGE: add rustdoc build for gitlab pages

[Manos Pitsidianakis]

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

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91c57efded..1cd6519506 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -715,31 +715,48 @@ 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/
+    - 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/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
-- 
γαῖα πυρί μιχθήτω

[RFC PATCH v1 4/6] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine

[Manos Pitsidianakis]

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3c93c0c0a6..153be0f42d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -912,7 +912,7 @@ 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";
-    DeviceState *dev = qdev_new(TYPE_PL011);
+    DeviceState *dev = qdev_new("x-pl011-rust");
     SysBusDevice *s = SYS_BUS_DEVICE(dev);
     MachineState *ms = MACHINE(vms);
 
-- 
γαῖα πυρί μιχθήτω

[RFC PATCH v1 6/6] DO NOT MERGE: update rustdoc gitlab pages gen

[Manos Pitsidianakis]

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

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 1cd6519506..da882813b8 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -744,11 +744,20 @@ build-tools-and-docs-debian:
 pages:
   image: rust:latest
   script:
-    - cd ./rust/pl011/
+    - 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 "generated.rs"
+    - cp ./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/doc ./public
+    - mv ./rust/pl011/target/x86_64-unknown-linux-gnu/doc ./public
   artifacts:
     when: on_success
     paths:
-- 
γαῖα πυρί μιχθήτω

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Stefan Hajnoczi]

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

On Mon, Jun 10, 2024 at 09:22:36PM +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.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> The cargo wrapper script hardcodes some rust target triples. This is 
> just temporary.
> ---
>  .gitignore               |   2 +
>  configure                |  12 +++
>  meson.build              |  11 ++
>  meson_options.txt        |   4 +
>  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 240 insertions(+)
>  create mode 100644 scripts/cargo_wrapper.py
> 
> diff --git a/.gitignore b/.gitignore
> index 61fa39967b..f42b0d937e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -2,6 +2,8 @@
>  /build/
>  /.cache/
>  /.vscode/
> +/target/
> +rust/**/target

Are these necessary since the cargo build command-line below uses
--target-dir <meson-build-dir>?

Adding new build output directories outside build/ makes it harder to
clean up the source tree and ensure no state from previous builds
remains.

>  *.pyc
>  .sdk
>  .stgit-*
> diff --git a/configure b/configure
> index 38ee257701..c195630771 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-rust) with_rust=enabled
> +  ;;
> +  --disable-rust) with_rust=disabled
> +  ;;
> +  --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,9 @@ 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" != enabled && meson_option_add "-Dwith_rust=$with_rust"

Duplicate line.

> +  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..d338effdaa
> --- /dev/null
> +++ b/scripts/cargo_wrapper.py
> @@ -0,0 +1,211 @@
> +#!/usr/bin/env python3
> +# Copyright (c) 2020 Red Hat, Inc.
> +# Copyright (c) 2023 Linaro Ltd.
> +#
> +# Authors:
> +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +#  Marc-André Lureau <marcandre.lureau@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import argparse
> +import configparser
> +import distutils.file_util
> +import json
> +import logging
> +import os
> +import os.path
> +import re
> +import subprocess
> +import sys
> +import pathlib
> +import shutil
> +import tomllib
> +
> +from pathlib import Path
> +from typing import Any, Dict, List, Tuple
> +
> +RUST_TARGET_TRIPLES = (
> +    "aarch64-unknown-linux-gnu",
> +    "x86_64-unknown-linux-gnu",
> +    "x86_64-apple-darwin",
> +    "aarch64-apple-darwin",
> +)

Is this hardcoded to avoid calling `rustc --print target-list`?

Or is this the support matrix? In that case it would be interesting to
figure out the target triples for all host OSes and CPUs that QEMU is
supported on.

> +
> +
> +def cfg_name(name: str) -> str:
> +    if (
> +        name.startswith("CONFIG_")
> +        or name.startswith("TARGET_")
> +        or name.startswith("HAVE_")
> +    ):
> +        return name
> +    return ""
> +
> +
> +def generate_cfg_flags(header: str) -> List[str]:
> +    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) -> pathlib.Path:
> +    return args.meson_build_dir
> +
> +
> +def manifest_path(args: argparse.Namespace) -> pathlib.Path:
> +    return args.crate_dir / "Cargo.toml"
> +
> +
> +def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
> +    # 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:
> +    envlog = " ".join(["{}={}".format(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 build_lib(args: argparse.Namespace) -> None:
> +    logging.debug("build-lib")
> +    target_dir = cargo_target_dir(args)
> +    cargo_toml_path = manifest_path(args)
> +
> +    with open(cargo_toml_path, "rb") as f:
> +        config = tomllib.load(f)
> +
> +    package_name = config["package"]["name"].strip('"').replace("-", "_")
> +
> +    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("cp %s %s", liba, args.outdir)
> +    shutil.copy2(liba, args.outdir)
> +
> +
> +def main() -> None:
> +    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=pathlib.Path,
> +        dest="meson_build_dir",
> +        required=True,
> +    )
> +    parser.add_argument(
> +        "--meson-source-dir",
> +        metavar="SOURCE_DIR",
> +        help="meson.current_source_dir()",
> +        type=pathlib.Path,
> +        dest="meson_source_dir",
> +        required=True,
> +    )
> +    parser.add_argument(
> +        "--crate-dir",
> +        metavar="CRATE_DIR",
> +        type=pathlib.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=pathlib.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, choices=RUST_TARGET_TRIPLES, 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()
> -- 
> γαῖα πυρί μιχθήτω
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Pierrick Bouvier]

Hello Manos,

On 6/10/24 11:22, Manos Pitsidianakis wrote:
> Hello everyone,
> 
> This is an early draft of my work on implementing a very simple device,
> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> and is used in hw/arm/virt.c).
> 
> The device is functional, with copied logic from the C code but with
> effort not to make a direct C to Rust translation. In other words, do
> not write Rust as a C developer would.
> 
> That goal is not complete but a best-effort case. To give a specific
> example, register values are typed but interrupt bit flags are not (but
> could be). I will leave such minutiae for later iterations.
> 
> By the way, the wiki page for Rust was revived to keep track of all
> current series on the mailing list https://wiki.qemu.org/RustInQemu
> 
> a #qemu-rust IRC channel was also created for rust-specific discussion
> that might flood #qemu
> 

Excellent work, and thanks for posting this RFC!

IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing 
(temporarily) the generated.rs file in patches is not a good move.
Any reason you kept it this way?

Maybe it could be better if build.rs file was *not* needed for new 
devices/folders, and could be abstracted as a detail of the python 
wrapper script instead of something that should be committed.

Having a simple rust/pl011/meson.build is nice and good taste!

> ------------------------------------------------------------------------
> A request: keep comments to Rust in relation to the QEMU project and no
> debates on the merits of the language itself. These are valid concerns,
> but it'd be better if they were on separate mailing list threads.
> ------------------------------------------------------------------------
> 
> Table of contents: [TOC]
> 
> - How can I try it? [howcanItryit]
> - What are the most important points to focus on, at this point?
>    [whatarethemostimportant]
>    - What are the issues with not using the compiler, rustc, directly?
>      [whataretheissueswith]
>      1. Tooling
>      2. Rust dependencies
> 
>    - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
>    - Should QEMU provide wrapping Rust APIs over QEMU internals?
>      [qemuprovidewrappingrustapis]
>    - Will QEMU now depend on Rust and thus not build on my XYZ platform?
>      [qemudependonrustnotbuildonxyz]
> - How is the compilation structured? [howisthecompilationstructured]
> - The generated.rs rust file includes a bunch of junk definitions?
>    [generatedrsincludesjunk]
> - The staticlib artifact contains a bunch of mangled .o objects?
>    [staticlibmangledobjects]
> 
> How can I try it?
> =================
> [howcanItryit] Back to [TOC]
> 
> Hopefully applying this patches (or checking out `master` branch from
> https://gitlab.com/epilys/rust-for-qemu/ current commit
> de81929e0e9d470deac2c6b449b7a5183325e7ee )
> 
> Tag for this RFC is rust-pl011-rfc-v1
> 
> Rustdoc documentation is hosted on
> 
> https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
> 
> If `cargo` and `bindgen` is installed in your system, you should be able
> to build qemu-system-aarch64 with configure flag --enable-rust and
> launch an arm virt VM. One of the patches hardcodes the default UART of
> the machine to the Rust one, so if something goes wrong you will see it
> upon launching qemu-system-aarch64.
> 
> To confirm it is there for sure, run e.g. info qom-tree on the monitor
> and look for x-pl011-rust.
> 
> 
> What are the most important points to focus on, at this point?
> ==============================================================
> [whatarethemostimportant] Back to [TOC]
> 
> In my opinion, integration of the go-to Rust build system (Cargo and
> crates.io) with the build system we use in QEMU. This is "easily" done
> in some definition of the word with a python wrapper script.
> 
> What are the issues with not using the compiler, rustc, directly?
> -----------------------------------------------------------------
> [whataretheissueswith] Back to [TOC]
> 
> 1. Tooling
>     Mostly writing up the build-sys tooling to do so. Ideally we'd
>     compile everything without cargo but rustc directly.
> 
>     If we decide we need Rust's `std` library support, we could
>     investigate whether building it from scratch is a good solution. This
>     will only build the bits we need in our devices.
>  > 2. Rust dependencies
>     We could go without them completely. I chose deliberately to include
>     one dependency in my UART implementation, `bilge`[0], because it has
>     an elegant way of representing typed bitfields for the UART's
>     registers.
> 
> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>       Crates.io page: https://crates.io/crates/bilge
>       Repository: https://github.com/hecatia-elegua/bilge
> 
> Should QEMU use third-party dependencies?
> -----------------------------------------
> [shouldqemuusethirdparty] Back to [TOC]
> 
> In my personal opinion, if we need a dependency we need a strong
> argument for it. A dependency needs a trusted upstream source, a QEMU
> maintainer to make sure it us up-to-date in QEMU etc.
> 
> We already fetch some projects with meson subprojects, so this is not a
> new reality. Cargo allows you to define "locked" dependencies which is
> the same as only fetching specific commits by SHA. No suspicious
> tarballs, and no disappearing dependencies a la left-pad in npm.
>

As a complement to this, and for other readers, in more than having a 
lock file (fixing version you use), cargo crates system is designed to 
be immutable (see: https://crates.io/policies), and it means there is a 
strong guarantee that a published package will stay there, to the 
opposite of npm, pypi, or most of other similar systems.

"Crate deletion by their owners is not possible to keep the registry as 
immutable as possible."

I believe this is a *key* feature of Rust ecosystem and should be 
emphasized regarding the policy for Rust dependencies to come.

> However, I believe it's worth considering vendoring every dependency by
> default, if they prove to be few, for the sake of having a local QEMU
> git clone buildable without network access.
>

I would not be in favor to vendor all dependencies. Beyond the "offline 
build" scenario, it has only downsides.

Sure, we should really debate before introducing a new dependency, but 
the technical difficulty to mirror its sources and dependencies should 
not be an argument for or against it.

What will happen the day we want to introduce something bigger than a 
simple dependency? (let's say "serde" for instance)

> Should QEMU provide wrapping Rust APIs over QEMU internals?
> -----------------------------------------------------------
> [qemuprovidewrappingrustapis] Back to [TOC]
> 
> My personal opinion is no, with the reasoning being that QEMU internals
> are not documented or stable. However I do not see why creating stable
> opt-in interfaces is bad. It just needs someone to volunteer to maintain
> it and ensure there are no breakages through versions.
> 
> Will QEMU now depend on Rust and thus not build on my XYZ platform?
> -------------------------------------------------------------------
> [qemudependonrustnotbuildonxyz] Back to [TOC]
> 
> No, worry about this in some years if this experiment takes off. Rust
> has broad platform support and is present in most distro package
> managers. In the future we might have gcc support for it as well.
> 
> For now, Rust will have an experimental status, and will be aimed to
> those who wish to try it. I leave it to the project leaders to make
> proper decisions and statements on this if necessary.
> 
> 
> How is the compilation structured?
> ==================================
> [howisthecompilationstructured] Back to [TOC]
> 
> First, a meson target that runs `bindgen` on a bunch of header files
> (defined in `rust/wrapper.h`) is created as a target and as a dependency
> for any rust hardware device that needs it. You can see the generated
> bindings by running
> 
>    ninja generated.rs
> 
> inside your build directory.
> 
> The devices are defined as dictionaries in rust/meson.build because they
> depend on the bindgen dependency, which is available much later in the
> meson process (when the static qemu lib and target emulator executables
> are defined).
> 
> A cargo wrapper python script under scripts/ exists to build the crate
> library, by providing the path to the generated.rs bindings via the
> environment. Then, the qemu-system-aarch64 binary links against the
> staticlib archive (i.e. libpl011.a)
> 
> The generated.rs rust file includes a bunch of junk definitions?
> ================================================================
> [generatedrsincludesjunk] Back to [TOC]
> 
> Yes, bindgen allows you to block certain types and identifiers from
> being generated but they are simply too many. I have trimmed some of the
> fat but vast improvements can be made.
> 
> The staticlib artifact contains a bunch of mangled .o objects?
> ==============================================================
> [staticlibmangledobjects] Back to [TOC]
> 
> Yes, until we compile without the `std` module library or we compile it
> manually instead of linking it, we will have some junk in it.
> 

Besides the size aspect, which potential advantage would there be to 
switch to no_std?
We don't build a bare metal or kernel binary here, so why introduce this 
restriction willingly?

> --
> 
> Manos Pitsidianakis (6):
>    build-sys: Add rust feature option
>    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
>    rust: add bindgen step as a meson dependency
>    DO NOT MERGE: update rustdoc gitlab pages gen
> 
>   .gitignore                     |   2 +
>   .gitlab-ci.d/buildtest.yml     |  64 ++--
>   configure                      |  12 +
>   hw/arm/virt.c                  |   2 +-
>   meson.build                    |  99 ++++++
>   meson_options.txt              |   4 +
>   rust/meson.build               |  93 ++++++
>   rust/pl011/.cargo/config.toml  |   2 +
>   rust/pl011/.gitignore          |   2 +
>   rust/pl011/Cargo.lock          | 120 +++++++
>   rust/pl011/Cargo.toml          |  26 ++
>   rust/pl011/README.md           |  42 +++
>   rust/pl011/build.rs            |  44 +++
>   rust/pl011/meson.build         |   7 +
>   rust/pl011/rustfmt.toml        |  10 +
>   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          | 575 +++++++++++++++++++++++++++++++++
>   rust/pl011/src/memory_ops.rs   |  38 +++
>   rust/wrapper.h                 |  39 +++
>   scripts/cargo_wrapper.py       | 221 +++++++++++++
>   scripts/meson-buildoptions.sh  |   5 +
>   24 files changed, 2113 insertions(+), 20 deletions(-)
>   create mode 100644 rust/meson.build
>   create mode 100644 rust/pl011/.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/meson.build
>   create mode 100644 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/wrapper.h
>   create mode 100644 scripts/cargo_wrapper.py
> 
> 
> base-commit: 01782d6b294f95bcde334386f0aaac593cd28c0d

Thanks,
Pierrick

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Stefan Hajnoczi]

On Mon, 10 Jun 2024 at 14:23, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> Hello everyone,
>
> This is an early draft of my work on implementing a very simple device,
> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> and is used in hw/arm/virt.c).
>
> The device is functional, with copied logic from the C code but with
> effort not to make a direct C to Rust translation. In other words, do
> not write Rust as a C developer would.
>
> That goal is not complete but a best-effort case. To give a specific
> example, register values are typed but interrupt bit flags are not (but
> could be). I will leave such minutiae for later iterations.
>
> By the way, the wiki page for Rust was revived to keep track of all
> current series on the mailing list https://wiki.qemu.org/RustInQemu
>
> a #qemu-rust IRC channel was also created for rust-specific discussion
> that might flood #qemu
>
> ------------------------------------------------------------------------
> A request: keep comments to Rust in relation to the QEMU project and no
> debates on the merits of the language itself. These are valid concerns,
> but it'd be better if they were on separate mailing list threads.
> ------------------------------------------------------------------------
>
> Table of contents: [TOC]
>
> - How can I try it? [howcanItryit]
> - What are the most important points to focus on, at this point?
>   [whatarethemostimportant]
>   - What are the issues with not using the compiler, rustc, directly?
>     [whataretheissueswith]
>     1. Tooling
>     2. Rust dependencies
>
>   - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
>   - Should QEMU provide wrapping Rust APIs over QEMU internals?
>     [qemuprovidewrappingrustapis]
>   - Will QEMU now depend on Rust and thus not build on my XYZ platform?
>     [qemudependonrustnotbuildonxyz]
> - How is the compilation structured? [howisthecompilationstructured]
> - The generated.rs rust file includes a bunch of junk definitions?
>   [generatedrsincludesjunk]
> - The staticlib artifact contains a bunch of mangled .o objects?
>   [staticlibmangledobjects]
>
> How can I try it?
> =================
> [howcanItryit] Back to [TOC]
>
> Hopefully applying this patches (or checking out `master` branch from
> https://gitlab.com/epilys/rust-for-qemu/ current commit
> de81929e0e9d470deac2c6b449b7a5183325e7ee )
>
> Tag for this RFC is rust-pl011-rfc-v1
>
> Rustdoc documentation is hosted on
>
> https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
>
> If `cargo` and `bindgen` is installed in your system, you should be able
> to build qemu-system-aarch64 with configure flag --enable-rust and
> launch an arm virt VM. One of the patches hardcodes the default UART of
> the machine to the Rust one, so if something goes wrong you will see it
> upon launching qemu-system-aarch64.
>
> To confirm it is there for sure, run e.g. info qom-tree on the monitor
> and look for x-pl011-rust.
>
>
> What are the most important points to focus on, at this point?
> ==============================================================
> [whatarethemostimportant] Back to [TOC]
>
> In my opinion, integration of the go-to Rust build system (Cargo and
> crates.io) with the build system we use in QEMU. This is "easily" done
> in some definition of the word with a python wrapper script.
>
> What are the issues with not using the compiler, rustc, directly?
> -----------------------------------------------------------------
> [whataretheissueswith] Back to [TOC]
>
> 1. Tooling
>    Mostly writing up the build-sys tooling to do so. Ideally we'd
>    compile everything without cargo but rustc directly.

Why would that be ideal?

>
>    If we decide we need Rust's `std` library support, we could
>    investigate whether building it from scratch is a good solution. This
>    will only build the bits we need in our devices.

Whether or not to use std is a fundamental decision. It might be
difficult to back from std later on. This is something that should be
discussed in more detail.

Do you want to avoid std for maximum flexibility in the future, or are
there QEMU use cases today where std is unavailable?

>
> 2. Rust dependencies
>    We could go without them completely. I chose deliberately to include
>    one dependency in my UART implementation, `bilge`[0], because it has
>    an elegant way of representing typed bitfields for the UART's
>    registers.
>
> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>      Crates.io page: https://crates.io/crates/bilge
>      Repository: https://github.com/hecatia-elegua/bilge

I guess there will be interest in using rust-vmm crates in some way.

Bindings to platform features that are not available in core or std
will also be desirable. We probably don't want to reinvent them.

>
> Should QEMU use third-party dependencies?
> -----------------------------------------
> [shouldqemuusethirdparty] Back to [TOC]
>
> In my personal opinion, if we need a dependency we need a strong
> argument for it. A dependency needs a trusted upstream source, a QEMU
> maintainer to make sure it us up-to-date in QEMU etc.
>
> We already fetch some projects with meson subprojects, so this is not a
> new reality. Cargo allows you to define "locked" dependencies which is
> the same as only fetching specific commits by SHA. No suspicious
> tarballs, and no disappearing dependencies a la left-pad in npm.
>
> However, I believe it's worth considering vendoring every dependency by
> default, if they prove to be few, for the sake of having a local QEMU
> git clone buildable without network access.

Do you mean vendoring by committing them to qemu.git or just the
practice of running `cargo vendor` locally for users who decide they
want to keep a copy of the dependencies?

>
> Should QEMU provide wrapping Rust APIs over QEMU internals?
> -----------------------------------------------------------
> [qemuprovidewrappingrustapis] Back to [TOC]
>
> My personal opinion is no, with the reasoning being that QEMU internals
> are not documented or stable. However I do not see why creating stable
> opt-in interfaces is bad. It just needs someone to volunteer to maintain
> it and ensure there are no breakages through versions.

Rust code will need to interface with QEMU's C APIs, so Rust wrappers
seem unavoidable. Using a protocol like vhost-user might be possible
in some cases. It separates the two codebases so they can both be
native and without bindings, but that won't work for all parts of the
QEMU source tree.

Stable APIs aren't necessary if most developers in the QEMU community
are willing to work in both languages. They can adjust both C and Rust
code when making changes to APIs. I find this preferable to having
Rust maintainers whose job is to keep wrappers up-to-date. Those Rust
maintainers would probably burn out. This seems like a question of
which approach the developer community is comfortable with.

>
> Will QEMU now depend on Rust and thus not build on my XYZ platform?
> -------------------------------------------------------------------
> [qemudependonrustnotbuildonxyz] Back to [TOC]
>
> No, worry about this in some years if this experiment takes off. Rust
> has broad platform support and is present in most distro package
> managers. In the future we might have gcc support for it as well.
>
> For now, Rust will have an experimental status, and will be aimed to
> those who wish to try it. I leave it to the project leaders to make
> proper decisions and statements on this if necessary.

This can be discussed in a separate email thread if you prefer, but I
do think it needs agreement soon so that people have the confidence to
invest their time in writing Rust. They need to know that the code
they develop will be available on most platforms where QEMU is
available and that others in the community won't object or insist on a
C implementation for platform support reasons.

>
>
> How is the compilation structured?
> ==================================
> [howisthecompilationstructured] Back to [TOC]
>
> First, a meson target that runs `bindgen` on a bunch of header files
> (defined in `rust/wrapper.h`) is created as a target and as a dependency
> for any rust hardware device that needs it. You can see the generated
> bindings by running
>
>   ninja generated.rs
>
> inside your build directory.
>
> The devices are defined as dictionaries in rust/meson.build because they
> depend on the bindgen dependency, which is available much later in the
> meson process (when the static qemu lib and target emulator executables
> are defined).
>
> A cargo wrapper python script under scripts/ exists to build the crate
> library, by providing the path to the generated.rs bindings via the
> environment. Then, the qemu-system-aarch64 binary links against the
> staticlib archive (i.e. libpl011.a)
>
> The generated.rs rust file includes a bunch of junk definitions?
> ================================================================
> [generatedrsincludesjunk] Back to [TOC]
>
> Yes, bindgen allows you to block certain types and identifiers from
> being generated but they are simply too many. I have trimmed some of the
> fat but vast improvements can be made.
>
> The staticlib artifact contains a bunch of mangled .o objects?
> ==============================================================
> [staticlibmangledobjects] Back to [TOC]
>
> Yes, until we compile without the `std` module library or we compile it
> manually instead of linking it, we will have some junk in it.

What is the consequence of this? As long as the linker is bringing in
.o files from the .a only through symbol dependencies, then unused .o
files in the .a won't be linked into the final QEMU binary.

>
> --
>
> Manos Pitsidianakis (6):
>   build-sys: Add rust feature option
>   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
>   rust: add bindgen step as a meson dependency
>   DO NOT MERGE: update rustdoc gitlab pages gen
>
>  .gitignore                     |   2 +
>  .gitlab-ci.d/buildtest.yml     |  64 ++--
>  configure                      |  12 +
>  hw/arm/virt.c                  |   2 +-
>  meson.build                    |  99 ++++++
>  meson_options.txt              |   4 +
>  rust/meson.build               |  93 ++++++
>  rust/pl011/.cargo/config.toml  |   2 +
>  rust/pl011/.gitignore          |   2 +
>  rust/pl011/Cargo.lock          | 120 +++++++
>  rust/pl011/Cargo.toml          |  26 ++
>  rust/pl011/README.md           |  42 +++
>  rust/pl011/build.rs            |  44 +++
>  rust/pl011/meson.build         |   7 +
>  rust/pl011/rustfmt.toml        |  10 +
>  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          | 575 +++++++++++++++++++++++++++++++++
>  rust/pl011/src/memory_ops.rs   |  38 +++
>  rust/wrapper.h                 |  39 +++
>  scripts/cargo_wrapper.py       | 221 +++++++++++++
>  scripts/meson-buildoptions.sh  |   5 +
>  24 files changed, 2113 insertions(+), 20 deletions(-)
>  create mode 100644 rust/meson.build
>  create mode 100644 rust/pl011/.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/meson.build
>  create mode 100644 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/wrapper.h
>  create mode 100644 scripts/cargo_wrapper.py
>
>
> base-commit: 01782d6b294f95bcde334386f0aaac593cd28c0d
> --
> γαῖα πυρί μιχθήτω
>
>

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Mon, 10 Jun 2024 22:59, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> What are the issues with not using the compiler, rustc, directly?
>> -----------------------------------------------------------------
>> [whataretheissueswith] Back to [TOC]
>>
>> 1. Tooling
>>    Mostly writing up the build-sys tooling to do so. Ideally we'd
>>    compile everything without cargo but rustc directly.
>
>Why would that be ideal?

It remove the indirection level of meson<->cargo<->rustc. I don't have a 
concrete idea on how to tackle this, but if cargo ends up not strictly 
necessary, I don't see why we cannot use one build system.

>
>>
>>    If we decide we need Rust's `std` library support, we could
>>    investigate whether building it from scratch is a good solution. This
>>    will only build the bits we need in our devices.
>
>Whether or not to use std is a fundamental decision. It might be
>difficult to back from std later on. This is something that should be
>discussed in more detail.
>
>Do you want to avoid std for maximum flexibility in the future, or are
>there QEMU use cases today where std is unavailable?

For flexibility, and for being compatible with more versions.

But I do not want to avoid it, what I am saying is we can do a custom 
build of it instead of linking to the rust toolchain's prebuilt version.

>
>>
>> 2. Rust dependencies
>>    We could go without them completely. I chose deliberately to include
>>    one dependency in my UART implementation, `bilge`[0], because it has
>>    an elegant way of representing typed bitfields for the UART's
>>    registers.
>>
>> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>>      Crates.io page: https://crates.io/crates/bilge
>>      Repository: https://github.com/hecatia-elegua/bilge
>
>I guess there will be interest in using rust-vmm crates in some way.
>
>Bindings to platform features that are not available in core or std
>will also be desirable. We probably don't want to reinvent them.


Agreed.

>
>>
>> Should QEMU use third-party dependencies?
>> -----------------------------------------
>> [shouldqemuusethirdparty] Back to [TOC]
>>
>> In my personal opinion, if we need a dependency we need a strong
>> argument for it. A dependency needs a trusted upstream source, a QEMU
>> maintainer to make sure it us up-to-date in QEMU etc.
>>
>> We already fetch some projects with meson subprojects, so this is not a
>> new reality. Cargo allows you to define "locked" dependencies which is
>> the same as only fetching specific commits by SHA. No suspicious
>> tarballs, and no disappearing dependencies a la left-pad in npm.
>>
>> However, I believe it's worth considering vendoring every dependency by
>> default, if they prove to be few, for the sake of having a local QEMU
>> git clone buildable without network access.
>
>Do you mean vendoring by committing them to qemu.git or just the
>practice of running `cargo vendor` locally for users who decide they
>want to keep a copy of the dependencies?


Committing, with an option to opt-out. They are generally not big in 
size. I am not of strong opinion on this one, I'm very open to 
alternatives.


>>
>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>> -----------------------------------------------------------
>> [qemuprovidewrappingrustapis] Back to [TOC]
>>
>> My personal opinion is no, with the reasoning being that QEMU internals
>> are not documented or stable. However I do not see why creating stable
>> opt-in interfaces is bad. It just needs someone to volunteer to maintain
>> it and ensure there are no breakages through versions.
>
>Rust code will need to interface with QEMU's C APIs, so Rust wrappers
>seem unavoidable. Using a protocol like vhost-user might be possible
>in some cases. It separates the two codebases so they can both be
>native and without bindings, but that won't work for all parts of the
>QEMU source tree.
>
>Stable APIs aren't necessary if most developers in the QEMU community
>are willing to work in both languages. They can adjust both C and Rust
>code when making changes to APIs. I find this preferable to having
>Rust maintainers whose job is to keep wrappers up-to-date. Those Rust
>maintainers would probably burn out. This seems like a question of
>which approach the developer community is comfortable with.


Me too.

>
>>
>> Will QEMU now depend on Rust and thus not build on my XYZ platform?
>> -------------------------------------------------------------------
>> [qemudependonrustnotbuildonxyz] Back to [TOC]
>>
>> No, worry about this in some years if this experiment takes off. Rust
>> has broad platform support and is present in most distro package
>> managers. In the future we might have gcc support for it as well.
>>
>> For now, Rust will have an experimental status, and will be aimed to
>> those who wish to try it. I leave it to the project leaders to make
>> proper decisions and statements on this if necessary.
>
>This can be discussed in a separate email thread if you prefer, but I
>do think it needs agreement soon so that people have the confidence to
>invest their time in writing Rust. They need to know that the code
>they develop will be available on most platforms where QEMU is
>available and that others in the community won't object or insist on a
>C implementation for platform support reasons.

Definitely, also it's out of scope for this RFC since we're not writing 
and rules/guidelines yet.

>
>>
>>
>> How is the compilation structured?
>> ==================================
>> [howisthecompilationstructured] Back to [TOC]
>>
>> First, a meson target that runs `bindgen` on a bunch of header files
>> (defined in `rust/wrapper.h`) is created as a target and as a dependency
>> for any rust hardware device that needs it. You can see the generated
>> bindings by running
>>
>>   ninja generated.rs
>>
>> inside your build directory.
>>
>> The devices are defined as dictionaries in rust/meson.build because they
>> depend on the bindgen dependency, which is available much later in the
>> meson process (when the static qemu lib and target emulator executables
>> are defined).
>>
>> A cargo wrapper python script under scripts/ exists to build the crate
>> library, by providing the path to the generated.rs bindings via the
>> environment. Then, the qemu-system-aarch64 binary links against the
>> staticlib archive (i.e. libpl011.a)
>>
>> The generated.rs rust file includes a bunch of junk definitions?
>> ================================================================
>> [generatedrsincludesjunk] Back to [TOC]
>>
>> Yes, bindgen allows you to block certain types and identifiers from
>> being generated but they are simply too many. I have trimmed some of the
>> fat but vast improvements can be made.
>>
>> The staticlib artifact contains a bunch of mangled .o objects?
>> ==============================================================
>> [staticlibmangledobjects] Back to [TOC]
>>
>> Yes, until we compile without the `std` module library or we compile it
>> manually instead of linking it, we will have some junk in it.
>
>What is the consequence of this? As long as the linker is bringing in
>.o files from the .a only through symbol dependencies, then unused .o
>files in the .a won't be linked into the final QEMU binary.

No consequence, I just want to warn anyone peeking into the rust output 
(not the final qemu binary) to expect junk.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>Hello Manos,
>
>On 6/10/24 11:22, Manos Pitsidianakis wrote:
>> Hello everyone,
>> 
>> This is an early draft of my work on implementing a very simple device,
>> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>> and is used in hw/arm/virt.c).
>> 
>> The device is functional, with copied logic from the C code but with
>> effort not to make a direct C to Rust translation. In other words, do
>> not write Rust as a C developer would.
>> 
>> That goal is not complete but a best-effort case. To give a specific
>> example, register values are typed but interrupt bit flags are not (but
>> could be). I will leave such minutiae for later iterations.
>> 
>> By the way, the wiki page for Rust was revived to keep track of all
>> current series on the mailing list https://wiki.qemu.org/RustInQemu
>> 
>> a #qemu-rust IRC channel was also created for rust-specific discussion
>> that might flood #qemu
>> 
>
>Excellent work, and thanks for posting this RFC!
>
>IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing 
>(temporarily) the generated.rs file in patches is not a good move.
>Any reason you kept it this way?

That was my first approach, I will rework it on the second version. The 
generated code should not exist in committed code at all.

It was initally tricky setting up the dependency orders correctly, so I 
first committed it and then made it a dependency.

>
>Maybe it could be better if build.rs file was *not* needed for new 
>devices/folders, and could be abstracted as a detail of the python 
>wrapper script instead of something that should be committed.


That'd mean you cannot work on the rust files with a LanguageServer, you 
cannot run cargo build or cargo check or cargo clippy, etc. That's why I 
left the alternative choice of including a manually generated bindings 
file (generated.rs.inc)

>
>Having a simple rust/pl011/meson.build is nice and good taste!
>
>> ------------------------------------------------------------------------
>> A request: keep comments to Rust in relation to the QEMU project and no
>> debates on the merits of the language itself. These are valid concerns,
>> but it'd be better if they were on separate mailing list threads.
>> ------------------------------------------------------------------------
>> 
>> Table of contents: [TOC]
>> 
>> - How can I try it? [howcanItryit]
>> - What are the most important points to focus on, at this point?
>>    [whatarethemostimportant]
>>    - What are the issues with not using the compiler, rustc, directly?
>>      [whataretheissueswith]
>>      1. Tooling
>>      2. Rust dependencies
>> 
>>    - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
>>    - Should QEMU provide wrapping Rust APIs over QEMU internals?
>>      [qemuprovidewrappingrustapis]
>>    - Will QEMU now depend on Rust and thus not build on my XYZ platform?
>>      [qemudependonrustnotbuildonxyz]
>> - How is the compilation structured? [howisthecompilationstructured]
>> - The generated.rs rust file includes a bunch of junk definitions?
>>    [generatedrsincludesjunk]
>> - The staticlib artifact contains a bunch of mangled .o objects?
>>    [staticlibmangledobjects]
>> 
>> How can I try it?
>> =================
>> [howcanItryit] Back to [TOC]
>> 
>> Hopefully applying this patches (or checking out `master` branch from
>> https://gitlab.com/epilys/rust-for-qemu/ current commit
>> de81929e0e9d470deac2c6b449b7a5183325e7ee )
>> 
>> Tag for this RFC is rust-pl011-rfc-v1
>> 
>> Rustdoc documentation is hosted on
>> 
>> https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
>> 
>> If `cargo` and `bindgen` is installed in your system, you should be able
>> to build qemu-system-aarch64 with configure flag --enable-rust and
>> launch an arm virt VM. One of the patches hardcodes the default UART of
>> the machine to the Rust one, so if something goes wrong you will see it
>> upon launching qemu-system-aarch64.
>> 
>> To confirm it is there for sure, run e.g. info qom-tree on the monitor
>> and look for x-pl011-rust.
>> 
>> 
>> What are the most important points to focus on, at this point?
>> ==============================================================
>> [whatarethemostimportant] Back to [TOC]
>> 
>> In my opinion, integration of the go-to Rust build system (Cargo and
>> crates.io) with the build system we use in QEMU. This is "easily" done
>> in some definition of the word with a python wrapper script.
>> 
>> What are the issues with not using the compiler, rustc, directly?
>> -----------------------------------------------------------------
>> [whataretheissueswith] Back to [TOC]
>> 
>> 1. Tooling
>>     Mostly writing up the build-sys tooling to do so. Ideally we'd
>>     compile everything without cargo but rustc directly.
>> 
>>     If we decide we need Rust's `std` library support, we could
>>     investigate whether building it from scratch is a good solution. This
>>     will only build the bits we need in our devices.
>>  > 2. Rust dependencies
>>     We could go without them completely. I chose deliberately to include
>>     one dependency in my UART implementation, `bilge`[0], because it has
>>     an elegant way of representing typed bitfields for the UART's
>>     registers.
>> 
>> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>>       Crates.io page: https://crates.io/crates/bilge
>>       Repository: https://github.com/hecatia-elegua/bilge
>> 
>> Should QEMU use third-party dependencies?
>> -----------------------------------------
>> [shouldqemuusethirdparty] Back to [TOC]
>> 
>> In my personal opinion, if we need a dependency we need a strong
>> argument for it. A dependency needs a trusted upstream source, a QEMU
>> maintainer to make sure it us up-to-date in QEMU etc.
>> 
>> We already fetch some projects with meson subprojects, so this is not a
>> new reality. Cargo allows you to define "locked" dependencies which is
>> the same as only fetching specific commits by SHA. No suspicious
>> tarballs, and no disappearing dependencies a la left-pad in npm.
>>
>
>As a complement to this, and for other readers, in more than having a 
>lock file (fixing version you use), cargo crates system is designed to 
>be immutable (see: https://crates.io/policies), and it means there is a 
>strong guarantee that a published package will stay there, to the 
>opposite of npm, pypi, or most of other similar systems.
>
>"Crate deletion by their owners is not possible to keep the registry as 
>immutable as possible."
>
>I believe this is a *key* feature of Rust ecosystem and should be 
>emphasized regarding the policy for Rust dependencies to come.
>
>> However, I believe it's worth considering vendoring every dependency by
>> default, if they prove to be few, for the sake of having a local QEMU
>> git clone buildable without network access.
>>
>
>I would not be in favor to vendor all dependencies. Beyond the "offline 
>build" scenario, it has only downsides.
>
>Sure, we should really debate before introducing a new dependency, but 
>the technical difficulty to mirror its sources and dependencies should 
>not be an argument for or against it.
>
>What will happen the day we want to introduce something bigger than a 
>simple dependency? (let's say "serde" for instance)


Yes, vendor-the-world is a different topic than vendor e.g. two crates 
such as the dependencies I'm using here.

>
>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>> -----------------------------------------------------------
>> [qemuprovidewrappingrustapis] Back to [TOC]
>> 
>> My personal opinion is no, with the reasoning being that QEMU internals
>> are not documented or stable. However I do not see why creating stable
>> opt-in interfaces is bad. It just needs someone to volunteer to maintain
>> it and ensure there are no breakages through versions.
>> 
>> Will QEMU now depend on Rust and thus not build on my XYZ platform?
>> -------------------------------------------------------------------
>> [qemudependonrustnotbuildonxyz] Back to [TOC]
>> 
>> No, worry about this in some years if this experiment takes off. Rust
>> has broad platform support and is present in most distro package
>> managers. In the future we might have gcc support for it as well.
>> 
>> For now, Rust will have an experimental status, and will be aimed to
>> those who wish to try it. I leave it to the project leaders to make
>> proper decisions and statements on this if necessary.
>> 
>> 
>> How is the compilation structured?
>> ==================================
>> [howisthecompilationstructured] Back to [TOC]
>> 
>> First, a meson target that runs `bindgen` on a bunch of header files
>> (defined in `rust/wrapper.h`) is created as a target and as a dependency
>> for any rust hardware device that needs it. You can see the generated
>> bindings by running
>> 
>>    ninja generated.rs
>> 
>> inside your build directory.
>> 
>> The devices are defined as dictionaries in rust/meson.build because they
>> depend on the bindgen dependency, which is available much later in the
>> meson process (when the static qemu lib and target emulator executables
>> are defined).
>> 
>> A cargo wrapper python script under scripts/ exists to build the crate
>> library, by providing the path to the generated.rs bindings via the
>> environment. Then, the qemu-system-aarch64 binary links against the
>> staticlib archive (i.e. libpl011.a)
>> 
>> The generated.rs rust file includes a bunch of junk definitions?
>> ================================================================
>> [generatedrsincludesjunk] Back to [TOC]
>> 
>> Yes, bindgen allows you to block certain types and identifiers from
>> being generated but they are simply too many. I have trimmed some of the
>> fat but vast improvements can be made.
>> 
>> The staticlib artifact contains a bunch of mangled .o objects?
>> ==============================================================
>> [staticlibmangledobjects] Back to [TOC]
>> 
>> Yes, until we compile without the `std` module library or we compile it
>> manually instead of linking it, we will have some junk in it.
>> 
>
>Besides the size aspect, which potential advantage would there be to 
>switch to no_std?
>We don't build a bare metal or kernel binary here, so why introduce this 
>restriction willingly?

We'll see that as we progress. Might enable more platform support, for 
example. I have no definite answers here. Also, I know binary bloat is a 
big complaint from people with dislike of Rust, so I pre-emptively 
addressed it.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Stefan Hajnoczi]

On Mon, 10 Jun 2024 at 16:27, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Mon, 10 Jun 2024 22:59, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >> What are the issues with not using the compiler, rustc, directly?
> >> -----------------------------------------------------------------
> >> [whataretheissueswith] Back to [TOC]
> >>
> >> 1. Tooling
> >>    Mostly writing up the build-sys tooling to do so. Ideally we'd
> >>    compile everything without cargo but rustc directly.
> >
> >Why would that be ideal?
>
> It remove the indirection level of meson<->cargo<->rustc. I don't have a
> concrete idea on how to tackle this, but if cargo ends up not strictly
> necessary, I don't see why we cannot use one build system.

The convenience of being able to use cargo dependencies without
special QEMU meson build system effort seems worth the overhead of
meson<->cargo<->rustc to me. There is a blog post that explores using
cargo crates using meson's wrap dependencies here, and it seems like
extra work:
https://coaxion.net/blog/2023/04/building-a-gstreamer-plugin-in-rust-with-meson-instead-of-cargo/

It's possible to use just meson today, but I don't think it's
practical when using cargo dependencies.

>
> >
> >>
> >>    If we decide we need Rust's `std` library support, we could
> >>    investigate whether building it from scratch is a good solution. This
> >>    will only build the bits we need in our devices.
> >
> >Whether or not to use std is a fundamental decision. It might be
> >difficult to back from std later on. This is something that should be
> >discussed in more detail.
> >
> >Do you want to avoid std for maximum flexibility in the future, or are
> >there QEMU use cases today where std is unavailable?
>
> For flexibility, and for being compatible with more versions.
>
> But I do not want to avoid it, what I am saying is we can do a custom
> build of it instead of linking to the rust toolchain's prebuilt version.

What advantages does a custom build of std bring?

>
> >
> >>
> >> 2. Rust dependencies
> >>    We could go without them completely. I chose deliberately to include
> >>    one dependency in my UART implementation, `bilge`[0], because it has
> >>    an elegant way of representing typed bitfields for the UART's
> >>    registers.
> >>
> >> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
> >>      Crates.io page: https://crates.io/crates/bilge
> >>      Repository: https://github.com/hecatia-elegua/bilge
> >
> >I guess there will be interest in using rust-vmm crates in some way.
> >
> >Bindings to platform features that are not available in core or std
> >will also be desirable. We probably don't want to reinvent them.
>
>
> Agreed.
>
> >
> >>
> >> Should QEMU use third-party dependencies?
> >> -----------------------------------------
> >> [shouldqemuusethirdparty] Back to [TOC]
> >>
> >> In my personal opinion, if we need a dependency we need a strong
> >> argument for it. A dependency needs a trusted upstream source, a QEMU
> >> maintainer to make sure it us up-to-date in QEMU etc.
> >>
> >> We already fetch some projects with meson subprojects, so this is not a
> >> new reality. Cargo allows you to define "locked" dependencies which is
> >> the same as only fetching specific commits by SHA. No suspicious
> >> tarballs, and no disappearing dependencies a la left-pad in npm.
> >>
> >> However, I believe it's worth considering vendoring every dependency by
> >> default, if they prove to be few, for the sake of having a local QEMU
> >> git clone buildable without network access.
> >
> >Do you mean vendoring by committing them to qemu.git or just the
> >practice of running `cargo vendor` locally for users who decide they
> >want to keep a copy of the dependencies?
>
>
> Committing, with an option to opt-out. They are generally not big in
> size. I am not of strong opinion on this one, I'm very open to
> alternatives.

Fedora and Debian want Rust applications to use distro-packaged
crates. No vendoring and no crates.io online access. It's a bit of a
pain because Rust developers need to make sure their code works with
whatever version of crates Fedora and Debian provide.

The `cargo vendor` command makes it easy for anyone wishing to collect
the required dependencies for offline builds (something I've used for
CentOS builds where vendoring is allowed).

I suggest not vendoring packages in qemu.git. Users can still run
`cargo vendor` for easy offline builds.

>
>
> >>
> >> Should QEMU provide wrapping Rust APIs over QEMU internals?
> >> -----------------------------------------------------------
> >> [qemuprovidewrappingrustapis] Back to [TOC]
> >>
> >> My personal opinion is no, with the reasoning being that QEMU internals
> >> are not documented or stable. However I do not see why creating stable
> >> opt-in interfaces is bad. It just needs someone to volunteer to maintain
> >> it and ensure there are no breakages through versions.
> >
> >Rust code will need to interface with QEMU's C APIs, so Rust wrappers
> >seem unavoidable. Using a protocol like vhost-user might be possible
> >in some cases. It separates the two codebases so they can both be
> >native and without bindings, but that won't work for all parts of the
> >QEMU source tree.
> >
> >Stable APIs aren't necessary if most developers in the QEMU community
> >are willing to work in both languages. They can adjust both C and Rust
> >code when making changes to APIs. I find this preferable to having
> >Rust maintainers whose job is to keep wrappers up-to-date. Those Rust
> >maintainers would probably burn out. This seems like a question of
> >which approach the developer community is comfortable with.
>
>
> Me too.
>
> >
> >>
> >> Will QEMU now depend on Rust and thus not build on my XYZ platform?
> >> -------------------------------------------------------------------
> >> [qemudependonrustnotbuildonxyz] Back to [TOC]
> >>
> >> No, worry about this in some years if this experiment takes off. Rust
> >> has broad platform support and is present in most distro package
> >> managers. In the future we might have gcc support for it as well.
> >>
> >> For now, Rust will have an experimental status, and will be aimed to
> >> those who wish to try it. I leave it to the project leaders to make
> >> proper decisions and statements on this if necessary.
> >
> >This can be discussed in a separate email thread if you prefer, but I
> >do think it needs agreement soon so that people have the confidence to
> >invest their time in writing Rust. They need to know that the code
> >they develop will be available on most platforms where QEMU is
> >available and that others in the community won't object or insist on a
> >C implementation for platform support reasons.
>
> Definitely, also it's out of scope for this RFC since we're not writing
> and rules/guidelines yet.
>
> >
> >>
> >>
> >> How is the compilation structured?
> >> ==================================
> >> [howisthecompilationstructured] Back to [TOC]
> >>
> >> First, a meson target that runs `bindgen` on a bunch of header files
> >> (defined in `rust/wrapper.h`) is created as a target and as a dependency
> >> for any rust hardware device that needs it. You can see the generated
> >> bindings by running
> >>
> >>   ninja generated.rs
> >>
> >> inside your build directory.
> >>
> >> The devices are defined as dictionaries in rust/meson.build because they
> >> depend on the bindgen dependency, which is available much later in the
> >> meson process (when the static qemu lib and target emulator executables
> >> are defined).
> >>
> >> A cargo wrapper python script under scripts/ exists to build the crate
> >> library, by providing the path to the generated.rs bindings via the
> >> environment. Then, the qemu-system-aarch64 binary links against the
> >> staticlib archive (i.e. libpl011.a)
> >>
> >> The generated.rs rust file includes a bunch of junk definitions?
> >> ================================================================
> >> [generatedrsincludesjunk] Back to [TOC]
> >>
> >> Yes, bindgen allows you to block certain types and identifiers from
> >> being generated but they are simply too many. I have trimmed some of the
> >> fat but vast improvements can be made.
> >>
> >> The staticlib artifact contains a bunch of mangled .o objects?
> >> ==============================================================
> >> [staticlibmangledobjects] Back to [TOC]
> >>
> >> Yes, until we compile without the `std` module library or we compile it
> >> manually instead of linking it, we will have some junk in it.
> >
> >What is the consequence of this? As long as the linker is bringing in
> >.o files from the .a only through symbol dependencies, then unused .o
> >files in the .a won't be linked into the final QEMU binary.
>
> No consequence, I just want to warn anyone peeking into the rust output
> (not the final qemu binary) to expect junk.

Okay, cool!

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Pierrick Bouvier]

On 6/10/24 13:29, Manos Pitsidianakis wrote:
> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> Hello Manos,
>>
>> On 6/10/24 11:22, Manos Pitsidianakis wrote:
>>> Hello everyone,
>>>
>>> This is an early draft of my work on implementing a very simple device,
>>> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>>> and is used in hw/arm/virt.c).
>>>
>>> The device is functional, with copied logic from the C code but with
>>> effort not to make a direct C to Rust translation. In other words, do
>>> not write Rust as a C developer would.
>>>
>>> That goal is not complete but a best-effort case. To give a specific
>>> example, register values are typed but interrupt bit flags are not (but
>>> could be). I will leave such minutiae for later iterations.
>>>
>>> By the way, the wiki page for Rust was revived to keep track of all
>>> current series on the mailing list https://wiki.qemu.org/RustInQemu
>>>
>>> a #qemu-rust IRC channel was also created for rust-specific discussion
>>> that might flood #qemu
>>>
>>
>> Excellent work, and thanks for posting this RFC!
>>
>> IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing
>> (temporarily) the generated.rs file in patches is not a good move.
>> Any reason you kept it this way?
> 
> That was my first approach, I will rework it on the second version. The
> generated code should not exist in committed code at all.
> 
> It was initally tricky setting up the dependency orders correctly, so I
> first committed it and then made it a dependency.
> 
>>
>> Maybe it could be better if build.rs file was *not* needed for new
>> devices/folders, and could be abstracted as a detail of the python
>> wrapper script instead of something that should be committed.
> 
> 
> That'd mean you cannot work on the rust files with a LanguageServer, you
> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
> left the alternative choice of including a manually generated bindings
> file (generated.rs.inc)
> 

Maybe I missed something, but it seems like it just checks/copies the 
generated.rs file where it's expected. Definitely something that could 
be done as part of the rust build.

Having to run the build before getting completion does not seem to be a 
huge compromise.

>>
>> Having a simple rust/pl011/meson.build is nice and good taste!
>>
>>> ------------------------------------------------------------------------
>>> A request: keep comments to Rust in relation to the QEMU project and no
>>> debates on the merits of the language itself. These are valid concerns,
>>> but it'd be better if they were on separate mailing list threads.
>>> ------------------------------------------------------------------------
>>>
>>> Table of contents: [TOC]
>>>
>>> - How can I try it? [howcanItryit]
>>> - What are the most important points to focus on, at this point?
>>>     [whatarethemostimportant]
>>>     - What are the issues with not using the compiler, rustc, directly?
>>>       [whataretheissueswith]
>>>       1. Tooling
>>>       2. Rust dependencies
>>>
>>>     - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
>>>     - Should QEMU provide wrapping Rust APIs over QEMU internals?
>>>       [qemuprovidewrappingrustapis]
>>>     - Will QEMU now depend on Rust and thus not build on my XYZ platform?
>>>       [qemudependonrustnotbuildonxyz]
>>> - How is the compilation structured? [howisthecompilationstructured]
>>> - The generated.rs rust file includes a bunch of junk definitions?
>>>     [generatedrsincludesjunk]
>>> - The staticlib artifact contains a bunch of mangled .o objects?
>>>     [staticlibmangledobjects]
>>>
>>> How can I try it?
>>> =================
>>> [howcanItryit] Back to [TOC]
>>>
>>> Hopefully applying this patches (or checking out `master` branch from
>>> https://gitlab.com/epilys/rust-for-qemu/ current commit
>>> de81929e0e9d470deac2c6b449b7a5183325e7ee )
>>>
>>> Tag for this RFC is rust-pl011-rfc-v1
>>>
>>> Rustdoc documentation is hosted on
>>>
>>> https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
>>>
>>> If `cargo` and `bindgen` is installed in your system, you should be able
>>> to build qemu-system-aarch64 with configure flag --enable-rust and
>>> launch an arm virt VM. One of the patches hardcodes the default UART of
>>> the machine to the Rust one, so if something goes wrong you will see it
>>> upon launching qemu-system-aarch64.
>>>
>>> To confirm it is there for sure, run e.g. info qom-tree on the monitor
>>> and look for x-pl011-rust.
>>>
>>>
>>> What are the most important points to focus on, at this point?
>>> ==============================================================
>>> [whatarethemostimportant] Back to [TOC]
>>>
>>> In my opinion, integration of the go-to Rust build system (Cargo and
>>> crates.io) with the build system we use in QEMU. This is "easily" done
>>> in some definition of the word with a python wrapper script.
>>>
>>> What are the issues with not using the compiler, rustc, directly?
>>> -----------------------------------------------------------------
>>> [whataretheissueswith] Back to [TOC]
>>>
>>> 1. Tooling
>>>      Mostly writing up the build-sys tooling to do so. Ideally we'd
>>>      compile everything without cargo but rustc directly.
>>>
>>>      If we decide we need Rust's `std` library support, we could
>>>      investigate whether building it from scratch is a good solution. This
>>>      will only build the bits we need in our devices.
>>>   > 2. Rust dependencies
>>>      We could go without them completely. I chose deliberately to include
>>>      one dependency in my UART implementation, `bilge`[0], because it has
>>>      an elegant way of representing typed bitfields for the UART's
>>>      registers.
>>>
>>> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>>>        Crates.io page: https://crates.io/crates/bilge
>>>        Repository: https://github.com/hecatia-elegua/bilge
>>>
>>> Should QEMU use third-party dependencies?
>>> -----------------------------------------
>>> [shouldqemuusethirdparty] Back to [TOC]
>>>
>>> In my personal opinion, if we need a dependency we need a strong
>>> argument for it. A dependency needs a trusted upstream source, a QEMU
>>> maintainer to make sure it us up-to-date in QEMU etc.
>>>
>>> We already fetch some projects with meson subprojects, so this is not a
>>> new reality. Cargo allows you to define "locked" dependencies which is
>>> the same as only fetching specific commits by SHA. No suspicious
>>> tarballs, and no disappearing dependencies a la left-pad in npm.
>>>
>>
>> As a complement to this, and for other readers, in more than having a
>> lock file (fixing version you use), cargo crates system is designed to
>> be immutable (see: https://crates.io/policies), and it means there is a
>> strong guarantee that a published package will stay there, to the
>> opposite of npm, pypi, or most of other similar systems.
>>
>> "Crate deletion by their owners is not possible to keep the registry as
>> immutable as possible."
>>
>> I believe this is a *key* feature of Rust ecosystem and should be
>> emphasized regarding the policy for Rust dependencies to come.
>>
>>> However, I believe it's worth considering vendoring every dependency by
>>> default, if they prove to be few, for the sake of having a local QEMU
>>> git clone buildable without network access.
>>>
>>
>> I would not be in favor to vendor all dependencies. Beyond the "offline
>> build" scenario, it has only downsides.
>>
>> Sure, we should really debate before introducing a new dependency, but
>> the technical difficulty to mirror its sources and dependencies should
>> not be an argument for or against it.
>>
>> What will happen the day we want to introduce something bigger than a
>> simple dependency? (let's say "serde" for instance)
> 
> 
> Yes, vendor-the-world is a different topic than vendor e.g. two crates
> such as the dependencies I'm using here.
>

If there must be a discussion about dependencies, it's probably better 
to consider the "worse" case to take a decison about vendoring this or not.

>>
>>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>>> -----------------------------------------------------------
>>> [qemuprovidewrappingrustapis] Back to [TOC]
>>>
>>> My personal opinion is no, with the reasoning being that QEMU internals
>>> are not documented or stable. However I do not see why creating stable
>>> opt-in interfaces is bad. It just needs someone to volunteer to maintain
>>> it and ensure there are no breakages through versions.
>>>
>>> Will QEMU now depend on Rust and thus not build on my XYZ platform?
>>> -------------------------------------------------------------------
>>> [qemudependonrustnotbuildonxyz] Back to [TOC]
>>>
>>> No, worry about this in some years if this experiment takes off. Rust
>>> has broad platform support and is present in most distro package
>>> managers. In the future we might have gcc support for it as well.
>>>
>>> For now, Rust will have an experimental status, and will be aimed to
>>> those who wish to try it. I leave it to the project leaders to make
>>> proper decisions and statements on this if necessary.
>>>
>>>
>>> How is the compilation structured?
>>> ==================================
>>> [howisthecompilationstructured] Back to [TOC]
>>>
>>> First, a meson target that runs `bindgen` on a bunch of header files
>>> (defined in `rust/wrapper.h`) is created as a target and as a dependency
>>> for any rust hardware device that needs it. You can see the generated
>>> bindings by running
>>>
>>>     ninja generated.rs
>>>
>>> inside your build directory.
>>>
>>> The devices are defined as dictionaries in rust/meson.build because they
>>> depend on the bindgen dependency, which is available much later in the
>>> meson process (when the static qemu lib and target emulator executables
>>> are defined).
>>>
>>> A cargo wrapper python script under scripts/ exists to build the crate
>>> library, by providing the path to the generated.rs bindings via the
>>> environment. Then, the qemu-system-aarch64 binary links against the
>>> staticlib archive (i.e. libpl011.a)
>>>
>>> The generated.rs rust file includes a bunch of junk definitions?
>>> ================================================================
>>> [generatedrsincludesjunk] Back to [TOC]
>>>
>>> Yes, bindgen allows you to block certain types and identifiers from
>>> being generated but they are simply too many. I have trimmed some of the
>>> fat but vast improvements can be made.
>>>
>>> The staticlib artifact contains a bunch of mangled .o objects?
>>> ==============================================================
>>> [staticlibmangledobjects] Back to [TOC]
>>>
>>> Yes, until we compile without the `std` module library or we compile it
>>> manually instead of linking it, we will have some junk in it.
>>>
>>
>> Besides the size aspect, which potential advantage would there be to
>> switch to no_std?
>> We don't build a bare metal or kernel binary here, so why introduce this
>> restriction willingly?
> 
> We'll see that as we progress. Might enable more platform support, for
> example. I have no definite answers here. Also, I know binary bloat is a
> big complaint from people with dislike of Rust, so I pre-emptively
> addressed it.

I understand.
Stripping rust binary does a big difference in size, including for 
release builds.
I hope we'll explore other options than using no_std if binary size 
should become a decisive criteria.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 00:38, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>>> Maybe it could be better if build.rs file was *not* needed for new 
>>> devices/folders, and could be abstracted as a detail of the python 
>>> wrapper script instead of something that should be committed.
>>
>> That'd mean you cannot work on the rust files with a LanguageServer, 
>> you cannot run cargo build or cargo check or cargo clippy, etc. 
>> That's why I left the alternative choice of including a manually 
>> generated bindings file (generated.rs.inc)
>
>Maybe I missed something, but it seems like it just checks/copies the 
>generated.rs file where it's expected. Definitely something that could 
>be done as part of the rust build.
>
>Having to run the build before getting completion does not seem to be a 
>huge compromise.

It only checks if it's called from meson, hence it should update the 
should choose meson's generated.rs file. Otherwise it falls back to any 
manual bindings you have put there that are not checked into git. So 
essentially it does what you suggest, I think :)


>>
>> Yes, vendor-the-world is a different topic than vendor e.g. two 
>> crates such as the dependencies I'm using here.
>>
>
>If there must be a discussion about dependencies, it's probably better 
>to consider the "worse" case to take a decison about vendoring this or not.
>

Agreed. To re-cap, my opinion is that vendoring 1-2 small crates is 
fine, but any more than that needs rethinking.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
> > > The staticlib artifact contains a bunch of mangled .o objects?
> > > ==============================================================
> > > [staticlibmangledobjects] Back to [TOC]
> > > 
> > > Yes, until we compile without the `std` module library or we compile it
> > > manually instead of linking it, we will have some junk in it.
> > > 
> > 
> > Besides the size aspect, which potential advantage would there be to
> > switch to no_std?
> > We don't build a bare metal or kernel binary here, so why introduce this
> > restriction willingly?
> 
> We'll see that as we progress. Might enable more platform support, for
> example. I have no definite answers here. Also, I know binary bloat is a big
> complaint from people with dislike of Rust, so I pre-emptively addressed it.

Requiring 'no_std' would significantly limit what 3rd party crates QEMU
can make use of, and thus would put more burden on QEMU maintainers.
I don't find "binary bloat" a credible technical argument on its own
either, so certainly not sufficient justification to take on the pain
of 'no_std'.


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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Philippe Mathieu-Daudé]

On 10/6/24 21:59, Stefan Hajnoczi wrote:
> On Mon, 10 Jun 2024 at 14:23, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:


>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>> -----------------------------------------------------------
>> [qemuprovidewrappingrustapis] Back to [TOC]
>>
>> My personal opinion is no, with the reasoning being that QEMU internals
>> are not documented or stable. However I do not see why creating stable
>> opt-in interfaces is bad. It just needs someone to volunteer to maintain
>> it and ensure there are no breakages through versions.
> 
> Rust code will need to interface with QEMU's C APIs, so Rust wrappers
> seem unavoidable. Using a protocol like vhost-user might be possible
> in some cases. It separates the two codebases so they can both be
> native and without bindings, but that won't work for all parts of the
> QEMU source tree.
> 
> Stable APIs aren't necessary if most developers in the QEMU community
> are willing to work in both languages. They can adjust both C and Rust
> code when making changes to APIs. I find this preferable to having
> Rust maintainers whose job is to keep wrappers up-to-date. Those Rust
> maintainers would probably burn out. This seems like a question of
> which approach the developer community is comfortable with.

Both APIs must be updated in sync in order to pass CI, so having
the same developer updating both languages seems a requisite.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> What are the issues with not using the compiler, rustc, directly?
> -----------------------------------------------------------------
> [whataretheissueswith] Back to [TOC]
> 
> 1. Tooling
>    Mostly writing up the build-sys tooling to do so. Ideally we'd 
>    compile everything without cargo but rustc directly.
> 
>    If we decide we need Rust's `std` library support, we could 
>    investigate whether building it from scratch is a good solution. This 
>    will only build the bits we need in our devices.

Re-building 'std' for QEMU would be a no-go for many distros who
will expect QEMU to use the distro provided 'std' package. So at
most that would have to be an optional feature.

> 2. Rust dependencies
>    We could go without them completely. I chose deliberately to include 
>    one dependency in my UART implementation, `bilge`[0], because it has 
>    an elegant way of representing typed bitfields for the UART's 
>    registers.
> 
> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>      Crates.io page: https://crates.io/crates/bilge
>      Repository: https://github.com/hecatia-elegua/bilge
> 
> Should QEMU use third-party dependencies?
> -----------------------------------------
> [shouldqemuusethirdparty] Back to [TOC]
> 
> In my personal opinion, if we need a dependency we need a strong 
> argument for it. A dependency needs a trusted upstream source, a QEMU 
> maintainer to make sure it us up-to-date in QEMU etc.

"strong" is a rather fuzzy term. In C we already have a huge number
of build dependencies

 $ wc -l tests/lcitool/projects/qemu.yml 
 127 tests/lcitool/projects/qemu.yml

we would have many more than that except that we're conservative
about adding deps on things because getting new libraries into
distros is quite painful, or we lag behind where we would want
to be to stick with compat for old distro versions.

In terms of Rust dependancies, I'd expect us to have fairly arbitrary
dependancies used. If the dep avoids QEMU maintainers having to
re-invent the wheel for something there is already a common crate
for, then it is a good thing to use it. I'd almost go as far as
encouraging use of external crates. Our maintainers should focus tmie
on writing code that's delivers compelling features to QEMU, rather
than re-creating common APIs that already have good crates.

> We already fetch some projects with meson subprojects, so this is not a 
> new reality. Cargo allows you to define "locked" dependencies which is 
> the same as only fetching specific commits by SHA. No suspicious 
> tarballs, and no disappearing dependencies a la left-pad in npm.
> 
> However, I believe it's worth considering vendoring every dependency by 
> default, if they prove to be few, for the sake of having a local QEMU 
> git clone buildable without network access.

A local git clone is already not buildable without network access,
given that you have to install countless extra distro packages
ahead of time. I think its reasonable to expect people working from
git to have to download rust deps. We should consider whether we
want vendoring in the release tarballs though.

> Should QEMU provide wrapping Rust APIs over QEMU internals?
> -----------------------------------------------------------
> [qemuprovidewrappingrustapis] Back to [TOC]
> 
> My personal opinion is no, with the reasoning being that QEMU internals 
> are not documented or stable. However I do not see why creating stable 
> opt-in interfaces is bad. It just needs someone to volunteer to maintain 
> it and ensure there are no breakages through versions.

I expect this will evolve organically with people providing wrappers
where appropriate to suit their development neds.

> Will QEMU now depend on Rust and thus not build on my XYZ platform?
> -------------------------------------------------------------------
> [qemudependonrustnotbuildonxyz] Back to [TOC]
> 
> No, worry about this in some years if this experiment takes off. Rust 
> has broad platform support and is present in most distro package 
> managers. In the future we might have gcc support for it as well.

Rust isn't going away, so if a platform wants to remain relevant
to the modern software world, then people who care about that
platform need to ensure Rust works on it. I wouldn't say that
QEMU needs to massively worry about this, since all the common
platforms are now covered precisely because Rust is becoming
so wildly used that a platform cannot ignore it.


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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> Hello everyone,
> 
> This is an early draft of my work on implementing a very simple device, 
> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
> and is used in hw/arm/virt.c).

looking at the diffstat:

>  .gitignore                     |   2 +
>  .gitlab-ci.d/buildtest.yml     |  64 ++--
>  configure                      |  12 +
>  hw/arm/virt.c                  |   2 +-
>  meson.build                    |  99 ++++++
>  meson_options.txt              |   4 +
>  rust/meson.build               |  93 ++++++
>  rust/pl011/.cargo/config.toml  |   2 +
>  rust/pl011/.gitignore          |   2 +
>  rust/pl011/Cargo.lock          | 120 +++++++
>  rust/pl011/Cargo.toml          |  26 ++
>  rust/pl011/README.md           |  42 +++
>  rust/pl011/build.rs            |  44 +++
>  rust/pl011/meson.build         |   7 +
>  rust/pl011/rustfmt.toml        |  10 +
>  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          | 575 +++++++++++++++++++++++++++++++++
>  rust/pl011/src/memory_ops.rs   |  38 +++

My thought is that if we're going to start implementing devices
or other parts of QEMU, in Rust, then I do not want to see it
placed in a completely separate directory sub-tree.

In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
so that the device is part of the normal Arm hardware directory structure 
and maintainer assignments.

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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Mon, Jun 10, 2024 at 04:47:33PM -0400, Stefan Hajnoczi wrote:
> On Mon, 10 Jun 2024 at 16:27, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> >
> > On Mon, 10 Jun 2024 22:59, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > >> Should QEMU use third-party dependencies?
> > >> -----------------------------------------
> > >> [shouldqemuusethirdparty] Back to [TOC]
> > >>
> > >> In my personal opinion, if we need a dependency we need a strong
> > >> argument for it. A dependency needs a trusted upstream source, a QEMU
> > >> maintainer to make sure it us up-to-date in QEMU etc.
> > >>
> > >> We already fetch some projects with meson subprojects, so this is not a
> > >> new reality. Cargo allows you to define "locked" dependencies which is
> > >> the same as only fetching specific commits by SHA. No suspicious
> > >> tarballs, and no disappearing dependencies a la left-pad in npm.
> > >>
> > >> However, I believe it's worth considering vendoring every dependency by
> > >> default, if they prove to be few, for the sake of having a local QEMU
> > >> git clone buildable without network access.
> > >
> > >Do you mean vendoring by committing them to qemu.git or just the
> > >practice of running `cargo vendor` locally for users who decide they
> > >want to keep a copy of the dependencies?
> >
> >
> > Committing, with an option to opt-out. They are generally not big in
> > size. I am not of strong opinion on this one, I'm very open to
> > alternatives.
> 
> Fedora and Debian want Rust applications to use distro-packaged
> crates. No vendoring and no crates.io online access. It's a bit of a
> pain because Rust developers need to make sure their code works with
> whatever version of crates Fedora and Debian provide.

NB Fedora isn't actually that strict for Rust.  The "no vendoring"
policy is merely a "SHOULD", rather than a "MUST" requirement:

  https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_vendored_dependencies

which is a more pragmmatic approach to the real world packaging where
there's potentially 100's of deps in an application chain.

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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> > > The staticlib artifact contains a bunch of mangled .o objects?
>> > > ==============================================================
>> > > [staticlibmangledobjects] Back to [TOC]
>> > > 
>> > > Yes, until we compile without the `std` module library or we compile it
>> > > manually instead of linking it, we will have some junk in it.
>> > > 
>> > 
>> > Besides the size aspect, which potential advantage would there be to
>> > switch to no_std?
>> > We don't build a bare metal or kernel binary here, so why introduce this
>> > restriction willingly?
>> 
>> We'll see that as we progress. Might enable more platform support, for
>> example. I have no definite answers here. Also, I know binary bloat is a big
>> complaint from people with dislike of Rust, so I pre-emptively addressed it.
>
> Requiring 'no_std' would significantly limit what 3rd party crates QEMU
> can make use of, and thus would put more burden on QEMU maintainers.
> I don't find "binary bloat" a credible technical argument on its own
> either, so certainly not sufficient justification to take on the pain
> of 'no_std'.

no_std is great for OS's and micro controllers but I don't think its
something we have to worry about for QEMU. One potential area of
co-operation would be the rust-vmm libraries and they definitely take
advantage of the stdlibs.

>
>
> With regards,
> Daniel

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 6/10/24 13:29, Manos Pitsidianakis wrote:
>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>>> Hello Manos,
>>>
<snip>
>>> Excellent work, and thanks for posting this RFC!
>>>
>>> IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing
>>> (temporarily) the generated.rs file in patches is not a good move.
>>> Any reason you kept it this way?
>> That was my first approach, I will rework it on the second version.
>> The
>> generated code should not exist in committed code at all.
>> It was initally tricky setting up the dependency orders correctly,
>> so I
>> first committed it and then made it a dependency.
>> 
>>>
>>> Maybe it could be better if build.rs file was *not* needed for new
>>> devices/folders, and could be abstracted as a detail of the python
>>> wrapper script instead of something that should be committed.
>> That'd mean you cannot work on the rust files with a LanguageServer,
>> you
>> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>> left the alternative choice of including a manually generated bindings
>> file (generated.rs.inc)
>> 
>
> Maybe I missed something, but it seems like it just checks/copies the
> generated.rs file where it's expected. Definitely something that could
> be done as part of the rust build.
>
> Having to run the build before getting completion does not seem to be
> a huge compromise.
>
<snip>

As long as the Language Server can kick in after a first build. Rust
definitely leans in to the concept of the tooling helping you out while
coding.

I think for the C LSPs compile_commands.json is generated during the
configure step but I could be wrong.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Alex Bennée]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Mon, 10 Jun 2024 at 16:27, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
>>
>> On Mon, 10 Jun 2024 22:59, Stefan Hajnoczi <stefanha@gmail.com> wrote:
<snip>
>> >>
>> >> 1. Tooling
>> >>    Mostly writing up the build-sys tooling to do so. Ideally we'd
>> >>    compile everything without cargo but rustc directly.
>> >
>> >Why would that be ideal?
>>
>> It remove the indirection level of meson<->cargo<->rustc. I don't have a
>> concrete idea on how to tackle this, but if cargo ends up not strictly
>> necessary, I don't see why we cannot use one build system.
>
> The convenience of being able to use cargo dependencies without
> special QEMU meson build system effort seems worth the overhead of
> meson<->cargo<->rustc to me. There is a blog post that explores using
> cargo crates using meson's wrap dependencies here, and it seems like
> extra work:
> https://coaxion.net/blog/2023/04/building-a-gstreamer-plugin-in-rust-with-meson-instead-of-cargo/
>
> It's possible to use just meson today, but I don't think it's
> practical when using cargo dependencies.

I did find the wrap approach very useful in giving a useful checkout
with --download that I can edit and tweak but is still integrated into
the build. This is helpful when working with very new libraries that
haven't been widely packaged yet. Distro's can choose or not choose to
use --download as they wish.

<snip>
>> >
>> >Do you mean vendoring by committing them to qemu.git or just the
>> >practice of running `cargo vendor` locally for users who decide they
>> >want to keep a copy of the dependencies?
>>
>>
>> Committing, with an option to opt-out. They are generally not big in
>> size. I am not of strong opinion on this one, I'm very open to
>> alternatives.

I think we generally host stuff we explicitly need in separate mirrors
or even a little light forking (testfloat does this).

> Fedora and Debian want Rust applications to use distro-packaged
> crates. No vendoring and no crates.io online access. It's a bit of a
> pain because Rust developers need to make sure their code works with
> whatever version of crates Fedora and Debian provide.
>
> The `cargo vendor` command makes it easy for anyone wishing to collect
> the required dependencies for offline builds (something I've used for
> CentOS builds where vendoring is allowed).
>
> I suggest not vendoring packages in qemu.git. Users can still run
> `cargo vendor` for easy offline builds.
>
<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Zhao Liu]

On Tue, Jun 11, 2024 at 09:22:44AM +0100, Daniel P. Berrangé wrote:
> Date: Tue, 11 Jun 2024 09:22:44 +0100
> From: "Daniel P. Berrangé" <berrange@redhat.com>
> Subject: Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust
> 
> On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> > Hello everyone,
> > 
> > This is an early draft of my work on implementing a very simple device, 
> > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
> > and is used in hw/arm/virt.c).
> 
> looking at the diffstat:
> 
> >  .gitignore                     |   2 +
> >  .gitlab-ci.d/buildtest.yml     |  64 ++--
> >  configure                      |  12 +
> >  hw/arm/virt.c                  |   2 +-
> >  meson.build                    |  99 ++++++
> >  meson_options.txt              |   4 +
> >  rust/meson.build               |  93 ++++++
> >  rust/pl011/.cargo/config.toml  |   2 +
> >  rust/pl011/.gitignore          |   2 +
> >  rust/pl011/Cargo.lock          | 120 +++++++
> >  rust/pl011/Cargo.toml          |  26 ++
> >  rust/pl011/README.md           |  42 +++
> >  rust/pl011/build.rs            |  44 +++
> >  rust/pl011/meson.build         |   7 +
> >  rust/pl011/rustfmt.toml        |  10 +
> >  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          | 575 +++++++++++++++++++++++++++++++++
> >  rust/pl011/src/memory_ops.rs   |  38 +++
> 
> My thought is that if we're going to start implementing devices
> or other parts of QEMU, in Rust, then I do not want to see it
> placed in a completely separate directory sub-tree.
> 
> In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
> so that the device is part of the normal Arm hardware directory structure 
> and maintainer assignments.

It has its advantages. Otherwise, as the number of Rust implementations
grows, the same mirror directory as QEMU will have to be rebuilt again
in the Rust directory.

Further, putting C implementations in the same directory, there is again
the question of why it needs to be duplicated :-) . This topic is
probably also beyond the scope of this RFC, but it's nice to have a Rust
example to start with.

Currently, pl011 exclusively occupies a cargo as a package. In the
future, will other Rust implementations utilize the workspace mechanism
to act as a second package in the same cargo? Or will new cargo be created
again?

Under a unified Rust directory, using a workspace to manage multiple
packages looks as if it would be easier to maintain. Decentralized to an
existing directory, they're all separate cargos, and external dependencies
tend to become fragmented?

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Zhao Liu]

On Tue, Jun 11, 2024 at 09:18:25AM +0100, Daniel P. Berrangé wrote:
> Date: Tue, 11 Jun 2024 09:18:25 +0100
> From: "Daniel P. Berrangé" <berrange@redhat.com>
> Subject: Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust
> 
> On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> > What are the issues with not using the compiler, rustc, directly?
> > -----------------------------------------------------------------
> > [whataretheissueswith] Back to [TOC]
> > 
> > 1. Tooling
> >    Mostly writing up the build-sys tooling to do so. Ideally we'd 
> >    compile everything without cargo but rustc directly.
> > 
> >    If we decide we need Rust's `std` library support, we could 
> >    investigate whether building it from scratch is a good solution. This 
> >    will only build the bits we need in our devices.
> 
> Re-building 'std' for QEMU would be a no-go for many distros who
> will expect QEMU to use the distro provided 'std' package. So at
> most that would have to be an optional feature.
> 
> > 2. Rust dependencies
> >    We could go without them completely. I chose deliberately to include 
> >    one dependency in my UART implementation, `bilge`[0], because it has 
> >    an elegant way of representing typed bitfields for the UART's 
> >    registers.
> > 
> > [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
> >      Crates.io page: https://crates.io/crates/bilge
> >      Repository: https://github.com/hecatia-elegua/bilge
> > 
> > Should QEMU use third-party dependencies?
> > -----------------------------------------
> > [shouldqemuusethirdparty] Back to [TOC]
> > 
> > In my personal opinion, if we need a dependency we need a strong 
> > argument for it. A dependency needs a trusted upstream source, a QEMU 
> > maintainer to make sure it us up-to-date in QEMU etc.
> 
> "strong" is a rather fuzzy term. In C we already have a huge number
> of build dependencies
> 
>  $ wc -l tests/lcitool/projects/qemu.yml 
>  127 tests/lcitool/projects/qemu.yml
> 
> we would have many more than that except that we're conservative
> about adding deps on things because getting new libraries into
> distros is quite painful, or we lag behind where we would want
> to be to stick with compat for old distro versions.
> 
> In terms of Rust dependancies, I'd expect us to have fairly arbitrary
> dependancies used. If the dep avoids QEMU maintainers having to
> re-invent the wheel for something there is already a common crate
> for, then it is a good thing to use it. I'd almost go as far as
> encouraging use of external crates. Our maintainers should focus tmie
> on writing code that's delivers compelling features to QEMU, rather
> than re-creating common APIs that already have good crates.

So should a base lib be introduced to import and wrap all external
dependencies?

Sort of like osdep.h, so that specific Rust implementations can't import
external third-party libraries directly, but only through the base lib.

The advantage of this is that we can unify the management of external
dependencies and avoid “potentially/overly arbitrary” importing of
specific Rust implementations.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 11:22, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
>> Hello everyone,
>> 
>> This is an early draft of my work on implementing a very simple device, 
>> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
>> and is used in hw/arm/virt.c).
>
>looking at the diffstat:
>
>>  .gitignore                     |   2 +
>>  .gitlab-ci.d/buildtest.yml     |  64 ++--
>>  configure                      |  12 +
>>  hw/arm/virt.c                  |   2 +-
>>  meson.build                    |  99 ++++++
>>  meson_options.txt              |   4 +
>>  rust/meson.build               |  93 ++++++
>>  rust/pl011/.cargo/config.toml  |   2 +
>>  rust/pl011/.gitignore          |   2 +
>>  rust/pl011/Cargo.lock          | 120 +++++++
>>  rust/pl011/Cargo.toml          |  26 ++
>>  rust/pl011/README.md           |  42 +++
>>  rust/pl011/build.rs            |  44 +++
>>  rust/pl011/meson.build         |   7 +
>>  rust/pl011/rustfmt.toml        |  10 +
>>  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          | 575 +++++++++++++++++++++++++++++++++
>>  rust/pl011/src/memory_ops.rs   |  38 +++
>
>My thought is that if we're going to start implementing devices
>or other parts of QEMU, in Rust, then I do not want to see it
>placed in a completely separate directory sub-tree.
>
>In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
>so that the device is part of the normal Arm hardware directory structure 
>and maintainer assignments.

I agree 100%, but I thought it was not my place to decide that, it's 
part of the "request for comments" side of this series.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 12:45, Zhao Liu <zhao1.liu@intel.com> wrote:
>On Tue, Jun 11, 2024 at 09:22:44AM +0100, Daniel P. Berrangé wrote:
>> On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
>> > Hello everyone,
>> > 
>> > This is an early draft of my work on implementing a very simple device, 
>> > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c 
>> > and is used in hw/arm/virt.c).
>> 
>> looking at the diffstat:
>> 
>> >  .gitignore                     |   2 +
>> >  .gitlab-ci.d/buildtest.yml     |  64 ++--
>> >  configure                      |  12 +
>> >  hw/arm/virt.c                  |   2 +-
>> >  meson.build                    |  99 ++++++
>> >  meson_options.txt              |   4 +
>> >  rust/meson.build               |  93 ++++++
>> >  rust/pl011/.cargo/config.toml  |   2 +
>> >  rust/pl011/.gitignore          |   2 +
>> >  rust/pl011/Cargo.lock          | 120 +++++++
>> >  rust/pl011/Cargo.toml          |  26 ++
>> >  rust/pl011/README.md           |  42 +++
>> >  rust/pl011/build.rs            |  44 +++
>> >  rust/pl011/meson.build         |   7 +
>> >  rust/pl011/rustfmt.toml        |  10 +
>> >  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          | 575 +++++++++++++++++++++++++++++++++
>> >  rust/pl011/src/memory_ops.rs   |  38 +++
>> 
>> My thought is that if we're going to start implementing devices
>> or other parts of QEMU, in Rust, then I do not want to see it
>> placed in a completely separate directory sub-tree.
>> 
>> In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
>> so that the device is part of the normal Arm hardware directory structure 
>> and maintainer assignments.
>
>It has its advantages. Otherwise, as the number of Rust implementations
>grows, the same mirror directory as QEMU will have to be rebuilt again
>in the Rust directory.

Offtopic for this RFC but:

It'd also mean that each crate would have its own subdir in the tree. In 
the future free-standing .rs files like in the kernel would be nice to 
have.

For those who are not familiar with Cargo, a cargo library is a single 
compilation unit. You cannot have interspersed .rs files and compile 
with cargo as one does generally. You'd have to generate the rustc 
commands for building.

>
>Further, putting C implementations in the same directory, there is again
>the question of why it needs to be duplicated :-) . This topic is
>probably also beyond the scope of this RFC, but it's nice to have a Rust
>example to start with.

pl011 was suggested by Peter as a very simple device to model. The 
duplication is not meant to replace the C version for now or at the 
foreseeable future. It's more of a reference implementation.

>
>Currently, pl011 exclusively occupies a cargo as a package. In the
>future, will other Rust implementations utilize the workspace mechanism
>to act as a second package in the same cargo? Or will new cargo be created
>again?

What do you mean by "new cargo"? I didn't catch that :(

A workspace would make sense if we have "general" crate libraries that 
hardware crates depend on.

>
>Under a unified Rust directory, using a workspace to manage multiple
>packages looks as if it would be easier to maintain. Decentralized to an
>existing directory, they're all separate cargos, and external dependencies
>tend to become fragmented?

Hmm potentially yes, but that's a "what if" scenario. Let's worry about 
that bridge when we cross it!

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 11:18, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
>> What are the issues with not using the compiler, rustc, directly?
>> -----------------------------------------------------------------
>> [whataretheissueswith] Back to [TOC]
>> 
>> 1. Tooling
>>    Mostly writing up the build-sys tooling to do so. Ideally we'd 
>>    compile everything without cargo but rustc directly.
>> 
>>    If we decide we need Rust's `std` library support, we could 
>>    investigate whether building it from scratch is a good solution. This 
>>    will only build the bits we need in our devices.
>
>Re-building 'std' for QEMU would be a no-go for many distros who
>will expect QEMU to use the distro provided 'std' package. So at
>most that would have to be an optional feature.

Yes this wasn't meant for the distro case, you're correct.

>
>> 2. Rust dependencies
>>    We could go without them completely. I chose deliberately to include 
>>    one dependency in my UART implementation, `bilge`[0], because it has 
>>    an elegant way of representing typed bitfields for the UART's 
>>    registers.
>> 
>> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>>      Crates.io page: https://crates.io/crates/bilge
>>      Repository: https://github.com/hecatia-elegua/bilge
>> 
>> Should QEMU use third-party dependencies?
>> -----------------------------------------
>> [shouldqemuusethirdparty] Back to [TOC]
>> 
>> In my personal opinion, if we need a dependency we need a strong 
>> argument for it. A dependency needs a trusted upstream source, a QEMU 
>> maintainer to make sure it us up-to-date in QEMU etc.
>
>"strong" is a rather fuzzy term. In C we already have a huge number
>of build dependencies

Rust crates.io dependencies tend to "explode" due to the large number of 
transitive dependencies and even different versions of the same crates.

Here's an example:

https://landaire.net/on-dependency-usage-in-rust/#what-about-dependency-explosion

This is something to be aware of in general when pulling dependencies.


>
> $ wc -l tests/lcitool/projects/qemu.yml 
> 127 tests/lcitool/projects/qemu.yml
>
>we would have many more than that except that we're conservative
>about adding deps on things because getting new libraries into
>distros is quite painful, or we lag behind where we would want
>to be to stick with compat for old distro versions.
>
>In terms of Rust dependancies, I'd expect us to have fairly arbitrary
>dependancies used. If the dep avoids QEMU maintainers having to
>re-invent the wheel for something there is already a common crate
>for, then it is a good thing to use it. I'd almost go as far as
>encouraging use of external crates. Our maintainers should focus tmie
>on writing code that's delivers compelling features to QEMU, rather
>than re-creating common APIs that already have good crates.

That was my reasoning for using the bitfield crate to represent UART 
registers.

>
>> We already fetch some projects with meson subprojects, so this is not a 
>> new reality. Cargo allows you to define "locked" dependencies which is 
>> the same as only fetching specific commits by SHA. No suspicious 
>> tarballs, and no disappearing dependencies a la left-pad in npm.
>> 
>> However, I believe it's worth considering vendoring every dependency by 
>> default, if they prove to be few, for the sake of having a local QEMU 
>> git clone buildable without network access.
>
>A local git clone is already not buildable without network access,
>given that you have to install countless extra distro packages
>ahead of time. I think its reasonable to expect people working from
>git to have to download rust deps. We should consider whether we
>want vendoring in the release tarballs though.


Sorry, I meant using cargo without network access. This requires setting 
up the registry index and caches on your $CARGO_HOME

>
>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>> -----------------------------------------------------------
>> [qemuprovidewrappingrustapis] Back to [TOC]
>> 
>> My personal opinion is no, with the reasoning being that QEMU internals 
>> are not documented or stable. However I do not see why creating stable 
>> opt-in interfaces is bad. It just needs someone to volunteer to maintain 
>> it and ensure there are no breakages through versions.
>
>I expect this will evolve organically with people providing wrappers
>where appropriate to suit their development neds.
>
>> Will QEMU now depend on Rust and thus not build on my XYZ platform?
>> -------------------------------------------------------------------
>> [qemudependonrustnotbuildonxyz] Back to [TOC]
>> 
>> No, worry about this in some years if this experiment takes off. Rust 
>> has broad platform support and is present in most distro package 
>> managers. In the future we might have gcc support for it as well.
>
>Rust isn't going away, so if a platform wants to remain relevant
>to the modern software world, then people who care about that
>platform need to ensure Rust works on it. I wouldn't say that
>QEMU needs to massively worry about this, since all the common
>platforms are now covered precisely because Rust is becoming
>so wildly used that a platform cannot ignore it.

Agreed.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
> > Hello Manos,
> > 
> > On 6/10/24 11:22, Manos Pitsidianakis wrote:
> > > Hello everyone,
> > > 
> > > This is an early draft of my work on implementing a very simple device,
> > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> > > and is used in hw/arm/virt.c).
> > > 
> > > The device is functional, with copied logic from the C code but with
> > > effort not to make a direct C to Rust translation. In other words, do
> > > not write Rust as a C developer would.
> > > 
> > > That goal is not complete but a best-effort case. To give a specific
> > > example, register values are typed but interrupt bit flags are not (but
> > > could be). I will leave such minutiae for later iterations.

snip

> > Maybe it could be better if build.rs file was *not* needed for new
> > devices/folders, and could be abstracted as a detail of the python
> > wrapper script instead of something that should be committed.
> 
> 
> That'd mean you cannot work on the rust files with a LanguageServer, you
> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
> left the alternative choice of including a manually generated bindings file
> (generated.rs.inc)

I would not expect QEMU developers to be running 'cargo <anything>'
directly at all.

QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
convenience facade.

Any use of 'cargo' would be an internal impl detail of meson rules
for building rust code, and developers should still exclusively work
with 'make' or 'ninja' to run builds & tests. 

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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> > Hello Manos,
>> > 
>> > On 6/10/24 11:22, Manos Pitsidianakis wrote:
>> > > Hello everyone,
>> > > 
>> > > This is an early draft of my work on implementing a very simple device,
>> > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>> > > and is used in hw/arm/virt.c).
>> > > 
>> > > The device is functional, with copied logic from the C code but with
>> > > effort not to make a direct C to Rust translation. In other words, do
>> > > not write Rust as a C developer would.
>> > > 
>> > > That goal is not complete but a best-effort case. To give a specific
>> > > example, register values are typed but interrupt bit flags are not (but
>> > > could be). I will leave such minutiae for later iterations.
>
>snip
>
>> > Maybe it could be better if build.rs file was *not* needed for new
>> > devices/folders, and could be abstracted as a detail of the python
>> > wrapper script instead of something that should be committed.
>> 
>> 
>> That'd mean you cannot work on the rust files with a LanguageServer, you
>> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>> left the alternative choice of including a manually generated bindings file
>> (generated.rs.inc)
>
>I would not expect QEMU developers to be running 'cargo <anything>'
>directly at all.
>
>QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
>convenience facade.
>
>Any use of 'cargo' would be an internal impl detail of meson rules
>for building rust code, and developers should still exclusively work
>with 'make' or 'ninja' to run builds & tests.

No, that's not true. If I wrote the pl011 device with this workflow I'd 
just waste time using meson. Part of the development is making sure the 
library type checks, compiles, using cargo to run style formatting, to 
check for lints, perhaps run tests. Doing this only through meson is an 
unnecessary complication.

To compile and run QEMU with a rust component, sure, you'd use meson.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Tue, Jun 11, 2024 at 01:58:10PM +0300, Manos Pitsidianakis wrote:
> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> > On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
> > > On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
> > > > Hello Manos,
> > > > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
> > > > > Hello everyone,
> > > > > > > This is an early draft of my work on implementing a very
> > > simple device,
> > > > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> > > > > and is used in hw/arm/virt.c).
> > > > > > > The device is functional, with copied logic from the C code
> > > but with
> > > > > effort not to make a direct C to Rust translation. In other words, do
> > > > > not write Rust as a C developer would.
> > > > > > > That goal is not complete but a best-effort case. To give a
> > > specific
> > > > > example, register values are typed but interrupt bit flags are not (but
> > > > > could be). I will leave such minutiae for later iterations.
> > 
> > snip
> > 
> > > > Maybe it could be better if build.rs file was *not* needed for new
> > > > devices/folders, and could be abstracted as a detail of the python
> > > > wrapper script instead of something that should be committed.
> > > 
> > > 
> > > That'd mean you cannot work on the rust files with a LanguageServer, you
> > > cannot run cargo build or cargo check or cargo clippy, etc. That's why I
> > > left the alternative choice of including a manually generated bindings file
> > > (generated.rs.inc)
> > 
> > I would not expect QEMU developers to be running 'cargo <anything>'
> > directly at all.
> > 
> > QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
> > convenience facade.
> > 
> > Any use of 'cargo' would be an internal impl detail of meson rules
> > for building rust code, and developers should still exclusively work
> > with 'make' or 'ninja' to run builds & tests.
> 
> No, that's not true. If I wrote the pl011 device with this workflow I'd just
> waste time using meson. Part of the development is making sure the library
> type checks, compiles, using cargo to run style formatting, to check for
> lints, perhaps run tests. Doing this only through meson is an unnecessary
> complication.

I don't see why it should waste time, when we ultimately end up calling
the same underlying tools. We need to have a consistent experiance for
developers working on QEMU, not have to use different tools for different
parts of QEMU depending on whether a piece of code happens to be rust
or C.

> To compile and run QEMU with a rust component, sure, you'd use meson.

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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 14:09, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Tue, Jun 11, 2024 at 01:58:10PM +0300, Manos Pitsidianakis wrote:
>> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>> > On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>> > > On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> > > > Hello Manos,
>> > > > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
>> > > > > Hello everyone,
>> > > > > > > This is an early draft of my work on implementing a very
>> > > simple device,
>> > > > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>> > > > > and is used in hw/arm/virt.c).
>> > > > > > > The device is functional, with copied logic from the C code
>> > > but with
>> > > > > effort not to make a direct C to Rust translation. In other words, do
>> > > > > not write Rust as a C developer would.
>> > > > > > > That goal is not complete but a best-effort case. To give a
>> > > specific
>> > > > > example, register values are typed but interrupt bit flags are not (but
>> > > > > could be). I will leave such minutiae for later iterations.
>> > 
>> > snip
>> > 
>> > > > Maybe it could be better if build.rs file was *not* needed for new
>> > > > devices/folders, and could be abstracted as a detail of the python
>> > > > wrapper script instead of something that should be committed.
>> > > 
>> > > 
>> > > That'd mean you cannot work on the rust files with a LanguageServer, you
>> > > cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>> > > left the alternative choice of including a manually generated bindings file
>> > > (generated.rs.inc)
>> > 
>> > I would not expect QEMU developers to be running 'cargo <anything>'
>> > directly at all.
>> > 
>> > QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
>> > convenience facade.
>> > 
>> > Any use of 'cargo' would be an internal impl detail of meson rules
>> > for building rust code, and developers should still exclusively work
>> > with 'make' or 'ninja' to run builds & tests.
>> 
>> No, that's not true. If I wrote the pl011 device with this workflow I'd just
>> waste time using meson. Part of the development is making sure the library
>> type checks, compiles, using cargo to run style formatting, to check for
>> lints, perhaps run tests. Doing this only through meson is an unnecessary
>> complication.
>
>I don't see why it should waste time, when we ultimately end up calling
>the same underlying tools. We need to have a consistent experiance for
>developers working on QEMU, not have to use different tools for different
>parts of QEMU depending on whether a piece of code happens to be rust
>or C.

This is a technicality but for the spirit of conversation and curiosity 
I will reply with my view: we end up calling meson and ninja, yes, but 
they wrap cargo under the hood. Cargo has its own tooling and workflows. 
At the end of compilation it spits out a dynamic or static library. That 
happens independently of QEMU.

If you start writing a new project, you don't go linking it to QEMU 
right away (you can, but there's no need to yet). You make an idiomatic 
Rust crate with rust tooling. FFI calls can exist and you are still able 
to compile/check your crate. This is the typical rust workflow, and it's 
ubiquitous, so I think we should not deprive anyone of it.

The only thing we cannot check with cargo alone is if the crate uses the 
QEMU apis correctly (logic errors, not compilation errors)

Until/if we can build with just meson and rustc (a hypothetical scenario 
at the moment) cargo becomes part of our development and build tools.

I understand your argument and it's not irrational at all. I offer this 
perspective as a Rust developer and not a QEMU developer. As a QEMU 
developer I shouldn't need to care about cargo unless I'm touching part 
of that code; the build system details (the cargo wrapper) can be 
abstracted away for everyone else.

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Antonio Caggiano]

Hi there :)

On 11/06/2024 12:58, Manos Pitsidianakis wrote:
> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> 
> wrote:
>> On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier 
>>> <pierrick.bouvier@linaro.org> wrote:
>>> > Hello Manos,
>>> > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
>>> > > Hello everyone,
>>> > > > > This is an early draft of my work on implementing a very 
>>> simple device,
>>> > > in this case the ARM PL011 (which in C code resides in 
>>> hw/char/pl011.c
>>> > > and is used in hw/arm/virt.c).
>>> > > > > The device is functional, with copied logic from the C code 
>>> but with
>>> > > effort not to make a direct C to Rust translation. In other 
>>> words, do
>>> > > not write Rust as a C developer would.
>>> > > > > That goal is not complete but a best-effort case. To give a 
>>> specific
>>> > > example, register values are typed but interrupt bit flags are 
>>> not (but
>>> > > could be). I will leave such minutiae for later iterations.
>>
>> snip
>>
>>> > Maybe it could be better if build.rs file was *not* needed for new
>>> > devices/folders, and could be abstracted as a detail of the python
>>> > wrapper script instead of something that should be committed.
>>>
>>>
>>> That'd mean you cannot work on the rust files with a LanguageServer, you
>>> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>>> left the alternative choice of including a manually generated 
>>> bindings file
>>> (generated.rs.inc)
>>
>> I would not expect QEMU developers to be running 'cargo <anything>'
>> directly at all.
>>
>> QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
>> convenience facade.
>>
>> Any use of 'cargo' would be an internal impl detail of meson rules
>> for building rust code, and developers should still exclusively work
>> with 'make' or 'ninja' to run builds & tests.
> 
> No, that's not true. If I wrote the pl011 device with this workflow I'd 
> just waste time using meson. Part of the development is making sure the 
> library type checks, compiles, using cargo to run style formatting, to 
> check for lints, perhaps run tests. Doing this only through meson is an 
> unnecessary complication.
> 

My favorite tool for Rust development is rust-analyzer, which works very 
well with cargo-based projects. Making it work with meson is just a 
matter of pointing rust-analyzer to the rust-project.json file generated 
by meson at configuration time (just like compile_commands.json).

Unfortunately, rust-analyzer also relies on cargo for doing its check. I 
was able to override that with ninja, but it requires `meson setup` with 
`RUSTFLAGS="--emit=metadata --error-format=json"`. That makes 
rust-analyzer happy, but compilation output is not readable anymore 
being json-like.

I ended up working with 2 build folders, one for me, one for 
rust-analyzer. So, yeah, it complicates a bit.

> To compile and run QEMU with a rust component, sure, you'd use meson.
> 

Cheers,
Antonio

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis]

Hello Antonio!

On Tue, 11 Jun 2024 15:45, Antonio Caggiano <quic_acaggian@quicinc.com> wrote:
>Hi there :)
>
>On 11/06/2024 12:58, Manos Pitsidianakis wrote:
>> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> 
>> wrote:
>>> On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>>>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier 
>>>> <pierrick.bouvier@linaro.org> wrote:
>>>> > Hello Manos,
>>>> > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
>>>> > > Hello everyone,
>>>> > > > > This is an early draft of my work on implementing a very 
>>>> simple device,
>>>> > > in this case the ARM PL011 (which in C code resides in 
>>>> hw/char/pl011.c
>>>> > > and is used in hw/arm/virt.c).
>>>> > > > > The device is functional, with copied logic from the C code 
>>>> but with
>>>> > > effort not to make a direct C to Rust translation. In other 
>>>> words, do
>>>> > > not write Rust as a C developer would.
>>>> > > > > That goal is not complete but a best-effort case. To give a 
>>>> specific
>>>> > > example, register values are typed but interrupt bit flags are 
>>>> not (but
>>>> > > could be). I will leave such minutiae for later iterations.
>>>
>>> snip
>>>
>>>> > Maybe it could be better if build.rs file was *not* needed for new
>>>> > devices/folders, and could be abstracted as a detail of the python
>>>> > wrapper script instead of something that should be committed.
>>>>
>>>>
>>>> That'd mean you cannot work on the rust files with a LanguageServer, you
>>>> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>>>> left the alternative choice of including a manually generated 
>>>> bindings file
>>>> (generated.rs.inc)
>>>
>>> I would not expect QEMU developers to be running 'cargo <anything>'
>>> directly at all.
>>>
>>> QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
>>> convenience facade.
>>>
>>> Any use of 'cargo' would be an internal impl detail of meson rules
>>> for building rust code, and developers should still exclusively work
>>> with 'make' or 'ninja' to run builds & tests.
>> 
>> No, that's not true. If I wrote the pl011 device with this workflow I'd 
>> just waste time using meson. Part of the development is making sure the 
>> library type checks, compiles, using cargo to run style formatting, to 
>> check for lints, perhaps run tests. Doing this only through meson is an 
>> unnecessary complication.
>> 
>
>My favorite tool for Rust development is rust-analyzer, which works very 
>well with cargo-based projects. Making it work with meson is just a 
>matter of pointing rust-analyzer to the rust-project.json file generated 
>by meson at configuration time (just like compile_commands.json).

That's only generated for meson rust targets, whereas we are currently 
compiling with a cargo wrapper script.

>
>Unfortunately, rust-analyzer also relies on cargo for doing its check. I 
>was able to override that with ninja, but it requires `meson setup` with 
>`RUSTFLAGS="--emit=metadata --error-format=json"`. That makes 
>rust-analyzer happy, but compilation output is not readable anymore 
>being json-like.
>
>I ended up working with 2 build folders, one for me, one for 
>rust-analyzer. So, yeah, it complicates a bit.
>
>> To compile and run QEMU with a rust component, sure, you'd use meson.
>> 
>
>Cheers,
>Antonio

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Jun 11, 2024 at 01:58:10PM +0300, Manos Pitsidianakis wrote:
>> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>> > On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
>> > > On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>> > > > Hello Manos,
>> > > > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
>> > > > > Hello everyone,
>> > > > > > > This is an early draft of my work on implementing a very
>> > > simple device,
>> > > > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>> > > > > and is used in hw/arm/virt.c).
>> > > > > > > The device is functional, with copied logic from the C code
>> > > but with
>> > > > > effort not to make a direct C to Rust translation. In other words, do
>> > > > > not write Rust as a C developer would.
>> > > > > > > That goal is not complete but a best-effort case. To give a
>> > > specific
>> > > > > example, register values are typed but interrupt bit flags are not (but
>> > > > > could be). I will leave such minutiae for later iterations.
>> > 
>> > snip
>> > 
>> > > > Maybe it could be better if build.rs file was *not* needed for new
>> > > > devices/folders, and could be abstracted as a detail of the python
>> > > > wrapper script instead of something that should be committed.
>> > > 
>> > > 
>> > > That'd mean you cannot work on the rust files with a LanguageServer, you
>> > > cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>> > > left the alternative choice of including a manually generated bindings file
>> > > (generated.rs.inc)
>> > 
>> > I would not expect QEMU developers to be running 'cargo <anything>'
>> > directly at all.
>> > 
>> > QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
>> > convenience facade.
>> > 
>> > Any use of 'cargo' would be an internal impl detail of meson rules
>> > for building rust code, and developers should still exclusively work
>> > with 'make' or 'ninja' to run builds & tests.
>> 
>> No, that's not true. If I wrote the pl011 device with this workflow I'd just
>> waste time using meson. Part of the development is making sure the library
>> type checks, compiles, using cargo to run style formatting, to check for
>> lints, perhaps run tests. Doing this only through meson is an unnecessary
>> complication.
>
> I don't see why it should waste time, when we ultimately end up calling
> the same underlying tools. We need to have a consistent experiance for
> developers working on QEMU, not have to use different tools for different
> parts of QEMU depending on whether a piece of code happens to be rust
> or C.

For example if I wanted to run rust-based unit tests (which I think
potentially offer an easier solution than qtest) I would expect that to
be done from the normal make/ninja targets.

>
>> To compile and run QEMU with a rust component, sure, you'd use meson.
>
> With regards,
> Daniel

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Tue, Jun 11, 2024 at 01:51:13PM +0100, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Jun 11, 2024 at 01:58:10PM +0300, Manos Pitsidianakis wrote:
> >> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> >> > On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
> >> > > On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
> >> > > > Hello Manos,
> >> > > > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
> >> > > > > Hello everyone,
> >> > > > > > > This is an early draft of my work on implementing a very
> >> > > simple device,
> >> > > > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> >> > > > > and is used in hw/arm/virt.c).
> >> > > > > > > The device is functional, with copied logic from the C code
> >> > > but with
> >> > > > > effort not to make a direct C to Rust translation. In other words, do
> >> > > > > not write Rust as a C developer would.
> >> > > > > > > That goal is not complete but a best-effort case. To give a
> >> > > specific
> >> > > > > example, register values are typed but interrupt bit flags are not (but
> >> > > > > could be). I will leave such minutiae for later iterations.
> >> > 
> >> > snip
> >> > 
> >> > > > Maybe it could be better if build.rs file was *not* needed for new
> >> > > > devices/folders, and could be abstracted as a detail of the python
> >> > > > wrapper script instead of something that should be committed.
> >> > > 
> >> > > 
> >> > > That'd mean you cannot work on the rust files with a LanguageServer, you
> >> > > cannot run cargo build or cargo check or cargo clippy, etc. That's why I
> >> > > left the alternative choice of including a manually generated bindings file
> >> > > (generated.rs.inc)
> >> > 
> >> > I would not expect QEMU developers to be running 'cargo <anything>'
> >> > directly at all.
> >> > 
> >> > QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
> >> > convenience facade.
> >> > 
> >> > Any use of 'cargo' would be an internal impl detail of meson rules
> >> > for building rust code, and developers should still exclusively work
> >> > with 'make' or 'ninja' to run builds & tests.
> >> 
> >> No, that's not true. If I wrote the pl011 device with this workflow I'd just
> >> waste time using meson. Part of the development is making sure the library
> >> type checks, compiles, using cargo to run style formatting, to check for
> >> lints, perhaps run tests. Doing this only through meson is an unnecessary
> >> complication.
> >
> > I don't see why it should waste time, when we ultimately end up calling
> > the same underlying tools. We need to have a consistent experiance for
> > developers working on QEMU, not have to use different tools for different
> > parts of QEMU depending on whether a piece of code happens to be rust
> > or C.
> 
> For example if I wanted to run rust-based unit tests (which I think
> potentially offer an easier solution than qtest) I would expect that to
> be done from the normal make/ninja targets.

Meson provides a nice "suite" concept to facilitate selection of a
subset of tests.

eg, to limit to running just 'rust' unit tests, I might expect we
should have

  meson test --suite rustunit

and have this invoked by 'make check-rustunit' 

Similar can be done for clippy, or other types of rust tests

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

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Paolo Bonzini]

On Mon, Jun 10, 2024 at 10:47 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, 10 Jun 2024 at 16:27, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> >
> > On Mon, 10 Jun 2024 22:59, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > >> What are the issues with not using the compiler, rustc, directly?
> > >> -----------------------------------------------------------------
> > >> [whataretheissueswith] Back to [TOC]
> > >>
> > >> 1. Tooling
> > >>    Mostly writing up the build-sys tooling to do so. Ideally we'd
> > >>    compile everything without cargo but rustc directly.
> > >
> > >Why would that be ideal?
> >
> > It remove the indirection level of meson<->cargo<->rustc. I don't have a
> > concrete idea on how to tackle this, but if cargo ends up not strictly
> > necessary, I don't see why we cannot use one build system.
>
> The convenience of being able to use cargo dependencies without
> special QEMU meson build system effort seems worth the overhead of
> meson<->cargo<->rustc to me. There is a blog post that explores using
> cargo crates using meson's wrap dependencies here, and it seems like
> extra work:
> https://coaxion.net/blog/2023/04/building-a-gstreamer-plugin-in-rust-with-meson-instead-of-cargo/

The worst part of using cargo from meson (like in libblkio) is the
lack of integration with Rust tests, but otherwise it's a much better
experience. IIUC Meson's cargo subprojects do not support build.rs,
which is a problem if one of your dependencies (for example libc)
needs it.

https://mesonbuild.com/Wrap-dependency-system-manual.html#cargo-wraps

On the other hand, I think it's possible, possibly even clearer, to
invoke bindgen from meson. I would prefer to have many small .rs files
produced by bindgen, to be imported via "use", and I would prefer an
allowlist approach that excludes symbols from system headers.

> > >I guess there will be interest in using rust-vmm crates in some way.

Yes, especially the ByteValued, VolatileMemory and Bytes traits.

One complication in that respect is that anything that does DMA
depends on either RCU or the BQL. We could, at least at the beginning,
introduce a dummy guard that simply enforces at run-time that the BQL
is taken.

Paolo

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Paolo Bonzini]

On Tue, Jun 11, 2024 at 10:22 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> > Hello everyone,
> >
> > This is an early draft of my work on implementing a very simple device,
> > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> > and is used in hw/arm/virt.c).
>
> looking at the diffstat:
>
> >  .gitignore                     |   2 +
> >  .gitlab-ci.d/buildtest.yml     |  64 ++--
> >  configure                      |  12 +
> >  hw/arm/virt.c                  |   2 +-
> >  meson.build                    |  99 ++++++
> >  meson_options.txt              |   4 +
> >  rust/meson.build               |  93 ++++++
> >  rust/pl011/.cargo/config.toml  |   2 +
> >  rust/pl011/.gitignore          |   2 +
> >  rust/pl011/Cargo.lock          | 120 +++++++
> >  rust/pl011/Cargo.toml          |  26 ++
> >  rust/pl011/README.md           |  42 +++
> >  rust/pl011/build.rs            |  44 +++
> >  rust/pl011/meson.build         |   7 +
> >  rust/pl011/rustfmt.toml        |  10 +
> >  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          | 575 +++++++++++++++++++++++++++++++++
> >  rust/pl011/src/memory_ops.rs   |  38 +++
>
> My thought is that if we're going to start implementing devices
> or other parts of QEMU, in Rust, then I do not want to see it
> placed in a completely separate directory sub-tree.
>
> In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
> so that the device is part of the normal Arm hardware directory structure
> and maintainer assignments.

I think that's incompatible with the layout that Cargo expects.
rust/hw/arm/pl011/ could be another possibility.

Paolo

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Daniel P. Berrangé]

On Tue, Jun 11, 2024 at 03:16:19PM +0200, Paolo Bonzini wrote:
> On Tue, Jun 11, 2024 at 10:22 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Jun 10, 2024 at 09:22:35PM +0300, Manos Pitsidianakis wrote:
> > > Hello everyone,
> > >
> > > This is an early draft of my work on implementing a very simple device,
> > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> > > and is used in hw/arm/virt.c).
> >
> > looking at the diffstat:
> >
> > >  .gitignore                     |   2 +
> > >  .gitlab-ci.d/buildtest.yml     |  64 ++--
> > >  configure                      |  12 +
> > >  hw/arm/virt.c                  |   2 +-
> > >  meson.build                    |  99 ++++++
> > >  meson_options.txt              |   4 +
> > >  rust/meson.build               |  93 ++++++
> > >  rust/pl011/.cargo/config.toml  |   2 +
> > >  rust/pl011/.gitignore          |   2 +
> > >  rust/pl011/Cargo.lock          | 120 +++++++
> > >  rust/pl011/Cargo.toml          |  26 ++
> > >  rust/pl011/README.md           |  42 +++
> > >  rust/pl011/build.rs            |  44 +++
> > >  rust/pl011/meson.build         |   7 +
> > >  rust/pl011/rustfmt.toml        |  10 +
> > >  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          | 575 +++++++++++++++++++++++++++++++++
> > >  rust/pl011/src/memory_ops.rs   |  38 +++
> >
> > My thought is that if we're going to start implementing devices
> > or other parts of QEMU, in Rust, then I do not want to see it
> > placed in a completely separate directory sub-tree.
> >
> > In this example, I would expect to have hw/arm/pl011.rs, or hw/arm/pl011/*.rs
> > so that the device is part of the normal Arm hardware directory structure
> > and maintainer assignments.
> 
> I think that's incompatible with the layout that Cargo expects.
> rust/hw/arm/pl011/ could be another possibility.

It doesn't look like its a problem in this patch series. It is just
introducing a "rust/pl011/Cargo.toml", and I don't see anything
that has an fundamental assumption that it is below a 'rust/' top
level dir, as opposed to under our existing dir structure.

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

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Alex Bennée]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Mon, Jun 10, 2024 at 09:22:36PM +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.
>> 
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> ---
>> The cargo wrapper script hardcodes some rust target triples. This is 
>> just temporary.
>> ---
>>  .gitignore               |   2 +
>>  configure                |  12 +++
>>  meson.build              |  11 ++
>>  meson_options.txt        |   4 +
>>  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 240 insertions(+)
>>  create mode 100644 scripts/cargo_wrapper.py
>> 
>> diff --git a/.gitignore b/.gitignore
>> index 61fa39967b..f42b0d937e 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -2,6 +2,8 @@
>>  /build/
>>  /.cache/
>>  /.vscode/
>> +/target/
>> +rust/**/target
>
> Are these necessary since the cargo build command-line below uses
> --target-dir <meson-build-dir>?
>
> Adding new build output directories outside build/ makes it harder to
> clean up the source tree and ensure no state from previous builds
> remains.

Indeed my tree looks like:

 $SRC
   /builds
     /buildA
     /buildB

etc. So I would expect the rust build stuff to be in the builddir
because I have multiple configurations.

>
>>  *.pyc
>>  .sdk
>>  .stgit-*
>> diff --git a/configure b/configure
>> index 38ee257701..c195630771 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-rust) with_rust=enabled
>> +  ;;
>> +  --disable-rust) with_rust=disabled
>> +  ;;
>> +  --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,9 @@ 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" != enabled && meson_option_add "-Dwith_rust=$with_rust"
>
> Duplicate line.
>
>> +  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..d338effdaa
>> --- /dev/null
>> +++ b/scripts/cargo_wrapper.py
>> @@ -0,0 +1,211 @@
>> +#!/usr/bin/env python3
>> +# Copyright (c) 2020 Red Hat, Inc.
>> +# Copyright (c) 2023 Linaro Ltd.
>> +#
>> +# Authors:
>> +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> +#  Marc-André Lureau <marcandre.lureau@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later.  See the COPYING file in the top-level directory.
>> +
>> +import argparse
>> +import configparser
>> +import distutils.file_util
>> +import json
>> +import logging
>> +import os
>> +import os.path
>> +import re
>> +import subprocess
>> +import sys
>> +import pathlib
>> +import shutil
>> +import tomllib
>> +
>> +from pathlib import Path
>> +from typing import Any, Dict, List, Tuple
>> +
>> +RUST_TARGET_TRIPLES = (
>> +    "aarch64-unknown-linux-gnu",
>> +    "x86_64-unknown-linux-gnu",
>> +    "x86_64-apple-darwin",
>> +    "aarch64-apple-darwin",
>> +)
>
> Is this hardcoded to avoid calling `rustc --print target-list`?
>
> Or is this the support matrix? In that case it would be interesting to
> figure out the target triples for all host OSes and CPUs that QEMU is
> supported on.
>
>> +
>> +
>> +def cfg_name(name: str) -> str:
>> +    if (
>> +        name.startswith("CONFIG_")
>> +        or name.startswith("TARGET_")
>> +        or name.startswith("HAVE_")
>> +    ):
>> +        return name
>> +    return ""
>> +
>> +
>> +def generate_cfg_flags(header: str) -> List[str]:
>> +    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) -> pathlib.Path:
>> +    return args.meson_build_dir
>> +
>> +
>> +def manifest_path(args: argparse.Namespace) -> pathlib.Path:
>> +    return args.crate_dir / "Cargo.toml"
>> +
>> +
>> +def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
>> +    # 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:
>> +    envlog = " ".join(["{}={}".format(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 build_lib(args: argparse.Namespace) -> None:
>> +    logging.debug("build-lib")
>> +    target_dir = cargo_target_dir(args)
>> +    cargo_toml_path = manifest_path(args)
>> +
>> +    with open(cargo_toml_path, "rb") as f:
>> +        config = tomllib.load(f)
>> +
>> +    package_name = config["package"]["name"].strip('"').replace("-", "_")
>> +
>> +    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("cp %s %s", liba, args.outdir)
>> +    shutil.copy2(liba, args.outdir)
>> +
>> +
>> +def main() -> None:
>> +    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=pathlib.Path,
>> +        dest="meson_build_dir",
>> +        required=True,
>> +    )
>> +    parser.add_argument(
>> +        "--meson-source-dir",
>> +        metavar="SOURCE_DIR",
>> +        help="meson.current_source_dir()",
>> +        type=pathlib.Path,
>> +        dest="meson_source_dir",
>> +        required=True,
>> +    )
>> +    parser.add_argument(
>> +        "--crate-dir",
>> +        metavar="CRATE_DIR",
>> +        type=pathlib.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=pathlib.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, choices=RUST_TARGET_TRIPLES, 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()
>> -- 
>> γαῖα πυρί μιχθήτω
>> 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Zhao Liu]

On Tue, Jun 11, 2024 at 01:41:57PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 11 Jun 2024 13:41:57 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust
> 
> > Currently, pl011 exclusively occupies a cargo as a package. In the
> > future, will other Rust implementations utilize the workspace mechanism
> > to act as a second package in the same cargo? Or will new cargo be created
> > again?
> 
> What do you mean by "new cargo"? I didn't catch that :(
> 
> A workspace would make sense if we have "general" crate libraries that
> hardware crates depend on.

Thanks Manos!

I mean if we spread the rust device across the QEMU submodules, wouldn't
we have to create their own cargo directories (aka single-package cargo)
for each rust device?

However, if the Rust code is all centralized under the /Rust directory,
then it can be managed by multiple-packages in cargo workspace. 

About the "general" crate, I'm not sure whether a base lib to manage
external crates is a good idea, like I replied in [1].

[1]: https://lore.kernel.org/qemu-devel/CAJSP0QWLe6yPDE3rPztx=oS0g+vKT9W3GykrNU0EQZcaW06sog@mail.gmail.com/T/#mfaf9abf06ed82dd7f8ce5e7520bbb4447083b550

> > 
> > Under a unified Rust directory, using a workspace to manage multiple
> > packages looks as if it would be easier to maintain. Decentralized to an
> > existing directory, they're all separate cargos, and external dependencies
> > tend to become fragmented?
> 
> Hmm potentially yes, but that's a "what if" scenario. Let's worry about that
> bridge when we cross it!
>

Yes!

Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Pierrick Bouvier]

On 6/11/24 02:21, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 6/10/24 13:29, Manos Pitsidianakis wrote:
>>> On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>>>> Hello Manos,
>>>>
> <snip>
>>>> Excellent work, and thanks for posting this RFC!
>>>>
>>>> IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing
>>>> (temporarily) the generated.rs file in patches is not a good move.
>>>> Any reason you kept it this way?
>>> That was my first approach, I will rework it on the second version.
>>> The
>>> generated code should not exist in committed code at all.
>>> It was initally tricky setting up the dependency orders correctly,
>>> so I
>>> first committed it and then made it a dependency.
>>>
>>>>
>>>> Maybe it could be better if build.rs file was *not* needed for new
>>>> devices/folders, and could be abstracted as a detail of the python
>>>> wrapper script instead of something that should be committed.
>>> That'd mean you cannot work on the rust files with a LanguageServer,
>>> you
>>> cannot run cargo build or cargo check or cargo clippy, etc. That's why I
>>> left the alternative choice of including a manually generated bindings
>>> file (generated.rs.inc)
>>>
>>
>> Maybe I missed something, but it seems like it just checks/copies the
>> generated.rs file where it's expected. Definitely something that could
>> be done as part of the rust build.
>>
>> Having to run the build before getting completion does not seem to be
>> a huge compromise.
>>
> <snip>
> 
> As long as the Language Server can kick in after a first build. Rust
> definitely leans in to the concept of the tooling helping you out while
> coding.
> 
> I think for the C LSPs compile_commands.json is generated during the
> configure step but I could be wrong.
> 

Yes, meson generates it.
I agree having support for completion tooling is important nowadays, 
whether in C or in Rust.

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Manos Pitsidianakis]

On Tue, 11 Jun 2024 at 17:05, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Jun 10, 2024 at 09:22:36PM +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.
> >
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> > The cargo wrapper script hardcodes some rust target triples. This is
> > just temporary.
> > ---
> >  .gitignore               |   2 +
> >  configure                |  12 +++
> >  meson.build              |  11 ++
> >  meson_options.txt        |   4 +
> >  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 240 insertions(+)
> >  create mode 100644 scripts/cargo_wrapper.py
> >
> > diff --git a/.gitignore b/.gitignore
> > index 61fa39967b..f42b0d937e 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -2,6 +2,8 @@
> >  /build/
> >  /.cache/
> >  /.vscode/
> > +/target/
> > +rust/**/target
>
> Are these necessary since the cargo build command-line below uses
> --target-dir <meson-build-dir>?
>
> Adding new build output directories outside build/ makes it harder to
> clean up the source tree and ensure no state from previous builds
> remains.

Agreed! These build directories would show up when using cargo
directly instead of through the cargo_wrapper.py script, i.e. during
development. I'd consider it an edge case, it won't happen much and if
it does it's better to gitignore them than accidentally checking them
in. Also, whatever artifacts are in a `target` directory won't be used
for compilation with qemu inside a build directory.


> >  *.pyc
> >  .sdk
> >  .stgit-*
> > diff --git a/configure b/configure
> > index 38ee257701..c195630771 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-rust) with_rust=enabled
> > +  ;;
> > +  --disable-rust) with_rust=disabled
> > +  ;;
> > +  --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,9 @@ 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" != enabled && meson_option_add "-Dwith_rust=$with_rust"
>
> Duplicate line.

Thanks!

>
> > +  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..d338effdaa
> > --- /dev/null
> > +++ b/scripts/cargo_wrapper.py
> > @@ -0,0 +1,211 @@
> > +#!/usr/bin/env python3
> > +# Copyright (c) 2020 Red Hat, Inc.
> > +# Copyright (c) 2023 Linaro Ltd.
> > +#
> > +# Authors:
> > +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > +#  Marc-André Lureau <marcandre.lureau@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > +# later.  See the COPYING file in the top-level directory.
> > +
> > +import argparse
> > +import configparser
> > +import distutils.file_util
> > +import json
> > +import logging
> > +import os
> > +import os.path
> > +import re
> > +import subprocess
> > +import sys
> > +import pathlib
> > +import shutil
> > +import tomllib
> > +
> > +from pathlib import Path
> > +from typing import Any, Dict, List, Tuple
> > +
> > +RUST_TARGET_TRIPLES = (
> > +    "aarch64-unknown-linux-gnu",
> > +    "x86_64-unknown-linux-gnu",
> > +    "x86_64-apple-darwin",
> > +    "aarch64-apple-darwin",
> > +)
>
> Is this hardcoded to avoid calling `rustc --print target-list`?
>
> Or is this the support matrix? In that case it would be interesting to
> figure out the target triples for all host OSes and CPUs that QEMU is
> supported on.

Yes, it's what I tested it on (the x86-64-apple-darwin part through rosetta).

Do you think running -print target-list would be a better choice here?
This is only for providing the valid choices for the target triplet
CLI argument in argparse.


>
> > +
> > +
> > +def cfg_name(name: str) -> str:
> > +    if (
> > +        name.startswith("CONFIG_")
> > +        or name.startswith("TARGET_")
> > +        or name.startswith("HAVE_")
> > +    ):
> > +        return name
> > +    return ""
> > +
> > +
> > +def generate_cfg_flags(header: str) -> List[str]:
> > +    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) -> pathlib.Path:
> > +    return args.meson_build_dir
> > +
> > +
> > +def manifest_path(args: argparse.Namespace) -> pathlib.Path:
> > +    return args.crate_dir / "Cargo.toml"
> > +
> > +
> > +def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
> > +    # 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:
> > +    envlog = " ".join(["{}={}".format(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 build_lib(args: argparse.Namespace) -> None:
> > +    logging.debug("build-lib")
> > +    target_dir = cargo_target_dir(args)
> > +    cargo_toml_path = manifest_path(args)
> > +
> > +    with open(cargo_toml_path, "rb") as f:
> > +        config = tomllib.load(f)
> > +
> > +    package_name = config["package"]["name"].strip('"').replace("-", "_")
> > +
> > +    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("cp %s %s", liba, args.outdir)
> > +    shutil.copy2(liba, args.outdir)
> > +
> > +
> > +def main() -> None:
> > +    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=pathlib.Path,
> > +        dest="meson_build_dir",
> > +        required=True,
> > +    )
> > +    parser.add_argument(
> > +        "--meson-source-dir",
> > +        metavar="SOURCE_DIR",
> > +        help="meson.current_source_dir()",
> > +        type=pathlib.Path,
> > +        dest="meson_source_dir",
> > +        required=True,
> > +    )
> > +    parser.add_argument(
> > +        "--crate-dir",
> > +        metavar="CRATE_DIR",
> > +        type=pathlib.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=pathlib.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, choices=RUST_TARGET_TRIPLES, 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()
> > --
> > γαῖα πυρί μιχθήτω
> >

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Stefan Hajnoczi]

On Tue, 11 Jun 2024 at 13:54, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Tue, 11 Jun 2024 at 17:05, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Jun 10, 2024 at 09:22:36PM +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.
> > >
> > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > ---
> > > The cargo wrapper script hardcodes some rust target triples. This is
> > > just temporary.
> > > ---
> > >  .gitignore               |   2 +
> > >  configure                |  12 +++
> > >  meson.build              |  11 ++
> > >  meson_options.txt        |   4 +
> > >  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 240 insertions(+)
> > >  create mode 100644 scripts/cargo_wrapper.py
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index 61fa39967b..f42b0d937e 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -2,6 +2,8 @@
> > >  /build/
> > >  /.cache/
> > >  /.vscode/
> > > +/target/
> > > +rust/**/target
> >
> > Are these necessary since the cargo build command-line below uses
> > --target-dir <meson-build-dir>?
> >
> > Adding new build output directories outside build/ makes it harder to
> > clean up the source tree and ensure no state from previous builds
> > remains.
>
> Agreed! These build directories would show up when using cargo
> directly instead of through the cargo_wrapper.py script, i.e. during
> development. I'd consider it an edge case, it won't happen much and if
> it does it's better to gitignore them than accidentally checking them
> in. Also, whatever artifacts are in a `target` directory won't be used
> for compilation with qemu inside a build directory.

Why would someone bypass the build system? I don't think we should
encourage developers to do this.

>
>
> > >  *.pyc
> > >  .sdk
> > >  .stgit-*
> > > diff --git a/configure b/configure
> > > index 38ee257701..c195630771 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-rust) with_rust=enabled
> > > +  ;;
> > > +  --disable-rust) with_rust=disabled
> > > +  ;;
> > > +  --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,9 @@ 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" != enabled && meson_option_add "-Dwith_rust=$with_rust"
> >
> > Duplicate line.
>
> Thanks!
>
> >
> > > +  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..d338effdaa
> > > --- /dev/null
> > > +++ b/scripts/cargo_wrapper.py
> > > @@ -0,0 +1,211 @@
> > > +#!/usr/bin/env python3
> > > +# Copyright (c) 2020 Red Hat, Inc.
> > > +# Copyright (c) 2023 Linaro Ltd.
> > > +#
> > > +# Authors:
> > > +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > +#  Marc-André Lureau <marcandre.lureau@redhat.com>
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > +# later.  See the COPYING file in the top-level directory.
> > > +
> > > +import argparse
> > > +import configparser
> > > +import distutils.file_util
> > > +import json
> > > +import logging
> > > +import os
> > > +import os.path
> > > +import re
> > > +import subprocess
> > > +import sys
> > > +import pathlib
> > > +import shutil
> > > +import tomllib
> > > +
> > > +from pathlib import Path
> > > +from typing import Any, Dict, List, Tuple
> > > +
> > > +RUST_TARGET_TRIPLES = (
> > > +    "aarch64-unknown-linux-gnu",
> > > +    "x86_64-unknown-linux-gnu",
> > > +    "x86_64-apple-darwin",
> > > +    "aarch64-apple-darwin",
> > > +)
> >
> > Is this hardcoded to avoid calling `rustc --print target-list`?
> >
> > Or is this the support matrix? In that case it would be interesting to
> > figure out the target triples for all host OSes and CPUs that QEMU is
> > supported on.
>
> Yes, it's what I tested it on (the x86-64-apple-darwin part through rosetta).
>
> Do you think running -print target-list would be a better choice here?
> This is only for providing the valid choices for the target triplet
> CLI argument in argparse.

How about not restricting choices? If the user specifies an invalid
choice then the compiler will fail with an error message. That seems
okay and avoids the issue altogether.

>
>
> >
> > > +
> > > +
> > > +def cfg_name(name: str) -> str:
> > > +    if (
> > > +        name.startswith("CONFIG_")
> > > +        or name.startswith("TARGET_")
> > > +        or name.startswith("HAVE_")
> > > +    ):
> > > +        return name
> > > +    return ""
> > > +
> > > +
> > > +def generate_cfg_flags(header: str) -> List[str]:
> > > +    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) -> pathlib.Path:
> > > +    return args.meson_build_dir
> > > +
> > > +
> > > +def manifest_path(args: argparse.Namespace) -> pathlib.Path:
> > > +    return args.crate_dir / "Cargo.toml"
> > > +
> > > +
> > > +def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
> > > +    # 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:
> > > +    envlog = " ".join(["{}={}".format(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 build_lib(args: argparse.Namespace) -> None:
> > > +    logging.debug("build-lib")
> > > +    target_dir = cargo_target_dir(args)
> > > +    cargo_toml_path = manifest_path(args)
> > > +
> > > +    with open(cargo_toml_path, "rb") as f:
> > > +        config = tomllib.load(f)
> > > +
> > > +    package_name = config["package"]["name"].strip('"').replace("-", "_")
> > > +
> > > +    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("cp %s %s", liba, args.outdir)
> > > +    shutil.copy2(liba, args.outdir)
> > > +
> > > +
> > > +def main() -> None:
> > > +    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=pathlib.Path,
> > > +        dest="meson_build_dir",
> > > +        required=True,
> > > +    )
> > > +    parser.add_argument(
> > > +        "--meson-source-dir",
> > > +        metavar="SOURCE_DIR",
> > > +        help="meson.current_source_dir()",
> > > +        type=pathlib.Path,
> > > +        dest="meson_source_dir",
> > > +        required=True,
> > > +    )
> > > +    parser.add_argument(
> > > +        "--crate-dir",
> > > +        metavar="CRATE_DIR",
> > > +        type=pathlib.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=pathlib.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, choices=RUST_TARGET_TRIPLES, 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()
> > > --
> > > γαῖα πυρί μιχθήτω
> > >
>

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Daniel P. Berrangé]

On Tue, Jun 11, 2024 at 02:25:39PM -0400, Stefan Hajnoczi wrote:
> On Tue, 11 Jun 2024 at 13:54, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> >
> > On Tue, 11 Jun 2024 at 17:05, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Jun 10, 2024 at 09:22:36PM +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.
> > > >
> > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > > ---
> > > > The cargo wrapper script hardcodes some rust target triples. This is
> > > > just temporary.
> > > > ---
> > > >  .gitignore               |   2 +
> > > >  configure                |  12 +++
> > > >  meson.build              |  11 ++
> > > >  meson_options.txt        |   4 +
> > > >  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
> > > >  5 files changed, 240 insertions(+)
> > > >  create mode 100644 scripts/cargo_wrapper.py

> > > > diff --git a/configure b/configure
> > > > index 38ee257701..c195630771 100755
> > > > --- a/configure
> > > > +++ b/configure

snip

> > > > +  test "$with_rust_target_triple" != "" && meson_option_add "-Dwith_rust_target_triple=$with_rust_target_triple"

So the --rust-target-triple is only needed when cross compiling,
but this is not the way we normally handle passing cross compiler
info to meson. Instead we create a meson cross compiler options
file containing the target info.

eg for ./configure --cross-prefix=x86_64-w64-mingw32-

we end up creating:

$ cat build/config-meson.cross 
# Automatically generated by configure - do not modify
[properties]
[built-in options]
c_args = []
cpp_args = []
objc_args = []
c_link_args = []
cpp_link_args = []
# environment defaults, can still be overridden on 
# the command line
werror = true
[project options]

[binaries]
c = ['x86_64-w64-mingw32-gcc','-m64']
cpp = ['x86_64-w64-mingw32-g++','-m64']
objc = ['x86_64-w64-mingw32-clang','-m64']
ar = ['x86_64-w64-mingw32-ar']
dlltool = ['x86_64-w64-mingw32-dlltool']
nm = ['x86_64-w64-mingw32-nm']
pkgconfig = ['x86_64-w64-mingw32-pkg-config']
pkg-config = ['x86_64-w64-mingw32-pkg-config']
ranlib = ['x86_64-w64-mingw32-ranlib']
strip = ['x86_64-w64-mingw32-strip']
widl = ['x86_64-w64-mingw32-widl']
windres = ['x86_64-w64-mingw32-windres']
windmc = ['x86_64-w64-mingw32-windmc']
[host_machine]
system = 'windows'
cpu_family = 'x86_64'
cpu = 'x86_64'
endian = 'little'


Should we not be passing the rust compiler target through
this meson options file by setting something like this

  rust = ['rustc', '--target', '$target_target_triple']


Also I don't think we should be requiring --rust-target-triple
to be passed by the user. For all the combinations we know &
test, we should have configure "do the right thing" and set a
suitable rust target triple based on the --cross-prefix argument
that is given, so there is no extra burden on users cross
compiling. Users should then only use --rust-target-triple
if our default logic is wrong for some reason.

> > > >    run_meson() {
> > > >      NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
> > > >    }
> > > > diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> > > > new file mode 100644
> > > > index 0000000000..d338effdaa
> > > > --- /dev/null
> > > > +++ b/scripts/cargo_wrapper.py
> > > > @@ -0,0 +1,211 @@
> > > > +#!/usr/bin/env python3
> > > > +# Copyright (c) 2020 Red Hat, Inc.
> > > > +# Copyright (c) 2023 Linaro Ltd.
> > > > +#
> > > > +# Authors:
> > > > +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > > +#  Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > +#
> > > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > > +# later.  See the COPYING file in the top-level directory.
> > > > +
> > > > +import argparse
> > > > +import configparser
> > > > +import distutils.file_util
> > > > +import json
> > > > +import logging
> > > > +import os
> > > > +import os.path
> > > > +import re
> > > > +import subprocess
> > > > +import sys
> > > > +import pathlib
> > > > +import shutil
> > > > +import tomllib
> > > > +
> > > > +from pathlib import Path
> > > > +from typing import Any, Dict, List, Tuple
> > > > +
> > > > +RUST_TARGET_TRIPLES = (
> > > > +    "aarch64-unknown-linux-gnu",
> > > > +    "x86_64-unknown-linux-gnu",
> > > > +    "x86_64-apple-darwin",
> > > > +    "aarch64-apple-darwin",
> > > > +)
> > >
> > > Is this hardcoded to avoid calling `rustc --print target-list`?
> > >
> > > Or is this the support matrix? In that case it would be interesting to
> > > figure out the target triples for all host OSes and CPUs that QEMU is
> > > supported on.
> >
> > Yes, it's what I tested it on (the x86-64-apple-darwin part through rosetta).
> >
> > Do you think running -print target-list would be a better choice here?
> > This is only for providing the valid choices for the target triplet
> > CLI argument in argparse.
> 
> How about not restricting choices? If the user specifies an invalid
> choice then the compiler will fail with an error message. That seems
> okay and avoids the issue altogether.

Yes, we should not artifically limit the choices of target at all, as
we don't do that for existing cross compiler targets.

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

Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

[Marc-André Lureau]

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

Hi

On Wed, Jun 12, 2024 at 12:05 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Tue, Jun 11, 2024 at 02:25:39PM -0400, Stefan Hajnoczi wrote:
> > On Tue, 11 Jun 2024 at 13:54, Manos Pitsidianakis
> > <manos.pitsidianakis@linaro.org> wrote:
> > >
> > > On Tue, 11 Jun 2024 at 17:05, Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> > > >
> > > > On Mon, Jun 10, 2024 at 09:22:36PM +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.
> > > > >
> > > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org
> >
> > > > > ---
> > > > > The cargo wrapper script hardcodes some rust target triples. This
> is
> > > > > just temporary.
> > > > > ---
> > > > >  .gitignore               |   2 +
> > > > >  configure                |  12 +++
> > > > >  meson.build              |  11 ++
> > > > >  meson_options.txt        |   4 +
> > > > >  scripts/cargo_wrapper.py | 211
> +++++++++++++++++++++++++++++++++++++++
> > > > >  5 files changed, 240 insertions(+)
> > > > >  create mode 100644 scripts/cargo_wrapper.py
>
> > > > > diff --git a/configure b/configure
> > > > > index 38ee257701..c195630771 100755
> > > > > --- a/configure
> > > > > +++ b/configure
>
> snip
>
> > > > > +  test "$with_rust_target_triple" != "" && meson_option_add
> "-Dwith_rust_target_triple=$with_rust_target_triple"
>
> So the --rust-target-triple is only needed when cross compiling,
> but this is not the way we normally handle passing cross compiler
> info to meson. Instead we create a meson cross compiler options
> file containing the target info.
>
> eg for ./configure --cross-prefix=x86_64-w64-mingw32-
>
> we end up creating:
>
> $ cat build/config-meson.cross
> # Automatically generated by configure - do not modify
> [properties]
> [built-in options]
> c_args = []
> cpp_args = []
> objc_args = []
> c_link_args = []
> cpp_link_args = []
> # environment defaults, can still be overridden on
> # the command line
> werror = true
> [project options]
>
> [binaries]
> c = ['x86_64-w64-mingw32-gcc','-m64']
> cpp = ['x86_64-w64-mingw32-g++','-m64']
> objc = ['x86_64-w64-mingw32-clang','-m64']
> ar = ['x86_64-w64-mingw32-ar']
> dlltool = ['x86_64-w64-mingw32-dlltool']
> nm = ['x86_64-w64-mingw32-nm']
> pkgconfig = ['x86_64-w64-mingw32-pkg-config']
> pkg-config = ['x86_64-w64-mingw32-pkg-config']
> ranlib = ['x86_64-w64-mingw32-ranlib']
> strip = ['x86_64-w64-mingw32-strip']
> widl = ['x86_64-w64-mingw32-widl']
> windres = ['x86_64-w64-mingw32-windres']
> windmc = ['x86_64-w64-mingw32-windmc']
> [host_machine]
> system = 'windows'
> cpu_family = 'x86_64'
> cpu = 'x86_64'
> endian = 'little'
>
>
> Should we not be passing the rust compiler target through
> this meson options file by setting something like this
>
>   rust = ['rustc', '--target', '$target_target_triple']
>

Agree


>
>
> Also I don't think we should be requiring --rust-target-triple
> to be passed by the user. For all the combinations we know &
> test, we should have configure "do the right thing" and set a
> suitable rust target triple based on the --cross-prefix argument
> that is given, so there is no extra burden on users cross
> compiling. Users should then only use --rust-target-triple
> if our default logic is wrong for some reason.
>
>
Then I think we would need to maintain some mapping between GNU
target-triplets and Rust. It would be convenient to allow users to
set/overwrite it though.


-- 
Marc-André Lureau

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