* [RFC PATCH v4 1/7] build-sys: Add rust feature option
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-08 14:49 ` Paolo Bonzini
2024-07-04 12:15 ` [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency Manos Pitsidianakis
` (6 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.
`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.
A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.
https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
MAINTAINERS | 5 +
configure | 11 ++
meson.build | 11 ++
meson_options.txt | 5 +
scripts/cargo_wrapper.py | 294 ++++++++++++++++++++++++++++++++++
scripts/meson-buildoptions.sh | 6 +
6 files changed, 332 insertions(+)
create mode 100644 scripts/cargo_wrapper.py
diff --git a/MAINTAINERS b/MAINTAINERS
index 6725913c8b..d01bd06ab7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4226,6 +4226,11 @@ F: docs/sphinx/
F: docs/_templates/
F: docs/devel/docs.rst
+Rust build system integration
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Maintained
+F: scripts/cargo_wrapper.py
+
Miscellaneous
-------------
Performance Tools and Tests
diff --git a/configure b/configure
index 8b6a2f16ce..180d87b9c9 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}"
@@ -757,6 +760,12 @@ for opt do
;;
--gdb=*) gdb_bin="$optarg"
;;
+ --enable-with-rust) with_rust=enabled
+ ;;
+ --disable-with-rust) with_rust=disabled
+ ;;
+ --with-rust-target-triple=*) with_rust_target_triple="$optarg"
+ ;;
# everything else has the same name in configure and meson
--*) meson_option_parse "$opt" "$optarg"
;;
@@ -1789,6 +1798,8 @@ if test "$skip_meson" = no; then
test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
test "$plugins" = yes && meson_option_add "-Dplugins=true"
test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
+ test "$with_rust" != enabled && meson_option_add "-Dwith_rust=$with_rust"
+ test "$with_rust_target_triple" != "" && meson_option_add "-Dwith_rust_target_triple=$with_rust_target_triple"
run_meson() {
NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
}
diff --git a/meson.build b/meson.build
index 2f981f936e..11b8b146da 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
@@ -2118,6 +2124,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 = {
@@ -4243,6 +4250,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 0269fa0f16..3443c48001 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -371,3 +371,8 @@ option('hexagon_idef_parser', type : 'boolean', value : true,
option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'], value: '1',
description: 'tweak required x86_64 architecture version beyond compiler default')
+
+option('with_rust', type: 'feature', value: 'auto',
+ description: 'Enable Rust support')
+option('with_rust_target_triple', type : 'string', value: '',
+ description: 'Override Rust target triple')
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
new file mode 100644
index 0000000000..d2c7265461
--- /dev/null
+++ b/scripts/cargo_wrapper.py
@@ -0,0 +1,294 @@
+#!/usr/bin/env python3
+
+"""Wrap cargo builds for meson integration
+
+This program builds Rust library crates and makes sure:
+ - They receive the correct --cfg compile flags from the QEMU build that calls
+ it.
+ - They receive the generated Rust bindings path so that they can copy it
+ inside their output subdirectories.
+ - Cargo puts all its build artifacts in the appropriate meson build directory.
+ - The produced static libraries are copied to the path the caller (meson)
+ defines.
+
+Copyright (c) 2020 Red Hat, Inc.
+Copyright (c) 2024 Linaro Ltd.
+
+Authors:
+ Marc-André Lureau <marcandre.lureau@redhat.com>
+ Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program. If not, see <http://www.gnu.org/licenses/>.
+"""
+
+
+import argparse
+import json
+import logging
+import os
+import subprocess
+import sys
+import shutil
+
+from pathlib import Path
+from typing import Any, Dict, List
+
+
+def generate_cfg_flags(header: str) -> List[str]:
+ """Converts defines from config[..].h headers to rustc --cfg flags."""
+
+ def cfg_name(name: str) -> str:
+ """Filter function for C #defines"""
+ if (
+ name.startswith("CONFIG_")
+ or name.startswith("TARGET_")
+ or name.startswith("HAVE_")
+ ):
+ return name
+ return ""
+
+ with open(header, encoding="utf-8") as cfg:
+ config = [l.split()[1:] for l in cfg if l.startswith("#define")]
+
+ cfg_list = []
+ for cfg in config:
+ name = cfg_name(cfg[0])
+ if not name:
+ continue
+ if len(cfg) >= 2 and cfg[1] != "1":
+ continue
+ cfg_list.append("--cfg")
+ cfg_list.append(name)
+ return cfg_list
+
+
+def cargo_target_dir(args: argparse.Namespace) -> Path:
+ """Place cargo's build artifacts into meson's build directory"""
+ return args.private_dir
+
+
+def manifest_path(args: argparse.Namespace) -> Path:
+ """Returns the Cargo.toml manifest path"""
+ return args.crate_dir / "Cargo.toml"
+
+
+def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]]:
+ """Returns the appropriate cargo invocation and environment"""
+
+ # See https://doc.rust-lang.org/cargo/reference/environment-variables.html
+ # Item `CARGO_ENCODED_RUSTFLAGS — A list of custom flags separated by
+ # 0x1f (ASCII Unit Separator) to pass to all compiler invocations that Cargo
+ # performs`
+ cfg = chr(0x1F).join(
+ [c for h in args.config_headers for c in generate_cfg_flags(h)]
+ )
+ target_dir = cargo_target_dir(args)
+ cargo_path = manifest_path(args)
+
+ cargo_cmd = [
+ "cargo",
+ "build",
+ "--target-dir",
+ str(target_dir),
+ "--manifest-path",
+ str(cargo_path),
+ ]
+ if args.target_triple:
+ cargo_cmd += ["--target", args.target_triple]
+ if args.profile == "release":
+ cargo_cmd += ["--release"]
+
+ env = os.environ
+ env["CARGO_ENCODED_RUSTFLAGS"] = cfg
+
+ return (env, cargo_cmd)
+
+
+def run_cargo(env: Dict[str, Any], cargo_cmd: List[str]) -> str:
+ """Calls cargo build invocation."""
+ envlog = " ".join([f"{k}={v}" for k, v in env.items()])
+ cmdlog = " ".join(cargo_cmd)
+ logging.debug("Running %s %s", envlog, cmdlog)
+ try:
+ out = subprocess.check_output(
+ cargo_cmd,
+ env=dict(os.environ, **env),
+ stderr=subprocess.STDOUT,
+ universal_newlines=True,
+ )
+ except subprocess.CalledProcessError as err:
+ print("Environment: " + envlog)
+ print("Command: " + cmdlog)
+ print(err.output)
+ sys.exit(1)
+
+ return out
+
+
+def get_package_name(cargo_toml_path: Path) -> str:
+ """Attempts to get package name from cargo manifest file with toml parsing libraries."""
+ # pylint: disable=import-outside-toplevel
+
+ try:
+ import tomllib
+ except ImportError:
+ import tomli as tomllib
+ with open(cargo_toml_path, "rb") as toml_file:
+ config = tomllib.load(toml_file)
+
+ package_name = config["package"]["name"].strip('"').replace("-", "_")
+ return package_name
+
+
+def get_package_name_json(cargo_toml_path: Path) -> str:
+ """Attempts to get package name from cargo-metadata output which has a standard JSON format."""
+
+ cmd = [
+ "cargo",
+ "metadata",
+ "--format-version",
+ "1",
+ "--no-deps",
+ "--manifest-path",
+ str(cargo_toml_path),
+ "--offline",
+ ]
+ try:
+ out = subprocess.check_output(
+ cmd,
+ env=os.environ,
+ stderr=subprocess.STDOUT,
+ universal_newlines=True,
+ )
+ except subprocess.CalledProcessError as err:
+ print("Command: ", " ".join(cmd))
+ print(err.output)
+ raise err
+ package_name = json.loads(out)["packages"][0]["name"].strip('"').replace("-", "_")
+ return package_name
+
+
+def build_lib(args: argparse.Namespace) -> None:
+ """Builds Rust lib given by command line arguments."""
+
+ logging.debug("build-lib")
+ target_dir = cargo_target_dir(args)
+ cargo_toml_path = manifest_path(args)
+
+ try:
+ # If we have tomllib or tomli, parse the .toml file
+ package_name = get_package_name(cargo_toml_path)
+ except ImportError as import_exc:
+ try:
+ # Parse the json output of cargo-metadata as a fallback
+ package_name = get_package_name_json(cargo_toml_path)
+ except Exception as exc:
+ raise exc from import_exc
+
+ liba_filename = "lib" + package_name + ".a"
+ profile_dir = args.profile
+ if args.profile == "dev":
+ profile_dir = "debug"
+
+ liba = target_dir / args.target_triple / profile_dir / liba_filename
+
+ env, cargo_cmd = get_cargo_rustc(args)
+ out = run_cargo(env, cargo_cmd)
+ logging.debug("cargo output: %s", out)
+ logging.debug("cp %s %s", liba, args.outdir)
+ shutil.copy2(liba, args.outdir)
+
+
+def main() -> None:
+ # pylint: disable=missing-function-docstring
+ parser = argparse.ArgumentParser()
+ parser.add_argument("-v", "--verbose", action="store_true")
+ parser.add_argument(
+ "--color",
+ metavar="WHEN",
+ choices=["auto", "always", "never"],
+ default="auto",
+ help="Coloring: auto, always, never",
+ )
+ parser.add_argument(
+ "--config-headers",
+ metavar="CONFIG_HEADER",
+ action="append",
+ dest="config_headers",
+ help="paths to any configuration C headers (*.h files), if any",
+ required=False,
+ default=[],
+ )
+ parser.add_argument(
+ "--meson-build-dir",
+ metavar="BUILD_DIR",
+ help="meson.current_build_dir()",
+ type=Path,
+ dest="meson_build_dir",
+ required=True,
+ )
+ parser.add_argument(
+ "--meson-source-dir",
+ metavar="SOURCE_DIR",
+ help="meson.current_source_dir()",
+ type=Path,
+ dest="meson_build_dir",
+ required=True,
+ )
+ parser.add_argument(
+ "--crate-dir",
+ metavar="CRATE_DIR",
+ type=Path,
+ dest="crate_dir",
+ help="Absolute path that contains the manifest file of the crate to compile. Example: '/path/to/qemu/rust/pl011'",
+ required=True,
+ )
+ parser.add_argument(
+ "--outdir",
+ metavar="OUTDIR",
+ type=Path,
+ dest="outdir",
+ help="Destination path to copy compiled artifacts to for Meson to use. Example values: '/path/to/qemu/build', '.'",
+ required=True,
+ )
+ # using @PRIVATE_DIR@ is necessary for `ninja clean` to clean up rust's intermediate build artifacts.
+ # NOTE: at the moment cleanup doesn't work due to a bug: https://github.com/mesonbuild/meson/issues/7584
+ parser.add_argument(
+ "--private-dir",
+ metavar="PRIVATE_DIR",
+ type=Path,
+ dest="private_dir",
+ help="Override cargo's target directory with a meson provided private directory.",
+ required=True,
+ )
+ parser.add_argument(
+ "--profile", type=str, choices=["release", "dev"], required=True
+ )
+ parser.add_argument("--target-triple", type=str, required=True)
+
+ subparsers = parser.add_subparsers()
+
+ buildlib = subparsers.add_parser("build-lib")
+ buildlib.set_defaults(func=build_lib)
+
+ args = parser.parse_args()
+ if args.verbose:
+ logging.basicConfig(level=logging.DEBUG)
+ logging.debug("args: %s", args)
+
+ args.func(args)
+
+
+if __name__ == "__main__":
+ main()
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index cfadb5ea86..23a24ccaa7 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -79,6 +79,8 @@ meson_options_help() {
printf "%s\n" ' auto/sigaltstack/ucontext/windows)'
printf "%s\n" ' --with-pkgversion=VALUE use specified string as sub-version of the'
printf "%s\n" ' package'
+ printf "%s\n" ' --with-rust-target-triple=VALUE'
+ printf "%s\n" ' Specify Rust host target triple'
printf "%s\n" ' --with-suffix=VALUE Suffix for QEMU data/modules/config directories'
printf "%s\n" ' (can be empty) [qemu]'
printf "%s\n" ' --with-trace-file=VALUE Trace file prefix for simple backend [trace]'
@@ -216,6 +218,7 @@ meson_options_help() {
printf "%s\n" ' vvfat vvfat image format support'
printf "%s\n" ' werror Treat warnings as errors'
printf "%s\n" ' whpx WHPX acceleration support'
+ printf "%s\n" ' with-rust Enable Rust support'
printf "%s\n" ' xen Xen backend support'
printf "%s\n" ' xen-pci-passthrough'
printf "%s\n" ' Xen PCI passthrough support'
@@ -552,6 +555,9 @@ _meson_option_parse() {
--enable-whpx) printf "%s" -Dwhpx=enabled ;;
--disable-whpx) printf "%s" -Dwhpx=disabled ;;
--x86-version=*) quote_sh "-Dx86_version=$2" ;;
+ --enable-with-rust) printf "%s" -Dwith_rust=enabled ;;
+ --disable-with-rust) printf "%s" -Dwith_rust=disabled ;;
+ --with-rust-target-triple=*) quote_sh "-Dwith_rust_target_triple=$2" ;;
--enable-xen) printf "%s" -Dxen=enabled ;;
--disable-xen) printf "%s" -Dxen=disabled ;;
--enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 1/7] build-sys: Add rust feature option
2024-07-04 12:15 ` [RFC PATCH v4 1/7] build-sys: Add rust feature option Manos Pitsidianakis
@ 2024-07-08 14:49 ` Paolo Bonzini
0 siblings, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 14:49 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Alex Bennée, Daniel P. Berrangé, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson, John Snow, Cleber Rosa
On Thu, Jul 4, 2024 at 2:16 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> 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.
Did you find any problem with the other approach that I sent, to
support --cargo and the CARGO environment variable in configure?
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
2024-07-04 12:15 ` [RFC PATCH v4 1/7] build-sys: Add rust feature option Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-08 15:07 ` Paolo Bonzini
` (2 more replies)
2024-07-04 12:15 ` [RFC PATCH v4 3/7] rust: add crate to expose bindings and interfaces Manos Pitsidianakis
` (5 subsequent siblings)
7 siblings, 3 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
Add mechanism to generate rust hw targets that depend on a custom
bindgen target for rust bindings to C.
This way bindings will be created before the rust crate is compiled.
The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
as a target:
ninja aarch64-softmmu-generated.rs
The way the bindings are generated is:
1. All required C headers are included in a single file, in our case
rust/wrapper.h for convenience. Otherwise we'd have to provide a list
of headers every time to the bindgen tool.
2. Meson creates a generated_rs target that runs bindgen making sure
the architecture etc header dependencies are present.
3. The generated_rs target takes a list of files, type symbols,
function symbols to block from being generated. This is not necessary
for the bindings to work, but saves us time and space.
4. Meson creates rust hardware target dependencies from the rust_targets
dictionary defined in rust/meson.build.
Since we cannot declare a dependency on generated_rs before it is
declared in meson.build, the rust crate targets must be defined after
the generated_rs target for each target architecture is defined. This
way meson sets up the dependency tree properly.
5. After compiling each rust crate with the cargo_wrapper.py script,
its static library artifact is linked as a `whole-archive` with the
final binary.
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
MAINTAINERS | 3 ++
meson.build | 57 ++++++++++++++++++++
rust/.gitignore | 3 ++
rust/meson.build | 112 +++++++++++++++++++++++++++++++++++++++
rust/wrapper.h | 39 ++++++++++++++
scripts/cargo_wrapper.py | 18 +++----
6 files changed, 220 insertions(+), 12 deletions(-)
create mode 100644 rust/.gitignore
create mode 100644 rust/meson.build
create mode 100644 rust/wrapper.h
diff --git a/MAINTAINERS b/MAINTAINERS
index d01bd06ab7..6e7b8207fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4230,6 +4230,9 @@ Rust build system integration
M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
S: Maintained
F: scripts/cargo_wrapper.py
+F: rust/meson.build
+F: rust/wrapper.h
+F: rust/.gitignore
Miscellaneous
-------------
diff --git a/meson.build b/meson.build
index 11b8b146da..71011fd3b3 100644
--- a/meson.build
+++ b/meson.build
@@ -3929,6 +3929,63 @@ foreach target : target_dirs
lib_deps += dep.partial_dependency(compile_args: true, includes: true)
endforeach
+ if with_rust and target_type == 'system'
+ # FIXME: meson outputs the following warnings, which should be resolved
+ # before merging:
+ # > WARNING: Project specifies a minimum meson_version '>=0.63.0' but
+ # > uses features which were added in newer versions:
+ # > * 0.64.0: {'fs.copyfile'}
+ # > * 1.0.0: {'dependencies arg in rust.bindgen', 'module rust as stable module'}
+ rust_bindgen = import('rust')
+
+ # We need one bindings_rs build target per arch target, so give them
+ # arch-specific names.
+ copy = fs.copyfile('rust/wrapper.h',
+ target + '_wrapper.h')
+ bindings_rs = rust_bindgen.bindgen(
+ input: copy,
+ dependencies: arch_deps + lib_deps,
+ output: 'bindings-' + target + '.rs',
+ include_directories: include_directories('.', 'include'),
+ args: [
+ '--ctypes-prefix', 'core::ffi',
+ '--formatter', 'rustfmt',
+ '--generate-block',
+ '--generate-cstr',
+ '--impl-debug',
+ '--merge-extern-blocks',
+ '--no-doc-comments',
+ '--no-include-path-detection',
+ '--use-core',
+ '--with-derive-default',
+ '--allowlist-file', meson.project_source_root() + '/include/.*',
+ '--allowlist-file', meson.project_source_root() + '/.*',
+ '--allowlist-file', meson.project_build_root() + '/.*'
+ ],
+ )
+
+ if target in rust_targets
+ rust_hw = ss.source_set()
+ foreach t: rust_targets[target]
+ rust_device_cargo_build = custom_target(t['name'],
+ output: t['output'],
+ depends: [bindings_rs],
+ build_always_stale: true,
+ command: t['command'])
+ rust_dep = declare_dependency(link_args: [
+ '-Wl,--whole-archive',
+ t['output'],
+ '-Wl,--no-whole-archive'
+ ],
+ sources: [rust_device_cargo_build])
+ rust_hw.add(rust_dep)
+ endforeach
+ rust_hw_config = rust_hw.apply(config_target, strict: false)
+ arch_srcs += rust_hw_config.sources()
+ arch_deps += rust_hw_config.dependencies()
+ endif
+ endif
+
lib = static_library('qemu-' + target,
sources: arch_srcs + genh,
dependencies: lib_deps,
diff --git a/rust/.gitignore b/rust/.gitignore
new file mode 100644
index 0000000000..1bf71b1f68
--- /dev/null
+++ b/rust/.gitignore
@@ -0,0 +1,3 @@
+# Ignore any cargo development build artifacts; for qemu-wide builds, all build
+# artifacts will go to the meson build directory.
+target
diff --git a/rust/meson.build b/rust/meson.build
new file mode 100644
index 0000000000..5fdc2621a3
--- /dev/null
+++ b/rust/meson.build
@@ -0,0 +1,112 @@
+# Supported hosts
+rust_supported_oses = {
+ 'linux': '-unknown-linux-gnu',
+ # 'darwin': '-apple-darwin',
+ # 'windows': '-pc-windows-gnu'
+}
+rust_supported_cpus = ['x86_64', 'aarch64']
+
+# Future-proof the above definitions against any change in the root meson.build file:
+foreach rust_os: rust_supported_oses.keys()
+ if not supported_oses.contains(rust_os)
+ message()
+ warning('UNSUPPORTED OS VALUES IN ' + meson.current_source_dir() + '/meson.build')
+ message()
+ message('This meson.build file claims OS `+' + rust_os + '` is supported but')
+ message('it is not included in the global supported OSes list in')
+ message(meson.global_source_root() + '/meson.build.')
+ endif
+endforeach
+foreach rust_cpu: rust_supported_cpus
+ if not supported_cpus.contains(rust_cpu)
+ message()
+ warning('UNSUPPORTED CPU VALUES IN ' + meson.current_source_dir() + '/meson.build')
+ message()
+ message('This meson.build file claims CPU `+' + rust_cpu + '` is supported but')
+ message('it is not included in the global supported CPUs list in')
+ message(meson.global_source_root() + '/meson.build.')
+ endif
+endforeach
+
+msrv = {
+ 'rustc': '1.77.2',
+ 'cargo': '1.77.2',
+ 'bindgen': '0.69.4',
+}
+
+foreach bin_dep: msrv.keys()
+ bin = find_program(bin_dep, required: true)
+ if bin.version() < msrv[bin_dep]
+ message()
+ error(bin_dep + ' version ' + bin.version() + ' is unsupported: Please upgrade to at least ' + msrv[bin_dep])
+ endif
+endforeach
+
+rust_target_triple = get_option('with_rust_target_triple')
+
+if rust_target_triple == ''
+ if not supported_oses.contains(host_os)
+ message()
+ error('QEMU does not support `' + host_os +'` as a Rust platform.')
+ elif not supported_cpus.contains(host_arch)
+ message()
+ error('QEMU does not support `' + host_arch +'` as a Rust architecture.')
+ endif
+ rust_target_triple = host_arch + rust_supported_oses[host_os]
+ # if host_os == 'windows' and host_arch == 'aarch64'
+ # rust_target_triple += 'llvm'
+ # endif
+else
+ # verify rust_target_triple if given as an option
+ rustc = find_program('rustc', required: true)
+ rustc_targets = run_command(rustc, '--print', 'target-list', capture: true, check: true).stdout().strip().split()
+ if not rustc_targets.contains(rust_target_triple)
+ message()
+ error('Given rust_target_triple ' + rust_target_triple + ' is not listed in rustc --print target-list output')
+ endif
+endif
+
+rust_targets = {}
+
+cargo_wrapper = [
+ find_program(meson.global_source_root() / 'scripts/cargo_wrapper.py'),
+ '--config-headers', meson.project_build_root() / 'config-host.h',
+ '--meson-build-root', meson.project_build_root(),
+]
+
+if get_option('b_colorout') != 'never'
+ cargo_wrapper += ['--color', 'always']
+endif
+
+if get_option('optimization') in ['0', '1', 'g']
+ rs_build_profile = 'dev'
+else
+ rs_build_profile = 'release'
+endif
+
+subdir('qemu-api')
+
+# Collect metadata for each (crate,qemu-target,compiler-target) combination.
+# Rust meson targets cannot be defined a priori because they depend on bindgen
+# generation that is created for each emulation target separately. Thus Rust
+# meson targets will be defined for each target after the target-specific
+# bindgen dependency is declared.
+rust_hw_target_list = {}
+
+foreach rust_hw_target, rust_hws: rust_hw_target_list
+ foreach rust_hw_dev: rust_hws
+ crate_metadata = {
+ 'name': rust_hw_dev['name'],
+ 'output': [rust_hw_dev['output']],
+ 'command': [cargo_wrapper,
+ '--crate-dir', meson.current_source_dir() / rust_hw_dev['dirname'],
+ '--profile', rs_build_profile,
+ '--target-triple', rust_target_triple,
+ '--private-dir', '@PRIVATE_DIR@',
+ '--outdir', '@OUTDIR@',
+ 'build-lib'
+ ]
+ }
+ rust_targets += { rust_hw_target: [crate_metadata] }
+ endforeach
+endforeach
diff --git a/rust/wrapper.h b/rust/wrapper.h
new file mode 100644
index 0000000000..51985f0ef1
--- /dev/null
+++ b/rust/wrapper.h
@@ -0,0 +1,39 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu-io.h"
+#include "sysemu/sysemu.h"
+#include "hw/sysbus.h"
+#include "exec/memory.h"
+#include "chardev/char-fe.h"
+#include "hw/clock.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/irq.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "chardev/char-serial.h"
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
index d2c7265461..e7d9238c16 100644
--- a/scripts/cargo_wrapper.py
+++ b/scripts/cargo_wrapper.py
@@ -111,6 +111,8 @@ def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]
env = os.environ
env["CARGO_ENCODED_RUSTFLAGS"] = cfg
+ env["MESON_BUILD_DIR"] = str(target_dir)
+ env["MESON_BUILD_ROOT"] = str(args.meson_build_root)
return (env, cargo_cmd)
@@ -231,19 +233,11 @@ def main() -> None:
default=[],
)
parser.add_argument(
- "--meson-build-dir",
- metavar="BUILD_DIR",
- help="meson.current_build_dir()",
+ "--meson-build-root",
+ metavar="BUILD_ROOT",
+ help="meson.project_build_root(): the root build directory. Example: '/path/to/qemu/build'",
type=Path,
- dest="meson_build_dir",
- required=True,
- )
- parser.add_argument(
- "--meson-source-dir",
- metavar="SOURCE_DIR",
- help="meson.current_source_dir()",
- type=Path,
- dest="meson_build_dir",
+ dest="meson_build_root",
required=True,
)
parser.add_argument(
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-04 12:15 ` [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency Manos Pitsidianakis
@ 2024-07-08 15:07 ` Paolo Bonzini
2024-07-09 10:53 ` Alex Bennée
2024-07-10 8:55 ` Alex Bennée
2 siblings, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 15:07 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Alex Bennée, Daniel P. Berrangé, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson, John Snow, Cleber Rosa
On Thu, Jul 4, 2024 at 2:16 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
> as a target:
>
> ninja aarch64-softmmu-generated.rs
Is there anything target-dependent in these files? You can generate it
just once I think.
> + if with_rust and target_type == 'system'
> + # FIXME: meson outputs the following warnings, which should be resolved
> + # before merging:
> + # > WARNING: Project specifies a minimum meson_version '>=0.63.0' but
> + # > uses features which were added in newer versions:
> + # > * 0.64.0: {'fs.copyfile'}
you can use configure_file() instead.
> + # > * 1.0.0: {'dependencies arg in rust.bindgen', 'module rust as stable module'}
You can use
if meson.version().version_compare('>=1.0.0')
...
else
error('Rust support requires Meson version 1.0.0')
endif
> + if target in rust_targets
> + rust_hw = ss.source_set()
> + foreach t: rust_targets[target]
> + rust_device_cargo_build = custom_target(t['name'],
> + output: t['output'],
> + depends: [bindings_rs],
> + build_always_stale: true,
> + command: t['command'])
Also "console: true".
> + rust_dep = declare_dependency(link_args: [
> + '-Wl,--whole-archive',
> + t['output'],
> + '-Wl,--no-whole-archive'
> + ],
This is not portable, but I think you can use just "link_whole:
rust_device_cargo_build".
> +msrv = {
> + 'rustc': '1.77.2',
> + 'cargo': '1.77.2',
I think rustc and cargo are always matching, so no need to check both of them?
> +foreach rust_hw_target, rust_hws: rust_hw_target_list
> + foreach rust_hw_dev: rust_hws
Finding the available devices should be done using Kconfig symbols,
probably by passing the path to the config-devices.mak file to
build.rs via an environment variable.
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu-io.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/sysbus.h"
> +#include "exec/memory.h"
> +#include "chardev/char-fe.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "hw/irq.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "chardev/char-serial.h"
I think all of these should be target-independent?
> diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> index d2c7265461..e7d9238c16 100644
> --- a/scripts/cargo_wrapper.py
> +++ b/scripts/cargo_wrapper.py
> @@ -111,6 +111,8 @@ def get_cargo_rustc(args: argparse.Namespace) -> tuple[Dict[str, Any], List[str]
>
> env = os.environ
> env["CARGO_ENCODED_RUSTFLAGS"] = cfg
> + env["MESON_BUILD_DIR"] = str(target_dir)
> + env["MESON_BUILD_ROOT"] = str(args.meson_build_root)
>
> return (env, cargo_cmd)
>
> @@ -231,19 +233,11 @@ def main() -> None:
> default=[],
> )
> parser.add_argument(
> - "--meson-build-dir",
> - metavar="BUILD_DIR",
> - help="meson.current_build_dir()",
> + "--meson-build-root",
> + metavar="BUILD_ROOT",
> + help="meson.project_build_root(): the root build directory. Example: '/path/to/qemu/build'",
> type=Path,
> - dest="meson_build_dir",
> - required=True,
> - )
> - parser.add_argument(
> - "--meson-source-dir",
> - metavar="SOURCE_DIR",
> - help="meson.current_source_dir()",
> - type=Path,
> - dest="meson_build_dir",
> + dest="meson_build_root",
> required=True,
> )
> parser.add_argument(
Please squash in the previous patch.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-04 12:15 ` [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency Manos Pitsidianakis
2024-07-08 15:07 ` Paolo Bonzini
@ 2024-07-09 10:53 ` Alex Bennée
2024-07-09 12:08 ` Peter Maydell
2024-07-09 14:52 ` Alex Bennée
2024-07-10 8:55 ` Alex Bennée
2 siblings, 2 replies; 38+ messages in thread
From: Alex Bennée @ 2024-07-09 10:53 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
> as a target:
>
> ninja aarch64-softmmu-generated.rs
>
> The way the bindings are generated is:
>
> 1. All required C headers are included in a single file, in our case
> rust/wrapper.h for convenience. Otherwise we'd have to provide a list
> of headers every time to the bindgen tool.
>
> 2. Meson creates a generated_rs target that runs bindgen making sure
> the architecture etc header dependencies are present.
>
> 3. The generated_rs target takes a list of files, type symbols,
> function symbols to block from being generated. This is not necessary
> for the bindings to work, but saves us time and space.
>
> 4. Meson creates rust hardware target dependencies from the rust_targets
> dictionary defined in rust/meson.build.
>
> Since we cannot declare a dependency on generated_rs before it is
> declared in meson.build, the rust crate targets must be defined after
> the generated_rs target for each target architecture is defined. This
> way meson sets up the dependency tree properly.
>
> 5. After compiling each rust crate with the cargo_wrapper.py script,
> its static library artifact is linked as a `whole-archive` with the
> final binary.
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
<snip>
> +
> +msrv = {
> + 'rustc': '1.77.2',
> + 'cargo': '1.77.2',
> + 'bindgen': '0.69.4',
> +}
This is still pretty bleeding edge (it even tripped up on the
.cargo/bin/cargo I have installed). This needs to be set to the
baseline which from:
https://wiki.qemu.org/RustInQemu/2022
Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
says 0.66.0). While it might make sense to delay merging if we are
waiting for one distro to produce a new LTS we shouldn't be needing
rustup by default.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 10:53 ` Alex Bennée
@ 2024-07-09 12:08 ` Peter Maydell
2024-07-09 12:28 ` Paolo Bonzini
2024-07-09 14:52 ` Alex Bennée
1 sibling, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2024-07-09 12:08 UTC (permalink / raw)
To: Alex Bennée
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
On Tue, 9 Jul 2024 at 11:53, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> > +msrv = {
> > + 'rustc': '1.77.2',
> > + 'cargo': '1.77.2',
> > + 'bindgen': '0.69.4',
> > +}
>
> This is still pretty bleeding edge (it even tripped up on the
> .cargo/bin/cargo I have installed). This needs to be set to the
> baseline which from:
>
> https://wiki.qemu.org/RustInQemu/2022
>
> Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
> says 0.66.0). While it might make sense to delay merging if we are
> waiting for one distro to produce a new LTS we shouldn't be needing
> rustup by default.
I suspect that some of the older distro releases in that chart
have already fallen off the end of our supported-platforms set,
so the minimum is probably newer than 1.24.0 now.
My take on this one is that (at some point, not necessarily
right now) we want to look at:
* what is the actual baseline requirement? We definitely want
to support "using rustup on an older system" (should be no
problem) and "current distro building QEMU using the distro's
rust", I assume. It would certainly be nice to have "building
QEMU on the older-but-still-in-our-support-list distro releases
with that distro's rust", but this probably implies not just
a minimum rust version but also a limited set of crates.
I think (but forget the details) that some of what we've done
with Python where we accept that the older-but-still-supported
distro will end up taking the "download at build time" path
in the build system might apply also for Rust.
* what, on the Rust side, is the version requirement? Presumably
there's some features that are pretty much "we really need
this and can't do without it" and some which are "this would
be pretty awkward not to have but if we have to we can implement
some kind of fallback/alternative with a TODO note to come
back and clean up when our baseline moves forwards".
At that point we have more information to figure out what
if any tradeoff we want to make.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 12:08 ` Peter Maydell
@ 2024-07-09 12:28 ` Paolo Bonzini
2024-07-09 13:00 ` Daniel P. Berrangé
` (2 more replies)
0 siblings, 3 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-09 12:28 UTC (permalink / raw)
To: Peter Maydell
Cc: Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Daniel P. Berrangé,
Marc-André Lureau, Thomas Huth, Markus Armbruster,
Philippe Mathieu-Daudé, Zhao Liu, Gustavo Romero,
Pierrick Bouvier, rowan.hart, Richard Henderson, John Snow,
Cleber Rosa
On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> * what is the actual baseline requirement? We definitely want
> to support "using rustup on an older system" (should be no
> problem) and "current distro building QEMU using the distro's
> rust", I assume. It would certainly be nice to have "building
> QEMU on the older-but-still-in-our-support-list distro releases
> with that distro's rust", but this probably implies not just
> a minimum rust version but also a limited set of crates.
I don't think limiting ourselves to the set of crates in the distro is
feasible. It's not the way the language works, for example I tried
checking if the "cstr" crate exists and I didn't find it in Debian.
We can define a known-good set of versions:
* either via Cargo.lock (i.e. recording the versions and downloading
them) or by including sources in the tree
* at any time in qemu.git, at branching time (three times a year), or
on every release
(That's six possibilities overall).
> I think (but forget the details) that some of what we've done
> with Python where we accept that the older-but-still-supported
> distro will end up taking the "download at build time" path
> in the build system might apply also for Rust.
Yes, and --without-download may be changed to the "--frozen" flag of cargo.
> * what, on the Rust side, is the version requirement? Presumably
> there's some features that are pretty much "we really need
> this and can't do without it" and some which are "this would
> be pretty awkward not to have but if we have to we can implement
> some kind of fallback/alternative with a TODO note to come
> back and clean up when our baseline moves forwards".
Here are the stopping points that I found over the last couple weeks:
1.56.0: 2021 edition
1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
crate, see below)
1.64.0: std::ffi::c_char
1.65.0: Generic Associated Types
1.74.0: Clippy can be configured in Cargo.toml
1.77.0: C string literals, offset_of!
I think 1.59.0 is pretty much the lower bound. Not having offset_of!
will be a bit painful, but it can be worked around (and pretty much
has to be, because 1.77.0 is really new).
As far as I understand, Debian bullseye has 1.63.0 these days and it's
the oldest platform we have.
Paolo
> At that point we have more information to figure out what
> if any tradeoff we want to make.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 12:28 ` Paolo Bonzini
@ 2024-07-09 13:00 ` Daniel P. Berrangé
2024-07-11 21:23 ` Pierrick Bouvier
2024-07-09 14:23 ` Alex Bennée
2024-07-10 15:03 ` Zhao Liu
2 siblings, 1 reply; 38+ messages in thread
From: Daniel P. Berrangé @ 2024-07-09 13:00 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
John Snow, Cleber Rosa
On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > * what is the actual baseline requirement? We definitely want
> > to support "using rustup on an older system" (should be no
> > problem) and "current distro building QEMU using the distro's
> > rust", I assume. It would certainly be nice to have "building
> > QEMU on the older-but-still-in-our-support-list distro releases
> > with that distro's rust", but this probably implies not just
> > a minimum rust version but also a limited set of crates.
>
> I don't think limiting ourselves to the set of crates in the distro is
> feasible. It's not the way the language works, for example I tried
> checking if the "cstr" crate exists and I didn't find it in Debian.
Yep, Rust is new enough that it is highly likely that crates will be
missing in multiple distros.
For ordinary users, cargo will happily download the missing pieces
so its not an issue.
For distro packagers, they'll just have to either package up the crates,
or bundle them in their QEMU build. Cargo makes the latter easy at
least. If distros don't want bundling, they'll need to go the more
involved route of packaging deps.
IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
versions we need as the minimum bar.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 13:00 ` Daniel P. Berrangé
@ 2024-07-11 21:23 ` Pierrick Bouvier
2024-07-12 6:14 ` Manos Pitsidianakis
0 siblings, 1 reply; 38+ messages in thread
From: Pierrick Bouvier @ 2024-07-11 21:23 UTC (permalink / raw)
To: Daniel P. Berrangé, Paolo Bonzini
Cc: Peter Maydell, Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, rowan.hart, Richard Henderson, John Snow,
Cleber Rosa
On 7/9/24 06:00, Daniel P. Berrangé wrote:
> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
>> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>> * what is the actual baseline requirement? We definitely want
>>> to support "using rustup on an older system" (should be no
>>> problem) and "current distro building QEMU using the distro's
>>> rust", I assume. It would certainly be nice to have "building
>>> QEMU on the older-but-still-in-our-support-list distro releases
>>> with that distro's rust", but this probably implies not just
>>> a minimum rust version but also a limited set of crates.
>>
>> I don't think limiting ourselves to the set of crates in the distro is
>> feasible. It's not the way the language works, for example I tried
>> checking if the "cstr" crate exists and I didn't find it in Debian.
>
> Yep, Rust is new enough that it is highly likely that crates will be
> missing in multiple distros.
>
> For ordinary users, cargo will happily download the missing pieces
> so its not an issue.
>
> For distro packagers, they'll just have to either package up the crates,
> or bundle them in their QEMU build. Cargo makes the latter easy at
> least. If distros don't want bundling, they'll need to go the more
> involved route of packaging deps.
>
> IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
> versions we need as the minimum bar.
>
> With regards,
> Daniel
I would like to add that, contrary to pip packages for python, it can be
*trusted* that Rust crates.io will keep packages and their different
versions, as it was designed to be an immutable registry.
Indeed, users can't remove their package, except for strong security
issues, to the opposite of pip.
Besides offline compilation scenario, are there any other reason for
wanting to use distro packages instead of letting cargo do the work?
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-11 21:23 ` Pierrick Bouvier
@ 2024-07-12 6:14 ` Manos Pitsidianakis
0 siblings, 0 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-12 6:14 UTC (permalink / raw)
To: Pierrick Bouvier, Daniel P. Berrangé , Paolo Bonzini
Cc: Peter Maydell, Alex Benné e, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé , Zhao Liu,
Gustavo Romero, rowan.hart, Richard Henderson, John Snow,
Cleber Rosa
On Fri, 12 Jul 2024 00:23, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>On 7/9/24 06:00, Daniel P. Berrangé wrote:
>> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
>>> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> * what is the actual baseline requirement? We definitely want
>>>> to support "using rustup on an older system" (should be no
>>>> problem) and "current distro building QEMU using the distro's
>>>> rust", I assume. It would certainly be nice to have "building
>>>> QEMU on the older-but-still-in-our-support-list distro releases
>>>> with that distro's rust", but this probably implies not just
>>>> a minimum rust version but also a limited set of crates.
>>>
>>> I don't think limiting ourselves to the set of crates in the distro is
>>> feasible. It's not the way the language works, for example I tried
>>> checking if the "cstr" crate exists and I didn't find it in Debian.
>>
>> Yep, Rust is new enough that it is highly likely that crates will be
>> missing in multiple distros.
>>
>> For ordinary users, cargo will happily download the missing pieces
>> so its not an issue.
>>
>> For distro packagers, they'll just have to either package up the crates,
>> or bundle them in their QEMU build. Cargo makes the latter easy at
>> least. If distros don't want bundling, they'll need to go the more
>> involved route of packaging deps.
>>
>> IOW, from a distro POV, IMHO, we should focus on the Rust toolchain
>> versions we need as the minimum bar.
>>
>> With regards,
>> Daniel
>
>I would like to add that, contrary to pip packages for python, it can be
>*trusted* that Rust crates.io will keep packages and their different
>versions, as it was designed to be an immutable registry.
>
>Indeed, users can't remove their package, except for strong security
>issues, to the opposite of pip.
>
>Besides offline compilation scenario, are there any other reason for
>wanting to use distro packages instead of letting cargo do the work?
Only distribution packaging is the concern here, where packaged build
dependencies may be required.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 12:28 ` Paolo Bonzini
2024-07-09 13:00 ` Daniel P. Berrangé
@ 2024-07-09 14:23 ` Alex Bennée
2024-07-10 15:03 ` Zhao Liu
2 siblings, 0 replies; 38+ messages in thread
From: Alex Bennée @ 2024-07-09 14:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi,
Mads Ynddal, Daniel P. Berrangé, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson, John Snow, Cleber Rosa
Paolo Bonzini <pbonzini@redhat.com> writes:
> On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>> * what is the actual baseline requirement? We definitely want
>> to support "using rustup on an older system" (should be no
>> problem) and "current distro building QEMU using the distro's
>> rust", I assume. It would certainly be nice to have "building
>> QEMU on the older-but-still-in-our-support-list distro releases
>> with that distro's rust", but this probably implies not just
>> a minimum rust version but also a limited set of crates.
>
> I don't think limiting ourselves to the set of crates in the distro is
> feasible. It's not the way the language works, for example I tried
> checking if the "cstr" crate exists and I didn't find it in Debian.
>
> We can define a known-good set of versions:
> * either via Cargo.lock (i.e. recording the versions and downloading
> them) or by including sources in the tree
> * at any time in qemu.git, at branching time (three times a year), or
> on every release
>
> (That's six possibilities overall).
>
>> I think (but forget the details) that some of what we've done
>> with Python where we accept that the older-but-still-supported
>> distro will end up taking the "download at build time" path
>> in the build system might apply also for Rust.
>
> Yes, and --without-download may be changed to the "--frozen" flag of cargo.
>
>> * what, on the Rust side, is the version requirement? Presumably
>> there's some features that are pretty much "we really need
>> this and can't do without it" and some which are "this would
>> be pretty awkward not to have but if we have to we can implement
>> some kind of fallback/alternative with a TODO note to come
>> back and clean up when our baseline moves forwards".
>
> Here are the stopping points that I found over the last couple weeks:
>
> 1.56.0: 2021 edition
> 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> crate, see below)
> 1.64.0: std::ffi::c_char
> 1.65.0: Generic Associated Types
> 1.74.0: Clippy can be configured in Cargo.toml
> 1.77.0: C string literals, offset_of!
>
> I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> will be a bit painful, but it can be worked around (and pretty much
> has to be, because 1.77.0 is really new).
>
> As far as I understand, Debian bullseye has 1.63.0 these days and it's
> the oldest platform we have.
I was going to say Bookworm has now superseded Bullseye which will reach
its release + 3 year support point in August. However the version you
mention in the Bookworm one!
>
> Paolo
>
>> At that point we have more information to figure out what
>> if any tradeoff we want to make.
>>
>> thanks
>> -- PMM
>>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 12:28 ` Paolo Bonzini
2024-07-09 13:00 ` Daniel P. Berrangé
2024-07-09 14:23 ` Alex Bennée
@ 2024-07-10 15:03 ` Zhao Liu
2024-07-10 14:50 ` Paolo Bonzini
2 siblings, 1 reply; 38+ messages in thread
From: Zhao Liu @ 2024-07-10 15:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Daniel P. Berrangé,
Marc-André Lureau, Thomas Huth, Markus Armbruster,
Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
rowan.hart, Richard Henderson, John Snow, Cleber Rosa
On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
>
> Here are the stopping points that I found over the last couple weeks:
>
> 1.56.0: 2021 edition
> 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> crate, see below)
> 1.64.0: std::ffi::c_char
> 1.65.0: Generic Associated Types
> 1.74.0: Clippy can be configured in Cargo.toml
> 1.77.0: C string literals, offset_of!
>
> I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> will be a bit painful, but it can be worked around (and pretty much
> has to be, because 1.77.0 is really new).
>
An additional question: does our minimum rust version requirement
indicate that users with this rust version can compile other
dependencies that satisfy QEMU requirements, such as bindgen?
Because I find 1.59.0 can only go to compile bindgen 0.63.0 [1].
[1]: bindgen 0.63.0 MSRV: https://github.com/rust-lang/rust-bindgen/tree/v0.63.0?tab=readme-ov-file#msrv
-Zhao
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-10 15:03 ` Zhao Liu
@ 2024-07-10 14:50 ` Paolo Bonzini
2024-07-11 8:30 ` Zhao Liu
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-10 14:50 UTC (permalink / raw)
To: Zhao Liu
Cc: Peter Maydell, Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Daniel P. Berrangé,
Marc-André Lureau, Thomas Huth, Markus Armbruster,
Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
rowan.hart, Richard Henderson, John Snow, Cleber Rosa
On Wed, Jul 10, 2024 at 4:48 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> >
> > Here are the stopping points that I found over the last couple weeks:
> >
> > 1.56.0: 2021 edition
> > 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> > crate, see below)
> > 1.64.0: std::ffi::c_char
> > 1.65.0: Generic Associated Types
> > 1.74.0: Clippy can be configured in Cargo.toml
> > 1.77.0: C string literals, offset_of!
> >
> > I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> > will be a bit painful, but it can be worked around (and pretty much
> > has to be, because 1.77.0 is really new).
> >
>
> An additional question: does our minimum rust version requirement
> indicate that users with this rust version can compile other
> dependencies that satisfy QEMU requirements, such as bindgen?
Yes (though in the case of bindgen, like cargo and rustc, we'll use it
from the distro; Debian bookworm has 0.60.1 so that's what we'll have
to stick with).
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-10 14:50 ` Paolo Bonzini
@ 2024-07-11 8:30 ` Zhao Liu
0 siblings, 0 replies; 38+ messages in thread
From: Zhao Liu @ 2024-07-11 8:30 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Alex Bennée, Manos Pitsidianakis, qemu-devel,
Stefan Hajnoczi, Mads Ynddal, Daniel P. Berrangé,
Marc-André Lureau, Thomas Huth, Markus Armbruster,
Philippe Mathieu-Daudé, Gustavo Romero, Pierrick Bouvier,
rowan.hart, Richard Henderson, John Snow, Cleber Rosa
On Wed, Jul 10, 2024 at 04:50:10PM +0200, Paolo Bonzini wrote:
> Date: Wed, 10 Jul 2024 16:50:10 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
>
> On Wed, Jul 10, 2024 at 4:48 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > On Tue, Jul 09, 2024 at 02:28:38PM +0200, Paolo Bonzini wrote:
> > >
> > > Here are the stopping points that I found over the last couple weeks:
> > >
> > > 1.56.0: 2021 edition
> > > 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr
> > > crate, see below)
> > > 1.64.0: std::ffi::c_char
> > > 1.65.0: Generic Associated Types
> > > 1.74.0: Clippy can be configured in Cargo.toml
> > > 1.77.0: C string literals, offset_of!
> > >
> > > I think 1.59.0 is pretty much the lower bound. Not having offset_of!
> > > will be a bit painful, but it can be worked around (and pretty much
> > > has to be, because 1.77.0 is really new).
> > >
> >
> > An additional question: does our minimum rust version requirement
> > indicate that users with this rust version can compile other
> > dependencies that satisfy QEMU requirements, such as bindgen?
>
> Yes (though in the case of bindgen, like cargo and rustc, we'll use it
> from the distro; Debian bookworm has 0.60.1 so that's what we'll have
> to stick with).
>
I just did some build tests to hopefully help Manos clarify the gap.
With 1.59.0 rust and 0.60.1 bindgen, I found the big problem is this
version can't support current external crates ("bilge" and
"arbitrary-int").
The lowest possible version of bilge is 0.1.1 (because 0.1.0 contains the
removed unstable feature), but v0.1.1 bilge depends on arbitrary-int
(lowest bound is 1.2.4), which is not supported by v1.59.0 rust. So for
this case either we have to keep raising the minimum rust version, or we
have to reconsider the external crates.
And with 1.63.0, the main problem is core_ffi_c is unstable (once_cell
doesn't matter since it's unneeded). Roughly speaking, it seems that if
we didn't have to use the c type as parameters, this problem could
possibly be worked around.
So maybe 1.63.0 is the proper minimum version and this is aligned with
Debian bullseye.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-09 10:53 ` Alex Bennée
2024-07-09 12:08 ` Peter Maydell
@ 2024-07-09 14:52 ` Alex Bennée
1 sibling, 0 replies; 38+ messages in thread
From: Alex Bennée @ 2024-07-09 14:52 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
Alex Bennée <alex.bennee@linaro.org> writes:
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>
>> Add mechanism to generate rust hw targets that depend on a custom
>> bindgen target for rust bindings to C.
>>
>> This way bindings will be created before the rust crate is compiled.
>>
>> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same name
>> as a target:
>>
>> ninja aarch64-softmmu-generated.rs
>>
>> The way the bindings are generated is:
>>
>> 1. All required C headers are included in a single file, in our case
>> rust/wrapper.h for convenience. Otherwise we'd have to provide a list
>> of headers every time to the bindgen tool.
>>
>> 2. Meson creates a generated_rs target that runs bindgen making sure
>> the architecture etc header dependencies are present.
>>
>> 3. The generated_rs target takes a list of files, type symbols,
>> function symbols to block from being generated. This is not necessary
>> for the bindings to work, but saves us time and space.
>>
>> 4. Meson creates rust hardware target dependencies from the rust_targets
>> dictionary defined in rust/meson.build.
>>
>> Since we cannot declare a dependency on generated_rs before it is
>> declared in meson.build, the rust crate targets must be defined after
>> the generated_rs target for each target architecture is defined. This
>> way meson sets up the dependency tree properly.
>>
>> 5. After compiling each rust crate with the cargo_wrapper.py script,
>> its static library artifact is linked as a `whole-archive` with the
>> final binary.
>>
>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> <snip>
>> +
>> +msrv = {
>> + 'rustc': '1.77.2',
>> + 'cargo': '1.77.2',
>> + 'bindgen': '0.69.4',
>> +}
>
> This is still pretty bleeding edge (it even tripped up on the
> .cargo/bin/cargo I have installed). This needs to be set to the
> baseline which from:
>
> https://wiki.qemu.org/RustInQemu/2022
>
> Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
> says 0.66.0). While it might make sense to delay merging if we are
> waiting for one distro to produce a new LTS we shouldn't be needing
> rustup by default.
Also bindgen, do we need such a new one? On Trixie:
Message:
../../rust/meson.build:41:4: ERROR: Problem encountered: bindgen version 0.66.1 is unsupported: Please upgrade to at least 0.69.4
A full log can be found at /home/alex/lsrc/qemu.git/builds/rust/meson-logs/meson-log.txt
ERROR: meson setup failed
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
2024-07-04 12:15 ` [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency Manos Pitsidianakis
2024-07-08 15:07 ` Paolo Bonzini
2024-07-09 10:53 ` Alex Bennée
@ 2024-07-10 8:55 ` Alex Bennée
2 siblings, 0 replies; 38+ messages in thread
From: Alex Bennée @ 2024-07-10 8:55 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini, John Snow, Cleber Rosa
Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
<snip>
> + # We need one bindings_rs build target per arch target, so give them
> + # arch-specific names.
> + copy = fs.copyfile('rust/wrapper.h',
> + target + '_wrapper.h')
> + bindings_rs = rust_bindgen.bindgen(
> + input: copy,
> + dependencies: arch_deps + lib_deps,
> + output: 'bindings-' + target + '.rs',
> + include_directories: include_directories('.', 'include'),
> + args: [
> + '--ctypes-prefix', 'core::ffi',
> + '--formatter', 'rustfmt',
I guess we also need to detect rustfmt, although it seems to be
non-fatal:
15:59:27 [alex@debian-arm64:~/l/q/b/rust] review/rust-pl011-v4 + ninja
[831/3041] Generating bindings for Rust rustmod-bindgen-aarch64-softmmu_wrapper.h
Failed to run rustfmt: cannot find binary path (non-fatal, continuing)
[3041/3041] Linking target tests/qtest/qos-test
<snip>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v4 3/7] rust: add crate to expose bindings and interfaces
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
2024-07-04 12:15 ` [RFC PATCH v4 1/7] build-sys: Add rust feature option Manos Pitsidianakis
2024-07-04 12:15 ` [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-08 15:40 ` Paolo Bonzini
2024-07-04 12:15 ` [RFC PATCH v4 4/7] rust: add PL011 device model Manos Pitsidianakis
` (4 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson
Add rust/qemu-api, which exposes rust-bindgen generated FFI bindings and
provides some declaration macros for symbols visible to the rest of
QEMU.
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
MAINTAINERS | 7 ++
rust/.cargo/config.toml | 2 +
rust/qemu-api/.gitignore | 2 +
rust/qemu-api/Cargo.lock | 7 ++
rust/qemu-api/Cargo.toml | 59 ++++++++++++++
rust/qemu-api/README.md | 17 ++++
rust/qemu-api/build.rs | 48 +++++++++++
rust/qemu-api/deny.toml | 57 +++++++++++++
rust/qemu-api/meson.build | 0
rust/qemu-api/rustfmt.toml | 1 +
rust/qemu-api/src/bindings.rs | 8 ++
rust/qemu-api/src/definitions.rs | 112 +++++++++++++++++++++++++
rust/qemu-api/src/device_class.rs | 131 ++++++++++++++++++++++++++++++
rust/qemu-api/src/lib.rs | 29 +++++++
rust/qemu-api/src/tests.rs | 48 +++++++++++
rust/rustfmt.toml | 7 ++
16 files changed, 535 insertions(+)
create mode 100644 rust/.cargo/config.toml
create mode 100644 rust/qemu-api/.gitignore
create mode 100644 rust/qemu-api/Cargo.lock
create mode 100644 rust/qemu-api/Cargo.toml
create mode 100644 rust/qemu-api/README.md
create mode 100644 rust/qemu-api/build.rs
create mode 100644 rust/qemu-api/deny.toml
create mode 100644 rust/qemu-api/meson.build
create mode 120000 rust/qemu-api/rustfmt.toml
create mode 100644 rust/qemu-api/src/bindings.rs
create mode 100644 rust/qemu-api/src/definitions.rs
create mode 100644 rust/qemu-api/src/device_class.rs
create mode 100644 rust/qemu-api/src/lib.rs
create mode 100644 rust/qemu-api/src/tests.rs
create mode 100644 rust/rustfmt.toml
diff --git a/MAINTAINERS b/MAINTAINERS
index 6e7b8207fb..8598d38eae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3340,6 +3340,11 @@ F: hw/core/register.c
F: include/hw/register.h
F: include/hw/registerfields.h
+Rust
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Maintained
+F: rust/qemu-api
+
SLIRP
M: Samuel Thibault <samuel.thibault@ens-lyon.org>
S: Maintained
@@ -4233,6 +4238,8 @@ F: scripts/cargo_wrapper.py
F: rust/meson.build
F: rust/wrapper.h
F: rust/.gitignore
+F: rust/rustfmt.toml
+F: rust/.cargo/config.toml
Miscellaneous
-------------
diff --git a/rust/.cargo/config.toml b/rust/.cargo/config.toml
new file mode 100644
index 0000000000..241210ffa7
--- /dev/null
+++ b/rust/.cargo/config.toml
@@ -0,0 +1,2 @@
+[build]
+rustflags = ["-Crelocation-model=pic", "-Ctarget-feature=+crt-static"]
diff --git a/rust/qemu-api/.gitignore b/rust/qemu-api/.gitignore
new file mode 100644
index 0000000000..71eaff2035
--- /dev/null
+++ b/rust/qemu-api/.gitignore
@@ -0,0 +1,2 @@
+# Ignore generated bindings file overrides.
+src/bindings.rs.inc
diff --git a/rust/qemu-api/Cargo.lock b/rust/qemu-api/Cargo.lock
new file mode 100644
index 0000000000..e9c51a243a
--- /dev/null
+++ b/rust/qemu-api/Cargo.lock
@@ -0,0 +1,7 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "qemu_api"
+version = "0.1.0"
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
new file mode 100644
index 0000000000..94a48b9ce9
--- /dev/null
+++ b/rust/qemu-api/Cargo.toml
@@ -0,0 +1,59 @@
+[package]
+name = "qemu_api"
+version = "0.1.0"
+edition = "2021"
+authors = ["Manos Pitsidianakis <manos.pitsidianakis@linaro.org>"]
+license = "GPL-2.0 OR GPL-3.0-or-later"
+readme = "README.md"
+homepage = "https://www.qemu.org"
+description = "Rust bindings for QEMU"
+repository = "https://gitlab.com/epilys/rust-for-qemu"
+resolver = "2"
+publish = false
+keywords = []
+categories = []
+
+[dependencies]
+
+[lints]
+[lints.rustdoc]
+broken_intra_doc_links = "deny"
+redundant_explicit_links = "deny"
+[lints.clippy]
+# lint groups
+correctness = { level = "deny", priority = -1 }
+suspicious = { level = "deny", priority = -1 }
+complexity = { level = "deny", priority = -1 }
+perf = { level = "deny", priority = -1 }
+cargo = { level = "deny", priority = -1 }
+nursery = { level = "deny", priority = -1 }
+style = { level = "deny", priority = -1 }
+# restriction group
+dbg_macro = "deny"
+rc_buffer = "deny"
+as_underscore = "deny"
+assertions_on_result_states = "deny"
+# pedantic group
+doc_markdown = "deny"
+expect_fun_call = "deny"
+borrow_as_ptr = "deny"
+case_sensitive_file_extension_comparisons = "deny"
+cast_lossless = "deny"
+cast_ptr_alignment = "allow"
+large_futures = "deny"
+waker_clone_wake = "deny"
+unused_enumerate_index = "deny"
+unnecessary_fallible_conversions = "deny"
+struct_field_names = "deny"
+manual_hash_one = "deny"
+into_iter_without_iter = "deny"
+option_if_let_else = "deny"
+missing_const_for_fn = "deny"
+significant_drop_tightening = "deny"
+multiple_crate_versions = "deny"
+significant_drop_in_scrutinee = "deny"
+cognitive_complexity = "deny"
+missing_safety_doc = "allow"
+
+# Do not include in any global workspace
+[workspace]
diff --git a/rust/qemu-api/README.md b/rust/qemu-api/README.md
new file mode 100644
index 0000000000..f16a1a929d
--- /dev/null
+++ b/rust/qemu-api/README.md
@@ -0,0 +1,17 @@
+# QEMU bindings and API wrappers
+
+This library exports helper Rust types, Rust macros and C FFI bindings for internal QEMU APIs.
+
+The C bindings can be generated with `bindgen`, using this build target:
+
+```console
+$ ninja bindings-aarch64-softmmu.rs
+```
+
+## Generate Rust documentation
+
+To generate docs for this crate, including private items:
+
+```sh
+cargo doc --no-deps --document-private-items
+```
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
new file mode 100644
index 0000000000..13164f8371
--- /dev/null
+++ b/rust/qemu-api/build.rs
@@ -0,0 +1,48 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use std::{env, path::Path};
+
+fn main() {
+ println!("cargo:rerun-if-env-changed=MESON_BUILD_ROOT");
+ println!("cargo:rerun-if-changed=src/bindings.rs.inc");
+
+ let out_dir = env::var_os("OUT_DIR").unwrap();
+
+ if let Some(build_dir) = std::env::var_os("MESON_BUILD_ROOT") {
+ let mut build_dir = Path::new(&build_dir).to_path_buf();
+ let mut out_dir = Path::new(&out_dir).to_path_buf();
+ assert!(
+ build_dir.exists(),
+ "MESON_BUILD_ROOT value does not exist on filesystem: {}",
+ build_dir.display()
+ );
+ assert!(
+ build_dir.is_dir(),
+ "MESON_BUILD_ROOT value is not actually a directory: {}",
+ build_dir.display()
+ );
+ // TODO: add logic for other guest target architectures.
+ build_dir.push("bindings-aarch64-softmmu.rs");
+ let bindings_rs = build_dir;
+ assert!(
+ bindings_rs.exists(),
+ "MESON_BUILD_ROOT/bindings-aarch64-softmmu.rs does not exist on filesystem: {}",
+ bindings_rs.display()
+ );
+ assert!(
+ bindings_rs.is_file(),
+ "MESON_BUILD_ROOT/bindings-aarch64-softmmu.rs is not a file: {}",
+ bindings_rs.display()
+ );
+ out_dir.push("bindings.rs");
+ std::fs::copy(bindings_rs, out_dir).unwrap();
+ println!("cargo:rustc-cfg=MESON_BINDINGS_RS");
+ } else if !Path::new("src/bindings.rs.inc").exists() {
+ panic!(
+ "No generated C bindings found! Either build them manually with bindgen or with meson \
+ (`ninja bindings-aarch64-softmmu.rs`) and copy them to src/bindings.rs.inc, or build \
+ through meson."
+ );
+ }
+}
diff --git a/rust/qemu-api/deny.toml b/rust/qemu-api/deny.toml
new file mode 100644
index 0000000000..3992380509
--- /dev/null
+++ b/rust/qemu-api/deny.toml
@@ -0,0 +1,57 @@
+# cargo-deny configuration file
+
+[graph]
+targets = [
+ "aarch64-unknown-linux-gnu",
+ "x86_64-unknown-linux-gnu",
+ "x86_64-apple-darwin",
+ "aarch64-apple-darwin",
+ "x86_64-pc-windows-gnu",
+ "aarch64-pc-windows-gnullvm",
+]
+#exclude = []
+all-features = false
+no-default-features = false
+#features = []
+
+[output]
+feature-depth = 1
+
+[advisories]
+db-path = "$CARGO_HOME/advisory-dbs"
+db-urls = ["https://github.com/rustsec/advisory-db"]
+ignore = []
+
+[licenses]
+allow = [
+ "GPL-2.0",
+ "MIT",
+ "Apache-2.0",
+ "Unicode-DFS-2016",
+]
+confidence-threshold = 0.8
+exceptions = []
+
+[licenses.private]
+ignore = false
+registries = []
+
+[bans]
+multiple-versions = "warn"
+wildcards = "deny"
+# The graph highlighting used when creating dotgraphs for crates
+# with multiple versions
+# * lowest-version - The path to the lowest versioned duplicate is highlighted
+# * simplest-path - The path to the version with the fewest edges is highlighted
+# * all - Both lowest-version and simplest-path are used
+highlight = "all"
+workspace-default-features = "allow"
+external-default-features = "allow"
+allow = []
+deny = []
+
+[sources]
+unknown-registry = "deny"
+unknown-git = "deny"
+allow-registry = ["https://github.com/rust-lang/crates.io-index"]
+allow-git = []
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/rust/qemu-api/rustfmt.toml b/rust/qemu-api/rustfmt.toml
new file mode 120000
index 0000000000..39f97b043b
--- /dev/null
+++ b/rust/qemu-api/rustfmt.toml
@@ -0,0 +1 @@
+../rustfmt.toml
\ No newline at end of file
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
new file mode 100644
index 0000000000..1220a3d8f0
--- /dev/null
+++ b/rust/qemu-api/src/bindings.rs
@@ -0,0 +1,8 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+#[cfg(MESON_BINDINGS_RS)]
+include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
+
+#[cfg(not(MESON_BINDINGS_RS))]
+include!("bindings.rs.inc");
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
new file mode 100644
index 0000000000..04b3a4d565
--- /dev/null
+++ b/rust/qemu-api/src/definitions.rs
@@ -0,0 +1,112 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+//! Definitions required by QEMU when registering a device.
+
+use crate::bindings::*;
+
+unsafe impl Sync for TypeInfo {}
+unsafe impl Sync for VMStateDescription {}
+
+#[macro_export]
+macro_rules! module_init {
+ ($func:expr, $type:expr) => {
+ #[used]
+ #[cfg_attr(target_os = "linux", link_section = ".ctors")]
+ #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
+ #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
+ pub static LOAD_MODULE: extern "C" fn() = {
+ assert!($type < $crate::bindings::module_init_type_MODULE_INIT_MAX);
+
+ extern "C" fn __load() {
+ // ::std::panic::set_hook(::std::boxed::Box::new(|_| {}));
+
+ unsafe {
+ $crate::bindings::register_module_init(Some($func), $type);
+ }
+ }
+
+ __load
+ };
+ };
+ (qom: $func:ident => $body:block) => {
+ // NOTE: To have custom identifiers for the ctor func we need to either supply
+ // them directly as a macro argument or create them with a proc macro.
+ #[used]
+ #[cfg_attr(target_os = "linux", link_section = ".ctors")]
+ #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
+ #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
+ pub static LOAD_MODULE: extern "C" fn() = {
+ extern "C" fn __load() {
+ // ::std::panic::set_hook(::std::boxed::Box::new(|_| {}));
+ #[no_mangle]
+ unsafe extern "C" fn $func() {
+ $body
+ }
+
+ unsafe {
+ $crate::bindings::register_module_init(
+ Some($func),
+ $crate::bindings::module_init_type_MODULE_INIT_QOM,
+ );
+ }
+ }
+
+ __load
+ };
+ };
+}
+
+#[macro_export]
+macro_rules! type_info {
+ ($(#[$outer:meta])*
+ $name:ident: $t:ty,
+ $(name: $tname:expr,)*
+ $(parent: $pname:expr,)*
+ $(instance_init: $ii_fn:expr,)*
+ $(instance_post_init: $ipi_fn:expr,)*
+ $(instance_finalize: $if_fn:expr,)*
+ $(abstract_: $a_val:expr,)*
+ $(class_init: $ci_fn:expr,)*
+ $(class_base_init: $cbi_fn:expr,)*
+ ) => {
+ #[used]
+ $(#[$outer])*
+ pub static $name: $crate::bindings::TypeInfo = $crate::bindings::TypeInfo {
+ $(name: {
+ #[used]
+ static TYPE_NAME: &::core::ffi::CStr = $tname;
+ $tname.as_ptr()
+ },)*
+ $(parent: {
+ #[used]
+ static PARENT_TYPE_NAME: &::core::ffi::CStr = $pname;
+ $pname.as_ptr()
+ },)*
+ instance_size: ::core::mem::size_of::<$t>(),
+ instance_align: ::core::mem::align_of::<$t>(),
+ $(
+ instance_init: $ii_fn,
+ )*
+ $(
+ instance_post_init: $ipi_fn,
+ )*
+ $(
+ instance_finalize: $if_fn,
+ )*
+ $(
+ abstract_: $a_val,
+ )*
+ class_size: 0,
+ $(
+ class_init: $ci_fn,
+ )*
+ $(
+ class_base_init: $cbi_fn,
+ )*
+ class_data: core::ptr::null_mut(),
+ interfaces: core::ptr::null_mut(),
+ ..unsafe { MaybeUninit::<$crate::bindings::TypeInfo>::zeroed().assume_init() }
+ };
+ }
+}
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
new file mode 100644
index 0000000000..855d70364a
--- /dev/null
+++ b/rust/qemu-api/src/device_class.rs
@@ -0,0 +1,131 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use std::sync::OnceLock;
+
+use crate::bindings::*;
+
+unsafe impl Send for Property {}
+unsafe impl Sync for Property {}
+
+#[macro_export]
+macro_rules! device_class_init {
+ ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, reset_fn => $reset_fn:expr, vmsd => $vmsd:ident$(,)*) => {
+ #[no_mangle]
+ pub unsafe extern "C" fn $func(
+ klass: *mut $crate::bindings::ObjectClass,
+ _: *mut ::core::ffi::c_void,
+ ) {
+ let mut dc =
+ ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap();
+ dc.as_mut().realize = $realize_fn;
+ dc.as_mut().reset = $reset_fn;
+ dc.as_mut().vmsd = &$vmsd;
+ $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr());
+ }
+ };
+}
+
+#[macro_export]
+macro_rules! define_property {
+ ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
+ $crate::bindings::Property {
+ name: {
+ #[used]
+ static _TEMP: &::core::ffi::CStr = $name;
+ _TEMP.as_ptr()
+ },
+ info: $prop,
+ offset: ::core::mem::offset_of!($state, $field)
+ .try_into()
+ .expect("Could not fit offset value to type"),
+ bitnr: 0,
+ bitmask: 0,
+ set_default: true,
+ defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval.into() },
+ arrayoffset: 0,
+ arrayinfo: ::core::ptr::null(),
+ arrayfieldsize: 0,
+ link_type: ::core::ptr::null(),
+ }
+ };
+ ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => {
+ $crate::bindings::Property {
+ name: {
+ #[used]
+ static _TEMP: &::core::ffi::CStr = $name;
+ _TEMP.as_ptr()
+ },
+ info: $prop,
+ offset: ::core::mem::offset_of!($state, $field)
+ .try_into()
+ .expect("Could not fit offset value to type"),
+ bitnr: 0,
+ bitmask: 0,
+ set_default: false,
+ defval: $crate::bindings::Property__bindgen_ty_1 { i: 0 },
+ arrayoffset: 0,
+ arrayinfo: ::core::ptr::null(),
+ arrayfieldsize: 0,
+ link_type: ::core::ptr::null(),
+ }
+ };
+}
+
+#[repr(C)]
+pub struct Properties<const N: usize>(pub OnceLock<[Property; N]>, pub fn() -> [Property; N]);
+
+impl<const N: usize> Properties<N> {
+ pub unsafe fn as_mut_ptr(&mut self) -> *mut Property {
+ _ = self.0.get_or_init(self.1);
+ self.0.get_mut().unwrap().as_mut_ptr()
+ }
+}
+
+#[macro_export]
+macro_rules! declare_properties {
+ ($ident:ident, $($prop:expr),*$(,)*) => {
+
+ const fn _calc_prop_len() -> usize {
+ let mut len = 1;
+ $({
+ _ = stringify!($prop);
+ len += 1;
+ })*
+ len
+ }
+ const PROP_LEN: usize = _calc_prop_len();
+
+ #[no_mangle]
+ fn _make_properties() -> [$crate::bindings::Property; PROP_LEN] {
+ [
+ $($prop),*,
+ unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() },
+ ]
+ }
+
+ #[no_mangle]
+ pub static mut $ident: $crate::device_class::Properties<PROP_LEN> = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties);
+ };
+}
+
+#[macro_export]
+macro_rules! vm_state_description {
+ ($(#[$outer:meta])*
+ $name:ident,
+ $(name: $vname:expr,)*
+ $(unmigratable: $um_val:expr,)*
+ ) => {
+ #[used]
+ $(#[$outer])*
+ pub static $name: $crate::bindings::VMStateDescription = $crate::bindings::VMStateDescription {
+ $(name: {
+ #[used]
+ static VMSTATE_NAME: &::core::ffi::CStr = $vname;
+ $vname.as_ptr()
+ },)*
+ unmigratable: true,
+ ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::VMStateDescription>::zeroed().assume_init() }
+ };
+ }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
new file mode 100644
index 0000000000..74825c84e7
--- /dev/null
+++ b/rust/qemu-api/src/lib.rs
@@ -0,0 +1,29 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+#![doc = include_str!("../README.md")]
+
+// FIXME: remove improper_ctypes
+#[allow(
+ improper_ctypes_definitions,
+ improper_ctypes,
+ non_camel_case_types,
+ non_snake_case,
+ non_upper_case_globals
+)]
+#[allow(
+ clippy::missing_const_for_fn,
+ clippy::useless_transmute,
+ clippy::too_many_arguments,
+ clippy::approx_constant,
+ clippy::use_self,
+ clippy::cast_lossless,
+)]
+#[rustfmt::skip]
+pub mod bindings;
+
+pub mod definitions;
+pub mod device_class;
+
+#[cfg(test)]
+mod tests;
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
new file mode 100644
index 0000000000..88c26308ee
--- /dev/null
+++ b/rust/qemu-api/src/tests.rs
@@ -0,0 +1,48 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use crate::{
+ bindings::*, declare_properties, define_property, device_class_init, vm_state_description,
+};
+
+#[test]
+fn test_device_decl_macros() {
+ // Test that macros can compile.
+ vm_state_description! {
+ VMSTATE,
+ name: c"name",
+ unmigratable: true,
+ }
+
+ #[repr(C)]
+ pub struct DummyState {
+ pub char_backend: CharBackend,
+ pub migrate_clock: bool,
+ }
+
+ declare_properties! {
+ DUMMY_PROPERTIES,
+ define_property!(
+ c"chardev",
+ DummyState,
+ char_backend,
+ unsafe { &qdev_prop_chr },
+ CharBackend
+ ),
+ define_property!(
+ c"migrate-clk",
+ DummyState,
+ migrate_clock,
+ unsafe { &qdev_prop_bool },
+ bool
+ ),
+ }
+
+ device_class_init! {
+ dummy_class_init,
+ props => DUMMY_PROPERTIES,
+ realize_fn => None,
+ reset_fn => None,
+ vmsd => VMSTATE,
+ }
+}
diff --git a/rust/rustfmt.toml b/rust/rustfmt.toml
new file mode 100644
index 0000000000..ebecb99fe0
--- /dev/null
+++ b/rust/rustfmt.toml
@@ -0,0 +1,7 @@
+edition = "2021"
+format_generated_files = false
+format_code_in_doc_comments = true
+format_strings = true
+imports_granularity = "Crate"
+group_imports = "StdExternalCrate"
+wrap_comments = true
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 3/7] rust: add crate to expose bindings and interfaces
2024-07-04 12:15 ` [RFC PATCH v4 3/7] rust: add crate to expose bindings and interfaces Manos Pitsidianakis
@ 2024-07-08 15:40 ` Paolo Bonzini
0 siblings, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 15:40 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson
On 7/4/24 14:15, Manos Pitsidianakis wrote:
> Add rust/qemu-api, which exposes rust-bindgen generated FFI bindings and
> provides some declaration macros for symbols visible to the rest of
> QEMU.
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> MAINTAINERS | 7 ++
> rust/.cargo/config.toml | 2 +
> rust/qemu-api/.gitignore | 2 +
> rust/qemu-api/Cargo.lock | 7 ++
> rust/qemu-api/Cargo.toml | 59 ++++++++++++++
> rust/qemu-api/README.md | 17 ++++
> rust/qemu-api/build.rs | 48 +++++++++++
> rust/qemu-api/deny.toml | 57 +++++++++++++
> rust/qemu-api/meson.build | 0
> rust/qemu-api/rustfmt.toml | 1 +
> rust/qemu-api/src/bindings.rs | 8 ++
> rust/qemu-api/src/definitions.rs | 112 +++++++++++++++++++++++++
> rust/qemu-api/src/device_class.rs | 131 ++++++++++++++++++++++++++++++
I'd rather put the various definitions in directories that roughly
correspond to the C files, so rust/qemu-api/src/{util,qom,hw/core/}.
> +correctness = { level = "deny", priority = -1 }
> +suspicious = { level = "deny", priority = -1 }
> +complexity = { level = "deny", priority = -1 }
> +perf = { level = "deny", priority = -1 }
> +cargo = { level = "deny", priority = -1 }
> +nursery = { level = "deny", priority = -1 }
> +style = { level = "deny", priority = -1 }
Groups are problematic because they can (and will) cause breakage when
new failures are added. I think we should have a non-fatal job to test
with groups, but by default we can add a large number of lints and
"allow" the groups.
> +cast_ptr_alignment = "allow"
Why?
> +missing_safety_doc = "allow"
Please add it to individual files at the top.
> diff --git a/rust/qemu-api/README.md b/rust/qemu-api/README.md
> new file mode 100644
> index 0000000000..f16a1a929d
> --- /dev/null
> +++ b/rust/qemu-api/README.md
> @@ -0,0 +1,17 @@
> +# QEMU bindings and API wrappers
> +
> +This library exports helper Rust types, Rust macros and C FFI bindings for internal QEMU APIs.
> +
> +The C bindings can be generated with `bindgen`, using this build target:
> +
> +```console
> +$ ninja bindings-aarch64-softmmu.rs
> +```
> +
> +## Generate Rust documentation
> +
> +To generate docs for this crate, including private items:
> +
> +```sh
> +cargo doc --no-deps --document-private-items
> +```
> diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
> new file mode 100644
> index 0000000000..13164f8371
> --- /dev/null
> +++ b/rust/qemu-api/build.rs
> @@ -0,0 +1,48 @@
> +// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
> +
> +use std::{env, path::Path};
> +
> +fn main() {
> + println!("cargo:rerun-if-env-changed=MESON_BUILD_ROOT");
> + println!("cargo:rerun-if-changed=src/bindings.rs.inc");
> +
> + let out_dir = env::var_os("OUT_DIR").unwrap();
> +
> + if let Some(build_dir) = std::env::var_os("MESON_BUILD_ROOT") {
> + let mut build_dir = Path::new(&build_dir).to_path_buf();
> + let mut out_dir = Path::new(&out_dir).to_path_buf();
> + assert!(
> + build_dir.exists(),
> + "MESON_BUILD_ROOT value does not exist on filesystem: {}",
> + build_dir.display()
> + );
> + assert!(
> + build_dir.is_dir(),
> + "MESON_BUILD_ROOT value is not actually a directory: {}",
> + build_dir.display()
> + );
> + // TODO: add logic for other guest target architectures.
> + build_dir.push("bindings-aarch64-softmmu.rs");
> + let bindings_rs = build_dir;
> + assert!(
> + bindings_rs.exists(),
> + "MESON_BUILD_ROOT/bindings-aarch64-softmmu.rs does not exist on filesystem: {}",
> + bindings_rs.display()
> + );
> + assert!(
> + bindings_rs.is_file(),
> + "MESON_BUILD_ROOT/bindings-aarch64-softmmu.rs is not a file: {}",
> + bindings_rs.display()
> + );
> + out_dir.push("bindings.rs");
> + std::fs::copy(bindings_rs, out_dir).unwrap();
> + println!("cargo:rustc-cfg=MESON_BINDINGS_RS");
> + } else if !Path::new("src/bindings.rs.inc").exists() {
> + panic!(
> + "No generated C bindings found! Either build them manually with bindgen or with meson \
> + (`ninja bindings-aarch64-softmmu.rs`) and copy them to src/bindings.rs.inc, or build \
> + through meson."
> + );
> + }
> +}
> diff --git a/rust/qemu-api/deny.toml b/rust/qemu-api/deny.toml
> new file mode 100644
> index 0000000000..3992380509
> --- /dev/null
> +++ b/rust/qemu-api/deny.toml
> @@ -0,0 +1,57 @@
> +# cargo-deny configuration file
> +
> +[graph]
> +targets = [
> + "aarch64-unknown-linux-gnu",
> + "x86_64-unknown-linux-gnu",
> + "x86_64-apple-darwin",
> + "aarch64-apple-darwin",
> + "x86_64-pc-windows-gnu",
> + "aarch64-pc-windows-gnullvm",
Do we actually support LLVM Windows targets?
> +unsafe impl Sync for TypeInfo {}
> +unsafe impl Sync for VMStateDescription {}
> +
> +#[macro_export]
> +macro_rules! module_init {
> + ($func:expr, $type:expr) => {
> + #[used]
> + #[cfg_attr(target_os = "linux", link_section = ".ctors")]
> + #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
> + #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
I'd rather use the ctor crate. Also, this should be src/util/module.rs
so that it's easy to find the matching C source.
> + pub static LOAD_MODULE: extern "C" fn() = {
> + assert!($type < $crate::bindings::module_init_type_MODULE_INIT_MAX);
> +
> + extern "C" fn __load() {
> + // ::std::panic::set_hook(::std::boxed::Box::new(|_| {}));
> +
> + unsafe {
> + $crate::bindings::register_module_init(Some($func), $type);
> + }
> + }
> +
> + __load
> + };
> + };
> + (qom: $func:ident => $body:block) => {
> + // NOTE: To have custom identifiers for the ctor func we need to either supply
> + // them directly as a macro argument or create them with a proc macro.
> + #[used]
> + #[cfg_attr(target_os = "linux", link_section = ".ctors")]
> + #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
> + #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
> + pub static LOAD_MODULE: extern "C" fn() = {
> + extern "C" fn __load() {
> + // ::std::panic::set_hook(::std::boxed::Box::new(|_| {}));
> + #[no_mangle]
> + unsafe extern "C" fn $func() {
> + $body
> + }
> +
> + unsafe {
> + $crate::bindings::register_module_init(
> + Some($func),
> + $crate::bindings::module_init_type_MODULE_INIT_QOM,
> + );
> + }
> + }
> +
> + __load
> + };
> + };
> +}
> +
> +#[macro_export]
> +macro_rules! type_info {
> + ($(#[$outer:meta])*
> + $name:ident: $t:ty,
> + $(name: $tname:expr,)*
> + $(parent: $pname:expr,)*
> + $(instance_init: $ii_fn:expr,)*
> + $(instance_post_init: $ipi_fn:expr,)*
> + $(instance_finalize: $if_fn:expr,)*
> + $(abstract_: $a_val:expr,)*
> + $(class_init: $ci_fn:expr,)*
> + $(class_base_init: $cbi_fn:expr,)*
> + ) => {
> + #[used]
> + $(#[$outer])*
> + pub static $name: $crate::bindings::TypeInfo = $crate::bindings::TypeInfo {
> + $(name: {
> + #[used]
> + static TYPE_NAME: &::core::ffi::CStr = $tname;
> + $tname.as_ptr()
> + },)*
Please use the cstr crate. Even better, add a trait
pub unsafe trait ObjectType: Sized {
const TYPE_NAME: &'static CStr;
}
and then just <$t as ObjectType>::TYPE_NAME
Likewise, the parent could be mandatory like
+ $name:ident: $t:ty($p:ty)
and use <$p as ObjectType>::TYPE_NAME.
> + ..unsafe { MaybeUninit::<$crate::bindings::TypeInfo>::zeroed().assume_init() }
I suggest something like the Zeroed trait I had in my experiment:
==========
#![allow(clippy::undocumented_unsafe_blocks)]
use std::mem::MaybeUninit;
/// Trait providing an easy way to obtain an all-zero
/// value for a struct
///
/// # Safety
///
/// Only add this to a type if `MaybeUninit::zeroed().assume_init()`
/// is valid for that type.
pub unsafe trait Zeroed: Sized {
fn zeroed() -> Self {
// SAFETY: If this weren't safe, just do not add the
// trait to a type.
unsafe { MaybeUninit::zeroed().assume_init() }
}
}
// Put here all the impls that you need for the bindgen-provided types.
unsafe impl Zeroed for crate::bindings::TypeInfo {}
=========
So that you can simply write Zeroed::zeroed(); there are other
occurrences of this for Property, VMStateDescription, MemoryRegionOps.
> +use std::sync::OnceLock;
OnceLock is not needed, initialization is single-threaded and executed
once only. You can just make declare_properties return a fn() and
$crate::bindings::device_class_set_props(dc.as_mut(), $props());
> + dc.as_mut().realize = $realize_fn;
> + dc.as_mut().reset = $reset_fn;
Nice. Might also add the same trick as type_info!, for example
$(realize: $realize_fn:expr,)*, to remove the "Some()".
> +#[macro_export]
> +macro_rules! define_property {
> + ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
> + $crate::bindings::Property {
> + name: {
> + #[used]
> + static _TEMP: &::core::ffi::CStr = $name;
> + _TEMP.as_ptr()
> + },
Also please use cstr!() here.
> + bitnr: 0,
> + bitmask: 0,
> + set_default: true,
> + defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval.into() },
> + arrayoffset: 0,
> + arrayinfo: ::core::ptr::null(),
> + arrayfieldsize: 0,
> + link_type: ::core::ptr::null(),
> + }
> + };
I think you can just use "..Zeroed::zeroed()" instead of listing all the
fields one by one.
> +#[repr(C)]
> +pub struct Properties<const N: usize>(pub OnceLock<[Property; N]>, pub fn() -> [Property; N]);
> +
> +impl<const N: usize> Properties<N> {
> + pub unsafe fn as_mut_ptr(&mut self) -> *mut Property {
> + _ = self.0.get_or_init(self.1);
> + self.0.get_mut().unwrap().as_mut_ptr()
> + }
> + > + #[no_mangle]
> + pub static mut $ident: $crate::device_class::Properties<PROP_LEN> = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties);
> + };
> +}
> +
> +#[macro_export]
> +macro_rules! vm_state_description {
> + ($(#[$outer:meta])*
> + $name:ident,
> + $(name: $vname:expr,)*
> + $(unmigratable: $um_val:expr,)*
> + ) => {
> + #[used]
> + $(#[$outer])*
> + pub static $name: $crate::bindings::VMStateDescription = $crate::bindings::VMStateDescription {
> + $(name: {
> + #[used]
> + static VMSTATE_NAME: &::core::ffi::CStr = $vname;
> + $vname.as_ptr()
> + },)*
> + unmigratable: true,
> + ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::VMStateDescription>::zeroed().assume_init() }
> + };
> + }
> +}
> diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
> new file mode 100644
> index 0000000000..74825c84e7
> --- /dev/null
> +++ b/rust/qemu-api/src/lib.rs
> @@ -0,0 +1,29 @@
> +// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
> +
> +#![doc = include_str!("../README.md")]
> +
> +// FIXME: remove improper_ctypes
> +#[allow(
> + improper_ctypes_definitions,
> + improper_ctypes,
> + non_camel_case_types,
> + non_snake_case,
> + non_upper_case_globals
> +)]
> +#[allow(
> + clippy::missing_const_for_fn,
> + clippy::useless_transmute,
This one definitely shouldn't be there.
Paolo
> + clippy::too_many_arguments,
> + clippy::approx_constant,
> + clippy::use_self,
> + clippy::cast_lossless,
> +)]
> +#[rustfmt::skip]
> +pub mod bindings;
> +
> +pub mod definitions;
> +pub mod device_class;
> +
> +#[cfg(test)]
> +mod tests;
> diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
> new file mode 100644
> index 0000000000..88c26308ee
> --- /dev/null
> +++ b/rust/qemu-api/src/tests.rs
> @@ -0,0 +1,48 @@
> +// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
> +
> +use crate::{
> + bindings::*, declare_properties, define_property, device_class_init, vm_state_description,
> +};
> +
> +#[test]
> +fn test_device_decl_macros() {
> + // Test that macros can compile.
> + vm_state_description! {
> + VMSTATE,
> + name: c"name",
> + unmigratable: true,
> + }
> +
> + #[repr(C)]
> + pub struct DummyState {
> + pub char_backend: CharBackend,
> + pub migrate_clock: bool,
> + }
> +
> + declare_properties! {
> + DUMMY_PROPERTIES,
> + define_property!(
> + c"chardev",
> + DummyState,
> + char_backend,
> + unsafe { &qdev_prop_chr },
> + CharBackend
> + ),
> + define_property!(
> + c"migrate-clk",
> + DummyState,
> + migrate_clock,
> + unsafe { &qdev_prop_bool },
> + bool
> + ),
> + }
> +
> + device_class_init! {
> + dummy_class_init,
> + props => DUMMY_PROPERTIES,
> + realize_fn => None,
> + reset_fn => None,
> + vmsd => VMSTATE,
> + }
> +}
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v4 4/7] rust: add PL011 device model
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
` (2 preceding siblings ...)
2024-07-04 12:15 ` [RFC PATCH v4 3/7] rust: add crate to expose bindings and interfaces Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-08 16:07 ` Paolo Bonzini
2024-07-04 12:15 ` [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes Manos Pitsidianakis
` (3 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Paolo Bonzini
This commit adds a re-implementation of hw/char/pl011.c in Rust.
How to build:
1. Make sure rust, cargo and bindgen (cargo install bindgen-cli) are
installed
2. Configure a QEMU build with:
--enable-system --target-list=aarch64-softmmu --enable-with-rust
3. Launching a VM with qemu-system-aarch64 should use the Rust version
of the pl011 device (unless it is not set up so in hw/arm/virt.c; the
type of the UART device is hardcoded).
To confirm, inspect `info qom-tree` in the monitor and look for an
`x-pl011-rust` device.
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
MAINTAINERS | 5 +
meson.build | 4 +
rust/meson.build | 2 +
rust/pl011/.gitignore | 2 +
rust/pl011/Cargo.lock | 125 ++++++++
rust/pl011/Cargo.toml | 67 ++++
rust/pl011/README.md | 31 ++
rust/pl011/deny.toml | 57 ++++
rust/pl011/meson.build | 7 +
rust/pl011/rustfmt.toml | 1 +
rust/pl011/src/definitions.rs | 39 +++
rust/pl011/src/device.rs | 509 ++++++++++++++++++++++++++++++
rust/pl011/src/device_class.rs | 48 +++
rust/pl011/src/lib.rs | 556 +++++++++++++++++++++++++++++++++
rust/pl011/src/memory_ops.rs | 45 +++
15 files changed, 1498 insertions(+)
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/deny.toml
create mode 100644 rust/pl011/meson.build
create mode 120000 rust/pl011/rustfmt.toml
create mode 100644 rust/pl011/src/definitions.rs
create mode 100644 rust/pl011/src/device.rs
create mode 100644 rust/pl011/src/device_class.rs
create mode 100644 rust/pl011/src/lib.rs
create mode 100644 rust/pl011/src/memory_ops.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index 8598d38eae..fedee0ddef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1186,6 +1186,11 @@ F: include/hw/*/microbit*.h
F: tests/qtest/microbit-test.c
F: docs/system/arm/nrf.rst
+ARM PL011 Rust device
+M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+S: Maintained
+F: rust/pl011/
+
AVR Machines
-------------
diff --git a/meson.build b/meson.build
index 71011fd3b3..945ce6aaaf 100644
--- a/meson.build
+++ b/meson.build
@@ -296,6 +296,10 @@ if get_option('with_rust').allowed()
endif
with_rust = cargo.found()
+if with_rust
+ subdir('rust')
+endif
+
# default flags for all hosts
# We use -fwrapv to tell the compiler that we require a C dialect where
# left shift of signed integers is well defined and has the expected
diff --git a/rust/meson.build b/rust/meson.build
index 5fdc2621a3..21115ac56d 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -93,6 +93,8 @@ subdir('qemu-api')
# bindgen dependency is declared.
rust_hw_target_list = {}
+subdir('pl011')
+
foreach rust_hw_target, rust_hws: rust_hw_target_list
foreach rust_hw_dev: rust_hws
crate_metadata = {
diff --git a/rust/pl011/.gitignore b/rust/pl011/.gitignore
new file mode 100644
index 0000000000..71eaff2035
--- /dev/null
+++ b/rust/pl011/.gitignore
@@ -0,0 +1,2 @@
+# Ignore generated bindings file overrides.
+src/bindings.rs.inc
diff --git a/rust/pl011/Cargo.lock b/rust/pl011/Cargo.lock
new file mode 100644
index 0000000000..411bfed9c9
--- /dev/null
+++ b/rust/pl011/Cargo.lock
@@ -0,0 +1,125 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "arbitrary-int"
+version = "1.2.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d"
+
+[[package]]
+name = "bilge"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dc707ed8ebf81de5cd6c7f48f54b4c8621760926cdf35a57000747c512e67b57"
+dependencies = [
+ "arbitrary-int",
+ "bilge-impl",
+]
+
+[[package]]
+name = "bilge-impl"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8"
+dependencies = [
+ "itertools",
+ "proc-macro-error",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "either"
+version = "1.12.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
+
+[[package]]
+name = "itertools"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57"
+dependencies = [
+ "either",
+]
+
+[[package]]
+name = "pl011"
+version = "0.1.0"
+dependencies = [
+ "arbitrary-int",
+ "bilge",
+ "bilge-impl",
+ "qemu_api",
+]
+
+[[package]]
+name = "proc-macro-error"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
+dependencies = [
+ "proc-macro-error-attr",
+ "proc-macro2",
+ "quote",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro-error-attr"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.84"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6"
+dependencies = [
+ "unicode-ident",
+]
+
+[[package]]
+name = "qemu_api"
+version = "0.1.0"
+
+[[package]]
+name = "quote"
+version = "1.0.36"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7"
+dependencies = [
+ "proc-macro2",
+]
+
+[[package]]
+name = "syn"
+version = "2.0.66"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "unicode-ident"
+version = "1.0.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b"
+
+[[package]]
+name = "version_check"
+version = "0.9.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
diff --git a/rust/pl011/Cargo.toml b/rust/pl011/Cargo.toml
new file mode 100644
index 0000000000..51f672d883
--- /dev/null
+++ b/rust/pl011/Cargo.toml
@@ -0,0 +1,67 @@
+[package]
+name = "pl011"
+version = "0.1.0"
+edition = "2021"
+authors = ["Manos Pitsidianakis <manos.pitsidianakis@linaro.org>"]
+license = "GPL-2.0 OR GPL-3.0-or-later"
+readme = "README.md"
+homepage = "https://www.qemu.org"
+description = "pl011 device model for QEMU"
+repository = "https://gitlab.com/epilys/rust-for-qemu"
+resolver = "2"
+publish = false
+keywords = []
+categories = []
+
+[lib]
+crate-type = ["staticlib"]
+
+# bilge deps included here to include them with docs
+[dependencies]
+arbitrary-int = { version = "1.2.7" }
+bilge = { version = "0.2.0" }
+bilge-impl = { version = "0.2.0" }
+qemu_api = { path = "../qemu-api" }
+
+[lints]
+[lints.rustdoc]
+broken_intra_doc_links = "deny"
+redundant_explicit_links = "deny"
+[lints.clippy]
+# lint groups
+correctness = { level = "deny", priority = -1 }
+suspicious = { level = "deny", priority = -1 }
+complexity = { level = "deny", priority = -1 }
+perf = { level = "deny", priority = -1 }
+cargo = { level = "deny", priority = -1 }
+nursery = { level = "deny", priority = -1 }
+style = { level = "deny", priority = -1 }
+# restriction group
+dbg_macro = "deny"
+rc_buffer = "deny"
+as_underscore = "deny"
+assertions_on_result_states = "deny"
+# pedantic group
+doc_markdown = "deny"
+expect_fun_call = "deny"
+borrow_as_ptr = "deny"
+case_sensitive_file_extension_comparisons = "deny"
+cast_lossless = "deny"
+cast_ptr_alignment = "allow"
+large_futures = "deny"
+waker_clone_wake = "deny"
+unused_enumerate_index = "deny"
+unnecessary_fallible_conversions = "deny"
+struct_field_names = "deny"
+manual_hash_one = "deny"
+into_iter_without_iter = "deny"
+option_if_let_else = "deny"
+missing_const_for_fn = "deny"
+significant_drop_tightening = "deny"
+multiple_crate_versions = "deny"
+significant_drop_in_scrutinee = "deny"
+cognitive_complexity = "deny"
+missing_safety_doc = "allow"
+
+# Do not include in any global workspace
+[workspace]
diff --git a/rust/pl011/README.md b/rust/pl011/README.md
new file mode 100644
index 0000000000..cd7dea3163
--- /dev/null
+++ b/rust/pl011/README.md
@@ -0,0 +1,31 @@
+# PL011 QEMU Device Model
+
+This library implements a device model for the PrimeCell® UART (PL011)
+device in QEMU.
+
+## Build static lib
+
+Host build target must be explicitly specified:
+
+```sh
+cargo build --target x86_64-unknown-linux-gnu
+```
+
+Replace host target triplet if necessary.
+
+## Generate Rust documentation
+
+To generate docs for this crate, including private items:
+
+```sh
+cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+```
+
+To include direct dependencies like `bilge` (bitmaps for register types):
+
+```sh
+cargo tree --depth 1 -e normal --prefix none \
+ | cut -d' ' -f1 \
+ | xargs printf -- '-p %s\n' \
+ | xargs cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+```
diff --git a/rust/pl011/deny.toml b/rust/pl011/deny.toml
new file mode 100644
index 0000000000..3992380509
--- /dev/null
+++ b/rust/pl011/deny.toml
@@ -0,0 +1,57 @@
+# cargo-deny configuration file
+
+[graph]
+targets = [
+ "aarch64-unknown-linux-gnu",
+ "x86_64-unknown-linux-gnu",
+ "x86_64-apple-darwin",
+ "aarch64-apple-darwin",
+ "x86_64-pc-windows-gnu",
+ "aarch64-pc-windows-gnullvm",
+]
+#exclude = []
+all-features = false
+no-default-features = false
+#features = []
+
+[output]
+feature-depth = 1
+
+[advisories]
+db-path = "$CARGO_HOME/advisory-dbs"
+db-urls = ["https://github.com/rustsec/advisory-db"]
+ignore = []
+
+[licenses]
+allow = [
+ "GPL-2.0",
+ "MIT",
+ "Apache-2.0",
+ "Unicode-DFS-2016",
+]
+confidence-threshold = 0.8
+exceptions = []
+
+[licenses.private]
+ignore = false
+registries = []
+
+[bans]
+multiple-versions = "warn"
+wildcards = "deny"
+# The graph highlighting used when creating dotgraphs for crates
+# with multiple versions
+# * lowest-version - The path to the lowest versioned duplicate is highlighted
+# * simplest-path - The path to the version with the fewest edges is highlighted
+# * all - Both lowest-version and simplest-path are used
+highlight = "all"
+workspace-default-features = "allow"
+external-default-features = "allow"
+allow = []
+deny = []
+
+[sources]
+unknown-registry = "deny"
+unknown-git = "deny"
+allow-registry = ["https://github.com/rust-lang/crates.io-index"]
+allow-git = []
diff --git a/rust/pl011/meson.build b/rust/pl011/meson.build
new file mode 100644
index 0000000000..cbac0fd94d
--- /dev/null
+++ b/rust/pl011/meson.build
@@ -0,0 +1,7 @@
+rust_pl011 = {
+ 'name': 'pl011',
+ 'dirname': 'pl011',
+ 'output': 'libpl011.a',
+ }
+
+rust_hw_target_list += {'aarch64-softmmu': [rust_pl011]}
diff --git a/rust/pl011/rustfmt.toml b/rust/pl011/rustfmt.toml
new file mode 120000
index 0000000000..39f97b043b
--- /dev/null
+++ b/rust/pl011/rustfmt.toml
@@ -0,0 +1 @@
+../rustfmt.toml
\ No newline at end of file
diff --git a/rust/pl011/src/definitions.rs b/rust/pl011/src/definitions.rs
new file mode 100644
index 0000000000..5efe8ae7f0
--- /dev/null
+++ b/rust/pl011/src/definitions.rs
@@ -0,0 +1,39 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+//! Definitions required by QEMU when registering the device.
+
+use core::{mem::MaybeUninit, ptr::NonNull};
+
+use qemu_api::bindings::*;
+
+use crate::{device::PL011State, device_class::pl011_class_init};
+
+qemu_api::type_info! {
+ PL011_ARM_INFO: PL011State,
+ name: c"x-pl011-rust",
+ parent: TYPE_SYS_BUS_DEVICE,
+ instance_init: Some(pl011_init),
+ abstract_: false,
+ class_init: Some(pl011_class_init),
+}
+
+#[used]
+pub static VMSTATE_PL011: VMStateDescription = VMStateDescription {
+ name: PL011_ARM_INFO.name,
+ unmigratable: true,
+ ..unsafe { MaybeUninit::<VMStateDescription>::zeroed().assume_init() }
+};
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
+ assert!(!obj.is_null());
+ let mut state = NonNull::new_unchecked(obj.cast::<PL011State>());
+ state.as_mut().init();
+}
+
+qemu_api::module_init! {
+ qom: register_type => {
+ type_register_static(&PL011_ARM_INFO);
+ }
+}
diff --git a/rust/pl011/src/device.rs b/rust/pl011/src/device.rs
new file mode 100644
index 0000000000..4aedd9582d
--- /dev/null
+++ b/rust/pl011/src/device.rs
@@ -0,0 +1,509 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use core::{
+ ffi::{c_int, c_uchar, c_uint, c_void, CStr},
+ ptr::{addr_of, addr_of_mut, NonNull},
+};
+
+use qemu_api::bindings::{self, *};
+
+use crate::{
+ definitions::PL011_ARM_INFO,
+ memory_ops::PL011_OPS,
+ registers::{self, Interrupt},
+ RegisterOffset,
+};
+
+static PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1];
+
+const DATA_BREAK: u32 = 1 << 10;
+
+/// QEMU sourced constant.
+pub const PL011_FIFO_DEPTH: usize = 16_usize;
+
+#[repr(C)]
+#[derive(Debug)]
+/// PL011 Device Model in QEMU
+pub struct PL011State {
+ pub parent_obj: SysBusDevice,
+ pub iomem: MemoryRegion,
+ pub readbuff: u32,
+ #[doc(alias = "fr")]
+ pub flags: registers::Flags,
+ #[doc(alias = "lcr")]
+ pub line_control: registers::LineControl,
+ #[doc(alias = "rsr")]
+ pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
+ #[doc(alias = "cr")]
+ pub control: registers::Control,
+ pub dmacr: u32,
+ pub int_enabled: u32,
+ pub int_level: u32,
+ pub read_fifo: [u32; PL011_FIFO_DEPTH],
+ pub ilpr: u32,
+ pub ibrd: u32,
+ pub fbrd: u32,
+ pub ifl: u32,
+ pub read_pos: usize,
+ pub read_count: usize,
+ pub read_trigger: usize,
+ #[doc(alias = "chr")]
+ pub char_backend: CharBackend,
+ /// QEMU interrupts
+ ///
+ /// ```text
+ /// * sysbus MMIO region 0: device registers
+ /// * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
+ /// * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
+ /// * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
+ /// * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
+ /// * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
+ /// * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
+ /// ```
+ #[doc(alias = "irq")]
+ pub interrupts: [qemu_irq; 6usize],
+ #[doc(alias = "clk")]
+ pub clock: NonNull<Clock>,
+ #[doc(alias = "migrate_clk")]
+ pub migrate_clock: bool,
+}
+
+#[used]
+pub static CLK_NAME: &CStr = c"clk";
+
+impl PL011State {
+ pub fn init(&mut self) {
+ unsafe {
+ memory_region_init_io(
+ addr_of_mut!(self.iomem),
+ addr_of_mut!(*self).cast::<Object>(),
+ &PL011_OPS,
+ addr_of_mut!(*self).cast::<c_void>(),
+ PL011_ARM_INFO.name,
+ 0x1000,
+ );
+ let sbd = addr_of_mut!(*self).cast::<SysBusDevice>();
+ let dev = addr_of_mut!(*self).cast::<DeviceState>();
+ sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
+ for irq in self.interrupts.iter_mut() {
+ sysbus_init_irq(sbd, irq);
+ }
+ self.clock = NonNull::new(qdev_init_clock_in(
+ dev,
+ CLK_NAME.as_ptr(),
+ None, /* pl011_clock_update */
+ addr_of_mut!(*self).cast::<c_void>(),
+ ClockEvent_ClockUpdate,
+ ))
+ .unwrap();
+ }
+ }
+
+ pub fn read(&mut self, offset: hwaddr, _size: core::ffi::c_uint) -> u64 {
+ use RegisterOffset::*;
+
+ match RegisterOffset::try_from(offset) {
+ Err(v) if (0x3f8..0x400).contains(&v) => {
+ u64::from(PL011_ID_ARM[((offset - 0xfe0) >> 2) as usize])
+ }
+ Err(_) => {
+ // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
+ 0
+ }
+ Ok(DR) => {
+ // s->flags &= ~PL011_FLAG_RXFF;
+ self.flags.set_receive_fifo_full(false);
+ let c = self.read_fifo[self.read_pos];
+ if self.read_count > 0 {
+ self.read_count -= 1;
+ self.read_pos = (self.read_pos + 1) & (self.fifo_depth() - 1);
+ }
+ if self.read_count == 0 {
+ // self.flags |= PL011_FLAG_RXFE;
+ self.flags.set_receive_fifo_empty(true);
+ }
+ if self.read_count + 1 == self.read_trigger {
+ //self.int_level &= ~ INT_RX;
+ self.int_level &= !registers::INT_RX;
+ }
+ // Update error bits.
+ self.receive_status_error_clear = c.to_be_bytes()[3].into();
+ self.update();
+ unsafe { qemu_chr_fe_accept_input(&mut self.char_backend) };
+ c.into()
+ }
+ Ok(RSR) => u8::from(self.receive_status_error_clear).into(),
+ Ok(FR) => u16::from(self.flags).into(),
+ Ok(FBRD) => self.fbrd.into(),
+ Ok(ILPR) => self.ilpr.into(),
+ Ok(IBRD) => self.ibrd.into(),
+ Ok(LCR_H) => u16::from(self.line_control).into(),
+ Ok(CR) => {
+ // We exercise our self-control.
+ u16::from(self.control).into()
+ }
+ Ok(FLS) => self.ifl.into(),
+ Ok(IMSC) => self.int_enabled.into(),
+ Ok(RIS) => self.int_level.into(),
+ Ok(MIS) => u64::from(self.int_level & self.int_enabled),
+ Ok(ICR) => {
+ // "The UARTICR Register is the interrupt clear register and is write-only"
+ // Source: ARM DDI 0183G 3.3.13 Interrupt Clear Register, UARTICR
+ 0
+ }
+ Ok(DMACR) => self.dmacr.into(),
+ }
+ }
+
+ pub fn write(&mut self, offset: hwaddr, value: u64) {
+ // eprintln!("write offset {offset} value {value}");
+ use RegisterOffset::*;
+ let value: u32 = value as u32;
+ match RegisterOffset::try_from(offset) {
+ Err(_bad_offset) => {
+ eprintln!("write bad offset {offset} value {value}");
+ }
+ Ok(DR) => {
+ // ??? Check if transmitter is enabled.
+ let ch: u8 = value as u8;
+ // XXX this blocks entire thread. Rewrite to use
+ // qemu_chr_fe_write and background I/O callbacks
+ unsafe {
+ qemu_chr_fe_write_all(addr_of_mut!(self.char_backend), &ch, 1);
+ }
+ self.loopback_tx(value);
+ self.int_level |= registers::INT_TX;
+ self.update();
+ }
+ Ok(RSR) => {
+ self.receive_status_error_clear = 0.into();
+ }
+ Ok(FR) => {
+ // flag writes are ignored
+ }
+ Ok(ILPR) => {
+ self.ilpr = value;
+ }
+ Ok(IBRD) => {
+ self.ibrd = value;
+ }
+ Ok(FBRD) => {
+ self.fbrd = value;
+ }
+ Ok(LCR_H) => {
+ let value = value as u16;
+ let new_val: registers::LineControl = value.into();
+ // Reset the FIFO state on FIFO enable or disable
+ if bool::from(self.line_control.fifos_enabled())
+ ^ bool::from(new_val.fifos_enabled())
+ {
+ self.reset_fifo();
+ }
+ if self.line_control.send_break() ^ new_val.send_break() {
+ let mut break_enable: c_int = new_val.send_break().into();
+ unsafe {
+ qemu_chr_fe_ioctl(
+ addr_of_mut!(self.char_backend),
+ CHR_IOCTL_SERIAL_SET_BREAK as i32,
+ addr_of_mut!(break_enable).cast::<c_void>(),
+ );
+ }
+ self.loopback_break(break_enable > 0);
+ }
+ self.line_control = new_val;
+ self.set_read_trigger();
+ }
+ Ok(CR) => {
+ // ??? Need to implement the enable bit.
+ let value = value as u16;
+ self.control = value.into();
+ self.loopback_mdmctrl();
+ }
+ Ok(FLS) => {
+ self.ifl = value;
+ self.set_read_trigger();
+ }
+ Ok(IMSC) => {
+ self.int_enabled = value;
+ self.update();
+ }
+ Ok(RIS) => {}
+ Ok(MIS) => {}
+ Ok(ICR) => {
+ self.int_level &= !value;
+ self.update();
+ }
+ Ok(DMACR) => {
+ self.dmacr = value;
+ if value & 3 > 0 {
+ // qemu_log_mask(LOG_UNIMP, "pl011: DMA not implemented\n");
+ eprintln!("pl011: DMA not implemented");
+ }
+ }
+ }
+ }
+
+ #[inline]
+ fn loopback_tx(&mut self, value: u32) {
+ if !self.loopback_enabled() {
+ return;
+ }
+
+ // Caveat:
+ //
+ // In real hardware, TX loopback happens at the serial-bit level
+ // and then reassembled by the RX logics back into bytes and placed
+ // into the RX fifo. That is, loopback happens after TX fifo.
+ //
+ // Because the real hardware TX fifo is time-drained at the frame
+ // rate governed by the configured serial format, some loopback
+ // bytes in TX fifo may still be able to get into the RX fifo
+ // that could be full at times while being drained at software
+ // pace.
+ //
+ // In such scenario, the RX draining pace is the major factor
+ // deciding which loopback bytes get into the RX fifo, unless
+ // hardware flow-control is enabled.
+ //
+ // For simplicity, the above described is not emulated.
+ self.put_fifo(value);
+ }
+
+ fn loopback_mdmctrl(&mut self) {
+ if !self.loopback_enabled() {
+ return;
+ }
+
+ /*
+ * Loopback software-driven modem control outputs to modem status inputs:
+ * FR.RI <= CR.Out2
+ * FR.DCD <= CR.Out1
+ * FR.CTS <= CR.RTS
+ * FR.DSR <= CR.DTR
+ *
+ * The loopback happens immediately even if this call is triggered
+ * by setting only CR.LBE.
+ *
+ * CTS/RTS updates due to enabled hardware flow controls are not
+ * dealt with here.
+ */
+
+ //fr = s->flags & ~(PL011_FLAG_RI | PL011_FLAG_DCD |
+ // PL011_FLAG_DSR | PL011_FLAG_CTS);
+ //fr |= (cr & CR_OUT2) ? PL011_FLAG_RI : 0;
+ //fr |= (cr & CR_OUT1) ? PL011_FLAG_DCD : 0;
+ //fr |= (cr & CR_RTS) ? PL011_FLAG_CTS : 0;
+ //fr |= (cr & CR_DTR) ? PL011_FLAG_DSR : 0;
+ //
+ self.flags.set_ring_indicator(self.control.out_2());
+ self.flags.set_data_carrier_detect(self.control.out_1());
+ self.flags.set_clear_to_send(self.control.request_to_send());
+ self.flags
+ .set_data_set_ready(self.control.data_transmit_ready());
+
+ // Change interrupts based on updated FR
+ let mut il = self.int_level;
+
+ il &= !Interrupt::MS;
+ //il |= (fr & PL011_FLAG_DSR) ? INT_DSR : 0;
+ //il |= (fr & PL011_FLAG_DCD) ? INT_DCD : 0;
+ //il |= (fr & PL011_FLAG_CTS) ? INT_CTS : 0;
+ //il |= (fr & PL011_FLAG_RI) ? INT_RI : 0;
+
+ if self.flags.data_set_ready() {
+ il |= Interrupt::DSR as u32;
+ }
+ if self.flags.data_carrier_detect() {
+ il |= Interrupt::DCD as u32;
+ }
+ if self.flags.clear_to_send() {
+ il |= Interrupt::CTS as u32;
+ }
+ if self.flags.ring_indicator() {
+ il |= Interrupt::RI as u32;
+ }
+ self.int_level = il;
+ self.update();
+ }
+
+ fn loopback_break(&mut self, enable: bool) {
+ if enable {
+ self.loopback_tx(DATA_BREAK);
+ }
+ }
+
+ fn set_read_trigger(&mut self) {
+ //#if 0
+ // /* The docs say the RX interrupt is triggered when the FIFO exceeds
+ // the threshold. However linux only reads the FIFO in response to an
+ // interrupt. Triggering the interrupt when the FIFO is non-empty seems
+ // to make things work. */
+ // if (s->lcr & LCR_FEN)
+ // s->read_trigger = (s->ifl >> 1) & 0x1c;
+ // else
+ //#endif
+ self.read_trigger = 1;
+ }
+
+ pub fn realize(&mut self) {
+ unsafe {
+ qemu_chr_fe_set_handlers(
+ addr_of_mut!(self.char_backend),
+ Some(pl011_can_receive),
+ Some(pl011_receive),
+ Some(pl011_event),
+ None,
+ addr_of_mut!(*self).cast::<c_void>(),
+ core::ptr::null_mut(),
+ true,
+ );
+ }
+ }
+
+ pub fn reset(&mut self) {
+ self.line_control.reset();
+ self.receive_status_error_clear.reset();
+ self.dmacr = 0;
+ self.int_enabled = 0;
+ self.int_level = 0;
+ self.ilpr = 0;
+ self.ibrd = 0;
+ self.fbrd = 0;
+ self.read_trigger = 1;
+ self.ifl = 0x12;
+ self.control.reset();
+ self.flags = 0.into();
+ self.reset_fifo();
+ }
+
+ pub fn reset_fifo(&mut self) {
+ self.read_count = 0;
+ self.read_pos = 0;
+
+ /* Reset FIFO flags */
+ self.flags.reset();
+ }
+
+ pub fn can_receive(&self) -> bool {
+ // trace_pl011_can_receive(s->lcr, s->read_count, r);
+ self.read_count < self.fifo_depth()
+ }
+
+ pub fn event(&mut self, event: QEMUChrEvent) {
+ if event == bindings::QEMUChrEvent_CHR_EVENT_BREAK && !self.fifo_enabled() {
+ self.put_fifo(DATA_BREAK);
+ self.receive_status_error_clear.set_break_error(true);
+ }
+ }
+
+ #[inline]
+ pub fn fifo_enabled(&self) -> bool {
+ matches!(self.line_control.fifos_enabled(), registers::Mode::FIFO)
+ }
+
+ #[inline]
+ pub fn loopback_enabled(&self) -> bool {
+ self.control.enable_loopback()
+ }
+
+ #[inline]
+ pub fn fifo_depth(&self) -> usize {
+ // Note: FIFO depth is expected to be power-of-2
+ if self.fifo_enabled() {
+ return PL011_FIFO_DEPTH;
+ }
+ 1
+ }
+
+ pub fn put_fifo(&mut self, value: c_uint) {
+ let depth = self.fifo_depth();
+ assert!(depth > 0);
+ let slot = (self.read_pos + self.read_count) & (depth - 1);
+ self.read_fifo[slot] = value;
+ self.read_count += 1;
+ // s->flags &= ~PL011_FLAG_RXFE;
+ self.flags.set_receive_fifo_empty(false);
+ if self.read_count == depth {
+ //s->flags |= PL011_FLAG_RXFF;
+ self.flags.set_receive_fifo_full(true);
+ }
+
+ if self.read_count == self.read_trigger {
+ self.int_level |= registers::INT_RX;
+ self.update();
+ }
+ }
+
+ pub fn update(&mut self) {
+ let flags = self.int_level & self.int_enabled;
+ for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
+ unsafe { qemu_set_irq(*irq, i32::from(flags & i != 0)) };
+ }
+ }
+}
+
+/// Which bits in the interrupt status matter for each outbound IRQ line ?
+pub const IRQMASK: [u32; 6] = [
+ /* combined IRQ */
+ Interrupt::E
+ | Interrupt::MS
+ | Interrupt::RT as u32
+ | Interrupt::TX as u32
+ | Interrupt::RX as u32,
+ Interrupt::RX as u32,
+ Interrupt::TX as u32,
+ Interrupt::RT as u32,
+ Interrupt::MS,
+ Interrupt::E,
+];
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_can_receive(opaque: *mut c_void) -> c_int {
+ assert!(!opaque.is_null());
+ let state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+ state.as_ref().can_receive().into()
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_receive(
+ opaque: *mut core::ffi::c_void,
+ buf: *const u8,
+ size: core::ffi::c_int,
+) {
+ assert!(!opaque.is_null());
+ let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+ if state.as_ref().loopback_enabled() {
+ return;
+ }
+ if size > 0 {
+ assert!(!buf.is_null());
+ state.as_mut().put_fifo(*buf.cast::<c_uint>())
+ }
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) {
+ assert!(!opaque.is_null());
+ let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+ state.as_mut().event(event)
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn rust_pl011_create(
+ addr: u64,
+ irq: qemu_irq,
+ chr: *mut Chardev,
+) -> *mut DeviceState {
+ let dev: *mut DeviceState = unsafe { qdev_new(PL011_ARM_INFO.name) };
+ assert!(!dev.is_null());
+ let sysbus: *mut SysBusDevice = dev as *mut SysBusDevice;
+
+ unsafe {
+ qdev_prop_set_chr(dev, bindings::TYPE_CHARDEV.as_ptr(), chr);
+ sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error);
+ sysbus_mmio_map(sysbus, 0, addr);
+ sysbus_connect_irq(sysbus, 0, irq);
+ }
+ dev
+}
diff --git a/rust/pl011/src/device_class.rs b/rust/pl011/src/device_class.rs
new file mode 100644
index 0000000000..a886731107
--- /dev/null
+++ b/rust/pl011/src/device_class.rs
@@ -0,0 +1,48 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use core::ptr::NonNull;
+
+use qemu_api::bindings::*;
+
+use crate::{definitions::VMSTATE_PL011, device::PL011State};
+
+qemu_api::declare_properties! {
+ PL011_PROPERTIES,
+ qemu_api::define_property!(
+ c"chardev",
+ PL011State,
+ char_backend,
+ unsafe { &qdev_prop_chr },
+ CharBackend
+ ),
+ qemu_api::define_property!(
+ c"migrate-clk",
+ PL011State,
+ migrate_clock,
+ unsafe { &qdev_prop_bool },
+ bool
+ ),
+}
+
+qemu_api::device_class_init! {
+ pl011_class_init,
+ props => PL011_PROPERTIES,
+ realize_fn => Some(pl011_realize),
+ reset_fn => Some(pl011_reset),
+ vmsd => VMSTATE_PL011,
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) {
+ assert!(!dev.is_null());
+ let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
+ state.as_mut().realize();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) {
+ assert!(!dev.is_null());
+ let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
+ state.as_mut().reset();
+}
diff --git a/rust/pl011/src/lib.rs b/rust/pl011/src/lib.rs
new file mode 100644
index 0000000000..ec7ed04fc0
--- /dev/null
+++ b/rust/pl011/src/lib.rs
@@ -0,0 +1,556 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+//
+// PL011 QEMU Device Model
+//
+// This library implements a device model for the PrimeCell® UART (PL011)
+// device in QEMU.
+//
+#![doc = include_str!("../README.md")]
+//! # Library crate
+//!
+//! See [`PL011State`](crate::device::PL011State) for the device model type and
+//! the [`registers`] module for register types.
+
+pub mod definitions;
+pub mod device;
+pub mod device_class;
+pub mod memory_ops;
+
+/// Offset of each register from the base memory address of the device.
+///
+/// # Source
+/// ARM DDI 0183G, Table 3-1 p.3-3
+#[doc(alias = "offset")]
+#[allow(non_camel_case_types)]
+#[repr(u64)]
+#[derive(Debug)]
+pub enum RegisterOffset {
+ /// Data Register
+ ///
+ /// A write to this register initiates the actual data transmission
+ #[doc(alias = "UARTDR")]
+ DR = 0x000,
+ /// Receive Status Register or Error Clear Register
+ #[doc(alias = "UARTRSR")]
+ #[doc(alias = "UARTECR")]
+ RSR = 0x004,
+ /// Flag Register
+ ///
+ /// A read of this register shows if transmission is complete
+ #[doc(alias = "UARTFR")]
+ FR = 0x018,
+ /// Fractional Baud Rate Register
+ ///
+ /// responsible for baud rate speed
+ #[doc(alias = "UARTFBRD")]
+ FBRD = 0x028,
+ /// `IrDA` Low-Power Counter Register
+ #[doc(alias = "UARTILPR")]
+ ILPR = 0x020,
+ /// Integer Baud Rate Register
+ ///
+ /// Responsible for baud rate speed
+ #[doc(alias = "UARTIBRD")]
+ IBRD = 0x024,
+ /// line control register (data frame format)
+ #[doc(alias = "UARTLCR_H")]
+ LCR_H = 0x02C,
+ /// Toggle UART, transmission or reception
+ #[doc(alias = "UARTCR")]
+ CR = 0x030,
+ /// Interrupt FIFO Level Select Register
+ #[doc(alias = "UARTIFLS")]
+ FLS = 0x034,
+ /// Interrupt Mask Set/Clear Register
+ #[doc(alias = "UARTIMSC")]
+ IMSC = 0x038,
+ /// Raw Interrupt Status Register
+ #[doc(alias = "UARTRIS")]
+ RIS = 0x03C,
+ /// Masked Interrupt Status Register
+ #[doc(alias = "UARTMIS")]
+ MIS = 0x040,
+ /// Interrupt Clear Register
+ #[doc(alias = "UARTICR")]
+ ICR = 0x044,
+ /// DMA control Register
+ #[doc(alias = "UARTDMACR")]
+ DMACR = 0x048,
+ ///// Reserved, offsets `0x04C` to `0x07C`.
+ //Reserved = 0x04C,
+}
+
+impl core::convert::TryFrom<u64> for RegisterOffset {
+ type Error = u64;
+
+ fn try_from(value: u64) -> Result<Self, Self::Error> {
+ macro_rules! case {
+ ($($discriminant:ident),*$(,)*) => {
+ /* check that matching on all macro arguments compiles, which means we are not
+ * missing any enum value; if the type definition ever changes this will stop
+ * compiling.
+ */
+ const fn _assert_exhaustive(val: RegisterOffset) {
+ match val {
+ $(RegisterOffset::$discriminant => (),)*
+ }
+ }
+
+ match value {
+ $(x if x == Self::$discriminant as u64 => Ok(Self::$discriminant),)*
+ _ => Err(value),
+ }
+ }
+ }
+ case! { DR, RSR, FR, FBRD, ILPR, IBRD, LCR_H, CR, FLS, IMSC, RIS, MIS, ICR, DMACR }
+ }
+}
+
+pub mod registers {
+ //! Device registers exposed as typed structs which are backed by arbitrary
+ //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
+ //!
+ //! All PL011 registers are essentially 32-bit wide, but are typed here as
+ //! bitmaps with only the necessary width. That is, if a struct bitmap
+ //! in this module is for example 16 bits long, it should be conceived
+ //! as a 32-bit register where the unmentioned higher bits are always
+ //! unused thus treated as zero when read or written.
+ use bilge::prelude::*;
+
+ // TODO: FIFO Mode has different semantics
+ /// Data Register, `UARTDR`
+ ///
+ /// The `UARTDR` register is the data register.
+ ///
+ /// For words to be transmitted:
+ ///
+ /// - if the FIFOs are enabled, data written to this location is pushed onto
+ /// the transmit
+ /// FIFO
+ /// - if the FIFOs are not enabled, data is stored in the transmitter
+ /// holding register (the
+ /// bottom word of the transmit FIFO).
+ ///
+ /// The write operation initiates transmission from the UART. The data is
+ /// prefixed with a start bit, appended with the appropriate parity bit
+ /// (if parity is enabled), and a stop bit. The resultant word is then
+ /// transmitted.
+ ///
+ /// For received words:
+ ///
+ /// - if the FIFOs are enabled, the data byte and the 4-bit status (break,
+ /// frame, parity,
+ /// and overrun) is pushed onto the 12-bit wide receive FIFO
+ /// - if the FIFOs are not enabled, the data byte and status are stored in
+ /// the receiving
+ /// holding register (the bottom word of the receive FIFO).
+ ///
+ /// The received data byte is read by performing reads from the `UARTDR`
+ /// register along with the corresponding status information. The status
+ /// information can also be read by a read of the `UARTRSR/UARTECR`
+ /// register.
+ ///
+ /// # Note
+ ///
+ /// You must disable the UART before any of the control registers are
+ /// reprogrammed. When the UART is disabled in the middle of
+ /// transmission or reception, it completes the current character before
+ /// stopping.
+ ///
+ /// # Source
+ /// ARM DDI 0183G 3.3.1 Data Register, UARTDR
+ #[bitsize(16)]
+ #[derive(Clone, Copy, DebugBits, FromBits)]
+ #[doc(alias = "UARTDR")]
+ pub struct Data {
+ _reserved: u4,
+ pub data: u8,
+ pub framing_error: bool,
+ pub parity_error: bool,
+ pub break_error: bool,
+ pub overrun_error: bool,
+ }
+
+ // TODO: FIFO Mode has different semantics
+ /// Receive Status Register / Error Clear Register, `UARTRSR/UARTECR`
+ ///
+ /// The UARTRSR/UARTECR register is the receive status register/error clear
+ /// register. Receive status can also be read from the `UARTRSR`
+ /// register. If the status is read from this register, then the status
+ /// information for break, framing and parity corresponds to the
+ /// data character read from the [Data register](Data), `UARTDR` prior to
+ /// reading the UARTRSR register. The status information for overrun is
+ /// set immediately when an overrun condition occurs.
+ ///
+ ///
+ /// # Note
+ /// The received data character must be read first from the [Data
+ /// Register](Data), `UARTDR` before reading the error status associated
+ /// with that data character from the `UARTRSR` register. This read
+ /// sequence cannot be reversed, because the `UARTRSR` register is
+ /// updated only when a read occurs from the `UARTDR` register. However,
+ /// the status information can also be obtained by reading the `UARTDR`
+ /// register
+ ///
+ /// # Source
+ /// ARM DDI 0183G 3.3.2 Receive Status Register/Error Clear Register,
+ /// UARTRSR/UARTECR
+ #[bitsize(8)]
+ #[derive(Clone, Copy, DebugBits, FromBits)]
+ pub struct ReceiveStatusErrorClear {
+ pub framing_error: bool,
+ pub parity_error: bool,
+ pub break_error: bool,
+ pub overrun_error: bool,
+ _reserved_unpredictable: u4,
+ }
+
+ impl ReceiveStatusErrorClear {
+ pub fn reset(&mut self) {
+ // All the bits are cleared to 0 on reset.
+ *self = 0.into();
+ }
+ }
+
+ impl Default for ReceiveStatusErrorClear {
+ fn default() -> Self {
+ 0.into()
+ }
+ }
+
+ #[bitsize(16)]
+ #[derive(Clone, Copy, DebugBits, FromBits)]
+ /// Flag Register, `UARTFR`
+ #[doc(alias = "UARTFR")]
+ pub struct Flags {
+ /// CTS Clear to send. This bit is the complement of the UART clear to
+ /// send, `nUARTCTS`, modem status input. That is, the bit is 1
+ /// when `nUARTCTS` is LOW.
+ pub clear_to_send: bool,
+ /// DSR Data set ready. This bit is the complement of the UART data set
+ /// ready, `nUARTDSR`, modem status input. That is, the bit is 1 when
+ /// `nUARTDSR` is LOW.
+ pub data_set_ready: bool,
+ /// DCD Data carrier detect. This bit is the complement of the UART data
+ /// carrier detect, `nUARTDCD`, modem status input. That is, the bit is
+ /// 1 when `nUARTDCD` is LOW.
+ pub data_carrier_detect: bool,
+ /// BUSY UART busy. If this bit is set to 1, the UART is busy
+ /// transmitting data. This bit remains set until the complete
+ /// byte, including all the stop bits, has been sent from the
+ /// shift register. This bit is set as soon as the transmit FIFO
+ /// becomes non-empty, regardless of whether the UART is enabled
+ /// or not.
+ pub busy: bool,
+ /// RXFE Receive FIFO empty. The meaning of this bit depends on the
+ /// state of the FEN bit in the UARTLCR_H register. If the FIFO
+ /// is disabled, this bit is set when the receive holding
+ /// register is empty. If the FIFO is enabled, the RXFE bit is
+ /// set when the receive FIFO is empty.
+ pub receive_fifo_empty: bool,
+ /// TXFF Transmit FIFO full. The meaning of this bit depends on the
+ /// state of the FEN bit in the UARTLCR_H register. If the FIFO
+ /// is disabled, this bit is set when the transmit holding
+ /// register is full. If the FIFO is enabled, the TXFF bit is
+ /// set when the transmit FIFO is full.
+ pub transmit_fifo_full: bool,
+ /// RXFF Receive FIFO full. The meaning of this bit depends on the state
+ /// of the FEN bit in the UARTLCR_H register. If the FIFO is
+ /// disabled, this bit is set when the receive holding register
+ /// is full. If the FIFO is enabled, the RXFF bit is set when
+ /// the receive FIFO is full.
+ pub receive_fifo_full: bool,
+ /// Transmit FIFO empty. The meaning of this bit depends on the state of
+ /// the FEN bit in the [Line Control register](LineControl),
+ /// `UARTLCR_H`. If the FIFO is disabled, this bit is set when the
+ /// transmit holding register is empty. If the FIFO is enabled,
+ /// the TXFE bit is set when the transmit FIFO is empty. This
+ /// bit does not indicate if there is data in the transmit shift
+ /// register.
+ pub transmit_fifo_empty: bool,
+ /// `RI`, is `true` when `nUARTRI` is `LOW`.
+ pub ring_indicator: bool,
+ _reserved_zero_no_modify: u7,
+ }
+
+ impl Flags {
+ pub fn reset(&mut self) {
+ // After reset TXFF, RXFF, and BUSY are 0, and TXFE and RXFE are 1
+ self.set_receive_fifo_full(false);
+ self.set_transmit_fifo_full(false);
+ self.set_busy(false);
+ self.set_receive_fifo_empty(true);
+ self.set_transmit_fifo_empty(true);
+ }
+ }
+
+ impl Default for Flags {
+ fn default() -> Self {
+ let mut ret: Self = 0.into();
+ ret.reset();
+ ret
+ }
+ }
+
+ #[bitsize(16)]
+ #[derive(Clone, Copy, DebugBits, FromBits)]
+ /// Line Control Register, `UARTLCR_H`
+ #[doc(alias = "UARTLCR_H")]
+ pub struct LineControl {
+ /// 15:8 - Reserved, do not modify, read as zero.
+ _reserved_zero_no_modify: u8,
+ /// 7 SPS Stick parity select.
+ /// 0 = stick parity is disabled
+ /// 1 = either:
+ /// • if the EPS bit is 0 then the parity bit is transmitted and checked
+ /// as a 1 • if the EPS bit is 1 then the parity bit is
+ /// transmitted and checked as a 0. This bit has no effect when
+ /// the PEN bit disables parity checking and generation. See Table 3-11
+ /// on page 3-14 for the parity truth table.
+ pub sticky_parity: bool,
+ /// WLEN Word length. These bits indicate the number of data bits
+ /// transmitted or received in a frame as follows: b11 = 8 bits
+ /// b10 = 7 bits
+ /// b01 = 6 bits
+ /// b00 = 5 bits.
+ pub word_length: WordLength,
+ /// FEN Enable FIFOs:
+ /// 0 = FIFOs are disabled (character mode) that is, the FIFOs become
+ /// 1-byte-deep holding registers 1 = transmit and receive FIFO
+ /// buffers are enabled (FIFO mode).
+ pub fifos_enabled: Mode,
+ /// 3 STP2 Two stop bits select. If this bit is set to 1, two stop bits
+ /// are transmitted at the end of the frame. The receive
+ /// logic does not check for two stop bits being received.
+ pub two_stops_bits: bool,
+ /// EPS Even parity select. Controls the type of parity the UART uses
+ /// during transmission and reception:
+ /// - 0 = odd parity. The UART generates or checks for an odd number of
+ /// 1s in the data and parity bits.
+ /// - 1 = even parity. The UART generates or checks for an even number
+ /// of 1s in the data and parity bits.
+ /// This bit has no effect when the `PEN` bit disables parity checking
+ /// and generation. See Table 3-11 on page 3-14 for the parity
+ /// truth table.
+ pub parity: Parity,
+ /// 1 PEN Parity enable:
+ ///
+ /// - 0 = parity is disabled and no parity bit added to the data frame
+ /// - 1 = parity checking and generation is enabled.
+ ///
+ /// See Table 3-11 on page 3-14 for the parity truth table.
+ pub parity_enabled: bool,
+ /// BRK Send break.
+ ///
+ /// If this bit is set to `1`, a low-level is continually output on the
+ /// `UARTTXD` output, after completing transmission of the
+ /// current character. For the proper execution of the break command,
+ /// the software must set this bit for at least two complete
+ /// frames. For normal use, this bit must be cleared to `0`.
+ pub send_break: bool,
+ }
+
+ impl LineControl {
+ pub fn reset(&mut self) {
+ // All the bits are cleared to 0 when reset.
+ *self = 0.into();
+ }
+ }
+
+ impl Default for LineControl {
+ fn default() -> Self {
+ 0.into()
+ }
+ }
+
+ #[bitsize(1)]
+ #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+ /// `EPS` "Even parity select", field of [Line Control
+ /// register](LineControl).
+ pub enum Parity {
+ /// - 0 = odd parity. The UART generates or checks for an odd number of
+ /// 1s in the data and parity bits.
+ Odd = 0,
+ /// - 1 = even parity. The UART generates or checks for an even number
+ /// of 1s in the data and parity bits.
+ Even = 1,
+ }
+
+ #[bitsize(1)]
+ #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+ /// `FEN` "Enable FIFOs" or Device mode, field of [Line Control
+ /// register](LineControl).
+ pub enum Mode {
+ /// 0 = FIFOs are disabled (character mode) that is, the FIFOs become
+ /// 1-byte-deep holding registers
+ Character = 0,
+ /// 1 = transmit and receive FIFO buffers are enabled (FIFO mode).
+ FIFO = 1,
+ }
+
+ impl From<Mode> for bool {
+ fn from(val: Mode) -> Self {
+ matches!(val, Mode::FIFO)
+ }
+ }
+
+ #[bitsize(2)]
+ #[derive(Clone, Copy, Debug, Eq, FromBits, PartialEq)]
+ /// `WLEN` Word length, field of [Line Control register](LineControl).
+ ///
+ /// These bits indicate the number of data bits transmitted or received in a
+ /// frame as follows:
+ pub enum WordLength {
+ /// b11 = 8 bits
+ _8Bits = 0b11,
+ /// b10 = 7 bits
+ _7Bits = 0b10,
+ /// b01 = 6 bits
+ _6Bits = 0b01,
+ /// b00 = 5 bits.
+ _5Bits = 0b00,
+ }
+
+ /// Control Register, `UARTCR`
+ ///
+ /// The `UARTCR` register is the control register. All the bits are cleared
+ /// to `0` on reset except for bits `9` and `8` that are set to `1`.
+ ///
+ /// # Source
+ /// ARM DDI 0183G, 3.3.8 Control Register, `UARTCR`, Table 3-12
+ #[bitsize(16)]
+ #[doc(alias = "UARTCR")]
+ #[derive(Clone, Copy, DebugBits, FromBits)]
+ pub struct Control {
+ /// `UARTEN` UART enable: 0 = UART is disabled. If the UART is disabled
+ /// in the middle of transmission or reception, it completes the current
+ /// character before stopping. 1 = the UART is enabled. Data
+ /// transmission and reception occurs for either UART signals or SIR
+ /// signals depending on the setting of the SIREN bit.
+ pub enable_uart: bool,
+ /// `SIREN` `SIR` enable: 0 = IrDA SIR ENDEC is disabled. `nSIROUT`
+ /// remains LOW (no light pulse generated), and signal transitions on
+ /// SIRIN have no effect. 1 = IrDA SIR ENDEC is enabled. Data is
+ /// transmitted and received on nSIROUT and SIRIN. UARTTXD remains HIGH,
+ /// in the marking state. Signal transitions on UARTRXD or modem status
+ /// inputs have no effect. This bit has no effect if the UARTEN bit
+ /// disables the UART.
+ pub enable_sir: bool,
+ /// `SIRLP` SIR low-power IrDA mode. This bit selects the IrDA encoding
+ /// mode. If this bit is cleared to 0, low-level bits are transmitted as
+ /// an active high pulse with a width of 3/ 16th of the bit period. If
+ /// this bit is set to 1, low-level bits are transmitted with a pulse
+ /// width that is 3 times the period of the IrLPBaud16 input signal,
+ /// regardless of the selected bit rate. Setting this bit uses less
+ /// power, but might reduce transmission distances.
+ pub sir_lowpower_irda_mode: u1,
+ /// Reserved, do not modify, read as zero.
+ _reserved_zero_no_modify: u4,
+ /// `LBE` Loopback enable. If this bit is set to 1 and the SIREN bit is
+ /// set to 1 and the SIRTEST bit in the Test Control register, UARTTCR
+ /// on page 4-5 is set to 1, then the nSIROUT path is inverted, and fed
+ /// through to the SIRIN path. The SIRTEST bit in the test register must
+ /// be set to 1 to override the normal half-duplex SIR operation. This
+ /// must be the requirement for accessing the test registers during
+ /// normal operation, and SIRTEST must be cleared to 0 when loopback
+ /// testing is finished. This feature reduces the amount of external
+ /// coupling required during system test. If this bit is set to 1, and
+ /// the SIRTEST bit is set to 0, the UARTTXD path is fed through to the
+ /// UARTRXD path. In either SIR mode or UART mode, when this bit is set,
+ /// the modem outputs are also fed through to the modem inputs. This bit
+ /// is cleared to 0 on reset, to disable loopback.
+ pub enable_loopback: bool,
+ /// `TXE` Transmit enable. If this bit is set to 1, the transmit section
+ /// of the UART is enabled. Data transmission occurs for either UART
+ /// signals, or SIR signals depending on the setting of the SIREN bit.
+ /// When the UART is disabled in the middle of transmission, it
+ /// completes the current character before stopping.
+ pub enable_transmit: bool,
+ /// `RXE` Receive enable. If this bit is set to 1, the receive section
+ /// of the UART is enabled. Data reception occurs for either UART
+ /// signals or SIR signals depending on the setting of the SIREN bit.
+ /// When the UART is disabled in the middle of reception, it completes
+ /// the current character before stopping.
+ pub enable_receive: bool,
+ /// `DTR` Data transmit ready. This bit is the complement of the UART
+ /// data transmit ready, `nUARTDTR`, modem status output. That is, when
+ /// the bit is programmed to a 1 then `nUARTDTR` is LOW.
+ pub data_transmit_ready: bool,
+ /// `RTS` Request to send. This bit is the complement of the UART
+ /// request to send, `nUARTRTS`, modem status output. That is, when the
+ /// bit is programmed to a 1 then `nUARTRTS` is LOW.
+ pub request_to_send: bool,
+ /// `Out1` This bit is the complement of the UART Out1 (`nUARTOut1`)
+ /// modem status output. That is, when the bit is programmed to a 1 the
+ /// output is 0. For DTE this can be used as Data Carrier Detect (DCD).
+ pub out_1: bool,
+ /// `Out2` This bit is the complement of the UART Out2 (`nUARTOut2`)
+ /// modem status output. That is, when the bit is programmed to a 1, the
+ /// output is 0. For DTE this can be used as Ring Indicator (RI).
+ pub out_2: bool,
+ /// `RTSEn` RTS hardware flow control enable. If this bit is set to 1,
+ /// RTS hardware flow control is enabled. Data is only requested when
+ /// there is space in the receive FIFO for it to be received.
+ pub rts_hardware_flow_control_enable: bool,
+ /// `CTSEn` CTS hardware flow control enable. If this bit is set to 1,
+ /// CTS hardware flow control is enabled. Data is only transmitted when
+ /// the `nUARTCTS` signal is asserted.
+ pub cts_hardware_flow_control_enable: bool,
+ }
+
+ impl Control {
+ pub fn reset(&mut self) {
+ *self = 0.into();
+ self.set_enable_receive(true);
+ self.set_enable_transmit(true);
+ }
+ }
+
+ impl Default for Control {
+ fn default() -> Self {
+ let mut ret: Self = 0.into();
+ ret.reset();
+ ret
+ }
+ }
+
+ /// Interrupt status bits in UARTRIS, UARTMIS, UARTIMSC
+ pub const INT_OE: u32 = 1 << 10;
+ pub const INT_BE: u32 = 1 << 9;
+ pub const INT_PE: u32 = 1 << 8;
+ pub const INT_FE: u32 = 1 << 7;
+ pub const INT_RT: u32 = 1 << 6;
+ pub const INT_TX: u32 = 1 << 5;
+ pub const INT_RX: u32 = 1 << 4;
+ pub const INT_DSR: u32 = 1 << 3;
+ pub const INT_DCD: u32 = 1 << 2;
+ pub const INT_CTS: u32 = 1 << 1;
+ pub const INT_RI: u32 = 1 << 0;
+ pub const INT_E: u32 = INT_OE | INT_BE | INT_PE | INT_FE;
+ pub const INT_MS: u32 = INT_RI | INT_DSR | INT_DCD | INT_CTS;
+
+ #[repr(u32)]
+ pub enum Interrupt {
+ OE = 1 << 10,
+ BE = 1 << 9,
+ PE = 1 << 8,
+ FE = 1 << 7,
+ RT = 1 << 6,
+ TX = 1 << 5,
+ RX = 1 << 4,
+ DSR = 1 << 3,
+ DCD = 1 << 2,
+ CTS = 1 << 1,
+ RI = 1 << 0,
+ }
+
+ impl Interrupt {
+ pub const E: u32 = INT_OE | INT_BE | INT_PE | INT_FE;
+ pub const MS: u32 = INT_RI | INT_DSR | INT_DCD | INT_CTS;
+ }
+}
+
+// TODO: You must disable the UART before any of the control registers are
+// reprogrammed. When the UART is disabled in the middle of transmission or
+// reception, it completes the current character before stopping
diff --git a/rust/pl011/src/memory_ops.rs b/rust/pl011/src/memory_ops.rs
new file mode 100644
index 0000000000..6144d28586
--- /dev/null
+++ b/rust/pl011/src/memory_ops.rs
@@ -0,0 +1,45 @@
+// Copyright 2024 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0 OR GPL-3.0-or-later
+
+use core::{mem::MaybeUninit, ptr::NonNull};
+
+use qemu_api::bindings::*;
+
+use crate::device::PL011State;
+
+pub static PL011_OPS: MemoryRegionOps = MemoryRegionOps {
+ read: Some(pl011_read),
+ write: Some(pl011_write),
+ read_with_attrs: None,
+ write_with_attrs: None,
+ endianness: device_endian_DEVICE_NATIVE_ENDIAN,
+ valid: unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_1>::zeroed().assume_init() },
+ impl_: MemoryRegionOps__bindgen_ty_2 {
+ min_access_size: 4,
+ max_access_size: 4,
+ ..unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_2>::zeroed().assume_init() }
+ },
+};
+
+#[no_mangle]
+unsafe extern "C" fn pl011_read(
+ opaque: *mut core::ffi::c_void,
+ addr: hwaddr,
+ size: core::ffi::c_uint,
+) -> u64 {
+ assert!(!opaque.is_null());
+ let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+ state.as_mut().read(addr, size)
+}
+
+#[no_mangle]
+unsafe extern "C" fn pl011_write(
+ opaque: *mut core::ffi::c_void,
+ addr: hwaddr,
+ data: u64,
+ _size: core::ffi::c_uint,
+) {
+ assert!(!opaque.is_null());
+ let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
+ state.as_mut().write(addr, data)
+}
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 4/7] rust: add PL011 device model
2024-07-04 12:15 ` [RFC PATCH v4 4/7] rust: add PL011 device model Manos Pitsidianakis
@ 2024-07-08 16:07 ` Paolo Bonzini
0 siblings, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 16:07 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Alex Bennée, Daniel P. Berrangé, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Thu, Jul 4, 2024 at 2:16 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> +ARM PL011 Rust device
> +M: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +S: Maintained
> +F: rust/pl011/
No need for this, since it's covered by rust/. If (when) it replaces
the main one, the PL011-specific stanza will be assigned to ARM
maintainers (while you keep covering it via rust/).
> +if with_rust
> + subdir('rust')
> +endif
Should be in patch 3.
> +subdir('pl011')
As I said before it should be handled via Kconfig, but let's do that
after the initial merge. However...
> +correctness = { level = "deny", priority = -1 }
> +suspicious = { level = "deny", priority = -1 }
> +complexity = { level = "deny", priority = -1 }
> +perf = { level = "deny", priority = -1 }
> +cargo = { level = "deny", priority = -1 }
> +nursery = { level = "deny", priority = -1 }
> +style = { level = "deny", priority = -1 }
> +# restriction group
> +dbg_macro = "deny"
> +rc_buffer = "deny"
> +as_underscore = "deny"
... repeated lints really suggest that you should use a workspace and
a single cargo invocation to build both the rust-qapi and pl011
crates, which I think is doable if you can run bindgen just once.
> +use core::{mem::MaybeUninit, ptr::NonNull};
Let's remove at least this unsafety.
> +#[used]
> +pub static VMSTATE_PL011: VMStateDescription = VMStateDescription {
> + name: PL011_ARM_INFO.name,
> + unmigratable: true,
> + ..unsafe { MaybeUninit::<VMStateDescription>::zeroed().assume_init() }
> +};
> +
> +#[no_mangle]
> +pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
> + assert!(!obj.is_null());
> + let mut state = NonNull::new_unchecked(obj.cast::<PL011State>());
> + state.as_mut().init();
This is fine for now, but please add a
// TODO: this assumes that "all zeroes" is a valid state for all fields of
// PL011State. This is not necessarily true of any #[repr(Rust)] structs,
// including bilge-generated types. It should instead use MaybeUninit.
> +}
> +
> +qemu_api::module_init! {
> + qom: register_type => {
> + type_register_static(&PL011_ARM_INFO);
> + }
Can you make the macro look like
MODULE_INIT_QOM: fn register_type() {
...
}
so that it's clear what "register_type" is, and so that it's easier to
extend it to more values?
> + #[doc(alias = "clk")]
> + pub clock: NonNull<Clock>,
It's null when init() runs, so please use *mut Clock.
> + #[doc(alias = "migrate_clk")]
> + pub migrate_clock: bool,
Please put all properties together in the struct for readability.
> +}
> +
> +#[used]
> +pub static CLK_NAME: &CStr = c"clk";
> +
> +impl PL011State {
> + pub fn init(&mut self) {
> + unsafe {
> + memory_region_init_io(
> + addr_of_mut!(self.iomem),
> + addr_of_mut!(*self).cast::<Object>(),
> + &PL011_OPS,
> + addr_of_mut!(*self).cast::<c_void>(),
> + PL011_ARM_INFO.name,
> + 0x1000,
> + );
> + let sbd = addr_of_mut!(*self).cast::<SysBusDevice>();
> + let dev = addr_of_mut!(*self).cast::<DeviceState>();
> + sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
> + for irq in self.interrupts.iter_mut() {
> + sysbus_init_irq(sbd, irq);
> + }
> + self.clock = NonNull::new(qdev_init_clock_in(
> + dev,
> + CLK_NAME.as_ptr(),
> + None, /* pl011_clock_update */
> + addr_of_mut!(*self).cast::<c_void>(),
> + ClockEvent_ClockUpdate,
> + ))
> + .unwrap();
> + }
> + }
> +
> + pub fn read(&mut self, offset: hwaddr, _size: core::ffi::c_uint) -> u64 {
> + use RegisterOffset::*;
> +
> + match RegisterOffset::try_from(offset) {
> + Err(v) if (0x3f8..0x400).contains(&v) => {
> + u64::from(PL011_ID_ARM[((offset - 0xfe0) >> 2) as usize])
> + }
> + Err(_) => {
> + // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
> + 0
> + }
> + Ok(DR) => {
> + // s->flags &= ~PL011_FLAG_RXFF;
> + self.flags.set_receive_fifo_full(false);
> + let c = self.read_fifo[self.read_pos];
> + if self.read_count > 0 {
> + self.read_count -= 1;
> + self.read_pos = (self.read_pos + 1) & (self.fifo_depth() - 1);
> + }
> + if self.read_count == 0 {
> + // self.flags |= PL011_FLAG_RXFE;
> + self.flags.set_receive_fifo_empty(true);
> + }
> + if self.read_count + 1 == self.read_trigger {
> + //self.int_level &= ~ INT_RX;
> + self.int_level &= !registers::INT_RX;
> + }
> + // Update error bits.
> + self.receive_status_error_clear = c.to_be_bytes()[3].into();
> + self.update();
> + unsafe { qemu_chr_fe_accept_input(&mut self.char_backend) };
Please add a comment here like
// TODO: this causes a callback that creates another "&mut self".
// This is forbidden by Rust aliasing rules and has to be fixed
// using interior mutability.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
` (3 preceding siblings ...)
2024-07-04 12:15 ` [RFC PATCH v4 4/7] rust: add PL011 device model Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-10 8:44 ` Alex Bennée
2024-07-04 12:15 ` [RFC PATCH v4 6/7] DO NOT MERGE: add rustdoc build for gitlab pages Manos Pitsidianakis
` (2 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson
Set rust source code to diff=rust (built-in with new git versions)
and merge=binary for Cargo.lock files (they should not be merged but
auto-generated by cargo)
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
.gitattributes | 3 +++
1 file changed, 3 insertions(+)
diff --git a/.gitattributes b/.gitattributes
index a217cb7bfe..6dc6383d3d 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -2,3 +2,6 @@
*.h.inc diff=c
*.m diff=objc
*.py diff=python
+*.rs diff=rust
+*.rs.inc diff=rust
+Cargo.lock diff=toml merge=binary
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes
2024-07-04 12:15 ` [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes Manos Pitsidianakis
@ 2024-07-10 8:44 ` Alex Bennée
0 siblings, 0 replies; 38+ messages in thread
From: Alex Bennée @ 2024-07-10 8:44 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Stefan Hajnoczi, Mads Ynddal, Peter Maydell,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson
Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
> Set rust source code to diff=rust (built-in with new git versions)
> and merge=binary for Cargo.lock files (they should not be merged but
> auto-generated by cargo)
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH v4 6/7] DO NOT MERGE: add rustdoc build for gitlab pages
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
` (4 preceding siblings ...)
2024-07-04 12:15 ` [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-04 12:15 ` [RFC PATCH v4 7/7] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
2024-07-08 16:26 ` [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Paolo Bonzini
7 siblings, 0 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
Wainer dos Santos Moschetta, Beraldo Leal
Deploy the generated rustdocs for my personal rust qemu fork on gitlab.
The URL is:
https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
.gitlab-ci.d/buildtest.yml | 64 +++++++++++++++++++++++++++-----------
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0eec570310..a2fedc87bd 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -716,31 +716,57 @@ build-tools-and-docs-debian:
# For contributor forks we want to publish from any repo so
# that users can see the results of their commits, regardless
# of what topic branch they're currently using
+# pages:
+# extends: .base_job_template
+# image: $CI_REGISTRY_IMAGE/qemu/debian:$QEMU_CI_CONTAINER_TAG
+# stage: test
+# needs:
+# - job: build-tools-and-docs-debian
+# script:
+# - mkdir -p public
+# # HTML-ised source tree
+# - make gtags
+# # We unset variables to work around a bug in some htags versions
+# # which causes it to fail when the environment is large
+# - CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
+# -anT --tree-view=filetree -m qemu_init
+# -t "Welcome to the QEMU sourcecode"
+# - mv HTML public/src
+# # Project documentation
+# - make -C build install DESTDIR=$(pwd)/temp-install
+# - mv temp-install/usr/local/share/doc/qemu/* public/
+# artifacts:
+# when: on_success
+# paths:
+# - public
+# variables:
+# QEMU_JOB_PUBLISH: 1
+# The Docker image that will be used to build your app
pages:
- extends: .base_job_template
- image: $CI_REGISTRY_IMAGE/qemu/debian:$QEMU_CI_CONTAINER_TAG
- stage: test
- needs:
- - job: build-tools-and-docs-debian
+ image: rust:latest
script:
- - mkdir -p public
- # HTML-ised source tree
- - make gtags
- # We unset variables to work around a bug in some htags versions
- # which causes it to fail when the environment is large
- - CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
- -anT --tree-view=filetree -m qemu_init
- -t "Welcome to the QEMU sourcecode"
- - mv HTML public/src
- # Project documentation
- - make -C build install DESTDIR=$(pwd)/temp-install
- - mv temp-install/usr/local/share/doc/qemu/* public/
+ - rustup component add rustfmt
+ - DEBIAN_FRONTEND=noninteractive apt-get update -y
+ - DEBIAN_FRONTEND=noninteractive apt-get install -y python3-venv meson libgcrypt20-dev zlib1g-dev autoconf automake libtool bison flex git libglib2.0-dev libfdt-dev libpixman-1-dev ninja-build make libclang-14-dev
+ - cargo install bindgen-cli
+ - mkdir ./build/
+ - cd ./build/
+ - ../configure --enable-system --disable-kvm --target-list=aarch64-softmmu --enable-with-rust
+ - ninja "bindings-aarch64-softmmu.rs"
+ - cp ./bindings-aarch64-softmmu.rs ../rust/qemu-api/src/bindings.rs.inc
+ - cd ../rust/pl011/
+ - cargo tree --depth 1 -e normal --prefix none | cut -d' ' -f1 | xargs
+ printf -- '-p %s\n' | xargs cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
+ - cd ./../..
+ - mv ./rust/pl011/target/x86_64-unknown-linux-gnu/doc ./public
artifacts:
when: on_success
paths:
- public
- variables:
- QEMU_JOB_PUBLISH: 1
+ rules:
+ # This ensures that only pushes to the default branch will trigger
+ # a pages deploy
+ - if: $CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH
coverity:
image: $CI_REGISTRY_IMAGE/qemu/fedora:$QEMU_CI_CONTAINER_TAG
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* [RFC PATCH v4 7/7] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
` (5 preceding siblings ...)
2024-07-04 12:15 ` [RFC PATCH v4 6/7] DO NOT MERGE: add rustdoc build for gitlab pages Manos Pitsidianakis
@ 2024-07-04 12:15 ` Manos Pitsidianakis
2024-07-08 16:26 ` [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Paolo Bonzini
7 siblings, 0 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-04 12:15 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson,
qemu-arm
Convenience patch for testing the rust device.
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
hw/arm/virt.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b0c68d66a3..49dd0b815e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -929,7 +929,11 @@ static void create_uart(const VirtMachineState *vms, int uart,
int irq = vms->irqmap[uart];
const char compat[] = "arm,pl011\0arm,primecell";
const char clocknames[] = "uartclk\0apb_pclk";
+#ifdef CONFIG_WITH_RUST
+ DeviceState *dev = qdev_new("x-pl011-rust");
+#else
DeviceState *dev = qdev_new(TYPE_PL011);
+#endif
SysBusDevice *s = SYS_BUS_DEVICE(dev);
MachineState *ms = MACHINE(vms);
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-04 12:15 [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Manos Pitsidianakis
` (6 preceding siblings ...)
2024-07-04 12:15 ` [RFC PATCH v4 7/7] DO NOT MERGE: replace TYPE_PL011 with x-pl011-rust in arm virt machine Manos Pitsidianakis
@ 2024-07-08 16:26 ` Paolo Bonzini
2024-07-08 16:33 ` Daniel P. Berrangé
7 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 16:26 UTC (permalink / raw)
To: Manos Pitsidianakis, qemu-devel
Cc: Stefan Hajnoczi, Mads Ynddal, Peter Maydell, Alex Bennée,
Daniel P. Berrangé, Marc-André Lureau, Thomas Huth,
Markus Armbruster, Philippe Mathieu-Daudé, Zhao Liu,
Gustavo Romero, Pierrick Bouvier, rowan.hart, Richard Henderson
On 7/4/24 14:15, Manos Pitsidianakis wrote:
> Changes from v3->v4:
> - Add rust-specific files to .gitattributes
> - Added help text to scripts/cargo_wrapper.py arguments (thanks Stephan)
> - Split bindings separate crate
> - Add declarative macros for symbols exported to QEMU to said crate
> - Lowered MSRV to 1.77.2
> - Removed auto-download and install of bindgen-cli
> - Fixed re-compilation of Rust objects in case they are missing from
> filesystem
> - Fixed optimized builds by adding #[used] (thanks Pierrick for the help
> debugging this)
I think the largest issue is that I'd rather have a single cargo build
using a virtual manifest, because my hunch is that it'd be the easiest
path towards Kconfig integration. But it's better to do this after
merge, as the changes are pretty large. It's also independent from any
other changes targeted at removing unsafe code, so no need to hold back
on merging.
Other comments I made that should however be addressed before merging,
from most to least important:
- TODO comments when the code is doing potential undefined behavior
- module structure should IMO resemble the C part of the tree
- only generate bindings.rs.inc once
- a couple abstractions that I'd like to have now: a trait to store the
CStr corresponding to the structs, and one to generate all-zero structs
without having to type "unsafe { MaybeUninit::zeroed().assume_init() }"
- I pointed out a couple lints that are too broad and should be enabled
per-file, even if right now it's basically all files that include them.
- add support for --cargo and CARGO environment variables (if my patch
works without too much hassle)
- I'd like to use ctor instead of non-portable linker magic, and the
cstr crate instead of CStr statics or c""
- please check if -Wl,--whole-archive can be replaced with link_whole:
- probably, until Rust is enabled by default we should treat
dependencies as a moving target and not commit Cargo.lock files. In the
meanwhile we can discuss how to handle them.
And a few aesthetic changes on top of this.
With respect to lints, marking entire groups as "deny" is problematic.
Before merge, I'd rather have the groups as just "warn", and add a long
list of denied lints[1]. After merge we can also add a non-fatal CI job
that runs clippy with nightly rust and with groups marked as "deny".
This matches the check-python-tox job in python/.
[1] https://github.com/bonzini/rust-qemu/commit/95b25f7c5f4e
Thanks,
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 16:26 ` [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011 Paolo Bonzini
@ 2024-07-08 16:33 ` Daniel P. Berrangé
2024-07-08 16:55 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Daniel P. Berrangé @ 2024-07-08 16:33 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Mon, Jul 08, 2024 at 06:26:22PM +0200, Paolo Bonzini wrote:
> On 7/4/24 14:15, Manos Pitsidianakis wrote:
> > Changes from v3->v4:
> > - Add rust-specific files to .gitattributes
> > - Added help text to scripts/cargo_wrapper.py arguments (thanks Stephan)
> > - Split bindings separate crate
> > - Add declarative macros for symbols exported to QEMU to said crate
> > - Lowered MSRV to 1.77.2
> > - Removed auto-download and install of bindgen-cli
> > - Fixed re-compilation of Rust objects in case they are missing from
> > filesystem
> > - Fixed optimized builds by adding #[used] (thanks Pierrick for the help
> > debugging this)
>
> I think the largest issue is that I'd rather have a single cargo build using
> a virtual manifest, because my hunch is that it'd be the easiest path
> towards Kconfig integration. But it's better to do this after merge, as the
> changes are pretty large. It's also independent from any other changes
> targeted at removing unsafe code, so no need to hold back on merging.
>
> Other comments I made that should however be addressed before merging, from
> most to least important:
>
> - TODO comments when the code is doing potential undefined behavior
>
> - module structure should IMO resemble the C part of the tree
>
> - only generate bindings.rs.inc once
>
> - a couple abstractions that I'd like to have now: a trait to store the CStr
> corresponding to the structs, and one to generate all-zero structs without
> having to type "unsafe { MaybeUninit::zeroed().assume_init() }"
>
> - I pointed out a couple lints that are too broad and should be enabled
> per-file, even if right now it's basically all files that include them.
>
> - add support for --cargo and CARGO environment variables (if my patch works
> without too much hassle)
>
> - I'd like to use ctor instead of non-portable linker magic, and the cstr
> crate instead of CStr statics or c""
>
> - please check if -Wl,--whole-archive can be replaced with link_whole:
>
> - probably, until Rust is enabled by default we should treat dependencies as
> a moving target and not commit Cargo.lock files. In the meanwhile we can
> discuss how to handle them.
>
> And a few aesthetic changes on top of this.
This series is still missing changes to enable build on all targets
during CI, including cross-compiles, to prove that we're doing the
correct thing on all our targetted platforms. That's a must have
before considering it suitable for merge.
I also believe we should default to enabling rust toolchain by
default in configure, and require and explicit --without-rust
to disable it, *despite* it not technically being a mandatory
feature....yet.
This is to give users a clear message that Rust is likely to
become a fundamental part of QEMU, so they need to give feedback
if they hit any problems / have use cases we've not anticipated
that are problematic wrt Rust.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 16:33 ` Daniel P. Berrangé
@ 2024-07-08 16:55 ` Paolo Bonzini
2024-07-08 17:12 ` Daniel P. Berrangé
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 16:55 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]
Il lun 8 lug 2024, 18:33 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:
> This series is still missing changes to enable build on all targets
> during CI, including cross-compiles, to prove that we're doing the
> correct thing on all our targetted platforms. That's a must have
> before considering it suitable for merge.
>
But we're not—in particular it's still using several features not in all
supported distros.
I also believe we should default to enabling rust toolchain by
> default in configure, and require and explicit --without-rust
> to disable it, *despite* it not technically being a mandatory
> feature....yet.
>
I guess the detection could be done, but actually enabling the build part
needs to wait until the minimum supported version is low enough.
Paolo
> This is to give users a clear message that Rust is likely to
> become a fundamental part of QEMU, so they need to give feedback
> if they hit any problems / have use cases we've not anticipated
> that are problematic wrt Rust.
>
> 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 :|
>
>
[-- Attachment #2: Type: text/html, Size: 2779 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 16:55 ` Paolo Bonzini
@ 2024-07-08 17:12 ` Daniel P. Berrangé
2024-07-08 18:34 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Daniel P. Berrangé @ 2024-07-08 17:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Mon, Jul 08, 2024 at 06:55:40PM +0200, Paolo Bonzini wrote:
> Il lun 8 lug 2024, 18:33 Daniel P. Berrangé <berrange@redhat.com> ha
> scritto:
>
> > This series is still missing changes to enable build on all targets
> > during CI, including cross-compiles, to prove that we're doing the
> > correct thing on all our targetted platforms. That's a must have
> > before considering it suitable for merge.
> >
>
> But we're not—in particular it's still using several features not in all
> supported distros.
That's exactly why I suggest its a pre-requisite for merging
this. Unless we're able to demonstrate that we can enable
Rust on all our CI platforms, the benefits of Rust will
not be realized in QEMU, and we'll have never ending debates
about whether each given feature needs to be in C or Rust.
> I also believe we should default to enabling rust toolchain by
> > default in configure, and require and explicit --without-rust
> > to disable it, *despite* it not technically being a mandatory
> > feature....yet.
> >
>
> I guess the detection could be done, but actually enabling the build part
> needs to wait until the minimum supported version is low enough.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 17:12 ` Daniel P. Berrangé
@ 2024-07-08 18:34 ` Paolo Bonzini
2024-07-08 18:39 ` Manos Pitsidianakis
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 18:34 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]
Il lun 8 lug 2024, 19:12 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:
> That's exactly why I suggest its a pre-requisite for merging
> this. Unless we're able to demonstrate that we can enable
> Rust on all our CI platforms, the benefits of Rust will
> not be realized in QEMU, and we'll have never ending debates
> about whether each given feature needs to be in C or Rust.
>
In that case we should develop it on a branch, so that more than one person
can contribute (unlike if we keep iterating on this RFC).
Paolo
>
> > I also believe we should default to enabling rust toolchain by
> > > default in configure, and require and explicit --without-rust
> > > to disable it, *despite* it not technically being a mandatory
> > > feature....yet.
> > >
> >
> > I guess the detection could be done, but actually enabling the build part
> > needs to wait until the minimum supported version is low enough.
>
> 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 :|
>
>
[-- Attachment #2: Type: text/html, Size: 2457 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 18:34 ` Paolo Bonzini
@ 2024-07-08 18:39 ` Manos Pitsidianakis
2024-07-08 18:48 ` Paolo Bonzini
2024-07-09 10:34 ` Manos Pitsidianakis
0 siblings, 2 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-08 18:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 749 bytes --]
On Mon, 8 Jul 2024, 21:34 Paolo Bonzini, <pbonzini@redhat.com> wrote:
>
>
> Il lun 8 lug 2024, 19:12 Daniel P. Berrangé <berrange@redhat.com> ha
> scritto:
>
>> That's exactly why I suggest its a pre-requisite for merging
>> this. Unless we're able to demonstrate that we can enable
>> Rust on all our CI platforms, the benefits of Rust will
>> not be realized in QEMU, and we'll have never ending debates
>> about whether each given feature needs to be in C or Rust.
>>
>
> In that case we should develop it on a branch, so that more than one
> person can contribute (unlike if we keep iterating on this RFC).
>
> Paolo
>
If you do, I'd really appreciate it if you did not use any part of my
patches.
Thanks,
Manos
[-- Attachment #2: Type: text/html, Size: 1606 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 18:39 ` Manos Pitsidianakis
@ 2024-07-08 18:48 ` Paolo Bonzini
2024-07-09 7:38 ` Manos Pitsidianakis
2024-07-09 10:34 ` Manos Pitsidianakis
1 sibling, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-08 18:48 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]
Il lun 8 lug 2024, 20:39 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
ha scritto:
>
>
> On Mon, 8 Jul 2024, 21:34 Paolo Bonzini, <pbonzini@redhat.com> wrote:
>
>>
>>
>> Il lun 8 lug 2024, 19:12 Daniel P. Berrangé <berrange@redhat.com> ha
>> scritto:
>>
>>> That's exactly why I suggest its a pre-requisite for merging
>>> this. Unless we're able to demonstrate that we can enable
>>> Rust on all our CI platforms, the benefits of Rust will
>>> not be realized in QEMU, and we'll have never ending debates
>>> about whether each given feature needs to be in C or Rust.
>>>
>>
>> In that case we should develop it on a branch, so that more than one
>> person can contribute (unlike if we keep iterating on this RFC).
>>
>
> If you do, I'd really appreciate it if you did not use any part of my
> patches.
>
"We" means that you would accept patches, review them and apply them until
any agreed-upon conditions for merging are satisfied, and then send either
a final series or a pull request for inclusion in QEMU.
Paolo
[-- Attachment #2: Type: text/html, Size: 2174 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 18:48 ` Paolo Bonzini
@ 2024-07-09 7:38 ` Manos Pitsidianakis
2024-07-09 7:54 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-09 7:38 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Mon, 8 Jul 2024 at 21:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
>
> Il lun 8 lug 2024, 20:39 Manos Pitsidianakis <manos.pitsidianakis@linaro.org> ha scritto:
>>
>>
>>
>> On Mon, 8 Jul 2024, 21:34 Paolo Bonzini, <pbonzini@redhat.com> wrote:
>>>
>>>
>>>
>>> Il lun 8 lug 2024, 19:12 Daniel P. Berrangé <berrange@redhat.com> ha scritto:
>>>>
>>>> That's exactly why I suggest its a pre-requisite for merging
>>>> this. Unless we're able to demonstrate that we can enable
>>>> Rust on all our CI platforms, the benefits of Rust will
>>>> not be realized in QEMU, and we'll have never ending debates
>>>> about whether each given feature needs to be in C or Rust.
>>>
>>>
>>> In that case we should develop it on a branch, so that more than one person can contribute (unlike if we keep iterating on this RFC).
>>
>>
>> If you do, I'd really appreciate it if you did not use any part of my patches.
>
>
> "We" means that you would accept patches, review them and apply them until any agreed-upon conditions for merging are satisfied, and then send either a final series or a pull request for inclusion in QEMU.
Ah, alright. That wasn't obvious because that e-mail was not directed
to me nor did it mention my name :)
I do not want to do that, in any case. I do not think it's the right approach.
--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-09 7:38 ` Manos Pitsidianakis
@ 2024-07-09 7:54 ` Paolo Bonzini
2024-07-09 12:18 ` Daniel P. Berrangé
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-09 7:54 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Tue, Jul 9, 2024 at 9:38 AM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
> Ah, alright. That wasn't obvious because that e-mail was not directed
> to me nor did it mention my name :)
Oh, ok. Sorry about that. Generally when I say "we" I include as large
a part of the community as applicable.
> I do not want to do that, in any case. I do not think it's the right approach.
No problem with that (and in fact I agree, as I'd prefer a speedy
merge and doing the work on the QEMU master branch); however, we need
to reach an agreement on that and everybody (including Daniel) needs
to explain the reason for their position.
Daniel's proposed criteria for merging include:
- CI integration
- CI passing for all supported targets (thus lowering the MSRV to 1.63.0)
- plus any the code changes that were or will be requested during review
That seems to be a pretty high amount of work, and until it's done
everyone else is unable to contribute, not even in directions
orthogonal to the above (cross compilation support, less unsafe code,
porting more devices). So something has to give: either we decide for
an early merge, where the code is marked as experimental and disabled
by default. Personally I think it's fine, the contingency plan is
simply to "git rm -rf rust/". Or we can keep the above stringent
requirements for merging, but then I don't see it as a one-person job.
If I can say so, developing on a branch would also be a useful warm-up
for you in the maintainer role, if we expect that there will be
significant community contributions to Rust.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-09 7:54 ` Paolo Bonzini
@ 2024-07-09 12:18 ` Daniel P. Berrangé
2024-07-09 16:51 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Daniel P. Berrangé @ 2024-07-09 12:18 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Tue, Jul 09, 2024 at 09:54:43AM +0200, Paolo Bonzini wrote:
> On Tue, Jul 9, 2024 at 9:38 AM Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> > Ah, alright. That wasn't obvious because that e-mail was not directed
> > to me nor did it mention my name :)
>
> Oh, ok. Sorry about that. Generally when I say "we" I include as large
> a part of the community as applicable.
>
> > I do not want to do that, in any case. I do not think it's the right approach.
>
> No problem with that (and in fact I agree, as I'd prefer a speedy
> merge and doing the work on the QEMU master branch); however, we need
> to reach an agreement on that and everybody (including Daniel) needs
> to explain the reason for their position.
>
> Daniel's proposed criteria for merging include:
> - CI integration
> - CI passing for all supported targets (thus lowering the MSRV to 1.63.0)
> - plus any the code changes that were or will be requested during review
>
> That seems to be a pretty high amount of work, and until it's done
> everyone else is unable to contribute, not even in directions
> orthogonal to the above (cross compilation support, less unsafe code,
> porting more devices).
My thought is that the initial merge focuses only on the build system
integration. So that's basically patches 1 + 2 in this series.
IMHO that is small enough that we should be able to demonstrate that
we detect Rust, run bindgen & compile its result, on all our supported
platforms without an unreasonable amount of effort.
> So something has to give: either we decide for
> an early merge, where the code is marked as experimental and disabled
> by default. Personally I think it's fine, the contingency plan is
> simply to "git rm -rf rust/". Or we can keep the above stringent
> requirements for merging, but then I don't see it as a one-person job.
Patch 3, the high level APIs is where I see most of the work and
collaboration being needed, but that doesn't need to be rushed into
the first merge. We would have a "rust" subsystem + maintainer who
would presumably have a staging tree, etc in the normal way we work
and collaborate
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-09 12:18 ` Daniel P. Berrangé
@ 2024-07-09 16:51 ` Paolo Bonzini
2024-07-09 18:02 ` Richard Henderson
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-07-09 16:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Manos Pitsidianakis, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Tue, Jul 9, 2024 at 2:18 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> My thought is that the initial merge focuses only on the build system
> integration. So that's basically patches 1 + 2 in this series.
>
> Patch 3, the high level APIs is where I see most of the work and
> collaboration being needed, but that doesn't need to be rushed into
> the first merge. We would have a "rust" subsystem + maintainer who
> would presumably have a staging tree, etc in the normal way we work
> and collaborate
It's complicated. A "perfect" build system integration would include
integration with Kconfig, but it's not simple work and it may not be
Manos's preference for what to work on (or maybe it is), and it's also
not a blocker for further work on patches 3-4.
On the other hand, patches 3 and 4 are _almost_ ready except for
requiring a very new Rust - we know how to tackle that, but again it
may take some time and it's completely separate work from better build
system integration.
In other words, improving build system integration is harder until
merge, but merge is blocked by independent work on lowering the
minimum supported Rust version. This is why I liked the idea of having
either a development tree to allow a merge into early 9.2.
On the other hand, given the exceptional scope (completely new code
that can be disabled at will) and circumstances, even a very early
merge into 9.1 (disabled by default) might be better to provide
developers with the easiest base for experimenting. The requirements
for merging, here, would basically amount to a good roadmap and some
established good habits.
An merge into early 9.2 would be a bit harder for experimenting, while
merging it now would sacrifice CI integration in the initial stages of
the work but make cooperation easier.
However, it's difficult to say what's best without knowing Manos's
rationale for preferring not to have a development tree yet.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-09 16:51 ` Paolo Bonzini
@ 2024-07-09 18:02 ` Richard Henderson
0 siblings, 0 replies; 38+ messages in thread
From: Richard Henderson @ 2024-07-09 18:02 UTC (permalink / raw)
To: Paolo Bonzini, Daniel P. Berrangé; +Cc: Manos Pitsidianakis, qemu-devel
On 7/9/24 09:51, Paolo Bonzini wrote:
> On Tue, Jul 9, 2024 at 2:18 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> My thought is that the initial merge focuses only on the build system
>> integration. So that's basically patches 1 + 2 in this series.
>>
>> Patch 3, the high level APIs is where I see most of the work and
>> collaboration being needed, but that doesn't need to be rushed into
>> the first merge. We would have a "rust" subsystem + maintainer who
>> would presumably have a staging tree, etc in the normal way we work
>> and collaborate
>
> It's complicated. A "perfect" build system integration would include
> integration with Kconfig, but it's not simple work and it may not be
> Manos's preference for what to work on (or maybe it is), and it's also
> not a blocker for further work on patches 3-4.
>
> On the other hand, patches 3 and 4 are _almost_ ready except for
> requiring a very new Rust - we know how to tackle that, but again it
> may take some time and it's completely separate work from better build
> system integration.
>
> In other words, improving build system integration is harder until
> merge, but merge is blocked by independent work on lowering the
> minimum supported Rust version. This is why I liked the idea of having
> either a development tree to allow a merge into early 9.2.
>
> On the other hand, given the exceptional scope (completely new code
> that can be disabled at will) and circumstances, even a very early
> merge into 9.1 (disabled by default) might be better to provide
> developers with the easiest base for experimenting. The requirements
> for merging, here, would basically amount to a good roadmap and some
> established good habits.
>
> An merge into early 9.2 would be a bit harder for experimenting, while
> merging it now would sacrifice CI integration in the initial stages of
> the work but make cooperation easier.
I would be in favor of a 9.1 merge (disabled, not auto).
Noting that soft-freeze is two weeks from today, so no dilly dallying. :-)
The only reasonable alternative that I see is the kind of development branch that qemu is
not used to having: a long-term shared branch on qemu-projects. With which I would be ok,
up to and including a branch merge at the end, as I'm used to working that way with other
projects.
I think the only reason to delay the current development work until 9.2 is if we were
still questioning whether using Rust *at all* is a good idea. I think we're beyond that
point.
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH v4 0/7] Add Rust support, implement ARM PL011
2024-07-08 18:39 ` Manos Pitsidianakis
2024-07-08 18:48 ` Paolo Bonzini
@ 2024-07-09 10:34 ` Manos Pitsidianakis
1 sibling, 0 replies; 38+ messages in thread
From: Manos Pitsidianakis @ 2024-07-09 10:34 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Daniel P. Berrangé, qemu-devel, Stefan Hajnoczi, Mads Ynddal,
Peter Maydell, Alex Bennée, Marc-André Lureau,
Thomas Huth, Markus Armbruster, Philippe Mathieu-Daudé,
Zhao Liu, Gustavo Romero, Pierrick Bouvier, rowan.hart,
Richard Henderson
On Mon, 8 Jul 2024 at 21:39, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
>
>
> On Mon, 8 Jul 2024, 21:34 Paolo Bonzini, <pbonzini@redhat.com> wrote:
>>
>>
>>
>> Il lun 8 lug 2024, 19:12 Daniel P. Berrangé <berrange@redhat.com> ha scritto:
>>>
>>> That's exactly why I suggest its a pre-requisite for merging
>>> this. Unless we're able to demonstrate that we can enable
>>> Rust on all our CI platforms, the benefits of Rust will
>>> not be realized in QEMU, and we'll have never ending debates
>>> about whether each given feature needs to be in C or Rust.
>>
>>
>> In that case we should develop it on a branch, so that more than one person can contribute (unlike if we keep iterating on this RFC).
>>
>> Paolo
>
>
>
>
> If you do, I'd really appreciate it if you did not use any part of my patches.
Someone pointed out to me that my wording is really bad here and I
agree, I apologize. I did not express myself with the right words.
What I wanted to say is that hypothetically my work can be used as a
base just not the patches/sign offs. The changes in the patches are
all open source and part of the QEMU community now, of course.
^ permalink raw reply [flat|nested] 38+ messages in thread