[PULL 00/49] rust, QOM, kvm changes for 2024-12-11

[PULL 00/49] rust, QOM, kvm changes for 2024-12-11

[Paolo Bonzini]

The following changes since commit ae35f033b874c627d81d51070187fbf55f0bf1a7:

  Update version for v9.2.0 release (2024-12-10 16:20:54 +0000)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 166e8a1fd15bfa527b25fc15ca315e572c0556d2:

  rust: qom: change the parent type to an associated type (2024-12-11 15:57:19 +0100)

----------------------------------------------------------------
* rust: better integration with clippy, rustfmt and rustdoc
* rust: interior mutability types
* rust: add a bit operations module
* rust: first part of QOM rework
* kvm: remove unnecessary #ifdef
* clock: small cleanups, improve handling of Clock lifetimes

----------------------------------------------------------------
Junjie Mao (1):
      rust/qemu-api: Fix fragment-specifiers in define_property macro

Paolo Bonzini (32):
      ci: enable rust in the Debian and Ubuntu system build job
      rust: apply --cfg MESON to all crates
      rust: allow using build-root bindings.rs from cargo
      rust: build: move rustc_args.py invocation to qemu-api crate
      rust: build: restrict --cfg generation to only required symbols
      rust: build: generate lint flags from Cargo.toml
      rust: cargo: store desired warning levels in workspace Cargo.toml
      rust: build: move strict lints handling to rustc_args.py
      rust: fix a couple style issues from clippy
      rust: build: establish a baseline of lints across all crates
      rust: build: add "make clippy", "make rustfmt", "make rustdoc"
      rust: ci: add job that runs Rust tools
      rust: fix doc test syntax
      clock: clear callback on unparent
      clock: treat outputs and inputs the same in NamedClockList
      clock: inline most of qdev_init_clocklist
      kvm: remove unnecessary #ifdef
      bql: check that the BQL is not dropped within marked sections
      rust: cell: add BQL-enforcing Cell variant
      rust: cell: add BQL-enforcing RefCell variant
      rust: define prelude
      rust: add bindings for interrupt sources
      rust: add a bit operation module
      rust: qom: add default definitions for ObjectImpl
      rust: qom: rename Class trait to ClassInitImpl
      rust: qom: convert type_info! macro to an associated const
      rust: qom: move ClassInitImpl to the instance side
      rust: qdev: move device_class_init! body to generic function, ClassInitImpl implementation to macro
      rust: qdev: move bridge for realize and reset functions out of pl011
      rust: qom: move bridge for TypeInfo functions out of pl011
      rust: qom: split ObjectType from ObjectImpl trait
      rust: qom: change the parent type to an associated type

Zhao Liu (16):
      arm: Replace type_register() with type_register_static()
      hw/block: Replace type_register() with type_register_static()
      hw/net: Replace type_register() with type_register_static()
      ppc: Replace type_register() with type_register_static()
      hw/rtc: Replace type_register() with type_register_static()
      hw/scsi: Replace type_register() with type_register_static()
      hw/sensor: Replace type_register() with type_register_static()
      hw/usb: Replace type_register() with type_register_static()
      hw/virtio: Replace type_register() with type_register_static()
      i386: Replace type_register() with type_register_static()
      target/mips: Replace type_register() with type_register_static()
      target/sparc: Replace type_register() with type_register_static()
      target/xtensa: Replace type_register() with type_register_static()
      ui: Replace type_register() with type_register_static()
      script/codeconverter/qom_type_info: Deprecate MakeTypeRegisterStatic and MakeTypeRegisterNotStatic
      qom/object: Remove type_register()

 meson.build                                        |  57 +-
 include/hw/clock.h                                 |   8 -
 include/hw/i386/pc.h                               |   4 +-
 include/qemu/main-loop.h                           |  15 +
 include/qom/object.h                               |  14 -
 target/i386/kvm/kvm_i386.h                         |  11 +-
 hw/arm/armsse.c                                    |   2 +-
 hw/arm/smmuv3.c                                    |   4 +-
 hw/block/m25p80.c                                  |   2 +-
 hw/core/clock.c                                    |  22 +-
 hw/core/qdev-clock.c                               |  86 +--
 hw/net/e1000.c                                     |   2 +-
 hw/net/eepro100.c                                  |   2 +-
 hw/ppc/spapr.c                                     |   2 +-
 hw/rtc/m48t59-isa.c                                |   2 +-
 hw/rtc/m48t59.c                                    |   2 +-
 hw/scsi/megasas.c                                  |   2 +-
 hw/scsi/mptsas.c                                   |   2 +-
 hw/sensor/tmp421.c                                 |   2 +-
 hw/usb/hcd-ehci-pci.c                              |   2 +-
 hw/usb/hcd-uhci.c                                  |   2 +-
 hw/virtio/virtio-pci.c                             |   8 +-
 qom/object.c                                       |   7 +-
 stubs/iothread-lock.c                              |  15 +
 system/cpus.c                                      |  15 +
 target/arm/cpu.c                                   |   2 +-
 target/arm/cpu64.c                                 |   2 +-
 target/i386/cpu.c                                  |   2 +-
 target/mips/cpu.c                                  |   2 +-
 target/ppc/kvm.c                                   |   2 +-
 target/sparc/cpu.c                                 |   2 +-
 target/xtensa/helper.c                             |   2 +-
 ui/console-vc.c                                    |   2 +-
 ui/dbus.c                                          |   2 +-
 ui/gtk.c                                           |   2 +-
 ui/spice-app.c                                     |   2 +-
 .gitlab-ci.d/buildtest.yml                         |   4 +-
 .gitlab-ci.d/static_checks.yml                     |  24 +
 rust/Cargo.toml                                    |  82 ++
 rust/hw/char/pl011/.gitignore                      |   2 -
 rust/hw/char/pl011/Cargo.toml                      |   3 +
 rust/hw/char/pl011/src/device.rs                   | 124 ++--
 rust/hw/char/pl011/src/device_class.rs             |  34 -
 rust/hw/char/pl011/src/lib.rs                      |  19 +-
 rust/hw/char/pl011/src/memory_ops.rs               |   4 +-
 rust/meson.build                                   |  22 +
 rust/qemu-api-macros/Cargo.toml                    |   3 +
 rust/qemu-api/.gitignore                           |   2 +-
 rust/qemu-api/Cargo.toml                           |   8 +-
 rust/qemu-api/README.md                            |  10 +-
 rust/qemu-api/build.rs                             |  39 +-
 rust/qemu-api/meson.build                          |  14 +-
 rust/qemu-api/src/bindings.rs                      |  29 +
 rust/qemu-api/src/bitops.rs                        | 119 +++
 rust/qemu-api/src/cell.rs                          | 822 +++++++++++++++++++++
 rust/qemu-api/src/definitions.rs                   | 149 +++-
 rust/qemu-api/src/device_class.rs                  | 126 +++-
 rust/qemu-api/src/irq.rs                           |  91 +++
 rust/qemu-api/src/lib.rs                           |  29 +-
 rust/qemu-api/src/prelude.rs                       |  10 +
 rust/qemu-api/src/sysbus.rs                        |  33 +
 rust/qemu-api/src/zeroable.rs                      |   6 +-
 rust/qemu-api/tests/tests.rs                       |  43 +-
 .../codeconverter/codeconverter/qom_type_info.py   |  20 -
 scripts/rust/rustc_args.py                         | 181 ++++-
 .../docker/dockerfiles/fedora-rust-nightly.docker  |   4 +
 tests/lcitool/refresh                              |   4 +
 67 files changed, 1927 insertions(+), 445 deletions(-)
 delete mode 100644 rust/hw/char/pl011/.gitignore
 create mode 100644 rust/qemu-api/src/bindings.rs
 create mode 100644 rust/qemu-api/src/bitops.rs
 create mode 100644 rust/qemu-api/src/cell.rs
 create mode 100644 rust/qemu-api/src/irq.rs
 create mode 100644 rust/qemu-api/src/prelude.rs
 create mode 100644 rust/qemu-api/src/sysbus.rs
-- 
2.47.1

[PULL 01/49] ci: enable rust in the Debian and Ubuntu system build job

[Paolo Bonzini]

We have fixed all incompatibilities with older versions of rustc
and bindgen.  Enable Rust on Debian to check that the minimum
supported version of Rust is indeed 1.63.0, and 0.60.x for bindgen.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 336223484d8..4265a577834 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -40,7 +40,7 @@ build-system-ubuntu:
     job: amd64-ubuntu2204-container
   variables:
     IMAGE: ubuntu2204
-    CONFIGURE_ARGS: --enable-docs
+    CONFIGURE_ARGS: --enable-docs --enable-rust
     TARGETS: alpha-softmmu microblazeel-softmmu mips64el-softmmu
     MAKE_CHECK_ARGS: check-build
 
@@ -71,7 +71,7 @@ build-system-debian:
     job: amd64-debian-container
   variables:
     IMAGE: debian
-    CONFIGURE_ARGS: --with-coroutine=sigaltstack
+    CONFIGURE_ARGS: --with-coroutine=sigaltstack --enable-rust
     TARGETS: arm-softmmu i386-softmmu riscv64-softmmu sh4eb-softmmu
       sparc-softmmu xtensa-softmmu
     MAKE_CHECK_ARGS: check-build
-- 
2.47.1

[PULL 02/49] rust: apply --cfg MESON to all crates

[Paolo Bonzini]

We might have more uses for --cfg MESON, even though right now it's only
qemu-api that has generated files.  Since we're going to add more flags
to the add_project_arguments calls for Rust, it makes sense to also add
--cfg MESON everywhere.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build               | 7 ++++---
 rust/qemu-api/meson.build | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 147097c652e..c35ce64cd61 100644
--- a/meson.build
+++ b/meson.build
@@ -3422,10 +3422,11 @@ if have_rust
   # is safe; https://github.com/rust-lang/rust/pull/54675 says that
   # passing -nodefaultlibs to the linker "was more ideological to
   # start with than anything".
-  add_project_arguments(rustc_args + ['-C', 'default-linker-libraries'],
+  add_project_arguments(rustc_args +
+      ['--cfg', 'MESON', '-C', 'default-linker-libraries'],
       native: false, language: 'rust')
-
-  add_project_arguments(rustc_args, native: true, language: 'rust')
+  add_project_arguments(rustc_args + ['--cfg', 'MESON'],
+      native: true, language: 'rust')
 endif
 
 hxtool = find_program('scripts/hxtool')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 6f637af7b1b..cad9ac4844e 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,4 +1,4 @@
-_qemu_api_cfg = ['--cfg', 'MESON']
+_qemu_api_cfg = []
 # _qemu_api_cfg += ['--cfg', 'feature="allocator"']
 if rustc.version().version_compare('>=1.77.0')
   _qemu_api_cfg += ['--cfg', 'has_offset_of']
-- 
2.47.1

[PULL 03/49] rust: allow using build-root bindings.rs from cargo

[Paolo Bonzini]

Right now, using cargo with QEMU requires copying by hand the bindings.rs to the
source tree.  Instead, we can use an include file to escape the cage of cargo's
mandated source directory structure.

By running cargo within meson's "devenv" and adding a MESON_BUILD_ROOT
environment variable, it is easy for build.rs to find the file.  However, the
file must be symlinked into cargo's output directory for rust-analyzer to find
it.

Suggested-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                   |  4 +++-
 rust/hw/char/pl011/.gitignore |  2 --
 rust/qemu-api/.gitignore      |  2 +-
 rust/qemu-api/README.md       |  4 ++--
 rust/qemu-api/build.rs        | 34 +++++++++++++++++++++++++++-------
 rust/qemu-api/meson.build     |  1 +
 rust/qemu-api/src/bindings.rs | 29 +++++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs      | 22 ----------------------
 8 files changed, 63 insertions(+), 35 deletions(-)
 delete mode 100644 rust/hw/char/pl011/.gitignore
 create mode 100644 rust/qemu-api/src/bindings.rs

diff --git a/meson.build b/meson.build
index c35ce64cd61..ea211c2dbb4 100644
--- a/meson.build
+++ b/meson.build
@@ -3,6 +3,8 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
                           'b_staticpic=false', 'stdsplit=false', 'optimization=2', 'b_pie=true'],
         version: files('VERSION'))
 
+meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
+
 add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
 add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
 add_test_setup('thorough', env: ['G_TEST_SLOW=1', 'SPEED=thorough'])
