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

[PATCH v2 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 7a28f05aafb7 of Paolo's
rust-next branch.

Changes since v1
 * Use attrs to parse "bit" field in #property.
 * Squash the "VALUE" renaming patch into the patch of "bit" property
   support.
 * Add test case of bit property.

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: Support property info for more common types
  rust/qdev: Support bit property in #property macro
  rust/qdev: Test bit property for #property
  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      |  22 +++++--
 rust/qemu-macros/src/tests.rs    | 107 ++++++++++++++++++++++++++++++-
 subprojects/.gitignore           |   5 +-
 6 files changed, 168 insertions(+), 130 deletions(-)

-- 
2.34.1

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

[Zhao Liu]

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 subprojects/.gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index 4f2a489105e9..085de898d9d3 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -17,7 +17,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
 /serde-1.0.219
 /syn-2.0.104
-- 
2.34.1

[PATCH v2 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.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 085de898d9d3..a7058cd4b0e5 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -22,3 +22,6 @@
 /serde-1.0.219
 /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 v2 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.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 8151fb6d4e1e..ed2b997b24a4 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -246,8 +246,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 v2 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)

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 v2 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 e384cad4d26b..d8113238e513 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 v2 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 d8113238e513..89c21e7c6b50 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 v2 07/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 89c21e7c6b50..14d6ecf2ca23 100644
--- a/rust/hw/core/src/qdev.rs
+++ b/rust/hw/core/src/qdev.rs
@@ -134,20 +134,24 @@ pub unsafe trait QDevProp {
     const VALUE: *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);
-}
-
-/// Use [`bindings::qdev_prop_uint64`] for `u64`.
-unsafe impl QDevProp for u64 {
-    const VALUE: *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 VALUE: *const $crate::bindings::PropertyInfo =
+                addr_of!($crate::bindings::$info);
+        }
+    };
 }
 
-/// 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);
-}
+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 v2 08/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.

In addtion, rename VALUE member of QDevProp trait to BASE_INFO.

And update the test cases about qdev property.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Changes since v1:
 * Use attrs to parse "lib" field in #property.
 * Rename VALUE to BASE_INFO in this patch.
 * Update test cases.
---
 rust/hw/core/src/qdev.rs      | 19 ++++++++++++-------
 rust/qemu-macros/src/lib.rs   | 18 +++++++++++++++---
 rust/qemu-macros/src/tests.rs |  8 +++++---
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs
index 14d6ecf2ca23..5cc8a35d1ab0 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 `VALUE` must be a valid raw
-/// reference to a [`bindings::PropertyInfo`].
+/// This trait is marked as `unsafe` because `BASE_INFO` and `BIT_INFO` must be
+/// valid raw references to [`bindings::PropertyInfo`].
 ///
 /// Note we could not use a regular reference:
 ///
@@ -131,14 +131,19 @@ 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;
+    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 VALUE: *const $crate::bindings::PropertyInfo =
+            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 ed2b997b24a4..c459f9bcb42f 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -183,6 +183,7 @@ fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
 #[derive(Default, Debug)]
 struct DeviceProperty {
     rename: Option<DevicePropertyName>,
+    bitnr: Option<syn::Expr>,
     defval: Option<syn::Expr>,
 }
 
@@ -191,6 +192,7 @@ fn parse_from(&mut self, a: &Attribute) -> syn::Result<()> {
         use attrs::{set, with, Attrs};
         let mut parser = Attrs::new();
         parser.once("rename", with::eq(set::parse(&mut self.rename)));
+        parser.once("bit", with::eq(set::parse(&mut self.bitnr)));
         parser.once("default", with::eq(set::parse(&mut self.defval)));
         a.parse_args_with(&mut parser)
     }
@@ -226,7 +228,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) => {{
@@ -256,14 +262,20 @@ 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 = 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
diff --git a/rust/qemu-macros/src/tests.rs b/rust/qemu-macros/src/tests.rs
index 1ce43aa568ec..c6b00c9783a8 100644
--- a/rust/qemu-macros/src/tests.rs
+++ b/rust/qemu-macros/src/tests.rs
@@ -60,7 +60,7 @@ struct DummyState {
                 migrate_clock: bool,
             }
         },
-        "Expected one of `default` or `rename`"
+        "Expected one of `bit`, `default` or `rename`"
     );
     // Check that repeated attributes are not allowed:
     derive_compile_fail!(
@@ -106,8 +106,9 @@ unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
                 const PROPERTIES: &'static [::hwcore::bindings::Property] = &[
                     ::hwcore::bindings::Property {
                         name: ::std::ffi::CStr::as_ptr(c"migrate_clock"),
-                        info: <bool as ::hwcore::QDevProp>::VALUE,
+                        info: <bool as ::hwcore::QDevProp>::BASE_INFO,
                         offset: ::core::mem::offset_of!(DummyState, migrate_clock) as isize,
+                        bitnr: 0,
                         set_default: true,
                         defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: true as u64 },
                         ..::common::Zeroable::ZERO
@@ -133,8 +134,9 @@ unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
                 const PROPERTIES: &'static [::hwcore::bindings::Property] = &[
                     ::hwcore::bindings::Property {
                         name: ::std::ffi::CStr::as_ptr(c"migrate-clk"),
-                        info: <bool as ::hwcore::QDevProp>::VALUE,
+                        info: <bool as ::hwcore::QDevProp>::BASE_INFO,
                         offset: ::core::mem::offset_of!(DummyState, migrate_clock) as isize,
+                        bitnr: 0,
                         set_default: true,
                         defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: true as u64 },
                         ..::common::Zeroable::ZERO
-- 
2.34.1

[PATCH v2 09/12] rust/qdev: Test bit property for #property

[Zhao Liu]

There's a diference between Rust and C:

Though C macro (e.g., DEFINE_PROP_BIT or DEFINE_PROP_BIT64) always
requires default value, Rust side allows to omit this "default" field
in #property, and provides a default value ("0" - false) for this
field.

This minor difference does not break user habits and should be
acceptable. Therefore, the test cases also cover this scenario.

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

diff --git a/rust/qemu-macros/src/tests.rs b/rust/qemu-macros/src/tests.rs
index c6b00c9783a8..7516bc3d92a0 100644
--- a/rust/qemu-macros/src/tests.rs
+++ b/rust/qemu-macros/src/tests.rs
@@ -89,6 +89,19 @@ struct DummyState {
         "Duplicate argument",
         "Already used here",
     );
+    derive_compile_fail!(
+        derive_device_or_error,
+        quote! {
+            #[repr(C)]
+            #[derive(Device)]
+            struct DummyState {
+                #[property(bit = 0, bit = 1)]
+                flags: u32,
+            }
+        },
+        "Duplicate argument",
+        "Already used here",
+    );
     // Check that the field name is preserved when `rename` isn't used:
     derive_compile!(
         derive_device_or_error,
@@ -145,6 +158,92 @@ unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
             }
         }
     );
