qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] rust: (mostly) type safe VMState
@ 2025-01-17  9:00 Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 01/10] rust: vmstate: add new type safe implementation Paolo Bonzini
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

The existing translation of the C macros for vmstate does not make
any attempt to type-check vmstate declarations against the struct, so
introduce a new system that computes VMStateField based on the actual
struct declaration.

Macros do not have full access to the type system, therefore a full
implementation of this scheme requires a helper trait to analyze the type
and produce a VMStateField from it; the trait stores the VMStateField
as ana associated const.  Then, a macro "vmstate_of!" accepts arguments
similar to "offset_of!" and tricks the compiler into looking up the
trait for the right type.

Structs and cells have their own sub-VMStateDescription.  This poses
a problem in that references can only be included in associated consts
starting with Rust 1.83.0, when the const_refs_static was stabilized.
So for now structs and cells cannot be done in a type-safe manner,
but arrays, pointers and scalars can.

Technically, scalar types have the same issue in that they point to a
VMStateInfo.  For this case however there is onlyya limited list of
VMStateInfos, so I am placing them in an enum, and going from enum
to &VMStateInfo only when building the VMStateField (i.e. in a static's
value rather than in an associated const, and in a macro rather than in
a const fn).  This isn't great, but it's easily removed when QEMU starts
accepting a newer Rust version.

The series also introduces the pattern of applying changes to a struct
using methods with a fn(mut self) -> Self signature, for example

    pub const fn with_version_id(mut self, version_id: i32) -> Self {
        self.version_id = version_id;
        self
    }

This is not quite the builder pattern because there is no need for a
final ".build()" call, but it's similar/related.

The unfavorable diffstat comes half from the const_refs_static workaround,
half from new documentation (so in that case it's a plus rather than
a minus).  And also, there is more functionality and extensibility in
the new mechanism, whereas only a subset of the C macros was included
in the previous implementation.  The exception is "field_exists", which
may be added later if needed as a modifier, similar to with_version_id().

Paolo

v1->v2:
- add attribution for call_func_with_field macro
- add VMS_VARRAY_FLAGS, check it in with_array_flag
- remove Option<NonNull<T>> support, add Box<T>
- fix cut and paste typo in vmstate_varray_flag
- fix rustfmt
- move SCALAR_TYPE definitions to the right patch
- keep $num as an ident rather than tt
- remove vmstate_cell
- make order of parameters consistent in vmstate_clock (last patch, new)

Paolo Bonzini (10):
  rust: vmstate: add new type safe implementation
  rust: vmstate: implement VMState for non-leaf types
  rust: vmstate: add varray support to vmstate_of!
  rust: vmstate: implement Zeroable for VMStateField
  rust: vmstate: implement VMState for scalar types
  rust: vmstate: add public utility macros to implement VMState
  rust: qemu_api: add vmstate_struct
  rust: pl011: switch vmstate to new-style macros
  rust: vmstate: remove translation of C vmstate macros
  rust: vmstate: make order of parameters consistent in vmstate_clock

 rust/hw/char/pl011/src/device.rs       |   3 +-
 rust/hw/char/pl011/src/device_class.rs |  38 +-
 rust/hw/char/pl011/src/lib.rs          |   6 +
 rust/qemu-api/src/prelude.rs           |   2 +
 rust/qemu-api/src/vmstate.rs           | 700 ++++++++++++++++---------
 rust/qemu-api/src/zeroable.rs          |  31 ++
 6 files changed, 504 insertions(+), 276 deletions(-)

-- 
2.47.1



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

* [PATCH 01/10] rust: vmstate: add new type safe implementation
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

The existing translation of the C macros for vmstate does not make
any attempt to type-check vmstate declarations against the struct, so
introduce a new system that computes VMStateField based on the actual
struct declaration.

Macros do not have full access to the type system, therefore a full
implementation of this scheme requires a helper trait to analyze the
type and produce a VMStateField from it; a macro "vmstate_of!" accepts
arguments similar to "offset_of!" and tricks the compiler into looking
up the trait for the right type.

The patch introduces not just vmstate_of!, but also the slightly too
clever enabling macro call_func_with_field!.  The particular trick used
here was proposed on the users.rust-lang.org forum, so I take no merit
and all the blame.

Introduce the trait and some functions to access it; the actual
implementation comes later.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/prelude.rs |   2 +
 rust/qemu-api/src/vmstate.rs | 113 +++++++++++++++++++++++++++++++++--
 2 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index 4ea70b9c823..2dc86e19b29 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -18,3 +18,5 @@
 pub use crate::qom_isa;
 
 pub use crate::sysbus::SysBusDeviceMethods;
+
+pub use crate::vmstate::VMState;
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 63c897abcdf..b839a7d6b7f 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -4,13 +4,114 @@
 
 //! Helper macros to declare migration state for device models.
 //!
-//! Some macros are direct equivalents to the C macros declared in
-//! `include/migration/vmstate.h` while
-//! [`vmstate_subsections`](crate::vmstate_subsections) and
-//! [`vmstate_fields`](crate::vmstate_fields) are meant to be used when
-//! declaring a device model state struct.
+//! This module includes three families of macros:
+//!
+//! * [`vmstate_unused!`](crate::vmstate_unused) and
+//!   [`vmstate_of!`](crate::vmstate_of), which are used to express the
+//!   migration format for a struct.  This is based on the [`VMState`] trait,
+//!   which is defined by all migrateable types.
+//!
+//! * helper macros to declare a device model state struct, in particular
+//!   [`vmstate_subsections`](crate::vmstate_subsections) and
+//!   [`vmstate_fields`](crate::vmstate_fields).
+//!
+//! * direct equivalents to the C macros declared in
+//!   `include/migration/vmstate.h`. These are not type-safe and should not be
+//!   used if the equivalent functionality is available with `vmstate_of!`.
 
-pub use crate::bindings::VMStateDescription;
+use core::marker::PhantomData;
+
+pub use crate::bindings::{VMStateDescription, VMStateField};
+
+/// This macro is used to call a function with a generic argument bound
+/// to the type of a field.  The function must take a
+/// [`PhantomData`]`<T>` argument; `T` is the type of
+/// field `$field` in the `$typ` type.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::call_func_with_field;
+/// # use core::marker::PhantomData;
+/// const fn size_of_field<T>(_: PhantomData<T>) -> usize {
+///     std::mem::size_of::<T>()
+/// }
+///
+/// struct Foo {
+///     x: u16,
+/// };
+/// // calls size_of_field::<u16>()
+/// assert_eq!(call_func_with_field!(size_of_field, Foo, x), 2);
+/// ```
+#[macro_export]
+macro_rules! call_func_with_field {
+    // Based on the answer by user steffahn (Frank Steffahn) at
+    // https://users.rust-lang.org/t/inferring-type-of-field/122857
+    // and used under MIT license
+    ($func:expr, $typ:ty, $($field:tt).+) => {
+        $func(loop {
+            #![allow(unreachable_code)]
+            const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> { ::core::marker::PhantomData }
+            // Unreachable code is exempt from checks on uninitialized values.
+            // Use that trick to infer the type of this PhantomData.
+            break ::core::marker::PhantomData;
+            break phantom__(&{ let value__: $typ; value__.$($field).+ });
+        })
+    };
+}
+
+/// A trait for types that can be included in a device's migration stream.  It
+/// provides the base contents of a `VMStateField` (minus the name and offset).
+///
+/// # Safety
+///
+/// The contents of this trait go straight into structs that are parsed by C
+/// code and used to introspect into other structs.  Be careful.
+pub unsafe trait VMState {
+    /// The base contents of a `VMStateField` (minus the name and offset) for
+    /// the type that is implementing the trait.
+    const BASE: VMStateField;
+}
+
+/// Internal utility function to retrieve a type's `VMStateField`;
+/// used by [`vmstate_of!`](crate::vmstate_of).
+pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
+    T::BASE
+}
+
+/// Return the `VMStateField` for a field of a struct.  The field must be
+/// visible in the current scope.
+///
+/// In order to support other types, the trait `VMState` must be implemented
+/// for them.
+#[macro_export]
+macro_rules! vmstate_of {
+    ($struct_name:ty, $field_name:ident $(,)?) => {
+        $crate::bindings::VMStateField {
+            name: ::core::concat!(::core::stringify!($field_name), "\0")
+                .as_bytes()
+                .as_ptr() as *const ::std::os::raw::c_char,
+            offset: $crate::offset_of!($struct_name, $field_name),
+            // Compute most of the VMStateField from the type of the field.
+            ..$crate::call_func_with_field!(
+                $crate::vmstate::vmstate_base,
+                $struct_name,
+                $field_name
+            )
+        }
+    };
+}
+
+// Add a couple builder-style methods to VMStateField, allowing
+// easy derivation of VMStateField constants from other types.
+impl VMStateField {
+    #[must_use]
+    pub const fn with_version_id(mut self, version_id: i32) -> Self {
+        assert!(version_id >= 0);
+        self.version_id = version_id;
+        self
+    }
+}
 
 #[doc(alias = "VMSTATE_UNUSED_BUFFER")]
 #[macro_export]
