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.1On 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
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
>
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
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
© 2016 - 2025 Red Hat, Inc.