* [RFC PATCH 1/9] rust: vmstate: add new type safe implementation
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-07 8:58 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 2/9] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/prelude.rs | 2 +
rust/qemu-api/src/vmstate.rs | 110 +++++++++++++++++++++++++++++++++--
2 files changed, 106 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..bfcf06e8f1d 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -4,13 +4,111 @@
//! 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 {
+ ($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] 24+ messages in thread
* Re: [RFC PATCH 1/9] rust: vmstate: add new type safe implementation
2024-12-31 0:23 ` [RFC PATCH 1/9] rust: vmstate: add new type safe implementation Paolo Bonzini
@ 2025-01-07 8:58 ` Zhao Liu
2025-01-07 12:23 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Zhao Liu @ 2025-01-07 8:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:28AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:28 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 1/9] rust: vmstate: add new type safe implementation
> X-Mailer: git-send-email 2.47.1
>
> 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.
This is very good work! I am curious about how QEMU plays with Rust
forum:
Rust forum's disscussion is under MIT and Apache 2.0 licenses [1], and
since vmstate.rs is under the GPLv2 license, do we need to specify that
certain code retains the MIT license?
[1]: https://users.rust-lang.org/t/tos-updated-to-match-rfcs-and-rust-repository/45650
> Introduce the trait and some functions to access it; the actual
> implementation comes later.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/qemu-api/src/prelude.rs | 2 +
> rust/qemu-api/src/vmstate.rs | 110 +++++++++++++++++++++++++++++++++--
> 2 files changed, 106 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..bfcf06e8f1d 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -4,13 +4,111 @@
>
> //! 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 {
> + ($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).+ });
> + })
> + };
> +}
Very flexible and powerful. (I even think this code could be released as
a new public crate.)
> +/// 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 $(,)?) => {
why allow a comma at the end? It seems other patches don't use that
style.
> + $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 {
Why not use u32 (and omit an assert)?
> + assert!(version_id >= 0);
> + self.version_id = version_id;
> + self
> + }
> +}
>
> #[doc(alias = "VMSTATE_UNUSED_BUFFER")]
> #[macro_export]
> --
> 2.47.1
Good design! Look good to me.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 1/9] rust: vmstate: add new type safe implementation
2025-01-07 8:58 ` Zhao Liu
@ 2025-01-07 12:23 ` Paolo Bonzini
2025-01-07 14:01 ` Zhao Liu
0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2025-01-07 12:23 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Jan 7, 2025 at 9:39 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> This is very good work! I am curious about how QEMU plays with Rust
> forum:
>
> Rust forum's disscussion is under MIT and Apache 2.0 licenses [1], and
> since vmstate.rs is under the GPLv2 license, do we need to specify that
> certain code retains the MIT license?
I will add a note similar to the one that is already there in offset_of.
> > +/// ```
> > +/// # 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 {
> > + ($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).+ });
> > + })
> > + };
> > +}
>
> Very flexible and powerful. (I even think this code could be released as
> a new public crate.)
It's probably not _that_ useful in general, unless you're implementing
this kind of reflection... otherwise I would have found an existing
solution. :) But yes, it's very powerful.
Out of curiosity, I asked claude.ai to explain it and it said "This is
a rather advanced use of Rust's type system and macro capabilities to
do compile-time reflection - basically inspecting the types of struct
fields without runtime overhead. While creative, this pattern isn't
commonly needed in everyday Rust code."
When fed the initial comment from the Rust forum it said "your comment
about wanting to access <T as SomeTrait>::SOMETHING for a field's type
is a classic serialization pattern - often used to get things like
type IDs, serialization formats, or field metadata at compile time".
That's actually pretty impressive; the LLM was also impressed and it
started asking me more about it ("Are you building a custom
serialization framework from scratch, or extending an existing one?").
> > +/// 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 $(,)?) => {
>
> why allow a comma at the end? It seems other patches don't use that
> style.
I copied it from the standard library offset_of, but I can remove it too.
> > +// 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 {
>
> Why not use u32 (and omit an assert)?
The version_id field is an int in the C struct; you'd still need to
assert that it's <= i32::MAX, and you'd also need an 'as i32'. In
practice it will always be a constant, therefore it will auto-cast to
either i32 or u32.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 1/9] rust: vmstate: add new type safe implementation
2025-01-07 12:23 ` Paolo Bonzini
@ 2025-01-07 14:01 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-07 14:01 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
> > Very flexible and powerful. (I even think this code could be released as
> > a new public crate.)
>
> It's probably not _that_ useful in general, unless you're implementing
> this kind of reflection... otherwise I would have found an existing
> solution. :) But yes, it's very powerful.
Personally, I feel that projects that glue C and Rust together require
similar tricks more, making them more challenging.
> Out of curiosity, I asked claude.ai to explain it and it said "This is
> a rather advanced use of Rust's type system and macro capabilities to
> do compile-time reflection - basically inspecting the types of struct
> fields without runtime overhead. While creative, this pattern isn't
> commonly needed in everyday Rust code."
>
> When fed the initial comment from the Rust forum it said "your comment
> about wanting to access <T as SomeTrait>::SOMETHING for a field's type
> is a classic serialization pattern - often used to get things like
> type IDs, serialization formats, or field metadata at compile time".
> That's actually pretty impressive; the LLM was also impressed and it
> started asking me more about it ("Are you building a custom
> serialization framework from scratch, or extending an existing one?").
Incredible, commercial LLMs are so proficient in Rust and provide such
professional comments (even a bit off-topic, it feels like LLMs could
even review patches).
Thank you for providing this interesting example. LLMs are indeed the
good tool to help get started with and practice Rust.
Thanks,
Zhao
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 2/9] rust: vmstate: implement VMState for non-leaf types
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
2024-12-31 0:23 ` [RFC PATCH 1/9] rust: vmstate: add new type safe implementation Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-07 15:43 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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 | 66 +++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index bfcf06e8f1d..e20f27b172b 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
@@ -108,6 +109,69 @@ 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);
+ 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
+
+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();
+ }
+ };
+}
+
+impl_vmstate_pointer!(*const T where T: VMState);
+impl_vmstate_pointer!(*mut T where T: VMState);
+impl_vmstate_pointer!(NonNull<T> where T: VMState);
+impl_vmstate_pointer!(Option<NonNull<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 SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
+ 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] 24+ messages in thread
* Re: [RFC PATCH 2/9] rust: vmstate: implement VMState for non-leaf types
2024-12-31 0:23 ` [RFC PATCH 2/9] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
@ 2025-01-07 15:43 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-07 15:43 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:29AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:29 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 2/9] 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 | 66 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index bfcf06e8f1d..e20f27b172b 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
> @@ -108,6 +109,69 @@ 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);
I see, this is the similar case to check overflow like your comment for
patch 1. I think we can use i32::MAX to avoid such magic number.
> + assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) == 0);
Do we need to also check varray flags? e.g.,
But the doc (in vmstate.h) of VMS_ARRAY said "May not be combined with
VMS_VARRAY*", I understand VMS_ARRAY shouldn't be combined with
VMS_VARRAY*, am I right?
If so, we can add a simple const variable that combines all VARRY flags,
e.g,
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
);
...
assert!((self.flags.0 & 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);
Maybe VMS_ARRAY_OF_POINTER should be checked here as well?
> + 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
> +
> +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;
Rebase issue: SCALAR_TYPE is introduced in patch 5.
> + 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);
> +impl_vmstate_pointer!(Option<NonNull<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 SCALAR_TYPE: VMStateFieldType = <T as VMState>::SCALAR_TYPE;
Ditto.
> + const BASE: VMStateField = <T as VMState>::BASE.with_array_flag(N);
> }
>
> #[doc(alias = "VMSTATE_UNUSED_BUFFER")]
> --
> 2.47.1
>
Overall, I think this patch handles the array/pointer/cell cases very
well. So (with some nits fixed :-)),
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of!
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
2024-12-31 0:23 ` [RFC PATCH 1/9] rust: vmstate: add new type safe implementation Paolo Bonzini
2024-12-31 0:23 ` [RFC PATCH 2/9] rust: vmstate: implement VMState for non-leaf types Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-08 3:28 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
Untested...
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 45 +++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index e20f27b172b..079c19c687b 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -69,6 +69,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`;
@@ -77,6 +86,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::BASE
+}
+
/// Return the `VMStateField` for a field of a struct. The field must be
/// visible in the current scope.
///
@@ -84,18 +100,24 @@ 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),)?
+ // The calls to `call_func_with_field!` are the magic that
+ // computes most of the VMStateField from the type of the field.
..$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))?)?
}
};
}
@@ -130,6 +152,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
@@ -141,6 +179,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] 24+ messages in thread
* Re: [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of!
2024-12-31 0:23 ` [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
@ 2025-01-08 3:28 ` Zhao Liu
2025-01-15 10:14 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Zhao Liu @ 2025-01-08 3:28 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:30AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:30 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of!
> X-Mailer: git-send-email 2.47.1
>
> Untested...
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/qemu-api/src/vmstate.rs | 45 +++++++++++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index e20f27b172b..079c19c687b 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -69,6 +69,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`;
> @@ -77,6 +86,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::BASE
> +}
a copy issue:
pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags {
T::VARRAY_FLAG
}
> +
> /// Return the `VMStateField` for a field of a struct. The field must be
> /// visible in the current scope.
> ///
> @@ -84,18 +100,24 @@ 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)?])? $(,)?) => {
Compared to something like "[num]", the format "[0 .. num]" is more
elegant, so I also support this style.
> $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),)?
> + // The calls to `call_func_with_field!` are the magic that
> + // computes most of the VMStateField from the type of the field.
> ..$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))?)?
> }
> };
> }
> @@ -130,6 +152,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);
I understand you checked VMS_ARRAY here since [T; N] has this array
flag.
What if a Rust device just store a pointer to the array? If we allow
this use, then it seems also possible to set varray flags...Then what
about dropping this limitation?
However, I also doube that pointer usage is bad; we should always use
Vec.
> + 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);
Similarly, what about using "assert!(num <= i32::MAX as u32);"?
> + self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0);
> + self.num = num as i32;
> + self
> + }
> }
>
> // Transparent wrappers: just use the internal type
> @@ -141,6 +179,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 [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of!
2025-01-08 3:28 ` Zhao Liu
@ 2025-01-15 10:14 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2025-01-15 10:14 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, junjie.mao
On 1/8/25 04:28, Zhao Liu wrote:
>> + #[must_use]
>> + pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMStateField {
>> + assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
>
> I understand you checked VMS_ARRAY here since [T; N] has this array
> flag.
>
> What if a Rust device just store a pointer to the array? If we allow
> this use, then it seems also possible to set varray flags...Then what
> about dropping this limitation?
Box can be added for that purpose:
impl_vmstate_pointer!(Box<T> where T: VMState);
Also I need to drop Option<NonNull<>> because of
if (field->flags & VMS_POINTER) {
first_elem = *(void **)first_elem;
assert(first_elem || !n_elems || !size);
}
in migration/vmstate.c.
> However, I also doube that pointer usage is bad; we should always use
> Vec.
Vec support is a bit tricky because the number of elements is not
accessible from C. But unbounded arrays are rare in devices. We can
think about it later.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (2 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 3/9] rust: vmstate: add varray support to vmstate_of! Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-06 14:31 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types Paolo Bonzini
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
This shortens a bit the constants. Do not bother using it
in the vmstate macros since most of them will go away soon.
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 079c19c687b..49d0a3c31d4 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
@@ -488,20 +488,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] 24+ messages in thread
* Re: [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField
2024-12-31 0:23 ` [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
@ 2025-01-06 14:31 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-06 14:31 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:31AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:31 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField
> X-Mailer: git-send-email 2.47.1
>
> This shortens a bit the constants. Do not bother using it
> in the vmstate macros since most of them will go away soon.
>
> 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(-)
>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (3 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 4/9] rust: vmstate: implement Zeroable for VMStateField Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-08 6:45 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 6/9] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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 | 125 ++++++++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 49d0a3c31d4..edd0cbff162 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
@@ -58,6 +61,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).
///
@@ -66,6 +133,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;
@@ -80,6 +153,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 {
@@ -96,11 +175,20 @@ 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]
macro_rules! vmstate_of {
- ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
+ ($struct_name:ty, $field_name:ident $([0 .. $num:tt $(* $factor:expr)?])? $(,)?) => {
$crate::bindings::VMStateField {
name: ::core::concat!(::core::stringify!($field_name), "\0")
.as_bytes()
@@ -109,6 +197,11 @@ macro_rules! vmstate_of {
$(.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,
@@ -175,6 +268,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
@@ -189,6 +283,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
macro_rules! impl_vmstate_pointer {
--
2.47.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types
2024-12-31 0:23 ` [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types Paolo Bonzini
@ 2025-01-08 6:45 ` Zhao Liu
2025-01-15 13:08 ` Paolo Bonzini
0 siblings, 1 reply; 24+ messages in thread
From: Zhao Liu @ 2025-01-08 6:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, zhao1.liu, junjie.mao
> #[macro_export]
> macro_rules! vmstate_of {
> - ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
> + ($struct_name:ty, $field_name:ident $([0 .. $num:tt $(* $factor:expr)?])? $(,)?) => {
Why change ident to tt?
> $crate::bindings::VMStateField {
> name: ::core::concat!(::core::stringify!($field_name), "\0")
> .as_bytes()
> @@ -109,6 +197,11 @@ macro_rules! vmstate_of {
> $(.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,
...
> +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);
missed VMS_VARRAY_INT32 :-)
> +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);
If we want to expand in the future (e.g., support vmstate_info_int32_equal
and vmstate_info_int32_le), then introducing new macro variants will be
straightforward. So, fair enough.
> +impl_vmstate_scalar!(vmstate_info_uint64, u64);
What about applying this to "usize" with vmstate_info_uint64?
For array length, I think usize is also used wildly. Maybe we can add
VMS_VARRAY_UINT64 and just treat usize as u64.
> +impl_vmstate_scalar!(vmstate_info_timer, bindings::QEMUTimer);
> +
> // Pointer types using the underlying type's VMState plus VMS_POINTER
>
> macro_rules! impl_vmstate_pointer {
> --
> 2.47.1
Overall, I think it's good; the design idea is coherent.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types
2025-01-08 6:45 ` Zhao Liu
@ 2025-01-15 13:08 ` Paolo Bonzini
2025-01-16 6:59 ` Zhao Liu
0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2025-01-15 13:08 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, qemu-rust, junjie.mao
On 1/8/25 07:45, Zhao Liu wrote:
>> #[macro_export]
>> macro_rules! vmstate_of {
>> - ($struct_name:ty, $field_name:ident $([0 .. $num:ident $(* $factor:expr)?])? $(,)?) => {
>> + ($struct_name:ty, $field_name:ident $([0 .. $num:tt $(* $factor:expr)?])? $(,)?) => {
>
> Why change ident to tt?
Rebase mistake. Initially I had $num:tt, however that becomes unclear
if you have [0 .. 0] where the second 0 is a field name.
>> +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);
>
> missed VMS_VARRAY_INT32 :-)
I left that out intentionally, as Rust is probably going to use
IndexMut<uNN> instead of 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);
>
> If we want to expand in the future (e.g., support vmstate_info_int32_equal
> and vmstate_info_int32_le), then introducing new macro variants will be
> straightforward. So, fair enough.
>
>> +impl_vmstate_scalar!(vmstate_info_uint64, u64);
>
> What about applying this to "usize" with vmstate_info_uint64?
There's 32-bit hosts too... So one would have to add vmstate_info_ulong
which is serialized as 64-bit.
We can add it later, but perhaps we could also create a derive(Index,
IndexMut) macro that makes it possible to specify the type of the index.
While Rust uses usize instead of uNN for array indices, that does not
have to be universal; using uNN is a lot better if it means you can get
rid of casts from register values to array indices and back. See for
example commit 6b4f7b0705b ("rust: pl011: fix migration stream",
2024-12-19).
That is indeed also an issue for HPET, but in that case it can be
isolated to a couple lines,
let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
and it could even be wrapped further
fn timer_and_addr(&self, addr: hwaddr) ->
Option<&BqlRefCell<HPETTimer>, hwaddr> {
let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
if timer_id > self.num_timers.get() {
// TODO: Add trace point -
trace_hpet_timer_id_out_of_range(timer_id)
None
} else {
Some((self.get_timer(timer_id), addr & 0x18))
}
}
...
match self.timer_and_addr(addr) {
None => 0 // Reserved,
Some(timer, addr) => timer.borrow_mut().read(addr, size)
}
So for HPET you didn't reach the threshold of having to create "pub
struct HPETTimers([BqlRefCell<HPETTimer>; MAX_HPET_TIMERS])" and
implement Index<>.
Paolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types
2025-01-15 13:08 ` Paolo Bonzini
@ 2025-01-16 6:59 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-16 6:59 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
> > > +impl_vmstate_scalar!(vmstate_info_uint64, u64);
> >
> > What about applying this to "usize" with vmstate_info_uint64?
>
> There's 32-bit hosts too... So one would have to add vmstate_info_ulong
> which is serialized as 64-bit.
>
> We can add it later, but perhaps we could also create a derive(Index,
> IndexMut) macro that makes it possible to specify the type of the index.
> While Rust uses usize instead of uNN for array indices, that does not have
> to be universal; using uNN is a lot better if it means you can get rid of
> casts from register values to array indices and back. See for example
> commit 6b4f7b0705b ("rust: pl011: fix migration stream", 2024-12-19).
Yes, I agree!
> That is indeed also an issue for HPET, but in that case it can be isolated
> to a couple lines,
>
> let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
>
> and it could even be wrapped further
>
> fn timer_and_addr(&self, addr: hwaddr) -> Option<&BqlRefCell<HPETTimer>,
> hwaddr> {
> let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
> if timer_id > self.num_timers.get() {
> // TODO: Add trace point -
> trace_hpet_timer_id_out_of_range(timer_id)
> None
> } else {
> Some((self.get_timer(timer_id), addr & 0x18))
> }
> }
>
> ...
>
> match self.timer_and_addr(addr) {
> None => 0 // Reserved,
> Some(timer, addr) => timer.borrow_mut().read(addr, size)
> }
>
>
> So for HPET you didn't reach the threshold of having to create "pub struct
> HPETTimers([BqlRefCell<HPETTimer>; MAX_HPET_TIMERS])" and implement Index<>.
>
Thank you Paolo! Will apply your wrapping suggestion!
Regards,
Zhao
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 6/9] rust: vmstate: add public utility macros to implement VMState
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (4 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 5/9] rust: vmstate: implement VMState for scalar types Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-08 8:15 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 7/9] rust: qemu_api: add vmstate_struct and vmstate_cell Paolo Bonzini
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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 edd0cbff162..b59a4b66339 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).
@@ -131,7 +136,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;
@@ -185,7 +192,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:tt $(* $factor:expr)?])? $(,)?) => {
@@ -263,6 +272,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 {
@@ -283,6 +318,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] 24+ messages in thread
* Re: [RFC PATCH 6/9] rust: vmstate: add public utility macros to implement VMState
2024-12-31 0:23 ` [RFC PATCH 6/9] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
@ 2025-01-08 8:15 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-08 8:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
> +/// 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);
> + }
> + };
> +}
> +
Powerful!
> // Transparent wrappers: just use the internal type
>
> macro_rules! impl_vmstate_transparent {
> @@ -283,6 +318,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 {
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 7/9] rust: qemu_api: add vmstate_struct and vmstate_cell
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (5 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 6/9] rust: vmstate: add public utility macros to implement VMState Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-07 16:49 ` Paolo Bonzini
2024-12-31 0:23 ` [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
2024-12-31 0:23 ` [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
These are not type safe, but they're the best that can be done without
const_refs_static.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index b59a4b66339..e45c93587b2 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -610,6 +610,40 @@ 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 {
+ ($field_name:ident, $struct_name:ty, $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,
+ 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
+ }
+ }};
+}
+
+#[macro_export]
+macro_rules! vmstate_cell {
+ ($field_name:ident, $struct_name:ty, $vmsd:expr, $type:ty) => {
+ $crate::vmstate_struct!($field_name, $struct_name, $vmsd, $type)
+ };
+}
+
#[doc(alias = "VMSTATE_CLOCK_V")]
#[macro_export]
macro_rules! vmstate_clock_v {
--
2.47.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [RFC PATCH 7/9] rust: qemu_api: add vmstate_struct and vmstate_cell
2024-12-31 0:23 ` [RFC PATCH 7/9] rust: qemu_api: add vmstate_struct and vmstate_cell Paolo Bonzini
@ 2025-01-07 16:49 ` Paolo Bonzini
0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2025-01-07 16:49 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
On Tue, Dec 31, 2024 at 1:23 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> These are not type safe, but they're the best that can be done without
> const_refs_static.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
FWIW, I'll change these patch to support varrays in v2.
Paolo
> ---
> rust/qemu-api/src/vmstate.rs | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
> index b59a4b66339..e45c93587b2 100644
> --- a/rust/qemu-api/src/vmstate.rs
> +++ b/rust/qemu-api/src/vmstate.rs
> @@ -610,6 +610,40 @@ 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 {
> + ($field_name:ident, $struct_name:ty, $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,
> + 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
> + }
> + }};
> +}
> +
> +#[macro_export]
> +macro_rules! vmstate_cell {
> + ($field_name:ident, $struct_name:ty, $vmsd:expr, $type:ty) => {
> + $crate::vmstate_struct!($field_name, $struct_name, $vmsd, $type)
> + };
> +}
> +
> #[doc(alias = "VMSTATE_CLOCK_V")]
> #[macro_export]
> macro_rules! vmstate_clock_v {
> --
> 2.47.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (6 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 7/9] rust: qemu_api: add vmstate_struct and vmstate_cell Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-08 8:27 ` Zhao Liu
2024-12-31 0:23 ` [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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] 24+ messages in thread
* Re: [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros
2024-12-31 0:23 ` [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
@ 2025-01-08 8:27 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-08 8:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:35AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:35 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros
> X-Mailer: git-send-email 2.47.1
>
> 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(-)
>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate macros
2024-12-31 0:23 [RFC PATCH 0/9] rust: (mostly) type safe VMState Paolo Bonzini
` (7 preceding siblings ...)
2024-12-31 0:23 ` [RFC PATCH 8/9] rust: pl011: switch vmstate to new-style macros Paolo Bonzini
@ 2024-12-31 0:23 ` Paolo Bonzini
2025-01-08 8:40 ` Zhao Liu
8 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2024-12-31 0:23 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust, zhao1.liu, junjie.mao
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.
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 e45c93587b2..68246fce043 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};
@@ -389,223 +389,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
}
}};
}
@@ -644,48 +437,27 @@ macro_rules! vmstate_cell {
};
}
-#[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] 24+ messages in thread
* Re: [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate macros
2024-12-31 0:23 ` [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate macros Paolo Bonzini
@ 2025-01-08 8:40 ` Zhao Liu
0 siblings, 0 replies; 24+ messages in thread
From: Zhao Liu @ 2025-01-08 8:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, junjie.mao
On Tue, Dec 31, 2024 at 01:23:36AM +0100, Paolo Bonzini wrote:
> Date: Tue, 31 Dec 2024 01:23:36 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [RFC PATCH 9/9] rust: vmstate: remove translation of C vmstate
> macros
> X-Mailer: git-send-email 2.47.1
>
> 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.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/qemu-api/src/vmstate.rs | 274 +++--------------------------------
> 1 file changed, 23 insertions(+), 251 deletions(-)
>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread