[PATCH 00/12] Next round of qemu_api patches

[PATCH 00/12] Next round of qemu_api patches

[Paolo Bonzini]

This includes:

1) the outcome of reviewing
   https://lore.kernel.org/qemu-devel/20241209123717.99077-1-pbonzini@redhat.com/T/,
   especially with respect to instance_finalize implementation and
   Rust-defined subclasses (patches 1-6)

2) the beginnings of separating instance_init/instance_post_init, making
   the latter take &self (patches 7-9)

3) extracting pl011's logic to convert an integer into an enum into a
   #[derive(TryInto)] procedural macro (patches 10-11)

4) a one-off change that I noticed while reading Cargo documentation
   (patch 12)

Zhao: I have placed these in rust-next, but feel free to ignore them for
your next HPET submission.  I have also included in my tree your test
additions for usage in CI, but I'm not including them here to give you
the opportunity to post them with a commit message and a signed-off-by.
If you don't have time, it's in safe hands. :)

Paolo

Paolo Bonzini (12):
  rust: qom: add ParentField
  rust: add a utility module for compile-time type checks
  rust: macros: check that the first field of a #[derive(Object)] struct
    is a ParentField
  rust: macros: check that #[derive(Object)] requires #[repr(C)]
  rust: qom: automatically use Drop trait to implement instance_finalize
  rust: qom: move device_id to PL011 class side
  rust: pl011: only leave embedded object initialization in
    instance_init
  rust: qom: make INSTANCE_POST_INIT take a shared reference
  rust: qdev: expose inherited methods to subclasses of SysBusDevice
  rust: qemu-api-macros: extend error reporting facility to parse errors
  rust: qemu-api-macros: add automatic TryFrom/TryInto derivation
  rust: hide warnings for subprojects

 rust/hw/char/pl011/src/device.rs              |  79 +++++----
 rust/hw/char/pl011/src/lib.rs                 |  28 +---
 rust/qemu-api-macros/src/lib.rs               | 150 +++++++++++++-----
 rust/qemu-api-macros/src/utils.rs             |  26 +++
 rust/qemu-api/meson.build                     |   1 +
 rust/qemu-api/src/assertions.rs               |  90 +++++++++++
 rust/qemu-api/src/irq.rs                      |   3 +-
 rust/qemu-api/src/lib.rs                      |   1 +
 rust/qemu-api/src/prelude.rs                  |   2 +
 rust/qemu-api/src/qom.rs                      |  77 +++++++--
 rust/qemu-api/src/sysbus.rs                   |  23 ++-
 rust/qemu-api/tests/tests.rs                  |   4 +-
 subprojects/arbitrary-int-1-rs.wrap           |   3 +
 subprojects/bilge-0.2-rs.wrap                 |   3 +
 subprojects/bilge-impl-0.2-rs.wrap            |   3 +
 subprojects/either-1-rs.wrap                  |   3 +
 subprojects/itertools-0.11-rs.wrap            |   3 +
 .../arbitrary-int-1-rs/meson.build            |   1 +
 .../packagefiles/bilge-0.2-rs/meson.build     |   1 +
 .../bilge-impl-0.2-rs/meson.build             |   1 +
 .../packagefiles/either-1-rs/meson.build      |   1 +
 .../itertools-0.11-rs/meson.build             |   1 +
 .../proc-macro-error-1-rs/meson.build         |   1 +
 .../proc-macro-error-attr-1-rs/meson.build    |   1 +
 .../packagefiles/proc-macro2-1-rs/meson.build |   1 +
 .../packagefiles/quote-1-rs/meson.build       |   1 +
 subprojects/packagefiles/syn-2-rs/meson.build |   1 +
 .../unicode-ident-1-rs/meson.build            |   1 +
 subprojects/proc-macro-error-1-rs.wrap        |   3 +
 subprojects/proc-macro-error-attr-1-rs.wrap   |   3 +
 subprojects/proc-macro2-1-rs.wrap             |   3 +
 subprojects/quote-1-rs.wrap                   |   3 +
 subprojects/syn-2-rs.wrap                     |   3 +
 subprojects/unicode-ident-1-rs.wrap           |   3 +
 subprojects/unicode-ident-1-rs/meson.build    |  20 ---
 35 files changed, 402 insertions(+), 146 deletions(-)
 create mode 100644 rust/qemu-api-macros/src/utils.rs
 create mode 100644 rust/qemu-api/src/assertions.rs
 delete mode 100644 subprojects/unicode-ident-1-rs/meson.build

-- 
2.47.1

[PATCH 01/12] rust: qom: add ParentField

[Paolo Bonzini]

Add a type that, together with the C function object_deinit, ensures the
correct drop order for QOM objects relative to their superclasses.

Right now it is not possible to implement the Drop trait for QOM classes
that are defined in Rust, as the drop() function would not be called when
the object goes away; instead what is called is ObjectImpl::INSTANCE_FINALIZE.
It would be nice for INSTANCE_FINALIZE to just drop the object, but this has
a problem: suppose you have

   pub struct MySuperclass {
       parent: DeviceState,
       field: Box<MyData>,
       ...
   }

   impl Drop for MySuperclass {
       ...
   }

   pub struct MySubclass {
       parent: MySuperclass,
       ...
   }

and an instance_finalize implementation that is like

    unsafe extern "C" fn drop_object<T: ObjectImpl>(obj: *mut Object) {
        unsafe { std::ptr::drop_in_place(obj.cast::<T>()) }
    }

When instance_finalize is called for MySubclass, it will walk the struct's
list of fields and call the drop method for MySuperclass.  Then, object_deinit
recurses to the superclass and calls the same drop method again.  This
will cause double-freeing of the Box<Data>.

What's happening here is that QOM wants to control the drop order of
MySuperclass and MySubclass's fields.  To do so, the parent field must
be marked ManuallyDrop<>, which is quite ugly.  Instead, add a wrapper
type ParentField<> that is specific to QOM.  This hides the implementation
detail of *what* is special about the ParentField, and will also be easy
to check in the #[derive(Object)] macro.

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

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 18cc122951d..689202f4550 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -14,7 +14,7 @@
     irq::InterruptSource,
     prelude::*,
     qdev::DeviceImpl,