-- 
2.47.1



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

* [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 01/10] rust: vmstate: add new type safe implementation Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-22 10:49   ` Zhao Liu
  2025-01-17  9:00 ` [PATCH 03/10] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

Arrays, pointers and cells use a VMStateField that is based on that
for the inner type.  The implementation therefore delegates to the
VMState implementation of the inner type.

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index b839a7d6b7f..abe15c96011 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -19,8 +19,9 @@
 //!   `include/migration/vmstate.h`. These are not type-safe and should not be
 //!   used if the equivalent functionality is available with `vmstate_of!`.
 
-use core::marker::PhantomData;
+use core::{marker::PhantomData, mem, ptr::NonNull};
 
+use crate::bindings::VMStateFlags;
 pub use crate::bindings::{VMStateDescription, VMStateField};
 
 /// This macro is used to call a function with a generic argument bound
@@ -102,6 +103,15 @@ macro_rules! vmstate_of {
     };
 }
 
+impl VMStateFlags {
+    const VMS_VARRAY_FLAGS: VMStateFlags = VMStateFlags(
+        VMStateFlags::VMS_VARRAY_INT32.0 |
+        VMStateFlags::VMS_VARRAY_UINT8.0 |
+        VMStateFlags::VMS_VARRAY_UINT16.0 |
+        VMStateFlags::VMS_VARRAY_UINT32.0
+    );
+}
+
 // Add a couple builder-style methods to VMStateField, allowing
 // easy derivation of VMStateField constants from other types.
 impl VMStateField {
@@ -111,6 +121,73 @@ pub const fn with_version_id(mut self, version_id: i32) -> Self {
         self.version_id = version_id;
         self
     }
+
+    #[must_use]
+    pub const fn with_array_flag(mut self, num: usize) -> Self {
+        assert!(num <= 0x7FFF_FFFFusize);
+        assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) == 0);
+        assert!((self.flags.0 & VMStateFlags::VMS_VARRAY_FLAGS.0) == 0);
+        if (self.flags.0 & VMStateFlags::VMS_POINTER.0) != 0 {
+            self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_POINTER.0);
+            self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0);
+        }
+        self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_SINGLE.0);
+        self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY.0);
+        self.num = num as i32;
+        self
+    }
+
+    #[must_use]
+    pub const fn with_pointer_flag(mut self) -> Self {
+        assert!((self.flags.0 & VMStateFlags::VMS_POINTER.0) == 0);
+        self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_POINTER.0);
+        self
+    }
+}
+
+// Transparent wrappers: just use the internal type
+
+macro_rules! impl_vmstate_transparent {
+    ($type:ty where $base:tt: VMState $($where:tt)*) => {
+        unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
+            const BASE: VMStateField = VMStateField {
+                size: mem::size_of::<$type>(),
+                ..<$base as VMState>::BASE
+            };
+        }
+    };
+}
+
+impl_vmstate_transparent!(std::cell::Cell<T> where T: VMState);
+impl_vmstate_transparent!(std::cell::UnsafeCell<T> where T: VMState);
+impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
+impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
+
+// Pointer types using the underlying type's VMState plus VMS_POINTER
+// Note that references are not supported, though references to cells
+// could be allowed.
+
+macro_rules! impl_vmstate_pointer {
+    ($type:ty where $base:tt: VMState $($where:tt)*) => {
+        unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
+            const BASE: VMStateField = <$base as VMState>::BASE.with_pointer_flag();
+        }
+    };
+}
+
+impl_vmstate_pointer!(*const T where T: VMState);
+impl_vmstate_pointer!(*mut T where T: VMState);
+impl_vmstate_pointer!(NonNull<T> where T: VMState);
+
+// Unlike C pointers, Box is always non-null therefore there is no need
+// to specify VMS_ALLOC.
+impl_vmstate_pointer!(Box<T> where T: VMState);
+
+// Arrays using the underlying type's VMState plus
+// VMS_ARRAY/VMS_ARRAY_OF_POINTER
+
+unsafe impl<T: VMState, const N: usize> VMState for [T; N] {
+    const BASE: VMStateField = <T as VMState>::BASE.with_array_flag(N);
 }
 
 #[doc(alias = "VMSTATE_UNUSED_BUFFER")]
-- 
2.47.1



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

* [PATCH 03/10] rust: vmstate: add varray support to vmstate_of!
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 01/10] rust: vmstate: add new type safe implementation Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-22 11:44   ` Zhao Liu
  2025-01-17  9:00 ` [PATCH 04/10] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index abe15c96011..3c3ed8510ab 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -72,6 +72,15 @@ pub unsafe trait VMState {
     /// The base contents of a `VMStateField` (minus the name and offset) for
     /// the type that is implementing the trait.
     const BASE: VMStateField;
+
+    /// A flag that is added to another field's `VMStateField` to specify the
+    /// length's type in a variable-sized array.  If this is not a supported
+    /// type for the length (i.e. if it is not `u8`, `u16`, `u32`), using it
+    /// in a call to [`vmstate_of!`](crate::vmstate_of) will cause a
+    /// compile-time error.
+    const VARRAY_FLAG: VMStateFlags = {
+        panic!("invalid type for variable-sized array");
+    };
 }
 
 /// Internal utility function to retrieve a type's `VMStateField`;
@@ -80,6 +89,13 @@ pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
     T::BASE
 }
 
+/// Internal utility function to retrieve a type's `VMStateFlags` when it
+/// is used as the element count of a `VMSTATE_VARRAY`; used by
+/// [`vmstate_of!`](crate::vmstate_of).
+pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField {
+    T::VARRAY_FLAG
+}
+
 /// Return the `VMStateField` for a field of a struct.  The field must be
 /// visible in the current scope.
 ///
@@ -87,18 +103,23 @@ pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
 /// for them.
 #[macro_export]
 macro_rules! vmstate_of {
-    ($struct_name:ty, $field_name:ident $(,)?) => {
+    ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
                 .as_ptr() as *const ::std::os::raw::c_char,
             offset: $crate::offset_of!($struct_name, $field_name),
             // Compute most of the VMStateField from the type of the field.
+            $(.num_offset: $crate::offset_of!($struct_name, $num),)?
             ..$crate::call_func_with_field!(
                 $crate::vmstate::vmstate_base,
                 $struct_name,
                 $field_name
-            )
+            )$(.with_varray_flag($crate::call_func_with_field!(
+                    $crate::vmstate::vmstate_varray_flag,
+                    $struct_name,
+                    $num))
+               $(.with_varray_multiply($factor))?)?
         }
     };
 }
@@ -143,6 +164,22 @@ pub const fn with_pointer_flag(mut self) -> Self {
         self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_POINTER.0);
         self
     }
+
+    #[must_use]
+    pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMStateField {
+        assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
+        self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
+        self.flags = VMStateFlags(self.flags.0 | flag.0);
+        self
+    }
+
+    #[must_use]
+    pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
+        assert!(num <= 0x7FFF_FFFFu32);
+        self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0);
+        self.num = num as i32;
+        self
+    }
 }
 
 // Transparent wrappers: just use the internal type
@@ -154,6 +191,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
                 size: mem::size_of::<$type>(),
                 ..<$base as VMState>::BASE
             };
+            const VARRAY_FLAG: VMStateFlags = <$base as VMState>::VARRAY_FLAG;
         }
     };
 }
-- 
2.47.1



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

