[PATCH 00/12] rust: miscellaneous cleanup & HPET #property conversion

[PATCH 00/12] rust: miscellaneous cleanup & HPET #property conversion

[Zhao Liu]

Hi,

This series include:
 * cleanup for .gitignore of subproject.
 * cleanup for clippy error & warnings based on v1.83.
 * support bit property in #property.
 * HPET #property conversion.
 * get rid of the legacy declare_properties & define_property.

And this series is based on the commit 355ce1d4fbaa of Paolo's
rust-ci branch (since I understand that branch includes the latest
rust PR patches.)

Thanks and Best Regards,
Zhao
---
Manos Pitsidianakis (2):
  rust/qdev: use addr_of! in QDevProp
  rust/qdev: Refine the documentation for QDevProp trait

Zhao Liu (10):
  subprojects: Update .gitignore for proc-macro2 and syn
  subprojects: Ignore .wraplock file generated by meson v1.9.0
  rust/qemu-macros: Fix Clippy's complaints about lambda parameter
    naming
  rust/common/uninit: Fix Clippy's complaints about lifetime
  rust/qdev: Rename PropertyInfo field from VALUE to BASE_INFO
  rust/qdev: Support property info for more common types
  rust/qdev: Support bit property in #property macro
  rust/hpet: Clean up type mismatch for num_timers property
  rust/hpet: Convert qdev properties to #property macro
  rust/qdev: Drop declare_properties & define_property macros

 rust/common/src/uninit.rs        |   4 +-
 rust/hw/core/src/qdev.rs         | 105 ++++++++++---------------------
 rust/hw/timer/hpet/src/device.rs |  55 +++-------------
 rust/qemu-macros/src/lib.rs      |  81 ++++++++++++++++--------
 subprojects/.gitignore           |   7 ++-
 5 files changed, 102 insertions(+), 150 deletions(-)

-- 
2.34.1

[PATCH 01/12] subprojects: Update .gitignore for proc-macro2 and syn

[Zhao Liu]

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 subprojects/.gitignore | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index f4281934ce11..9d579f72d121 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -16,7 +16,7 @@
 /libc-0.2.162
 /proc-macro-error-1.0.4
 /proc-macro-error-attr-1.0.4
-/proc-macro2-1.0.84
+/proc-macro2-1.0.95
 /quote-1.0.36
-/syn-2.0.66
+/syn-2.0.104
 /unicode-ident-1.0.12
-- 
2.34.1

[PATCH 02/12] subprojects: Ignore .wraplock file generated by meson v1.9.0

[Zhao Liu]

