From 60c96a8775c163383043d4ece6065dcb8f940856 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 16 Jul 2025 10:50:58 +0200 Subject: rust: qemu-macros: switch #[property] parsing to use combinators Since we are going to add more attribute parsing for high-level migration state macros, use the attrs crate instead of a handwritten parser for device properties as well. Signed-off-by: Paolo Bonzini --- rust/qemu-macros/src/lib.rs | 86 +++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 53 deletions(-) (limited to 'rust/qemu-macros/src/lib.rs') diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs index 830b432698..7ab1806177 100644 --- a/rust/qemu-macros/src/lib.rs +++ b/rust/qemu-macros/src/lib.rs @@ -3,10 +3,14 @@ // SPDX-License-Identifier: GPL-2.0-or-later use proc_macro::TokenStream; -use quote::{quote, quote_spanned, ToTokens}; +use quote::{quote, quote_spanned}; use syn::{ - parse::Parse, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, - token::Comma, Data, DeriveInput, Error, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token, + parse::{Parse, ParseStream}, + parse_macro_input, parse_quote, + punctuated::Punctuated, + spanned::Spanned, + token::Comma, + Attribute, Data, DeriveInput, Error, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token, Variant, }; mod bits; @@ -159,61 +163,37 @@ enum DevicePropertyName { Str(syn::LitStr), } -#[derive(Debug)] +impl Parse for DevicePropertyName { + fn parse(input: ParseStream<'_>) -> syn::Result { + let lo = input.lookahead1(); + if lo.peek(syn::LitStr) { + Ok(Self::Str(input.parse()?)) + } else if lo.peek(syn::LitCStr) { + Ok(Self::CStr(input.parse()?)) + } else { + Err(lo.error()) + } + } +} + +#[derive(Default, Debug)] struct DeviceProperty { rename: Option, defval: Option, } -impl Parse for DeviceProperty { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - let _: syn::Token![#] = input.parse()?; - let bracketed; - _ = syn::bracketed!(bracketed in input); - let attribute = bracketed.parse::()?; - debug_assert_eq!(&attribute.to_string(), "property"); - let mut retval = Self { - rename: 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", - )); - } - if content.peek(syn::LitStr) { - retval.rename = Some(DevicePropertyName::Str(content.parse::()?)); - } else { - retval.rename = - Some(DevicePropertyName::CStr(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", - )); - } - retval.defval = Some(content.parse()?); - } else { - return Err(syn::Error::new( - value.span(), - format!("unrecognized field `{value}`"), - )); - } +impl DeviceProperty { + 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("default", with::eq(set::parse(&mut self.defval))); + a.parse_args_with(&mut parser) + } - if !content.is_empty() { - let _: syn::Token![,] = content.parse()?; - } - } + fn parse(a: &Attribute) -> syn::Result { + let mut retval = Self::default(); + retval.parse_from(a)?; Ok(retval) } } @@ -235,7 +215,7 @@ fn derive_device_or_error(input: DeriveInput) -> Result, Error>>()?; let name = &input.ident; -- cgit 1.4.1 From 1bbac0ca88cdfd6ac019685a855c92831e3862e3 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sun, 21 Sep 2025 00:05:11 +0800 Subject: rust/qemu-macros: Fix Clippy's complaints about lambda parameter naming error: `rename` shadows a previous, unrelated binding --> qemu-macros/src/lib.rs:265:14 | 265 | |rename| -> Result { | ^^^^^^ | 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 Signed-off-by: Zhao Liu Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-4-zhao1.liu@intel.com --- rust/qemu-macros/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/qemu-macros/src/lib.rs') diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs index 7ab1806177..37e1b723bd 100644 --- a/rust/qemu-macros/src/lib.rs +++ b/rust/qemu-macros/src/lib.rs @@ -242,8 +242,8 @@ fn derive_device_or_error(input: DeriveInput) -> Result Result { - match rename { + |prop_rename| -> Result { + 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()) -- cgit 1.4.1 From 9686aa9a05d9ef25c13ddc0af0123e2fc569b0ac Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Sun, 21 Sep 2025 00:05:16 +0800 Subject: rust/qdev: Support bit property in #property macro 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 Signed-off-by: Paolo Bonzini Link: https://lore.kernel.org/r/20250920160520.3699591-9-zhao1.liu@intel.com --- 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(-) (limited to 'rust/qemu-macros/src/lib.rs') diff --git a/rust/hw/core/src/qdev.rs b/rust/hw/core/src/qdev.rs index 85422e0379..9c82e1716c 100644 --- a/rust/hw/core/src/qdev.rs +++ b/rust/hw/core/src/qdev.rs @@ -109,8 +109,8 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// /// # 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 @@ unsafe extern "C" fn rust_resettable_exit_fn( /// 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 @@ macro_rules! impl_qdev_prop { 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 37e1b723bd..3e21b67b47 100644 --- a/rust/qemu-macros/src/lib.rs +++ b/rust/qemu-macros/src/lib.rs @@ -179,6 +179,7 @@ impl Parse for DevicePropertyName { #[derive(Default, Debug)] struct DeviceProperty { rename: Option, + bitnr: Option, defval: Option, } @@ -187,6 +188,7 @@ impl DeviceProperty { 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) } @@ -222,7 +224,11 @@ fn derive_device_or_error(input: DeriveInput) -> Result {{ @@ -252,14 +258,20 @@ fn derive_device_or_error(input: DeriveInput) -> Result::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 00a106612f..ec137132ae 100644 --- a/rust/qemu-macros/src/tests.rs +++ b/rust/qemu-macros/src/tests.rs @@ -60,7 +60,7 @@ fn test_derive_device() { 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 @@ fn test_derive_device() { const PROPERTIES: &'static [::hwcore::bindings::Property] = &[ ::hwcore::bindings::Property { name: ::std::ffi::CStr::as_ptr(c"migrate_clock"), - info: ::VALUE, + info: ::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 @@ fn test_derive_device() { const PROPERTIES: &'static [::hwcore::bindings::Property] = &[ ::hwcore::bindings::Property { name: ::std::ffi::CStr::as_ptr(c"migrate-clk"), - info: ::VALUE, + info: ::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 -- cgit 1.4.1