* [PATCH 04/10] rust: vmstate: implement Zeroable for VMStateField
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (2 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 03/10] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 05/10] rust: vmstate: implement VMState for scalar types Paolo Bonzini
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

This shortens a bit the constants.  Do not bother using it
in the vmstate macros since most of them will go away soon.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/vmstate.rs  | 18 +++---------------
 rust/qemu-api/src/zeroable.rs | 31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 3c3ed8510ab..22a5ed50d4a 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -21,8 +21,8 @@
 
 use core::{marker::PhantomData, mem, ptr::NonNull};
 
-use crate::bindings::VMStateFlags;
 pub use crate::bindings::{VMStateDescription, VMStateField};
+use crate::bindings::VMStateFlags;
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -503,20 +503,8 @@ macro_rules! vmstate_fields {
         static _FIELDS: &[$crate::bindings::VMStateField] = &[
             $($field),*,
             $crate::bindings::VMStateField {
-                name: ::core::ptr::null(),
-                err_hint: ::core::ptr::null(),
-                offset: 0,
-                size: 0,
-                start: 0,
-                num: 0,
-                num_offset: 0,
-                size_offset: 0,
-                info: ::core::ptr::null(),
-                flags: VMStateFlags::VMS_END,
-                vmsd: ::core::ptr::null(),
-                version_id: 0,
-                struct_version_id: 0,
-                field_exists: None,
+                flags: $crate::bindings::VMStateFlags::VMS_END,
+                ..$crate::zeroable::Zeroable::ZERO
             }
         ];
         _FIELDS.as_ptr()
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 6125aeed8b4..57cac96de06 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -49,6 +49,37 @@ unsafe impl Zeroable for crate::bindings::Property {
     };
 }
 
+// bindgen does not derive Default here
+#[allow(clippy::derivable_impls)]
+impl Default for crate::bindings::VMStateFlags {
+    fn default() -> Self {
+        Self(0)
+    }
+}
+
+unsafe impl Zeroable for crate::bindings::VMStateFlags {
+    const ZERO: Self = Self(0);
+}
+
+unsafe impl Zeroable for crate::bindings::VMStateField {
+    const ZERO: Self = Self {
+        name: ptr::null(),
+        err_hint: ptr::null(),
+        offset: 0,
+        size: 0,
+        start: 0,
+        num: 0,
+        num_offset: 0,
+        size_offset: 0,
+        info: ptr::null(),
+        flags: Zeroable::ZERO,
+        vmsd: ptr::null(),
+        version_id: 0,
+        struct_version_id: 0,
+        field_exists: None,
+    };
+}
+
 unsafe impl Zeroable for crate::bindings::VMStateDescription {
     const ZERO: Self = Self {
         name: ptr::null(),
-- 
2.47.1



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

* [PATCH 05/10] rust: vmstate: implement VMState for scalar types
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (3 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 04/10] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-22 12:33   ` Zhao Liu
  2025-01-17  9:00 ` [PATCH 06/10] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

Scalar types are those that have their own VMStateInfo.  This poses
a problem in that references to VMStateInfo can only be included in
associated consts starting with Rust 1.83.0, when the const_refs_static
was stabilized.  Removing the requirement is done by placing a limited
list of VMStateInfos in an enum, and going from enum to &VMStateInfo
only when building the VMStateField.

The same thing cannot be done with VMS_STRUCT because the set of
VMStateDescriptions extends to structs defined by the devices.
Therefore, structs and cells cannot yet use vmstate_of!.

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 22a5ed50d4a..b0a2ce105cc 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -21,8 +21,11 @@
 
 use core::{marker::PhantomData, mem, ptr::NonNull};
 
-pub use crate::bindings::{VMStateDescription, VMStateField};
 use crate::bindings::VMStateFlags;
+pub use crate::{
+    bindings::{self, VMStateDescription, VMStateField},
+    zeroable::Zeroable,
+};
 
 /// This macro is used to call a function with a generic argument bound
 /// to the type of a field.  The function must take a
@@ -61,6 +64,70 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> { ::core::marker:
     };
 }
 
+/// Workaround for lack of `const_refs_static`: references to global variables
+/// can be included in a `static`, but not in a `const`; unfortunately, this
+/// is exactly what would go in the `VMStateField`'s `info` member.
+///
+/// This enum contains the contents of the `VMStateField`'s `info` member,
+/// but as an `enum` instead of a pointer.
+#[allow(non_camel_case_types)]
+pub enum VMStateFieldType {
+    null,
+    vmstate_info_bool,
+    vmstate_info_int8,
+    vmstate_info_int16,
+    vmstate_info_int32,
+    vmstate_info_int64,
+    vmstate_info_uint8,
+    vmstate_info_uint16,
+    vmstate_info_uint32,
+    vmstate_info_uint64,
+    vmstate_info_timer,
+}
+
+/// Workaround for lack of `const_refs_static`.  Converts a `VMStateFieldType`
+/// to a `*const VMStateInfo`, for inclusion in a `VMStateField`.
+#[macro_export]
+macro_rules! info_enum_to_ref {
+    ($e:expr) => {
+        unsafe {
+            match $e {
+                $crate::vmstate::VMStateFieldType::null => ::core::ptr::null(),
+                $crate::vmstate::VMStateFieldType::vmstate_info_bool => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_bool)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_int8 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_int8)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_int16 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_int16)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_int32 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_int32)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_int64 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_int64)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_uint8 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint8)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_uint16 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint16)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_uint32 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_uint64 => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint64)
+                }
+                $crate::vmstate::VMStateFieldType::vmstate_info_timer => {
+                    ::core::ptr::addr_of!($crate::bindings::vmstate_info_timer)
+                }
+            }
+        }
+    };
+}
+
 /// A trait for types that can be included in a device's migration stream.  It
 /// provides the base contents of a `VMStateField` (minus the name and offset).
 ///