The .wraplock file is automatically generated by meson v1.9.0 (the
related issue: https://github.com/mesonbuild/meson/issues/14948).

Ignore it for now.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 subprojects/.gitignore | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index 9d579f72d121..20b25845760f 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -20,3 +20,6 @@
 /quote-1.0.36
 /syn-2.0.104
 /unicode-ident-1.0.12
+
+# Workaround for Meson v1.9.0 https://github.com/mesonbuild/meson/issues/14948
+/.wraplock
-- 
2.34.1

[PATCH 03/12] rust/qemu-macros: Fix Clippy's complaints about lambda parameter naming

[Zhao Liu]

error: `rename` shadows a previous, unrelated binding
   --> qemu-macros/src/lib.rs:265:14
    |
265 |             |rename| -> Result<proc_macro2::TokenStream, Error> {
    |              ^^^^^^
    |
note: previous binding is here
   --> qemu-macros/src/lib.rs:245:30
    |
245 |         let DeviceProperty { rename, defval } = prop;
    |                              ^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated
    = note: requested on the command line with `-D clippy::shadow-unrelated`

Rename the lambda parameter to "prop_rename" to fix the above clippy
error.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/qemu-macros/src/lib.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index 830b4326985b..ed4064d6e110 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -262,8 +262,8 @@ macro_rules! str_to_c_str {
 
         let prop_name = rename.map_or_else(
             || str_to_c_str!(field_name.to_string(), field_name.span()),
-            |rename| -> Result<proc_macro2::TokenStream, Error> {
-                match rename {
+            |prop_rename| -> Result<proc_macro2::TokenStream, Error> {
+                match prop_rename {
                     DevicePropertyName::CStr(cstr_lit) => Ok(quote! { #cstr_lit }),
                     DevicePropertyName::Str(str_lit) => {
                         str_to_c_str!(str_lit.value(), str_lit.span())
-- 
2.34.1

[PATCH 04/12] rust/common/uninit: Fix Clippy's complaints about lifetime

[Zhao Liu]

Clippy complains about the following cases and following its suggestion
to fix these warnings.

warning: the following explicit lifetimes could be elided: 'a
  --> common/src/uninit.rs:38:6
   |
38 | impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
   |      ^^                                   ^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
   = note: `#[warn(clippy::needless_lifetimes)]` on by default
help: elide the lifetimes
   |
38 - impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
38 + impl<T, U> Deref for MaybeUninitField<'_, T, U> {
   |

warning: the following explicit lifetimes could be elided: 'a
  --> common/src/uninit.rs:49:6
   |
49 | impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
   |      ^^                                      ^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
help: elide the lifetimes
   |
49 - impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
49 + impl<T, U> DerefMut for MaybeUninitField<'_, T, U> {
   |

warning: `common` (lib) generated 2 warnings (run `cargo clippy --fix --lib -p common` to apply 2 suggestions)

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/common/src/uninit.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/common/src/uninit.rs b/rust/common/src/uninit.rs
index e7f9fcd2e3fb..8d021b1dfc6e 100644
--- a/rust/common/src/uninit.rs
+++ b/rust/common/src/uninit.rs
@@ -35,7 +35,7 @@ pub const fn parent_mut(f: &mut Self) -> *mut T {
     }
 }
 
-impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
+impl<T, U> Deref for MaybeUninitField<'_, T, U> {
     type Target = MaybeUninit<U>;
 
     fn deref(&self) -> &MaybeUninit<U> {
@@ -46,7 +46,7 @@ fn deref(&self) -> &MaybeUninit<U> {
     }
 }
 
-impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
+impl<T, U> DerefMut for MaybeUninitField<'_, T, U> {
     fn deref_mut(&mut self) -> &mut MaybeUninit<U> {
         // SAFETY: self.child was obtained by dereferencing a valid mutable
         // reference; the content of the memory may be invalid or uninitialized
-- 
2.34.1

[PATCH 05/12] rust/qdev: use addr_of! in QDevProp

[Zhao Liu]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

We want a &raw pointer, so unsafe { &_ } is not needed.

Suggested-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index 71b9ef141c3a..3ee5b242622d 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -6,7 +6,7 @@
 
 use std::{
     ffi::{c_int, c_void, CStr, CString},
-    ptr::NonNull,
+    ptr::{addr_of, NonNull},
 };
 
 use chardev::Chardev;
@@ -129,17 +129,17 @@ pub unsafe trait QDevProp {
 
 /// Use [`bindings::qdev_prop_bool`] for `bool`.
 unsafe impl QDevProp for bool {
-    const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_bool };
+    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
 }
 
 /// Use [`bindings::qdev_prop_uint64`] for `u64`.
 unsafe impl QDevProp for u64 {
-    const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_uint64 };
+    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
 }
 
 /// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`].
 unsafe impl QDevProp for chardev::CharBackend {
-    const VALUE: *const bindings::PropertyInfo = unsafe { &bindings::qdev_prop_chr };
+    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
 }
 
 /// Trait to define device properties.
-- 
2.34.1

[PATCH 06/12] rust/qdev: Refine the documentation for QDevProp trait

[Zhao Liu]

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Refine the documentation to clarify:
 * `unsfae` requires that `VALUE` must be valid.
 * using `*const` instead of `&` because the latter will cause compiler
   error.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index 3ee5b242622d..2735e2b2c109 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -109,9 +109,16 @@ pub trait ResettablePhasesImpl {
 ///
 /// # Safety
 ///
-/// This trait is marked as `unsafe` because currently having a `const` refer to
-/// an `extern static` as a reference instead of a raw pointer results in this
-/// compiler error:
+/// This trait is marked as `unsafe` because `VALUE` must be a valid raw
+/// reference to a [`bindings::PropertyInfo`].
+///
+/// Note we could not use a regular reference:
+///
+/// ```text
+/// const VALUE: &bindings::PropertyInfo = ...
+/// ```
+///
+/// because this results in the following compiler error:
 ///
 /// ```text
 /// constructing invalid value: encountered reference to `extern` static in `const`
@@ -119,7 +126,7 @@ pub trait ResettablePhasesImpl {
 ///
 /// This is because the compiler generally might dereference a normal reference
 /// during const evaluation, but not in this case (if it did, it'd need to
-/// dereference the raw pointer so this would fail to compile).
+/// dereference the raw pointer so using a `*const` would also fail to compile).
 ///
 /// It is the implementer's responsibility to provide a valid
 /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
-- 
2.34.1

[PATCH 07/12] rust/qdev: Rename PropertyInfo field from VALUE to BASE_INFO

[Zhao Liu]

Bit property info will added next. To distinguish different info fields,
rename `VALUE` to `BASE_INFO`, then it can better reflect that it
represents the basic property info.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs    | 12 ++++++------
 rust/qemu-macros/src/lib.rs |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index 2735e2b2c109..d887046d8de1 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -109,13 +109,13 @@ pub trait ResettablePhasesImpl {
 ///
 /// # Safety
 ///
-/// This trait is marked as `unsafe` because `VALUE` must be a valid raw
+/// This trait is marked as `unsafe` because `BASE_INFO` must be a valid raw
 /// reference to a [`bindings::PropertyInfo`].
 ///
 /// Note we could not use a regular reference:
 ///
 /// ```text
-/// const VALUE: &bindings::PropertyInfo = ...
+/// const BASE_INFO: &bindings::PropertyInfo = ...
 /// ```
 ///
 /// because this results in the following compiler error:
@@ -131,22 +131,22 @@ pub trait ResettablePhasesImpl {
 /// It is the implementer's responsibility to provide a valid
 /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
 pub unsafe trait QDevProp {
-    const VALUE: *const bindings::PropertyInfo;
+    const BASE_INFO: *const bindings::PropertyInfo;
 }
 
 /// Use [`bindings::qdev_prop_bool`] for `bool`.
 unsafe impl QDevProp for bool {
-    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
+    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
 }
 
 /// Use [`bindings::qdev_prop_uint64`] for `u64`.
 unsafe impl QDevProp for u64 {
-    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
+    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
 }
 
 /// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`].
 unsafe impl QDevProp for chardev::CharBackend {
-    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
+    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
 }
 
 /// Trait to define device properties.
diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index ed4064d6e110..b43ca31bae30 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -272,7 +272,7 @@ macro_rules! str_to_c_str {
             },
         )?;
         let field_ty = field.ty.clone();
-        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::VALUE };
+        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO };
         let set_default = defval.is_some();
         let defval = defval.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
         properties_expanded.push(quote! {
-- 
2.34.1

[PATCH 08/12] rust/qdev: Support property info for more common types

[Zhao Liu]

Add a helper macro to implement QDevProp trait for u8/u16/u32/usize/i32
/i64.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index d887046d8de1..b57dc05ebb0e 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -134,20 +134,24 @@ pub unsafe trait QDevProp {
     const BASE_INFO: *const bindings::PropertyInfo;
 }
 
-/// Use [`bindings::qdev_prop_bool`] for `bool`.
-unsafe impl QDevProp for bool {
-    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
-}
-
-/// Use [`bindings::qdev_prop_uint64`] for `u64`.
-unsafe impl QDevProp for u64 {
-    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
+macro_rules! impl_qdev_prop {
+    ($type:ty,$info:ident) => {
+        unsafe impl $crate::qdev::QDevProp for $type {
+            const BASE_INFO: *const $crate::bindings::PropertyInfo =
+                addr_of!($crate::bindings::$info);
+        }
+    };
 }
 
-/// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`].
-unsafe impl QDevProp for chardev::CharBackend {
-    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
-}
+impl_qdev_prop!(bool, qdev_prop_bool);
+impl_qdev_prop!(u8, qdev_prop_uint8);
+impl_qdev_prop!(u16, qdev_prop_uint16);
+impl_qdev_prop!(u32, qdev_prop_uint32);
+impl_qdev_prop!(u64, qdev_prop_uint64);
+impl_qdev_prop!(usize, qdev_prop_usize);
+impl_qdev_prop!(i32, qdev_prop_int32);
+impl_qdev_prop!(i64, qdev_prop_int64);
+impl_qdev_prop!(chardev::CharBackend, qdev_prop_chr);
 
 /// Trait to define device properties.
 ///
-- 
2.34.1

[PATCH 09/12] rust/qdev: Support bit property in #property macro

[Zhao Liu]

Add BIT_INFO to QDevProp trait, so that bit related property info could
be bound to u32 & u64.

Then add "bit=*" field in #property attributes macro to allow device to
configure bit property.

In addtion, convert the #property field parsing from `if-else` pattern
to `match` pattern, to help readability. And note, the `bitnr` member of
`Property` struct is generated by manual TokenStream construction,
instead of conditional repetition (like #(bitnr: #bitnr,)?) since
`quote` doesn't support this.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs    | 15 +++++---
 rust/qemu-macros/src/lib.rs | 77 +++++++++++++++++++++++++------------
 2 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index b57dc05ebb0e..a8cd9e3c2fd5 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -109,8 +109,8 @@ pub trait ResettablePhasesImpl {
 ///
 /// # Safety
 ///
-/// This trait is marked as `unsafe` because `BASE_INFO` must be a valid raw
-/// reference to a [`bindings::PropertyInfo`].
+/// This trait is marked as `unsafe` because `BASE_INFO` and `BIT_INFO` must be
+/// the valid raw references to [`bindings::PropertyInfo`].
 ///
 /// Note we could not use a regular reference:
 ///
@@ -132,13 +132,18 @@ pub trait ResettablePhasesImpl {
 /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
 pub unsafe trait QDevProp {
     const BASE_INFO: *const bindings::PropertyInfo;
+    const BIT_INFO: *const bindings::PropertyInfo = {
+        panic!("invalid type for bit property");
+    };
 }
 
 macro_rules! impl_qdev_prop {
-    ($type:ty,$info:ident) => {
+    ($type:ty,$info:ident$(, $bit_info:ident)?) => {
         unsafe impl $crate::qdev::QDevProp for $type {
             const BASE_INFO: *const $crate::bindings::PropertyInfo =
                 addr_of!($crate::bindings::$info);
+            $(const BIT_INFO: *const $crate::bindings::PropertyInfo =
+                addr_of!($crate::bindings::$bit_info);)?
         }
     };
 }
@@ -146,8 +151,8 @@ unsafe impl $crate::qdev::QDevProp for $type {
 impl_qdev_prop!(bool, qdev_prop_bool);
 impl_qdev_prop!(u8, qdev_prop_uint8);
 impl_qdev_prop!(u16, qdev_prop_uint16);
-impl_qdev_prop!(u32, qdev_prop_uint32);
-impl_qdev_prop!(u64, qdev_prop_uint64);
+impl_qdev_prop!(u32, qdev_prop_uint32, qdev_prop_bit);
+impl_qdev_prop!(u64, qdev_prop_uint64, qdev_prop_bit64);
 impl_qdev_prop!(usize, qdev_prop_usize);
 impl_qdev_prop!(i32, qdev_prop_int32);
 impl_qdev_prop!(i64, qdev_prop_int64);
diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index b43ca31bae30..8109ff239227 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -162,6 +162,7 @@ enum DevicePropertyName {
 #[derive(Debug)]
 struct DeviceProperty {
     rename: Option<DevicePropertyName>,
+    bitnr: Option<syn::Expr>,
     defval: Option<syn::Expr>,
 }
 
@@ -174,40 +175,56 @@ fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
         debug_assert_eq!(&attribute.to_string(), "property");
         let mut retval = Self {
             rename: None,
+            bitnr: None,
             defval: None,
         };
         let content;
         _ = syn::parenthesized!(content in bracketed);
         while !content.is_empty() {
             let value: syn::Ident = content.parse()?;
-            if value == "rename" {
-                let _: syn::Token![=] = content.parse()?;
-                if retval.rename.is_some() {
-                    return Err(syn::Error::new(
-                        value.span(),
-                        "`rename` can only be used at most once",
-                    ));
+            match value {
+                ref v if v == "rename" => {
+                    let _: syn::Token![=] = content.parse()?;
+                    if retval.rename.is_some() {
+                        return Err(syn::Error::new(
+                            value.span(),
+                            "`rename` can only be used at most once",
+                        ));
+                    }
+                    if content.peek(syn::LitStr) {
+                        retval.rename =
+                            Some(DevicePropertyName::Str(content.parse::<syn::LitStr>()?));
+                    } else {
+                        retval.rename =
+                            Some(DevicePropertyName::CStr(content.parse::<syn::LitCStr>()?));
+                    }
                 }
-                if content.peek(syn::LitStr) {
-                    retval.rename = Some(DevicePropertyName::Str(content.parse::<syn::LitStr>()?));
-                } else {
-                    retval.rename =
-                        Some(DevicePropertyName::CStr(content.parse::<syn::LitCStr>()?));
+                ref v if v == "bit" => {
+                    let _: syn::Token![=] = content.parse()?;
+                    if retval.bitnr.is_some() {
+                        return Err(syn::Error::new(
+                            value.span(),
+                            "`bit` can only be used at most once",
+                        ));
+                    }
+                    retval.bitnr = Some(content.parse()?);
+                }
+                ref v if v == "default" => {
+                    let _: syn::Token![=] = content.parse()?;
+                    if retval.defval.is_some() {
+                        return Err(syn::Error::new(
+                            value.span(),
+                            "`default` can only be used at most once",
+                        ));
+                    }
+                    retval.defval = Some(content.parse()?);
                 }
-            } else if value == "default" {
-                let _: syn::Token![=] = content.parse()?;
-                if retval.defval.is_some() {
+                _ => {
                     return Err(syn::Error::new(
                         value.span(),
-                        "`default` can only be used at most once",
+                        format!("unrecognized field `{value}`"),
                     ));
                 }
-                retval.defval = Some(content.parse()?);
-            } else {
-                return Err(syn::Error::new(
-                    value.span(),
-                    format!("unrecognized field `{value}`"),
-                ));
             }
 
             if !content.is_empty() {
@@ -242,7 +259,11 @@ fn derive_device_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream
     let mut properties_expanded = vec![];
 
     for (field, prop) in properties {
-        let DeviceProperty { rename, defval } = prop;
+        let DeviceProperty {
+            rename,
+            bitnr,
+            defval,
+        } = prop;
         let field_name = field.ident.unwrap();
         macro_rules! str_to_c_str {
             ($value:expr, $span:expr) => {{
@@ -272,14 +293,20 @@ macro_rules! str_to_c_str {
             },
         )?;
         let field_ty = field.ty.clone();
-        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO };
+        let qdev_prop = if bitnr.is_none() {
+            quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO }
+        } else {
+            quote! { <#field_ty as ::hwcore::QDevProp>::BIT_INFO }
+        };
+        let bitnr = bitnr.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
         let set_default = defval.is_some();
         let defval = defval.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
         properties_expanded.push(quote! {
             ::hwcore::bindings::Property {
                 name: ::std::ffi::CStr::as_ptr(#prop_name),
-                info: #qdev_prop ,
+                info: #qdev_prop,
                 offset: ::core::mem::offset_of!(#name, #field_name) as isize,
+                bitnr: #bitnr,
                 set_default: #set_default,
                 defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
                 ..::common::Zeroable::ZERO
-- 
2.34.1

[PATCH 10/12] rust/hpet: Clean up type mismatch for num_timers property

[Zhao Liu]

Now `num_timers` is `usize` other than `u8`. Although the type field in
`declare_properties` macro hasn't been used, it's better to explicitly
point this out and clean up this before doing other property work.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/timer/hpet/src/device.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index 3cfbe9c32bb1..fce75415579d 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -909,7 +909,7 @@ impl ObjectImpl for HPETState {
         HPETState,
         num_timers,
         unsafe { &qdev_prop_usize },
-        u8,
+        usize,
         default = HPET_MIN_TIMERS
     ),
     define_property!(
-- 
2.34.1

[PATCH 11/12] rust/hpet: Convert qdev properties to #property macro

[Zhao Liu]

Convert HPET's properties to #property macro:
 * num_timers: usize property.
 * flags: u32 bit property.
 * int_route_cap: u32 property.
 * hpet_offset_saved: bool property.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/timer/hpet/src/device.rs | 55 ++++----------------------------
 1 file changed, 7 insertions(+), 48 deletions(-)

diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index fce75415579d..86638c076666 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -13,9 +13,8 @@
 use bql::{BqlCell, BqlRefCell};
 use common::{bitops::IntegerExt, uninit_field_mut};
 use hwcore::{
-    bindings::{qdev_prop_bit, qdev_prop_bool, qdev_prop_uint32, qdev_prop_usize},
-    declare_properties, define_property, DeviceImpl, DeviceMethods, DeviceState, InterruptSource,
-    Property, ResetType, ResettablePhasesImpl, SysBusDevice, SysBusDeviceImpl, SysBusDeviceMethods,
+    DeviceImpl, DeviceMethods, DeviceState, InterruptSource, ResetType, ResettablePhasesImpl,
+    SysBusDevice, SysBusDeviceImpl, SysBusDeviceMethods,
 };
 use migration::{
     self, impl_vmstate_struct, vmstate_fields, vmstate_of, vmstate_subsections, vmstate_validate,
@@ -520,7 +519,7 @@ fn write(&mut self, reg: TimerRegister, value: u64, shift: u32, len: u32) {
 
 /// HPET Event Timer Block Abstraction
 #[repr(C)]
-#[derive(qom::Object)]
+#[derive(qom::Object, hwcore::Device)]
 pub struct HPETState {
     parent_obj: ParentField<SysBusDevice>,
     iomem: MemoryRegion,
@@ -540,10 +539,12 @@ pub struct HPETState {
     // Internal state
     /// Capabilities that QEMU HPET supports.
     /// bit 0: MSI (or FSB) support.
+    #[property(rename = "msi", bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8, default = false)]
     flags: u32,
 
     /// Offset of main counter relative to qemu clock.
     hpet_offset: BqlCell<u64>,
+    #[property(rename = "hpet-offset-saved", default = true)]
     hpet_offset_saved: bool,
 
     irqs: [InterruptSource; HPET_NUM_IRQ_ROUTES],
@@ -555,11 +556,13 @@ pub struct HPETState {
     /// the timers' interrupt can be routed, and is encoded in the
     /// bits 32:64 of timer N's config register:
     #[doc(alias = "intcap")]
+    #[property(rename = "hpet-intcap", default = 0)]
     int_route_cap: u32,
 
     /// HPET timer array managed by this timer block.
     #[doc(alias = "timer")]
     timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
+    #[property(rename = "timers", default = HPET_MIN_TIMERS)]
     num_timers: usize,
     num_timers_save: BqlCell<u8>,
 
@@ -901,44 +904,6 @@ impl ObjectImpl for HPETState {
     const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::<Self>;
 }
 
-// TODO: Make these properties user-configurable!
-declare_properties! {
-    HPET_PROPERTIES,
-    define_property!(
-        c"timers",
-        HPETState,
-        num_timers,
-        unsafe { &qdev_prop_usize },
-        usize,
-        default = HPET_MIN_TIMERS
-    ),
-    define_property!(
-        c"msi",
-        HPETState,
-        flags,
-        unsafe { &qdev_prop_bit },
-        u32,
-        bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8,
-        default = false,
-    ),
-    define_property!(
-        c"hpet-intcap",
-        HPETState,
-        int_route_cap,
-        unsafe { &qdev_prop_uint32 },
-        u32,
-        default = 0
-    ),
-    define_property!(
-        c"hpet-offset-saved",
-        HPETState,
-        hpet_offset_saved,
-        unsafe { &qdev_prop_bool },
-        bool,
-        default = true
-    ),
-}
-
 static VMSTATE_HPET_RTC_IRQ_LEVEL: VMStateDescription<HPETState> =
     VMStateDescriptionBuilder::<HPETState>::new()
         .name(c"hpet/rtc_irq_level")
@@ -1001,12 +966,6 @@ impl ObjectImpl for HPETState {
         ))
         .build();
 
-// SAFETY: HPET_PROPERTIES is a valid Property array constructed with the
-// hwcore::declare_properties macro.
-unsafe impl hwcore::DevicePropertiesImpl for HPETState {
-    const PROPERTIES: &'static [Property] = &HPET_PROPERTIES;
-}
-
 impl DeviceImpl for HPETState {
     const VMSTATE: Option<VMStateDescription<Self>> = Some(VMSTATE_HPET);
     const REALIZE: Option<fn(&Self) -> util::Result<()>> = Some(Self::realize);
-- 
2.34.1

[PATCH 12/12] rust/qdev: Drop declare_properties & define_property macros

[Zhao Liu]

After HPET's #property conversion, there's no use case for
declare_properties & define_property. So get rid of them for now.

In future, if there's something that #property really cannot resolve,
they can be brought back.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 rust/hw/core/src/qdev.rs | 53 ----------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index a8cd9e3c2fd5..a32ada4c175e 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -248,59 +248,6 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
     }
 }
 
-#[macro_export]
-macro_rules! define_property {
-    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, bit = $bitnr:expr, default = $defval:expr$(,)*) => {
-        $crate::bindings::Property {
-            // use associated function syntax for type checking
-            name: ::std::ffi::CStr::as_ptr($name),
-            info: $prop,
-            offset: ::std::mem::offset_of!($state, $field) as isize,
-            bitnr: $bitnr,
-            set_default: true,
-            defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
-            ..::common::zeroable::Zeroable::ZERO
-        }
-    };
-    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => {
-        $crate::bindings::Property {
-            // use associated function syntax for type checking
-            name: ::std::ffi::CStr::as_ptr($name),
-            info: $prop,
-            offset: ::std::mem::offset_of!($state, $field) as isize,
-            set_default: true,
-            defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
-            ..::common::zeroable::Zeroable::ZERO
-        }
-    };
-    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty$(,)*) => {
-        $crate::bindings::Property {
-            // use associated function syntax for type checking
-            name: ::std::ffi::CStr::as_ptr($name),
-            info: $prop,
-            offset: ::std::mem::offset_of!($state, $field) as isize,
-            set_default: false,
-            ..::common::zeroable::Zeroable::ZERO
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! declare_properties {
-    ($ident:ident, $($prop:expr),*$(,)*) => {
-        pub static $ident: [$crate::bindings::Property; {
-            let mut len = 0;
-            $({
-                _ = stringify!($prop);
-                len += 1;
-            })*
-            len
-        }] = [
-            $($prop),*,
-        ];
-    };
-}
-
 unsafe impl ObjectType for DeviceState {
     type Class = DeviceClass;
     const TYPE_NAME: &'static CStr =
-- 
2.34.1

Re: [PATCH 01/12] subprojects: Update .gitignore for proc-macro2 and syn

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  subprojects/.gitignore | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/subprojects/.gitignore b/subprojects/.gitignore
> index f4281934ce11..9d579f72d121 100644
> --- a/subprojects/.gitignore
> +++ b/subprojects/.gitignore
> @@ -16,7 +16,7 @@
>  /libc-0.2.162
>  /proc-macro-error-1.0.4
>  /proc-macro-error-attr-1.0.4
> -/proc-macro2-1.0.84
> +/proc-macro2-1.0.95
>  /quote-1.0.36
> -/syn-2.0.66
> +/syn-2.0.104
>  /unicode-ident-1.0.12
> --
> 2.34.1
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Maybe we could change the entries to match any directory with a version, e.g.

/proc-macro2-[0-9.]*/

Re: [PATCH 02/12] subprojects: Ignore .wraplock file generated by meson v1.9.0

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> The .wraplock file is automatically generated by meson v1.9.0 (the
> related issue: https://github.com/mesonbuild/meson/issues/14948).
>
> Ignore it for now.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  subprojects/.gitignore | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/subprojects/.gitignore b/subprojects/.gitignore
> index 9d579f72d121..20b25845760f 100644
> --- a/subprojects/.gitignore
> +++ b/subprojects/.gitignore
> @@ -20,3 +20,6 @@
>  /quote-1.0.36
>  /syn-2.0.104
>  /unicode-ident-1.0.12
> +
> +# Workaround for Meson v1.9.0 https://github.com/mesonbuild/meson/issues/14948
> +/.wraplock
> --
> 2.34.1
>

Re: [PATCH 03/12] rust/qemu-macros: Fix Clippy's complaints about lambda parameter naming

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> error: `rename` shadows a previous, unrelated binding
>    --> qemu-macros/src/lib.rs:265:14
>     |
> 265 |             |rename| -> Result<proc_macro2::TokenStream, Error> {
>     |              ^^^^^^
>     |
> note: previous binding is here
>    --> qemu-macros/src/lib.rs:245:30
>     |
> 245 |         let DeviceProperty { rename, defval } = prop;
>     |                              ^^^^^^
>     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated
>     = note: requested on the command line with `-D clippy::shadow-unrelated`
>
> Rename the lambda parameter to "prop_rename" to fix the above clippy
> error.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  rust/qemu-macros/src/lib.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
> index 830b4326985b..ed4064d6e110 100644
> --- a/rust/qemu-macros/src/lib.rs
> +++ b/rust/qemu-macros/src/lib.rs
> @@ -262,8 +262,8 @@ macro_rules! str_to_c_str {
>
>          let prop_name = rename.map_or_else(
>              || str_to_c_str!(field_name.to_string(), field_name.span()),
> -            |rename| -> Result<proc_macro2::TokenStream, Error> {
> -                match rename {
> +            |prop_rename| -> Result<proc_macro2::TokenStream, Error> {
> +                match prop_rename {
>                      DevicePropertyName::CStr(cstr_lit) => Ok(quote! { #cstr_lit }),
>                      DevicePropertyName::Str(str_lit) => {
>                          str_to_c_str!(str_lit.value(), str_lit.span())
> --
> 2.34.1
>

Re: [PATCH 04/12] rust/common/uninit: Fix Clippy's complaints about lifetime

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Clippy complains about the following cases and following its suggestion
> to fix these warnings.
>
> warning: the following explicit lifetimes could be elided: 'a
>   --> common/src/uninit.rs:38:6
>    |
> 38 | impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
>    |      ^^                                   ^^
>    |
>    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
>    = note: `#[warn(clippy::needless_lifetimes)]` on by default
> help: elide the lifetimes
>    |
> 38 - impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
> 38 + impl<T, U> Deref for MaybeUninitField<'_, T, U> {
>    |
>
> warning: the following explicit lifetimes could be elided: 'a
>   --> common/src/uninit.rs:49:6
>    |
> 49 | impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
>    |      ^^                                      ^^
>    |
>    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
> help: elide the lifetimes
>    |
> 49 - impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
> 49 + impl<T, U> DerefMut for MaybeUninitField<'_, T, U> {
>    |
>
> warning: `common` (lib) generated 2 warnings (run `cargo clippy --fix --lib -p common` to apply 2 suggestions)
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  rust/common/src/uninit.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/common/src/uninit.rs b/rust/common/src/uninit.rs
> index e7f9fcd2e3fb..8d021b1dfc6e 100644
> --- a/rust/common/src/uninit.rs
> +++ b/rust/common/src/uninit.rs
> @@ -35,7 +35,7 @@ pub const fn parent_mut(f: &mut Self) -> *mut T {
>      }
>  }
>
> -impl<'a, T, U> Deref for MaybeUninitField<'a, T, U> {
> +impl<T, U> Deref for MaybeUninitField<'_, T, U> {
>      type Target = MaybeUninit<U>;
>
>      fn deref(&self) -> &MaybeUninit<U> {
> @@ -46,7 +46,7 @@ fn deref(&self) -> &MaybeUninit<U> {
>      }
>  }
>
> -impl<'a, T, U> DerefMut for MaybeUninitField<'a, T, U> {
> +impl<T, U> DerefMut for MaybeUninitField<'_, T, U> {
>      fn deref_mut(&mut self) -> &mut MaybeUninit<U> {
>          // SAFETY: self.child was obtained by dereferencing a valid mutable
>          // reference; the content of the memory may be invalid or uninitialized
> --
> 2.34.1
>

Re: [PATCH 07/12] rust/qdev: Rename PropertyInfo field from VALUE to BASE_INFO

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Bit property info will added next. To distinguish different info fields,
> rename `VALUE` to `BASE_INFO`, then it can better reflect that it
> represents the basic property info.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

You can squash this patch to the bit info one

>  rust/hw/core/src/qdev.rs    | 12 ++++++------
>  rust/qemu-macros/src/lib.rs |  2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
> index 2735e2b2c109..d887046d8de1 100644
> --- a/rust/hw/core/src/qdev.rs
> +++ b/rust/hw/core/src/qdev.rs
> @@ -109,13 +109,13 @@ pub trait ResettablePhasesImpl {
>  ///
>  /// # Safety
>  ///
> -/// This trait is marked as `unsafe` because `VALUE` must be a valid raw
> +/// This trait is marked as `unsafe` because `BASE_INFO` must be a valid raw
>  /// reference to a [`bindings::PropertyInfo`].
>  ///
>  /// Note we could not use a regular reference:
>  ///
>  /// ```text
> -/// const VALUE: &bindings::PropertyInfo = ...
> +/// const BASE_INFO: &bindings::PropertyInfo = ...
>  /// ```
>  ///
>  /// because this results in the following compiler error:
> @@ -131,22 +131,22 @@ pub trait ResettablePhasesImpl {
>  /// It is the implementer's responsibility to provide a valid
>  /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
>  pub unsafe trait QDevProp {
> -    const VALUE: *const bindings::PropertyInfo;
> +    const BASE_INFO: *const bindings::PropertyInfo;
>  }
>
>  /// Use [`bindings::qdev_prop_bool`] for `bool`.
>  unsafe impl QDevProp for bool {
> -    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
> +    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
>  }
>
>  /// Use [`bindings::qdev_prop_uint64`] for `u64`.
>  unsafe impl QDevProp for u64 {
> -    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
> +    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
>  }
>
>  /// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`].
>  unsafe impl QDevProp for chardev::CharBackend {
> -    const VALUE: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
> +    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
>  }
>
>  /// Trait to define device properties.
> diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
> index ed4064d6e110..b43ca31bae30 100644
> --- a/rust/qemu-macros/src/lib.rs
> +++ b/rust/qemu-macros/src/lib.rs
> @@ -272,7 +272,7 @@ macro_rules! str_to_c_str {
>              },
>          )?;
>          let field_ty = field.ty.clone();
> -        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::VALUE };
> +        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO };
>          let set_default = defval.is_some();
>          let defval = defval.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
>          properties_expanded.push(quote! {
> --
> 2.34.1
>

Re: [PATCH 08/12] rust/qdev: Support property info for more common types

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Add a helper macro to implement QDevProp trait for u8/u16/u32/usize/i32
> /i64.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

You can do something like this btw:

macro_rules! impl_qdev_prop {
    ($($type:ty => $info:ident),*$(,)?) => {
        $(unsafe impl $crate::qdev::QDevProp for $type {
            const BASE_INFO: *const $crate::bindings::PropertyInfo =
                addr_of!($crate::bindings::$info);
        })*
    };
 }

impl_qdev_prop!(
    bool => qdev_prop_bool,
    u8 => qdev_prop_uint8,
    ...
);


>  rust/hw/core/src/qdev.rs | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
> index d887046d8de1..b57dc05ebb0e 100644
> --- a/rust/hw/core/src/qdev.rs
> +++ b/rust/hw/core/src/qdev.rs
> @@ -134,20 +134,24 @@ pub unsafe trait QDevProp {
>      const BASE_INFO: *const bindings::PropertyInfo;
>  }
>
> -/// Use [`bindings::qdev_prop_bool`] for `bool`.
> -unsafe impl QDevProp for bool {
> -    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_bool);
> -}
> -
> -/// Use [`bindings::qdev_prop_uint64`] for `u64`.
> -unsafe impl QDevProp for u64 {
> -    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_uint64);
> +macro_rules! impl_qdev_prop {
> +    ($type:ty,$info:ident) => {
> +        unsafe impl $crate::qdev::QDevProp for $type {
> +            const BASE_INFO: *const $crate::bindings::PropertyInfo =
> +                addr_of!($crate::bindings::$info);
> +        }
> +    };
>  }
>
> -/// Use [`bindings::qdev_prop_chr`] for [`chardev::CharBackend`].
> -unsafe impl QDevProp for chardev::CharBackend {
> -    const BASE_INFO: *const bindings::PropertyInfo = addr_of!(bindings::qdev_prop_chr);
> -}
> +impl_qdev_prop!(bool, qdev_prop_bool);
> +impl_qdev_prop!(u8, qdev_prop_uint8);
> +impl_qdev_prop!(u16, qdev_prop_uint16);
> +impl_qdev_prop!(u32, qdev_prop_uint32);
> +impl_qdev_prop!(u64, qdev_prop_uint64);
> +impl_qdev_prop!(usize, qdev_prop_usize);
> +impl_qdev_prop!(i32, qdev_prop_int32);
> +impl_qdev_prop!(i64, qdev_prop_int64);
> +impl_qdev_prop!(chardev::CharBackend, qdev_prop_chr);
>
>  /// Trait to define device properties.
>  ///
> --
> 2.34.1
>

Re: [PATCH 09/12] rust/qdev: Support bit property in #property macro

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Add BIT_INFO to QDevProp trait, so that bit related property info could
> be bound to u32 & u64.
>
> Then add "bit=*" field in #property attributes macro to allow device to
> configure bit property.
>
> In addtion, convert the #property field parsing from `if-else` pattern
> to `match` pattern, to help readability. And note, the `bitnr` member of
> `Property` struct is generated by manual TokenStream construction,
> instead of conditional repetition (like #(bitnr: #bitnr,)?) since
> `quote` doesn't support this.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  rust/hw/core/src/qdev.rs    | 15 +++++---
>  rust/qemu-macros/src/lib.rs | 77 +++++++++++++++++++++++++------------
>  2 files changed, 62 insertions(+), 30 deletions(-)
>
> diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
> index b57dc05ebb0e..a8cd9e3c2fd5 100644
> --- a/rust/hw/core/src/qdev.rs
> +++ b/rust/hw/core/src/qdev.rs
> @@ -109,8 +109,8 @@ pub trait ResettablePhasesImpl {
>  ///
>  /// # Safety
>  ///
> -/// This trait is marked as `unsafe` because `BASE_INFO` must be a valid raw
> -/// reference to a [`bindings::PropertyInfo`].
> +/// This trait is marked as `unsafe` because `BASE_INFO` and `BIT_INFO` must be
> +/// the valid raw references to [`bindings::PropertyInfo`].

s/the //

>  ///
>  /// Note we could not use a regular reference:
>  ///
> @@ -132,13 +132,18 @@ pub trait ResettablePhasesImpl {
>  /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
>  pub unsafe trait QDevProp {
>      const BASE_INFO: *const bindings::PropertyInfo;
> +    const BIT_INFO: *const bindings::PropertyInfo = {
> +        panic!("invalid type for bit property");
> +    };

Why is this needed?


>  }
>
>  macro_rules! impl_qdev_prop {
> -    ($type:ty,$info:ident) => {
> +    ($type:ty,$info:ident$(, $bit_info:ident)?) => {
>          unsafe impl $crate::qdev::QDevProp for $type {
>              const BASE_INFO: *const $crate::bindings::PropertyInfo =
>                  addr_of!($crate::bindings::$info);
> +            $(const BIT_INFO: *const $crate::bindings::PropertyInfo =
> +                addr_of!($crate::bindings::$bit_info);)?
>          }
>      };
>  }
> @@ -146,8 +151,8 @@ unsafe impl $crate::qdev::QDevProp for $type {
>  impl_qdev_prop!(bool, qdev_prop_bool);
>  impl_qdev_prop!(u8, qdev_prop_uint8);
>  impl_qdev_prop!(u16, qdev_prop_uint16);
> -impl_qdev_prop!(u32, qdev_prop_uint32);
> -impl_qdev_prop!(u64, qdev_prop_uint64);
> +impl_qdev_prop!(u32, qdev_prop_uint32, qdev_prop_bit);
> +impl_qdev_prop!(u64, qdev_prop_uint64, qdev_prop_bit64);
>  impl_qdev_prop!(usize, qdev_prop_usize);
>  impl_qdev_prop!(i32, qdev_prop_int32);
>  impl_qdev_prop!(i64, qdev_prop_int64);
> diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
> index b43ca31bae30..8109ff239227 100644
> --- a/rust/qemu-macros/src/lib.rs
> +++ b/rust/qemu-macros/src/lib.rs
> @@ -162,6 +162,7 @@ enum DevicePropertyName {
>  #[derive(Debug)]
>  struct DeviceProperty {
>      rename: Option<DevicePropertyName>,
> +    bitnr: Option<syn::Expr>,
>      defval: Option<syn::Expr>,
>  }
>
> @@ -174,40 +175,56 @@ fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
>          debug_assert_eq!(&attribute.to_string(), "property");
>          let mut retval = Self {
>              rename: None,
> +            bitnr: None,
>              defval: None,
>          };
>          let content;
>          _ = syn::parenthesized!(content in bracketed);
>          while !content.is_empty() {
>              let value: syn::Ident = content.parse()?;
> -            if value == "rename" {
> -                let _: syn::Token![=] = content.parse()?;
> -                if retval.rename.is_some() {
> -                    return Err(syn::Error::new(
> -                        value.span(),
> -                        "`rename` can only be used at most once",
> -                    ));
> +            match value {
> +                ref v if v == "rename" => {
> +                    let _: syn::Token![=] = content.parse()?;
> +                    if retval.rename.is_some() {
> +                        return Err(syn::Error::new(
> +                            value.span(),
> +                            "`rename` can only be used at most once",
> +                        ));
> +                    }
> +                    if content.peek(syn::LitStr) {
> +                        retval.rename =
> +                            Some(DevicePropertyName::Str(content.parse::<syn::LitStr>()?));
> +                    } else {
> +                        retval.rename =
> +                            Some(DevicePropertyName::CStr(content.parse::<syn::LitCStr>()?));
> +                    }
>                  }
> -                if content.peek(syn::LitStr) {
> -                    retval.rename = Some(DevicePropertyName::Str(content.parse::<syn::LitStr>()?));
> -                } else {
> -                    retval.rename =
> -                        Some(DevicePropertyName::CStr(content.parse::<syn::LitCStr>()?));
> +                ref v if v == "bit" => {
> +                    let _: syn::Token![=] = content.parse()?;
> +                    if retval.bitnr.is_some() {
> +                        return Err(syn::Error::new(
> +                            value.span(),
> +                            "`bit` can only be used at most once",
> +                        ));
> +                    }
> +                    retval.bitnr = Some(content.parse()?);
> +                }
> +                ref v if v == "default" => {
> +                    let _: syn::Token![=] = content.parse()?;
> +                    if retval.defval.is_some() {
> +                        return Err(syn::Error::new(
> +                            value.span(),
> +                            "`default` can only be used at most once",
> +                        ));
> +                    }
> +                    retval.defval = Some(content.parse()?);
>                  }
> -            } else if value == "default" {
> -                let _: syn::Token![=] = content.parse()?;
> -                if retval.defval.is_some() {
> +                _ => {
>                      return Err(syn::Error::new(
>                          value.span(),
> -                        "`default` can only be used at most once",
> +                        format!("unrecognized field `{value}`"),
>                      ));
>                  }
> -                retval.defval = Some(content.parse()?);
> -            } else {
> -                return Err(syn::Error::new(
> -                    value.span(),
> -                    format!("unrecognized field `{value}`"),
> -                ));
>              }
>
>              if !content.is_empty() {
> @@ -242,7 +259,11 @@ fn derive_device_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream
>      let mut properties_expanded = vec![];
>
>      for (field, prop) in properties {
> -        let DeviceProperty { rename, defval } = prop;
> +        let DeviceProperty {
> +            rename,
> +            bitnr,
> +            defval,
> +        } = prop;
>          let field_name = field.ident.unwrap();
>          macro_rules! str_to_c_str {
>              ($value:expr, $span:expr) => {{
> @@ -272,14 +293,20 @@ macro_rules! str_to_c_str {
>              },
>          )?;
>          let field_ty = field.ty.clone();
> -        let qdev_prop = quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO };
> +        let qdev_prop = if bitnr.is_none() {
> +            quote! { <#field_ty as ::hwcore::QDevProp>::BASE_INFO }
> +        } else {
> +            quote! { <#field_ty as ::hwcore::QDevProp>::BIT_INFO }
> +        };
> +        let bitnr = bitnr.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
>          let set_default = defval.is_some();
>          let defval = defval.unwrap_or(syn::Expr::Verbatim(quote! { 0 }));
>          properties_expanded.push(quote! {
>              ::hwcore::bindings::Property {
>                  name: ::std::ffi::CStr::as_ptr(#prop_name),
> -                info: #qdev_prop ,
> +                info: #qdev_prop,
>                  offset: ::core::mem::offset_of!(#name, #field_name) as isize,
> +                bitnr: #bitnr,
>                  set_default: #set_default,
>                  defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
>                  ..::common::Zeroable::ZERO
> --
> 2.34.1
>

Re: [PATCH 11/12] rust/hpet: Convert qdev properties to #property macro

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Convert HPET's properties to #property macro:
>  * num_timers: usize property.
>  * flags: u32 bit property.
>  * int_route_cap: u32 property.
>  * hpet_offset_saved: bool property.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  rust/hw/timer/hpet/src/device.rs | 55 ++++----------------------------
>  1 file changed, 7 insertions(+), 48 deletions(-)
>
> diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
> index fce75415579d..86638c076666 100644
> --- a/rust/hw/timer/hpet/src/device.rs
> +++ b/rust/hw/timer/hpet/src/device.rs
> @@ -13,9 +13,8 @@
>  use bql::{BqlCell, BqlRefCell};
>  use common::{bitops::IntegerExt, uninit_field_mut};
>  use hwcore::{
> -    bindings::{qdev_prop_bit, qdev_prop_bool, qdev_prop_uint32, qdev_prop_usize},
> -    declare_properties, define_property, DeviceImpl, DeviceMethods, DeviceState, InterruptSource,
> -    Property, ResetType, ResettablePhasesImpl, SysBusDevice, SysBusDeviceImpl, SysBusDeviceMethods,
> +    DeviceImpl, DeviceMethods, DeviceState, InterruptSource, ResetType, ResettablePhasesImpl,
> +    SysBusDevice, SysBusDeviceImpl, SysBusDeviceMethods,
>  };
>  use migration::{
>      self, impl_vmstate_struct, vmstate_fields, vmstate_of, vmstate_subsections, vmstate_validate,
> @@ -520,7 +519,7 @@ fn write(&mut self, reg: TimerRegister, value: u64, shift: u32, len: u32) {
>
>  /// HPET Event Timer Block Abstraction
>  #[repr(C)]
> -#[derive(qom::Object)]
> +#[derive(qom::Object, hwcore::Device)]
>  pub struct HPETState {
>      parent_obj: ParentField<SysBusDevice>,
>      iomem: MemoryRegion,
> @@ -540,10 +539,12 @@ pub struct HPETState {
>      // Internal state
>      /// Capabilities that QEMU HPET supports.
>      /// bit 0: MSI (or FSB) support.
> +    #[property(rename = "msi", bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8, default = false)]
>      flags: u32,
>
>      /// Offset of main counter relative to qemu clock.
>      hpet_offset: BqlCell<u64>,
> +    #[property(rename = "hpet-offset-saved", default = true)]
>      hpet_offset_saved: bool,
>
>      irqs: [InterruptSource; HPET_NUM_IRQ_ROUTES],
> @@ -555,11 +556,13 @@ pub struct HPETState {
>      /// the timers' interrupt can be routed, and is encoded in the
>      /// bits 32:64 of timer N's config register:
>      #[doc(alias = "intcap")]
> +    #[property(rename = "hpet-intcap", default = 0)]
>      int_route_cap: u32,
>
>      /// HPET timer array managed by this timer block.
>      #[doc(alias = "timer")]
>      timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
> +    #[property(rename = "timers", default = HPET_MIN_TIMERS)]
>      num_timers: usize,
>      num_timers_save: BqlCell<u8>,
>
> @@ -901,44 +904,6 @@ impl ObjectImpl for HPETState {
>      const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::<Self>;
>  }
>
> -// TODO: Make these properties user-configurable!
> -declare_properties! {
> -    HPET_PROPERTIES,
> -    define_property!(
> -        c"timers",
> -        HPETState,
> -        num_timers,
> -        unsafe { &qdev_prop_usize },
> -        usize,
> -        default = HPET_MIN_TIMERS
> -    ),
> -    define_property!(
> -        c"msi",
> -        HPETState,
> -        flags,
> -        unsafe { &qdev_prop_bit },
> -        u32,
> -        bit = HPET_FLAG_MSI_SUPPORT_SHIFT as u8,
> -        default = false,
> -    ),
> -    define_property!(
> -        c"hpet-intcap",
> -        HPETState,
> -        int_route_cap,
> -        unsafe { &qdev_prop_uint32 },
> -        u32,
> -        default = 0
> -    ),
> -    define_property!(
> -        c"hpet-offset-saved",
> -        HPETState,
> -        hpet_offset_saved,
> -        unsafe { &qdev_prop_bool },
> -        bool,
> -        default = true
> -    ),
> -}
> -
>  static VMSTATE_HPET_RTC_IRQ_LEVEL: VMStateDescription<HPETState> =
>      VMStateDescriptionBuilder::<HPETState>::new()
>          .name(c"hpet/rtc_irq_level")
> @@ -1001,12 +966,6 @@ impl ObjectImpl for HPETState {
>          ))
>          .build();
>
> -// SAFETY: HPET_PROPERTIES is a valid Property array constructed with the
> -// hwcore::declare_properties macro.
> -unsafe impl hwcore::DevicePropertiesImpl for HPETState {
> -    const PROPERTIES: &'static [Property] = &HPET_PROPERTIES;
> -}
> -
>  impl DeviceImpl for HPETState {
>      const VMSTATE: Option<VMStateDescription<Self>> = Some(VMSTATE_HPET);
>      const REALIZE: Option<fn(&Self) -> util::Result<()>> = Some(Self::realize);
> --
> 2.34.1
>

Re: [PATCH 12/12] rust/qdev: Drop declare_properties & define_property macros

[Manos Pitsidianakis]

On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> After HPET's #property conversion, there's no use case for
> declare_properties & define_property. So get rid of them for now.
>
> In future, if there's something that #property really cannot resolve,
> they can be brought back.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  rust/hw/core/src/qdev.rs | 53 ----------------------------------------
>  1 file changed, 53 deletions(-)
>
> diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
> index a8cd9e3c2fd5..a32ada4c175e 100644
> --- a/rust/hw/core/src/qdev.rs
> +++ b/rust/hw/core/src/qdev.rs
> @@ -248,59 +248,6 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
>      }
>  }
>
> -#[macro_export]
> -macro_rules! define_property {
> -    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, bit = $bitnr:expr, default = $defval:expr$(,)*) => {
> -        $crate::bindings::Property {
> -            // use associated function syntax for type checking
> -            name: ::std::ffi::CStr::as_ptr($name),
> -            info: $prop,
> -            offset: ::std::mem::offset_of!($state, $field) as isize,
> -            bitnr: $bitnr,
> -            set_default: true,
> -            defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
> -            ..::common::zeroable::Zeroable::ZERO
> -        }
> -    };
> -    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty, default = $defval:expr$(,)*) => {
> -        $crate::bindings::Property {
> -            // use associated function syntax for type checking
> -            name: ::std::ffi::CStr::as_ptr($name),
> -            info: $prop,
> -            offset: ::std::mem::offset_of!($state, $field) as isize,
> -            set_default: true,
> -            defval: $crate::bindings::Property__bindgen_ty_1 { u: $defval as u64 },
> -            ..::common::zeroable::Zeroable::ZERO
> -        }
> -    };
> -    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:ty$(,)*) => {
> -        $crate::bindings::Property {
> -            // use associated function syntax for type checking
> -            name: ::std::ffi::CStr::as_ptr($name),
> -            info: $prop,
> -            offset: ::std::mem::offset_of!($state, $field) as isize,
> -            set_default: false,
> -            ..::common::zeroable::Zeroable::ZERO
> -        }
> -    };
> -}
> -
> -#[macro_export]
> -macro_rules! declare_properties {
> -    ($ident:ident, $($prop:expr),*$(,)*) => {
> -        pub static $ident: [$crate::bindings::Property; {
> -            let mut len = 0;
> -            $({
> -                _ = stringify!($prop);
> -                len += 1;
> -            })*
> -            len
> -        }] = [
> -            $($prop),*,
> -        ];
> -    };
> -}
> -
>  unsafe impl ObjectType for DeviceState {
>      type Class = DeviceClass;
>      const TYPE_NAME: &'static CStr =
> --
> 2.34.1
>

Re: [PATCH 01/12] subprojects: Update .gitignore for proc-macro2 and syn

[Zhao Liu]

On Tue, Sep 16, 2025 at 12:58:24PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 16 Sep 2025 12:58:24 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: Re: [PATCH 01/12] subprojects: Update .gitignore for proc-macro2
>  and syn
> 
> On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >  subprojects/.gitignore | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/subprojects/.gitignore b/subprojects/.gitignore
> > index f4281934ce11..9d579f72d121 100644
> > --- a/subprojects/.gitignore
> > +++ b/subprojects/.gitignore
> > @@ -16,7 +16,7 @@
> >  /libc-0.2.162
> >  /proc-macro-error-1.0.4
> >  /proc-macro-error-attr-1.0.4
> > -/proc-macro2-1.0.84
> > +/proc-macro2-1.0.95
> >  /quote-1.0.36
> > -/syn-2.0.66
> > +/syn-2.0.104
> >  /unicode-ident-1.0.12
> > --
> > 2.34.1
> >
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Thanks!

> Maybe we could change the entries to match any directory with a version, e.g.
> 
> /proc-macro2-[0-9.]*/

Here I prefer exact matching, as it allows for timely cleanup of outdated
subprojects.

Regards,
Zhao

Re: [PATCH 07/12] rust/qdev: Rename PropertyInfo field from VALUE to BASE_INFO

[Zhao Liu]

On Tue, Sep 16, 2025 at 01:10:39PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 16 Sep 2025 13:10:39 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: Re: [PATCH 07/12] rust/qdev: Rename PropertyInfo field from VALUE
>  to BASE_INFO
> 
> On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > Bit property info will added next. To distinguish different info fields,
> > rename `VALUE` to `BASE_INFO`, then it can better reflect that it
> > represents the basic property info.
> >
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Thanks!

> You can squash this patch to the bit info one

Yes, after patch 8, renaming doesn't need to change too many places.

Regards,
Zhao

Re: [PATCH 08/12] rust/qdev: Support property info for more common types

[Zhao Liu]

On Tue, Sep 16, 2025 at 01:13:28PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 16 Sep 2025 13:13:28 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: Re: [PATCH 08/12] rust/qdev: Support property info for more common
>  types
> 
> On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > Add a helper macro to implement QDevProp trait for u8/u16/u32/usize/i32
> > /i64.
> >
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Thanks!

> You can do something like this btw:
> 
> macro_rules! impl_qdev_prop {
>     ($($type:ty => $info:ident),*$(,)?) => {
>         $(unsafe impl $crate::qdev::QDevProp for $type {
>             const BASE_INFO: *const $crate::bindings::PropertyInfo =
>                 addr_of!($crate::bindings::$info);
>         })*
>     };
>  }
> 
> impl_qdev_prop!(
>     bool => qdev_prop_bool,
>     u8 => qdev_prop_uint8,
>     ...
> );
> 

Yes, this can work! But a benefit of non-repetitive macro is that it
can also be exposed for use by other crates if necessary.

Regards,
Zhao

Re: [PATCH 09/12] rust/qdev: Support bit property in #property macro

[Zhao Liu]

On Tue, Sep 16, 2025 at 01:16:25PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 16 Sep 2025 13:16:25 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: Re: [PATCH 09/12] rust/qdev: Support bit property in #property
>  macro
> 
> On Tue, Sep 16, 2025 at 11:34 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > Add BIT_INFO to QDevProp trait, so that bit related property info could
> > be bound to u32 & u64.
> >
> > Then add "bit=*" field in #property attributes macro to allow device to
> > configure bit property.
> >
> > In addtion, convert the #property field parsing from `if-else` pattern
> > to `match` pattern, to help readability. And note, the `bitnr` member of
> > `Property` struct is generated by manual TokenStream construction,
> > instead of conditional repetition (like #(bitnr: #bitnr,)?) since
> > `quote` doesn't support this.
> >
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >  rust/hw/core/src/qdev.rs    | 15 +++++---
> >  rust/qemu-macros/src/lib.rs | 77 +++++++++++++++++++++++++------------
> >  2 files changed, 62 insertions(+), 30 deletions(-)
> >
> > diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
> > index b57dc05ebb0e..a8cd9e3c2fd5 100644
> > --- a/rust/hw/core/src/qdev.rs
> > +++ b/rust/hw/core/src/qdev.rs
> > @@ -109,8 +109,8 @@ pub trait ResettablePhasesImpl {
> >  ///
> >  /// # Safety
> >  ///
> > -/// This trait is marked as `unsafe` because `BASE_INFO` must be a valid raw
> > -/// reference to a [`bindings::PropertyInfo`].
> > +/// This trait is marked as `unsafe` because `BASE_INFO` and `BIT_INFO` must be
> > +/// the valid raw references to [`bindings::PropertyInfo`].
> 
> s/the //

Okay.

> >  ///
> >  /// Note we could not use a regular reference:
> >  ///
> > @@ -132,13 +132,18 @@ pub trait ResettablePhasesImpl {
> >  /// [`bindings::PropertyInfo`] pointer for the trait implementation to be safe.
> >  pub unsafe trait QDevProp {
> >      const BASE_INFO: *const bindings::PropertyInfo;
> > +    const BIT_INFO: *const bindings::PropertyInfo = {
> > +        panic!("invalid type for bit property");
> > +    };
> 
> Why is this needed?

Only 3 types supports bit:

u32: qdev_prop_bit
u64: qdev_prop_bit64
OnOffAuto: qdev_prop_on_off_auto_bit64 (not support yet)

So for other types don't support bit, they need default BIT_INFO item,
otherwise, we will meet the error:

 not all trait items implemented, missing: `BIT_INFO`


And this panic can provide richer info about why a type can't support
bit property. (I just refer the implementation of `trait VMState`).

Thanks,
Zhao

Re: [PATCH 09/12] rust/qdev: Support bit property in #property macro

[Paolo Bonzini]

On 9/16/25 10:55, Zhao Liu wrote:
> Add BIT_INFO to QDevProp trait, so that bit related property info could
> be bound to u32 & u64.
> 
> Then add "bit=*" field in #property attributes macro to allow device to
> configure bit property.
> 
> In addtion, convert the #property field parsing from `if-else` pattern
> to `match` pattern, to help readability. And note, the `bitnr` member of
> `Property` struct is generated by manual TokenStream construction,
> instead of conditional repetition (like #(bitnr: #bitnr,)?) since
> `quote` doesn't support this.

We're almost certainly going to have more attribute parsing for 
ToMigrationState, therefore IMO it's time to drop the handwritten parser 
in favor of 
https://lore.kernel.org/qemu-devel/20250717062727.305466-1-pbonzini@redhat.com/. 
   Then the extra parsing would be just one line:

+        parser.once("bitnr", with::eq(set::parse(&mut self.bitnr)));

No other complaint!

Paolo

Re: [PATCH 09/12] rust/qdev: Support bit property in #property macro

[Paolo Bonzini]

On 9/17/25 09:25, Zhao Liu wrote:
> Only 3 types supports bit:
> 
> u32: qdev_prop_bit
> u64: qdev_prop_bit64
> OnOffAuto: qdev_prop_on_off_auto_bit64 (not support yet)

Yes, this one needs to wait for QAPI (unless we move OnOffAuto to core 
code).

> So for other types don't support bit, they need default BIT_INFO item,
> otherwise, we will meet the error:
> 
>   not all trait items implemented, missing: `BIT_INFO`
> 
> And this panic can provide richer info about why a type can't support
> bit property. (I just refer the implementation of `trait VMState`).

Yep, looks good. I added to rust-next a couple patches that you can 
rebase on, to use the attrs crate.

Also please add a testcase.

Paolo