+    // Check that `bit` value is used for the bit property without default
+    // value (note: though C macro (e.g., DEFINE_PROP_BIT) always requires
+    // default value, Rust side allows to default this field to "0"):
+    derive_compile!(
+        derive_device_or_error,
+        quote! {
+            #[repr(C)]
+            #[derive(Device)]
+            pub struct DummyState {
+                parent: ParentField<DeviceState>,
+                #[property(bit = 3)]
+                flags: u32,
+            }
+        },
+        quote! {
+            unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
+                const PROPERTIES: &'static [::hwcore::bindings::Property] = &[
+                    ::hwcore::bindings::Property {
+                        name: ::std::ffi::CStr::as_ptr(c"flags"),
+                        info: <u32 as ::hwcore::QDevProp>::BIT_INFO,
+                        offset: ::core::mem::offset_of!(DummyState, flags) as isize,
+                        bitnr: 3,
+                        set_default: false,
+                        defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: 0 as u64 },
+                        ..::common::Zeroable::ZERO
+                    }
+                ];
+            }
+        }
+    );
+    // Check that `bit` value is used for the bit property when used:
+    derive_compile!(
+        derive_device_or_error,
+        quote! {
+            #[repr(C)]
+            #[derive(Device)]
+            pub struct DummyState {
+                parent: ParentField<DeviceState>,
+                #[property(bit = 3, default = true)]
+                flags: u32,
+            }
+        },
+        quote! {
+            unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
+                const PROPERTIES: &'static [::hwcore::bindings::Property] = &[
+                    ::hwcore::bindings::Property {
+                        name: ::std::ffi::CStr::as_ptr(c"flags"),
+                        info: <u32 as ::hwcore::QDevProp>::BIT_INFO,
+                        offset: ::core::mem::offset_of!(DummyState, flags) as isize,
+                        bitnr: 3,
+                        set_default: true,
+                        defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: true as u64 },
+                        ..::common::Zeroable::ZERO
+                    }
+                ];
+            }
+        }
+    );
+    // Check that `bit` value is used for the bit property with rename when used:
+    derive_compile!(
+        derive_device_or_error,
+        quote! {
+            #[repr(C)]
+            #[derive(Device)]
+            pub struct DummyState {
+                parent: ParentField<DeviceState>,
+                #[property(rename = "msi", bit = 3, default = false)]
+                flags: u64,
+            }
+        },
+        quote! {
+            unsafe impl ::hwcore::DevicePropertiesImpl for DummyState {
+                const PROPERTIES: &'static [::hwcore::bindings::Property] = &[
+                    ::hwcore::bindings::Property {
+                        name: ::std::ffi::CStr::as_ptr(c"msi"),
+                        info: <u64 as ::hwcore::QDevProp>::BIT_INFO,
+                        offset: ::core::mem::offset_of!(DummyState, flags) as isize,
+                        bitnr: 3,
+                        set_default: true,
+                        defval: ::hwcore::bindings::Property__bindgen_ty_1 { u: false as u64 },
+                        ..::common::Zeroable::ZERO
+                    }
+                ];
+            }
+        }
+    );
 }
 
 #[test]
-- 
2.34.1

[PATCH v2 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 v2 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.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 v2 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.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
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 5cc8a35d1ab0..c3097a284d73 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 v2 00/12] rust: miscellaneous cleanup & HPET #property conversion

[Paolo Bonzini]

Queued, thanks.

Paolo

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

[Stefan Zabka]

At the risk of being terribly annoying:

On 20/09/2025 18:05, Zhao Liu wrote:> From: Manos Pitsidianakis 
<manos.pitsidianakis@linaro.org>
> 
> Refine the documentation to clarify:
>   * `unsfae` requires that `VALUE` must be valid.

I'm assuming this is a typo. If so, is it important enough to be fixed 
or is it fine, since it's "only" the commit message?