@@ -69,6 +136,12 @@ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> { ::core::marker:
 /// The contents of this trait go straight into structs that are parsed by C
 /// code and used to introspect into other structs.  Be careful.
 pub unsafe trait VMState {
+    /// The `info` member of a `VMStateField` is a pointer and as such cannot
+    /// yet be included in the [`BASE`](VMState::BASE) associated constant;
+    /// this is only allowed by Rust 1.83.0 and newer.  For now, include the
+    /// member as an enum which is stored in a separate constant.
+    const SCALAR_TYPE: VMStateFieldType = VMStateFieldType::null;
+
     /// The base contents of a `VMStateField` (minus the name and offset) for
     /// the type that is implementing the trait.
     const BASE: VMStateField;
@@ -83,6 +156,12 @@ pub unsafe trait VMState {
     };
 }
 
+/// Internal utility function to retrieve a type's `VMStateFieldType`;
+/// used by [`vmstate_of!`](crate::vmstate_of).
+pub const fn vmstate_scalar_type<T: VMState>(_: PhantomData<T>) -> VMStateFieldType {
+    T::SCALAR_TYPE
+}
+
 /// Internal utility function to retrieve a type's `VMStateField`;
 /// used by [`vmstate_of!`](crate::vmstate_of).
 pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
@@ -99,6 +178,15 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField
 /// Return the `VMStateField` for a field of a struct.  The field must be
 /// visible in the current scope.
 ///
+/// Only a limited set of types is supported out of the box:
+/// * scalar types (integer and `bool`)
+/// * the C struct `QEMUTimer`
+/// * a transparent wrapper for any of the above (`Cell`, `UnsafeCell`,
+///   [`BqlCell`](crate::cell::BqlCell), [`BqlRefCell`](crate::cell::BqlRefCell)
+/// * a raw pointer to any of the above
+/// * a `NonNull` pointer to any of the above, possibly wrapped with `Option`
+/// * an array of any of the above
+///
 /// In order to support other types, the trait `VMState` must be implemented
 /// for them.
 #[macro_export]
@@ -109,8 +197,14 @@ macro_rules! vmstate_of {
                 .as_bytes()
                 .as_ptr() as *const ::std::os::raw::c_char,
             offset: $crate::offset_of!($struct_name, $field_name),
-            // Compute most of the VMStateField from the type of the field.
             $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+            // The calls to `call_func_with_field!` are the magic that
+            // computes most of the VMStateField from the type of the field.
+            info: $crate::info_enum_to_ref!($crate::call_func_with_field!(
+                $crate::vmstate::vmstate_scalar_type,
+                $struct_name,
+                $field_name
+            )),
             ..$crate::call_func_with_field!(
                 $crate::vmstate::vmstate_base,
                 $struct_name,
@@ -187,6 +281,7 @@ pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
 macro_rules! impl_vmstate_transparent {
     ($type:ty where $base:tt: VMState $($where:tt)*) => {
         unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
+            const SCALAR_TYPE: VMStateFieldType = <$base as VMState>::SCALAR_TYPE;
             const BASE: VMStateField = VMStateField {
                 size: mem::size_of::<$type>(),
                 ..<$base as VMState>::BASE
@@ -201,6 +296,33 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
 impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
 
+// Scalar types using predefined VMStateInfos
+
+macro_rules! impl_vmstate_scalar {
+    ($info:ident, $type:ty$(, $varray_flag:ident)?) => {
+        unsafe impl VMState for $type {
+            const SCALAR_TYPE: VMStateFieldType = VMStateFieldType::$info;
+            const BASE: VMStateField = VMStateField {
+                size: mem::size_of::<$type>(),
+                flags: VMStateFlags::VMS_SINGLE,
+                ..Zeroable::ZERO
+            };
+            $(const VARRAY_FLAG: VMStateFlags = VMStateFlags::$varray_flag;)?
+        }
+    };
+}
+
+impl_vmstate_scalar!(vmstate_info_bool, bool);
+impl_vmstate_scalar!(vmstate_info_int8, i8);
+impl_vmstate_scalar!(vmstate_info_int16, i16);
+impl_vmstate_scalar!(vmstate_info_int32, i32);
+impl_vmstate_scalar!(vmstate_info_int64, i64);
+impl_vmstate_scalar!(vmstate_info_uint8, u8, VMS_VARRAY_UINT8);
+impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY_UINT16);
+impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY_UINT32);
+impl_vmstate_scalar!(vmstate_info_uint64, u64);
+impl_vmstate_scalar!(vmstate_info_timer, bindings::QEMUTimer);
+
 // Pointer types using the underlying type's VMState plus VMS_POINTER
 // Note that references are not supported, though references to cells
 // could be allowed.
@@ -208,6 +330,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
 macro_rules! impl_vmstate_pointer {
     ($type:ty where $base:tt: VMState $($where:tt)*) => {
         unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
+            const SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
             const BASE: VMStateField = <$base as VMState>::BASE.with_pointer_flag();
         }
     };
@@ -225,6 +348,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
 // VMS_ARRAY/VMS_ARRAY_OF_POINTER
 
 unsafe impl<T: VMState, const N: usize> VMState for [T; N] {
+    const SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
     const BASE: VMStateField = <T as VMState>::BASE.with_array_flag(N);
 }
 
-- 
2.47.1



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

* [PATCH 06/10] rust: vmstate: add public utility macros to implement VMState
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (4 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 05/10] rust: vmstate: implement VMState for scalar types Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 07/10] rust: qemu_api: add vmstate_struct Paolo Bonzini
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index b0a2ce105cc..8f32c72a0fd 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -4,13 +4,18 @@
 
 //! Helper macros to declare migration state for device models.
 //!
-//! This module includes three families of macros:
+//! This module includes four families of macros:
 //!
 //! * [`vmstate_unused!`](crate::vmstate_unused) and
 //!   [`vmstate_of!`](crate::vmstate_of), which are used to express the
 //!   migration format for a struct.  This is based on the [`VMState`] trait,
 //!   which is defined by all migrateable types.
 //!
+//! * [`impl_vmstate_forward`](crate::impl_vmstate_forward) and
+//!   [`impl_vmstate_bitsized`](crate::impl_vmstate_bitsized), which help with
+//!   the definition of the [`VMState`] trait (respectively for transparent
+//!   structs and for `bilge`-defined types)
+//!
 //! * helper macros to declare a device model state struct, in particular
 //!   [`vmstate_subsections`](crate::vmstate_subsections) and
 //!   [`vmstate_fields`](crate::vmstate_fields).
@@ -134,7 +139,9 @@ macro_rules! info_enum_to_ref {
 /// # Safety
 ///
 /// The contents of this trait go straight into structs that are parsed by C
-/// code and used to introspect into other structs.  Be careful.
+/// code and used to introspect into other structs.  Generally, you don't need
+/// to implement it except via macros that do it for you, such as
+/// `impl_vmstate_bitsized!`.
 pub unsafe trait VMState {
     /// The `info` member of a `VMStateField` is a pointer and as such cannot
     /// yet be included in the [`BASE`](VMState::BASE) associated constant;
@@ -188,7 +195,9 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField
 /// * an array of any of the above
 ///
 /// In order to support other types, the trait `VMState` must be implemented
-/// for them.
+/// for them.  The macros
+/// [`impl_vmstate_bitsized!`](crate::impl_vmstate_bitsized)
+/// and [`impl_vmstate_forward!`](crate::impl_vmstate_forward) help with this.
 #[macro_export]
 macro_rules! vmstate_of {
     ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
@@ -276,6 +285,32 @@ pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
     }
 }
 
+/// This macro can be used (by just passing it a type) to forward the `VMState`
+/// trait to the first field of a tuple.  This is a workaround for lack of
+/// support of nested [`offset_of`](core::mem::offset_of) until Rust 1.82.0.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::vmstate::impl_vmstate_forward;
+/// pub struct Fifo([u8; 16]);
+/// impl_vmstate_forward!(Fifo);
+/// ```
+#[macro_export]
+macro_rules! impl_vmstate_forward {
+    // This is similar to impl_vmstate_transparent below, but it
+    // uses the same trick as vmstate_of! to obtain the type of
+    // the first field of the tuple
+    ($tuple:ty) => {
+        unsafe impl $crate::vmstate::VMState for $tuple {
+            const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
+                $crate::call_func_with_field!($crate::vmstate::vmstate_scalar_type, $tuple, 0);
+            const BASE: $crate::bindings::VMStateField =
+                $crate::call_func_with_field!($crate::vmstate::vmstate_base, $tuple, 0);
+        }
+    };
+}
+
 // Transparent wrappers: just use the internal type
 
 macro_rules! impl_vmstate_transparent {
@@ -296,6 +331,26 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
 impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
 impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
 
+#[macro_export]
+macro_rules! impl_vmstate_bitsized {
+    ($type:ty) => {
+        unsafe impl $crate::vmstate::VMState for $type {
+            const SCALAR_TYPE: $crate::vmstate::VMStateFieldType =
+                                        <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
+                                          as ::bilge::prelude::Number>::UnderlyingType
+                                         as $crate::vmstate::VMState>::SCALAR_TYPE;
+            const BASE: $crate::bindings::VMStateField =
+                                        <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
+                                          as ::bilge::prelude::Number>::UnderlyingType
+                                         as $crate::vmstate::VMState>::BASE;
+            const VARRAY_FLAG: $crate::bindings::VMStateFlags =
+                                        <<<$type as ::bilge::prelude::Bitsized>::ArbitraryInt
+                                          as ::bilge::prelude::Number>::UnderlyingType
+                                         as $crate::vmstate::VMState>::VARRAY_FLAG;
+        }
+    };
+}
+
 // Scalar types using predefined VMStateInfos
 
 macro_rules! impl_vmstate_scalar {
-- 
2.47.1



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

* [PATCH 07/10] rust: qemu_api: add vmstate_struct
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (5 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 06/10] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-22 13:17   ` Zhao Liu
  2025-01-17  9:00 ` [PATCH 08/10] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

It is not type safe, but it's the best that can be done without
const_refs_static.  It can also be used with BqlCell and BqlRefCell.

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 8f32c72a0fd..94a2a29fe7e 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -628,6 +628,39 @@ macro_rules! vmstate_array_of_pointer_to_struct {
     }};
 }
 
+// FIXME: including the `vmsd` field in a `const` is not possible without
+// the const_refs_static feature (stabilized in Rust 1.83.0).  Without it,
+// it is not possible to use VMS_STRUCT in a transparent manner using
+// `vmstate_of!`.  While VMSTATE_CLOCK can at least try to be type-safe,
+// VMSTATE_STRUCT includes $type only for documentation purposes; it
+// is checked against $field_name and $struct_name, but not against $vmsd
+// which is what really would matter.
+#[doc(alias = "VMSTATE_STRUCT")]
+#[macro_export]
+macro_rules! vmstate_struct {
+    ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])?, $vmsd:expr, $type:ty $(,)?) => {
+        $crate::bindings::VMStateField {
+            name: ::core::concat!(::core::stringify!($field_name), "\0")
+                .as_bytes()
+                .as_ptr() as *const ::std::os::raw::c_char,
+            $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+            offset: {
+                $crate::assert_field_type!($struct_name, $field_name, $type);
+                $crate::offset_of!($struct_name, $field_name)
+            },
+            size: ::core::mem::size_of::<$type>(),
+            flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
+            vmsd: unsafe { $vmsd },
+            ..$crate::zeroable::Zeroable::ZERO $(
+                .with_varray_flag($crate::call_func_with_field!(
+                    $crate::vmstate::vmstate_varray_flag,
+                    $struct_name,
+                    $num))
+               $(.with_varray_multiply($factor))?)?
+        }
+    };
+}
+
 #[doc(alias = "VMSTATE_CLOCK_V")]
 #[macro_export]
 macro_rules! vmstate_clock_v {
-- 
2.47.1



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

* [PATCH 08/10] rust: pl011: switch vmstate to new-style macros
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (6 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 07/10] rust: qemu_api: add vmstate_struct Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 09/10] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock Paolo Bonzini
  9 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       |  3 ++-
 rust/hw/char/pl011/src/device_class.rs | 36 +++++++++++++-------------
 rust/hw/char/pl011/src/lib.rs          |  6 +++++
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 994c2fc0593..11a87664c7a 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,7 +10,7 @@
 
 use qemu_api::{
     bindings::{self, *},
-    c_str,
+    c_str, impl_vmstate_forward,
     irq::InterruptSource,
     prelude::*,
     qdev::DeviceImpl,
@@ -54,6 +54,7 @@ impl DeviceId {
 #[repr(transparent)]
 #[derive(Debug, Default)]
 pub struct Fifo([registers::Data; PL011_FIFO_DEPTH as usize]);
+impl_vmstate_forward!(Fifo);
 
 impl Fifo {
     const fn len(&self) -> u32 {
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index 7f3ca895071..e0d3532e956 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -6,11 +6,11 @@
 use std::os::raw::{c_int, c_void};
 
 use qemu_api::{
-    bindings::*, c_str, vmstate_clock, vmstate_fields, vmstate_subsections, vmstate_uint32,
-    vmstate_uint32_array, vmstate_unused, zeroable::Zeroable,
+    bindings::*, c_str, vmstate_clock, vmstate_fields, vmstate_of, vmstate_subsections,
+    vmstate_unused, zeroable::Zeroable,
 };
 
-use crate::device::{PL011State, PL011_FIFO_DEPTH};
+use crate::device::PL011State;
 
 extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
     unsafe {
@@ -52,21 +52,21 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
     post_load: Some(pl011_post_load),
     fields: vmstate_fields! {
         vmstate_unused!(core::mem::size_of::<u32>()),
-        vmstate_uint32!(flags, PL011State),
-        vmstate_uint32!(line_control, PL011State),
-        vmstate_uint32!(receive_status_error_clear, PL011State),
-        vmstate_uint32!(control, PL011State),
-        vmstate_uint32!(dmacr, PL011State),
-        vmstate_uint32!(int_enabled, PL011State),
-        vmstate_uint32!(int_level, PL011State),
-        vmstate_uint32_array!(read_fifo, PL011State, PL011_FIFO_DEPTH),
-        vmstate_uint32!(ilpr, PL011State),
-        vmstate_uint32!(ibrd, PL011State),
-        vmstate_uint32!(fbrd, PL011State),
-        vmstate_uint32!(ifl, PL011State),
-        vmstate_uint32!(read_pos, PL011State),
-        vmstate_uint32!(read_count, PL011State),
-        vmstate_uint32!(read_trigger, PL011State),
+        vmstate_of!(PL011State, flags),
+        vmstate_of!(PL011State, line_control),
+        vmstate_of!(PL011State, receive_status_error_clear),
+        vmstate_of!(PL011State, control),
+        vmstate_of!(PL011State, dmacr),
+        vmstate_of!(PL011State, int_enabled),
+        vmstate_of!(PL011State, int_level),
+        vmstate_of!(PL011State, read_fifo),
+        vmstate_of!(PL011State, ilpr),
+        vmstate_of!(PL011State, ibrd),
+        vmstate_of!(PL011State, fbrd),
+        vmstate_of!(PL011State, ifl),
+        vmstate_of!(PL011State, read_pos),
+        vmstate_of!(PL011State, read_count),
+        vmstate_of!(PL011State, read_trigger),
     },
     subsections: vmstate_subsections! {
         VMSTATE_PL011_CLOCK
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 0a89d393e0f..f30f9850ad4 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -106,6 +106,7 @@ pub mod registers {
     //! Device registers exposed as typed structs which are backed by arbitrary
     //! integer bitmaps. [`Data`], [`Control`], [`LineControl`], etc.
     use bilge::prelude::*;
+    use qemu_api::impl_vmstate_bitsized;
 
     /// Receive Status Register / Data Register common error bits
     ///
@@ -172,6 +173,7 @@ pub struct Data {
         pub errors: Errors,
         _reserved: u16,
     }
+    impl_vmstate_bitsized!(Data);
 
     impl Data {
         // bilge is not very const-friendly, unfortunately
@@ -208,6 +210,7 @@ pub struct ReceiveStatusErrorClear {
         pub errors: Errors,
         _reserved_unpredictable: u24,
     }
+    impl_vmstate_bitsized!(ReceiveStatusErrorClear);
 
     impl ReceiveStatusErrorClear {
         pub fn set_from_data(&mut self, data: Data) {
@@ -280,6 +283,7 @@ pub struct Flags {
         pub ring_indicator: bool,
         _reserved_zero_no_modify: u23,
     }
+    impl_vmstate_bitsized!(Flags);
 
     impl Flags {
         pub fn reset(&mut self) {
@@ -354,6 +358,7 @@ pub struct LineControl {
         /// 31:8 - Reserved, do not modify, read as zero.
         _reserved_zero_no_modify: u24,
     }
+    impl_vmstate_bitsized!(LineControl);
 
     impl LineControl {
         pub fn reset(&mut self) {
@@ -498,6 +503,7 @@ pub struct Control {
         /// 31:16 - Reserved, do not modify, read as zero.
         _reserved_zero_no_modify2: u16,
     }
+    impl_vmstate_bitsized!(Control);
 
     impl Control {
         pub fn reset(&mut self) {
-- 
2.47.1



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

* [PATCH 09/10] rust: vmstate: remove translation of C vmstate macros
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (7 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 08/10] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-17  9:00 ` [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock Paolo Bonzini
  9 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

Keep vmstate_clock!; because it uses a field of type VMStateDescription,
it cannot be converted to the VMState trait without access to the
const_refs_static feature.

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

diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 94a2a29fe7e..70dd3c4fc48 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -21,8 +21,8 @@
 //!   [`vmstate_fields`](crate::vmstate_fields).
 //!
 //! * direct equivalents to the C macros declared in
-//!   `include/migration/vmstate.h`. These are not type-safe and should not be
-//!   used if the equivalent functionality is available with `vmstate_of!`.
+//!   `include/migration/vmstate.h`. These are not type-safe and only provide
+//!   functionality that is missing from `vmstate_of!`.
 
 use core::{marker::PhantomData, mem, ptr::NonNull};
 
@@ -407,223 +407,16 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
     const BASE: VMStateField = <T as VMState>::BASE.with_array_flag(N);
 }
 
-#[doc(alias = "VMSTATE_UNUSED_BUFFER")]
-#[macro_export]
-macro_rules! vmstate_unused_buffer {
-    ($field_exists_fn:expr, $version_id:expr, $size:expr) => {{
-        $crate::bindings::VMStateField {
-            name: c_str!("unused").as_ptr(),
-            err_hint: ::core::ptr::null(),
-            offset: 0,
-            size: $size,
-            start: 0,
-            num: 0,
-            num_offset: 0,
-            size_offset: 0,
-            info: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_info_unused_buffer) },
-            flags: VMStateFlags::VMS_BUFFER,
-            vmsd: ::core::ptr::null(),
-            version_id: $version_id,
-            struct_version_id: 0,
-            field_exists: $field_exists_fn,
-        }
-    }};
-}
-
-#[doc(alias = "VMSTATE_UNUSED_V")]
-#[macro_export]
-macro_rules! vmstate_unused_v {
-    ($version_id:expr, $size:expr) => {{
-        $crate::vmstate_unused_buffer!(None, $version_id, $size)
-    }};
-}
-
 #[doc(alias = "VMSTATE_UNUSED")]
 #[macro_export]
 macro_rules! vmstate_unused {
     ($size:expr) => {{
-        $crate::vmstate_unused_v!(0, $size)
-    }};
-}
-
-#[doc(alias = "VMSTATE_SINGLE_TEST")]
-#[macro_export]
-macro_rules! vmstate_single_test {
-    ($field_name:ident, $struct_name:ty, $field_exists_fn:expr, $version_id:expr, $info:expr, $size:expr) => {{
         $crate::bindings::VMStateField {
-            name: ::core::concat!(::core::stringify!($field_name), 0)
-                .as_bytes()
-                .as_ptr() as *const ::std::os::raw::c_char,
-            err_hint: ::core::ptr::null(),
-            offset: $crate::offset_of!($struct_name, $field_name),
+            name: $crate::c_str!("unused").as_ptr(),
             size: $size,
-            start: 0,
-            num: 0,
-            num_offset: 0,
-            size_offset: 0,
-            info: unsafe { $info },
-            flags: VMStateFlags::VMS_SINGLE,
-            vmsd: ::core::ptr::null(),
-            version_id: $version_id,
-            struct_version_id: 0,
-            field_exists: $field_exists_fn,
-        }
-    }};
-}
-
-#[doc(alias = "VMSTATE_SINGLE")]
-#[macro_export]
-macro_rules! vmstate_single {
-    ($field_name:ident, $struct_name:ty, $version_id:expr, $info:expr, $size:expr) => {{
-        $crate::vmstate_single_test!($field_name, $struct_name, None, $version_id, $info, $size)
-    }};
-}
-
-#[doc(alias = "VMSTATE_UINT32_V")]
-#[macro_export]
-macro_rules! vmstate_uint32_v {
-    ($field_name:ident, $struct_name:ty, $version_id:expr) => {{
-        $crate::vmstate_single!(
-            $field_name,
-            $struct_name,
-            $version_id,
-            ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32),
-            ::core::mem::size_of::<u32>()
-        )
-    }};
-}
-
-#[doc(alias = "VMSTATE_UINT32")]
-#[macro_export]
-macro_rules! vmstate_uint32 {
-    ($field_name:ident, $struct_name:ty) => {{
-        $crate::vmstate_uint32_v!($field_name, $struct_name, 0)
-    }};
-}
-
-#[doc(alias = "VMSTATE_ARRAY")]
-#[macro_export]
-macro_rules! vmstate_array {
-    ($field_name:ident, $struct_name:ty, $length:expr, $version_id:expr, $info:expr, $size:expr) => {{
-        $crate::bindings::VMStateField {
-            name: ::core::concat!(::core::stringify!($field_name), 0)
-                .as_bytes()
-                .as_ptr() as *const ::std::os::raw::c_char,
-            err_hint: ::core::ptr::null(),
-            offset: $crate::offset_of!($struct_name, $field_name),
-            size: $size,
-            start: 0,
-            num: $length as _,
-            num_offset: 0,
-            size_offset: 0,
-            info: unsafe { $info },
-            flags: VMStateFlags::VMS_ARRAY,
-            vmsd: ::core::ptr::null(),
-            version_id: $version_id,
-            struct_version_id: 0,
-            field_exists: None,
-        }
-    }};
-}
-
-#[doc(alias = "VMSTATE_UINT32_ARRAY_V")]
-#[macro_export]
-macro_rules! vmstate_uint32_array_v {
-    ($field_name:ident, $struct_name:ty, $length:expr, $version_id:expr) => {{
-        $crate::vmstate_array!(
-            $field_name,
-            $struct_name,
-            $length,
-            $version_id,
-            ::core::ptr::addr_of!($crate::bindings::vmstate_info_uint32),
-            ::core::mem::size_of::<u32>()
-        )
-    }};
-}
-
-#[doc(alias = "VMSTATE_UINT32_ARRAY")]
-#[macro_export]
-macro_rules! vmstate_uint32_array {
-    ($field_name:ident, $struct_name:ty, $length:expr) => {{
-        $crate::vmstate_uint32_array_v!($field_name, $struct_name, $length, 0)
-    }};
-}
-
-#[doc(alias = "VMSTATE_STRUCT_POINTER_V")]
-#[macro_export]
-macro_rules! vmstate_struct_pointer_v {
-    ($field_name:ident, $struct_name:ty, $version_id:expr, $vmsd:expr, $type:ty) => {{
-        $crate::bindings::VMStateField {
-            name: ::core::concat!(::core::stringify!($field_name), 0)
-                .as_bytes()
-                .as_ptr() as *const ::std::os::raw::c_char,
-            err_hint: ::core::ptr::null(),
-            offset: $crate::offset_of!($struct_name, $field_name),
-            size: ::core::mem::size_of::<*const $type>(),
-            start: 0,
-            num: 0,
-            num_offset: 0,
-            size_offset: 0,
-            info: ::core::ptr::null(),
-            flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0),
-            vmsd: unsafe { $vmsd },
-            version_id: $version_id,
-            struct_version_id: 0,
-            field_exists: None,
-        }
-    }};
-}
-
-#[doc(alias = "VMSTATE_ARRAY_OF_POINTER")]
-#[macro_export]
-macro_rules! vmstate_array_of_pointer {
-    ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr, $info:expr, $type:ty) => {{
-        $crate::bindings::VMStateField {
-            name: ::core::concat!(::core::stringify!($field_name), 0)
-                .as_bytes()
-                .as_ptr() as *const ::std::os::raw::c_char,
-            version_id: $version_id,
-            num: $num as _,
-            info: unsafe { $info },
-            size: ::core::mem::size_of::<*const $type>(),
-            flags: VMStateFlags(VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0),
-            offset: $crate::offset_of!($struct_name, $field_name),
-            err_hint: ::core::ptr::null(),
-            start: 0,
-            num_offset: 0,
-            size_offset: 0,
-            vmsd: ::core::ptr::null(),
-            struct_version_id: 0,
-            field_exists: None,
-        }
-    }};
-}
-
-#[doc(alias = "VMSTATE_ARRAY_OF_POINTER_TO_STRUCT")]
-#[macro_export]
-macro_rules! vmstate_array_of_pointer_to_struct {
-    ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr, $vmsd:expr, $type:ty) => {{
-        $crate::bindings::VMStateField {
-            name: ::core::concat!(::core::stringify!($field_name), 0)
-                .as_bytes()
-                .as_ptr() as *const ::std::os::raw::c_char,
-            version_id: $version_id,
-            num: $num as _,
-            vmsd: unsafe { $vmsd },
-            size: ::core::mem::size_of::<*const $type>(),
-            flags: VMStateFlags(
-                VMStateFlags::VMS_ARRAY.0
-                    | VMStateFlags::VMS_STRUCT.0
-                    | VMStateFlags::VMS_ARRAY_OF_POINTER.0,
-            ),
-            offset: $crate::offset_of!($struct_name, $field_name),
-            err_hint: ::core::ptr::null(),
-            start: 0,
-            num_offset: 0,
-            size_offset: 0,
-            vmsd: ::core::ptr::null(),
-            struct_version_id: 0,
-            field_exists: None,
+            info: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_info_unused_buffer) },
+            flags: $crate::bindings::VMStateFlags::VMS_BUFFER,
+            ..$crate::zeroable::Zeroable::ZERO
         }
     }};
 }
@@ -661,48 +454,27 @@ macro_rules! vmstate_struct {
     };
 }
 
-#[doc(alias = "VMSTATE_CLOCK_V")]
-#[macro_export]
-macro_rules! vmstate_clock_v {
-    ($field_name:ident, $struct_name:ty, $version_id:expr) => {{
-        $crate::vmstate_struct_pointer_v!(
-            $field_name,
-            $struct_name,
-            $version_id,
-            ::core::ptr::addr_of!($crate::bindings::vmstate_clock),
-            $crate::bindings::Clock
-        )
-    }};
-}
-
 #[doc(alias = "VMSTATE_CLOCK")]
 #[macro_export]
 macro_rules! vmstate_clock {
     ($field_name:ident, $struct_name:ty) => {{
-        $crate::vmstate_clock_v!($field_name, $struct_name, 0)
-    }};
-}
-
-#[doc(alias = "VMSTATE_ARRAY_CLOCK_V")]
-#[macro_export]
-macro_rules! vmstate_array_clock_v {
-    ($field_name:ident, $struct_name:ty, $num:expr, $version_id:expr) => {{
-        $crate::vmstate_array_of_pointer_to_struct!(
-            $field_name,
-            $struct_name,
-            $num,
-            $version_id,
-            ::core::ptr::addr_of!($crate::bindings::vmstate_clock),
-            $crate::bindings::Clock
-        )
-    }};
-}
-
-#[doc(alias = "VMSTATE_ARRAY_CLOCK")]
-#[macro_export]
-macro_rules! vmstate_array_clock {
-    ($field_name:ident, $struct_name:ty, $num:expr) => {{
-        $crate::vmstate_array_clock_v!($field_name, $struct_name, $name, 0)
+        $crate::bindings::VMStateField {
+            name: ::core::concat!(::core::stringify!($field_name), "\0")
+                .as_bytes()
+                .as_ptr() as *const ::std::os::raw::c_char,
+            offset: {
+                $crate::assert_field_type!(
+                    $struct_name,
+                    $field_name,
+                    core::ptr::NonNull<$crate::bindings::Clock>
+                );
+                $crate::offset_of!($struct_name, $field_name)
+            },
+            size: ::core::mem::size_of::<*const $crate::bindings::Clock>(),
+            flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0),
+            vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) },
+            ..$crate::zeroable::Zeroable::ZERO
+        }
     }};
 }
 
-- 
2.47.1



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

* [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock
  2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
                   ` (8 preceding siblings ...)
  2025-01-17  9:00 ` [PATCH 09/10] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
@ 2025-01-17  9:00 ` Paolo Bonzini
  2025-01-22 10:37   ` Philippe Mathieu-Daudé
  2025-01-22 13:18   ` Zhao Liu
  9 siblings, 2 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-17  9:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: zhao1.liu, qemu-rust

Place struct_name before field_name, similar to offset_of.

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

diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index e0d3532e956..b052d98803f 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -27,7 +27,7 @@ extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
     minimum_version_id: 1,
     needed: Some(pl011_clock_needed),
     fields: vmstate_fields! {
-        vmstate_clock!(clock, PL011State),
+        vmstate_clock!(PL011State, clock),
     },
     ..Zeroable::ZERO
 };
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 70dd3c4fc48..89ca643a58f 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -457,7 +457,7 @@ macro_rules! vmstate_struct {
 #[doc(alias = "VMSTATE_CLOCK")]
 #[macro_export]
 macro_rules! vmstate_clock {
-    ($field_name:ident, $struct_name:ty) => {{
+    ($struct_name:ty, $field_name:ident) => {{
         $crate::bindings::VMStateField {
             name: ::core::concat!(::core::stringify!($field_name), "\0")
                 .as_bytes()
-- 
2.47.1



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

* Re: [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock
  2025-01-17  9:00 ` [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock Paolo Bonzini
@ 2025-01-22 10:37   ` Philippe Mathieu-Daudé
  2025-01-22 13:18   ` Zhao Liu
  1 sibling, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-01-22 10:37 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: zhao1.liu, qemu-rust

On 17/1/25 10:00, Paolo Bonzini wrote:
> Place struct_name before field_name, similar to offset_of.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   rust/hw/char/pl011/src/device_class.rs | 2 +-
>   rust/qemu-api/src/vmstate.rs           | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



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

* Re: [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types
  2025-01-17  9:00 ` [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
@ 2025-01-22 10:49   ` Zhao Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Zhao Liu @ 2025-01-22 10:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, Jan 17, 2025 at 10:00:38AM +0100, Paolo Bonzini wrote:
> Date: Fri, 17 Jan 2025 10:00:38 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types
> X-Mailer: git-send-email 2.47.1
> 
> Arrays, pointers and cells use a VMStateField that is based on that
> for the inner type.  The implementation therefore delegates to the
> VMState implementation of the inner type.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 79 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
> 

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



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

* Re: [PATCH 03/10] rust: vmstate: add varray support to vmstate_of!
  2025-01-17  9:00 ` [PATCH 03/10] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
@ 2025-01-22 11:44   ` Zhao Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Zhao Liu @ 2025-01-22 11:44 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, Jan 17, 2025 at 10:00:39AM +0100, Paolo Bonzini wrote:
> Date: Fri, 17 Jan 2025 10:00:39 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 03/10] rust: vmstate: add varray support to vmstate_of!
> X-Mailer: git-send-email 2.47.1
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 42 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)

...

> +/// Internal utility function to retrieve a type's `VMStateFlags` when it
> +/// is used as the element count of a `VMSTATE_VARRAY`; used by
> +/// [`vmstate_of!`](crate::vmstate_of).
> +pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField {

A typo? It should return VMStateFlags type.

> +    T::VARRAY_FLAG
> +}
> +

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



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

* Re: [PATCH 05/10] rust: vmstate: implement VMState for scalar types
  2025-01-17  9:00 ` [PATCH 05/10] rust: vmstate: implement VMState for scalar types Paolo Bonzini
@ 2025-01-22 12:33   ` Zhao Liu
  2025-01-22 17:28     ` Paolo Bonzini
  0 siblings, 1 reply; 18+ messages in thread
From: Zhao Liu @ 2025-01-22 12:33 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, Jan 17, 2025 at 10:00:41AM +0100, Paolo Bonzini wrote:
> Date: Fri, 17 Jan 2025 10:00:41 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 05/10] rust: vmstate: implement VMState for scalar types
> X-Mailer: git-send-email 2.47.1
> 
> Scalar types are those that have their own VMStateInfo.  This poses
> a problem in that references to VMStateInfo can only be included in
> associated consts starting with Rust 1.83.0, when the const_refs_static
> was stabilized.  Removing the requirement is done by placing a limited
> list of VMStateInfos in an enum, and going from enum to &VMStateInfo
> only when building the VMStateField.
> 
> The same thing cannot be done with VMS_STRUCT because the set of
> VMStateDescriptions extends to structs defined by the devices.
> Therefore, structs and cells cannot yet use vmstate_of!.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 128 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 126 insertions(+), 2 deletions(-)


>  /// Internal utility function to retrieve a type's `VMStateField`;
>  /// used by [`vmstate_of!`](crate::vmstate_of).
>  pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
> @@ -99,6 +178,15 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField
>  /// Return the `VMStateField` for a field of a struct.  The field must be
>  /// visible in the current scope.
>  ///
> +/// Only a limited set of types is supported out of the box:
> +/// * scalar types (integer and `bool`)
> +/// * the C struct `QEMUTimer`
> +/// * a transparent wrapper for any of the above (`Cell`, `UnsafeCell`,
> +///   [`BqlCell`](crate::cell::BqlCell), [`BqlRefCell`](crate::cell::BqlRefCell)
> +/// * a raw pointer to any of the above
> +/// * a `NonNull` pointer to any of the above, possibly wrapped with `Option`

I just found your rust-next has already updated and removed `Option` :-)

> +/// * an array of any of the above
> +///
>  /// In order to support other types, the trait `VMState` must be implemented
>  /// for them.
>  #[macro_export]
> @@ -109,8 +197,14 @@ macro_rules! vmstate_of {
>                  .as_bytes()
>                  .as_ptr() as *const ::std::os::raw::c_char,
>              offset: $crate::offset_of!($struct_name, $field_name),
> -            // Compute most of the VMStateField from the type of the field.

Rebase mistake? This comment seems no need to be deleted.

>              $(.num_offset: $crate::offset_of!($struct_name, $num),)?
> +            // The calls to `call_func_with_field!` are the magic that
> +            // computes most of the VMStateField from the type of the field.
> +            info: $crate::info_enum_to_ref!($crate::call_func_with_field!(
> +                $crate::vmstate::vmstate_scalar_type,
> +                $struct_name,
> +                $field_name
> +            )),
>

Only a nit above,

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



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

* Re: [PATCH 07/10] rust: qemu_api: add vmstate_struct
  2025-01-17  9:00 ` [PATCH 07/10] rust: qemu_api: add vmstate_struct Paolo Bonzini
@ 2025-01-22 13:17   ` Zhao Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Zhao Liu @ 2025-01-22 13:17 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, Jan 17, 2025 at 10:00:43AM +0100, Paolo Bonzini wrote:
> Date: Fri, 17 Jan 2025 10:00:43 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 07/10] rust: qemu_api: add vmstate_struct
> X-Mailer: git-send-email 2.47.1
> 
> It is not type safe, but it's the best that can be done without
> const_refs_static.  It can also be used with BqlCell and BqlRefCell.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/vmstate.rs | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)

...

> +#[doc(alias = "VMSTATE_STRUCT")]
> +#[macro_export]
> +macro_rules! vmstate_struct {
> +    ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])?, $vmsd:expr, $type:ty $(,)?) => {
> +        $crate::bindings::VMStateField {
> +            name: ::core::concat!(::core::stringify!($field_name), "\0")
> +                .as_bytes()
> +                .as_ptr() as *const ::std::os::raw::c_char,
> +            $(.num_offset: $crate::offset_of!($struct_name, $num),)?
> +            offset: {
> +                $crate::assert_field_type!($struct_name, $field_name, $type);
> +                $crate::offset_of!($struct_name, $field_name)
> +            },
> +            size: ::core::mem::size_of::<$type>(),
> +            flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
> +            vmsd: unsafe { $vmsd },

Yes, this `unsafe` is fitting.

> +            ..$crate::zeroable::Zeroable::ZERO $(
> +                .with_varray_flag($crate::call_func_with_field!(
> +                    $crate::vmstate::vmstate_varray_flag,
> +                    $struct_name,
> +                    $num))
> +               $(.with_varray_multiply($factor))?)?
> +        }
> +    };
> +}
> +

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



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

* Re: [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock
  2025-01-17  9:00 ` [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock Paolo Bonzini
  2025-01-22 10:37   ` Philippe Mathieu-Daudé
@ 2025-01-22 13:18   ` Zhao Liu
  1 sibling, 0 replies; 18+ messages in thread
From: Zhao Liu @ 2025-01-22 13:18 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust

On Fri, Jan 17, 2025 at 10:00:46AM +0100, Paolo Bonzini wrote:
> Date: Fri, 17 Jan 2025 10:00:46 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 10/10] rust: vmstate: make order of parameters consistent
>  in vmstate_clock
> X-Mailer: git-send-email 2.47.1
> 
> Place struct_name before field_name, similar to offset_of.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device_class.rs | 2 +-
>  rust/qemu-api/src/vmstate.rs           | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

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



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

* Re: [PATCH 05/10] rust: vmstate: implement VMState for scalar types
  2025-01-22 12:33   ` Zhao Liu
@ 2025-01-22 17:28     ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2025-01-22 17:28 UTC (permalink / raw)
  To: Zhao Liu; +Cc: qemu-devel, qemu-rust

On 1/22/25 13:33, Zhao Liu wrote:
> On Fri, Jan 17, 2025 at 10:00:41AM +0100, Paolo Bonzini wrote:
>> Date: Fri, 17 Jan 2025 10:00:41 +0100
>> From: Paolo Bonzini <pbonzini@redhat.com>
>> Subject: [PATCH 05/10] rust: vmstate: implement VMState for scalar types
>> X-Mailer: git-send-email 2.47.1
>>
>> Scalar types are those that have their own VMStateInfo.  This poses
>> a problem in that references to VMStateInfo can only be included in
>> associated consts starting with Rust 1.83.0, when the const_refs_static
>> was stabilized.  Removing the requirement is done by placing a limited
>> list of VMStateInfos in an enum, and going from enum to &VMStateInfo
>> only when building the VMStateField.
>>
>> The same thing cannot be done with VMS_STRUCT because the set of
>> VMStateDescriptions extends to structs defined by the devices.
>> Therefore, structs and cells cannot yet use vmstate_of!.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   rust/qemu-api/src/vmstate.rs | 128 ++++++++++++++++++++++++++++++++++-
>>   1 file changed, 126 insertions(+), 2 deletions(-)
> 
> 
>>   /// Internal utility function to retrieve a type's `VMStateField`;
>>   /// used by [`vmstate_of!`](crate::vmstate_of).
>>   pub const fn vmstate_base<T: VMState>(_: PhantomData<T>) -> VMStateField {
>> @@ -99,6 +178,15 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateField
>>   /// Return the `VMStateField` for a field of a struct.  The field must be
>>   /// visible in the current scope.
>>   ///
>> +/// Only a limited set of types is supported out of the box:
>> +/// * scalar types (integer and `bool`)
>> +/// * the C struct `QEMUTimer`
>> +/// * a transparent wrapper for any of the above (`Cell`, `UnsafeCell`,
>> +///   [`BqlCell`](crate::cell::BqlCell), [`BqlRefCell`](crate::cell::BqlRefCell)
>> +/// * a raw pointer to any of the above
>> +/// * a `NonNull` pointer to any of the above, possibly wrapped with `Option`
> 
> I just found your rust-next has already updated and removed `Option` :-)
> 
>> +/// * an array of any of the above
>> +///
>>   /// In order to support other types, the trait `VMState` must be implemented
>>   /// for them.
>>   #[macro_export]
>> @@ -109,8 +197,14 @@ macro_rules! vmstate_of {
>>                   .as_bytes()
>>                   .as_ptr() as *const ::std::os::raw::c_char,
>>               offset: $crate::offset_of!($struct_name, $field_name),
>> -            // Compute most of the VMStateField from the type of the field.
> 
> Rebase mistake? This comment seems no need to be deleted.

It's moved below because there's now more than one call_func_with_field 
call.  I can make it so that the phrasing and placement remains the same 
throughout the series.

Paolo

>>               $(.num_offset: $crate::offset_of!($struct_name, $num),)?
>> +            // The calls to `call_func_with_field!` are the magic that
>> +            // computes most of the VMStateField from the type of the field.
>> +            info: $crate::info_enum_to_ref!($crate::call_func_with_field!(
>> +                $crate::vmstate::vmstate_scalar_type,
>> +                $struct_name,
>> +                $field_name
>> +            )),
>>
> 
> Only a nit above,
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> 
> 
> 



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

end of thread, other threads:[~2025-01-22 17:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-17  9:00 [PATCH 00/10] rust: (mostly) type safe VMState Paolo Bonzini
2025-01-17  9:00 ` [PATCH 01/10] rust: vmstate: add new type safe implementation Paolo Bonzini
2025-01-17  9:00 ` [PATCH 02/10] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
2025-01-22 10:49   ` Zhao Liu
2025-01-17  9:00 ` [PATCH 03/10] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
2025-01-22 11:44   ` Zhao Liu
2025-01-17  9:00 ` [PATCH 04/10] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
2025-01-17  9:00 ` [PATCH 05/10] rust: vmstate: implement VMState for scalar types Paolo Bonzini
2025-01-22 12:33   ` Zhao Liu
2025-01-22 17:28     ` Paolo Bonzini
2025-01-17  9:00 ` [PATCH 06/10] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
2025-01-17  9:00 ` [PATCH 07/10] rust: qemu_api: add vmstate_struct Paolo Bonzini
2025-01-22 13:17   ` Zhao Liu
2025-01-17  9:00 ` [PATCH 08/10] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
2025-01-17  9:00 ` [PATCH 09/10] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
2025-01-17  9:00 ` [PATCH 10/10] rust: vmstate: make order of parameters consistent in vmstate_clock Paolo Bonzini
2025-01-22 10:37   ` Philippe Mathieu-Daudé
2025-01-22 13:18   ` Zhao Liu

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