@@ -4090,7 +4092,7 @@ if have_rust
   bindings_rs = rust.bindgen(
     input: 'rust/wrapper.h',
     dependencies: common_ss.all_dependencies(),
-    output: 'bindings.rs',
+    output: 'bindings.inc.rs',
     include_directories: include_directories('.', 'include'),
     bindgen_version: ['>=0.60.0'],
     args: bindgen_args,
diff --git a/rust/hw/char/pl011/.gitignore b/rust/hw/char/pl011/.gitignore
deleted file mode 100644
index 71eaff2035d..00000000000
--- a/rust/hw/char/pl011/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-# Ignore generated bindings file overrides.
-src/bindings.rs.inc
diff --git a/rust/qemu-api/.gitignore b/rust/qemu-api/.gitignore
index b9e7e004c86..df6c2163e03 100644
--- a/rust/qemu-api/.gitignore
+++ b/rust/qemu-api/.gitignore
@@ -1,2 +1,2 @@
 # Ignore generated bindings file overrides.
-src/bindings.rs
+/src/bindings.inc.rs
diff --git a/rust/qemu-api/README.md b/rust/qemu-api/README.md
index 7588fa29ef3..53810f48882 100644
--- a/rust/qemu-api/README.md
+++ b/rust/qemu-api/README.md
@@ -5,7 +5,7 @@ This library exports helper Rust types, Rust macros and C FFI bindings for inter
 The C bindings can be generated with `bindgen`, using this build target:
 
 ```console
-$ ninja bindings.rs
+$ ninja bindings.inc.rs
 ```
 
 ## Generate Rust documentation
@@ -13,5 +13,5 @@ $ ninja bindings.rs
 To generate docs for this crate, including private items:
 
 ```sh
-cargo doc --no-deps --document-private-items
+pyvenv/bin/meson devenv -w ../rust cargo doc --no-deps --document-private-items
 ```
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
index 20f8f718b90..40644d5e4fe 100644
--- a/rust/qemu-api/build.rs
+++ b/rust/qemu-api/build.rs
@@ -2,22 +2,42 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::path::Path;
+#[cfg(unix)]
+use std::os::unix::fs::symlink as symlink_file;
+#[cfg(windows)]
+use std::os::windows::fs::symlink_file;
+use std::{env, fs::remove_file, io::Result, path::Path};
 
 use version_check as rustc;
 
-fn main() {
-    if !Path::new("src/bindings.rs").exists() {
-        panic!(
-            "No generated C bindings found! Either build them manually with bindgen or with meson \
-             (`ninja bindings.rs`) and copy them to src/bindings.rs, or build through meson."
-        );
+fn main() -> Result<()> {
+    // Placing bindings.inc.rs in the source directory is supported
+    // but not documented or encouraged.
+    let path = env::var("MESON_BUILD_ROOT")
+        .unwrap_or_else(|_| format!("{}/src", env!("CARGO_MANIFEST_DIR")));
+
+    let file = format!("{}/bindings.inc.rs", path);
+    let file = Path::new(&file);
+    if !Path::new(&file).exists() {
+        panic!(concat!(
+            "No generated C bindings found! If you want to run `cargo`, start a subshell\n",
+            "with `meson devenv`, or point MESON_BUILD_ROOT to the top of the build tree."
+        ));
     }
 
+    let out_dir = env::var("OUT_DIR").unwrap();
+    let dest_path = format!("{}/bindings.inc.rs", out_dir);
+    let dest_path = Path::new(&dest_path);
+    if dest_path.symlink_metadata().is_ok() {
+        remove_file(dest_path)?;
+    }
+    symlink_file(file, dest_path)?;
+
     // Check for available rustc features
     if rustc::is_min_version("1.77.0").unwrap_or(false) {
         println!("cargo:rustc-cfg=has_offset_of");
     }
 
     println!("cargo:rerun-if-changed=build.rs");
+    Ok(())
 }
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index cad9ac4844e..3be7b7e5ced 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -9,6 +9,7 @@ _qemu_api_rs = static_library(
   structured_sources(
     [
       'src/lib.rs',
+      'src/bindings.rs',
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
new file mode 100644
index 00000000000..0b76ec58bee
--- /dev/null
+++ b/rust/qemu-api/src/bindings.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#![allow(
+    dead_code,
+    improper_ctypes_definitions,
+    improper_ctypes,
+    non_camel_case_types,
+    non_snake_case,
+    non_upper_case_globals,
+    unsafe_op_in_unsafe_fn,
+    clippy::missing_const_for_fn,
+    clippy::too_many_arguments,
+    clippy::approx_constant,
+    clippy::use_self,
+    clippy::useless_transmute,
+    clippy::missing_safety_doc
+)]
+
+#[cfg(MESON)]
+include!("bindings.inc.rs");
+
+#[cfg(not(MESON))]
+include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
+
+unsafe impl Send for Property {}
+unsafe impl Sync for Property {}
+unsafe impl Sync for TypeInfo {}
+unsafe impl Sync for VMStateDescription {}
+unsafe impl Sync for VMStateField {}
+unsafe impl Sync for VMStateInfo {}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index aa8d16ec94b..440aff3817d 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -4,31 +4,9 @@
 
 #![cfg_attr(not(MESON), doc = include_str!("../README.md"))]
 
-#[allow(
-    dead_code,
-    improper_ctypes_definitions,
-    improper_ctypes,
-    non_camel_case_types,
-    non_snake_case,
-    non_upper_case_globals,
-    unsafe_op_in_unsafe_fn,
-    clippy::missing_const_for_fn,
-    clippy::too_many_arguments,
-    clippy::approx_constant,
-    clippy::use_self,
-    clippy::useless_transmute,
-    clippy::missing_safety_doc,
-)]
 #[rustfmt::skip]
 pub mod bindings;
 
-unsafe impl Send for bindings::Property {}
-unsafe impl Sync for bindings::Property {}
-unsafe impl Sync for bindings::TypeInfo {}
-unsafe impl Sync for bindings::VMStateDescription {}
-unsafe impl Sync for bindings::VMStateField {}
-unsafe impl Sync for bindings::VMStateInfo {}
-
 pub mod c_str;
 pub mod definitions;
 pub mod device_class;
-- 
2.47.1

[PULL 04/49] rust: build: move rustc_args.py invocation to qemu-api crate

[Paolo Bonzini]

Only qemu-api needs access to the symbols in config-host.h.  Remove
the temptation to use them elsewhere by limiting the --cfg arguments to
the qemu-api crate.

Per-crate invocation of the script will also be needed to add --check-cfg
options for each crate's features (when more complex, build-time
configurable devices are added in the future).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build               | 56 +++++++++++++++++----------------------
 rust/qemu-api/meson.build |  5 +++-
 2 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/meson.build b/meson.build
index ea211c2dbb4..2f1f2aece4d 100644
--- a/meson.build
+++ b/meson.build
@@ -120,7 +120,30 @@ if have_rust
 endif
 
 if have_rust
+  rustc_args = find_program('scripts/rust/rustc_args.py')
   rustfmt = find_program('rustfmt', required: false)
+
+  # Prohibit code that is forbidden in Rust 2024
+  rustc_lint_args = ['-D', 'unsafe_op_in_unsafe_fn']
+
+  # Occasionally, we may need to silence warnings and clippy lints that
+  # were only introduced in newer Rust compiler versions.  Do not croak
+  # in that case; a CI job with rust_strict_lints == true ensures that
+  # we do not have misspelled allow() attributes.
+  if not get_option('strict_rust_lints')
+    rustc_lint_args += ['-A', 'unknown_lints']
+  endif
+
+  # Apart from procedural macros, our Rust executables will often link
+  # with C code, so include all the libraries that C code needs.  This
+  # is safe; https://github.com/rust-lang/rust/pull/54675 says that
+  # passing -nodefaultlibs to the linker "was more ideological to
+  # start with than anything".
+  add_project_arguments(rustc_lint_args +
+      ['--cfg', 'MESON', '-C', 'default-linker-libraries'],
+      native: false, language: 'rust')
+  add_project_arguments(rustc_lint_args + ['--cfg', 'MESON'],
+      native: true, language: 'rust')
 endif
 
 dtrace = not_found
@@ -3399,37 +3422,8 @@ endif
 # Generated sources #
 #####################
 
-genh += configure_file(output: 'config-host.h', configuration: config_host_data)
-
-if have_rust
-  rustc_args = run_command(
-    find_program('scripts/rust/rustc_args.py'),
-    '--config-headers', meson.project_build_root() / 'config-host.h',
-    capture : true,
-    check: true).stdout().strip().split()
-
-  # Prohibit code that is forbidden in Rust 2024
-  rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
-
-  # Occasionally, we may need to silence warnings and clippy lints that
-  # were only introduced in newer Rust compiler versions.  Do not croak
-  # in that case; a CI job with rust_strict_lints == true ensures that
-  # we do not have misspelled allow() attributes.
-  if not get_option('strict_rust_lints')
-    rustc_args += ['-A', 'unknown_lints']
-  endif
-
-  # Apart from procedural macros, our Rust executables will often link
-  # with C code, so include all the libraries that C code needs.  This
-  # is safe; https://github.com/rust-lang/rust/pull/54675 says that
-  # passing -nodefaultlibs to the linker "was more ideological to
-  # start with than anything".
-  add_project_arguments(rustc_args +
-      ['--cfg', 'MESON', '-C', 'default-linker-libraries'],
-      native: false, language: 'rust')
-  add_project_arguments(rustc_args + ['--cfg', 'MESON'],
-      native: true, language: 'rust')
-endif
+config_host_h = configure_file(output: 'config-host.h', configuration: config_host_data)
+genh += config_host_h
 
 hxtool = find_program('scripts/hxtool')
 shaderinclude = find_program('scripts/shaderinclude.py')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 3be7b7e5ced..5df6b35bf88 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,4 +1,7 @@
-_qemu_api_cfg = []
+_qemu_api_cfg = run_command(rustc_args,
+  '--config-headers', config_host_h,
+  capture: true, check: true).stdout().strip().split()
+
 # _qemu_api_cfg += ['--cfg', 'feature="allocator"']
 if rustc.version().version_compare('>=1.77.0')
   _qemu_api_cfg += ['--cfg', 'has_offset_of']
-- 
2.47.1

[PULL 05/49] rust: build: restrict --cfg generation to only required symbols

[Paolo Bonzini]

Parse the Cargo.toml file, looking for the unexpected_cfgs
configuration.  When generating --cfg options from the
config-host.h file, only use those that are included in the
configuration.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build  |  2 +-
 scripts/rust/rustc_args.py | 61 ++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 5df6b35bf88..2ff6d2ce3d0 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,5 +1,5 @@
 _qemu_api_cfg = run_command(rustc_args,
-  '--config-headers', config_host_h,
+  '--config-headers', config_host_h, files('Cargo.toml'),
   capture: true, check: true).stdout().strip().split()
 
 # _qemu_api_cfg += ['--cfg', 'feature="allocator"']
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index e4cc9720e16..942dd2b2bab 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -26,30 +26,51 @@
 
 import argparse
 import logging
+from pathlib import Path
+from typing import Any, Iterable, Mapping, Optional, Set
 
-from typing import List
+try:
+    import tomllib
+except ImportError:
+    import tomli as tomllib
 
 
-def generate_cfg_flags(header: str) -> List[str]:
+class CargoTOML:
+    tomldata: Mapping[Any, Any]
+    check_cfg: Set[str]
+
+    def __init__(self, path: str):
+        with open(path, 'rb') as f:
+            self.tomldata = tomllib.load(f)
+
+        self.check_cfg = set(self.find_check_cfg())
+
+    def find_check_cfg(self) -> Iterable[str]:
+        toml_lints = self.lints
+        rust_lints = toml_lints.get("rust", {})
+        cfg_lint = rust_lints.get("unexpected_cfgs", {})
+        return cfg_lint.get("check-cfg", [])
+
+    @property
+    def lints(self) -> Mapping[Any, Any]:
+        return self.get_table("lints")
+
+    def get_table(self, key: str) -> Mapping[Any, Any]:
+        table = self.tomldata.get(key, {})
+
+        return table
+
+
+def generate_cfg_flags(header: str, cargo_toml: CargoTOML) -> Iterable[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:
+        name = cfg[0]
+        if f'cfg({name})' not in cargo_toml.check_cfg:
             continue
         if len(cfg) >= 2 and cfg[1] != "1":
             continue
@@ -59,7 +80,6 @@ def cfg_name(name: str) -> str:
 
 
 def main() -> None:
-    # pylint: disable=missing-function-docstring
     parser = argparse.ArgumentParser()
     parser.add_argument("-v", "--verbose", action="store_true")
     parser.add_argument(
@@ -71,12 +91,21 @@ def main() -> None:
         required=False,
         default=[],
     )
+    parser.add_argument(
+        metavar="TOML_FILE",
+        action="store",
+        dest="cargo_toml",
+        help="path to Cargo.toml file",
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
     logging.debug("args: %s", args)
+
+    cargo_toml = CargoTOML(args.cargo_toml)
+
     for header in args.config_headers:
-        for tok in generate_cfg_flags(header):
+        for tok in generate_cfg_flags(header, cargo_toml):
             print(tok)
 
 
-- 
2.47.1

[PULL 06/49] rust: build: generate lint flags from Cargo.toml

[Paolo Bonzini]

Cargo.toml makes it possible to describe the desired lint level settings
in a nice format.  We can extend this to Meson-built crates, by teaching
rustc_args.py to fetch lint and --check-cfg arguments from Cargo.toml.
--check-cfg arguments come from the unexpected_cfgs lint as well as crate
features

Start with qemu-api, since it already has a [lints.rust] table and
an invocation of rustc_args.py.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                |  3 +-
 rust/qemu-api/meson.build  |  4 +-
 scripts/rust/rustc_args.py | 83 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 2f1f2aece4d..1e1d8f5cd61 100644
--- a/meson.build
+++ b/meson.build
@@ -120,7 +120,8 @@ if have_rust
 endif
 
 if have_rust
-  rustc_args = find_program('scripts/rust/rustc_args.py')
+  rustc_args = [find_program('scripts/rust/rustc_args.py'),
+    '--rustc-version', rustc.version()]
   rustfmt = find_program('rustfmt', required: false)
 
   # Prohibit code that is forbidden in Rust 2024
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 2ff6d2ce3d0..1ed79672cc9 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,6 +1,6 @@
 _qemu_api_cfg = run_command(rustc_args,
-  '--config-headers', config_host_h, files('Cargo.toml'),
-  capture: true, check: true).stdout().strip().split()
+  '--config-headers', config_host_h, '--features', '--lints', files('Cargo.toml'),
+  capture: true, check: true).stdout().strip().splitlines()
 
 # _qemu_api_cfg += ['--cfg', 'feature="allocator"']
 if rustc.version().version_compare('>=1.77.0')
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 942dd2b2bab..9b9778a1cac 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -25,9 +25,10 @@
 """
 
 import argparse
+from dataclasses import dataclass
 import logging
 from pathlib import Path
-from typing import Any, Iterable, Mapping, Optional, Set
+from typing import Any, Iterable, List, Mapping, Optional, Set
 
 try:
     import tomllib
@@ -61,6 +62,45 @@ def get_table(self, key: str) -> Mapping[Any, Any]:
         return table
 
 
+@dataclass
+class LintFlag:
+    flags: List[str]
+    priority: int
+
+
+def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
+    """Converts Cargo.toml lints to rustc -A/-D/-F/-W flags."""
+
+    toml_lints = cargo_toml.lints
+
+    lint_list = []
+    for k, v in toml_lints.items():
+        prefix = "" if k == "rust" else k + "::"
+        for lint, data in v.items():
+            level = data if isinstance(data, str) else data["level"]
+            priority = 0 if isinstance(data, str) else data.get("priority", 0)
+            if level == "deny":
+                flag = "-D"
+            elif level == "allow":
+                flag = "-A"
+            elif level == "warn":
+                flag = "-W"
+            elif level == "forbid":
+                flag = "-F"
+            else:
+                raise Exception(f"invalid level {level} for {prefix}{lint}")
+
+            # This may change if QEMU ever invokes clippy-driver or rustdoc by
+            # hand.  For now, check the syntax but do not add non-rustc lints to
+            # the command line.
+            if k == "rust":
+                lint_list.append(LintFlag(flags=[flag, prefix + lint], priority=priority))
+
+    lint_list.sort(key=lambda x: x.priority)
+    for lint in lint_list:
+        yield from lint.flags
+
+
 def generate_cfg_flags(header: str, cargo_toml: CargoTOML) -> Iterable[str]:
     """Converts defines from config[..].h headers to rustc --cfg flags."""
 
@@ -97,13 +137,54 @@ def main() -> None:
         dest="cargo_toml",
         help="path to Cargo.toml file",
     )
+    parser.add_argument(
+        "--features",
+        action="store_true",
+        dest="features",
+        help="generate --check-cfg arguments for features",
+        required=False,
+        default=None,
+    )
+    parser.add_argument(
+        "--lints",
+        action="store_true",
+        dest="lints",
+        help="generate arguments from [lints] table",
+        required=False,
+        default=None,
+    )
+    parser.add_argument(
+        "--rustc-version",
+        metavar="VERSION",
+        dest="rustc_version",
+        action="store",
+        help="version of rustc",
+        required=False,
+        default="1.0.0",
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
     logging.debug("args: %s", args)
 
+    rustc_version = tuple((int(x) for x in args.rustc_version.split('.')[0:2]))
     cargo_toml = CargoTOML(args.cargo_toml)
 
+    if args.lints:
+        for tok in generate_lint_flags(cargo_toml):
+            print(tok)
+
+    if rustc_version >= (1, 80):
+        if args.lints:
+            for cfg in sorted(cargo_toml.check_cfg):
+                print("--check-cfg")
+                print(cfg)
+        if args.features:
+            for feature in cargo_toml.get_table("features"):
+                if feature != "default":
+                    print("--check-cfg")
+                    print(f'cfg(feature,values("{feature}"))')
+
     for header in args.config_headers:
         for tok in generate_cfg_flags(header, cargo_toml):
             print(tok)
-- 
2.47.1

[PULL 07/49] rust: cargo: store desired warning levels in workspace Cargo.toml

[Paolo Bonzini]

An extra benefit of workspaces is that they allow to place lint level
settings in a single Cargo.toml; the settings are then inherited by
packages in the workspace.

Correspondingly, teach rustc_args.py to get the unexpected_cfgs
configuration from the workspace Cargo.toml.

Note that it is still possible to allow or deny warnings per crate or
module, via the #![] attribute syntax.  The rust/qemu-api/src/bindings.rs
file is an example.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                     |  7 +++---
 rust/Cargo.toml                 |  8 +++++++
 rust/hw/char/pl011/Cargo.toml   |  3 +++
 rust/qemu-api-macros/Cargo.toml |  3 +++
 rust/qemu-api/Cargo.toml        |  5 ++---
 rust/qemu-api/meson.build       |  2 +-
 scripts/rust/rustc_args.py      | 38 +++++++++++++++++++++++++++------
 7 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/meson.build b/meson.build
index 1e1d8f5cd61..218ae441e38 100644
--- a/meson.build
+++ b/meson.build
@@ -121,11 +121,12 @@ endif
 
 if have_rust
   rustc_args = [find_program('scripts/rust/rustc_args.py'),
-    '--rustc-version', rustc.version()]
+    '--rustc-version', rustc.version(),
+    '--workspace', meson.project_source_root() / 'rust']
   rustfmt = find_program('rustfmt', required: false)
 
-  # Prohibit code that is forbidden in Rust 2024
-  rustc_lint_args = ['-D', 'unsafe_op_in_unsafe_fn']
+  rustc_lint_args = run_command(rustc_args, '--lints',
+     capture: true, check: true).stdout().strip().splitlines()
 
   # Occasionally, we may need to silence warnings and clippy lints that
   # were only introduced in newer Rust compiler versions.  Do not croak
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 0c94d5037da..4bb52bf0bd5 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -5,3 +5,11 @@ members = [
     "qemu-api",
     "hw/char/pl011",
 ]
+
+[workspace.lints.rust]
+unexpected_cfgs = { level = "deny", check-cfg = [
+    'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
+    'cfg(has_offset_of)'] }
+
+# Prohibit code that is forbidden in Rust 2024
+unsafe_op_in_unsafe_fn = "deny"
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index a373906b9fb..58f3e859f7e 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -21,3 +21,6 @@ bilge = { version = "0.2.0" }
 bilge-impl = { version = "0.2.0" }
 qemu_api = { path = "../../../qemu-api" }
 qemu_api_macros = { path = "../../../qemu-api-macros" }
+
+[lints]
+workspace = true
diff --git a/rust/qemu-api-macros/Cargo.toml b/rust/qemu-api-macros/Cargo.toml
index a8f7377106b..5a27b52ee6e 100644
--- a/rust/qemu-api-macros/Cargo.toml
+++ b/rust/qemu-api-macros/Cargo.toml
@@ -20,3 +20,6 @@ proc-macro = true
 proc-macro2 = "1"
 quote = "1"
 syn = { version = "2", features = ["extra-traits"] }
+
+[lints]
+workspace = true
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index cc716d75d46..669f288d1cb 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -23,6 +23,5 @@ version_check = "~0.9"
 default = []
 allocator = []
 
-[lints.rust]
-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
-                                                 'cfg(has_offset_of)'] }
+[lints]
+workspace = true
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 1ed79672cc9..d719c13f46d 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,5 +1,5 @@
 _qemu_api_cfg = run_command(rustc_args,
-  '--config-headers', config_host_h, '--features', '--lints', files('Cargo.toml'),
+  '--config-headers', config_host_h, '--features', files('Cargo.toml'),
   capture: true, check: true).stdout().strip().splitlines()
 
 # _qemu_api_cfg += ['--cfg', 'feature="allocator"']
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 9b9778a1cac..9df131a02bd 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -38,11 +38,21 @@
 
 class CargoTOML:
     tomldata: Mapping[Any, Any]
+    workspace_data: Mapping[Any, Any]
     check_cfg: Set[str]
 
-    def __init__(self, path: str):
-        with open(path, 'rb') as f:
-            self.tomldata = tomllib.load(f)
+    def __init__(self, path: Optional[str], workspace: Optional[str]):
+        if path is not None:
+            with open(path, 'rb') as f:
+                self.tomldata = tomllib.load(f)
+        else:
+            self.tomldata = {"lints": {"workspace": True}}
+
+        if workspace is not None:
+            with open(workspace, 'rb') as f:
+                self.workspace_data = tomllib.load(f)
+            if "workspace" not in self.workspace_data:
+                self.workspace_data["workspace"] = {}
 
         self.check_cfg = set(self.find_check_cfg())
 
@@ -54,10 +64,12 @@ def find_check_cfg(self) -> Iterable[str]:
 
     @property
     def lints(self) -> Mapping[Any, Any]:
-        return self.get_table("lints")
+        return self.get_table("lints", True)
 
-    def get_table(self, key: str) -> Mapping[Any, Any]:
+    def get_table(self, key: str, can_be_workspace: bool = False) -> Mapping[Any, Any]:
         table = self.tomldata.get(key, {})
+        if can_be_workspace and table.get("workspace", False) is True:
+            table = self.workspace_data["workspace"].get(key, {})
 
         return table
 
@@ -136,6 +148,16 @@ def main() -> None:
         action="store",
         dest="cargo_toml",
         help="path to Cargo.toml file",
+        nargs='?',
+    )
+    parser.add_argument(
+        "--workspace",
+        metavar="DIR",
+        action="store",
+        dest="workspace",
+        help="path to root of the workspace",
+        required=False,
+        default=None,
     )
     parser.add_argument(
         "--features",
@@ -168,7 +190,11 @@ def main() -> None:
     logging.debug("args: %s", args)
 
     rustc_version = tuple((int(x) for x in args.rustc_version.split('.')[0:2]))
-    cargo_toml = CargoTOML(args.cargo_toml)
+    if args.workspace:
+        workspace_cargo_toml = Path(args.workspace, "Cargo.toml").resolve()
+        cargo_toml = CargoTOML(args.cargo_toml, str(workspace_cargo_toml))
+    else:
+        cargo_toml = CargoTOML(args.cargo_toml, None)
 
     if args.lints:
         for tok in generate_lint_flags(cargo_toml):
-- 
2.47.1

[PULL 08/49] rust: build: move strict lints handling to rustc_args.py

[Paolo Bonzini]

Make Cargo use unknown_lints = "allow" as well.  This is more future
proof as we might add new lints to rust/Cargo.toml that are not supported
by older versions of rustc or clippy.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                | 12 ++++--------
 rust/Cargo.toml            |  6 ++++++
 scripts/rust/rustc_args.py | 19 ++++++++++++++++---
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/meson.build b/meson.build
index 218ae441e38..85f74854735 100644
--- a/meson.build
+++ b/meson.build
@@ -123,19 +123,15 @@ if have_rust
   rustc_args = [find_program('scripts/rust/rustc_args.py'),
     '--rustc-version', rustc.version(),
     '--workspace', meson.project_source_root() / 'rust']
+  if get_option('strict_rust_lints')
+    rustc_args += ['--strict-lints']
+  endif
+
   rustfmt = find_program('rustfmt', required: false)
 
   rustc_lint_args = run_command(rustc_args, '--lints',
      capture: true, check: true).stdout().strip().splitlines()
 
-  # Occasionally, we may need to silence warnings and clippy lints that
-  # were only introduced in newer Rust compiler versions.  Do not croak
-  # in that case; a CI job with rust_strict_lints == true ensures that
-  # we do not have misspelled allow() attributes.
-  if not get_option('strict_rust_lints')
-    rustc_lint_args += ['-A', 'unknown_lints']
-  endif
-
   # Apart from procedural macros, our Rust executables will often link
   # with C code, so include all the libraries that C code needs.  This
   # is safe; https://github.com/rust-lang/rust/pull/54675 says that
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 4bb52bf0bd5..358c517bc56 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -11,5 +11,11 @@ unexpected_cfgs = { level = "deny", check-cfg = [
     'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
     'cfg(has_offset_of)'] }
 
+# Occasionally, we may need to silence warnings and clippy lints that
+# were only introduced in newer Rust compiler versions.  Do not croak
+# in that case; a CI job with rust_strict_lints == true disables this
+# and ensures that we do not have misspelled allow() attributes.
+unknown_lints = "allow"
+
 # Prohibit code that is forbidden in Rust 2024
 unsafe_op_in_unsafe_fn = "deny"
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 9df131a02bd..5525b3886fa 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -35,6 +35,8 @@
 except ImportError:
     import tomli as tomllib
 
+STRICT_LINTS = {"unknown_lints", "warnings"}
+
 
 class CargoTOML:
     tomldata: Mapping[Any, Any]
@@ -80,7 +82,7 @@ class LintFlag:
     priority: int
 
 
-def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
+def generate_lint_flags(cargo_toml: CargoTOML, strict_lints: bool) -> Iterable[str]:
     """Converts Cargo.toml lints to rustc -A/-D/-F/-W flags."""
 
     toml_lints = cargo_toml.lints
@@ -105,9 +107,13 @@ def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
             # This may change if QEMU ever invokes clippy-driver or rustdoc by
             # hand.  For now, check the syntax but do not add non-rustc lints to
             # the command line.
-            if k == "rust":
+            if k == "rust" and not (strict_lints and lint in STRICT_LINTS):
                 lint_list.append(LintFlag(flags=[flag, prefix + lint], priority=priority))
 
+    if strict_lints:
+        for lint in STRICT_LINTS:
+            lint_list.append(LintFlag(flags=["-D", lint], priority=1000000))
+
     lint_list.sort(key=lambda x: x.priority)
     for lint in lint_list:
         yield from lint.flags
@@ -184,6 +190,13 @@ def main() -> None:
         required=False,
         default="1.0.0",
     )
+    parser.add_argument(
+        "--strict-lints",
+        action="store_true",
+        dest="strict_lints",
+        help="apply stricter checks (for nightly Rust)",
+        default=False,
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
@@ -197,7 +210,7 @@ def main() -> None:
         cargo_toml = CargoTOML(args.cargo_toml, None)
 
     if args.lints:
-        for tok in generate_lint_flags(cargo_toml):
+        for tok in generate_lint_flags(cargo_toml, args.strict_lints):
             print(tok)
 
     if rustc_version >= (1, 80):
-- 
2.47.1

[PULL 09/49] rust: fix a couple style issues from clippy

[Paolo Bonzini]

These are reported as clippy::semicolon_inside_block and clippy::as_ptr_cast_mut.

clippy::semicolon_inside_block can be configured not to lint single-line
blocks; just go with the default.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs     | 8 +++++---
 rust/hw/char/pl011/src/memory_ops.rs | 4 +++-
 rust/qemu-api/tests/tests.rs         | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 476cacc8449..317a9b3c5ad 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,7 +2,7 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use core::ptr::{addr_of, addr_of_mut, NonNull};
+use core::ptr::{addr_of_mut, NonNull};
 use std::{
     ffi::CStr,
     os::raw::{c_int, c_uchar, c_uint, c_void},
@@ -499,7 +499,9 @@ pub fn update(&self) {
         let flags = self.int_level & self.int_enabled;
         for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
             // SAFETY: self.interrupts have been initialized in init().
-            unsafe { qemu_set_irq(*irq, i32::from(flags & i != 0)) };
+            unsafe {
+                qemu_set_irq(*irq, i32::from(flags & i != 0));
+            }
         }
     }
 
@@ -601,7 +603,7 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
         let sysbus: *mut SysBusDevice = dev.cast::<SysBusDevice>();
 
         qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
-        sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error);
+        sysbus_realize_and_unref(sysbus, addr_of_mut!(error_fatal));
         sysbus_mmio_map(sysbus, 0, addr);
         sysbus_connect_irq(sysbus, 0, irq);
         dev
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
index 169d485a4d2..c4e8599ba43 100644
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ b/rust/hw/char/pl011/src/memory_ops.rs
@@ -33,7 +33,9 @@
             // SAFETY: self.char_backend is a valid CharBackend instance after it's been
             // initialized in realize().
             let cb_ptr = unsafe { core::ptr::addr_of_mut!(state.as_mut().char_backend) };
-            unsafe { qemu_chr_fe_accept_input(cb_ptr) };
+            unsafe {
+                qemu_chr_fe_accept_input(cb_ptr);
+            }
 
             val
         }
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 43a4827de12..925f5a3c77b 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -74,6 +74,6 @@ impl Class for DummyClass {
 
     unsafe {
         module_call_init(module_init_type::MODULE_INIT_QOM);
-        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()) as *mut _);
+        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast());
     }
 }
-- 
2.47.1

[PULL 10/49] rust: build: establish a baseline of lints across all crates

[Paolo Bonzini]

Many lints that default to allow can be helpful in detecting bugs or
keeping the code style homogeneous.  Add them liberally, though perhaps
not as liberally as in hw/char/pl011/src/lib.rs.  In particular, enabling
entire groups can be problematic because of bitrot when new links are
added in the future.

For Clippy, this is actually a feature that is only present in Cargo
1.74.0 but, since we are not using Cargo to *build* QEMU, only developers
will need a new-enough cargo and only to run tools such as clippy.
The requirement does not apply to distros that are building QEMU.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.toml               | 68 +++++++++++++++++++++++++++++++++++
 rust/hw/char/pl011/src/lib.rs | 19 ++--------
 rust/qemu-api/src/bindings.rs |  6 ++--
 3 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 358c517bc56..6ec19b67297 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -19,3 +19,71 @@ unknown_lints = "allow"
 
 # Prohibit code that is forbidden in Rust 2024
 unsafe_op_in_unsafe_fn = "deny"
+
+[workspace.lints.rustdoc]
+private_intra_doc_links = "allow"
+
+broken_intra_doc_links = "deny"
+invalid_html_tags = "deny"
+invalid_rust_codeblocks = "deny"
+bare_urls = "deny"
+unescaped_backticks = "deny"
+redundant_explicit_links = "deny"
+
+[workspace.lints.clippy]
+# default-warn lints
+result_unit_err = "allow"
+should_implement_trait = "deny"
+# can be for a reason, e.g. in callbacks
+unused_self = "allow"
+
+# default-allow lints
+as_underscore = "deny"
+assertions_on_result_states = "deny"
+bool_to_int_with_if = "deny"
+borrow_as_ptr = "deny"
+cast_lossless = "deny"
+dbg_macro = "deny"
+debug_assert_with_mut_call = "deny"
+derive_partial_eq_without_eq = "deny"
+doc_markdown = "deny"
+empty_structs_with_brackets = "deny"
+ignored_unit_patterns = "deny"
+implicit_clone = "deny"
+macro_use_imports = "deny"
+missing_const_for_fn = "deny"
+missing_safety_doc = "deny"
+multiple_crate_versions = "deny"
+mut_mut = "deny"
+needless_bitwise_bool = "deny"
+needless_pass_by_ref_mut = "deny"
+no_effect_underscore_binding = "deny"
+option_option = "deny"
+or_fun_call = "deny"
+ptr_as_ptr = "deny"
+pub_underscore_fields = "deny"
+redundant_clone = "deny"
+redundant_closure_for_method_calls = "deny"
+redundant_else = "deny"
+redundant_pub_crate = "deny"
+ref_binding_to_reference = "deny"
+ref_option_ref = "deny"
+return_self_not_must_use = "deny"
+same_name_method = "deny"
+semicolon_inside_block = "deny"
+shadow_unrelated = "deny"
+significant_drop_in_scrutinee = "deny"
+significant_drop_tightening = "deny"
+suspicious_operation_groupings = "deny"
+transmute_ptr_to_ptr = "deny"
+transmute_undefined_repr = "deny"
+type_repetition_in_bounds = "deny"
+used_underscore_binding = "deny"
+
+# nice to have, but cannot be enabled yet
+#wildcard_imports = "deny"   # still have many bindings::* imports
+#ptr_cast_constness = "deny" # needs 1.65.0 for cast_mut()/cast_const()
+
+# these may have false positives
+#option_if_let_else = "deny"
+cognitive_complexity = "deny"
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index cd0a49acb91..4dc0e8f345f 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -14,28 +14,15 @@
 //! the [`registers`] module for register types.
 
 #![deny(
-    rustdoc::broken_intra_doc_links,
-    rustdoc::redundant_explicit_links,
     clippy::correctness,
     clippy::suspicious,
     clippy::complexity,
     clippy::perf,
     clippy::cargo,
     clippy::nursery,
-    clippy::style,
-    // restriction group
-    clippy::dbg_macro,
-    clippy::as_underscore,
-    clippy::assertions_on_result_states,
-    // pedantic group
-    clippy::doc_markdown,
-    clippy::borrow_as_ptr,
-    clippy::cast_lossless,
-    clippy::option_if_let_else,
-    clippy::missing_const_for_fn,
-    clippy::cognitive_complexity,
-    clippy::missing_safety_doc,
-    )]
+    clippy::style
+)]
+#![allow(clippy::upper_case_acronyms)]
 #![allow(clippy::result_unit_err)]
 
 extern crate bilge;
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 0b76ec58bee..8a9b821bb91 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -7,10 +7,10 @@
     non_snake_case,
     non_upper_case_globals,
     unsafe_op_in_unsafe_fn,
+    clippy::pedantic,
+    clippy::restriction,
+    clippy::style,
     clippy::missing_const_for_fn,
-    clippy::too_many_arguments,
-    clippy::approx_constant,
-    clippy::use_self,
     clippy::useless_transmute,
     clippy::missing_safety_doc
 )]
-- 
2.47.1

[PULL 11/49] rust: build: add "make clippy", "make rustfmt", "make rustdoc"

[Paolo Bonzini]

Abstract common invocations of "cargo", that do not require copying
the generated bindgen file or setting up MESON_BUILD_ROOT.

In the future these could also do completely without cargo and invoke
the underlying programs directly.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/meson.build        | 22 ++++++++++++++++++++++
 rust/qemu-api/README.md | 10 ++++++----
 rust/qemu-api/build.rs  |  9 +++++++--
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/rust/meson.build b/rust/meson.build
index def77389cdd..91e52b8fb8e 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -2,3 +2,25 @@ subdir('qemu-api-macros')
 subdir('qemu-api')
 
 subdir('hw')
+
+cargo = find_program('cargo', required: false)
+
+if cargo.found()
+  run_target('clippy',
+    command: [config_host['MESON'], 'devenv',
+              '--workdir', '@CURRENT_SOURCE_DIR@',
+              cargo, 'clippy', '--tests'],
+    depends: bindings_rs)
+
+  run_target('rustfmt',
+    command: [config_host['MESON'], 'devenv',
+              '--workdir', '@CURRENT_SOURCE_DIR@',
+              cargo, 'fmt'],
+    depends: bindings_rs)
+
+  run_target('rustdoc',
+    command: [config_host['MESON'], 'devenv',
+              '--workdir', '@CURRENT_SOURCE_DIR@',
+              cargo, 'doc', '--no-deps', '--document-private-items'],
+    depends: bindings_rs)
+endif
diff --git a/rust/qemu-api/README.md b/rust/qemu-api/README.md
index 53810f48882..ed1b7ab263d 100644
--- a/rust/qemu-api/README.md
+++ b/rust/qemu-api/README.md
@@ -5,13 +5,15 @@ This library exports helper Rust types, Rust macros and C FFI bindings for inter
 The C bindings can be generated with `bindgen`, using this build target:
 
 ```console
-$ ninja bindings.inc.rs
+$ make bindings.inc.rs
 ```
 
 ## Generate Rust documentation
 
-To generate docs for this crate, including private items:
+Common Cargo tasks can be performed from the QEMU build directory
 
-```sh
-pyvenv/bin/meson devenv -w ../rust cargo doc --no-deps --document-private-items
+```console
+$ make clippy
+$ make rustfmt
+$ make rustdoc
 ```
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
index 40644d5e4fe..471e6c633df 100644
--- a/rust/qemu-api/build.rs
+++ b/rust/qemu-api/build.rs
@@ -20,8 +20,13 @@ fn main() -> Result<()> {
     let file = Path::new(&file);
     if !Path::new(&file).exists() {
         panic!(concat!(
-            "No generated C bindings found! If you want to run `cargo`, start a subshell\n",
-            "with `meson devenv`, or point MESON_BUILD_ROOT to the top of the build tree."
+            "\n",
+            "    No generated C bindings found! Maybe you wanted one of\n",
+            "    `make clippy`, `make rustfmt`, `make rustdoc`?\n",
+            "\n",
+            "    For other uses of `cargo`, start a subshell with\n",
+            "    `pyvenv/bin/meson devenv`, or point MESON_BUILD_ROOT to\n",
+            "    the top of the build tree."
         ));
     }
 
-- 
2.47.1

[PULL 12/49] rust: ci: add job that runs Rust tools

[Paolo Bonzini]

Code checks, as well as documentation generation, are not yet tied
to "make check" because they need new version of the Rust toolchain
(even nightly in the case of "rustfmt").  Run them in CI using the
existing nightly-Rust container.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.d/static_checks.yml                | 24 +++++++++++++++++++
 .../dockerfiles/fedora-rust-nightly.docker    |  4 ++++
 tests/lcitool/refresh                         |  4 ++++
 3 files changed, 32 insertions(+)

diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml
index ad9f426a52f..c0ba4533826 100644
--- a/.gitlab-ci.d/static_checks.yml
+++ b/.gitlab-ci.d/static_checks.yml
@@ -46,3 +46,27 @@ check-python-tox:
     QEMU_JOB_OPTIONAL: 1
   needs:
     job: python-container
+
+check-rust-tools-nightly:
+  extends: .base_job_template
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/fedora-rust-nightly:$QEMU_CI_CONTAINER_TAG
+  script:
+    - source scripts/ci/gitlab-ci-section
+    - section_start test "Running Rust code checks"
+    - cd build
+    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} fmt --check
+    - make clippy
+    - make rustdoc
+    - section_end test
+  variables:
+    GIT_DEPTH: 1
+  allow_failure: true
+  needs:
+    - job: build-system-fedora-rust-nightly
+      artifacts: true
+  artifacts:
+    when: on_success
+    expire_in: 2 days
+    paths:
+      - rust/target/doc
diff --git a/tests/docker/dockerfiles/fedora-rust-nightly.docker b/tests/docker/dockerfiles/fedora-rust-nightly.docker
index 9180c8b5222..a8e4fb279a7 100644
--- a/tests/docker/dockerfiles/fedora-rust-nightly.docker
+++ b/tests/docker/dockerfiles/fedora-rust-nightly.docker
@@ -155,6 +155,7 @@ ENV PYTHON "/usr/bin/python3"
 RUN dnf install -y wget
 ENV RUSTUP_HOME=/usr/local/rustup CARGO_HOME=/usr/local/cargo
 ENV RUSTC=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc
+ENV CARGO=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo
 RUN set -eux && \
   rustArch='x86_64-unknown-linux-gnu' && \
   rustupSha256='6aeece6993e902708983b209d04c0d1dbb14ebb405ddb87def578d41f920f56d' && \
@@ -165,10 +166,13 @@ RUN set -eux && \
   ./rustup-init -y --no-modify-path --profile default --default-toolchain nightly --default-host ${rustArch} && \
   chmod -R a+w $RUSTUP_HOME $CARGO_HOME && \
   /usr/local/cargo/bin/rustup --version && \
+  /usr/local/cargo/bin/rustup run nightly cargo --version && \
   /usr/local/cargo/bin/rustup run nightly rustc --version && \
+  test "$CARGO" = "$(/usr/local/cargo/bin/rustup +nightly which cargo)" && \
   test "$RUSTC" = "$(/usr/local/cargo/bin/rustup +nightly which rustc)"
 ENV PATH=$CARGO_HOME/bin:$PATH
 RUN /usr/local/cargo/bin/rustup run nightly cargo install bindgen-cli
+RUN $CARGO --list
 # As a final step configure the user (if env is defined)
 ARG USER
 ARG UID
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index 51012783c0f..6720516b942 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -121,6 +121,7 @@ fedora_rustup_nightly_extras = [
     "RUN dnf install -y wget\n",
     "ENV RUSTUP_HOME=/usr/local/rustup CARGO_HOME=/usr/local/cargo\n",
     "ENV RUSTC=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc\n",
+    "ENV CARGO=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo\n",
     "RUN set -eux && \\\n",
     "  rustArch='x86_64-unknown-linux-gnu' && \\\n",
     "  rustupSha256='6aeece6993e902708983b209d04c0d1dbb14ebb405ddb87def578d41f920f56d' && \\\n",
@@ -131,10 +132,13 @@ fedora_rustup_nightly_extras = [
     "  ./rustup-init -y --no-modify-path --profile default --default-toolchain nightly --default-host ${rustArch} && \\\n",
     "  chmod -R a+w $RUSTUP_HOME $CARGO_HOME && \\\n",
     "  /usr/local/cargo/bin/rustup --version && \\\n",
+    "  /usr/local/cargo/bin/rustup run nightly cargo --version && \\\n",
     "  /usr/local/cargo/bin/rustup run nightly rustc --version && \\\n",
+    '  test "$CARGO" = "$(/usr/local/cargo/bin/rustup +nightly which cargo)" && \\\n',
     '  test "$RUSTC" = "$(/usr/local/cargo/bin/rustup +nightly which rustc)"\n',
     'ENV PATH=$CARGO_HOME/bin:$PATH\n',
     'RUN /usr/local/cargo/bin/rustup run nightly cargo install bindgen-cli\n',
+    'RUN $CARGO --list\n',
 ]
 
 ubuntu2204_bindgen_extras = [
-- 
2.47.1

[PULL 13/49] rust: fix doc test syntax

[Paolo Bonzini]

Allow "cargo test --doc" to pass.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/zeroable.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 13cdb2ccba5..6125aeed8b4 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -7,9 +7,9 @@
 /// behavior.  This trait in principle could be implemented as just:
 ///
 /// ```
-///     const ZERO: Self = unsafe {
-///         ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init()
-///     },
+/// pub unsafe trait Zeroable: Default {
+///     const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() };
+/// }
 /// ```
 ///
 /// The need for a manual implementation is only because `zeroed()` cannot
-- 
2.47.1

[PULL 14/49] rust/qemu-api: Fix fragment-specifiers in define_property macro

[Paolo Bonzini]

From: Junjie Mao <junjie.mao@hotmail.com>

For the matcher of macro, "expr" is used for expressions, while "ident"
is used for variable/function names, and "ty" matches types.

In define_property macro, $field is a member name of type $state, so it
should be defined as "ident", though offset_of! doesn't complain about
this. $type is the type of $field, since it is not used in the macro, so
that no type mismatch error is triggered either.

Fix fragment-specifiers of $field and $type.

Signed-off-by: Junjie Mao <junjie.mao@hotmail.com>
Co-developed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20241017143245.1248589-2-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/device_class.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 0ba798d3e3c..922bbce1bb8 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -27,7 +27,7 @@ macro_rules! device_class_init {
 
 #[macro_export]
 macro_rules! define_property {
-    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => {
         $crate::bindings::Property {
             // use associated function syntax for type checking
             name: ::std::ffi::CStr::as_ptr($name),
@@ -38,7 +38,7 @@ macro_rules! define_property {
             ..$crate::zeroable::Zeroable::ZERO
         }
     };
-    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty$(,)*) => {
         $crate::bindings::Property {
             // use associated function syntax for type checking
             name: ::std::ffi::CStr::as_ptr($name),
-- 
2.47.1

[PULL 15/49] clock: clear callback on unparent

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/clock.h   |  8 --------
 hw/core/clock.c      | 22 +++++++++++++++++-----
 hw/core/qdev-clock.c |  5 +----
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/hw/clock.h b/include/hw/clock.h
index eb58599131c..a279bd4ba5e 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -141,14 +141,6 @@ Clock *clock_new(Object *parent, const char *name);
 void clock_set_callback(Clock *clk, ClockCallback *cb,
                         void *opaque, unsigned int events);
 
-/**
- * clock_clear_callback:
- * @clk: the clock to delete the callback from
- *
- * Unregister the callback registered with clock_set_callback.
- */
-void clock_clear_callback(Clock *clk);
-
 /**
  * clock_set_source:
  * @clk: the clock.
diff --git a/hw/core/clock.c b/hw/core/clock.c
index cbe7b1bc469..391095eb4e5 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -44,16 +44,12 @@ Clock *clock_new(Object *parent, const char *name)
 void clock_set_callback(Clock *clk, ClockCallback *cb, void *opaque,
                         unsigned int events)
 {
+    assert(OBJECT(clk)->parent);
     clk->callback = cb;
     clk->callback_opaque = opaque;
     clk->callback_events = events;
 }
 
-void clock_clear_callback(Clock *clk)
-{
-    clock_set_callback(clk, NULL, NULL, 0);
-}
-
 bool clock_set(Clock *clk, uint64_t period)
 {
     if (clk->period == period) {
@@ -168,6 +164,16 @@ static void clock_period_prop_get(Object *obj, Visitor *v, const char *name,
     visit_type_uint64(v, name, &period, errp);
 }
 
+static void clock_unparent(Object *obj)
+{
+    /*
+     * Callback are registered by the parent, which might die anytime after
+     * it's unparented the children.  Avoid having a callback to a deleted
+     * object in case the clock is still referenced somewhere else (eg: by
+     * a clock output).
+     */
+    clock_set_callback(CLOCK(obj), NULL, NULL, 0);
+}
 
 static void clock_initfn(Object *obj)
 {
@@ -200,11 +206,17 @@ static void clock_finalizefn(Object *obj)
     g_free(clk->canonical_path);
 }
 
+static void clock_class_init(ObjectClass *klass, void *data)
+{
+    klass->unparent = clock_unparent;
+}
+
 static const TypeInfo clock_info = {
     .name              = TYPE_CLOCK,
     .parent            = TYPE_OBJECT,
     .instance_size     = sizeof(Clock),
     .instance_init     = clock_initfn,
+    .class_init        = clock_class_init,
     .instance_finalize = clock_finalizefn,
 };
 
diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 82799577f3e..ca65685c04e 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -87,11 +87,8 @@ void qdev_finalize_clocklist(DeviceState *dev)
         if (!ncl->output && !ncl->alias) {
             /*
              * We kept a reference on the input clock to ensure it lives up to
-             * this point so we can safely remove the callback.
-             * It avoids having a callback to a deleted object if ncl->clock
-             * is still referenced somewhere else (eg: by a clock output).
+             * this point; it is used by the monitor to show the frequency.
              */
-            clock_clear_callback(ncl->clock);
             object_unref(OBJECT(ncl->clock));
         }
         g_free(ncl->name);
-- 
2.47.1

[PULL 16/49] clock: treat outputs and inputs the same in NamedClockList

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-clock.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index ca65685c04e..2f9d6cb7579 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -48,14 +48,6 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
     if (clk == NULL) {
         clk = CLOCK(object_new(TYPE_CLOCK));
         object_property_add_child(OBJECT(dev), name, OBJECT(clk));
-        if (output) {
-            /*
-             * Remove object_new()'s initial reference.
-             * Note that for inputs, the reference created by object_new()
-             * will be deleted in qdev_finalize_clocklist().
-             */
-            object_unref(OBJECT(clk));
-        }
     } else {
         object_property_add_link(OBJECT(dev), name,
                                  object_get_typename(OBJECT(clk)),
@@ -84,7 +76,7 @@ void qdev_finalize_clocklist(DeviceState *dev)
 
     QLIST_FOREACH_SAFE(ncl, &dev->clocks, node, ncl_next) {
         QLIST_REMOVE(ncl, node);
-        if (!ncl->output && !ncl->alias) {
+        if (!ncl->alias) {
             /*
              * We kept a reference on the input clock to ensure it lives up to
              * this point; it is used by the monitor to show the frequency.
-- 
2.47.1

[PULL 17/49] clock: inline most of qdev_init_clocklist

[Paolo Bonzini]

Move object creation out of qdev_init_clocklist.  The input/output
cases are very simple, and the aliases are completely different.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-clock.c | 71 +++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 44 deletions(-)

diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 2f9d6cb7579..dacafa4e036 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -22,7 +22,7 @@
  * Add a new clock in a device
  */
 static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
-                                           bool output, Clock *clk)
+                                           bool alias, bool output, Clock *clk)
 {
     NamedClockList *ncl;
 
@@ -38,31 +38,8 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
      */
     ncl = g_new0(NamedClockList, 1);
     ncl->name = g_strdup(name);
+    ncl->alias = alias;
     ncl->output = output;
-    ncl->alias = (clk != NULL);
-
-    /*
-     * Trying to create a clock whose name clashes with some other
-     * clock or property is a bug in the caller and we will abort().
-     */
-    if (clk == NULL) {
-        clk = CLOCK(object_new(TYPE_CLOCK));
-        object_property_add_child(OBJECT(dev), name, OBJECT(clk));
-    } else {
-        object_property_add_link(OBJECT(dev), name,
-                                 object_get_typename(OBJECT(clk)),
-                                 (Object **) &ncl->clock,
-                                 NULL, OBJ_PROP_LINK_STRONG);
-        /*
-         * Since the link property has the OBJ_PROP_LINK_STRONG flag, the clk
-         * object reference count gets decremented on property deletion.
-         * However object_property_add_link does not increment it since it
-         * doesn't know the linked object. Increment it here to ensure the
-         * aliased clock stays alive during this device life-time.
-         */
-        object_ref(OBJECT(clk));
-    }
-
     ncl->clock = clk;
 
     QLIST_INSERT_HEAD(&dev->clocks, ncl, node);
@@ -90,29 +67,25 @@ void qdev_finalize_clocklist(DeviceState *dev)
 
 Clock *qdev_init_clock_out(DeviceState *dev, const char *name)
 {
-    NamedClockList *ncl;
+    Clock *clk = CLOCK(object_new(TYPE_CLOCK));
+    object_property_add_child(OBJECT(dev), name, OBJECT(clk));
 
-    assert(name);
-
-    ncl = qdev_init_clocklist(dev, name, true, NULL);
-
-    return ncl->clock;
+    qdev_init_clocklist(dev, name, false, true, clk);
+    return clk;
 }
 
 Clock *qdev_init_clock_in(DeviceState *dev, const char *name,
                           ClockCallback *callback, void *opaque,
                           unsigned int events)
 {
-    NamedClockList *ncl;
-
-    assert(name);
-
-    ncl = qdev_init_clocklist(dev, name, false, NULL);
+    Clock *clk = CLOCK(object_new(TYPE_CLOCK));
+    object_property_add_child(OBJECT(dev), name, OBJECT(clk));
 
+    qdev_init_clocklist(dev, name, false, false, clk);
     if (callback) {
-        clock_set_callback(ncl->clock, callback, opaque, events);
+        clock_set_callback(clk, callback, opaque, events);
     }
-    return ncl->clock;
+    return clk;
 }
 
 void qdev_init_clocks(DeviceState *dev, const ClockPortInitArray clocks)
@@ -183,15 +156,25 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char *name)
 Clock *qdev_alias_clock(DeviceState *dev, const char *name,
                         DeviceState *alias_dev, const char *alias_name)
 {
-    NamedClockList *ncl;
+    NamedClockList *ncl = qdev_get_clocklist(dev, name);
+    Clock *clk = ncl->clock;
 
-    assert(name && alias_name);
+    ncl = qdev_init_clocklist(alias_dev, alias_name, true, ncl->output, clk);
 
-    ncl = qdev_get_clocklist(dev, name);
+    object_property_add_link(OBJECT(alias_dev), alias_name,
+                             TYPE_CLOCK,
+                             (Object **) &ncl->clock,
+                             NULL, OBJ_PROP_LINK_STRONG);
+    /*
+     * Since the link property has the OBJ_PROP_LINK_STRONG flag, the clk
+     * object reference count gets decremented on property deletion.
+     * However object_property_add_link does not increment it since it
+     * doesn't know the linked object. Increment it here to ensure the
+     * aliased clock stays alive during this device life-time.
+     */
+    object_ref(OBJECT(clk));
 
-    qdev_init_clocklist(alias_dev, alias_name, ncl->output, ncl->clock);
-
-    return ncl->clock;
+    return clk;
 }
 
 void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
-- 
2.47.1

[PULL 18/49] kvm: remove unnecessary #ifdef

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm/kvm_i386.h | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h
index 9de9c0d3038..7edb154a16e 100644
--- a/target/i386/kvm/kvm_i386.h
+++ b/target/i386/kvm/kvm_i386.h
@@ -13,8 +13,7 @@
 
 #include "sysemu/kvm.h"
 
-#ifdef CONFIG_KVM
-
+/* always false if !CONFIG_KVM */
 #define kvm_pit_in_kernel() \
     (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
 #define kvm_pic_in_kernel()  \
@@ -22,14 +21,6 @@
 #define kvm_ioapic_in_kernel() \
     (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
 
-#else
-
-#define kvm_pit_in_kernel()      0
-#define kvm_pic_in_kernel()      0
-#define kvm_ioapic_in_kernel()   0
-
-#endif  /* CONFIG_KVM */
-
 bool kvm_has_smm(void);
 bool kvm_enable_x2apic(void);
 bool kvm_hv_vpindex_settable(void);
-- 
2.47.1

[PULL 19/49] arm: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-2-zhao1.liu@intel.com
---
 hw/arm/armsse.c    | 2 +-
 hw/arm/smmuv3.c    | 4 ++--
 target/arm/cpu.c   | 2 +-
 target/arm/cpu64.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 255346a595a..58ed504b2bc 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1731,7 +1731,7 @@ static void armsse_register_types(void)
             .class_init = armsse_class_init,
             .class_data = (void *)&armsse_variants[i],
         };
-        type_register(&ti);
+        type_register_static(&ti);
     }
 }
 
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 4c49b5a885f..6e847e8773d 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -2065,8 +2065,8 @@ static const TypeInfo smmuv3_iommu_memory_region_info = {
 
 static void smmuv3_register_types(void)
 {
-    type_register(&smmuv3_type_info);
-    type_register(&smmuv3_iommu_memory_region_info);
+    type_register_static(&smmuv3_type_info);
+    type_register_static(&smmuv3_iommu_memory_region_info);
 }
 
 type_init(smmuv3_register_types)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6938161b954..17519b5e468 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2755,7 +2755,7 @@ void arm_cpu_register(const ARMCPUInfo *info)
     };
 
     type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
-    type_register(&type_info);
+    type_register_static(&type_info);
     g_free((void *)type_info.name);
 }
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 458d1cee012..c1cac912a08 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -841,7 +841,7 @@ void aarch64_cpu_register(const ARMCPUInfo *info)
     };
 
     type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
-    type_register(&type_info);
+    type_register_static(&type_info);
     g_free((void *)type_info.name);
 }
 
-- 
2.47.1

[PULL 20/49] hw/block: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-3-zhao1.liu@intel.com
---
 hw/block/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index e2e84f8b5f8..748594524e3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1894,7 +1894,7 @@ static void m25p80_register_types(void)
             .class_init = m25p80_class_init,
             .class_data = (void *)&known_devices[i],
         };
-        type_register(&ti);
+        type_register_static(&ti);
     }
 }
 
-- 
2.47.1

[PULL 21/49] hw/net: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-4-zhao1.liu@intel.com
---
 hw/net/e1000.c    | 2 +-
 hw/net/eepro100.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 5012b964640..ab72236d183 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1774,7 +1774,7 @@ static void e1000_register_types(void)
         type_info.class_data = (void *)info;
         type_info.class_init = e1000_class_init;
 
-        type_register(&type_info);
+        type_register_static(&type_info);
     }
 }
 
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index c8a88b9813f..20b22d8e49c 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -2102,7 +2102,7 @@ static void eepro100_register_types(void)
             { },
         };
 
-        type_register(&type_info);
+        type_register_static(&type_info);
     }
 }
 
-- 
2.47.1

[PULL 22/49] ppc: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-5-zhao1.liu@intel.com
---
 hw/ppc/spapr.c   | 2 +-
 target/ppc/kvm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0d4efaa0c09..827e7d6b140 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4723,7 +4723,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc)
     static void MACHINE_VER_SYM(register, spapr, __VA_ARGS__)(void)  \
     {                                                                \
         MACHINE_VER_DELETION(__VA_ARGS__);                           \
-        type_register(&MACHINE_VER_SYM(info, spapr, __VA_ARGS__));   \
+        type_register_static(&MACHINE_VER_SYM(info, spapr, __VA_ARGS__));   \
     }                                                                \
     type_init(MACHINE_VER_SYM(register, spapr, __VA_ARGS__))
 
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 3efc28f18b3..0d464824dbc 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2633,7 +2633,7 @@ static int kvm_ppc_register_host_cpu_type(void)
         return -1;
     }
     type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
-    type_register(&type_info);
+    type_register_static(&type_info);
     /* override TCG default cpu type with 'host' cpu model */
     object_class_foreach(pseries_machine_class_fixup, TYPE_SPAPR_MACHINE,
                          false, NULL);
-- 
2.47.1

[PULL 23/49] hw/rtc: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-6-zhao1.liu@intel.com
---
 hw/rtc/m48t59-isa.c | 2 +-
 hw/rtc/m48t59.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c
index 6e9723fdf19..b642b82680d 100644
--- a/hw/rtc/m48t59-isa.c
+++ b/hw/rtc/m48t59-isa.c
@@ -161,7 +161,7 @@ static void m48t59_isa_register_types(void)
     for (i = 0; i < ARRAY_SIZE(m48txx_isa_info); i++) {
         isa_type_info.name = m48txx_isa_info[i].bus_name;
         isa_type_info.class_data = &m48txx_isa_info[i];
-        type_register(&isa_type_info);
+        type_register_static(&isa_type_info);
     }
 }
 
diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index 48846d8df40..90299ea56fe 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -679,7 +679,7 @@ static void m48t59_register_types(void)
     for (i = 0; i < ARRAY_SIZE(m48txx_sysbus_info); i++) {
         sysbus_type_info.name = m48txx_sysbus_info[i].bus_name;
         sysbus_type_info.class_data = &m48txx_sysbus_info[i];
-        type_register(&sysbus_type_info);
+        type_register_static(&sysbus_type_info);
     }
 }
 
-- 
2.47.1

[PULL 24/49] hw/scsi: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because\
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-7-zhao1.liu@intel.com
---
 hw/scsi/megasas.c | 2 +-
 hw/scsi/mptsas.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index b33229d71a4..df58aeb9952 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2576,7 +2576,7 @@ static void megasas_register_types(void)
         type_info.class_init = megasas_class_init;
         type_info.interfaces = info->interfaces;
 
-        type_register(&type_info);
+        type_register_static(&type_info);
     }
 }
 
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 361b75e633a..c6bc3479e99 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1450,7 +1450,7 @@ static const TypeInfo mptsas_info = {
 
 static void mptsas_register_types(void)
 {
-    type_register(&mptsas_info);
+    type_register_static(&mptsas_info);
 }
 
 type_init(mptsas_register_types)
-- 
2.47.1

[PULL 25/49] hw/sensor: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-8-zhao1.liu@intel.com
---
 hw/sensor/tmp421.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
index b6f0b62ab11..82e604279c5 100644
--- a/hw/sensor/tmp421.c
+++ b/hw/sensor/tmp421.c
@@ -384,7 +384,7 @@ static void tmp421_register_types(void)
             .class_init = tmp421_class_init,
             .class_data = (void *) &devices[i],
         };
-        type_register(&ti);
+        type_register_static(&ti);
     }
 }
 
-- 
2.47.1

[PULL 26/49] hw/usb: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-9-zhao1.liu@intel.com
---
 hw/usb/hcd-ehci-pci.c | 2 +-
 hw/usb/hcd-uhci.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index c94fc9f6c58..dd06451e234 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -228,7 +228,7 @@ static void ehci_pci_register_types(void)
     for (i = 0; i < ARRAY_SIZE(ehci_pci_info); i++) {
         ehci_type_info.name = ehci_pci_info[i].name;
         ehci_type_info.class_data = ehci_pci_info + i;
-        type_register(&ehci_type_info);
+        type_register_static(&ehci_type_info);
     }
 }
 
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 3d0339af7b9..65c1f93cc9f 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1362,7 +1362,7 @@ static void uhci_register_types(void)
     for (i = 0; i < ARRAY_SIZE(uhci_info); i++) {
         uhci_type_info.name = uhci_info[i].name;
         uhci_type_info.class_data = uhci_info + i;
-        type_register(&uhci_type_info);
+        type_register_static(&uhci_type_info);
     }
 }
 
-- 
2.47.1

[PULL 27/49] hw/virtio: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-10-zhao1.liu@intel.com
---
 hw/virtio/virtio-pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 5a394821da9..5c6c2019cef 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2511,9 +2511,9 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t)
         base_type_info.class_data = (void *)t;
     }
 
-    type_register(&base_type_info);
+    type_register_static(&base_type_info);
     if (generic_type_info.name) {
-        type_register(&generic_type_info);
+        type_register_static(&generic_type_info);
     }
 
     if (t->non_transitional_name) {
@@ -2527,7 +2527,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t)
                 { }
             },
         };
-        type_register(&non_transitional_type_info);
+        type_register_static(&non_transitional_type_info);
     }
 
     if (t->transitional_name) {
@@ -2544,7 +2544,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t)
                 { }
             },
         };
-        type_register(&transitional_type_info);
+        type_register_static(&transitional_type_info);
     }
     g_free(base_name);
 }
-- 
2.47.1

[PULL 28/49] i386: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-11-zhao1.liu@intel.com
---
 include/hw/i386/pc.h | 4 ++--
 target/i386/cpu.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 14ee06287da..b3477ad6a29 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -316,7 +316,7 @@ extern const size_t pc_compat_2_3_len;
     }; \
     static void pc_machine_init_##suffix(void) \
     { \
-        type_register(&pc_machine_type_##suffix); \
+        type_register_static(&pc_machine_type_##suffix); \
     } \
     type_init(pc_machine_init_##suffix)
 
@@ -344,7 +344,7 @@ extern const size_t pc_compat_2_3_len;
     static void MACHINE_VER_SYM(register, namesym, __VA_ARGS__)(void) \
     { \
         MACHINE_VER_DELETION(__VA_ARGS__); \
-        type_register(&MACHINE_VER_SYM(info, namesym, __VA_ARGS__)); \
+        type_register_static(&MACHINE_VER_SYM(info, namesym, __VA_ARGS__)); \
     } \
     type_init(MACHINE_VER_SYM(register, namesym, __VA_ARGS__));
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3725dbbc4b3..305f2a41cfb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6429,7 +6429,7 @@ static void x86_register_cpu_model_type(const char *name, X86CPUModel *model)
         .class_data = model,
     };
 
-    type_register(&ti);
+    type_register_static(&ti);
 }
 
 
-- 
2.47.1

[PULL 29/49] target/mips: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-12-zhao1.liu@intel.com
---
 target/mips/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d0a43b6d5c7..4feacc88c0b 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -626,7 +626,7 @@ static void mips_register_cpudef_type(const struct mips_def_t *def)
         .class_data = (void *)def,
     };
 
-    type_register(&ti);
+    type_register_static(&ti);
     g_free(typename);
 }
 
-- 
2.47.1

[PULL 30/49] target/sparc: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-13-zhao1.liu@intel.com
---
 target/sparc/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index dd7af86de73..adba24af2c6 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -1008,7 +1008,7 @@ static void sparc_register_cpudef_type(const struct sparc_def_t *def)
         .class_data = (void *)def,
     };
 
-    type_register(&ti);
+    type_register_static(&ti);
     g_free(typename);
 }
 
-- 
2.47.1

[PULL 31/49] target/xtensa: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-14-zhao1.liu@intel.com
---
 target/xtensa/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index ca214b948a9..2978c471c1f 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -198,7 +198,7 @@ void xtensa_register_core(XtensaConfigList *node)
     node->next = xtensa_cores;
     xtensa_cores = node;
     type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"), node->config->name);
-    type_register(&type);
+    type_register_static(&type);
     g_free((gpointer)type.name);
 }
 
-- 
2.47.1

[PULL 32/49] ui: Replace type_register() with type_register_static()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Replace type_register() with type_register_static() because
type_register() will be deprecated.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-15-zhao1.liu@intel.com
---
 ui/console-vc.c | 2 +-
 ui/dbus.c       | 2 +-
 ui/gtk.c        | 2 +-
 ui/spice-app.c  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/console-vc.c b/ui/console-vc.c
index 53fcee88f4a..fe20579832a 100644
--- a/ui/console-vc.c
+++ b/ui/console-vc.c
@@ -1073,6 +1073,6 @@ void qemu_console_early_init(void)
 {
     /* set the default vc driver */
     if (!object_class_by_name(TYPE_CHARDEV_VC)) {
-        type_register(&char_vc_type_info);
+        type_register_static(&char_vc_type_info);
     }
 }
diff --git a/ui/dbus.c b/ui/dbus.c
index 7ecd39e784a..d60b59cc546 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -476,7 +476,7 @@ early_dbus_init(DisplayOptions *opts)
 #endif
     }
 
-    type_register(&dbus_vc_type_info);
+    type_register_static(&dbus_vc_type_info);
 }
 
 static void
diff --git a/ui/gtk.c b/ui/gtk.c
index bf9d3dd679a..f9a53ea78ed 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2540,7 +2540,7 @@ static void early_gtk_display_init(DisplayOptions *opts)
     keycode_map = gd_get_keymap(&keycode_maplen);
 
 #if defined(CONFIG_VTE)
-    type_register(&char_gd_vc_type_info);
+    type_register_static(&char_gd_vc_type_info);
 #endif
 }
 
diff --git a/ui/spice-app.c b/ui/spice-app.c
index a10b4a58fe7..2a93ae59184 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -173,7 +173,7 @@ static void spice_app_display_early_init(DisplayOptions *opts)
         exit(1);
     }
 
-    type_register(&char_vc_type_info);
+    type_register_static(&char_vc_type_info);
 
     sock_path = g_strjoin("", app_dir, "/", "spice.sock", NULL);
     qopts = qemu_opts_create(list, NULL, 0, &error_abort);
-- 
2.47.1

[PULL 33/49] script/codeconverter/qom_type_info: Deprecate MakeTypeRegisterStatic and MakeTypeRegisterNotStatic

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

Deprecate MakeTypeRegisterStatic and MakeTypeRegisterNotStatic because
type_register() will be deprecated, then only type_register_static()
is used.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-16-zhao1.liu@intel.com
---
 .../codeconverter/qom_type_info.py            | 20 -------------------
 1 file changed, 20 deletions(-)

diff --git a/scripts/codeconverter/codeconverter/qom_type_info.py b/scripts/codeconverter/codeconverter/qom_type_info.py
index 255cb59923d..f92c3a4730e 100644
--- a/scripts/codeconverter/codeconverter/qom_type_info.py
+++ b/scripts/codeconverter/codeconverter/qom_type_info.py
@@ -901,26 +901,6 @@ class TypeRegisterCall(FileMatch):
     regexp = S(r'^[ \t]*', NAMED('func_name', 'type_register'),
                r'\s*\(&\s*', NAMED('name', RE_IDENTIFIER), r'\s*\);[ \t]*\n')
 
-class MakeTypeRegisterStatic(TypeRegisterCall):
-    """Make type_register() call static if variable is static const"""
-    def gen_patches(self):
-        var = self.file.find_match(TypeInfoVar, self.name)
-        if var is None:
-            self.warn("can't find TypeInfo var declaration for %s", self.name)
-            return
-        if var.is_static() and var.is_const():
-            yield self.group_match('func_name').make_patch('type_register_static')
-
-class MakeTypeRegisterNotStatic(TypeRegisterStaticCall):
-    """Make type_register() call static if variable is static const"""
-    def gen_patches(self):
-        var = self.file.find_match(TypeInfoVar, self.name)
-        if var is None:
-            self.warn("can't find TypeInfo var declaration for %s", self.name)
-            return
-        if not var.is_static() or not var.is_const():
-            yield self.group_match('func_name').make_patch('type_register')
-
 class TypeInfoMacro(FileMatch):
     """TYPE_INFO macro usage"""
     regexp = S(r'^[ \t]*TYPE_INFO\s*\(\s*', NAMED('name', RE_IDENTIFIER), r'\s*\)[ \t]*;?[ \t]*\n')
-- 
2.47.1

[PULL 34/49] qom/object: Remove type_register()

[Paolo Bonzini]

From: Zhao Liu <zhao1.liu@intel.com>

At present, type_register() and type_register_static() are identical,
although their documentation expects the *_static variant to accept
the Typeinfo with the strings that have the static lifetime.

However, the code implementation doesn't have any check or guarantee for
static lifetime. In fact, this is unnecessary because type_new()
duplicates all strings, thereby taking ownership of them.

Therefore, type_register() and type_register_static() are redundant, so
one of them should be removed.

Since the changes required to remove type_register() were smaller,
type_register() was replaced with type_register_static() throughout the
code base.  Drop its definition, and delete the requirement about string
lifetime from the documentation.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20241029085934.2799066-17-zhao1.liu@intel.com
---
 include/qom/object.h | 14 --------------
 qom/object.c         |  7 +------
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 43c135984a6..a201c9712a1 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -880,24 +880,10 @@ const char *object_get_typename(const Object *obj);
  * type_register_static:
  * @info: The #TypeInfo of the new type.
  *
- * @info and all of the strings it points to should exist for the life time
- * that the type is registered.
- *
  * Returns: the new #Type.
  */
 Type type_register_static(const TypeInfo *info);
 
-/**
- * type_register:
- * @info: The #TypeInfo of the new type
- *
- * Unlike type_register_static(), this call does not require @info or its
- * string members to continue to exist after the call returns.
- *
- * Returns: the new #Type.
- */
-Type type_register(const TypeInfo *info);
-
 /**
  * type_register_static_array:
  * @infos: The array of the new type #TypeInfo structures.
diff --git a/qom/object.c b/qom/object.c
index 9edc06d391f..c7660f9a097 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -175,17 +175,12 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
     return ti;
 }
 
-TypeImpl *type_register(const TypeInfo *info)
+TypeImpl *type_register_static(const TypeInfo *info)
 {
     assert(info->parent);
     return type_register_internal(info);
 }
 
-TypeImpl *type_register_static(const TypeInfo *info)
-{
-    return type_register(info);
-}
-
 void type_register_static_array(const TypeInfo *infos, int nr_infos)
 {
     int i;
-- 
2.47.1

[PULL 35/49] bql: check that the BQL is not dropped within marked sections

[Paolo Bonzini]

The Big QEMU Lock (BQL) is used to provide interior mutability to Rust
code.  While BqlCell performs indivisible accesses, an equivalent of
RefCell will allow the borrower to hold to the interior content for a
long time.  If the BQL is dropped, another thread could come and mutate
the data from C code (Rust code would panic on borrow_mut() instead).
In order to prevent this, add a new BQL primitive that can mark
BQL-atomic sections and aborts if the BQL is dropped within them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/main-loop.h | 15 +++++++++++++++
 stubs/iothread-lock.c    | 15 +++++++++++++++
 system/cpus.c            | 15 +++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 5764db157c9..646306c272f 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -262,6 +262,21 @@ AioContext *iohandler_get_aio_context(void);
  */
 bool bql_locked(void);
 
+/**
+ * bql_block: Allow/deny releasing the BQL
+ *
+ * The Big QEMU Lock (BQL) is used to provide interior mutability to
+ * Rust code, but this only works if other threads cannot run while
+ * the Rust code has an active borrow.  This is because C code in
+ * other threads could come in and mutate data under the Rust code's
+ * feet.
+ *
+ * @increase: Whether to increase or decrease the blocking counter.
+ *            Releasing the BQL while the counter is nonzero triggers
+ *            an assertion failure.
+ */
+void bql_block_unlock(bool increase);
+
 /**
  * qemu_in_main_thread: return whether it's possible to safely access
  * the global state of the block layer.
diff --git a/stubs/iothread-lock.c b/stubs/iothread-lock.c
index d7890e5581c..54676598950 100644
--- a/stubs/iothread-lock.c
+++ b/stubs/iothread-lock.c
@@ -1,6 +1,8 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 
+static uint32_t bql_unlock_blocked;
+
 bool bql_locked(void)
 {
     return false;
@@ -12,4 +14,17 @@ void bql_lock_impl(const char *file, int line)
 
 void bql_unlock(void)
 {
+    assert(!bql_unlock_blocked);
+}
+
+void bql_block_unlock(bool increase)
+{
+    uint32_t new_value;
+
+    assert(bql_locked());
+
+    /* check for overflow! */
+    new_value = bql_unlock_blocked + increase - !increase;
+    assert((new_value > bql_unlock_blocked) == increase);
+    bql_unlock_blocked = new_value;
 }
diff --git a/system/cpus.c b/system/cpus.c
index 1c818ff6828..ba633c7688b 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -514,6 +514,20 @@ bool qemu_in_vcpu_thread(void)
 
 QEMU_DEFINE_STATIC_CO_TLS(bool, bql_locked)
 
+static uint32_t bql_unlock_blocked;
+
+void bql_block_unlock(bool increase)
+{
+    uint32_t new_value;
+
+    assert(bql_locked());
+
+    /* check for overflow! */
+    new_value = bql_unlock_blocked + increase - !increase;
+    assert((new_value > bql_unlock_blocked) == increase);
+    bql_unlock_blocked = new_value;
+}
+
 bool bql_locked(void)
 {
     return get_bql_locked();
@@ -540,6 +554,7 @@ void bql_lock_impl(const char *file, int line)
 void bql_unlock(void)
 {
     g_assert(bql_locked());
+    g_assert(!bql_unlock_blocked);
     set_bql_locked(false);
     qemu_mutex_unlock(&bql);
 }
-- 
2.47.1

[PULL 36/49] rust: cell: add BQL-enforcing Cell variant

[Paolo Bonzini]

QEMU objects usually have their pointer shared with the "outside
world" very early in their lifetime, for example when they create their
MemoryRegions.  Because at this point it is not valid anymore to
create a &mut reference to the device, individual parts of the
device struct must be made mutable in a controlled manner.

QEMU's Big Lock (BQL) effectively turns multi-threaded code into
single-threaded code while device code runs, as long as the BQL is not
released while the device is borrowed (because C code could sneak in and
mutate the device).  We can then introduce custom interior mutability primitives
that are semantically similar to the standard library's (single-threaded)
Cell and RefCell, but account for QEMU's threading model.  Accessing
the "BqlCell" or borrowing the "BqlRefCell" requires proving that the
BQL is held, and attempting to access without the BQL is a runtime panic,
similar to RefCell's already-borrowed panic.

With respect to naming I also considered omitting the "Bql" prefix or
moving it to the module, e.g.  qemu_api::bql::{Cell, RefCell}.  However,
this could easily lead to mistakes and confusion; for example rustc could
suggest the wrong import, leading to subtle bugs.

As a start introduce the an equivalent of Cell.  Almost all of the code
was taken from Rust's standard library, while removing unstable features
and probably-unnecessary functionality that constitute a large of the
original code.  A lot of what's left is documentation, as well as unit
tests in the form of doctests.  These are not yet integrated in "make
check" but can be run with "cargo test --doc".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build |   1 +
 rust/qemu-api/src/cell.rs | 298 ++++++++++++++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs  |   1 +
 3 files changed, 300 insertions(+)
 create mode 100644 rust/qemu-api/src/cell.rs

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index d719c13f46d..edc21e1a3f8 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -13,6 +13,7 @@ _qemu_api_rs = static_library(
     [
       'src/lib.rs',
       'src/bindings.rs',
+      'src/cell.rs',
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs
new file mode 100644
index 00000000000..2e4ea8d590d
--- /dev/null
+++ b/rust/qemu-api/src/cell.rs
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: MIT
+//
+// This file is based on library/core/src/cell.rs from
+// Rust 1.82.0.
+//
+// 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.
+
+//! BQL-protected mutable containers.
+//!
+//! Rust memory safety is based on this rule: Given an object `T`, it is only
+//! possible to have one of the following:
+//!
+//! - Having several immutable references (`&T`) to the object (also known as
+//!   **aliasing**).
+//! - Having one mutable reference (`&mut T`) to the object (also known as
+//!   **mutability**).
+//!
+//! This is enforced by the Rust compiler. However, there are situations where
+//! this rule is not flexible enough. Sometimes it is required to have multiple
+//! references to an object and yet mutate it. In particular, QEMU objects
+//! usually have their pointer shared with the "outside world very early in
+//! their lifetime", for example when they create their
+//! [`MemoryRegion`s](crate::bindings::MemoryRegion).  Therefore, individual
+//! parts of a  device must be made mutable in a controlled manner through the
+//! use of cell types.
+//!
+//! This module provides a way to do so via the Big QEMU Lock.  While
+//! [`BqlCell<T>`] is essentially the same single-threaded primitive that is
+//! available in `std::cell`, the BQL allows it to be used from a multi-threaded
+//! context and to share references across threads, while maintaining Rust's
+//! safety guarantees.  For this reason, unlike its `std::cell` counterpart,
+//! `BqlCell` implements the `Sync` trait.
+//!
+//! BQL checks are performed in debug builds but can be optimized away in
+//! release builds, providing runtime safety during development with no overhead
+//! in production.
+//!
+//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two
+//! are not interchangeable. Using `std::cell` types in QEMU device
+//! implementations is usually incorrect and can lead to thread-safety issues.
+//!
+//! ## `BqlCell<T>`
+//!
+//! [`BqlCell<T>`] implements interior mutability by moving values in and out of
+//! the cell. That is, an `&mut T` to the inner value can never be obtained as
+//! long as the cell is shared. The value itself cannot be directly obtained
+//! without copying it, cloning it, or replacing it with something else. This
+//! type provides the following methods, all of which can be called only while
+//! the BQL is held:
+//!
+//!  - For types that implement [`Copy`], the [`get`](BqlCell::get) method
+//!    retrieves the current interior value by duplicating it.
+//!  - For types that implement [`Default`], the [`take`](BqlCell::take) method
+//!    replaces the current interior value with [`Default::default()`] and
+//!    returns the replaced value.
+//!  - All types have:
+//!    - [`replace`](BqlCell::replace): replaces the current interior value and
+//!      returns the replaced value.
+//!    - [`set`](BqlCell::set): this method replaces the interior value,
+//!      dropping the replaced value.
+
+use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem};
+
+use crate::bindings;
+
+// TODO: When building doctests do not include the actual BQL, because cargo
+// does not know how to link them to libqemuutil.  This can be fixed by
+// running rustdoc from "meson test" instead of relying on cargo.
+pub fn bql_locked() -> bool {
+    // SAFETY: the function does nothing but return a thread-local bool
+    !cfg!(MESON) || unsafe { bindings::bql_locked() }
+}
+
+/// A mutable memory location that is protected by the Big QEMU Lock.
+///
+/// # Memory layout
+///
+/// `BqlCell<T>` has the same in-memory representation as its inner type `T`.
+#[repr(transparent)]
+pub struct BqlCell<T> {
+    value: UnsafeCell<T>,
+}
+
+// SAFETY: Same as for std::sync::Mutex.  In the end this *is* a Mutex,
+// except it is stored out-of-line
+unsafe impl<T: Send> Send for BqlCell<T> {}
+unsafe impl<T: Send> Sync for BqlCell<T> {}
+
+impl<T: Copy> Clone for BqlCell<T> {
+    #[inline]
+    fn clone(&self) -> BqlCell<T> {
+        BqlCell::new(self.get())
+    }
+}
+
+impl<T: Default> Default for BqlCell<T> {
+    /// Creates a `BqlCell<T>`, with the `Default` value for T.
+    #[inline]
+    fn default() -> BqlCell<T> {
+        BqlCell::new(Default::default())
+    }
+}
+
+impl<T: PartialEq + Copy> PartialEq for BqlCell<T> {
+    #[inline]
+    fn eq(&self, other: &BqlCell<T>) -> bool {
+        self.get() == other.get()
+    }
+}
+
+impl<T: Eq + Copy> Eq for BqlCell<T> {}
+
+impl<T: PartialOrd + Copy> PartialOrd for BqlCell<T> {
+    #[inline]
+    fn partial_cmp(&self, other: &BqlCell<T>) -> Option<Ordering> {
+        self.get().partial_cmp(&other.get())
+    }
+}
+
+impl<T: Ord + Copy> Ord for BqlCell<T> {
+    #[inline]
+    fn cmp(&self, other: &BqlCell<T>) -> Ordering {
+        self.get().cmp(&other.get())
+    }
+}
+
+impl<T> From<T> for BqlCell<T> {
+    /// Creates a new `BqlCell<T>` containing the given value.
+    fn from(t: T) -> BqlCell<T> {
+        BqlCell::new(t)
+    }
+}
+
+impl<T: fmt::Debug + Copy> fmt::Debug for BqlCell<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.get().fmt(f)
+    }
+}
+
+impl<T: fmt::Display + Copy> fmt::Display for BqlCell<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.get().fmt(f)
+    }
+}
+
+impl<T> BqlCell<T> {
+    /// Creates a new `BqlCell` containing the given value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    /// ```
+    #[inline]
+    pub const fn new(value: T) -> BqlCell<T> {
+        BqlCell {
+            value: UnsafeCell::new(value),
+        }
+    }
+
+    /// Sets the contained value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    ///
+    /// c.set(10);
+    /// ```
+    #[inline]
+    pub fn set(&self, val: T) {
+        self.replace(val);
+    }
+
+    /// Replaces the contained value with `val`, and returns the old contained
+    /// value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let cell = BqlCell::new(5);
+    /// assert_eq!(cell.get(), 5);
+    /// assert_eq!(cell.replace(10), 5);
+    /// assert_eq!(cell.get(), 10);
+    /// ```
+    #[inline]
+    pub fn replace(&self, val: T) -> T {
+        assert!(bql_locked());
+        // SAFETY: This can cause data races if called from multiple threads,
+        // but it won't happen as long as C code accesses the value
+        // under BQL protection only.
+        mem::replace(unsafe { &mut *self.value.get() }, val)
+    }
+
+    /// Unwraps the value, consuming the cell.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    /// let five = c.into_inner();
+    ///
+    /// assert_eq!(five, 5);
+    /// ```
+    pub fn into_inner(self) -> T {
+        assert!(bql_locked());
+        self.value.into_inner()
+    }
+}
+
+impl<T: Copy> BqlCell<T> {
+    /// Returns a copy of the contained value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    ///
+    /// let five = c.get();
+    /// ```
+    #[inline]
+    pub fn get(&self) -> T {
+        assert!(bql_locked());
+        // SAFETY: This can cause data races if called from multiple threads,
+        // but it won't happen as long as C code accesses the value
+        // under BQL protection only.
+        unsafe { *self.value.get() }
+    }
+}
+
+impl<T> BqlCell<T> {
+    /// Returns a raw pointer to the underlying data in this cell.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    ///
+    /// let ptr = c.as_ptr();
+    /// ```
+    #[inline]
+    pub const fn as_ptr(&self) -> *mut T {
+        self.value.get()
+    }
+}
+
+impl<T: Default> BqlCell<T> {
+    /// Takes the value of the cell, leaving `Default::default()` in its place.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlCell;
+    ///
+    /// let c = BqlCell::new(5);
+    /// let five = c.take();
+    ///
+    /// assert_eq!(five, 5);
+    /// assert_eq!(c.into_inner(), 0);
+    /// ```
+    pub fn take(&self) -> T {
+        self.replace(Default::default())
+    }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 440aff3817d..b04d110b3f5 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -8,6 +8,7 @@
 pub mod bindings;
 
 pub mod c_str;
+pub mod cell;
 pub mod definitions;
 pub mod device_class;
 pub mod offset_of;
-- 
2.47.1

[PULL 37/49] rust: cell: add BQL-enforcing RefCell variant

[Paolo Bonzini]

Similar to the existing BqlCell, introduce a custom interior mutability
primitive that resembles RefCell but accounts for QEMU's threading model.
Borrowing the RefCell requires proving that the BQL is held, and
attempting to access without the BQL is a runtime panic.

Almost all of the code was taken from Rust's standard library, while
removing unstable features and probably-unnecessary functionality that
amounts to 60% of the original code.  A lot of what's left is documentation,
as well as unit tests in the form of doctests.  These are not yet integrated
in "make check" but can be run with "cargo test --doc".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/Cargo.toml  |   3 +-
 rust/qemu-api/meson.build |   3 +
 rust/qemu-api/src/cell.rs | 544 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 539 insertions(+), 11 deletions(-)

diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index 669f288d1cb..4aa22f31986 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -20,8 +20,9 @@ qemu_api_macros = { path = "../qemu-api-macros" }
 version_check = "~0.9"
 
 [features]
-default = []
+default = ["debug_cell"]
 allocator = []
+debug_cell = []
 
 [lints]
 workspace = true
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index edc21e1a3f8..cacb112c5c3 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -6,6 +6,9 @@ _qemu_api_cfg = run_command(rustc_args,
 if rustc.version().version_compare('>=1.77.0')
   _qemu_api_cfg += ['--cfg', 'has_offset_of']
 endif
+if get_option('debug_mutex')
+  _qemu_api_cfg += ['--feature', 'debug_cell']
+endif
 
 _qemu_api_rs = static_library(
   'qemu_api',
diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs
index 2e4ea8d590d..28349de291a 100644
--- a/rust/qemu-api/src/cell.rs
+++ b/rust/qemu-api/src/cell.rs
@@ -46,20 +46,30 @@
 //! parts of a  device must be made mutable in a controlled manner through the
 //! use of cell types.
 //!
-//! This module provides a way to do so via the Big QEMU Lock.  While
-//! [`BqlCell<T>`] is essentially the same single-threaded primitive that is
-//! available in `std::cell`, the BQL allows it to be used from a multi-threaded
-//! context and to share references across threads, while maintaining Rust's
-//! safety guarantees.  For this reason, unlike its `std::cell` counterpart,
-//! `BqlCell` implements the `Sync` trait.
+//! [`BqlCell<T>`] and [`BqlRefCell<T>`] allow doing this via the Big QEMU Lock.
+//! While they are essentially the same single-threaded primitives that are
+//! available in `std::cell`, the BQL allows them to be used from a
+//! multi-threaded context and to share references across threads, while
+//! maintaining Rust's safety guarantees.  For this reason, unlike
+//! their `std::cell` counterparts, `BqlCell` and `BqlRefCell` implement the
+//! `Sync` trait.
 //!
 //! BQL checks are performed in debug builds but can be optimized away in
 //! release builds, providing runtime safety during development with no overhead
 //! in production.
 //!
-//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two
-//! are not interchangeable. Using `std::cell` types in QEMU device
-//! implementations is usually incorrect and can lead to thread-safety issues.
+//! The two provide different ways of handling interior mutability.
+//! `BqlRefCell` is best suited for data that is primarily accessed by the
+//! device's own methods, where multiple reads and writes can be grouped within
+//! a single borrow and a mutable reference can be passed around.  Instead,
+//! [`BqlCell`] is a better choice when sharing small pieces of data with
+//! external code (especially C code), because it provides simple get/set
+//! operations that can be used one at a time.
+//!
+//! Warning: While `BqlCell` and `BqlRefCell` are similar to their `std::cell`
+//! counterparts, they are not interchangeable. Using `std::cell` types in
+//! QEMU device implementations is usually incorrect and can lead to
+//! thread-safety issues.
 //!
 //! ## `BqlCell<T>`
 //!
@@ -80,8 +90,37 @@
 //!      returns the replaced value.
 //!    - [`set`](BqlCell::set): this method replaces the interior value,
 //!      dropping the replaced value.
+//!
+//! ## `BqlRefCell<T>`
+//!
+//! [`BqlRefCell<T>`] uses Rust's lifetimes to implement "dynamic borrowing", a
+//! process whereby one can claim temporary, exclusive, mutable access to the
+//! inner value:
+//!
+//! ```ignore
+//! fn clear_interrupts(&self, val: u32) {
+//!     // A mutable borrow gives read-write access to the registers
+//!     let mut regs = self.registers.borrow_mut();
+//!     let old = regs.interrupt_status();
+//!     regs.update_interrupt_status(old & !val);
+//! }
+//! ```
+//!
+//! Borrows for `BqlRefCell<T>`s are tracked at _runtime_, unlike Rust's native
+//! reference types which are entirely tracked statically, at compile time.
+//! Multiple immutable borrows are allowed via [`borrow`](BqlRefCell::borrow),
+//! or a single mutable borrow via [`borrow_mut`](BqlRefCell::borrow_mut).  The
+//! thread will panic if these rules are violated or if the BQL is not held.
 
-use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem};
+use std::{
+    cell::{Cell, UnsafeCell},
+    cmp::Ordering,
+    fmt,
+    marker::PhantomData,
+    mem,
+    ops::{Deref, DerefMut},
+    ptr::NonNull,
+};
 
 use crate::bindings;
 
@@ -93,6 +132,15 @@ pub fn bql_locked() -> bool {
     !cfg!(MESON) || unsafe { bindings::bql_locked() }
 }
 
+fn bql_block_unlock(increase: bool) {
+    if cfg!(MESON) {
+        // SAFETY: this only adjusts a counter
+        unsafe {
+            bindings::bql_block_unlock(increase);
+        }
+    }
+}
+
 /// A mutable memory location that is protected by the Big QEMU Lock.
 ///
 /// # Memory layout
@@ -296,3 +344,479 @@ pub fn take(&self) -> T {
         self.replace(Default::default())
     }
 }
+
+/// A mutable memory location with dynamically checked borrow rules,
+/// protected by the Big QEMU Lock.
+///
+/// See the [module-level documentation](self) for more.
+///
+/// # Memory layout
+///
+/// `BqlRefCell<T>` starts with the same in-memory representation as its
+/// inner type `T`.
+#[repr(C)]
+pub struct BqlRefCell<T> {
+    // It is important that this is the first field (which is not the case
+    // for std::cell::BqlRefCell), so that we can use offset_of! on it.
+    // UnsafeCell and repr(C) both prevent usage of niches.
+    value: UnsafeCell<T>,
+    borrow: Cell<BorrowFlag>,
+    // Stores the location of the earliest currently active borrow.
+    // This gets updated whenever we go from having zero borrows
+    // to having a single borrow. When a borrow occurs, this gets included
+    // in the panic message
+    #[cfg(feature = "debug_cell")]
+    borrowed_at: Cell<Option<&'static std::panic::Location<'static>>>,
+}
+
+// Positive values represent the number of `BqlRef` active. Negative values
+// represent the number of `BqlRefMut` active. Right now QEMU's implementation
+// does not allow to create `BqlRefMut`s that refer to distinct, nonoverlapping
+// components of a `BqlRefCell` (e.g., different ranges of a slice).
+//
+// `BqlRef` and `BqlRefMut` are both two words in size, and so there will likely
+// never be enough `BqlRef`s or `BqlRefMut`s in existence to overflow half of
+// the `usize` range. Thus, a `BorrowFlag` will probably never overflow or
+// underflow. However, this is not a guarantee, as a pathological program could
+// repeatedly create and then mem::forget `BqlRef`s or `BqlRefMut`s. Thus, all
+// code must explicitly check for overflow and underflow in order to avoid
+// unsafety, or at least behave correctly in the event that overflow or
+// underflow happens (e.g., see BorrowRef::new).
+type BorrowFlag = isize;
+const UNUSED: BorrowFlag = 0;
+
+#[inline(always)]
+const fn is_writing(x: BorrowFlag) -> bool {
+    x < UNUSED
+}
+
+#[inline(always)]
+const fn is_reading(x: BorrowFlag) -> bool {
+    x > UNUSED
+}
+
+impl<T> BqlRefCell<T> {
+    /// Creates a new `BqlRefCell` containing `value`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new(5);
+    /// ```
+    #[inline]
+    pub const fn new(value: T) -> BqlRefCell<T> {
+        BqlRefCell {
+            value: UnsafeCell::new(value),
+            borrow: Cell::new(UNUSED),
+            #[cfg(feature = "debug_cell")]
+            borrowed_at: Cell::new(None),
+        }
+    }
+}
+
+// This ensures the panicking code is outlined from `borrow_mut` for
+// `BqlRefCell`.
+#[inline(never)]
+#[cold]
+#[cfg(feature = "debug_cell")]
+fn panic_already_borrowed(source: &Cell<Option<&'static std::panic::Location<'static>>>) -> ! {
+    // If a borrow occurred, then we must already have an outstanding borrow,
+    // so `borrowed_at` will be `Some`
+    panic!("already borrowed at {:?}", source.take().unwrap())
+}
+
+#[inline(never)]
+#[cold]
+#[cfg(not(feature = "debug_cell"))]
+fn panic_already_borrowed() -> ! {
+    panic!("already borrowed")
+}
+
+impl<T> BqlRefCell<T> {
+    #[inline]
+    #[allow(clippy::unused_self)]
+    fn panic_already_borrowed(&self) -> ! {
+        #[cfg(feature = "debug_cell")]
+        {
+            panic_already_borrowed(&self.borrowed_at)
+        }
+        #[cfg(not(feature = "debug_cell"))]
+        {
+            panic_already_borrowed()
+        }
+    }
+
+    /// Immutably borrows the wrapped value.
+    ///
+    /// The borrow lasts until the returned `BqlRef` exits scope. Multiple
+    /// immutable borrows can be taken out at the same time.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the value is currently mutably borrowed.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new(5);
+    ///
+    /// let borrowed_five = c.borrow();
+    /// let borrowed_five2 = c.borrow();
+    /// ```
+    ///
+    /// An example of panic:
+    ///
+    /// ```should_panic
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new(5);
+    ///
+    /// let m = c.borrow_mut();
+    /// let b = c.borrow(); // this causes a panic
+    /// ```
+    #[inline]
+    #[track_caller]
+    pub fn borrow(&self) -> BqlRef<'_, T> {
+        if let Some(b) = BorrowRef::new(&self.borrow) {
+            // `borrowed_at` is always the *first* active borrow
+            if b.borrow.get() == 1 {
+                #[cfg(feature = "debug_cell")]
+                self.borrowed_at.set(Some(std::panic::Location::caller()));
+            }
+
+            bql_block_unlock(true);
+
+            // SAFETY: `BorrowRef` ensures that there is only immutable access
+            // to the value while borrowed.
+            let value = unsafe { NonNull::new_unchecked(self.value.get()) };
+            BqlRef { value, borrow: b }
+        } else {
+            self.panic_already_borrowed()
+        }
+    }
+
+    /// Mutably borrows the wrapped value.
+    ///
+    /// The borrow lasts until the returned `BqlRefMut` or all `BqlRefMut`s
+    /// derived from it exit scope. The value cannot be borrowed while this
+    /// borrow is active.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the value is currently borrowed.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new("hello".to_owned());
+    ///
+    /// *c.borrow_mut() = "bonjour".to_owned();
+    ///
+    /// assert_eq!(&*c.borrow(), "bonjour");
+    /// ```
+    ///
+    /// An example of panic:
+    ///
+    /// ```should_panic
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new(5);
+    /// let m = c.borrow();
+    ///
+    /// let b = c.borrow_mut(); // this causes a panic
+    /// ```
+    #[inline]
+    #[track_caller]
+    pub fn borrow_mut(&self) -> BqlRefMut<'_, T> {
+        if let Some(b) = BorrowRefMut::new(&self.borrow) {
+            #[cfg(feature = "debug_cell")]
+            {
+                self.borrowed_at.set(Some(std::panic::Location::caller()));
+            }
+
+            // SAFETY: this only adjusts a counter
+            bql_block_unlock(true);
+
+            // SAFETY: `BorrowRefMut` guarantees unique access.
+            let value = unsafe { NonNull::new_unchecked(self.value.get()) };
+            BqlRefMut {
+                value,
+                _borrow: b,
+                marker: PhantomData,
+            }
+        } else {
+            self.panic_already_borrowed()
+        }
+    }
+
+    /// Returns a raw pointer to the underlying data in this cell.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use qemu_api::cell::BqlRefCell;
+    ///
+    /// let c = BqlRefCell::new(5);
+    ///
+    /// let ptr = c.as_ptr();
+    /// ```
+    #[inline]
+    pub const fn as_ptr(&self) -> *mut T {
+        self.value.get()
+    }
+}
+
+// SAFETY: Same as for std::sync::Mutex.  In the end this is a Mutex that is
+// stored out-of-line.  Even though BqlRefCell includes Cells, they are
+// themselves protected by the Big QEMU Lock.  Furtheremore, the Big QEMU
+// Lock cannot be released while any borrows is active.
+unsafe impl<T> Send for BqlRefCell<T> where T: Send {}
+unsafe impl<T> Sync for BqlRefCell<T> {}
+
+impl<T: Clone> Clone for BqlRefCell<T> {
+    /// # Panics
+    ///
+    /// Panics if the value is currently mutably borrowed.
+    #[inline]
+    #[track_caller]
+    fn clone(&self) -> BqlRefCell<T> {
+        BqlRefCell::new(self.borrow().clone())
+    }
+
+    /// # Panics
+    ///
+    /// Panics if `source` is currently mutably borrowed.
+    #[inline]
+    #[track_caller]
+    fn clone_from(&mut self, source: &Self) {
+        self.value.get_mut().clone_from(&source.borrow())
+    }
+}
+
+impl<T: Default> Default for BqlRefCell<T> {
+    /// Creates a `BqlRefCell<T>`, with the `Default` value for T.
+    #[inline]
+    fn default() -> BqlRefCell<T> {
+        BqlRefCell::new(Default::default())
+    }
+}
+
+impl<T: PartialEq> PartialEq for BqlRefCell<T> {
+    /// # Panics
+    ///
+    /// Panics if the value in either `BqlRefCell` is currently mutably
+    /// borrowed.
+    #[inline]
+    fn eq(&self, other: &BqlRefCell<T>) -> bool {
+        *self.borrow() == *other.borrow()
+    }
+}
+
+impl<T: Eq> Eq for BqlRefCell<T> {}
+
+impl<T: PartialOrd> PartialOrd for BqlRefCell<T> {
+    /// # Panics
+    ///
+    /// Panics if the value in either `BqlRefCell` is currently mutably
+    /// borrowed.
+    #[inline]
+    fn partial_cmp(&self, other: &BqlRefCell<T>) -> Option<Ordering> {
+        self.borrow().partial_cmp(&*other.borrow())
+    }
+}
+
+impl<T: Ord> Ord for BqlRefCell<T> {
+    /// # Panics
+    ///
+    /// Panics if the value in either `BqlRefCell` is currently mutably
+    /// borrowed.
+    #[inline]
+    fn cmp(&self, other: &BqlRefCell<T>) -> Ordering {
+        self.borrow().cmp(&*other.borrow())
+    }
+}
+
+impl<T> From<T> for BqlRefCell<T> {
+    /// Creates a new `BqlRefCell<T>` containing the given value.
+    fn from(t: T) -> BqlRefCell<T> {
+        BqlRefCell::new(t)
+    }
+}
+
+struct BorrowRef<'b> {
+    borrow: &'b Cell<BorrowFlag>,
+}
+
+impl<'b> BorrowRef<'b> {
+    #[inline]
+    fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
+        let b = borrow.get().wrapping_add(1);
+        if !is_reading(b) {
+            // Incrementing borrow can result in a non-reading value (<= 0) in these cases:
+            // 1. It was < 0, i.e. there are writing borrows, so we can't allow a read
+            //    borrow due to Rust's reference aliasing rules
+            // 2. It was isize::MAX (the max amount of reading borrows) and it overflowed
+            //    into isize::MIN (the max amount of writing borrows) so we can't allow an
+            //    additional read borrow because isize can't represent so many read borrows
+            //    (this can only happen if you mem::forget more than a small constant amount
+            //    of `BqlRef`s, which is not good practice)
+            None
+        } else {
+            // Incrementing borrow can result in a reading value (> 0) in these cases:
+            // 1. It was = 0, i.e. it wasn't borrowed, and we are taking the first read
+            //    borrow
+            // 2. It was > 0 and < isize::MAX, i.e. there were read borrows, and isize is
+            //    large enough to represent having one more read borrow
+            borrow.set(b);
+            Some(BorrowRef { borrow })
+        }
+    }
+}
+
+impl Drop for BorrowRef<'_> {
+    #[inline]
+    fn drop(&mut self) {
+        let borrow = self.borrow.get();
+        debug_assert!(is_reading(borrow));
+        self.borrow.set(borrow - 1);
+        bql_block_unlock(false)
+    }
+}
+
+impl Clone for BorrowRef<'_> {
+    #[inline]
+    fn clone(&self) -> Self {
+        BorrowRef::new(self.borrow).unwrap()
+    }
+}
+
+/// Wraps a borrowed reference to a value in a `BqlRefCell` box.
+/// A wrapper type for an immutably borrowed value from a `BqlRefCell<T>`.
+///
+/// See the [module-level documentation](self) for more.
+pub struct BqlRef<'b, T: 'b> {
+    // NB: we use a pointer instead of `&'b T` to avoid `noalias` violations, because a
+    // `BqlRef` argument doesn't hold immutability for its whole scope, only until it drops.
+    // `NonNull` is also covariant over `T`, just like we would have with `&T`.
+    value: NonNull<T>,
+    borrow: BorrowRef<'b>,
+}
+
+impl<T> Deref for BqlRef<'_, T> {
+    type Target = T;
+
+    #[inline]
+    fn deref(&self) -> &T {
+        // SAFETY: the value is accessible as long as we hold our borrow.
+        unsafe { self.value.as_ref() }
+    }
+}
+
+impl<'b, T> BqlRef<'b, T> {
+    /// Copies a `BqlRef`.
+    ///
+    /// The `BqlRefCell` is already immutably borrowed, so this cannot fail.
+    ///
+    /// This is an associated function that needs to be used as
+    /// `BqlRef::clone(...)`. A `Clone` implementation or a method would
+    /// interfere with the widespread use of `r.borrow().clone()` to clone
+    /// the contents of a `BqlRefCell`.
+    #[must_use]
+    #[inline]
+    #[allow(clippy::should_implement_trait)]
+    pub fn clone(orig: &BqlRef<'b, T>) -> BqlRef<'b, T> {
+        BqlRef {
+            value: orig.value,
+            borrow: orig.borrow.clone(),
+        }
+    }
+}
+
+impl<T: fmt::Debug> fmt::Debug for BqlRef<'_, T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
+
+impl<T: fmt::Display> fmt::Display for BqlRef<'_, T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
+
+struct BorrowRefMut<'b> {
+    borrow: &'b Cell<BorrowFlag>,
+}
+
+impl<'b> BorrowRefMut<'b> {
+    #[inline]
+    fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRefMut<'b>> {
+        // There must currently be no existing references when borrow_mut() is
+        // called, so we explicitly only allow going from UNUSED to UNUSED - 1.
+        match borrow.get() {
+            UNUSED => {
+                borrow.set(UNUSED - 1);
+                Some(BorrowRefMut { borrow })
+            }
+            _ => None,
+        }
+    }
+}
+
+impl Drop for BorrowRefMut<'_> {
+    #[inline]
+    fn drop(&mut self) {
+        let borrow = self.borrow.get();
+        debug_assert!(is_writing(borrow));
+        self.borrow.set(borrow + 1);
+        bql_block_unlock(false)
+    }
+}
+
+/// A wrapper type for a mutably borrowed value from a `BqlRefCell<T>`.
+///
+/// See the [module-level documentation](self) for more.
+pub struct BqlRefMut<'b, T: 'b> {
+    // NB: we use a pointer instead of `&'b mut T` to avoid `noalias` violations, because a
+    // `BqlRefMut` argument doesn't hold exclusivity for its whole scope, only until it drops.
+    value: NonNull<T>,
+    _borrow: BorrowRefMut<'b>,
+    // `NonNull` is covariant over `T`, so we need to reintroduce invariance.
+    marker: PhantomData<&'b mut T>,
+}
+
+impl<T> Deref for BqlRefMut<'_, T> {
+    type Target = T;
+
+    #[inline]
+    fn deref(&self) -> &T {
+        // SAFETY: the value is accessible as long as we hold our borrow.
+        unsafe { self.value.as_ref() }
+    }
+}
+
+impl<T> DerefMut for BqlRefMut<'_, T> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut T {
+        // SAFETY: the value is accessible as long as we hold our borrow.
+        unsafe { self.value.as_mut() }
+    }
+}
+
+impl<T: fmt::Debug> fmt::Debug for BqlRefMut<'_, T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
+
+impl<T: fmt::Display> fmt::Display for BqlRefMut<'_, T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        (**self).fmt(f)
+    }
+}
-- 
2.47.1

[PULL 38/49] rust: define prelude

[Paolo Bonzini]

Add a module that will contain frequently used traits and
occasionally structs.  They can be included quickly with
"use qemu_api::prelude::*".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build    | 1 +
 rust/qemu-api/src/lib.rs     | 5 +++++
 rust/qemu-api/src/prelude.rs | 6 ++++++
 3 files changed, 12 insertions(+)
 create mode 100644 rust/qemu-api/src/prelude.rs

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index cacb112c5c3..f8b4cd39a26 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -21,6 +21,7 @@ _qemu_api_rs = static_library(
       'src/definitions.rs',
       'src/device_class.rs',
       'src/offset_of.rs',
+      'src/prelude.rs',
       'src/vmstate.rs',
       'src/zeroable.rs',
     ],
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index b04d110b3f5..e5956cd5eb6 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -7,6 +7,11 @@
 #[rustfmt::skip]
 pub mod bindings;
 
+// preserve one-item-per-"use" syntax, it is clearer
+// for prelude-like modules
+#[rustfmt::skip]
+pub mod prelude;
+
 pub mod c_str;
 pub mod cell;
 pub mod definitions;
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
new file mode 100644
index 00000000000..dfaddbd062a
--- /dev/null
+++ b/rust/qemu-api/src/prelude.rs
@@ -0,0 +1,6 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+pub use crate::cell::BqlCell;
+pub use crate::cell::BqlRefCell;
-- 
2.47.1

[PULL 39/49] rust: add bindings for interrupt sources

[Paolo Bonzini]

The InterruptSource bindings let us call qemu_set_irq() and sysbus_init_irq()
as safe code.

Interrupt sources, qemu_irq in C code, are pointers to IRQState objects.
They are QOM link properties and can be written to outside the control
of the device (i.e. from a shared reference); therefore they must be
interior-mutable in Rust.  Since thread-safety is provided by the BQL,
what we want here is the newly-introduced BqlCell.  A pointer to the
contents of the BqlCell (an IRQState**, or equivalently qemu_irq*)
is then passed to the C sysbus_init_irq function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 22 ++++----
 rust/qemu-api/meson.build        |  2 +
 rust/qemu-api/src/irq.rs         | 91 ++++++++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs         |  2 +
 rust/qemu-api/src/sysbus.rs      | 27 ++++++++++
 5 files changed, 134 insertions(+), 10 deletions(-)
 create mode 100644 rust/qemu-api/src/irq.rs
 create mode 100644 rust/qemu-api/src/sysbus.rs

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 317a9b3c5ad..c5c8c463d37 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -13,6 +13,7 @@
     c_str,
     definitions::ObjectImpl,
     device_class::TYPE_SYS_BUS_DEVICE,
+    irq::InterruptSource,
 };
 
 use crate::{
@@ -94,7 +95,7 @@ pub struct PL011State {
     ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
     /// ```
     #[doc(alias = "irq")]
-    pub interrupts: [qemu_irq; 6usize],
+    pub interrupts: [InterruptSource; IRQMASK.len()],
     #[doc(alias = "clk")]
     pub clock: NonNull<Clock>,
     #[doc(alias = "migrate_clk")]
@@ -139,7 +140,8 @@ impl PL011State {
     unsafe fn init(&mut self) {
         const CLK_NAME: &CStr = c_str!("clk");
 
-        let dev = addr_of_mut!(*self).cast::<DeviceState>();
+        let sbd = unsafe { &mut *(addr_of_mut!(*self).cast::<SysBusDevice>()) };
+
         // SAFETY:
         //
         // self and self.iomem are guaranteed to be valid at this point since callers
@@ -153,12 +155,15 @@ unsafe fn init(&mut self) {
                 Self::TYPE_INFO.name,
                 0x1000,
             );
-            let sbd = addr_of_mut!(*self).cast::<SysBusDevice>();
             sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
-            for irq in self.interrupts.iter_mut() {
-                sysbus_init_irq(sbd, irq);
-            }
         }
+
+        for irq in self.interrupts.iter() {
+            sbd.init_irq(irq);
+        }
+
+        let dev = addr_of_mut!(*self).cast::<DeviceState>();
+
         // SAFETY:
         //
         // self.clock is not initialized at this point; but since `NonNull<_>` is Copy,
@@ -498,10 +503,7 @@ pub fn put_fifo(&mut self, value: c_uint) {
     pub fn update(&self) {
         let flags = self.int_level & self.int_enabled;
         for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
-            // SAFETY: self.interrupts have been initialized in init().
-            unsafe {
-                qemu_set_irq(*irq, i32::from(flags & i != 0));
-            }
+            irq.set(flags & i != 0);
         }
     }
 
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index f8b4cd39a26..b927eb58c8e 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -20,8 +20,10 @@ _qemu_api_rs = static_library(
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
+      'src/irq.rs',
       'src/offset_of.rs',
       'src/prelude.rs',
+      'src/sysbus.rs',
       'src/vmstate.rs',
       'src/zeroable.rs',
     ],
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
new file mode 100644
index 00000000000..6258141bdf0
--- /dev/null
+++ b/rust/qemu-api/src/irq.rs
@@ -0,0 +1,91 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Bindings for interrupt sources
+
+use core::ptr;
+use std::{marker::PhantomData, os::raw::c_int};
+
+use crate::{
+    bindings::{qemu_set_irq, IRQState},
+    prelude::*,
+};
+
+/// Interrupt sources are used by devices to pass changes to a value (typically
+/// a boolean).  The interrupt sink is usually an interrupt controller or
+/// GPIO controller.
+///
+/// As far as devices are concerned, interrupt sources are always active-high:
+/// for example, `InterruptSource<bool>`'s [`raise`](InterruptSource::raise)
+/// method sends a `true` value to the sink.  If the guest has to see a
+/// different polarity, that change is performed by the board between the
+/// device and the interrupt controller.
+///
+/// Interrupts are implemented as a pointer to the interrupt "sink", which has
+/// type [`IRQState`].  A device exposes its source as a QOM link property using
+/// a function such as
+/// [`SysBusDevice::init_irq`](crate::sysbus::SysBusDevice::init_irq), and
+/// initially leaves the pointer to a NULL value, representing an unconnected
+/// interrupt. To connect it, whoever creates the device fills the pointer with
+/// the sink's `IRQState *`, for example using `sysbus_connect_irq`.  Because
+/// devices are generally shared objects, interrupt sources are an example of
+/// the interior mutability pattern.
+///
+/// Interrupt sources can only be triggered under the Big QEMU Lock; `BqlCell`
+/// allows access from whatever thread has it.
+#[derive(Debug)]
+#[repr(transparent)]
+pub struct InterruptSource<T = bool>
+where
+    c_int: From<T>,
+{
+    cell: BqlCell<*mut IRQState>,
+    _marker: PhantomData<T>,
+}
+
+impl InterruptSource<bool> {
+    /// Send a low (`false`) value to the interrupt sink.
+    pub fn lower(&self) {
+        self.set(false);
+    }
+
+    /// Send a high-low pulse to the interrupt sink.
+    pub fn pulse(&self) {
+        self.set(true);
+        self.set(false);
+    }
+
+    /// Send a high (`true`) value to the interrupt sink.
+    pub fn raise(&self) {
+        self.set(true);
+    }
+}
+
+impl<T> InterruptSource<T>
+where
+    c_int: From<T>,
+{
+    /// Send `level` to the interrupt sink.
+    pub fn set(&self, level: T) {
+        let ptr = self.cell.get();
+        // SAFETY: the pointer is retrieved under the BQL and remains valid
+        // until the BQL is released, which is after qemu_set_irq() is entered.
+        unsafe {
+            qemu_set_irq(ptr, level.into());
+        }
+    }
+
+    pub(crate) const fn as_ptr(&self) -> *mut *mut IRQState {
+        self.cell.as_ptr()
+    }
+}
+
+impl Default for InterruptSource {
+    fn default() -> Self {
+        InterruptSource {
+            cell: BqlCell::new(ptr::null_mut()),
+            _marker: PhantomData,
+        }
+    }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index e5956cd5eb6..0efbef47441 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -16,7 +16,9 @@
 pub mod cell;
 pub mod definitions;
 pub mod device_class;
+pub mod irq;
 pub mod offset_of;
+pub mod sysbus;
 pub mod vmstate;
 pub mod zeroable;
 
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
new file mode 100644
index 00000000000..4e192c75898
--- /dev/null
+++ b/rust/qemu-api/src/sysbus.rs
@@ -0,0 +1,27 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::ptr::addr_of;
+
+pub use bindings::{SysBusDevice, SysBusDeviceClass};
+
+use crate::{bindings, cell::bql_locked, irq::InterruptSource};
+
+impl SysBusDevice {
+    /// Return `self` cast to a mutable pointer, for use in calls to C code.
+    const fn as_mut_ptr(&self) -> *mut SysBusDevice {
+        addr_of!(*self) as *mut _
+    }
+
+    /// Expose an interrupt source outside the device as a qdev GPIO output.
+    /// Note that the ordering of calls to `init_irq` is important, since
+    /// whoever creates the sysbus device will refer to the interrupts with
+    /// a number that corresponds to the order of calls to `init_irq`.
+    pub fn init_irq(&self, irq: &InterruptSource) {
+        assert!(bql_locked());
+        unsafe {
+            bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr());
+        }
+    }
+}
-- 
2.47.1

[PULL 40/49] rust: add a bit operation module

[Paolo Bonzini]

The bindgen supports `static inline` function binding since v0.64.0 as
an experimental feature (`--wrap-static-fns`), and stabilizes it after
v0.70.0.

But the oldest version of bindgen supported by QEMU is v0.60.1, so
there's no way to generate the binding for deposit64() which is `static
inline` (in include/qemu/bitops.h).

Instead, implement it by hand in Rust and make it available for all
unsigned types through an IntegerExt trait. Since it only involves bit
operations, the Rust version of the code is almost identical to the
original C version, but it applies to more types than just u64.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Co-authored-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build    |   1 +
 rust/qemu-api/src/bitops.rs  | 119 +++++++++++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs     |   1 +
 rust/qemu-api/src/prelude.rs |   2 +
 4 files changed, 123 insertions(+)
 create mode 100644 rust/qemu-api/src/bitops.rs

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b927eb58c8e..adcee661150 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -16,6 +16,7 @@ _qemu_api_rs = static_library(
     [
       'src/lib.rs',
       'src/bindings.rs',
+      'src/bitops.rs',
       'src/cell.rs',
       'src/c_str.rs',
       'src/definitions.rs',
diff --git a/rust/qemu-api/src/bitops.rs b/rust/qemu-api/src/bitops.rs
new file mode 100644
index 00000000000..023ec1a9983
--- /dev/null
+++ b/rust/qemu-api/src/bitops.rs
@@ -0,0 +1,119 @@
+// Copyright (C) 2024 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! This module provides bit operation extensions to integer types.
+//! It is usually included via the `qemu_api` prelude.
+
+use std::ops::{
+    Add, AddAssign, BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Div, DivAssign,
+    Mul, MulAssign, Not, Rem, RemAssign, Shl, ShlAssign, Shr, ShrAssign,
+};
+
+/// Trait for extensions to integer types
+pub trait IntegerExt:
+    Add<Self, Output = Self> + AddAssign<Self> +
+    BitAnd<Self, Output = Self> + BitAndAssign<Self> +
+    BitOr<Self, Output = Self> + BitOrAssign<Self> +
+    BitXor<Self, Output = Self> + BitXorAssign<Self> +
+    Copy +
+    Div<Self, Output = Self> + DivAssign<Self> +
+    Eq +
+    Mul<Self, Output = Self> + MulAssign<Self> +
+    Not<Output = Self> + Ord + PartialOrd +
+    Rem<Self, Output = Self> + RemAssign<Self> +
+    Shl<Self, Output = Self> + ShlAssign<Self> +
+    Shl<u32, Output = Self> + ShlAssign<u32> + // add more as needed
+    Shr<Self, Output = Self> + ShrAssign<Self> +
+    Shr<u32, Output = Self> + ShrAssign<u32> // add more as needed
+{
+    const BITS: u32;
+    const MAX: Self;
+    const MIN: Self;
+    const ONE: Self;
+    const ZERO: Self;
+
+    #[inline]
+    #[must_use]
+    fn bit(start: u32) -> Self
+    {
+        debug_assert!(start < Self::BITS);
+
+        Self::ONE << start
+    }
+
+    #[inline]
+    #[must_use]
+    fn mask(start: u32, length: u32) -> Self
+    {
+        /* FIXME: Implement a more elegant check with error handling support? */
+        debug_assert!(start < Self::BITS && length > 0 && length <= Self::BITS - start);
+
+        (Self::MAX >> (Self::BITS - length)) << start
+    }
+
+    #[inline]
+    #[must_use]
+    fn deposit<U: IntegerExt>(self, start: u32, length: u32,
+                          fieldval: U) -> Self
+        where Self: From<U>
+    {
+        debug_assert!(length <= U::BITS);
+
+        let mask = Self::mask(start, length);
+        (self & !mask) | ((Self::from(fieldval) << start) & mask)
+    }
+
+    #[inline]
+    #[must_use]
+    fn extract(self, start: u32, length: u32) -> Self
+    {
+        let mask = Self::mask(start, length);
+        (self & mask) >> start
+    }
+}
+
+macro_rules! impl_num_ext {
+    ($type:ty) => {
+        impl IntegerExt for $type {
+            const BITS: u32 = <$type>::BITS;
+            const MAX: Self = <$type>::MAX;
+            const MIN: Self = <$type>::MIN;
+            const ONE: Self = 1;
+            const ZERO: Self = 0;
+        }
+    };
+}
+
+impl_num_ext!(u8);
+impl_num_ext!(u16);
+impl_num_ext!(u32);
+impl_num_ext!(u64);
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_deposit() {
+        assert_eq!(15u32.deposit(8, 8, 1u32), 256 + 15);
+        assert_eq!(15u32.deposit(8, 1, 255u8), 256 + 15);
+    }
+
+    #[test]
+    fn test_extract() {
+        assert_eq!(15u32.extract(2, 4), 3);
+    }
+
+    #[test]
+    fn test_bit() {
+        assert_eq!(u8::bit(7), 128);
+        assert_eq!(u32::bit(16), 0x10000);
+    }
+
+    #[test]
+    fn test_mask() {
+        assert_eq!(u8::mask(7, 1), 128);
+        assert_eq!(u32::mask(8, 8), 0xff00);
+    }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 0efbef47441..9e007e16354 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -12,6 +12,7 @@
 #[rustfmt::skip]
 pub mod prelude;
 
+pub mod bitops;
 pub mod c_str;
 pub mod cell;
 pub mod definitions;
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index dfaddbd062a..a39e228babf 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -2,5 +2,7 @@
 // Author(s): Paolo Bonzini <pbonzini@redhat.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+pub use crate::bitops::IntegerExt;
+
 pub use crate::cell::BqlCell;
 pub use crate::cell::BqlRefCell;
-- 
2.47.1

[PULL 41/49] rust: qom: add default definitions for ObjectImpl

[Paolo Bonzini]

Remove a bunch of duplicate const definitions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 6 ------
 rust/qemu-api/src/definitions.rs | 8 ++++----
 rust/qemu-api/tests/tests.rs     | 4 ----
 3 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index c5c8c463d37..3d173ae816d 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -109,10 +109,7 @@ impl ObjectImpl for PL011State {
     const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE);
-    const ABSTRACT: bool = false;
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_init);
-    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
-    const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
 }
 
 #[repr(C)]
@@ -666,8 +663,5 @@ impl ObjectImpl for PL011Luminary {
     const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
-    const ABSTRACT: bool = false;
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_luminary_init);
-    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
-    const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
 }
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 26597934bbd..92b3c6f9118 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -14,10 +14,10 @@ pub trait ObjectImpl {
     const TYPE_INFO: TypeInfo;
     const TYPE_NAME: &'static CStr;
     const PARENT_TYPE_NAME: Option<&'static CStr>;
-    const ABSTRACT: bool;
-    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)>;
-    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)>;
-    const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)>;
+    const ABSTRACT: bool = false;
+    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+    const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
 }
 
 pub trait Class {
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 925f5a3c77b..f793ff26e5d 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -58,10 +58,6 @@ impl ObjectImpl for DummyState {
         const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
         const TYPE_NAME: &'static CStr = c_str!("dummy");
         const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
-        const ABSTRACT: bool = false;
-        const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
-        const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
-        const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
     }
 
     impl Class for DummyClass {
-- 
2.47.1

[PULL 42/49] rust: qom: rename Class trait to ClassInitImpl

[Paolo Bonzini]

While at it, document it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs |  4 ++--
 rust/qemu-api/src/definitions.rs | 25 ++++++++++++++++++++++---
 rust/qemu-api/tests/tests.rs     |  4 ++--
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 3d173ae816d..bd12067aaf0 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -117,7 +117,7 @@ pub struct PL011Class {
     _inner: [u8; 0],
 }
 
-impl qemu_api::definitions::Class for PL011Class {
+impl qemu_api::definitions::ClassInitImpl for PL011Class {
     const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
         Some(crate::device_class::pl011_class_init);
     const CLASS_BASE_INIT: Option<
@@ -650,7 +650,7 @@ pub struct PL011LuminaryClass {
     }
 }
 
-impl qemu_api::definitions::Class for PL011LuminaryClass {
+impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass {
     const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
         None;
     const CLASS_BASE_INIT: Option<
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 92b3c6f9118..3291f4242ce 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -20,8 +20,27 @@ pub trait ObjectImpl {
     const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
 }
 
-pub trait Class {
+/// Trait used to fill in a class struct.
+///
+/// Each QOM class that has virtual methods describes them in a
+/// _class struct_.  Class structs include a parent field corresponding
+/// to the vtable of the parent class, all the way up to [`ObjectClass`].
+/// Each QOM type has one such class struct.
+///
+/// The Rust implementation of methods will usually come from a trait
+/// like [`ObjectImpl`].
+pub trait ClassInitImpl {
+    /// Function that is called after all parent class initialization
+    /// has occurred.  On entry, the virtual method pointers are set to
+    /// the default values coming from the parent classes; the function
+    /// can change them to override virtual methods of a parent class.
     const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)>;
+
+    /// Called on descendent classes after all parent class initialization
+    /// has occurred, but before the class itself is initialized.  This
+    /// is only useful if a class is not a leaf, and can be used to undo
+    /// the effects of copying the contents of the parent's class struct
+    /// to the descendants.
     const CLASS_BASE_INIT: Option<
         unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
     >;
@@ -82,8 +101,8 @@ macro_rules! type_info {
             instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE,
             abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT,
             class_size:  ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(),
-            class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_INIT,
-            class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_BASE_INIT,
+            class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT,
+            class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT,
             class_data: ::core::ptr::null_mut(),
             interfaces: ::core::ptr::null_mut(),
         };
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index f793ff26e5d..704c63c846f 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -7,7 +7,7 @@
 use qemu_api::{
     bindings::*,
     c_str, declare_properties, define_property,
-    definitions::{Class, ObjectImpl},
+    definitions::{ClassInitImpl, ObjectImpl},
     device_class, device_class_init,
     zeroable::Zeroable,
 };
@@ -60,7 +60,7 @@ impl ObjectImpl for DummyState {
         const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
     }
 
-    impl Class for DummyClass {
+    impl ClassInitImpl for DummyClass {
         const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
             Some(dummy_class_init);
         const CLASS_BASE_INIT: Option<
-- 
2.47.1

[PULL 43/49] rust: qom: convert type_info! macro to an associated const

[Paolo Bonzini]

type_info! is only used in the definition of ObjectImpl::TYPE_INFO, and
in fact in all of them.  Pull type_info!'s definition into the ObjectImpl
trait, thus simplifying the external interface of qemu_api::definitions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs |  6 ++--
 rust/qemu-api/src/definitions.rs | 50 ++++++++++++++------------------
 rust/qemu-api/tests/tests.rs     |  1 -
 3 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bd12067aaf0..bcb146c24d6 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -106,7 +106,6 @@ pub struct PL011State {
 
 impl ObjectImpl for PL011State {
     type Class = PL011Class;
-    const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE);
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_init);
@@ -149,7 +148,7 @@ unsafe fn init(&mut self) {
                 addr_of_mut!(*self).cast::<Object>(),
                 &PL011_OPS,
                 addr_of_mut!(*self).cast::<c_void>(),
-                Self::TYPE_INFO.name,
+                Self::TYPE_NAME.as_ptr(),
                 0x1000,
             );
             sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
@@ -598,7 +597,7 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
     chr: *mut Chardev,
 ) -> *mut DeviceState {
     unsafe {
-        let dev: *mut DeviceState = qdev_new(PL011State::TYPE_INFO.name);
+        let dev: *mut DeviceState = qdev_new(PL011State::TYPE_NAME.as_ptr());
         let sysbus: *mut SysBusDevice = dev.cast::<SysBusDevice>();
 
         qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
@@ -660,7 +659,6 @@ impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass {
 
 impl ObjectImpl for PL011Luminary {
     type Class = PL011LuminaryClass;
-    const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_luminary_init);
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 3291f4242ce..6ecfaf51b09 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -9,15 +9,34 @@
 use crate::bindings::{Object, ObjectClass, TypeInfo};
 
 /// Trait a type must implement to be registered with QEMU.
-pub trait ObjectImpl {
-    type Class;
-    const TYPE_INFO: TypeInfo;
+pub trait ObjectImpl: Sized {
+    type Class: ClassInitImpl;
     const TYPE_NAME: &'static CStr;
     const PARENT_TYPE_NAME: Option<&'static CStr>;
     const ABSTRACT: bool = false;
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
     const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
     const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+
+    const TYPE_INFO: TypeInfo = TypeInfo {
+        name: Self::TYPE_NAME.as_ptr(),
+        parent: if let Some(pname) = Self::PARENT_TYPE_NAME {
+            pname.as_ptr()
+        } else {
+            core::ptr::null_mut()
+        },
+        instance_size: core::mem::size_of::<Self>(),
+        instance_align: core::mem::align_of::<Self>(),
+        instance_init: Self::INSTANCE_INIT,
+        instance_post_init: Self::INSTANCE_POST_INIT,
+        instance_finalize: Self::INSTANCE_FINALIZE,
+        abstract_: Self::ABSTRACT,
+        class_size: core::mem::size_of::<Self::Class>(),
+        class_init: <Self::Class as ClassInitImpl>::CLASS_INIT,
+        class_base_init: <Self::Class as ClassInitImpl>::CLASS_BASE_INIT,
+        class_data: core::ptr::null_mut(),
+        interfaces: core::ptr::null_mut(),
+    };
 }
 
 /// Trait used to fill in a class struct.
@@ -83,28 +102,3 @@ extern "C" fn ctor_fn() {
         }
     };
 }
-
-#[macro_export]
-macro_rules! type_info {
-    ($t:ty) => {
-        $crate::bindings::TypeInfo {
-            name: <$t as $crate::definitions::ObjectImpl>::TYPE_NAME.as_ptr(),
-            parent: if let Some(pname) = <$t as $crate::definitions::ObjectImpl>::PARENT_TYPE_NAME {
-                pname.as_ptr()
-            } else {
-                ::core::ptr::null_mut()
-            },
-            instance_size: ::core::mem::size_of::<$t>(),
-            instance_align: ::core::mem::align_of::<$t>(),
-            instance_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_INIT,
-            instance_post_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_POST_INIT,
-            instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE,
-            abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT,
-            class_size:  ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(),
-            class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_INIT,
-            class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::ClassInitImpl>::CLASS_BASE_INIT,
-            class_data: ::core::ptr::null_mut(),
-            interfaces: ::core::ptr::null_mut(),
-        };
-    }
-}
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 704c63c846f..7f9df348b00 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -55,7 +55,6 @@ pub struct DummyClass {
 
     impl ObjectImpl for DummyState {
         type Class = DummyClass;
-        const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
         const TYPE_NAME: &'static CStr = c_str!("dummy");
         const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
     }
-- 
2.47.1

[PULL 44/49] rust: qom: move ClassInitImpl to the instance side

[Paolo Bonzini]

Put all traits on the instance struct, which makes it possible to reuse
class structs if no new virtual methods or class fields are added.
This is almost always the case for devices (because they are leaf
classes), which is the primary use case for Rust.

This is also simpler: soon we will find the implemented methods without
macros, and this removes the need to go from the class struct to the
instance struct to find the implementation of the *Impl traits.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 4 ++--
 rust/qemu-api/src/definitions.rs | 8 ++++----
 rust/qemu-api/tests/tests.rs     | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bcb146c24d6..2384d4bcb95 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -116,7 +116,7 @@ pub struct PL011Class {
     _inner: [u8; 0],
 }
 
-impl qemu_api::definitions::ClassInitImpl for PL011Class {
+impl qemu_api::definitions::ClassInitImpl for PL011State {
     const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
         Some(crate::device_class::pl011_class_init);
     const CLASS_BASE_INIT: Option<
@@ -649,7 +649,7 @@ pub struct PL011LuminaryClass {
     }
 }
 
-impl qemu_api::definitions::ClassInitImpl for PL011LuminaryClass {
+impl qemu_api::definitions::ClassInitImpl for PL011Luminary {
     const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
         None;
     const CLASS_BASE_INIT: Option<
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 6ecfaf51b09..487712611f6 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -9,8 +9,8 @@
 use crate::bindings::{Object, ObjectClass, TypeInfo};
 
 /// Trait a type must implement to be registered with QEMU.
-pub trait ObjectImpl: Sized {
-    type Class: ClassInitImpl;
+pub trait ObjectImpl: ClassInitImpl + Sized {
+    type Class;
     const TYPE_NAME: &'static CStr;
     const PARENT_TYPE_NAME: Option<&'static CStr>;
     const ABSTRACT: bool = false;
@@ -32,8 +32,8 @@ pub trait ObjectImpl: Sized {
         instance_finalize: Self::INSTANCE_FINALIZE,
         abstract_: Self::ABSTRACT,
         class_size: core::mem::size_of::<Self::Class>(),
-        class_init: <Self::Class as ClassInitImpl>::CLASS_INIT,
-        class_base_init: <Self::Class as ClassInitImpl>::CLASS_BASE_INIT,
+        class_init: <Self as ClassInitImpl>::CLASS_INIT,
+        class_base_init: <Self as ClassInitImpl>::CLASS_BASE_INIT,
         class_data: core::ptr::null_mut(),
         interfaces: core::ptr::null_mut(),
     };
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 7f9df348b00..fd0c979121c 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -59,7 +59,7 @@ impl ObjectImpl for DummyState {
         const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
     }
 
-    impl ClassInitImpl for DummyClass {
+    impl ClassInitImpl for DummyState {
         const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
             Some(dummy_class_init);
         const CLASS_BASE_INIT: Option<
-- 
2.47.1

[PULL 45/49] rust: qdev: move device_class_init! body to generic function, ClassInitImpl implementation to macro

[Paolo Bonzini]

Use a trait to access the former parameters to device_class_init!.
This allows hiding the details of the class_init implementation behind
a generic function and makes higher-level functionality available from
qemu_api.

The implementation of ClassInitImpl is then the same for all devices and
is easily macroized.  Later on, we can remove the need to implement
ClassInitImpl by hand for all device types, and stop making
rust_device_class_init<>() public.

While at it, document the members of DeviceImpl.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       | 34 +++++-----
 rust/hw/char/pl011/src/device_class.rs |  8 ---
 rust/qemu-api/src/device_class.rs      | 87 +++++++++++++++++++++-----
 rust/qemu-api/tests/tests.rs           | 30 ++++-----
 4 files changed, 103 insertions(+), 56 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 2384d4bcb95..28b1924337d 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -12,11 +12,13 @@
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
-    device_class::TYPE_SYS_BUS_DEVICE,
+    device_class::{DeviceImpl, TYPE_SYS_BUS_DEVICE},
+    impl_device_class,
     irq::InterruptSource,
 };
 
 use crate::{
+    device_class,
     memory_ops::PL011_OPS,
     registers::{self, Interrupt},
     RegisterOffset,
@@ -116,14 +118,20 @@ pub struct PL011Class {
     _inner: [u8; 0],
 }
 
-impl qemu_api::definitions::ClassInitImpl for PL011State {
-    const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
-        Some(crate::device_class::pl011_class_init);
-    const CLASS_BASE_INIT: Option<
-        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
-    > = None;
+impl DeviceImpl for PL011State {
+    fn properties() -> &'static [Property] {
+        &device_class::PL011_PROPERTIES
+    }
+    fn vmsd() -> Option<&'static VMStateDescription> {
+        Some(&device_class::VMSTATE_PL011)
+    }
+    const REALIZE: Option<unsafe extern "C" fn(*mut DeviceState, *mut *mut Error)> =
+        Some(device_class::pl011_realize);
+    const RESET: Option<unsafe extern "C" fn(*mut DeviceState)> = Some(device_class::pl011_reset);
 }
 
+impl_device_class!(PL011State);
+
 impl PL011State {
     /// Initializes a pre-allocated, unitialized instance of `PL011State`.
     ///
@@ -649,17 +657,13 @@ pub struct PL011LuminaryClass {
     }
 }
 
-impl qemu_api::definitions::ClassInitImpl for PL011Luminary {
-    const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
-        None;
-    const CLASS_BASE_INIT: Option<
-        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
-    > = None;
-}
-
 impl ObjectImpl for PL011Luminary {
     type Class = PL011LuminaryClass;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
     const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_luminary_init);
 }
+
+impl DeviceImpl for PL011Luminary {}
+
+impl_device_class!(PL011Luminary);
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index a707fde1384..c61b6bb0258 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -93,14 +93,6 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
     ),
 }
 
-qemu_api::device_class_init! {
-    pl011_class_init,
-    props => PL011_PROPERTIES,
-    realize_fn => Some(pl011_realize),
-    legacy_reset_fn => Some(pl011_reset),
-    vmsd => VMSTATE_PL011,
-}
-
 /// # Safety
 ///
 /// We expect the FFI user of this function to pass a valid pointer, that has
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 922bbce1bb8..f683f94f2a5 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -2,25 +2,80 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::ffi::CStr;
+use std::{ffi::CStr, os::raw::c_void};
 
-use crate::bindings;
+use crate::{
+    bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription},
+    zeroable::Zeroable,
+};
+
+/// Trait providing the contents of [`DeviceClass`].
+pub trait DeviceImpl {
+    /// _Realization_ is the second stage of device creation. It contains
+    /// all operations that depend on device properties and can fail (note:
+    /// this is not yet supported for Rust devices).
+    ///
+    /// If not `None`, the parent class's `realize` method is overridden
+    /// with the function pointed to by `REALIZE`.
+    const REALIZE: Option<unsafe extern "C" fn(*mut DeviceState, *mut *mut Error)> = None;
+
+    /// If not `None`, the parent class's `reset` method is overridden
+    /// with the function pointed to by `RESET`.
+    ///
+    /// Rust does not yet support the three-phase reset protocol; this is
+    /// usually okay for leaf classes.
+    const RESET: Option<unsafe extern "C" fn(dev: *mut DeviceState)> = None;
+
+    /// An array providing the properties that the user can set on the
+    /// device.  Not a `const` because referencing statics in constants
+    /// is unstable until Rust 1.83.0.
+    fn properties() -> &'static [Property] {
+        &[Zeroable::ZERO; 1]
+    }
+
+    /// A `VMStateDescription` providing the migration format for the device
+    /// Not a `const` because referencing statics in constants is unstable
+    /// until Rust 1.83.0.
+    fn vmsd() -> Option<&'static VMStateDescription> {
+        None
+    }
+}
+
+/// # Safety
+///
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `DeviceClass`, because `T` implements
+/// `DeviceImpl`.
+pub unsafe extern "C" fn rust_device_class_init<T: DeviceImpl>(
+    klass: *mut ObjectClass,
+    _: *mut c_void,
+) {
+    let mut dc = ::core::ptr::NonNull::new(klass.cast::<DeviceClass>()).unwrap();
+    unsafe {
+        let dc = dc.as_mut();
+        if let Some(realize_fn) = <T as DeviceImpl>::REALIZE {
+            dc.realize = Some(realize_fn);
+        }
+        if let Some(reset_fn) = <T as DeviceImpl>::RESET {
+            bindings::device_class_set_legacy_reset(dc, Some(reset_fn));
+        }
+        if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
+            dc.vmsd = vmsd;
+        }
+        bindings::device_class_set_props(dc, <T as DeviceImpl>::properties().as_ptr());
+    }
+}
 
 #[macro_export]
-macro_rules! device_class_init {
-    ($func:ident, props => $props:ident, realize_fn => $realize_fn:expr, legacy_reset_fn => $legacy_reset_fn:expr, vmsd => $vmsd:ident$(,)*) => {
-        pub unsafe extern "C" fn $func(
-            klass: *mut $crate::bindings::ObjectClass,
-            _: *mut ::std::os::raw::c_void,
-        ) {
-            let mut dc =
-                ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap();
-            unsafe {
-                dc.as_mut().realize = $realize_fn;
-                dc.as_mut().vmsd = &$vmsd;
-                $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn);
-                $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_ptr());
-            }
+macro_rules! impl_device_class {
+    ($type:ty) => {
+        impl $crate::definitions::ClassInitImpl for $type {
+            const CLASS_INIT: Option<
+                unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void),
+            > = Some($crate::device_class::rust_device_class_init::<$type>);
+            const CLASS_BASE_INIT: Option<
+                unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut ::std::os::raw::c_void),
+            > = None;
         }
     };
 }
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index fd0c979121c..b8b12a40422 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -2,13 +2,14 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::{ffi::CStr, os::raw::c_void};
+use std::ffi::CStr;
 
 use qemu_api::{
     bindings::*,
     c_str, declare_properties, define_property,
-    definitions::{ClassInitImpl, ObjectImpl},
-    device_class, device_class_init,
+    definitions::ObjectImpl,
+    device_class::{self, DeviceImpl},
+    impl_device_class,
     zeroable::Zeroable,
 };
 
@@ -45,28 +46,23 @@ pub struct DummyClass {
             ),
     }
 
-    device_class_init! {
-        dummy_class_init,
-        props => DUMMY_PROPERTIES,
-        realize_fn => None,
-        legacy_reset_fn => None,
-        vmsd => VMSTATE,
-    }
-
     impl ObjectImpl for DummyState {
         type Class = DummyClass;
         const TYPE_NAME: &'static CStr = c_str!("dummy");
         const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
     }
 
-    impl ClassInitImpl for DummyState {
-        const CLASS_INIT: Option<unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void)> =
-            Some(dummy_class_init);
-        const CLASS_BASE_INIT: Option<
-            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
-        > = None;
+    impl DeviceImpl for DummyState {
+        fn properties() -> &'static [Property] {
+            &DUMMY_PROPERTIES
+        }
+        fn vmsd() -> Option<&'static VMStateDescription> {
+            Some(&VMSTATE)
+        }
     }
 
+    impl_device_class!(DummyState);
+
     unsafe {
         module_call_init(module_init_type::MODULE_INIT_QOM);
         object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast());
-- 
2.47.1

[PULL 46/49] rust: qdev: move bridge for realize and reset functions out of pl011

[Paolo Bonzini]

Allow the DeviceImpl trait to expose safe Rust functions.
rust_device_class_init<> adds thunks around the functions
in DeviceImpl.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       |  5 ++--
 rust/hw/char/pl011/src/device_class.rs | 26 -------------------
 rust/qemu-api/src/definitions.rs       |  2 +-
 rust/qemu-api/src/device_class.rs      | 36 +++++++++++++++++++++-----
 4 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 28b1924337d..56403c36609 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -125,9 +125,8 @@ fn properties() -> &'static [Property] {
     fn vmsd() -> Option<&'static VMStateDescription> {
         Some(&device_class::VMSTATE_PL011)
     }
-    const REALIZE: Option<unsafe extern "C" fn(*mut DeviceState, *mut *mut Error)> =
-        Some(device_class::pl011_realize);
-    const RESET: Option<unsafe extern "C" fn(*mut DeviceState)> = Some(device_class::pl011_reset);
+    const REALIZE: Option<fn(&mut Self)> = Some(Self::realize);
+    const RESET: Option<fn(&mut Self)> = Some(Self::reset);
 }
 
 impl_device_class!(PL011State);
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index c61b6bb0258..975c3d42be7 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -92,29 +92,3 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
         default = true
     ),
 }
-
-/// # Safety
-///
-/// We expect the FFI user of this function to pass a valid pointer, that has
-/// the same size as [`PL011State`]. We also expect the device is
-/// readable/writeable from one thread at any time.
-pub unsafe extern "C" fn pl011_realize(dev: *mut DeviceState, _errp: *mut *mut Error) {
-    unsafe {
-        assert!(!dev.is_null());
-        let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
-        state.as_mut().realize();
-    }
-}
-
-/// # Safety
-///
-/// We expect the FFI user of this function to pass a valid pointer, that has
-/// the same size as [`PL011State`]. We also expect the device is
-/// readable/writeable from one thread at any time.
-pub unsafe extern "C" fn pl011_reset(dev: *mut DeviceState) {
-    unsafe {
-        assert!(!dev.is_null());
-        let mut state = NonNull::new_unchecked(dev.cast::<PL011State>());
-        state.as_mut().reset();
-    }
-}
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 487712611f6..0467e6290e0 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -47,7 +47,7 @@ pub trait ObjectImpl: ClassInitImpl + Sized {
 /// Each QOM type has one such class struct.
 ///
 /// The Rust implementation of methods will usually come from a trait
-/// like [`ObjectImpl`].
+/// like [`ObjectImpl`] or [`DeviceImpl`](crate::device_class::DeviceImpl).
 pub trait ClassInitImpl {
     /// Function that is called after all parent class initialization
     /// has occurred.  On entry, the virtual method pointers are set to
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index f683f94f2a5..f25904be4f6 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -17,14 +17,14 @@ pub trait DeviceImpl {
     ///
     /// If not `None`, the parent class's `realize` method is overridden
     /// with the function pointed to by `REALIZE`.
-    const REALIZE: Option<unsafe extern "C" fn(*mut DeviceState, *mut *mut Error)> = None;
+    const REALIZE: Option<fn(&mut Self)> = None;
 
     /// If not `None`, the parent class's `reset` method is overridden
     /// with the function pointed to by `RESET`.
     ///
     /// Rust does not yet support the three-phase reset protocol; this is
     /// usually okay for leaf classes.
-    const RESET: Option<unsafe extern "C" fn(dev: *mut DeviceState)> = None;
+    const RESET: Option<fn(&mut Self)> = None;
 
     /// An array providing the properties that the user can set on the
     /// device.  Not a `const` because referencing statics in constants
@@ -41,6 +41,30 @@ fn vmsd() -> Option<&'static VMStateDescription> {
     }
 }
 
+/// # Safety
+///
+/// This function is only called through the QOM machinery and
+/// the `impl_device_class!` macro.
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `T`. We also expect the device is
+/// readable/writeable from one thread at any time.
+unsafe extern "C" fn rust_realize_fn<T: DeviceImpl>(dev: *mut DeviceState, _errp: *mut *mut Error) {
+    assert!(!dev.is_null());
+    let state = dev.cast::<T>();
+    T::REALIZE.unwrap()(unsafe { &mut *state });
+}
+
+/// # Safety
+///
+/// We expect the FFI user of this function to pass a valid pointer that
+/// can be downcasted to type `T`. We also expect the device is
+/// readable/writeable from one thread at any time.
+unsafe extern "C" fn rust_reset_fn<T: DeviceImpl>(dev: *mut DeviceState) {
+    assert!(!dev.is_null());
+    let state = dev.cast::<T>();
+    T::RESET.unwrap()(unsafe { &mut *state });
+}
+
 /// # Safety
 ///
 /// We expect the FFI user of this function to pass a valid pointer that
@@ -53,11 +77,11 @@ fn vmsd() -> Option<&'static VMStateDescription> {
     let mut dc = ::core::ptr::NonNull::new(klass.cast::<DeviceClass>()).unwrap();
     unsafe {
         let dc = dc.as_mut();
-        if let Some(realize_fn) = <T as DeviceImpl>::REALIZE {
-            dc.realize = Some(realize_fn);
+        if <T as DeviceImpl>::REALIZE.is_some() {
+            dc.realize = Some(rust_realize_fn::<T>);
         }
-        if let Some(reset_fn) = <T as DeviceImpl>::RESET {
-            bindings::device_class_set_legacy_reset(dc, Some(reset_fn));
+        if <T as DeviceImpl>::RESET.is_some() {
+            bindings::device_class_set_legacy_reset(dc, Some(rust_reset_fn::<T>));
         }
         if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
             dc.vmsd = vmsd;
-- 
2.47.1

[PULL 47/49] rust: qom: move bridge for TypeInfo functions out of pl011

[Paolo Bonzini]

Allow the ObjectImpl trait to expose Rust functions that avoid raw
pointers (though INSTANCE_INIT for example is still unsafe).
ObjectImpl::TYPE_INFO adds thunks around the functions in
ObjectImpl.

While at it, document `TypeInfo`.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 40 +++++++--------------
 rust/qemu-api/src/definitions.rs | 61 +++++++++++++++++++++++++++++---
 2 files changed, 69 insertions(+), 32 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 56403c36609..b9f8fb134b5 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -110,7 +110,7 @@ impl ObjectImpl for PL011State {
     type Class = PL011Class;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE);
-    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_init);
+    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
 #[repr(C)]
@@ -615,19 +615,6 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
     }
 }
 
-/// # Safety
-///
-/// We expect the FFI user of this function to pass a valid pointer, that has
-/// the same size as [`PL011State`]. We also expect the device is
-/// readable/writeable from one thread at any time.
-pub unsafe extern "C" fn pl011_init(obj: *mut Object) {
-    unsafe {
-        debug_assert!(!obj.is_null());
-        let mut state = NonNull::new_unchecked(obj.cast::<PL011State>());
-        state.as_mut().init();
-    }
-}
-
 #[repr(C)]
 #[derive(Debug, qemu_api_macros::Object)]
 /// PL011 Luminary device model.
@@ -640,19 +627,16 @@ pub struct PL011LuminaryClass {
     _inner: [u8; 0],
 }
 
-/// Initializes a pre-allocated, unitialized instance of `PL011Luminary`.
-///
-/// # Safety
-///
-/// We expect the FFI user of this function to pass a valid pointer, that has
-/// the same size as [`PL011Luminary`]. We also expect the device is
-/// readable/writeable from one thread at any time.
-pub unsafe extern "C" fn pl011_luminary_init(obj: *mut Object) {
-    unsafe {
-        debug_assert!(!obj.is_null());
-        let mut state = NonNull::new_unchecked(obj.cast::<PL011Luminary>());
-        let state = state.as_mut();
-        state.parent_obj.device_id = DeviceId::Luminary;
+impl PL011Luminary {
+    /// Initializes a pre-allocated, unitialized instance of `PL011Luminary`.
+    ///
+    /// # Safety
+    ///
+    /// We expect the FFI user of this function to pass a valid pointer, that
+    /// has the same size as [`PL011Luminary`]. We also expect the device is
+    /// readable/writeable from one thread at any time.
+    unsafe fn init(&mut self) {
+        self.parent_obj.device_id = DeviceId::Luminary;
     }
 }
 
@@ -660,7 +644,7 @@ impl ObjectImpl for PL011Luminary {
     type Class = PL011LuminaryClass;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
     const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
-    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = Some(pl011_luminary_init);
+    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
 impl DeviceImpl for PL011Luminary {}
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 0467e6290e0..f2970758986 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -8,16 +8,63 @@
 
 use crate::bindings::{Object, ObjectClass, TypeInfo};
 
+unsafe extern "C" fn rust_instance_init<T: ObjectImpl>(obj: *mut Object) {
+    // SAFETY: obj is an instance of T, since rust_instance_init<T>
+    // is called from QOM core as the instance_init function
+    // for class T
+    unsafe { T::INSTANCE_INIT.unwrap()(&mut *obj.cast::<T>()) }
+}
+
+unsafe extern "C" fn rust_instance_post_init<T: ObjectImpl>(obj: *mut Object) {
+    // SAFETY: obj is an instance of T, since rust_instance_post_init<T>
+    // is called from QOM core as the instance_post_init function
+    // for class T
+    //
+    // FIXME: it's not really guaranteed that there are no backpointers to
+    // obj; it's quite possible that they have been created by instance_init().
+    // The receiver should be &self, not &mut self.
+    T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::<T>() })
+}
+
 /// Trait a type must implement to be registered with QEMU.
+///
+/// # Safety
+///
+/// - the struct must be `#[repr(C)]`
+///
+/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is
+///   automatic if the class is defined via `ObjectImpl`).
+///
+/// - the first field of the struct must be of the instance struct corresponding
+///   to the superclass declared as `PARENT_TYPE_NAME`
 pub trait ObjectImpl: ClassInitImpl + Sized {
+    /// The QOM class object corresponding to this struct.  Not used yet.
     type Class;
+
+    /// The name of the type, which can be passed to `object_new()` to
+    /// generate an instance of this type.
     const TYPE_NAME: &'static CStr;
+
+    /// The parent of the type.  This should match the first field of
+    /// the struct that implements `ObjectImpl`:
     const PARENT_TYPE_NAME: Option<&'static CStr>;
+
+    /// Whether the object can be instantiated
     const ABSTRACT: bool = false;
-    const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
-    const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
     const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
 
+    /// Function that is called to initialize an object.  The parent class will
+    /// have already been initialized so the type is only responsible for
+    /// initializing its own members.
+    ///
+    /// FIXME: The argument is not really a valid reference. `&mut
+    /// MaybeUninit<Self>` would be a better description.
+    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = None;
+
+    /// Function that is called to finish initialization of an object, once
+    /// `INSTANCE_INIT` functions have been called.
+    const INSTANCE_POST_INIT: Option<fn(&mut Self)> = None;
+
     const TYPE_INFO: TypeInfo = TypeInfo {
         name: Self::TYPE_NAME.as_ptr(),
         parent: if let Some(pname) = Self::PARENT_TYPE_NAME {
@@ -27,8 +74,14 @@ pub trait ObjectImpl: ClassInitImpl + Sized {
         },
         instance_size: core::mem::size_of::<Self>(),
         instance_align: core::mem::align_of::<Self>(),
-        instance_init: Self::INSTANCE_INIT,
-        instance_post_init: Self::INSTANCE_POST_INIT,
+        instance_init: match Self::INSTANCE_INIT {
+            None => None,
+            Some(_) => Some(rust_instance_init::<Self>),
+        },
+        instance_post_init: match Self::INSTANCE_POST_INIT {
+            None => None,
+            Some(_) => Some(rust_instance_post_init::<Self>),
+        },
         instance_finalize: Self::INSTANCE_FINALIZE,
         abstract_: Self::ABSTRACT,
         class_size: core::mem::size_of::<Self::Class>(),
-- 
2.47.1

[PULL 48/49] rust: qom: split ObjectType from ObjectImpl trait

[Paolo Bonzini]

Define a separate trait for fields that also applies to classes that are
defined by C code.  This makes it possible to add metadata to core classes,
which has multiple uses:

- it makes it possible to access the parent struct's TYPE_* for types
  that are defined in Rust code, and to avoid repeating it in every subclass

- implementors of ObjectType will be allowed to implement the IsA<> trait and
  therefore to perform typesafe casts from one class to another.

- in the future, an ObjectType could be created with Foo::new() in a type-safe
  manner, without having to pass a TYPE_* constant.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs  | 17 ++++++++++++-----
 rust/qemu-api/src/definitions.rs  | 27 +++++++++++++++++++++------
 rust/qemu-api/src/device_class.rs | 11 ++++++-----
 rust/qemu-api/src/prelude.rs      |  2 ++
 rust/qemu-api/src/sysbus.rs       | 10 ++++++++--
 rust/qemu-api/tests/tests.rs      | 17 +++++++++--------
 6 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index b9f8fb134b5..0ab825b1ca4 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -12,9 +12,10 @@
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
-    device_class::{DeviceImpl, TYPE_SYS_BUS_DEVICE},
+    device_class::DeviceImpl,
     impl_device_class,
     irq::InterruptSource,
+    prelude::*,
 };
 
 use crate::{
@@ -106,10 +107,13 @@ pub struct PL011State {
     device_id: DeviceId,
 }
 
-impl ObjectImpl for PL011State {
+unsafe impl ObjectType for PL011State {
     type Class = PL011Class;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_SYS_BUS_DEVICE);
+}
+
+impl ObjectImpl for PL011State {
+    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<SysBusDevice as ObjectType>::TYPE_NAME);
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
@@ -640,10 +644,13 @@ unsafe fn init(&mut self) {
     }
 }
 
-impl ObjectImpl for PL011Luminary {
+unsafe impl ObjectType for PL011Luminary {
     type Class = PL011LuminaryClass;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011_LUMINARY;
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(crate::TYPE_PL011);
+}
+
+impl ObjectImpl for PL011Luminary {
+    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<PL011State as ObjectType>::TYPE_NAME);
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index f2970758986..b98a6926785 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -26,25 +26,40 @@
     T::INSTANCE_POST_INIT.unwrap()(unsafe { &mut *obj.cast::<T>() })
 }
 
-/// Trait a type must implement to be registered with QEMU.
+/// Trait exposed by all structs corresponding to QOM objects.
 ///
 /// # Safety
 ///
-/// - the struct must be `#[repr(C)]`
+/// For classes declared in C:
 ///
-/// - `Class` and `TYPE` must match the data in the `TypeInfo` (this is
-///   automatic if the class is defined via `ObjectImpl`).
+/// - `Class` and `TYPE` must match the data in the `TypeInfo`;
+///
+/// - the first field of the struct must be of the instance type corresponding
+///   to the superclass, as declared in the `TypeInfo`
+///
+/// - likewise, the first field of the `Class` struct must be of the class type
+///   corresponding to the superclass
+///
+/// For classes declared in Rust and implementing [`ObjectImpl`]:
+///
+/// - the struct must be `#[repr(C)]`;
 ///
 /// - the first field of the struct must be of the instance struct corresponding
-///   to the superclass declared as `PARENT_TYPE_NAME`
-pub trait ObjectImpl: ClassInitImpl + Sized {
+///   to the superclass, as declared in `ObjectImpl::PARENT_TYPE_NAME`
+///
+/// - likewise, the first field of the `Class` must be of the class struct
+///   corresponding to the superclass
+pub unsafe trait ObjectType: Sized {
     /// The QOM class object corresponding to this struct.  Not used yet.
     type Class;
 
     /// The name of the type, which can be passed to `object_new()` to
     /// generate an instance of this type.
     const TYPE_NAME: &'static CStr;
+}
 
+/// Trait a type must implement to be registered with QEMU.
+pub trait ObjectImpl: ObjectType + ClassInitImpl {
     /// The parent of the type.  This should match the first field of
     /// the struct that implements `ObjectImpl`:
     const PARENT_TYPE_NAME: Option<&'static CStr>;
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index f25904be4f6..03d03feee83 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -6,6 +6,7 @@
 
 use crate::{
     bindings::{self, DeviceClass, DeviceState, Error, ObjectClass, Property, VMStateDescription},
+    prelude::*,
     zeroable::Zeroable,
 };
 
@@ -146,8 +147,8 @@ macro_rules! declare_properties {
     };
 }
 
-// workaround until we can use --generate-cstr in bindgen.
-pub const TYPE_DEVICE: &CStr =
-    unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
-pub const TYPE_SYS_BUS_DEVICE: &CStr =
-    unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) };
+unsafe impl ObjectType for bindings::DeviceState {
+    type Class = bindings::DeviceClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_DEVICE) };
+}
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index a39e228babf..1b8677b2d9a 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -6,3 +6,5 @@
 
 pub use crate::cell::BqlCell;
 pub use crate::cell::BqlRefCell;
+
+pub use crate::definitions::ObjectType;
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index 4e192c75898..5ee068541cf 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -2,11 +2,17 @@
 // Author(s): Paolo Bonzini <pbonzini@redhat.com>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::ptr::addr_of;
+use std::{ffi::CStr, ptr::addr_of};
 
 pub use bindings::{SysBusDevice, SysBusDeviceClass};
 
-use crate::{bindings, cell::bql_locked, irq::InterruptSource};
+use crate::{bindings, cell::bql_locked, irq::InterruptSource, prelude::*};
+
+unsafe impl ObjectType for SysBusDevice {
+    type Class = SysBusDeviceClass;
+    const TYPE_NAME: &'static CStr =
+        unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) };
+}
 
 impl SysBusDevice {
     /// Return `self` cast to a mutable pointer, for use in calls to C code.
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index b8b12a40422..1d027dd6527 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -5,12 +5,8 @@
 use std::ffi::CStr;
 
 use qemu_api::{
-    bindings::*,
-    c_str, declare_properties, define_property,
-    definitions::ObjectImpl,
-    device_class::{self, DeviceImpl},
-    impl_device_class,
-    zeroable::Zeroable,
+    bindings::*, c_str, declare_properties, define_property, definitions::ObjectImpl,
+    device_class::DeviceImpl, impl_device_class, prelude::*, zeroable::Zeroable,
 };
 
 #[test]
@@ -46,10 +42,15 @@ pub struct DummyClass {
             ),
     }
 
-    impl ObjectImpl for DummyState {
+    unsafe impl ObjectType for DummyState {
         type Class = DummyClass;
         const TYPE_NAME: &'static CStr = c_str!("dummy");
-        const PARENT_TYPE_NAME: Option<&'static CStr> = Some(device_class::TYPE_DEVICE);
+    }
+
+    impl ObjectImpl for DummyState {
+        const PARENT_TYPE_NAME: Option<&'static CStr> =
+            Some(<DeviceState as ObjectType>::TYPE_NAME);
+        const ABSTRACT: bool = false;
     }
 
     impl DeviceImpl for DummyState {
-- 
2.47.1

[PULL 49/49] rust: qom: change the parent type to an associated type

[Paolo Bonzini]

Avoid duplicated code to retrieve the QOM type strings from the
Rust type.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs |  6 ++++--
 rust/qemu-api/src/definitions.rs | 12 ++++--------
 rust/qemu-api/tests/tests.rs     |  3 +--
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 0ab825b1ca4..3e29442a625 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -113,7 +113,8 @@ unsafe impl ObjectType for PL011State {
 }
 
 impl ObjectImpl for PL011State {
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<SysBusDevice as ObjectType>::TYPE_NAME);
+    type ParentType = SysBusDevice;
+
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
@@ -650,7 +651,8 @@ unsafe impl ObjectType for PL011Luminary {
 }
 
 impl ObjectImpl for PL011Luminary {
-    const PARENT_TYPE_NAME: Option<&'static CStr> = Some(<PL011State as ObjectType>::TYPE_NAME);
+    type ParentType = PL011State;
+
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index b98a6926785..df91a2e31a9 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -45,10 +45,10 @@
 /// - the struct must be `#[repr(C)]`;
 ///
 /// - the first field of the struct must be of the instance struct corresponding
-///   to the superclass, as declared in `ObjectImpl::PARENT_TYPE_NAME`
+///   to the superclass, which is `ObjectImpl::ParentType`
 ///
 /// - likewise, the first field of the `Class` must be of the class struct
-///   corresponding to the superclass
+///   corresponding to the superclass, which is `ObjectImpl::ParentType::Class`.
 pub unsafe trait ObjectType: Sized {
     /// The QOM class object corresponding to this struct.  Not used yet.
     type Class;
@@ -62,7 +62,7 @@ pub unsafe trait ObjectType: Sized {
 pub trait ObjectImpl: ObjectType + ClassInitImpl {
     /// The parent of the type.  This should match the first field of
     /// the struct that implements `ObjectImpl`:
-    const PARENT_TYPE_NAME: Option<&'static CStr>;
+    type ParentType: ObjectType;
 
     /// Whether the object can be instantiated
     const ABSTRACT: bool = false;
@@ -82,11 +82,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl {
 
     const TYPE_INFO: TypeInfo = TypeInfo {
         name: Self::TYPE_NAME.as_ptr(),
-        parent: if let Some(pname) = Self::PARENT_TYPE_NAME {
-            pname.as_ptr()
-        } else {
-            core::ptr::null_mut()
-        },
+        parent: Self::ParentType::TYPE_NAME.as_ptr(),
         instance_size: core::mem::size_of::<Self>(),
         instance_align: core::mem::align_of::<Self>(),
         instance_init: match Self::INSTANCE_INIT {
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 1d027dd6527..278efe967fe 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -48,8 +48,7 @@ unsafe impl ObjectType for DummyState {
     }
 
     impl ObjectImpl for DummyState {
-        const PARENT_TYPE_NAME: Option<&'static CStr> =
-            Some(<DeviceState as ObjectType>::TYPE_NAME);
+        type ParentType = DeviceState;
         const ABSTRACT: bool = false;
     }
 
-- 
2.47.1

Re: [PULL 00/49] rust, QOM, kvm changes for 2024-12-11

[Stefan Hajnoczi]

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

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.

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

Re: [PULL 37/49] rust: cell: add BQL-enforcing RefCell variant

[Bernhard Beschow]

Am 11. Dezember 2024 16:27:07 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>Similar to the existing BqlCell, introduce a custom interior mutability
>primitive that resembles RefCell but accounts for QEMU's threading model.
>Borrowing the RefCell requires proving that the BQL is held, and
>attempting to access without the BQL is a runtime panic.
>
>Almost all of the code was taken from Rust's standard library, while
>removing unstable features and probably-unnecessary functionality that
>amounts to 60% of the original code.  A lot of what's left is documentation,
>as well as unit tests in the form of doctests.  These are not yet integrated
>in "make check" but can be run with "cargo test --doc".
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> rust/qemu-api/Cargo.toml  |   3 +-
> rust/qemu-api/meson.build |   3 +
> rust/qemu-api/src/cell.rs | 544 +++++++++++++++++++++++++++++++++++++-
> 3 files changed, 539 insertions(+), 11 deletions(-)
>
>diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
>index 669f288d1cb..4aa22f31986 100644
>--- a/rust/qemu-api/Cargo.toml
>+++ b/rust/qemu-api/Cargo.toml
>@@ -20,8 +20,9 @@ qemu_api_macros = { path = "../qemu-api-macros" }
> version_check = "~0.9"
> 
> [features]
>-default = []
>+default = ["debug_cell"]
> allocator = []
>+debug_cell = []
> 
> [lints]
> workspace = true
>diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
>index edc21e1a3f8..cacb112c5c3 100644
>--- a/rust/qemu-api/meson.build
>+++ b/rust/qemu-api/meson.build
>@@ -6,6 +6,9 @@ _qemu_api_cfg = run_command(rustc_args,
> if rustc.version().version_compare('>=1.77.0')
>   _qemu_api_cfg += ['--cfg', 'has_offset_of']
> endif
>+if get_option('debug_mutex')
>+  _qemu_api_cfg += ['--feature', 'debug_cell']

That should be:

_qemu_api_cfg += ['--cfg', 'feature="debug_cell"']

otherwise this results in compile error "Unrecogninzed option: 'feature'"

Best regards,
Bernhard

>+endif
> 
> _qemu_api_rs = static_library(
>   'qemu_api',
>diff --git a/rust/qemu-api/src/cell.rs b/rust/qemu-api/src/cell.rs
>index 2e4ea8d590d..28349de291a 100644
>--- a/rust/qemu-api/src/cell.rs
>+++ b/rust/qemu-api/src/cell.rs
>@@ -46,20 +46,30 @@
> //! parts of a  device must be made mutable in a controlled manner through the
> //! use of cell types.
> //!
>-//! This module provides a way to do so via the Big QEMU Lock.  While
>-//! [`BqlCell<T>`] is essentially the same single-threaded primitive that is
>-//! available in `std::cell`, the BQL allows it to be used from a multi-threaded
>-//! context and to share references across threads, while maintaining Rust's
>-//! safety guarantees.  For this reason, unlike its `std::cell` counterpart,
>-//! `BqlCell` implements the `Sync` trait.
>+//! [`BqlCell<T>`] and [`BqlRefCell<T>`] allow doing this via the Big QEMU Lock.
>+//! While they are essentially the same single-threaded primitives that are
>+//! available in `std::cell`, the BQL allows them to be used from a
>+//! multi-threaded context and to share references across threads, while
>+//! maintaining Rust's safety guarantees.  For this reason, unlike
>+//! their `std::cell` counterparts, `BqlCell` and `BqlRefCell` implement the
>+//! `Sync` trait.
> //!
> //! BQL checks are performed in debug builds but can be optimized away in
> //! release builds, providing runtime safety during development with no overhead
> //! in production.
> //!
>-//! Warning: While `BqlCell` is similar to its `std::cell` counterpart, the two
>-//! are not interchangeable. Using `std::cell` types in QEMU device
>-//! implementations is usually incorrect and can lead to thread-safety issues.
>+//! The two provide different ways of handling interior mutability.
>+//! `BqlRefCell` is best suited for data that is primarily accessed by the
>+//! device's own methods, where multiple reads and writes can be grouped within
>+//! a single borrow and a mutable reference can be passed around.  Instead,
>+//! [`BqlCell`] is a better choice when sharing small pieces of data with
>+//! external code (especially C code), because it provides simple get/set
>+//! operations that can be used one at a time.
>+//!
>+//! Warning: While `BqlCell` and `BqlRefCell` are similar to their `std::cell`
>+//! counterparts, they are not interchangeable. Using `std::cell` types in
>+//! QEMU device implementations is usually incorrect and can lead to
>+//! thread-safety issues.
> //!
> //! ## `BqlCell<T>`
> //!
>@@ -80,8 +90,37 @@
> //!      returns the replaced value.
> //!    - [`set`](BqlCell::set): this method replaces the interior value,
> //!      dropping the replaced value.
>+//!
>+//! ## `BqlRefCell<T>`
>+//!
>+//! [`BqlRefCell<T>`] uses Rust's lifetimes to implement "dynamic borrowing", a
>+//! process whereby one can claim temporary, exclusive, mutable access to the
>+//! inner value:
>+//!
>+//! ```ignore
>+//! fn clear_interrupts(&self, val: u32) {
>+//!     // A mutable borrow gives read-write access to the registers
>+//!     let mut regs = self.registers.borrow_mut();
>+//!     let old = regs.interrupt_status();
>+//!     regs.update_interrupt_status(old & !val);
>+//! }
>+//! ```
>+//!
>+//! Borrows for `BqlRefCell<T>`s are tracked at _runtime_, unlike Rust's native
>+//! reference types which are entirely tracked statically, at compile time.
>+//! Multiple immutable borrows are allowed via [`borrow`](BqlRefCell::borrow),
>+//! or a single mutable borrow via [`borrow_mut`](BqlRefCell::borrow_mut).  The
>+//! thread will panic if these rules are violated or if the BQL is not held.
> 
>-use std::{cell::UnsafeCell, cmp::Ordering, fmt, mem};
>+use std::{
>+    cell::{Cell, UnsafeCell},
>+    cmp::Ordering,
>+    fmt,
>+    marker::PhantomData,
>+    mem,
>+    ops::{Deref, DerefMut},
>+    ptr::NonNull,
>+};
> 
> use crate::bindings;
> 
>@@ -93,6 +132,15 @@ pub fn bql_locked() -> bool {
>     !cfg!(MESON) || unsafe { bindings::bql_locked() }
> }
> 
>+fn bql_block_unlock(increase: bool) {
>+    if cfg!(MESON) {
>+        // SAFETY: this only adjusts a counter
>+        unsafe {
>+            bindings::bql_block_unlock(increase);
>+        }
>+    }
>+}
>+
> /// A mutable memory location that is protected by the Big QEMU Lock.
> ///
> /// # Memory layout
>@@ -296,3 +344,479 @@ pub fn take(&self) -> T {
>         self.replace(Default::default())
>     }
> }
>+
>+/// A mutable memory location with dynamically checked borrow rules,
>+/// protected by the Big QEMU Lock.
>+///
>+/// See the [module-level documentation](self) for more.
>+///
>+/// # Memory layout
>+///
>+/// `BqlRefCell<T>` starts with the same in-memory representation as its
>+/// inner type `T`.
>+#[repr(C)]
>+pub struct BqlRefCell<T> {
>+    // It is important that this is the first field (which is not the case
>+    // for std::cell::BqlRefCell), so that we can use offset_of! on it.
>+    // UnsafeCell and repr(C) both prevent usage of niches.
>+    value: UnsafeCell<T>,
>+    borrow: Cell<BorrowFlag>,
>+    // Stores the location of the earliest currently active borrow.
>+    // This gets updated whenever we go from having zero borrows
>+    // to having a single borrow. When a borrow occurs, this gets included
>+    // in the panic message
>+    #[cfg(feature = "debug_cell")]
>+    borrowed_at: Cell<Option<&'static std::panic::Location<'static>>>,
>+}
>+
>+// Positive values represent the number of `BqlRef` active. Negative values
>+// represent the number of `BqlRefMut` active. Right now QEMU's implementation
>+// does not allow to create `BqlRefMut`s that refer to distinct, nonoverlapping
>+// components of a `BqlRefCell` (e.g., different ranges of a slice).
>+//
>+// `BqlRef` and `BqlRefMut` are both two words in size, and so there will likely
>+// never be enough `BqlRef`s or `BqlRefMut`s in existence to overflow half of
>+// the `usize` range. Thus, a `BorrowFlag` will probably never overflow or
>+// underflow. However, this is not a guarantee, as a pathological program could
>+// repeatedly create and then mem::forget `BqlRef`s or `BqlRefMut`s. Thus, all
>+// code must explicitly check for overflow and underflow in order to avoid
>+// unsafety, or at least behave correctly in the event that overflow or
>+// underflow happens (e.g., see BorrowRef::new).
>+type BorrowFlag = isize;
>+const UNUSED: BorrowFlag = 0;
>+
>+#[inline(always)]
>+const fn is_writing(x: BorrowFlag) -> bool {
>+    x < UNUSED
>+}
>+
>+#[inline(always)]
>+const fn is_reading(x: BorrowFlag) -> bool {
>+    x > UNUSED
>+}
>+
>+impl<T> BqlRefCell<T> {
>+    /// Creates a new `BqlRefCell` containing `value`.
>+    ///
>+    /// # Examples
>+    ///
>+    /// ```
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new(5);
>+    /// ```
>+    #[inline]
>+    pub const fn new(value: T) -> BqlRefCell<T> {
>+        BqlRefCell {
>+            value: UnsafeCell::new(value),
>+            borrow: Cell::new(UNUSED),
>+            #[cfg(feature = "debug_cell")]
>+            borrowed_at: Cell::new(None),
>+        }
>+    }
>+}
>+
>+// This ensures the panicking code is outlined from `borrow_mut` for
>+// `BqlRefCell`.
>+#[inline(never)]
>+#[cold]
>+#[cfg(feature = "debug_cell")]
>+fn panic_already_borrowed(source: &Cell<Option<&'static std::panic::Location<'static>>>) -> ! {
>+    // If a borrow occurred, then we must already have an outstanding borrow,
>+    // so `borrowed_at` will be `Some`
>+    panic!("already borrowed at {:?}", source.take().unwrap())
>+}
>+
>+#[inline(never)]
>+#[cold]
>+#[cfg(not(feature = "debug_cell"))]
>+fn panic_already_borrowed() -> ! {
>+    panic!("already borrowed")
>+}
>+
>+impl<T> BqlRefCell<T> {
>+    #[inline]
>+    #[allow(clippy::unused_self)]
>+    fn panic_already_borrowed(&self) -> ! {
>+        #[cfg(feature = "debug_cell")]
>+        {
>+            panic_already_borrowed(&self.borrowed_at)
>+        }
>+        #[cfg(not(feature = "debug_cell"))]
>+        {
>+            panic_already_borrowed()
>+        }
>+    }
>+
>+    /// Immutably borrows the wrapped value.
>+    ///
>+    /// The borrow lasts until the returned `BqlRef` exits scope. Multiple
>+    /// immutable borrows can be taken out at the same time.
>+    ///
>+    /// # Panics
>+    ///
>+    /// Panics if the value is currently mutably borrowed.
>+    ///
>+    /// # Examples
>+    ///
>+    /// ```
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new(5);
>+    ///
>+    /// let borrowed_five = c.borrow();
>+    /// let borrowed_five2 = c.borrow();
>+    /// ```
>+    ///
>+    /// An example of panic:
>+    ///
>+    /// ```should_panic
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new(5);
>+    ///
>+    /// let m = c.borrow_mut();
>+    /// let b = c.borrow(); // this causes a panic
>+    /// ```
>+    #[inline]
>+    #[track_caller]
>+    pub fn borrow(&self) -> BqlRef<'_, T> {
>+        if let Some(b) = BorrowRef::new(&self.borrow) {
>+            // `borrowed_at` is always the *first* active borrow
>+            if b.borrow.get() == 1 {
>+                #[cfg(feature = "debug_cell")]
>+                self.borrowed_at.set(Some(std::panic::Location::caller()));
>+            }
>+
>+            bql_block_unlock(true);
>+
>+            // SAFETY: `BorrowRef` ensures that there is only immutable access
>+            // to the value while borrowed.
>+            let value = unsafe { NonNull::new_unchecked(self.value.get()) };
>+            BqlRef { value, borrow: b }
>+        } else {
>+            self.panic_already_borrowed()
>+        }
>+    }
>+
>+    /// Mutably borrows the wrapped value.
>+    ///
>+    /// The borrow lasts until the returned `BqlRefMut` or all `BqlRefMut`s
>+    /// derived from it exit scope. The value cannot be borrowed while this
>+    /// borrow is active.
>+    ///
>+    /// # Panics
>+    ///
>+    /// Panics if the value is currently borrowed.
>+    ///
>+    /// # Examples
>+    ///
>+    /// ```
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new("hello".to_owned());
>+    ///
>+    /// *c.borrow_mut() = "bonjour".to_owned();
>+    ///
>+    /// assert_eq!(&*c.borrow(), "bonjour");
>+    /// ```
>+    ///
>+    /// An example of panic:
>+    ///
>+    /// ```should_panic
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new(5);
>+    /// let m = c.borrow();
>+    ///
>+    /// let b = c.borrow_mut(); // this causes a panic
>+    /// ```
>+    #[inline]
>+    #[track_caller]
>+    pub fn borrow_mut(&self) -> BqlRefMut<'_, T> {
>+        if let Some(b) = BorrowRefMut::new(&self.borrow) {
>+            #[cfg(feature = "debug_cell")]
>+            {
>+                self.borrowed_at.set(Some(std::panic::Location::caller()));
>+            }
>+
>+            // SAFETY: this only adjusts a counter
>+            bql_block_unlock(true);
>+
>+            // SAFETY: `BorrowRefMut` guarantees unique access.
>+            let value = unsafe { NonNull::new_unchecked(self.value.get()) };
>+            BqlRefMut {
>+                value,
>+                _borrow: b,
>+                marker: PhantomData,
>+            }
>+        } else {
>+            self.panic_already_borrowed()
>+        }
>+    }
>+
>+    /// Returns a raw pointer to the underlying data in this cell.
>+    ///
>+    /// # Examples
>+    ///
>+    /// ```
>+    /// use qemu_api::cell::BqlRefCell;
>+    ///
>+    /// let c = BqlRefCell::new(5);
>+    ///
>+    /// let ptr = c.as_ptr();
>+    /// ```
>+    #[inline]
>+    pub const fn as_ptr(&self) -> *mut T {
>+        self.value.get()
>+    }
>+}
>+
>+// SAFETY: Same as for std::sync::Mutex.  In the end this is a Mutex that is
>+// stored out-of-line.  Even though BqlRefCell includes Cells, they are
>+// themselves protected by the Big QEMU Lock.  Furtheremore, the Big QEMU
>+// Lock cannot be released while any borrows is active.
>+unsafe impl<T> Send for BqlRefCell<T> where T: Send {}
>+unsafe impl<T> Sync for BqlRefCell<T> {}
>+
>+impl<T: Clone> Clone for BqlRefCell<T> {
>+    /// # Panics
>+    ///
>+    /// Panics if the value is currently mutably borrowed.
>+    #[inline]
>+    #[track_caller]
>+    fn clone(&self) -> BqlRefCell<T> {
>+        BqlRefCell::new(self.borrow().clone())
>+    }
>+
>+    /// # Panics
>+    ///
>+    /// Panics if `source` is currently mutably borrowed.
>+    #[inline]
>+    #[track_caller]
>+    fn clone_from(&mut self, source: &Self) {
>+        self.value.get_mut().clone_from(&source.borrow())
>+    }
>+}
>+
>+impl<T: Default> Default for BqlRefCell<T> {
>+    /// Creates a `BqlRefCell<T>`, with the `Default` value for T.
>+    #[inline]
>+    fn default() -> BqlRefCell<T> {
>+        BqlRefCell::new(Default::default())
>+    }
>+}
>+
>+impl<T: PartialEq> PartialEq for BqlRefCell<T> {
>+    /// # Panics
>+    ///
>+    /// Panics if the value in either `BqlRefCell` is currently mutably
>+    /// borrowed.
>+    #[inline]
>+    fn eq(&self, other: &BqlRefCell<T>) -> bool {
>+        *self.borrow() == *other.borrow()
>+    }
>+}
>+
>+impl<T: Eq> Eq for BqlRefCell<T> {}
>+
>+impl<T: PartialOrd> PartialOrd for BqlRefCell<T> {
>+    /// # Panics
>+    ///
>+    /// Panics if the value in either `BqlRefCell` is currently mutably
>+    /// borrowed.
>+    #[inline]
>+    fn partial_cmp(&self, other: &BqlRefCell<T>) -> Option<Ordering> {
>+        self.borrow().partial_cmp(&*other.borrow())
>+    }
>+}
>+
>+impl<T: Ord> Ord for BqlRefCell<T> {
>+    /// # Panics
>+    ///
>+    /// Panics if the value in either `BqlRefCell` is currently mutably
>+    /// borrowed.
>+    #[inline]
>+    fn cmp(&self, other: &BqlRefCell<T>) -> Ordering {
>+        self.borrow().cmp(&*other.borrow())
>+    }
>+}
>+
>+impl<T> From<T> for BqlRefCell<T> {
>+    /// Creates a new `BqlRefCell<T>` containing the given value.
>+    fn from(t: T) -> BqlRefCell<T> {
>+        BqlRefCell::new(t)
>+    }
>+}
>+
>+struct BorrowRef<'b> {
>+    borrow: &'b Cell<BorrowFlag>,
>+}
>+
>+impl<'b> BorrowRef<'b> {
>+    #[inline]
>+    fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
>+        let b = borrow.get().wrapping_add(1);
>+        if !is_reading(b) {
>+            // Incrementing borrow can result in a non-reading value (<= 0) in these cases:
>+            // 1. It was < 0, i.e. there are writing borrows, so we can't allow a read
>+            //    borrow due to Rust's reference aliasing rules
>+            // 2. It was isize::MAX (the max amount of reading borrows) and it overflowed
>+            //    into isize::MIN (the max amount of writing borrows) so we can't allow an
>+            //    additional read borrow because isize can't represent so many read borrows
>+            //    (this can only happen if you mem::forget more than a small constant amount
>+            //    of `BqlRef`s, which is not good practice)
>+            None
>+        } else {
>+            // Incrementing borrow can result in a reading value (> 0) in these cases:
>+            // 1. It was = 0, i.e. it wasn't borrowed, and we are taking the first read
>+            //    borrow
>+            // 2. It was > 0 and < isize::MAX, i.e. there were read borrows, and isize is
>+            //    large enough to represent having one more read borrow
>+            borrow.set(b);
>+            Some(BorrowRef { borrow })
>+        }
>+    }
>+}
>+
>+impl Drop for BorrowRef<'_> {
>+    #[inline]
>+    fn drop(&mut self) {
>+        let borrow = self.borrow.get();
>+        debug_assert!(is_reading(borrow));
>+        self.borrow.set(borrow - 1);
>+        bql_block_unlock(false)
>+    }
>+}
>+
>+impl Clone for BorrowRef<'_> {
>+    #[inline]
>+    fn clone(&self) -> Self {
>+        BorrowRef::new(self.borrow).unwrap()
>+    }
>+}
>+
>+/// Wraps a borrowed reference to a value in a `BqlRefCell` box.
>+/// A wrapper type for an immutably borrowed value from a `BqlRefCell<T>`.
>+///
>+/// See the [module-level documentation](self) for more.
>+pub struct BqlRef<'b, T: 'b> {
>+    // NB: we use a pointer instead of `&'b T` to avoid `noalias` violations, because a
>+    // `BqlRef` argument doesn't hold immutability for its whole scope, only until it drops.
>+    // `NonNull` is also covariant over `T`, just like we would have with `&T`.
>+    value: NonNull<T>,
>+    borrow: BorrowRef<'b>,
>+}
>+
>+impl<T> Deref for BqlRef<'_, T> {
>+    type Target = T;
>+
>+    #[inline]
>+    fn deref(&self) -> &T {
>+        // SAFETY: the value is accessible as long as we hold our borrow.
>+        unsafe { self.value.as_ref() }
>+    }
>+}
>+
>+impl<'b, T> BqlRef<'b, T> {
>+    /// Copies a `BqlRef`.
>+    ///
>+    /// The `BqlRefCell` is already immutably borrowed, so this cannot fail.
>+    ///
>+    /// This is an associated function that needs to be used as
>+    /// `BqlRef::clone(...)`. A `Clone` implementation or a method would
>+    /// interfere with the widespread use of `r.borrow().clone()` to clone
>+    /// the contents of a `BqlRefCell`.
>+    #[must_use]
>+    #[inline]
>+    #[allow(clippy::should_implement_trait)]
>+    pub fn clone(orig: &BqlRef<'b, T>) -> BqlRef<'b, T> {
>+        BqlRef {
>+            value: orig.value,
>+            borrow: orig.borrow.clone(),
>+        }
>+    }
>+}
>+
>+impl<T: fmt::Debug> fmt::Debug for BqlRef<'_, T> {
>+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>+        (**self).fmt(f)
>+    }
>+}
>+
>+impl<T: fmt::Display> fmt::Display for BqlRef<'_, T> {
>+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>+        (**self).fmt(f)
>+    }
>+}
>+
>+struct BorrowRefMut<'b> {
>+    borrow: &'b Cell<BorrowFlag>,
>+}
>+
>+impl<'b> BorrowRefMut<'b> {
>+    #[inline]
>+    fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRefMut<'b>> {
>+        // There must currently be no existing references when borrow_mut() is
>+        // called, so we explicitly only allow going from UNUSED to UNUSED - 1.
>+        match borrow.get() {
>+            UNUSED => {
>+                borrow.set(UNUSED - 1);
>+                Some(BorrowRefMut { borrow })
>+            }
>+            _ => None,
>+        }
>+    }
>+}
>+
>+impl Drop for BorrowRefMut<'_> {
>+    #[inline]
>+    fn drop(&mut self) {
>+        let borrow = self.borrow.get();
>+        debug_assert!(is_writing(borrow));
>+        self.borrow.set(borrow + 1);
>+        bql_block_unlock(false)
>+    }
>+}
>+
>+/// A wrapper type for a mutably borrowed value from a `BqlRefCell<T>`.
>+///
>+/// See the [module-level documentation](self) for more.
>+pub struct BqlRefMut<'b, T: 'b> {
>+    // NB: we use a pointer instead of `&'b mut T` to avoid `noalias` violations, because a
>+    // `BqlRefMut` argument doesn't hold exclusivity for its whole scope, only until it drops.
>+    value: NonNull<T>,
>+    _borrow: BorrowRefMut<'b>,
>+    // `NonNull` is covariant over `T`, so we need to reintroduce invariance.
>+    marker: PhantomData<&'b mut T>,
>+}
>+
>+impl<T> Deref for BqlRefMut<'_, T> {
>+    type Target = T;
>+
>+    #[inline]
>+    fn deref(&self) -> &T {
>+        // SAFETY: the value is accessible as long as we hold our borrow.
>+        unsafe { self.value.as_ref() }
>+    }
>+}
>+
>+impl<T> DerefMut for BqlRefMut<'_, T> {
>+    #[inline]
>+    fn deref_mut(&mut self) -> &mut T {
>+        // SAFETY: the value is accessible as long as we hold our borrow.
>+        unsafe { self.value.as_mut() }
>+    }
>+}
>+
>+impl<T: fmt::Debug> fmt::Debug for BqlRefMut<'_, T> {
>+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>+        (**self).fmt(f)
>+    }
>+}
>+
>+impl<T: fmt::Display> fmt::Display for BqlRefMut<'_, T> {
>+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>+        (**self).fmt(f)
>+    }
>+}

Re: [PULL 16/49] clock: treat outputs and inputs the same in NamedClockList

[Philippe Mathieu-Daudé]

Hi Paolo,

The commit description is missing... See:
https://lore.kernel.org/qemu-devel/20241129180326.722436-3-pbonzini@redhat.com/

 >> Leave around a reference not just for inputs but also for outputs.
 >> This is a better choice because in principle the monitor could walk
 >> the NamedClockList after the clocks have been unparented (which would
 >> free them) and before qdev_finalize_clocklist().

Any clue what happened?

On 11/12/24 17:26, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/core/qdev-clock.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index ca65685c04e..2f9d6cb7579 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -48,14 +48,6 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
>       if (clk == NULL) {
>           clk = CLOCK(object_new(TYPE_CLOCK));
>           object_property_add_child(OBJECT(dev), name, OBJECT(clk));
> -        if (output) {
> -            /*
> -             * Remove object_new()'s initial reference.
> -             * Note that for inputs, the reference created by object_new()
> -             * will be deleted in qdev_finalize_clocklist().
> -             */
> -            object_unref(OBJECT(clk));
> -        }
>       } else {
>           object_property_add_link(OBJECT(dev), name,
>                                    object_get_typename(OBJECT(clk)),
> @@ -84,7 +76,7 @@ void qdev_finalize_clocklist(DeviceState *dev)
>   
>       QLIST_FOREACH_SAFE(ncl, &dev->clocks, node, ncl_next) {
>           QLIST_REMOVE(ncl, node);
> -        if (!ncl->output && !ncl->alias) {
> +        if (!ncl->alias) {
>               /*
>                * We kept a reference on the input clock to ensure it lives up to
>                * this point; it is used by the monitor to show the frequency.