-    qom::ObjectImpl,
+    qom::{ObjectImpl, ParentField},
 };
 
 use crate::{
@@ -86,7 +86,7 @@ fn index(&self, idx: u32) -> &Self::Output {
 #[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)]
 /// PL011 Device Model in QEMU
 pub struct PL011State {
-    pub parent_obj: SysBusDevice,
+    pub parent_obj: ParentField<SysBusDevice>,
     pub iomem: MemoryRegion,
     #[doc(alias = "fr")]
     pub flags: registers::Flags,
@@ -645,7 +645,7 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
 #[derive(Debug, qemu_api_macros::Object)]
 /// PL011 Luminary device model.
 pub struct PL011Luminary {
-    parent_obj: PL011State,
+    parent_obj: ParentField<PL011State>,
 }
 
 impl PL011Luminary {
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 7d5fbef1e17..1341a173893 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -55,6 +55,7 @@
 
 use std::{
     ffi::CStr,
+    fmt,
     ops::{Deref, DerefMut},
     os::raw::c_void,
 };
@@ -105,6 +106,52 @@ fn as_ref(&self) -> &$parent {
     };
 }
 
+/// This is the same as [`ManuallyDrop<T>`](std::mem::ManuallyDrop), though
+/// it hides the standard methods of `ManuallyDrop`.
+///
+/// The first field of an `ObjectType` must be of type `ParentField<T>`.
+/// (Technically, this is only necessary if there is at least one Rust
+/// superclass in the hierarchy).  This is to ensure that the parent field is
+/// dropped after the subclass; this drop order is enforced by the C
+/// `object_deinit` function.
+///
+/// # Examples
+///
+/// ```ignore
+/// #[repr(C)]
+/// #[derive(qemu_api_macros::Object)]
+/// pub struct MyDevice {
+///     parent: ParentField<DeviceState>,
+///     ...
+/// }
+/// ```
+#[derive(Debug)]
+#[repr(transparent)]
+pub struct ParentField<T: ObjectType>(std::mem::ManuallyDrop<T>);
+
+impl<T: ObjectType> Deref for ParentField<T> {
+    type Target = T;
+
+    #[inline(always)]
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T: ObjectType> DerefMut for ParentField<T> {
+    #[inline(always)]
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<T: fmt::Display + ObjectType> fmt::Display for ParentField<T> {
+    #[inline(always)]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        self.0.fmt(f)
+    }
+}
+
 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
@@ -151,8 +198,9 @@ fn as_ref(&self) -> &$parent {
 ///
 /// - the struct must be `#[repr(C)]`;
 ///
-/// - the first field of the struct must be of the instance struct corresponding
-///   to the superclass, which is `ObjectImpl::ParentType`
+/// - the first field of the struct must be of type
+///   [`ParentField<T>`](ParentField), where `T` is the parent type
+///   [`ObjectImpl::ParentType`]
 ///
 /// - likewise, the first field of the `Class` must be of the class struct
 ///   corresponding to the superclass, which is `ObjectImpl::ParentType::Class`.
@@ -384,8 +432,8 @@ impl<T: ObjectType> ObjectCastMut for &mut T {}
 
 /// Trait a type must implement to be registered with QEMU.
 pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> {
-    /// The parent of the type.  This should match the first field of
-    /// the struct that implements `ObjectImpl`:
+    /// The parent of the type.  This should match the first field of the
+    /// struct that implements `ObjectImpl`, minus the `ParentField<_>` wrapper.
     type ParentType: ObjectType;
 
     /// Whether the object can be instantiated
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 1d2825b0986..526c3f4f8ea 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -15,7 +15,7 @@
     declare_properties, define_property,
     prelude::*,
     qdev::{DeviceImpl, DeviceState, Property},
-    qom::ObjectImpl,
+    qom::{ObjectImpl, ParentField},
     vmstate::VMStateDescription,
     zeroable::Zeroable,
 };
@@ -31,7 +31,7 @@
 #[repr(C)]
 #[derive(qemu_api_macros::Object)]
 pub struct DummyState {
-    parent: DeviceState,
+    parent: ParentField<DeviceState>,
     migrate_clock: bool,
 }
 
-- 
2.47.1

[PATCH 02/12] rust: add a utility module for compile-time type checks

[Paolo Bonzini]

It is relatively common in the low-level qemu_api code to assert that
a field of a struct has a specific type; for example, it can be used
to ensure that the fields match what the qemu_api and C code expects
for safety.

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

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 9425ba7100c..60944a657de 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -15,6 +15,7 @@ _qemu_api_rs = static_library(
   structured_sources(
     [
       'src/lib.rs',
+      'src/assertions.rs',
       'src/bindings.rs',
       'src/bitops.rs',
       'src/callbacks.rs',
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
new file mode 100644
index 00000000000..1a6cb3a6c7c
--- /dev/null
+++ b/rust/qemu-api/src/assertions.rs
@@ -0,0 +1,90 @@
+// Copyright 2024, Red Hat Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! This module provides macros to check the equality of types and
+//! the type of `struct` fields.  This can be useful to ensure that
+//! types match the expectations of C code.
+
+// Based on https://stackoverflow.com/questions/64251852/x/70978292#70978292
+// (stackoverflow answers are released under MIT license).
+
+#[doc(hidden)]
+pub trait EqType {
+    type Itself;
+}
+
+impl<T> EqType for T {
+    type Itself = T;
+}
+
+/// Assert that two types are the same.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::assert_same_type;
+/// # use std::ops::Deref;
+/// assert_same_type!(u32, u32);
+/// assert_same_type!(<Box<u32> as Deref>::Target, u32);
+/// ```
+///
+/// Different types will cause a compile failure
+///
+/// ```compile_fail
+/// # use qemu_api::assert_same_type;
+/// assert_same_type!(&Box<u32>, &u32);
+/// ```
+#[macro_export]
+macro_rules! assert_same_type {
+    ($t1:ty, $t2:ty) => {
+        const _: () = {
+            #[allow(unused)]
+            fn assert_same_type(v: $t1) {
+                fn types_must_be_equal<T, U>(_: T)
+                where
+                    T: $crate::assertions::EqType<Itself = U>,
+                {
+                }
+                types_must_be_equal::<_, $t2>(v);
+            }
+        };
+    };
+}
+
+/// Assert that a field of a struct has the given type.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::assert_field_type;
+/// pub struct A {
+///     field1: u32,
+/// }
+///
+/// assert_field_type!(A, field1, u32);
+/// ```
+///
+/// Different types will cause a compile failure
+///
+/// ```compile_fail
+/// # use qemu_api::assert_field_type;
+/// # pub struct A { field1: u32 }
+/// assert_field_type!(A, field1, i32);
+/// ```
+#[macro_export]
+macro_rules! assert_field_type {
+    ($t:ty, $i:ident, $ti:ty) => {
+        const _: () = {
+            #[allow(unused)]
+            fn assert_field_type(v: $t) {
+                fn types_must_be_equal<T, U>(_: T)
+                where
+                    T: $crate::assertions::EqType<Itself = U>,
+                {
+                }
+                types_must_be_equal::<_, $ti>(v.$i);
+            }
+        };
+    };
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 4b43e02c0f9..83c6a987c05 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 assertions;
 pub mod bitops;
 pub mod c_str;
 pub mod callbacks;
-- 
2.47.1

[PATCH 03/12] rust: macros: check that the first field of a #[derive(Object)] struct is a ParentField

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api-macros/src/lib.rs | 61 +++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 74a8bc7503e..778b698be45 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -19,6 +19,27 @@ fn from(err: CompileError) -> Self {
     }
 }
 
+fn get_fields<'a>(
+    input: &'a DeriveInput,
+    msg: &str,
+) -> Result<&'a Punctuated<Field, Comma>, CompileError> {
+    if let Data::Struct(s) = &input.data {
+        if let Fields::Named(fs) = &s.fields {
+            Ok(&fs.named)
+        } else {
+            Err(CompileError(
+                format!("Named fields required for {}", msg),
+                input.ident.span(),
+            ))
+        }
+    } else {
+        Err(CompileError(
+            format!("Struct required for {}", msg),
+            input.ident.span(),
+        ))
+    }
+}
+
 fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), CompileError> {
     let expected = parse_quote! { #[repr(C)] };
 
@@ -32,38 +53,28 @@ fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), CompileError> {
     }
 }
 
-#[proc_macro_derive(Object)]
-pub fn derive_object(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
-    let name = input.ident;
+fn derive_object_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, CompileError> {
+    let name = &input.ident;
+    let parent = &get_fields(&input, "#[derive(Object)]")?[0].ident;
+
+    Ok(quote! {
+        ::qemu_api::assert_field_type!(#name, #parent,
+            ::qemu_api::qom::ParentField<<#name as ::qemu_api::qom::ObjectImpl>::ParentType>);
 
-    let expanded = quote! {
         ::qemu_api::module_init! {
             MODULE_INIT_QOM => unsafe {
                 ::qemu_api::bindings::type_register_static(&<#name as ::qemu_api::qom::ObjectImpl>::TYPE_INFO);
             }
         }
-    };
-
-    TokenStream::from(expanded)
+    })
 }
 
-fn get_fields(input: &DeriveInput) -> Result<&Punctuated<Field, Comma>, CompileError> {
-    if let Data::Struct(s) = &input.data {
-        if let Fields::Named(fs) = &s.fields {
-            Ok(&fs.named)
-        } else {
-            Err(CompileError(
-                "Cannot generate offsets for unnamed fields.".to_string(),
-                input.ident.span(),
-            ))
-        }
-    } else {
-        Err(CompileError(
-            "Cannot generate offsets for union or enum.".to_string(),
-            input.ident.span(),
-        ))
-    }
+#[proc_macro_derive(Object)]
+pub fn derive_object(input: TokenStream) -> TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let expanded = derive_object_or_error(input).unwrap_or_else(Into::into);
+
+    TokenStream::from(expanded)
 }
 
 #[rustfmt::skip::macros(quote)]
@@ -71,7 +82,7 @@ fn derive_offsets_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStrea
     is_c_repr(&input, "#[derive(offsets)]")?;
 
     let name = &input.ident;
-    let fields = get_fields(&input)?;
+    let fields = get_fields(&input, "#[derive(offsets)]")?;
     let field_names: Vec<&Ident> = fields.iter().map(|f| f.ident.as_ref().unwrap()).collect();
     let field_types: Vec<&Type> = fields.iter().map(|f| &f.ty).collect();
     let field_vis: Vec<&Visibility> = fields.iter().map(|f| &f.vis).collect();
-- 
2.47.1

[PATCH 04/12] rust: macros: check that #[derive(Object)] requires #[repr(C)]

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api-macros/src/lib.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 778b698be45..0f04cca3841 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -54,6 +54,8 @@ fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), CompileError> {
 }
 
 fn derive_object_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, CompileError> {
+    is_c_repr(&input, "#[derive(Object)]")?;
+
     let name = &input.ident;
     let parent = &get_fields(&input, "#[derive(Object)]")?[0].ident;
 
-- 
2.47.1

[PATCH 05/12] rust: qom: automatically use Drop trait to implement instance_finalize

[Paolo Bonzini]

Replace the customizable INSTANCE_FINALIZE with a generic function
that drops the Rust object.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/qom.rs | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 1341a173893..861f1e50ac4 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -180,6 +180,16 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
     T::class_init(unsafe { &mut *klass.cast::<T::Class>() })
 }
 
+unsafe extern "C" fn drop_object<T: ObjectImpl>(obj: *mut Object) {
+    // SAFETY: obj is an instance of T, since drop_object<T> is called
+    // from the QOM core function object_deinit() as the instance_finalize
+    // function for class T.  Note that while object_deinit() will drop the
+    // superclass field separately after this function returns, `T` must
+    // implement the unsafe trait ObjectType; the safety rules for the
+    // trait mandate that the parent field is manually dropped.
+    unsafe { std::ptr::drop_in_place(obj.cast::<T>()) }
+}
+
 /// Trait exposed by all structs corresponding to QOM objects.
 ///
 /// # Safety
@@ -438,7 +448,6 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> {
 
     /// Whether the object can be instantiated
     const ABSTRACT: bool = false;
-    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
@@ -474,7 +483,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> {
             None => None,
             Some(_) => Some(rust_instance_post_init::<Self>),
         },
-        instance_finalize: Self::INSTANCE_FINALIZE,
+        instance_finalize: Some(drop_object::<Self>),
         abstract_: Self::ABSTRACT,
         class_size: core::mem::size_of::<Self::Class>(),
         class_init: Some(rust_class_init::<Self>),
-- 
2.47.1

[PATCH 06/12] rust: qom: move device_id to PL011 class side

[Paolo Bonzini]

There is no need to monkeypatch DeviceId::Luminary into the already-initialized
PL011State.  Instead, now that we can define a class hierarchy, we can define
PL011Class and make device_id a field in there.

There is also no need anymore to have "Arm" as zero, so change DeviceId into a
wrapper for the array; all it does is provide an Index<hwaddr> implementation
because arrays can only be indexed by usize.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 59 +++++++++++++++-----------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 689202f4550..215f94a6e4a 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -5,7 +5,7 @@
 use core::ptr::{addr_of_mut, NonNull};
 use std::{
     ffi::CStr,
-    os::raw::{c_int, c_uchar, c_uint, c_void},
+    os::raw::{c_int, c_uint, c_void},
 };
 
 use qemu_api::{
@@ -14,7 +14,7 @@
     irq::InterruptSource,
     prelude::*,
     qdev::DeviceImpl,
-    qom::{ObjectImpl, ParentField},
+    qom::{ClassInitImpl, ObjectImpl, ParentField},
 };
 
 use crate::{
@@ -33,27 +33,20 @@
 /// QEMU sourced constant.
 pub const PL011_FIFO_DEPTH: u32 = 16;
 
-#[derive(Clone, Copy, Debug)]
-enum DeviceId {
-    #[allow(dead_code)]
-    Arm = 0,
-    Luminary,
-}
+#[derive(Clone, Copy)]
+struct DeviceId(&'static [u8; 8]);
 
 impl std::ops::Index<hwaddr> for DeviceId {
-    type Output = c_uchar;
+    type Output = u8;
 
     fn index(&self, idx: hwaddr) -> &Self::Output {
-        match self {
-            Self::Arm => &Self::PL011_ID_ARM[idx as usize],
-            Self::Luminary => &Self::PL011_ID_LUMINARY[idx as usize],
-        }
+        &self.0[idx as usize]
     }
 }
 
 impl DeviceId {
-    const PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1];
-    const PL011_ID_LUMINARY: [c_uchar; 8] = [0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1];
+    const ARM: Self = Self(&[0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1]);
+    const LUMINARY: Self = Self(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
 }
 
 // FIFOs use 32-bit indices instead of usize, for compatibility with
@@ -126,17 +119,28 @@ pub struct PL011State {
     pub clock: NonNull<Clock>,
     #[doc(alias = "migrate_clk")]
     pub migrate_clock: bool,
-    /// The byte string that identifies the device.
-    device_id: DeviceId,
 }
 
 qom_isa!(PL011State : SysBusDevice, DeviceState, Object);
 
+pub struct PL011Class {
+    parent_class: <SysBusDevice as ObjectType>::Class,
+    /// The byte string that identifies the device.
+    device_id: DeviceId,
+}
+
 unsafe impl ObjectType for PL011State {
-    type Class = <SysBusDevice as ObjectType>::Class;
+    type Class = PL011Class;
     const TYPE_NAME: &'static CStr = crate::TYPE_PL011;
 }
 
+impl ClassInitImpl<PL011Class> for PL011State {
+    fn class_init(klass: &mut PL011Class) {
+        klass.device_id = DeviceId::ARM;
+        <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
+    }
+}
+
 impl ObjectImpl for PL011State {
     type ParentType = SysBusDevice;
 
@@ -214,7 +218,8 @@ pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow<u
 
         let value = match RegisterOffset::try_from(offset) {
             Err(v) if (0x3f8..0x400).contains(&(v >> 2)) => {
-                u32::from(self.device_id[(offset - 0xfe0) >> 2])
+                let device_id = self.get_class().device_id;
+                u32::from(device_id[(offset - 0xfe0) >> 2])
             }
             Err(_) => {
                 // qemu_log_mask(LOG_GUEST_ERROR, "pl011_read: Bad offset 0x%x\n", (int)offset);
@@ -648,16 +653,10 @@ pub struct PL011Luminary {
     parent_obj: ParentField<PL011State>,
 }
 
-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;
+impl ClassInitImpl<PL011Class> for PL011Luminary {
+    fn class_init(klass: &mut PL011Class) {
+        klass.device_id = DeviceId::LUMINARY;
+        <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
     }
 }
 
@@ -670,8 +669,6 @@ unsafe impl ObjectType for PL011Luminary {
 
 impl ObjectImpl for PL011Luminary {
     type ParentType = PL011State;
-
-    const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
 }
 
 impl DeviceImpl for PL011Luminary {}
-- 
2.47.1

[PATCH 07/12] rust: pl011: only leave embedded object initialization in instance_init

[Paolo Bonzini]

Leave IRQ and MMIO initialization to instance_post_init.  In Rust the
two callbacks are more distinct, because only instance_post_init has a
fully initialized object available.

While at it, add a wrapper for sysbus_init_mmio so that accesses to
the SysBusDevice correctly use shared references.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 18 ++++++++++--------
 rust/qemu-api/src/sysbus.rs      | 12 ++++++++++++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 215f94a6e4a..72a4cea042c 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -145,6 +145,7 @@ impl ObjectImpl for PL011State {
     type ParentType = SysBusDevice;
 
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
+    const INSTANCE_POST_INIT: Option<fn(&mut Self)> = Some(Self::post_init);
 }
 
 impl DeviceImpl for PL011State {
@@ -183,14 +184,6 @@ unsafe fn init(&mut self) {
                 Self::TYPE_NAME.as_ptr(),
                 0x1000,
             );
-
-            let sbd: &mut SysBusDevice = self.upcast_mut();
-            sysbus_init_mmio(sbd, addr_of_mut!(self.iomem));
-        }
-
-        for irq in self.interrupts.iter() {
-            let sbd: &SysBusDevice = self.upcast();
-            sbd.init_irq(irq);
         }
 
         // SAFETY:
@@ -213,6 +206,15 @@ unsafe fn init(&mut self) {
         }
     }
 
+    fn post_init(&mut self) {
+        let sbd: &SysBusDevice = self.upcast();
+
+        sbd.init_mmio(&self.iomem);
+        for irq in self.interrupts.iter() {
+            sbd.init_irq(irq);
+        }
+    }
+
     pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> std::ops::ControlFlow<u64, u64> {
         use RegisterOffset::*;
 
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index 8193734bde4..b96eaaf25f2 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -38,6 +38,18 @@ const fn as_mut_ptr(&self) -> *mut SysBusDevice {
         addr_of!(*self) as *mut _
     }
 
+    /// Expose a memory region to the board so that it can give it an address
+    /// in guest memory.  Note that the ordering of calls to `init_mmio` is
+    /// important, since whoever creates the sysbus device will refer to the
+    /// region with a number that corresponds to the order of calls to
+    /// `init_mmio`.
+    pub fn init_mmio(&self, iomem: &bindings::MemoryRegion) {
+        assert!(bql_locked());
+        unsafe {
+            bindings::sysbus_init_mmio(self.as_mut_ptr(), addr_of!(*iomem) 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
-- 
2.47.1

[PATCH 08/12] rust: qom: make INSTANCE_POST_INIT take a shared reference

[Paolo Bonzini]

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

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 72a4cea042c..6792d13fb77 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -145,7 +145,7 @@ impl ObjectImpl for PL011State {
     type ParentType = SysBusDevice;
 
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
-    const INSTANCE_POST_INIT: Option<fn(&mut Self)> = Some(Self::post_init);
+    const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init);
 }
 
 impl DeviceImpl for PL011State {
@@ -206,7 +206,7 @@ unsafe fn init(&mut self) {
         }
     }
 
-    fn post_init(&mut self) {
+    fn post_init(&self) {
         let sbd: &SysBusDevice = self.upcast();
 
         sbd.init_mmio(&self.iomem);
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 861f1e50ac4..5934eb0cc50 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -163,11 +163,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
     // 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>() })
+    T::INSTANCE_POST_INIT.unwrap()(unsafe { &*obj.cast::<T>() })
 }
 
 unsafe extern "C" fn rust_class_init<T: ObjectType + ClassInitImpl<T::Class>>(
@@ -459,7 +455,7 @@ pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> {
 
     /// 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 INSTANCE_POST_INIT: Option<fn(&Self)> = None;
 
     /// Called on descendent classes after all parent class initialization
     /// has occurred, but before the class itself is initialized.  This
-- 
2.47.1

[PATCH 09/12] rust: qdev: expose inherited methods to subclasses of SysBusDevice

[Paolo Bonzini]

The ObjectDeref trait now provides all the magic that is required to fake
inheritance.  Replace the "impl SysBusDevice" block of qemu_api::sysbus
with a trait, so that sysbus_init_irq() can be invoked as "self.init_irq()"
without any intermediate upcast.

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

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 6792d13fb77..994c2fc0593 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -207,11 +207,9 @@ unsafe fn init(&mut self) {
     }
 
     fn post_init(&self) {
-        let sbd: &SysBusDevice = self.upcast();
-
-        sbd.init_mmio(&self.iomem);
+        self.init_mmio(&self.iomem);
         for irq in self.interrupts.iter() {
-            sbd.init_irq(irq);
+            self.init_irq(irq);
         }
     }
 
diff --git a/rust/qemu-api/src/irq.rs b/rust/qemu-api/src/irq.rs
index 6258141bdf0..378e5202951 100644
--- a/rust/qemu-api/src/irq.rs
+++ b/rust/qemu-api/src/irq.rs
@@ -24,8 +24,7 @@
 ///
 /// 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
+/// a function such as [`SysBusDeviceMethods::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
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 6f32deeb2ed..4ea70b9c823 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -16,3 +16,5 @@
 pub use crate::qom::ObjectType;
 
 pub use crate::qom_isa;
+
+pub use crate::sysbus::SysBusDeviceMethods;
diff --git a/rust/qemu-api/src/sysbus.rs b/rust/qemu-api/src/sysbus.rs
index b96eaaf25f2..e6762b5c145 100644
--- a/rust/qemu-api/src/sysbus.rs
+++ b/rust/qemu-api/src/sysbus.rs
@@ -32,18 +32,17 @@ fn class_init(sdc: &mut SysBusDeviceClass) {
     }
 }
 
-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 _
-    }
-
+/// Trait for methods of [`SysBusDevice`] and its subclasses.
+pub trait SysBusDeviceMethods: ObjectDeref
+where
+    Self::Target: IsA<SysBusDevice>,
+{
     /// Expose a memory region to the board so that it can give it an address
     /// in guest memory.  Note that the ordering of calls to `init_mmio` is
     /// important, since whoever creates the sysbus device will refer to the
     /// region with a number that corresponds to the order of calls to
     /// `init_mmio`.
-    pub fn init_mmio(&self, iomem: &bindings::MemoryRegion) {
+    fn init_mmio(&self, iomem: &bindings::MemoryRegion) {
         assert!(bql_locked());
         unsafe {
             bindings::sysbus_init_mmio(self.as_mut_ptr(), addr_of!(*iomem) as *mut _);
@@ -54,10 +53,12 @@ pub fn init_mmio(&self, iomem: &bindings::MemoryRegion) {
     /// 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) {
+    fn init_irq(&self, irq: &InterruptSource) {
         assert!(bql_locked());
         unsafe {
             bindings::sysbus_init_irq(self.as_mut_ptr(), irq.as_ptr());
         }
     }
 }
+
+impl<R: ObjectDeref> SysBusDeviceMethods for R where R::Target: IsA<SysBusDevice> {}
-- 
2.47.1

[PATCH 10/12] rust: qemu-api-macros: extend error reporting facility to parse errors

[Paolo Bonzini]

Generalize the CompileError tuple to an enum, that can be either an error
message or a parse error from syn.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api-macros/src/lib.rs   | 27 ++++++++++-----------------
 rust/qemu-api-macros/src/utils.rs | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 17 deletions(-)
 create mode 100644 rust/qemu-api-macros/src/utils.rs

diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 0f04cca3841..539c48df298 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -3,57 +3,50 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 use proc_macro::TokenStream;
-use proc_macro2::Span;
-use quote::{quote, quote_spanned};
+use quote::quote;
 use syn::{
     parse_macro_input, parse_quote, punctuated::Punctuated, token::Comma, Data, DeriveInput, Field,
     Fields, Ident, Type, Visibility,
 };
 
-struct CompileError(String, Span);
-
-impl From<CompileError> for proc_macro2::TokenStream {
-    fn from(err: CompileError) -> Self {
-        let CompileError(msg, span) = err;
-        quote_spanned! { span => compile_error!(#msg); }
-    }
-}
+mod utils;
+use utils::MacroError;
 
 fn get_fields<'a>(
     input: &'a DeriveInput,
     msg: &str,
-) -> Result<&'a Punctuated<Field, Comma>, CompileError> {
+) -> Result<&'a Punctuated<Field, Comma>, MacroError> {
     if let Data::Struct(s) = &input.data {
         if let Fields::Named(fs) = &s.fields {
             Ok(&fs.named)
         } else {
-            Err(CompileError(
+            Err(MacroError::Message(
                 format!("Named fields required for {}", msg),
                 input.ident.span(),
             ))
         }
     } else {
-        Err(CompileError(
+        Err(MacroError::Message(
             format!("Struct required for {}", msg),
             input.ident.span(),
         ))
     }
 }
 
-fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), CompileError> {
+fn is_c_repr(input: &DeriveInput, msg: &str) -> Result<(), MacroError> {
     let expected = parse_quote! { #[repr(C)] };
 
     if input.attrs.iter().any(|attr| attr == &expected) {
         Ok(())
     } else {
-        Err(CompileError(
+        Err(MacroError::Message(
             format!("#[repr(C)] required for {}", msg),
             input.ident.span(),
         ))
     }
 }
 
-fn derive_object_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, CompileError> {
+fn derive_object_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, MacroError> {
     is_c_repr(&input, "#[derive(Object)]")?;
 
     let name = &input.ident;
@@ -80,7 +73,7 @@ pub fn derive_object(input: TokenStream) -> TokenStream {
 }
 
 #[rustfmt::skip::macros(quote)]
-fn derive_offsets_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, CompileError> {
+fn derive_offsets_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, MacroError> {
     is_c_repr(&input, "#[derive(offsets)]")?;
 
     let name = &input.ident;
diff --git a/rust/qemu-api-macros/src/utils.rs b/rust/qemu-api-macros/src/utils.rs
new file mode 100644
index 00000000000..02c91aed7f6
--- /dev/null
+++ b/rust/qemu-api-macros/src/utils.rs
@@ -0,0 +1,26 @@
+// Procedural macro utilities.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use proc_macro2::Span;
+use quote::quote_spanned;
+
+pub enum MacroError {
+    Message(String, Span),
+    ParseError(syn::Error),
+}
+
+impl From<syn::Error> for MacroError {
+    fn from(err: syn::Error) -> Self {
+        MacroError::ParseError(err)
+    }
+}
+
+impl From<MacroError> for proc_macro2::TokenStream {
+    fn from(err: MacroError) -> Self {
+        match err {
+            MacroError::Message(msg, span) => quote_spanned! { span => compile_error!(#msg); },
+            MacroError::ParseError(err) => err.into_compile_error(),
+        }
+    }
+}
-- 
2.47.1

[PATCH 11/12] rust: qemu-api-macros: add automatic TryFrom/TryInto derivation

[Paolo Bonzini]

This is going to be fairly common. Using a custom procedural macro
provides better error messages and automatically finds the right
type.

Note that this is different from the same-named macro in the
derive_more crate.  That one provides conversion from e.g. tuples
to enums with tuple variants, not from integers to enums.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/lib.rs   | 28 +------------
 rust/qemu-api-macros/src/lib.rs | 74 ++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 69064d6929b..0a89d393e0f 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -45,7 +45,7 @@
 #[doc(alias = "offset")]
 #[allow(non_camel_case_types)]
 #[repr(u64)]
-#[derive(Debug)]
+#[derive(Debug, qemu_api_macros::TryInto)]
 pub enum RegisterOffset {
     /// Data Register
     ///
@@ -102,32 +102,6 @@ pub enum RegisterOffset {
     //Reserved = 0x04C,
 }
 
-impl core::convert::TryFrom<u64> for RegisterOffset {
-    type Error = u64;
-
-    fn try_from(value: u64) -> Result<Self, Self::Error> {
-        macro_rules! case {
-            ($($discriminant:ident),*$(,)*) => {
-                /* check that matching on all macro arguments compiles, which means we are not
-                 * missing any enum value; if the type definition ever changes this will stop
-                 * compiling.
-                 */
-                const fn _assert_exhaustive(val: RegisterOffset) {
-                    match val {
-                        $(RegisterOffset::$discriminant => (),)*
-                    }
-                }
-
-                match value {
-                    $(x if x == Self::$discriminant as u64 => Ok(Self::$discriminant),)*
-                     _ => Err(value),
-                }
-            }
-        }
-        case! { DR, RSR, FR, FBRD, ILPR, IBRD, LCR_H, CR, FLS, IMSC, RIS, MIS, ICR, DMACR }
-    }
-}
-
 pub mod registers {
     //! Device registers exposed as typed structs which are backed by arbitrary
     //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 539c48df298..7ec218202f4 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -5,8 +5,8 @@
 use proc_macro::TokenStream;
 use quote::quote;
 use syn::{
-    parse_macro_input, parse_quote, punctuated::Punctuated, token::Comma, Data, DeriveInput, Field,
-    Fields, Ident, Type, Visibility,
+    parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Data,
+    DeriveInput, Field, Fields, Ident, Meta, Path, Token, Type, Variant, Visibility,
 };
 
 mod utils;
@@ -98,3 +98,73 @@ pub fn derive_offsets(input: TokenStream) -> TokenStream {
 
     TokenStream::from(expanded)
 }
+
+#[allow(non_snake_case)]
+fn get_repr_uN(input: &DeriveInput, msg: &str) -> Result<Path, MacroError> {
+    let repr = input.attrs.iter().find(|attr| attr.path().is_ident("repr"));
+    if let Some(repr) = repr {
+        let nested = repr.parse_args_with(Punctuated::<Meta, Token![,]>::parse_terminated)?;
+        for meta in nested {
+            match meta {
+                Meta::Path(path) if path.is_ident("u8") => return Ok(path),
+                Meta::Path(path) if path.is_ident("u16") => return Ok(path),
+                Meta::Path(path) if path.is_ident("u32") => return Ok(path),
+                Meta::Path(path) if path.is_ident("u64") => return Ok(path),
+                _ => {}
+            }
+        }
+    }
+
+    Err(MacroError::Message(
+        format!("#[repr(u8/u16/u32/u64) required for {}", msg),
+        input.ident.span(),
+    ))
+}
+
+fn get_variants(input: &DeriveInput) -> Result<&Punctuated<Variant, Comma>, MacroError> {
+    if let Data::Enum(e) = &input.data {
+        if let Some(v) = e.variants.iter().find(|v| v.fields != Fields::Unit) {
+            return Err(MacroError::Message(
+                "Cannot derive TryInto for enum with non-unit variants.".to_string(),
+                v.fields.span(),
+            ));
+        }
+        Ok(&e.variants)
+    } else {
+        Err(MacroError::Message(
+            "Cannot derive TryInto for union or struct.".to_string(),
+            input.ident.span(),
+        ))
+    }
+}
+
+#[rustfmt::skip::macros(quote)]
+fn derive_tryinto_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream, MacroError> {
+    let repr = get_repr_uN(&input, "#[derive(TryInto)]")?;
+
+    let name = &input.ident;
+    let variants = get_variants(&input)?;
+    let discriminants: Vec<&Ident> = variants.iter().map(|f| &f.ident).collect();
+
+    Ok(quote! {
+        impl core::convert::TryFrom<#repr> for #name {
+            type Error = #repr;
+
+            fn try_from(value: #repr) -> Result<Self, Self::Error> {
+                #(const #discriminants: #repr = #name::#discriminants as #repr;)*;
+                match value {
+                    #(#discriminants => Ok(Self::#discriminants),)*
+                    _ => Err(value),
+                }
+            }
+        }
+    })
+}
+
+#[proc_macro_derive(TryInto)]
+pub fn derive_tryinto(input: TokenStream) -> TokenStream {
+    let input = parse_macro_input!(input as DeriveInput);
+    let expanded = derive_tryinto_or_error(input).unwrap_or_else(Into::into);
+
+    TokenStream::from(expanded)
+}
-- 
2.47.1

[PATCH 12/12] rust: hide warnings for subprojects

[Paolo Bonzini]

This matches cargo's own usage of "--cap-lints allow" when building dependencies.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/arbitrary-int-1-rs.wrap           |  3 +++
 subprojects/bilge-0.2-rs.wrap                 |  3 +++
 subprojects/bilge-impl-0.2-rs.wrap            |  3 +++
 subprojects/either-1-rs.wrap                  |  3 +++
 subprojects/itertools-0.11-rs.wrap            |  3 +++
 .../arbitrary-int-1-rs/meson.build            |  1 +
 .../packagefiles/bilge-0.2-rs/meson.build     |  1 +
 .../bilge-impl-0.2-rs/meson.build             |  1 +
 .../packagefiles/either-1-rs/meson.build      |  1 +
 .../itertools-0.11-rs/meson.build             |  1 +
 .../proc-macro-error-1-rs/meson.build         |  1 +
 .../proc-macro-error-attr-1-rs/meson.build    |  1 +
 .../packagefiles/proc-macro2-1-rs/meson.build |  1 +
 .../packagefiles/quote-1-rs/meson.build       |  1 +
 subprojects/packagefiles/syn-2-rs/meson.build |  1 +
 .../unicode-ident-1-rs/meson.build            |  1 +
 subprojects/proc-macro-error-1-rs.wrap        |  3 +++
 subprojects/proc-macro-error-attr-1-rs.wrap   |  3 +++
 subprojects/proc-macro2-1-rs.wrap             |  3 +++
 subprojects/quote-1-rs.wrap                   |  3 +++
 subprojects/syn-2-rs.wrap                     |  3 +++
 subprojects/unicode-ident-1-rs.wrap           |  3 +++
 subprojects/unicode-ident-1-rs/meson.build    | 20 -------------------
 23 files changed, 44 insertions(+), 20 deletions(-)
 delete mode 100644 subprojects/unicode-ident-1-rs/meson.build

diff --git a/subprojects/arbitrary-int-1-rs.wrap b/subprojects/arbitrary-int-1-rs.wrap
index e580538a877..a1838b20b0f 100644
--- a/subprojects/arbitrary-int-1-rs.wrap
+++ b/subprojects/arbitrary-int-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = arbitrary-int-1.2.7.tar.gz
 source_hash = c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d
 #method = cargo
 patch_directory = arbitrary-int-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/bilge-0.2-rs.wrap b/subprojects/bilge-0.2-rs.wrap
index 7a4339d2989..900bb1497b9 100644
--- a/subprojects/bilge-0.2-rs.wrap
+++ b/subprojects/bilge-0.2-rs.wrap
@@ -5,3 +5,6 @@ source_filename = bilge-0.2.0.tar.gz
 source_hash = dc707ed8ebf81de5cd6c7f48f54b4c8621760926cdf35a57000747c512e67b57
 #method = cargo
 patch_directory = bilge-0.2-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/bilge-impl-0.2-rs.wrap b/subprojects/bilge-impl-0.2-rs.wrap
index b24c34a9043..d14c3dc769b 100644
--- a/subprojects/bilge-impl-0.2-rs.wrap
+++ b/subprojects/bilge-impl-0.2-rs.wrap
@@ -6,3 +6,6 @@ source_hash = feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8
 #method = cargo
 patch_directory = bilge-impl-0.2-rs
 diff_files = bilge-impl-1.63.0.patch
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/either-1-rs.wrap b/subprojects/either-1-rs.wrap
index 6046712036c..352e11cfee6 100644
--- a/subprojects/either-1-rs.wrap
+++ b/subprojects/either-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = either-1.12.0.tar.gz
 source_hash = 3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b
 #method = cargo
 patch_directory = either-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/itertools-0.11-rs.wrap b/subprojects/itertools-0.11-rs.wrap
index 66b05252cd5..ee12d0053bc 100644
--- a/subprojects/itertools-0.11-rs.wrap
+++ b/subprojects/itertools-0.11-rs.wrap
@@ -5,3 +5,6 @@ source_filename = itertools-0.11.0.tar.gz
 source_hash = b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57
 #method = cargo
 patch_directory = itertools-0.11-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/packagefiles/arbitrary-int-1-rs/meson.build b/subprojects/packagefiles/arbitrary-int-1-rs/meson.build
index cff3f62ce74..00733d1faab 100644
--- a/subprojects/packagefiles/arbitrary-int-1-rs/meson.build
+++ b/subprojects/packagefiles/arbitrary-int-1-rs/meson.build
@@ -9,6 +9,7 @@ _arbitrary_int_rs = static_library(
   files('src/lib.rs'),
   gnu_symbol_visibility: 'hidden',
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
+  rust_args: ['--cap-lints', 'allow'],
   rust_abi: 'rust',
   dependencies: [],
 )
diff --git a/subprojects/packagefiles/bilge-0.2-rs/meson.build b/subprojects/packagefiles/bilge-0.2-rs/meson.build
index e69bac91b40..ce13d0fe80f 100644
--- a/subprojects/packagefiles/bilge-0.2-rs/meson.build
+++ b/subprojects/packagefiles/bilge-0.2-rs/meson.build
@@ -17,6 +17,7 @@ lib = static_library(
   'src/lib.rs',
   override_options : ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi : 'rust',
+  rust_args: ['--cap-lints', 'allow'],
   dependencies: [
     arbitrary_int_dep,
     bilge_impl_dep,
diff --git a/subprojects/packagefiles/bilge-impl-0.2-rs/meson.build b/subprojects/packagefiles/bilge-impl-0.2-rs/meson.build
index f8f3486fc08..42b03dcd53c 100644
--- a/subprojects/packagefiles/bilge-impl-0.2-rs/meson.build
+++ b/subprojects/packagefiles/bilge-impl-0.2-rs/meson.build
@@ -25,6 +25,7 @@ _bilge_impl_rs = rust.proc_macro(
   files('src/lib.rs'),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'use_fallback',
     '--cfg', 'feature="syn-error"',
     '--cfg', 'feature="proc-macro"',
diff --git a/subprojects/packagefiles/either-1-rs/meson.build b/subprojects/packagefiles/either-1-rs/meson.build
index 608e64e31fc..04c96cc5fb3 100644
--- a/subprojects/packagefiles/either-1-rs/meson.build
+++ b/subprojects/packagefiles/either-1-rs/meson.build
@@ -11,6 +11,7 @@ _either_rs = static_library(
   override_options: ['rust_std=2018', 'build.rust_std=2018'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'feature="use_std"',
     '--cfg', 'feature="use_alloc"',
   ],
diff --git a/subprojects/packagefiles/itertools-0.11-rs/meson.build b/subprojects/packagefiles/itertools-0.11-rs/meson.build
index 30982a4ee76..2a3fbe9ee5a 100644
--- a/subprojects/packagefiles/itertools-0.11-rs/meson.build
+++ b/subprojects/packagefiles/itertools-0.11-rs/meson.build
@@ -15,6 +15,7 @@ _itertools_rs = static_library(
   override_options: ['rust_std=2018', 'build.rust_std=2018'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'feature="use_std"',
     '--cfg', 'feature="use_alloc"',
   ],
diff --git a/subprojects/packagefiles/proc-macro-error-1-rs/meson.build b/subprojects/packagefiles/proc-macro-error-1-rs/meson.build
index ae27a696862..10c2741085c 100644
--- a/subprojects/packagefiles/proc-macro-error-1-rs/meson.build
+++ b/subprojects/packagefiles/proc-macro-error-1-rs/meson.build
@@ -20,6 +20,7 @@ _proc_macro_error_rs = static_library(
   override_options: ['rust_std=2018', 'build.rust_std=2018'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'use_fallback',
     '--cfg', 'feature="syn-error"',
     '--cfg', 'feature="proc-macro"',
diff --git a/subprojects/packagefiles/proc-macro-error-attr-1-rs/meson.build b/subprojects/packagefiles/proc-macro-error-attr-1-rs/meson.build
index 3281b264333..c4c4c5e397c 100644
--- a/subprojects/packagefiles/proc-macro-error-attr-1-rs/meson.build
+++ b/subprojects/packagefiles/proc-macro-error-attr-1-rs/meson.build
@@ -16,6 +16,7 @@ _proc_macro_error_attr_rs = rust.proc_macro(
   files('src/lib.rs'),
   override_options: ['rust_std=2018', 'build.rust_std=2018'],
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'use_fallback',
     '--cfg', 'feature="syn-error"',
     '--cfg', 'feature="proc-macro"'
diff --git a/subprojects/packagefiles/proc-macro2-1-rs/meson.build b/subprojects/packagefiles/proc-macro2-1-rs/meson.build
index f9c8675ebad..5759df3ecc9 100644
--- a/subprojects/packagefiles/proc-macro2-1-rs/meson.build
+++ b/subprojects/packagefiles/proc-macro2-1-rs/meson.build
@@ -15,6 +15,7 @@ _proc_macro2_rs = static_library(
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'feature="proc-macro"',
     '--cfg', 'no_literal_byte_character',
     '--cfg', 'no_literal_c_string',
diff --git a/subprojects/packagefiles/quote-1-rs/meson.build b/subprojects/packagefiles/quote-1-rs/meson.build
index 7f7792569b9..bf41fad99bb 100644
--- a/subprojects/packagefiles/quote-1-rs/meson.build
+++ b/subprojects/packagefiles/quote-1-rs/meson.build
@@ -15,6 +15,7 @@ _quote_rs = static_library(
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'feature="proc-macro"',
   ],
   dependencies: [
diff --git a/subprojects/packagefiles/syn-2-rs/meson.build b/subprojects/packagefiles/syn-2-rs/meson.build
index 2c62cf7e1b0..a0094174084 100644
--- a/subprojects/packagefiles/syn-2-rs/meson.build
+++ b/subprojects/packagefiles/syn-2-rs/meson.build
@@ -19,6 +19,7 @@ _syn_rs = static_library(
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
   rust_args: [
+    '--cap-lints', 'allow',
     '--cfg', 'feature="full"',
     '--cfg', 'feature="derive"',
     '--cfg', 'feature="parsing"',
diff --git a/subprojects/packagefiles/unicode-ident-1-rs/meson.build b/subprojects/packagefiles/unicode-ident-1-rs/meson.build
index 9d76ebbd1ad..11a5dab97df 100644
--- a/subprojects/packagefiles/unicode-ident-1-rs/meson.build
+++ b/subprojects/packagefiles/unicode-ident-1-rs/meson.build
@@ -10,6 +10,7 @@ _unicode_ident_rs = static_library(
   gnu_symbol_visibility: 'hidden',
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
+  rust_args: ['--cap-lints', 'allow'],
   dependencies: [],
   native: true,
 )
diff --git a/subprojects/proc-macro-error-1-rs.wrap b/subprojects/proc-macro-error-1-rs.wrap
index b7db03b06a0..59f892f7825 100644
--- a/subprojects/proc-macro-error-1-rs.wrap
+++ b/subprojects/proc-macro-error-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = proc-macro-error-1.0.4.tar.gz
 source_hash = da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c
 #method = cargo
 patch_directory = proc-macro-error-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/proc-macro-error-attr-1-rs.wrap b/subprojects/proc-macro-error-attr-1-rs.wrap
index d13d8a239ac..5aeb224a103 100644
--- a/subprojects/proc-macro-error-attr-1-rs.wrap
+++ b/subprojects/proc-macro-error-attr-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = proc-macro-error-attr-1.0.4.tar.gz
 source_hash = a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869
 #method = cargo
 patch_directory = proc-macro-error-attr-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/proc-macro2-1-rs.wrap b/subprojects/proc-macro2-1-rs.wrap
index 7053e2c013c..6c9369f0df3 100644
--- a/subprojects/proc-macro2-1-rs.wrap
+++ b/subprojects/proc-macro2-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = proc-macro2-1.0.84.0.tar.gz
 source_hash = ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6
 #method = cargo
 patch_directory = proc-macro2-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/quote-1-rs.wrap b/subprojects/quote-1-rs.wrap
index 6e7ea69049f..8b721dfa00b 100644
--- a/subprojects/quote-1-rs.wrap
+++ b/subprojects/quote-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = quote-1.0.36.0.tar.gz
 source_hash = 0fa76aaf39101c457836aec0ce2316dbdc3ab723cdda1c6bd4e6ad4208acaca7
 #method = cargo
 patch_directory = quote-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/syn-2-rs.wrap b/subprojects/syn-2-rs.wrap
index 13ffdac3c3b..d79cf750fb4 100644
--- a/subprojects/syn-2-rs.wrap
+++ b/subprojects/syn-2-rs.wrap
@@ -5,3 +5,6 @@ source_filename = syn-2.0.66.0.tar.gz
 source_hash = c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5
 #method = cargo
 patch_directory = syn-2-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/unicode-ident-1-rs.wrap b/subprojects/unicode-ident-1-rs.wrap
index 4609f96ed97..50988f612e2 100644
--- a/subprojects/unicode-ident-1-rs.wrap
+++ b/subprojects/unicode-ident-1-rs.wrap
@@ -5,3 +5,6 @@ source_filename = unicode-ident-1.0.12.tar.gz
 source_hash = 3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b
 #method = cargo
 patch_directory = unicode-ident-1-rs
+
+# bump this version number on every change to meson.build or the patches:
+# v2
diff --git a/subprojects/unicode-ident-1-rs/meson.build b/subprojects/unicode-ident-1-rs/meson.build
deleted file mode 100644
index 54f23768545..00000000000
--- a/subprojects/unicode-ident-1-rs/meson.build
+++ /dev/null
@@ -1,20 +0,0 @@
-project('unicode-ident-1-rs', 'rust',
-  version: '1.0.12',
-  license: '(MIT OR Apache-2.0) AND Unicode-DFS-2016',
-  default_options: [])
-
-_unicode_ident_rs = static_library(
-  'unicode_ident',
-  files('src/lib.rs'),
-  gnu_symbol_visibility: 'hidden',
-  override_options: ['rust_std=2021', 'build.rust_std=2021'],
-  rust_abi: 'rust',
-  dependencies: [],
-  native: true,
-)
-
-unicode_ident_dep = declare_dependency(
-  link_with: _unicode_ident_rs,
-)
-
-meson.override_dependency('unicode-ident-1-rs', unicode_ident_dep, native: true)
-- 
2.47.1

Re: [PATCH 00/12] Next round of qemu_api patches

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:42PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:42 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 00/12] Next round of qemu_api patches
> X-Mailer: git-send-email 2.47.1
> 
> This includes:
> 
> 1) the outcome of reviewing
>    https://lore.kernel.org/qemu-devel/20241209123717.99077-1-pbonzini@redhat.com/T/,
>    especially with respect to instance_finalize implementation and
>    Rust-defined subclasses (patches 1-6)
> 
> 2) the beginnings of separating instance_init/instance_post_init, making
>    the latter take &self (patches 7-9)
> 
> 3) extracting pl011's logic to convert an integer into an enum into a
>    #[derive(TryInto)] procedural macro (patches 10-11)
> 
> 4) a one-off change that I noticed while reading Cargo documentation
>    (patch 12)

This series is the v2 of https://lore.kernel.org/qemu-devel/20241219121216.416905-1-pbonzini@redhat.com/

I will go through this series this week. :)

> Zhao: I have placed these in rust-next, but feel free to ignore them for
> your next HPET submission.  I have also included in my tree your test
> additions for usage in CI, but I'm not including them here to give you
> the opportunity to post them with a commit message and a signed-off-by.
> If you don't have time, it's in safe hands. :)

If possible, I would really appreciate it if you could add it to your
series :-) It seems fragmented if I send it separately.

Thanks,
Zhao

Re: [PATCH 01/12] rust: qom: add ParentField

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:43PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:43 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 01/12] rust: qom: add ParentField
> X-Mailer: git-send-email 2.47.1
> 
> Add a type that, together with the C function object_deinit, ensures the
> correct drop order for QOM objects relative to their superclasses.
> 
> Right now it is not possible to implement the Drop trait for QOM classes
> that are defined in Rust, as the drop() function would not be called when
> the object goes away; instead what is called is ObjectImpl::INSTANCE_FINALIZE.
> It would be nice for INSTANCE_FINALIZE to just drop the object, but this has
> a problem: suppose you have
> 
>    pub struct MySuperclass {
>        parent: DeviceState,
>        field: Box<MyData>,
>        ...
>    }
> 
>    impl Drop for MySuperclass {
>        ...
>    }
> 
>    pub struct MySubclass {
>        parent: MySuperclass,
>        ...
>    }
> 
> and an instance_finalize implementation that is like
> 
>     unsafe extern "C" fn drop_object<T: ObjectImpl>(obj: *mut Object) {
>         unsafe { std::ptr::drop_in_place(obj.cast::<T>()) }
>     }
> 
> When instance_finalize is called for MySubclass, it will walk the struct's
> list of fields and call the drop method for MySuperclass.  Then, object_deinit
> recurses to the superclass and calls the same drop method again.  This
> will cause double-freeing of the Box<Data>.
> 
> What's happening here is that QOM wants to control the drop order of
> MySuperclass and MySubclass's fields.  To do so, the parent field must
> be marked ManuallyDrop<>, which is quite ugly.  Instead, add a wrapper
> type ParentField<> that is specific to QOM.  This hides the implementation
> detail of *what* is special about the ParentField, and will also be easy
> to check in the #[derive(Object)] macro.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs |  6 ++--
>  rust/qemu-api/src/qom.rs         | 56 +++++++++++++++++++++++++++++---
>  rust/qemu-api/tests/tests.rs     |  4 +--
>  3 files changed, 57 insertions(+), 9 deletions(-)

...

>  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
> @@ -151,8 +198,9 @@ fn as_ref(&self) -> &$parent {
>  ///
>  /// - the struct must be `#[repr(C)]`;
>  ///
> -/// - the first field of the struct must be of the instance struct corresponding
> -///   to the superclass, which is `ObjectImpl::ParentType`
> +/// - the first field of the struct must be of type
> +///   [`ParentField<T>`](ParentField), where `T` is the parent type
> +///   [`ObjectImpl::ParentType`]
>  ///
>  /// - likewise, the first field of the `Class` must be of the class struct
>  ///   corresponding to the superclass, which is `ObjectImpl::ParentType::Class`.

I think the "likewise" word should be deleted as well, since Class'
parent field doesn't need any wrapper because Class also doesn't have
finalize method. The remaining description is clear enough.

Others look good to me!

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 02/12] rust: add a utility module for compile-time type checks

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:44PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:44 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 02/12] rust: add a utility module for compile-time type
>  checks
> X-Mailer: git-send-email 2.47.1
> 
> It is relatively common in the low-level qemu_api code to assert that
> a field of a struct has a specific type; for example, it can be used
> to ensure that the fields match what the qemu_api and C code expects
> for safety.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/meson.build       |  1 +
>  rust/qemu-api/src/assertions.rs | 90 +++++++++++++++++++++++++++++++++
>  rust/qemu-api/src/lib.rs        |  1 +
>  3 files changed, 92 insertions(+)
>  create mode 100644 rust/qemu-api/src/assertions.rs
> 

Very useful! Previously I found qdev property macro lacks such type
check, but I falied to think of a good way to implement type_check in
Rust, and glad to see the correct approach! Besides qdev property, I
think vmstate also needs this.

And I think we can make the examples as the unit tests.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 03/12] rust: macros: check that the first field of a #[derive(Object)] struct is a ParentField

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:45PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:45 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 03/12] rust: macros: check that the first field of a
>  #[derive(Object)] struct is a ParentField
> X-Mailer: git-send-email 2.47.1
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api-macros/src/lib.rs | 61 +++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 25 deletions(-)

Good idea!

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 04/12] rust: macros: check that #[derive(Object)] requires #[repr(C)]

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:46PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:46 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 04/12] rust: macros: check that #[derive(Object)] requires
>  #[repr(C)]
> X-Mailer: git-send-email 2.47.1
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api-macros/src/lib.rs | 2 ++
>  1 file changed, 2 insertions(+)
> 

LGTM,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

(I think this patch would be best placed before patch 3. What do you
 think?)

Re: [PATCH 06/12] rust: qom: move device_id to PL011 class side

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:48PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:48 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 06/12] rust: qom: move device_id to PL011 class side
> X-Mailer: git-send-email 2.47.1
> 
> There is no need to monkeypatch DeviceId::Luminary into the already-initialized
> PL011State.  Instead, now that we can define a class hierarchy, we can define
> PL011Class and make device_id a field in there.
> 
> There is also no need anymore to have "Arm" as zero, so change DeviceId into a
> wrapper for the array; all it does is provide an Index<hwaddr> implementation
> because arrays can only be indexed by usize.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs | 59 +++++++++++++++-----------------
>  1 file changed, 28 insertions(+), 31 deletions(-)

...

>  impl DeviceId {
> -    const PL011_ID_ARM: [c_uchar; 8] = [0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1];
> -    const PL011_ID_LUMINARY: [c_uchar; 8] = [0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1];
> +    const ARM: Self = Self(&[0x11, 0x10, 0x14, 0x00, 0x0d, 0xf0, 0x05, 0xb1]);
> +    const LUMINARY: Self = Self(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
>  }
>  

It seems you didn't choose to hardcode device ID to save 3 lines of code :)

Both are fine for me,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 07/12] rust: pl011: only leave embedded object initialization in instance_init

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:49PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:49 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 07/12] rust: pl011: only leave embedded object
>  initialization in instance_init
> X-Mailer: git-send-email 2.47.1
> 
> Leave IRQ and MMIO initialization to instance_post_init.  In Rust the
> two callbacks are more distinct, because only instance_post_init has a
> fully initialized object available.
> 
> While at it, add a wrapper for sysbus_init_mmio so that accesses to
> the SysBusDevice correctly use shared references.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs | 18 ++++++++++--------
>  rust/qemu-api/src/sysbus.rs      | 12 ++++++++++++
>  2 files changed, 22 insertions(+), 8 deletions(-)
> 

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 08/12] rust: qom: make INSTANCE_POST_INIT take a shared reference

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:50PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:50 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 08/12] rust: qom: make INSTANCE_POST_INIT take a shared
>  reference
> X-Mailer: git-send-email 2.47.1
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs | 4 ++--
>  rust/qemu-api/src/qom.rs         | 8 ++------
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 09/12] rust: qdev: expose inherited methods to subclasses of SysBusDevice

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:51PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:51 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 09/12] rust: qdev: expose inherited methods to subclasses
>  of SysBusDevice
> X-Mailer: git-send-email 2.47.1
> 
> The ObjectDeref trait now provides all the magic that is required to fake
> inheritance.  Replace the "impl SysBusDevice" block of qemu_api::sysbus
> with a trait, so that sysbus_init_irq() can be invoked as "self.init_irq()"
> without any intermediate upcast.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs |  6 ++----
>  rust/qemu-api/src/irq.rs         |  3 +--
>  rust/qemu-api/src/prelude.rs     |  2 ++
>  rust/qemu-api/src/sysbus.rs      | 17 +++++++++--------
>  4 files changed, 14 insertions(+), 14 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 10/12] rust: qemu-api-macros: extend error reporting facility to parse errors

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:52PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:52 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 10/12] rust: qemu-api-macros: extend error reporting
>  facility to parse errors
> X-Mailer: git-send-email 2.47.1
> 
> Generalize the CompileError tuple to an enum, that can be either an error
> message or a parse error from syn.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api-macros/src/lib.rs   | 27 ++++++++++-----------------
>  rust/qemu-api-macros/src/utils.rs | 26 ++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 17 deletions(-)
>  create mode 100644 rust/qemu-api-macros/src/utils.rs
>

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 11/12] rust: qemu-api-macros: add automatic TryFrom/TryInto derivation

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:53PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:53 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 11/12] rust: qemu-api-macros: add automatic TryFrom/TryInto
>  derivation
> X-Mailer: git-send-email 2.47.1
> 
> This is going to be fairly common. Using a custom procedural macro
> provides better error messages and automatically finds the right
> type.
> 
> Note that this is different from the same-named macro in the
> derive_more crate.  That one provides conversion from e.g. tuples
> to enums with tuple variants, not from integers to enums.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/lib.rs   | 28 +------------
>  rust/qemu-api-macros/src/lib.rs | 74 ++++++++++++++++++++++++++++++++-
>  2 files changed, 73 insertions(+), 29 deletions(-)
>

This improvement is also very elegant.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 12/12] rust: hide warnings for subprojects

[Zhao Liu]

On Fri, Dec 20, 2024 at 03:29:54PM +0100, Paolo Bonzini wrote:
> Date: Fri, 20 Dec 2024 15:29:54 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 12/12] rust: hide warnings for subprojects
> X-Mailer: git-send-email 2.47.1
> 
> This matches cargo's own usage of "--cap-lints allow" when building dependencies.

+Add remove the unnecessary meson.build file.

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  subprojects/arbitrary-int-1-rs.wrap           |  3 +++
>  subprojects/bilge-0.2-rs.wrap                 |  3 +++
>  subprojects/bilge-impl-0.2-rs.wrap            |  3 +++
>  subprojects/either-1-rs.wrap                  |  3 +++
>  subprojects/itertools-0.11-rs.wrap            |  3 +++
>  .../arbitrary-int-1-rs/meson.build            |  1 +
>  .../packagefiles/bilge-0.2-rs/meson.build     |  1 +
>  .../bilge-impl-0.2-rs/meson.build             |  1 +
>  .../packagefiles/either-1-rs/meson.build      |  1 +
>  .../itertools-0.11-rs/meson.build             |  1 +
>  .../proc-macro-error-1-rs/meson.build         |  1 +
>  .../proc-macro-error-attr-1-rs/meson.build    |  1 +
>  .../packagefiles/proc-macro2-1-rs/meson.build |  1 +
>  .../packagefiles/quote-1-rs/meson.build       |  1 +
>  subprojects/packagefiles/syn-2-rs/meson.build |  1 +
>  .../unicode-ident-1-rs/meson.build            |  1 +
>  subprojects/proc-macro-error-1-rs.wrap        |  3 +++
>  subprojects/proc-macro-error-attr-1-rs.wrap   |  3 +++
>  subprojects/proc-macro2-1-rs.wrap             |  3 +++
>  subprojects/quote-1-rs.wrap                   |  3 +++
>  subprojects/syn-2-rs.wrap                     |  3 +++
>  subprojects/unicode-ident-1-rs.wrap           |  3 +++
>  subprojects/unicode-ident-1-rs/meson.build    | 20 -------------------
>  23 files changed, 44 insertions(+), 20 deletions(-)
>  delete mode 100644 subprojects/unicode-ident-1-rs/meson.build
> 

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 02/12] rust: add a utility module for compile-time type checks

[Paolo Bonzini]

On 12/25/24 17:24, Zhao Liu wrote:
> On Fri, Dec 20, 2024 at 03:29:44PM +0100, Paolo Bonzini wrote:
>> Date: Fri, 20 Dec 2024 15:29:44 +0100
>> From: Paolo Bonzini <pbonzini@redhat.com>
>> Subject: [PATCH 02/12] rust: add a utility module for compile-time type
>>   checks
>> X-Mailer: git-send-email 2.47.1
>>
>> It is relatively common in the low-level qemu_api code to assert that
>> a field of a struct has a specific type; for example, it can be used
>> to ensure that the fields match what the qemu_api and C code expects
>> for safety.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   rust/qemu-api/meson.build       |  1 +
>>   rust/qemu-api/src/assertions.rs | 90 +++++++++++++++++++++++++++++++++
>>   rust/qemu-api/src/lib.rs        |  1 +
>>   3 files changed, 92 insertions(+)
>>   create mode 100644 rust/qemu-api/src/assertions.rs
>>
> 
> Very useful! Previously I found qdev property macro lacks such type
> check, but I falied to think of a good way to implement type_check in
> Rust, and glad to see the correct approach! Besides qdev property, I
> think vmstate also needs this.

Right, though for vmstate (and probably qdev properties too) it's 
probably better to go from types to C structs 
(PropertyInfo/VMStateField) and avoid having to do the assertions.

> And I think we can make the examples as the unit tests.

Having doctests support in "make check" is on my list; I have started 
contributing a rust.doctest() function to Meson for that purpose.  For 
now they are limited to what can be run through Cargo (i.e. you cannot 
use libqemuutil.a functions) but we'll see if the function is accepted.

Paolo

> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> 
> 
>