From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
To: qemu-devel@nongnu.org
Cc: qemu-rust@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Zhao Liu" <zhao1.liu@intel.com>,
"Manos Pitsidianakis" <manos.pitsidianakis@linaro.org>
Subject: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
Date: Thu, 22 May 2025 11:12:30 +0300 [thread overview]
Message-ID: <20250522-rust-qdev-properties-v1-1-5b18b218bad1@linaro.org> (raw)
Add derive macro for declaring qdev properties directly above the field
definitions. To do this, we split DeviceImpl::properties method on a
separate trait so we can implement only that part in the derive macro
expansion (we cannot partially implement the DeviceImpl trait).
Adding a `property` attribute above the field declaration will generate
a `qemu_api::bindings::Property` array member in the device's property
list.
As a proof of concept, I added a typed alias for booleans,
`bool_property` that allows you to skip specifying qdev_prop_bool.
This is unnecessary though, because once we have the
const_refs_to_static feature we can introduce a QdevProp trait that
returns a reference to a type's qdev_prop_* global variable. We cannot
do this now because for our minimum Rust version we cannot refer to
statics from const values.
It'd look like this:
pub trait QDevProp {
const VALUE: &'static bindings::PropertyInfo;
}
impl QDevProp for bool {
const VALUE: &'static bindings::PropertyInfo = unsafe {
&bindings::qdev_prop_bool };
}
impl QDevProp for u64 {
const VALUE: &'static bindings::PropertyInfo = unsafe {
&bindings::qdev_prop_uint64 };
}
// etc.. for all basic types
So, this patch is not for merging yet, I will wait until we upgrade the
Rust version and re-spin it with a proper trait-based implementation (and
also split it into individual steps/patches).
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
rust/hw/char/pl011/src/device.rs | 13 +--
rust/hw/char/pl011/src/device_class.rs | 26 +-----
rust/hw/timer/hpet/src/hpet.rs | 4 +-
rust/qemu-api-macros/src/lib.rs | 157 ++++++++++++++++++++++++++++++++-
rust/qemu-api/src/qdev.rs | 22 +++--
rust/qemu-api/tests/tests.rs | 9 +-
6 files changed, 187 insertions(+), 44 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bde3be65c5b0d9dbb117407734d93fff577ddecf..e22f5421dc5d9cd5c6fa8b11ab746e5c254bdb37 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,7 +10,10 @@
irq::{IRQState, InterruptSource},
memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
- qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
+ qdev::{
+ Clock, ClockEvent, DeviceImpl, DevicePropertiesImpl, DeviceState, ResetType,
+ ResettablePhasesImpl,
+ },
qom::{ObjectImpl, Owned, ParentField},
static_assert,
sysbus::{SysBusDevice, SysBusDeviceImpl},
@@ -98,12 +101,13 @@ pub struct PL011Registers {
}
#[repr(C)]
-#[derive(qemu_api_macros::Object)]
+#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
/// PL011 Device Model in QEMU
pub struct PL011State {
pub parent_obj: ParentField<SysBusDevice>,
pub iomem: MemoryRegion,
#[doc(alias = "chr")]
+ #[property(name = c"chardev", qdev_prop = qemu_api::bindings::qdev_prop_chr)]
pub char_backend: CharBackend,
pub regs: BqlRefCell<PL011Registers>,
/// QEMU interrupts
@@ -122,6 +126,7 @@ pub struct PL011State {
#[doc(alias = "clk")]
pub clock: Owned<Clock>,
#[doc(alias = "migrate_clk")]
+ #[bool_property(name = c"migrate-clk", default = true)]
pub migrate_clock: bool,
}
@@ -169,9 +174,6 @@ impl ObjectImpl for PL011State {
}
impl DeviceImpl for PL011State {
- fn properties() -> &'static [Property] {
- &device_class::PL011_PROPERTIES
- }
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&device_class::VMSTATE_PL011)
}
@@ -703,6 +705,7 @@ impl PL011Impl for PL011Luminary {
const DEVICE_ID: DeviceId = DeviceId(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
}
+impl DevicePropertiesImpl for PL011Luminary {}
impl DeviceImpl for PL011Luminary {}
impl ResettablePhasesImpl for PL011Luminary {}
impl SysBusDeviceImpl for PL011Luminary {}
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index d328d846323f6080a9573053767e51481eb32941..83d70d7d82aac4a3252a0b4cb24af705b01d3635 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -8,11 +8,8 @@
};
use qemu_api::{
- bindings::{qdev_prop_bool, qdev_prop_chr},
- prelude::*,
- vmstate::VMStateDescription,
- vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
- zeroable::Zeroable,
+ prelude::*, vmstate::VMStateDescription, vmstate_clock, vmstate_fields, vmstate_of,
+ vmstate_struct, vmstate_subsections, vmstate_unused, zeroable::Zeroable,
};
use crate::device::{PL011Registers, PL011State};
@@ -82,22 +79,3 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
},
..Zeroable::ZERO
};
-
-qemu_api::declare_properties! {
- PL011_PROPERTIES,
- qemu_api::define_property!(
- c"chardev",
- PL011State,
- char_backend,
- unsafe { &qdev_prop_chr },
- CharBackend
- ),
- qemu_api::define_property!(
- c"migrate-clk",
- PL011State,
- migrate_clock,
- unsafe { &qdev_prop_bool },
- bool,
- default = true
- ),
-}
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 779681d650998291f138e8cc61807612b8597961..21ebcfc9f22f8f5463812db218a1dc2039eda75b 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -1033,11 +1033,13 @@ impl ObjectImpl for HPETState {
..Zeroable::ZERO
};
-impl DeviceImpl for HPETState {
+impl qemu_api::qdev::DevicePropertiesImpl for HPETState {
fn properties() -> &'static [Property] {
&HPET_PROPERTIES
}
+}
+impl DeviceImpl for HPETState {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE_HPET)
}
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index f97449bb304b575c7d8c3272f287a81a9f8c9131..c5b746198d183d214526c8f0132b23d375e2d27b 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -3,10 +3,11 @@
// SPDX-License-Identifier: GPL-2.0-or-later
use proc_macro::TokenStream;
-use quote::quote;
+use quote::{quote, quote_spanned, ToTokens};
use syn::{
- parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Data,
- DeriveInput, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token, Variant,
+ parse::Parse, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
+ token::Comma, Data, DeriveInput, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token,
+ Variant,
};
mod utils;
@@ -143,6 +144,156 @@ pub const fn raw_get(slot: *mut Self) -> *mut <Self as crate::cell::Wrapper>::Wr
})
}
+#[derive(Debug)]
+struct DeviceProperty {
+ name: Option<syn::LitCStr>,
+ qdev_prop: Option<syn::Path>,
+ assert_type: Option<proc_macro2::TokenStream>,
+ bitnr: Option<syn::Expr>,
+ defval: Option<syn::Expr>,
+}
+
+impl Parse for DeviceProperty {
+ fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
+ let _: syn::Token![#] = input.parse()?;
+ let bracketed;
+ _ = syn::bracketed!(bracketed in input);
+ let attribute = bracketed.parse::<syn::Ident>()?.to_string();
+ let (assert_type, qdev_prop) = match attribute.as_str() {
+ "property" => (None, None),
+ "bool_property" => (
+ Some(quote! { bool }),
+ Some(syn::parse2(
+ quote! { ::qemu_api::bindings::qdev_prop_bool },
+ )?),
+ ),
+ other => unreachable!("Got unexpected DeviceProperty attribute `{}`", other),
+ };
+ let mut retval = Self {
+ name: None,
+ qdev_prop,
+ assert_type,
+ bitnr: None,
+ defval: None,
+ };
+ let content;
+ _ = syn::parenthesized!(content in bracketed);
+ while !content.is_empty() {
+ let value: syn::Ident = content.parse()?;
+ if value == "name" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.name.is_some() {
+ panic!("`name` can only be used at most once");
+ }
+ retval.name = Some(content.parse()?);
+ } else if value == "qdev_prop" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.assert_type.is_some() {
+ // qdev_prop will be Some(_), but we want to print a helpful error message
+ // explaining why you should use #[property(...)] instead of saying "you
+ // defined qdev_prop twice".
+ panic!("Use `property` attribute instead of `{attribute}` if you want to override `qdev_prop` value.");
+ }
+ if retval.qdev_prop.is_some() {
+ panic!("`qdev_prop` can only be used at most once");
+ }
+ retval.qdev_prop = Some(content.parse()?);
+ } else if value == "bitnr" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.bitnr.is_some() {
+ panic!("`bitnr` can only be used at most once");
+ }
+ retval.bitnr = Some(content.parse()?);
+ } else if value == "default" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.defval.is_some() {
+ panic!("`default` can only be used at most once");
+ }
+ retval.defval = Some(content.parse()?);
+ } else {
+ panic!("unrecognized field `{value}`");
+ }
+
+ if !content.is_empty() {
+ let _: syn::Token![,] = content.parse()?;
+ }
+ }
+ Ok(retval)
+ }
+}
+
+#[proc_macro_derive(DeviceProperties, attributes(property, bool_property))]
+pub fn derive_device_properties(input: TokenStream) -> TokenStream {
+ let span = proc_macro::Span::call_site();
+ let input = parse_macro_input!(input as DeriveInput);
+ let properties: Vec<(syn::Field, proc_macro2::Span, DeviceProperty)> = match input.data {
+ syn::Data::Struct(syn::DataStruct {
+ fields: syn::Fields::Named(fields),
+ ..
+ }) => fields
+ .named
+ .iter()
+ .flat_map(|f| {
+ f.attrs
+ .iter()
+ .filter(|a| a.path().is_ident("property") || a.path().is_ident("bool_property"))
+ .map(|a| {
+ (
+ f.clone(),
+ f.span(),
+ syn::parse(a.to_token_stream().into())
+ .expect("could not parse property attr"),
+ )
+ })
+ })
+ .collect::<Vec<_>>(),
+ _other => unreachable!(),
+ };
+ let name = &input.ident;
+
+ let mut assertions = vec![];
+ let mut properties_expanded = vec![];
+ let zero = syn::Expr::Verbatim(quote! { 0 });
+ for (field, field_span, prop) in properties {
+ let prop_name = prop.name.as_ref().unwrap();
+ let field_name = field.ident.as_ref().unwrap();
+ let qdev_prop = prop.qdev_prop.as_ref().unwrap();
+ let bitnr = prop.bitnr.as_ref().unwrap_or(&zero);
+ let set_default = prop.defval.is_some();
+ let defval = prop.defval.as_ref().unwrap_or(&zero);
+ if let Some(assert_type) = prop.assert_type {
+ assertions.push(quote_spanned! {field_span=>
+ ::qemu_api::assert_field_type! ( #name, #field_name, #assert_type );
+ });
+ }
+ properties_expanded.push(quote_spanned! {field_span=>
+ ::qemu_api::bindings::Property {
+ // use associated function syntax for type checking
+ name: ::std::ffi::CStr::as_ptr(#prop_name),
+ info: unsafe { &#qdev_prop },
+ offset: ::core::mem::offset_of!(#name, #field_name) as isize,
+ bitnr: #bitnr,
+ set_default: #set_default,
+ defval: ::qemu_api::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
+ ..::qemu_api::zeroable::Zeroable::ZERO
+ }
+ });
+ }
+ let properties_expanded = quote_spanned! {span.into()=>
+ #(#assertions)*
+
+ impl ::qemu_api::qdev::DevicePropertiesImpl for #name {
+ fn properties() -> &'static [::qemu_api::bindings::Property] {
+ static PROPERTIES: &'static [::qemu_api::bindings::Property] = &[#(#properties_expanded),*];
+
+ PROPERTIES
+ }
+ }
+ };
+
+ TokenStream::from(properties_expanded)
+}
+
#[proc_macro_derive(Wrapper)]
pub fn derive_opaque(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 1279d7a58d50e1bf6c8d2e6f00d7229bbb19e003..2fd8b2750ffabcaa1065558d38a700e35fbc9136 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -100,8 +100,19 @@ pub trait ResettablePhasesImpl {
T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
}
+pub trait DevicePropertiesImpl {
+ /// An array providing the properties that the user can set on the
+ /// device. Not a `const` because referencing statics in constants
+ /// is unstable until Rust 1.83.0.
+ fn properties() -> &'static [Property] {
+ &[]
+ }
+}
+
/// Trait providing the contents of [`DeviceClass`].
-pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
+pub trait DeviceImpl:
+ ObjectImpl + ResettablePhasesImpl + DevicePropertiesImpl + IsA<DeviceState>
+{
/// _Realization_ is the second stage of device creation. It contains
/// all operations that depend on device properties and can fail (note:
/// this is not yet supported for Rust devices).
@@ -110,13 +121,6 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
/// with the function pointed to by `REALIZE`.
const REALIZE: Option<fn(&Self)> = None;
- /// An array providing the properties that the user can set on the
- /// device. Not a `const` because referencing statics in constants
- /// is unstable until Rust 1.83.0.
- fn properties() -> &'static [Property] {
- &[]
- }
-
/// A `VMStateDescription` providing the migration format for the device
/// Not a `const` because referencing statics in constants is unstable
/// until Rust 1.83.0.
@@ -171,7 +175,7 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
self.vmsd = vmsd;
}
- let prop = <T as DeviceImpl>::properties();
+ let prop = <T as DevicePropertiesImpl>::properties();
if !prop.is_empty() {
unsafe {
bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index a658a49fcfdda8fa4b9d139c10afb6ff3243790b..e8eadfd6e9add385ffc97de015b84aae825c18ee 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -9,7 +9,7 @@
cell::{self, BqlCell},
declare_properties, define_property,
prelude::*,
- qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
+ qdev::{DeviceImpl, DevicePropertiesImpl, DeviceState, Property, ResettablePhasesImpl},
qom::{ObjectImpl, ParentField},
sysbus::SysBusDevice,
vmstate::VMStateDescription,
@@ -68,10 +68,13 @@ impl ObjectImpl for DummyState {
impl ResettablePhasesImpl for DummyState {}
-impl DeviceImpl for DummyState {
+impl DevicePropertiesImpl for DummyState {
fn properties() -> &'static [Property] {
&DUMMY_PROPERTIES
}
+}
+
+impl DeviceImpl for DummyState {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE)
}
@@ -85,6 +88,8 @@ pub struct DummyChildState {
qom_isa!(DummyChildState: Object, DeviceState, DummyState);
+impl DevicePropertiesImpl for DummyChildState {}
+
pub struct DummyChildClass {
parent_class: <DummyState as ObjectType>::Class,
}
---
base-commit: 2af4a82ab2cce3412ffc92cd4c96bd870e33bc8e
change-id: 20250522-rust-qdev-properties-728e8f6a468e
--
γαῖα πυρί μιχθήτω
next reply other threads:[~2025-05-22 8:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-22 8:12 Manos Pitsidianakis [this message]
2025-05-22 11:17 ` [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro Zhao Liu
2025-05-23 13:05 ` Paolo Bonzini
2025-05-28 10:49 ` Manos Pitsidianakis
2025-05-28 11:38 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250522-rust-qdev-properties-v1-1-5b18b218bad1@linaro.org \
--to=manos.pitsidianakis@linaro.org \
--cc=alex.bennee@linaro.org \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-rust@nongnu.org \
--cc=zhao1.liu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).